From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Kci1k-0001aO-0u for qemu-devel@nongnu.org; Mon, 08 Sep 2008 10:47:40 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1Kci1i-0001YM-8c for qemu-devel@nongnu.org; Mon, 08 Sep 2008 10:47:39 -0400 Received: from [199.232.76.173] (port=37617 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Kci1i-0001YA-4D for qemu-devel@nongnu.org; Mon, 08 Sep 2008 10:47:38 -0400 Received: from mail.windriver.com ([147.11.1.11]:51999 helo=mail.wrs.com) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1Kci1h-0006uz-UU for qemu-devel@nongnu.org; Mon, 08 Sep 2008 10:47:38 -0400 Message-ID: <48C53B04.9030006@windriver.com> Date: Mon, 08 Sep 2008 09:47:32 -0500 From: Jason Wessel MIME-Version: 1.0 References: <1220571341.2638.6.camel@hephaestion> <1220580385.2638.15.camel@hephaestion> <48C1346F.3000405@windriver.com> <1220640699.5470.15.camel@hephaestion> <48C1862C.3050307@windriver.com> <1220649226.9611.13.camel@hephaestion> In-Reply-To: <1220649226.9611.13.camel@hephaestion> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [PATCH] Add USB sys file-system support (v3) Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: TJ Cc: qemu-devel@nongnu.org, kvm TJ, The v3 usb patch looks good to me and works well. When used in conjunction with ioctl() work around I mentioned before, I was able to use QEMU to quickly find and fix a 2 different 2.6.27 usb serial regressions. :-) It would be great to push this into the qemu development tree. I see two minor problems with your patch for pushing it into the qemu development tree. 1) Some white space issues - In the patch it has mixed spaces and tabs - The tabs should be 8 spaces - This qemu source file has the standard 4 space indentation 2) Minor printf() clean ups The printf() on line 614 should be a dprintf() IE: printf("husb: open device %d.%d\n", bus_num, addr); - I realize this was not one that you added in your patch, but I looked at the call stack to see the prior ways this was used, all the messages of this type were dprintf() This comment is also based on the printf() you added later in the same function. The printf() on line 627 should be a term_printf() assuming you deem it is important to provide this up to the end user. IE: printf("husb: config #%d need %d\n", dev->descr[i + 5], configuration); Reviewed-by: Jason Wessel Fixing those two minor issues, I would consider the patch ready to commit. Cheers, Jason. TJ wrote: > > ======= > > This patch adds support for host USB devices discovered via: > > /sys/bus/usb/devices/* and opened from /dev/bus/usb/*/* > /dev/bus/usb/devices and opened from /dev/bus/usb/*/* > > in addition to the existing discovery via: > > /proc/bus/usb/devices and opened from /proc/bus/usb/*/* > > Signed-off-by: TJ > --- > qemu/usb-linux.c | 369 +++++++++++++++++++++++++++++++++++++++++++----------- > 1 files changed, 294 insertions(+), 75 deletions(-)