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