Greg -- Thanks for the comments.
I am sending updated TI USB 3410/5052 driver patches in the next three messages (the one big patch hasn't yet made it through to the list). The patch is also available at http://www.brimson.com/downloads/ti_usb_3410_5052_2.6.10-rc2-bk13.patch These patches are against 2.6.10-rc2-bk13. Here are my responses for the couple of things I did not change. 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 nor to copy data into or out of a circular buffer. 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. 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. >>+/* 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. 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. >>+++ 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. Please apply. Thanks, -- Al ------------------------------------------------------- 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
