Re: [Qemu-devel] [PATCH 26/26] usbredir: Add support for buffered bulk input

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 12/19/2012 10:51 AM, Gerd Hoffmann wrote:
>    Hi,
>
>> These 2 are 1 on 1 copies from the kernel:
>>
>> hw/usb/redirect-ftdi-ids.h   | 1255
>> ++++++++++++++++++++++++++++++++++++++++++
>> hw/usb/redirect-pl2303-ids.h |  150 +++++
>>
>> They contain "boring" stuff like:
>>
>> #define FTDI_8U232AM_PID 0x6001 /* Similar device to SIO above */
>> #define FTDI_8U232AM_ALT_PID 0x6006 /* FTDI's alternate PID for above */
>
> We have header files copyed from linux already (linux-headers/), did you
> look into placing them there?  Could be it doesn't work that easily due
> to internal / user interface split of the kernel headers, but worth
> checking ...

That seem to be only headers from under linux/include. And not from
under linux/drivers/usb/.... like I'm doing, so for now I'm just
going to leave the headers in hw/usb.

>
>> This one:
>> hw/usb/redirect-usb-ids.h
>>
>> Contains the usb-id tables copied from the kernel, as stated
>> in the comments:
>
> Manual process or scripted?  If case of the latter we should put the
> scripts into scripts/ for easy future updates.
>
>> So updating them can be done by emptying the list and then copying in
>> the new list from the latest kernel, this is an (easy) manual procedure
>> for now.
>
> Ah, manual.  ok.
>
>>> I also think this shouldn't be tied to redir, I think it is better to
>>> have a hw/usb/quirks.c file where the device id database and helper
>>> functions to match devices against the list are living.
>>
>> Makes sense, one problem I see though is that right now I've
>> 2 id tables, named usbredir_raw_serial_ids and usbredir_ftdi_serial_ids
>> since ftdi based adapters need some extra special handling (a quirk
>> to the quirk).
>>
>> Once we start working with quirk tables it makes sense to have 1 large
>> table with an extra uint32_t field which contains the actual quirks as
>> bitmask. All perfectly sensible, but this breaks the easy copy and
>> paste syncing of the tables from the Linux kernel ...
>>
>> So any good ideas for this from you ?
>
> With a scripted update we could add the bitmask easily I guess.
>
> But I think for now we can stick to two lists.  Just make sure
> usb-quirks.c has a sensible interface.  Pass in device ids+class, get
> back a quicks bitmask.  Then we can create the bitmask by just checking
> which list has the device in question.
>
> And when we figure some day this becomes too messy we can switch to a
> model where where we have the quirk bits next to the devices in the
> table and just do a lookup, without users noticing the change.

Ok, I'll send a new version with this implemented.

Regards,

Hans