linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] usb: core: Add "quirks" parameter for usbcore
@ 2018-02-25 12:38 Kai-Heng Feng
  2018-02-25 15:18 ` Matthew Wilcox
  2018-02-26 22:30 ` kbuild test robot
  0 siblings, 2 replies; 4+ messages in thread
From: Kai-Heng Feng @ 2018-02-25 12:38 UTC (permalink / raw)
  To: gregkh; +Cc: oneukum, corbet, linux-doc, linux-kernel, linux-usb, Kai-Heng Feng

Trying quirks in usbcore needs to rebuild the driver or the entire
kernel if it's builtin. It can save a lot of time if usbcore has similar
ability like "usbhid.quirks=" and "usb-storage.quirks=".

Rename the original quirk detection function to "static" as we introduce
this new "dynamic" function.

Now users can use "usbcore.quirks=" as short term workaround before the
next kernel release. Also, the quirk parameter can XOR the builtin
quirks for debugging purpose.

This is inspired by usbhid and usb-storage.

Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
v3: Stop overridding static quirks.
    Use XOR for dynamic quirks.
    Save parsed quirk as a list to avoid parsing quirk table everytime.

v2: Use in-kernel tolower() function.

 Documentation/admin-guide/kernel-parameters.txt |  55 ++++++++
 drivers/usb/core/quirks.c                       | 160 +++++++++++++++++++++++-
 drivers/usb/core/usb.c                          |   1 +
 drivers/usb/core/usb.h                          |   1 +
 4 files changed, 212 insertions(+), 5 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 1d1d53f85ddd..70a7398c20e2 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -4368,6 +4368,61 @@
 
 	usbcore.nousb	[USB] Disable the USB subsystem
 
