From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jason Wessel Subject: Re: [PATCH] Add USB sys file-system support (v3) Date: Mon, 08 Sep 2008 09:47:32 -0500 Message-ID: <48C53B04.9030006@windriver.com> 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> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: qemu-devel@nongnu.org, kvm To: TJ Return-path: Received: from mail.windriver.com ([147.11.1.11]:52102 "EHLO mail.wrs.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753321AbYIHOrl (ORCPT ); Mon, 8 Sep 2008 10:47:41 -0400 In-Reply-To: <1220649226.9611.13.camel@hephaestion> Sender: kvm-owner@vger.kernel.org List-ID: 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(-)