Message-ID: <4FA519B3.9030901@fold.natur.cuni.cz>
Date: Sat, 05 May 2012 14:14:43 +0200
From: Martin Mokrejs <mmokrejs@fold.natur.cuni.cz>
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:11.0) Gecko/20120319 Firefox/11.0 SeaMonkey/2.8
MIME-Version: 1.0
To: Paul Bolle <pebolle@tiscali.nl>
CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>, linux-usb@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH] usb: also announce bcdDevice
References: <1336218719.8450.19.camel@x61.thuisdomein>
In-Reply-To: <1336218719.8450.19.camel@x61.thuisdomein>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 3370
Lines: 72

Paul Bolle wrote:
> Currently announce_device() does print the idVendor and idProduct values
> but does not print the bcdDevice value. USB devices are accurately
> identified by all three values. See, for instance, the USB storage
> quirks which will only apply for a certain (range of) bcdDevice
> value(s). So it seems useful to also print bcdDevice when announcing USB
> devices.

Could it also report negotiated speed? full-speed, high-speed, super-speed?
Thanks,
Martin

> 
> Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
> ---
> 0) This is something I ran into while trying to track down the log
> errors generated by each of the USB sticks I happen to have lying
> around. Of course, there are other ways to track down the bcdDevice
> value of a specific device ("lusb -v -d $vendorid:$productid | grep
> bcdDevice" or "usb-devices | grep $vendorid.*$productid" come to mind).
> But since idVendor and idProduct are already printed by this debugging
> aid I think adding bcdDevice makes sense too. But is this all worth the
> small bit of additional noise?
> 
> 1) The patch generates a checkpatch warning: "quoted string split across
> lines". But since it is hard to grep for this entire string without
> knowing the printk "conversion specifications" in the string beforehand,
> I think the warning can be ignored here. Note that I actually found this
> string by git grepping for just "New USB device found".
> 
>  drivers/usb/core/Kconfig |    4 ++--
>  drivers/usb/core/hub.c   |    6 ++++--
>  2 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/usb/core/Kconfig b/drivers/usb/core/Kconfig
> index 18d02e3..85f795b 100644
> --- a/drivers/usb/core/Kconfig
> +++ b/drivers/usb/core/Kconfig
> @@ -14,8 +14,8 @@ config USB_ANNOUNCE_NEW_DEVICES
>  	depends on USB
>  	default N
>  	help
> -	  Say Y here if you want the USB core to always announce the
> -	  idVendor, idProduct, Manufacturer, Product, and SerialNumber
> +	  Say Y here if you want the USB core to always announce the idVendor,
> +	  idProduct, bcdDevice, Manufacturer, Product, and SerialNumber
>  	  strings for every new USB device to the syslog.  This option is
>  	  usually used by distro vendors to help with debugging and to
>  	  let users know what specific device was added to the machine
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index ec6c97d..6e1bfaea 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -1726,9 +1726,11 @@ static void show_string(struct usb_device *udev, char *id, char *string)
>  
>  static void announce_device(struct usb_device *udev)
>  {
> -	dev_info(&udev->dev, "New USB device found, idVendor=%04x, idProduct=%04x\n",
> +	dev_info(&udev->dev, "New USB device found, idVendor=%04x, "
> +		"idProduct=%04x, bcdDevice=%04x\n",
>  		le16_to_cpu(udev->descriptor.idVendor),
> -		le16_to_cpu(udev->descriptor.idProduct));
> +		le16_to_cpu(udev->descriptor.idProduct),
> +		le16_to_cpu(udev->descriptor.bcdDevice));
>  	dev_info(&udev->dev,
>  		"New USB device strings: Mfr=%d, Product=%d, SerialNumber=%d\n",
>  		udev->descriptor.iManufacturer,
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/