+	usbcore.quirks=
+			[USB] A list of quirks entries to supplement or
+			override the built-in usb core quirk list.  List
+			entries are separated by commas.  Each entry has
+			the form VID:PID:Flags where VID and PID are Vendor
+			and Product ID values (4-digit hex numbers) and
+			Flags is a set of characters, each corresponding
+			to a common usb core quirk flag as follows:
+				a = USB_QUIRK_STRING_FETCH_255 (string
+					descriptors must not be fetched using
+					a 255-byte read);
+				b = USB_QUIRK_RESET_RESUME (device can't resume
+					correctly so reset it instead);
+				c = USB_QUIRK_NO_SET_INTF (device can't handle
+					Set-Interface requests);
+				d = USB_QUIRK_CONFIG_INTF_STRINGS (device can't
+					handle its Configuration or Interface
+					strings);
+				e = USB_QUIRK_RESET (device can't be reset
+					(e.g morph devices), don't use reset);
+				f = USB_QUIRK_HONOR_BNUMINTERFACES (device has
+					more interface descriptions than the
+					bNumInterfaces count, and can't handle
+					talking to these interfaces);
+				g = USB_QUIRK_DELAY_INIT (device needs a pause
+					during initialization, after we read
+					the device descriptor);
+				h = USB_QUIRK_LINEAR_UFRAME_INTR_BINTERVAL (For
+					high speed and super speed interrupt
+					endpoints, the USB 2.0 and USB 3.0 spec
+					require the interval in microframes (1
+					microframe = 125 microseconds) to be
+					calculated as interval = 2 ^
+					(bInterval-1).
+					Devices with this quirk report their
+					bInterval as the result of this
+					calculation instead of the exponent
+					variable used in the calculation);
+				i = USB_QUIRK_DEVICE_QUALIFIER (device can't
+					handle device_qualifier descriptor
+					requests);
+				j = USB_QUIRK_IGNORE_REMOTE_WAKEUP (device
+					generates spurious wakeup, ignore
+					remote wakeup capability);
+				k = USB_QUIRK_NO_LPM (device can't handle Link
+					Power Management);
+				l = USB_QUIRK_LINEAR_FRAME_INTR_BINTERVAL
+					(Device reports its bInterval as linear
+					frames instead of the USB 2.0
+					calculation);
+				m = USB_QUIRK_DISCONNECT_SUSPEND (Device needs
+					to be disconnected before suspend to
+					prevent spurious wakeup)
+			Example: quirks=0781:5580:bk,0a5c:5834:gij
+
 	usbhid.mousepoll=
 			[USBHID] The interval which mice are to be polled at.
 
diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c
index 4024926c1d68..5e89097bceb8 100644
--- a/drivers/usb/core/quirks.c
+++ b/drivers/usb/core/quirks.c
@@ -11,6 +11,23 @@
 #include <linux/usb/hcd.h>
 #include "usb.h"
 
+static char quirks_param[128];
+module_param_string(quirks, quirks_param, sizeof(quirks_param), 0644);
+MODULE_PARM_DESC(quirks, "Add/modify USB quirks by specifying quirks=vendorID:productID:quirks");
+
+static char quirks_param_orig[128];
+
+static LIST_HEAD(quirk_list);
+
+static struct mutex quirk_mutex;
+
+struct quirk_entry {
+	u16 vid;
+	u16 pid;
+	u32 flags;
+	struct list_head list;
+};
+
 /* Lists of quirky USB devices, split in device quirks and interface quirks.
  * Device quirks are applied at the very beginning of the enumeration process,
  * right after reading the device descriptor. They can thus only match on device
@@ -305,6 +322,105 @@ static bool usb_match_any_interface(struct usb_device *udev,
 	return false;
 }
 
+static int usb_build_quirk_list(const char *quirks_param)
+{
+	char *param, *p, *field;
+	u16 vid, pid;
+	u32 flags;
+	struct quirk_entry *quirk;
+
+	usb_release_quirk_list();
+
+	param = vmalloc(strlen(quirks_param));
+	if (!param)
+		return -ENOMEM;
+
+	strcpy(param, quirks_param);
+	p = param;
+	while (p && *p) {
+		/* Each entry consists of VID:PID:flags */
+		field = strsep(&p, ":");
+		if (!field)
+			break;
+
+		if (kstrtou16(field, 16, &vid))
+			break;
+
+		field = strsep(&p, ":");
+		if (!field)
+			break;
+
+		if (kstrtou16(field, 16, &pid))
+			break;
+
+		field = strsep(&p, ",");
+		if (!field)
+			break;
+
+		/* Collect the flags */
+		for (flags = 0; *field; field++) {
+			switch (tolower(*field)) {
+			case 'a':
+				flags |= USB_QUIRK_STRING_FETCH_255;
+				break;
+			case 'b':
+				flags |= USB_QUIRK_RESET_RESUME;
+				break;
+			case 'c':
+				flags |= USB_QUIRK_NO_SET_INTF;
+				break;
+			case 'd':
+				flags |= USB_QUIRK_CONFIG_INTF_STRINGS;
+				break;
+			case 'e':
+				flags |= USB_QUIRK_RESET;
+				break;
+			case 'f':
+				flags |= USB_QUIRK_HONOR_BNUMINTERFACES;
+				break;
+			case 'g':
+				flags |= USB_QUIRK_DELAY_INIT;
+				break;
+			case 'h':
+				flags |= USB_QUIRK_LINEAR_UFRAME_INTR_BINTERVAL;
+				break;
+			case 'i':
+				flags |= USB_QUIRK_DEVICE_QUALIFIER;
+				break;
+			case 'j':
+				flags |= USB_QUIRK_IGNORE_REMOTE_WAKEUP;
+				break;
+			case 'k':
+				flags |= USB_QUIRK_NO_LPM;
+				break;
+			case 'l':
+				flags |= USB_QUIRK_LINEAR_FRAME_INTR_BINTERVAL;
+				break;
+			case 'm':
+				flags |= USB_QUIRK_DISCONNECT_SUSPEND;
+				break;
+			/* Ignore unrecognized flag characters */
+			}
+		}
+
+		quirk = vmalloc(sizeof(struct quirk_entry));
+		if (!quirk) {
+			vfree(param);
+			return -ENOMEM;
+		}
+
+		quirk->vid = vid;
+		quirk->pid = pid;
+		quirk->flags = flags;
+
+		list_add(&quirk->list, &quirk_list);
+	}
+
+	vfree(param);
+
+	return 0;
+}
+
 static int usb_amd_resume_quirk(struct usb_device *udev)
 {
 	struct usb_hcd *hcd;
@@ -317,8 +433,8 @@ static int usb_amd_resume_quirk(struct usb_device *udev)
 	return 0;
 }
 
