Re: [PATCH v3] usb: core: Add "quirks" parameter for usbcore

[kbuild test robot]

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 infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH v3] usb: core: Add "quirks" parameter for usbcore

[Kai-Heng Feng]

On Sun, Feb 25, 2018 at 11:18 PM, Matthew Wilcox  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?
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] usb: core: Add "quirks" parameter for usbcore

[Matthew Wilcox]

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?

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3] usb: core: Add "quirks" parameter for usbcore

[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 
---
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);
+