On Fri, 26 Jan 2007, Oliver Neukum wrote:
> Hi,
>
> this is the newest version of the patch to introduce a blacklist.
>
> - It handles autosuspend oddities and problems with string descriptors
> - It exports the detected quirks through sysfs
>
> It compiles and detects quirky devices.
>
> Regards
> Oliver
This is very good. I have only a few small comments.
> --- /dev/null 2007-01-15 18:37:01.000000000 +0100
> +++ linux-blacklist/include/linux/usb/quirks.h 2007-01-26
> 13:49:08.000000000 +0100
> @@ -0,0 +1,8 @@
> +/*
> + * This file holds the definitions of quirks found in devices
> + * Only quirks that affect the whole device, not an interface,
> + * belong here
> + */
> +
> +#define USB_QUIRK_NO_AUTOSUSPEND 1 /* devices which must not be
> autosuspended */
> +#define USB_QUIRK_STRING_FETCH 2 /* devices which
> crash when string descriptors are fetched */
The comment should simply say "don't fetch the string descriptors". It
doesn't have to be the case that doing so will crash the device. For
example, that HP scanner used to work with an older version of uhci-hcd,
and it would work even now if one of its descriptors wasn't exactly 32
bytes long.
If you really want to get fancy, note that we use two schemes for reading
string descriptors. The first scheme sends a 255-byte request for the
whole descriptor. If that doesn't work, the second scheme asks for the
2-byte descriptor header and then sends a request using the descriptor's
exact length. (See messages.c:usb_string_sub.) I bet the HP scanner
would work fine if we simply skipped the first scheme -- that's what
crashes the device.
> struct usb_device *children[USB_MAXCHILDREN];
>
> int pm_usage_cnt; /* usage counter for autosuspend */
> + int quirks; /* quirks of the whole device */
> #ifdef CONFIG_PM
> struct delayed_work autosuspend; /* for delayed autosuspends */
> struct mutex pm_mutex; /* protects PM operations */
> @@ -1460,3 +1461,4 @@
> #endif /* __KERNEL__ */
>
> #endif
> +
Extra blank line added for no reason.
> --- linux-2.6.20-rc6/drivers/usb/core/sysfs.c 2007-01-25 11:36:31.000000000
> +0100
> +++ linux-blacklist/drivers/usb/core/sysfs.c 2007-01-26 09:36:23.000000000
> +0100
> @@ -148,6 +148,16 @@
> }
> static DEVICE_ATTR(maxchild, S_IRUGO, show_maxchild, NULL);
>
> +static ssize_t
> +show_quirks(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + struct usb_device *udev;
> +
> + udev = to_usb_device(dev);
> + return sprintf(buf, "%d\n", udev->quirks);
In hex, please.
> +void usb_detect_quirks(struct usb_device *udev)
> +{
> + udev->quirks = usb_autosuspend_search_quirk(udev);
> +
> + if (udev->quirks & USB_QUIRK_NO_AUTOSUSPEND) {
> + /* unbalanced resume that'll prevent autosuspension */
> + usb_autoresume_device(udev);
> + dev_dbg(&udev->dev, "USB power management quirks: %d\n",
> udev->quirks);
Alternative 1: Change to "USB autosuspend quirk\n".
Alternative 2: Change to "USB device quirks: %x\n" and make it conditional
on (udev->quirks) instead of (udev->quirks & USB_QUIRK_NO_AUTOSUSPEND).
Alan Stern
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