-static u32 __usb_detect_quirks(struct usb_device *udev,
-			       const struct usb_device_id *id)
+static u32 usb_detect_static_quirks(struct usb_device *udev,
+				    const struct usb_device_id *id)
 {
 	u32 quirks = 0;
 
@@ -336,21 +452,45 @@ static u32 __usb_detect_quirks(struct usb_device *udev,
 	return quirks;
 }
 
+static u32 usb_detect_dynamic_quirks(struct usb_device *udev)
+{
+	u16 vid = le16_to_cpu(udev->descriptor.idVendor);
+	u16 pid = le16_to_cpu(udev->descriptor.idProduct);
+	struct quirk_entry *quirk;
+
+	mutex_lock(&quirk_mutex);
+	if (strcmp(quirks_param, quirks_param_orig) != 0) {
+		strcpy(quirks_param_orig, quirks_param);
+		if (usb_build_quirk_list(quirks_param))
+			dev_warn(&udev->dev, "failed to build quirk list");
+	}
+	mutex_unlock(&quirk_mutex);
+
+	list_for_each_entry(quirk, &quirk_list, list) {
+		if (vid == quirk->vid && pid == quirk->pid)
+			return quirk->flags;
+	}
+
+	return 0;
+}
+
 /*
  * Detect any quirks the device has, and do any housekeeping for it if needed.
  */
 void usb_detect_quirks(struct usb_device *udev)
 {
-	udev->quirks = __usb_detect_quirks(udev, usb_quirk_list);
+	udev->quirks = usb_detect_static_quirks(udev, usb_quirk_list);
 
 	/*
 	 * Pixart-based mice would trigger remote wakeup issue on AMD
 	 * Yangtze chipset, so set them as RESET_RESUME flag.
 	 */
 	if (usb_amd_resume_quirk(udev))
-		udev->quirks |= __usb_detect_quirks(udev,
+		udev->quirks |= usb_detect_static_quirks(udev,
 				usb_amd_resume_quirk_list);
 
+	udev->quirks ^= usb_detect_dynamic_quirks(udev);
+
 	if (udev->quirks)
 		dev_dbg(&udev->dev, "USB quirks for this device: %x\n",
 			udev->quirks);
@@ -369,7 +509,7 @@ void usb_detect_interface_quirks(struct usb_device *udev)
 {
 	u32 quirks;
 
-	quirks = __usb_detect_quirks(udev, usb_interface_quirk_list);
+	quirks = usb_detect_static_quirks(udev, usb_interface_quirk_list);
 	if (quirks == 0)
 		return;
 
@@ -377,3 +517,13 @@ void usb_detect_interface_quirks(struct usb_device *udev)
 		quirks);
 	udev->quirks |= quirks;
 }
+
+void usb_release_quirk_list(void)
+{
+	struct quirk_entry *quirk, *tmp;
+
+	list_for_each_entry_safe(quirk, tmp, &quirk_list, list) {
+		list_del(&quirk->list);
+		vfree(quirk);
+	}
+}
diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
index 2f5fbc56a9dd..0adb6345ff2e 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -1259,6 +1259,7 @@ static void __exit usb_exit(void)
 	if (usb_disabled())
 		return;
 
+	usb_release_quirk_list();
 	usb_deregister_device_driver(&usb_generic_driver);
 	usb_major_cleanup();
 	usb_deregister(&usbfs_driver);
diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h
index 149cc7480971..546a2219454b 100644
--- a/drivers/usb/core/usb.h
+++ b/drivers/usb/core/usb.h
@@ -36,6 +36,7 @@ extern void usb_deauthorize_interface(struct usb_interface *);
 extern void usb_authorize_interface(struct usb_interface *);
 extern void usb_detect_quirks(struct usb_device *udev);
 extern void usb_detect_interface_quirks(struct usb_device *udev);
+extern void usb_release_quirk_list(void);
 extern int usb_remove_device(struct usb_device *udev);
 
 extern int usb_get_device_descriptor(struct usb_device *dev,
-- 
2.15.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v3] usb: core: Add "quirks" parameter for usbcore
  2018-02-25 12:38 [PATCH v3] usb: core: Add "quirks" parameter for usbcore Kai-Heng Feng
@ 2018-02-25 15:18 ` Matthew Wilcox
  2018-02-25 15:45   ` Kai-Heng Feng
  2018-02-26 22:30 ` kbuild test robot
  1 sibling, 1 reply; 4+ messages in thread
From: Matthew Wilcox @ 2018-02-25 15:18 UTC (permalink / raw)
  To: Kai-Heng Feng; +Cc: gregkh, oneukum, corbet, linux-doc, linux-kernel, linux-usb

On Sun, Feb 25, 2018 at 08:38:33PM +0800, Kai-Heng Feng wrote:
> v2: Use in-kernel tolower() function.

... why are you using tolower at all?

You've got 13 quirks already; you may need to use upper case as well
before too long.

> +		quirk = vmalloc(sizeof(struct quirk_entry));

vmalloc?  For a struct that's 24 bytes?  You know that allocates an
entire 4k page, right?


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v3] usb: core: Add "quirks" parameter for usbcore
  2018-02-25 15:18 ` Matthew Wilcox
@ 2018-02-25 15:45   ` Kai-Heng Feng
  0 siblings, 0 replies; 4+ messages in thread
From: Kai-Heng Feng @ 2018-02-25 15:45 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Greg Kroah-Hartman, Oliver Neukum, Jonathan Corbet, linux-doc,
	LKML, USB list

On Sun, Feb 25, 2018 at 11:18 PM, Matthew Wilcox <willy@infradead.org> wrote:
> On Sun, Feb 25, 2018 at 08:38:33PM +0800, Kai-Heng Feng wrote:
>> v2: Use in-kernel tolower() function.
>
> ... why are you using tolower at all?
>
> You've got 13 quirks already; you may need to use upper case as well
> before too long.

Makes sense. I'll remove tolower() in next version.

>
>> +             quirk = vmalloc(sizeof(struct quirk_entry));
>
> vmalloc?  For a struct that's 24 bytes?  You know that allocates an
> entire 4k page, right?
>

Right, I'll use kmalloc() instead. Or is there any more efficient
malloc function?

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v3] usb: core: Add "quirks" parameter for usbcore
  2018-02-25 12:38 [PATCH v3] usb: core: Add "quirks" parameter for usbcore Kai-Heng Feng
  2018-02-25 15:18 ` Matthew Wilcox
@ 2018-02-26 22:30 ` kbuild test robot
  1 sibling, 0 replies; 4+ messages in thread
From: kbuild test robot @ 2018-02-26 22:30 UTC (permalink / raw)
  To: Kai-Heng Feng
  Cc: kbuild-all, gregkh, oneukum, corbet, linux-doc, linux-kernel,
	linux-usb, Kai-Heng Feng

[-- Attachment #1: Type: text/plain, Size: 1765 bytes --]

Hi Kai-Heng,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on usb/usb-testing]
[also build test ERROR on v4.16-rc3 next-20180226]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Kai-Heng-Feng/usb-core-Add-quirks-parameter-for-usbcore/20180227-041204
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
config: arm-badge4_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

All errors (new ones prefixed by >>):

>> drivers/usb//core/quirks.c:15:43: error: expected ')' before 'sizeof'
    module_param_string(quirks, quirks_param, sizeof(quirks_param), 0644);
                                              ^~~~~~
>> drivers/usb//core/quirks.c:16:26: error: expected ')' before string constant
    MODULE_PARM_DESC(quirks, "Add/modify USB quirks by specifying quirks=vendorID:productID:quirks");
                             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

vim +15 drivers/usb//core/quirks.c

    13	
    14	static char quirks_param[128];
  > 15	module_param_string(quirks, quirks_param, sizeof(quirks_param), 0644);
  > 16	MODULE_PARM_DESC(quirks, "Add/modify USB quirks by specifying quirks=vendorID:productID:quirks");
    17	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 15276 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2018-02-26 22:30 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-25 12:38 [PATCH v3] usb: core: Add "quirks" parameter for usbcore Kai-Heng Feng
2018-02-25 15:18 ` Matthew Wilcox
2018-02-25 15:45   ` Kai-Heng Feng
2018-02-26 22:30 ` kbuild test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).