Am Freitag, 19. Januar 2007 16:31 schrieb Alan Stern:
> On Fri, 19 Jan 2007, Oliver Neukum wrote:
>
> > Hi,
> >
> > I did the dirty deed. Could you please tell me which quirky devices
> > you know of? I am afraid this list needs to be populated and there
> > are outstanding regressions in Adrian's list.
>
> I'll get back to you on the devices.
>
> I think this patch could be generalized. We may have other quirks to
> worry about besides PM/autosuspend. This should become a general USB
> blacklist mechanism.
If you want the grand solution, you shall get it.
> One example that comes up immediately is the HP scanner which crashes
> when we try to read the string descriptors. Others may crop up in the
> future.
Do you have the ID? Searching hasn't yielded what I need.
> > --- a/include/linux/usb.h 2007-01-18 13:15:25.000000000 +0100
> > +++ b/include/linux/usb.h 2007-01-19 12:12:07.000000000 +0100
> > @@ -387,6 +387,7 @@
> > struct usb_device *children[USB_MAXCHILDREN];
> >
> > int pm_usage_cnt; /* usage counter for autosuspend */
> > + int pm_quirks; /* quirks of power management */
>
> So this could simply be "quirks", not "pm_quirks", with a corresponding
> change to the comment.
Yes.
> > +/* Power management quirks */
> > +#define USB_PM_DISCONNECT 1 /* devices which disconnect when resuming */
>
> Make the name USB_QUIRK_NO_AUTOSUSPEND. We don't really care that the
> device disconnects when resuming; the only important part is that we
> should never try to autosuspend it.
This seems contradictory. If you want a generic blacklisting, you cannot
limit the information to autosuspending. The device won't care why it
is suspended. Such devices should be treated differently under certain
situations. Either refuse to suspend or disconnect when asked to suspend.
> The list of quirk flag definitions doesn't belong in include/linux/usb.h.
> We could put it in drivers/usb/core/usb.h or drivers/usb/core/quirks.h.
> Or if you think that some USB drivers might eventually want to use a
> quirk, you could put the definitions in include/linux/usb/quirks.h.
OK.
>
> I dislike seeing this in hub.c. That file is already too big, and anyway
> managing a blacklist has nothing to do with being a hub driver. You
> should create a new source file: drivers/usb/core/quirks.c. Put both the
> search routine and the initializer for the quirks array in the new file.
OK. It seems better to me to seperate code, quirk types and the
blacklist itself.
Regards
Oliver
Signed-off-by: Oliver Neukum <[EMAIL PROTECTED]>
----
--- /dev/null 2007-01-15 18:37:01.000000000 +0100
+++ b/include/linux/usb/quirks.h 2007-01-24 18:44:42.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_AUTOSUSPEND_DISCONNECT 1 /* devices which disconnect
when resuming */
+#define USB_QUIRK_STRING_FETCH_CRASH 2 /* devices which crash when
string descriptors are fetched */
--- /dev/null 2007-01-15 18:37:01.000000000 +0100
+++ b/drivers/usb/core/quirks.c 2007-01-24 17:42:18.000000000 +0100
@@ -0,0 +1,45 @@
+/*
+ * drivers/usb/core/quirks.c
+ *
+ * (c) Copyright Oliver Neukum 2007
+ */
+
+#include <linux/usb.h>
+#include <linux/usb/quirks.h>
+#include "usb.h"
+#include "quirky_devices.h"
+
+static int usb_autosuspend_search_quirk(struct usb_device *udev)
+{
+ u16 vendor = le16_to_cpu(udev->descriptor.idVendor);
+ u16 product = le16_to_cpu(udev->descriptor.idProduct);
+ u16 version = le16_to_cpu(udev->descriptor.bcdDevice);
+
+ const struct usb_blacklist *blentry = usb_blacklist;
+
+ while (blentry->idVendor) {
+ if (blentry->idVendor == vendor &&
+ blentry->idProduct == product &&
+ blentry->bcdVersionMin <= version &&
+ blentry->bcdVersionMax >= version)
+ return blentry->quirks;
+ blentry++;
+ }
+
+ return 0;
+}
+
+/*
+ * detect any quirks the device as opposed to the interfaces the device has
+ */
+
+void usb_detect_quirks(struct usb_device *udev)
+{
+ udev->quirks = usb_autosuspend_search_quirk(udev);
+
+ if (udev->quirks & USB_QUIRK_AUTOSUSPEND_DISCONNECT) {
+ /* unbalanced resume that'll prevent autosuspension */
+ usb_autoresume_device(udev);
+ dev_dbg(&udev->dev, "USB power management quirks: %d\n",
udev->quirks);
+ }
+}
--- /dev/null 2007-01-15 18:37:01.000000000 +0100
+++ b/drivers/usb/core/quirky_devices.h 2007-01-24 18:51:14.000000000 +0100
@@ -0,0 +1,22 @@
+/*
+ * This is a list of USB devices which are quirky
+ * Keep this list ordered by
+ * 1. Vendor
+ * 2. Product
+ * 3. Version, the most specific first
+ */
+
+
+static const struct usb_blacklist {
+ u16 idVendor; /* vendor id of the quirky device */
+ u16 idProduct; /* product id of the quirky device */
+ u16 bcdVersionMin; /* minimum version id of the afflicted devices */
+ u16 bcdVersionMax; /* maximum version id of the afflicted devices */
+ int quirks; /* the actual qirks, several to be joined with binary
or */
+} usb_blacklist[] = {
+ /* Elsa MicroLink 56k (V.250) */
+ {0x05cc, 0x2267, 0x0100, 0x0100, USB_QUIRK_AUTOSUSPEND_DISCONNECT},
+ {0, 0, 0, 0, 0} /* keep this last */
+};
--- a/drivers/usb/core/usb.h 2007-01-23 15:16:33.000000000 +0100
+++ b/drivers/usb/core/usb.h 2007-01-24 17:39:42.000000000 +0100
@@ -13,6 +13,7 @@
struct usb_interface *intf);
extern void usb_release_interface_cache(struct kref *ref);
extern void usb_disable_device (struct usb_device *dev, int skip_ep0);
+extern void usb_detect_quirks(struct usb_device *udev);
extern int usb_get_device_descriptor(struct usb_device *dev,
unsigned int size);
--- a/drivers/usb/core/Makefile 2007-01-23 15:15:04.000000000 +0100
+++ b/drivers/usb/core/Makefile 2007-01-24 10:51:45.000000000 +0100
@@ -4,7 +4,7 @@
usbcore-objs := usb.o hub.o hcd.o urb.o message.o driver.o \
config.o file.o buffer.o sysfs.o endpoint.o \
- devio.o notify.o generic.o
+ devio.o notify.o generic.o quirks.o
ifeq ($(CONFIG_PCI),y)
usbcore-objs += hcd-pci.o
--- a/drivers/usb/core/hub.c 2007-01-23 15:16:33.000000000 +0100
+++ b/drivers/usb/core/hub.c 2007-01-24 10:09:14.000000000 +0100
@@ -1278,6 +1278,9 @@
if (!try_module_get(THIS_MODULE))
return -EINVAL;
+ /* Determine quirks */
+ usb_detect_quirks(udev);
+
err = usb_get_configuration(udev);
if (err < 0) {
dev_err(&udev->dev, "can't read configurations, error %d\n",
--- a/drivers/usb/core/message.c 2007-01-23 15:16:33.000000000 +0100
+++ b/drivers/usb/core/message.c 2007-01-25 10:30:21.000000000 +0100
@@ -11,6 +11,7 @@
#include <linux/timer.h>
#include <linux/ctype.h>
#include <linux/device.h>
+#include <linux/usb/quirks.h>
#include <asm/byteorder.h>
#include <asm/scatterlist.h>
@@ -810,6 +811,8 @@
char *smallbuf = NULL;
int len;
+ if (udev->quirks & USB_QUIRK_STRING_FETCH_CRASH)
+ return NULL;
if (index > 0 && (buf = kmalloc(256, GFP_KERNEL)) != NULL) {
if ((len = usb_string(udev, index, buf, 256)) > 0) {
if ((smallbuf = kmalloc(++len, GFP_KERNEL)) == NULL)
--- a/include/linux/usb.h 2007-01-23 15:16:33.000000000 +0100
+++ b/include/linux/usb.h 2007-01-24 10:12:25.000000000 +0100
@@ -388,6 +388,7 @@
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 */
@@ -1485,3 +1486,4 @@
#endif /* __KERNEL__ */
#endif
+
-------------------------------------------------------------------------
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
