On Wed, 14 Jan 2009 20:59:41 -0600
Kyle Guinn <elyk03@xxxxxxxxx> wrote:
> gspca: Add MR97310A driver
>
> From: Kyle Guinn <elyk03@xxxxxxxxx>
>
> This patch adds support for USB webcams based on the MR97310A chip.
> It was tested with an Aiptek PenCam VGA+ webcam.
Hi again,
Here are some remarks about your patch.
[snip]
> +/* the bytes to write are in gspca_dev->usb_buf */
> +static int reg_w(struct gspca_dev *gspca_dev,
> + __u16 index, int len)
> +{
> + int rc;
> +
> + rc = usb_bulk_msg(gspca_dev->dev,
> + usb_sndbulkpipe(gspca_dev->dev, 4),
> + gspca_dev->usb_buf, len, 0, 500);
> + if (rc < 0)
> + PDEBUG(D_ERR, "reg write [%02x] error %d", index,
> rc);
> + return rc;
> +}
The 'index' parameter is not useful: the register is always in the first
byte of the buffer.
[snip]
> +/* this function is called at probe time */
> +static int sd_config(struct gspca_dev *gspca_dev,
> + const struct usb_device_id *id)
> +{
> + struct cam *cam;
> +
> + cam = &gspca_dev->cam;
> + cam->epaddr = 0x01;
This variable has been removed in the last versions of gspca.
> + cam->cam_mode = vga_mode;
> + cam->nmodes = ARRAY_SIZE(vga_mode);
> + return 0;
> +}
[snip]
> +static int sd_start(struct gspca_dev *gspca_dev)
> +{
> + struct sd *sd = (struct sd *) gspca_dev;
> + __u8 *data = gspca_dev->usb_buf;
> + int err_code;
> + int intpipe;
> +
> + PDEBUG(D_STREAM, "camera start, iface %d, alt 8",
> gspca_dev->iface);
> + err_code = usb_set_interface(gspca_dev->dev,
> gspca_dev->iface, 8);
> + if (err_code < 0) {
> + PDEBUG(D_ERR|D_STREAM, "Set packet size: set
> interface error");
> + return err_code;
> + }
The usb_set_interface() is already done in the gspca main.
[snip]
> + sd->sof_read = 0;
> +
> + intpipe = usb_sndintpipe(gspca_dev->dev, 0);
> + err_code = usb_clear_halt(gspca_dev->dev, intpipe);
Is this really needed?
> + data[0] = 0x00;
> + data[1] = 0x4d; /* ISOC transfering enable... */
> + reg_w(gspca_dev, data[0], 2);
> + return err_code;
> +}
[snip]
--
Ken ar c'hentan | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
