On Thu, Dec 02, 2004 at 11:00:26AM -0600, Al Borchers wrote:
> Greg KH wrote:
> > Why not use the in-kernel implemtation of a circular buffer? I've been
> > thinking about that for the other usb-serial drivers that also roll
> > their own data structure for this. We shouldn't add yet-another one if
> > we can help it.
>
> I agree. I wish the kernel circ_buf.h would do what I want, but it
> doesn't. circ_buf.h does not provide functions to allocate a buffer
That's up to the caller.
> nor to copy data into or out of a circular buffer.
That's up to the caller too.
> Also, circ_buf.h
> does not include the buffer size in the struct, so either you have to
> pass an extra argument with the buffer size to each function or use a
> global/constant for the size--neither of which I like.
But that's how everyone else who uses it does it.
> As a separate project, I could submit an enhanced circ_buf.h that includes
> these new functions and the size in the struct, and would still be backward
> compatible, if you think that is a good idea.
I think it's probably better to start by using the existing data
structure, instead of rolling your own. Then provide a "better" one for
this driver, and others to use if they so desire.
Sound good?
>
> >>+/* supported devices */
> >>+static struct usb_device_id ti_id_table_3410[1+TI_EXTRA_VID_PID_COUNT+1] =
> >>{
> >>+ { USB_DEVICE(TI_VENDOR_ID, TI_3410_PRODUCT_ID) },
> >>+};to
> >
> >
> > Why have a number for the array size?
> >
> >>+static struct usb_device_id ti_id_table_5052[4+TI_EXTRA_VID_PID_COUNT+1] =
> >>{
> >>+ { USB_DEVICE(TI_VENDOR_ID, TI_5052_BOOT_PRODUCT_ID) },
> >>+ { USB_DEVICE(TI_VENDOR_ID, TI_5152_BOOT_PRODUCT_ID) },
> >>+ { USB_DEVICE(TI_VENDOR_ID, TI_5052_EEPROM_PRODUCT_ID) },
> >>+ { USB_DEVICE(TI_VENDOR_ID, TI_5052_FIRMWARE_PRODUCT_ID) },
> >>+};
> >
> >
> > Same here?
> >
> > Ah, you are trying to do what the visor driver did in adding a new
> > device id. Just put two empty '{ }' at the end of the structure then.
>
> I added a comment to explain. There are 4 default ids, TI_EXTRA_VID_PID_COUNT
> user defined ids (currently 5), and 1 trailing null.
Ok.
> TI expects their chip customers to customize the vendor/product ids in
> firmware,
> so they want the driver to support multiple user defined vendor/product ids.
> This driver supports TI_EXTRA_VID_PID_COUNT user defined ids. That constant
> is
> currently 5, but could be changed easily. Using {}. {}, {}, {}, {}, {}, at
> the
> end of the array would make it harder to change.
I really want to move this kind of functionality into the USB core, like
PCI devices have. But time is needed for us to get there...
> >>+++ linux-2.6.10-rc2-bk3.new/drivers/usb/serial/ti_usb_3410_5052.h
> >
> > Is another header file really needed?
>
> The constants and structs in the header come from TI's device and firmware
> documentation and are independant of the driver implementation. The driver
> implementation specific constants and structs are in the driver .c file. So
> there is a reason for the separate header.
Ok, that's fair enough.
> Please apply.
Care to redo the circ_buf stuff first?
thanks,
greg k-h
-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://productguide.itmanagersjournal.com/
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
