From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:33278) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TlIFB-0002N7-2X for qemu-devel@nongnu.org; Wed, 19 Dec 2012 06:55:26 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TlIF9-0002HL-RW for qemu-devel@nongnu.org; Wed, 19 Dec 2012 06:55:24 -0500 Received: from mx1.redhat.com ([209.132.183.28]:18722) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TlI6B-00008Z-4N for qemu-devel@nongnu.org; Wed, 19 Dec 2012 06:46:07 -0500 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id qBJBk5JL011878 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Wed, 19 Dec 2012 06:46:05 -0500 Message-ID: <50D1A98A.5070707@redhat.com> Date: Wed, 19 Dec 2012 12:48:26 +0100 From: Hans de Goede MIME-Version: 1.0 References: <1355492147-5023-1-git-send-email-hdegoede@redhat.com> <1355492147-5023-27-git-send-email-hdegoede@redhat.com> <50D0717B.7040103@redhat.com> <50D18895.3040201@redhat.com> <50D18E2D.9050905@redhat.com> In-Reply-To: <50D18E2D.9050905@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 26/26] usbredir: Add support for buffered bulk input List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gerd Hoffmann Cc: qemu-devel@nongnu.org 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