| Message ID | 20231009161402.104224-1-hgajjar@de.adit-jv.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] usb: core: hub: Add quirks for reducing device address timeout | expand |
On Mon, Oct 09, 2023 at 06:14:02PM +0200, Hardik Gajjar wrote: > Currently, the timeout for the set address command is fixed at > 5 seconds in the xhci driver. This means the host waits up to 5 > seconds to receive a response for the set_address command from > the device. > > In the automotive context, most smartphone enumerations, including > screen projection, should ideally complete within 3 seconds. > Achieving this is impossible in scenarios where the set_address is > not successful and waits for a timeout. > > The shortened address device timeout quirks provide the flexibility > to align with a 3-second time limit in the event of errors. > By swiftly triggering a failure response and swiftly initiating > retry procedures, these quirks ensure efficient and rapid recovery, > particularly in automotive contexts where rapid smartphone enumeration > and screen projection are vital. So you have known-broken devices where you want a shorter error timeout? But you don't list those devices in this patch adding the quirk settings, shouldn't that be required, otherwise this looks like an unused quirk. > The quirk will set the timeout to 500 ms from 5 seconds. > > To use the quirk, please write "vendor_id:product_id:p" to > /sys/bus/usb/drivers/hub/module/parameter/quirks > > For example, > echo "0x2c48:0x0132:p" > /sys/bus/usb/drivers/hub/module/parameter/quirks" > > Signed-off-by: Hardik Gajjar <hgajjar@de.adit-jv.com> > --- > changes since version 1: > - implement quirk instead of new API in xhci driver > --- > drivers/usb/core/hub.c | 15 +++++++++++++-- > drivers/usb/core/quirks.c | 3 +++ > drivers/usb/host/xhci-mem.c | 1 + > drivers/usb/host/xhci-ring.c | 3 ++- > drivers/usb/host/xhci.c | 9 +++++---- > drivers/usb/host/xhci.h | 1 + > include/linux/usb/hcd.h | 3 ++- > include/linux/usb/quirks.h | 3 +++ > 8 files changed, 30 insertions(+), 8 deletions(-) > > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c > index 3c54b218301c..975449b03426 100644 > --- a/drivers/usb/core/hub.c > +++ b/drivers/usb/core/hub.c > @@ -54,6 +54,9 @@ > #define USB_TP_TRANSMISSION_DELAY_MAX 65535 /* ns */ > #define USB_PING_RESPONSE_TIME 400 /* ns */ > > +#define USB_DEFAULT_ADDR_DEVICE_TIMEOUT (HZ * 5) /* 5000ms */ > +#define USB_SHORT_ADDR_DEVICE_TIMEOUT 125 /* ~500ms */ > + > /* Protect struct usb_device->state and ->children members > * Note: Both are also protected by ->dev.sem, except that ->state can > * change to USB_STATE_NOTATTACHED even when the semaphore isn't held. */ > @@ -4626,8 +4629,16 @@ EXPORT_SYMBOL_GPL(usb_ep0_reinit); > static int hub_set_address(struct usb_device *udev, int devnum) > { > int retval; > + int timeout = USB_DEFAULT_ADDR_DEVICE_TIMEOUT; > struct usb_hcd *hcd = bus_to_hcd(udev->bus); > > + struct usb_hub *hub = usb_hub_to_struct_hub(udev->parent); > + > + if (hub->hdev->quirks & USB_QUIRK_SHORT_DEVICE_ADDR_TIMEOUT) > + timeout = USB_SHORT_ADDR_DEVICE_TIMEOUT; > + > + dev_dbg(&udev->dev, "address_device timeout %d\n", timeout); Is this debugging code still needed? > /* > * The host controller will choose the device address, > * instead of the core having chosen it earlier > @@ -4639,11 +4650,11 @@ static int hub_set_address(struct usb_device *udev, int devnum) > if (udev->state != USB_STATE_DEFAULT) > return -EINVAL; > if (hcd->driver->address_device) > - retval = hcd->driver->address_device(hcd, udev); > + retval = hcd->driver->address_device(hcd, udev, timeout); > else > retval = usb_control_msg(udev, usb_sndaddr0pipe(), > USB_REQ_SET_ADDRESS, 0, devnum, 0, > - NULL, 0, USB_CTRL_SET_TIMEOUT); > + NULL, 0, jiffies_to_msecs(timeout)); > if (retval == 0) { > update_devnum(udev, devnum); > /* Device now using proper address. */ > diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c > index 15e9bd180a1d..01ed26bd41f0 100644 > --- a/drivers/usb/core/quirks.c > +++ b/drivers/usb/core/quirks.c > @@ -138,6 +138,9 @@ static int quirks_param_set(const char *value, const struct kernel_param *kp) > case 'o': > flags |= USB_QUIRK_HUB_SLOW_RESET; > break; > + case 'p': > + flags |= USB_QUIRK_SHORT_DEVICE_ADDR_TIMEOUT; > + break; > /* Ignore unrecognized flag characters */ > } > } > diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c > index 8714ab5bf04d..492433fdac77 100644 > --- a/drivers/usb/host/xhci-mem.c > +++ b/drivers/usb/host/xhci-mem.c > @@ -1729,6 +1729,7 @@ struct xhci_command *xhci_alloc_command(struct xhci_hcd *xhci, > } > > command->status = 0; > + command->timeout = 0; > INIT_LIST_HEAD(&command->cmd_list); > return command; > } > diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c > index 1dde53f6eb31..0bd19a1efdec 100644 > --- a/drivers/usb/host/xhci-ring.c > +++ b/drivers/usb/host/xhci-ring.c > @@ -4301,7 +4301,8 @@ static int queue_command(struct xhci_hcd *xhci, struct xhci_command *cmd, > /* if there are no other commands queued we start the timeout timer */ > if (list_empty(&xhci->cmd_list)) { > xhci->current_cmd = cmd; > - xhci_mod_cmd_timer(xhci, XHCI_CMD_DEFAULT_TIMEOUT); > + xhci_mod_cmd_timer(xhci, (cmd->timeout) ? cmd->timeout : > + XHCI_CMD_DEFAULT_TIMEOUT); > } > > list_add_tail(&cmd->cmd_list, &xhci->cmd_list); > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c > index e1b1b64a0723..1d088ceb2b74 100644 > --- a/drivers/usb/host/xhci.c > +++ b/drivers/usb/host/xhci.c > @@ -4002,7 +4002,7 @@ int xhci_alloc_dev(struct usb_hcd *hcd, struct usb_device *udev) > * SetAddress request to the device. > */ > static int xhci_setup_device(struct usb_hcd *hcd, struct usb_device *udev, > - enum xhci_setup_dev setup) > + enum xhci_setup_dev setup, int timeout) What is the units of timeout here? > { > const char *act = setup == SETUP_CONTEXT_ONLY ? "context" : "address"; > unsigned long flags; > @@ -4059,6 +4059,7 @@ static int xhci_setup_device(struct usb_hcd *hcd, struct usb_device *udev, > } > > command->in_ctx = virt_dev->in_ctx; > + command->timeout = timeout; > > slot_ctx = xhci_get_slot_ctx(xhci, virt_dev->in_ctx); > ctrl_ctx = xhci_get_input_control_ctx(virt_dev->in_ctx); > @@ -4185,14 +4186,14 @@ static int xhci_setup_device(struct usb_hcd *hcd, struct usb_device *udev, > return ret; > } > > -static int xhci_address_device(struct usb_hcd *hcd, struct usb_device *udev) > +static int xhci_address_device(struct usb_hcd *hcd, struct usb_device *udev, int timeout) > { > - return xhci_setup_device(hcd, udev, SETUP_CONTEXT_ADDRESS); > + return xhci_setup_device(hcd, udev, SETUP_CONTEXT_ADDRESS, timeout); > } > > static int xhci_enable_device(struct usb_hcd *hcd, struct usb_device *udev) > { > - return xhci_setup_device(hcd, udev, SETUP_CONTEXT_ONLY); > + return xhci_setup_device(hcd, udev, SETUP_CONTEXT_ONLY, 0); 0 is no timeout at all? Or max timeout? Where is this documented? And why is this only added to the xhci driver and not all other host controllers? > } > > /* > diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h > index 7e282b4522c0..ebdca8dd01c2 100644 > --- a/drivers/usb/host/xhci.h > +++ b/drivers/usb/host/xhci.h > @@ -818,6 +818,7 @@ struct xhci_command { > struct completion *completion; > union xhci_trb *command_trb; > struct list_head cmd_list; > + int timeout; What is the units here. > }; > > /* drop context bitmasks */ > diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h > index 61d4f0b793dc..b0fda87ad3a2 100644 > --- a/include/linux/usb/hcd.h > +++ b/include/linux/usb/hcd.h > @@ -373,7 +373,8 @@ struct hc_driver { > */ > void (*reset_bandwidth)(struct usb_hcd *, struct usb_device *); > /* Returns the hardware-chosen device address */ > - int (*address_device)(struct usb_hcd *, struct usb_device *udev); > + int (*address_device)(struct usb_hcd *, struct usb_device *udev, > + int timeout); Again, units please. > /* prepares the hardware to send commands to the device */ > int (*enable_device)(struct usb_hcd *, struct usb_device *udev); > /* Notifies the HCD after a hub descriptor is fetched. > diff --git a/include/linux/usb/quirks.h b/include/linux/usb/quirks.h > index eeb7c2157c72..0cb464e3eaf4 100644 > --- a/include/linux/usb/quirks.h > +++ b/include/linux/usb/quirks.h > @@ -72,4 +72,7 @@ > /* device has endpoints that should be ignored */ > #define USB_QUIRK_ENDPOINT_IGNORE BIT(15) > > +/* short device address timeout */ > +#define USB_QUIRK_SHORT_DEVICE_ADDR_TIMEOUT BIT(16) Don't you also need to have a Documentation/ update for the new user/kernel api you are adding? thanks, greg k-h
On Mon, Oct 09, 2023 at 06:14:02PM +0200, Hardik Gajjar wrote: > Currently, the timeout for the set address command is fixed at > 5 seconds in the xhci driver. This means the host waits up to 5 > seconds to receive a response for the set_address command from > the device. > > In the automotive context, most smartphone enumerations, including > screen projection, should ideally complete within 3 seconds. > Achieving this is impossible in scenarios where the set_address is > not successful and waits for a timeout. What will you do about scenarios where the Set-Address completes very quickly but the following Get-Device-Descriptor times out after 5 seconds? Or any of the other transfers involved in device initialization and enumeration? > The shortened address device timeout quirks provide the flexibility > to align with a 3-second time limit in the event of errors. > By swiftly triggering a failure response and swiftly initiating > retry procedures, these quirks ensure efficient and rapid recovery, > particularly in automotive contexts where rapid smartphone enumeration > and screen projection are vital. > > The quirk will set the timeout to 500 ms from 5 seconds. > > To use the quirk, please write "vendor_id:product_id:p" to > /sys/bus/usb/drivers/hub/module/parameter/quirks > > For example, > echo "0x2c48:0x0132:p" > /sys/bus/usb/drivers/hub/module/parameter/quirks" > > Signed-off-by: Hardik Gajjar <hgajjar@de.adit-jv.com> > --- > changes since version 1: > - implement quirk instead of new API in xhci driver > --- > drivers/usb/core/hub.c | 15 +++++++++++++-- > drivers/usb/core/quirks.c | 3 +++ > drivers/usb/host/xhci-mem.c | 1 + > drivers/usb/host/xhci-ring.c | 3 ++- > drivers/usb/host/xhci.c | 9 +++++---- > drivers/usb/host/xhci.h | 1 + > include/linux/usb/hcd.h | 3 ++- > include/linux/usb/quirks.h | 3 +++ > 8 files changed, 30 insertions(+), 8 deletions(-) > > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c > index 3c54b218301c..975449b03426 100644 > --- a/drivers/usb/core/hub.c > +++ b/drivers/usb/core/hub.c > @@ -54,6 +54,9 @@ > #define USB_TP_TRANSMISSION_DELAY_MAX 65535 /* ns */ > #define USB_PING_RESPONSE_TIME 400 /* ns */ > > +#define USB_DEFAULT_ADDR_DEVICE_TIMEOUT (HZ * 5) /* 5000ms */ > +#define USB_SHORT_ADDR_DEVICE_TIMEOUT 125 /* ~500ms */ That number, 125, is meaningless. It's in units of jiffies, which vary from one system to another. If you want the timeout to be about 500 ms, you should write it as (HZ / 2). Alan Stern
On Mon, Oct 09, 2023 at 07:43:09PM +0200, Greg KH wrote: > On Mon, Oct 09, 2023 at 06:14:02PM +0200, Hardik Gajjar wrote: > > Currently, the timeout for the set address command is fixed at > > 5 seconds in the xhci driver. This means the host waits up to 5 > > seconds to receive a response for the set_address command from > > the device. > > > > In the automotive context, most smartphone enumerations, including > > screen projection, should ideally complete within 3 seconds. > > Achieving this is impossible in scenarios where the set_address is > > not successful and waits for a timeout. > > > > The shortened address device timeout quirks provide the flexibility > > to align with a 3-second time limit in the event of errors. > > By swiftly triggering a failure response and swiftly initiating > > retry procedures, these quirks ensure efficient and rapid recovery, > > particularly in automotive contexts where rapid smartphone enumeration > > and screen projection are vital. > > So you have known-broken devices where you want a shorter error timeout? > But you don't list those devices in this patch adding the quirk > settings, shouldn't that be required, otherwise this looks like an > unused quirk. Yes, we have identified the device (hub) that is causing the issue. However, I would prefer not to force this timeout globally. Instead, I can dynamically control it by writing to the /sys/bus/usb/drivers/hub/module/parameter/quirks file from init.rc (Android) during the boot-up process. > > > The quirk will set the timeout to 500 ms from 5 seconds. > > > > To use the quirk, please write "vendor_id:product_id:p" to > > /sys/bus/usb/drivers/hub/module/parameter/quirks > > > > For example, > > echo "0x2c48:0x0132:p" > /sys/bus/usb/drivers/hub/module/parameter/quirks" > > > > Signed-off-by: Hardik Gajjar <hgajjar@de.adit-jv.com> > > --- > > changes since version 1: > > - implement quirk instead of new API in xhci driver > > --- > > drivers/usb/core/hub.c | 15 +++++++++++++-- > > drivers/usb/core/quirks.c | 3 +++ > > drivers/usb/host/xhci-mem.c | 1 + > > drivers/usb/host/xhci-ring.c | 3 ++- > > drivers/usb/host/xhci.c | 9 +++++---- > > drivers/usb/host/xhci.h | 1 + > > include/linux/usb/hcd.h | 3 ++- > > include/linux/usb/quirks.h | 3 +++ > > 8 files changed, 30 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c > > index 3c54b218301c..975449b03426 100644 > > --- a/drivers/usb/core/hub.c > > +++ b/drivers/usb/core/hub.c > > @@ -54,6 +54,9 @@ > > #define USB_TP_TRANSMISSION_DELAY_MAX 65535 /* ns */ > > #define USB_PING_RESPONSE_TIME 400 /* ns */ > > > > +#define USB_DEFAULT_ADDR_DEVICE_TIMEOUT (HZ * 5) /* 5000ms */ > > +#define USB_SHORT_ADDR_DEVICE_TIMEOUT 125 /* ~500ms */ > > + > > /* Protect struct usb_device->state and ->children members > > * Note: Both are also protected by ->dev.sem, except that ->state can > > * change to USB_STATE_NOTATTACHED even when the semaphore isn't held. */ > > @@ -4626,8 +4629,16 @@ EXPORT_SYMBOL_GPL(usb_ep0_reinit); > > static int hub_set_address(struct usb_device *udev, int devnum) > > { > > int retval; > > + int timeout = USB_DEFAULT_ADDR_DEVICE_TIMEOUT; > > struct usb_hcd *hcd = bus_to_hcd(udev->bus); > > > > + struct usb_hub *hub = usb_hub_to_struct_hub(udev->parent); > > + > > + if (hub->hdev->quirks & USB_QUIRK_SHORT_DEVICE_ADDR_TIMEOUT) > > + timeout = USB_SHORT_ADDR_DEVICE_TIMEOUT; > > + > > + dev_dbg(&udev->dev, "address_device timeout %d\n", timeout); > > Is this debugging code still needed? will remove in patch v3 > > > /* > > * The host controller will choose the device address, > > * instead of the core having chosen it earlier > > @@ -4639,11 +4650,11 @@ static int hub_set_address(struct usb_device *udev, int devnum) > > if (udev->state != USB_STATE_DEFAULT) > > return -EINVAL; > > if (hcd->driver->address_device) > > - retval = hcd->driver->address_device(hcd, udev); > > + retval = hcd->driver->address_device(hcd, udev, timeout); > > else > > retval = usb_control_msg(udev, usb_sndaddr0pipe(), > > USB_REQ_SET_ADDRESS, 0, devnum, 0, > > - NULL, 0, USB_CTRL_SET_TIMEOUT); > > + NULL, 0, jiffies_to_msecs(timeout)); > > if (retval == 0) { > > update_devnum(udev, devnum); > > /* Device now using proper address. */ > > diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c > > index 15e9bd180a1d..01ed26bd41f0 100644 > > --- a/drivers/usb/core/quirks.c > > +++ b/drivers/usb/core/quirks.c > > @@ -138,6 +138,9 @@ static int quirks_param_set(const char *value, const struct kernel_param *kp) > > case 'o': > > flags |= USB_QUIRK_HUB_SLOW_RESET; > > break; > > + case 'p': > > + flags |= USB_QUIRK_SHORT_DEVICE_ADDR_TIMEOUT; > > + break; > > /* Ignore unrecognized flag characters */ > > } > > } > > diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c > > index 8714ab5bf04d..492433fdac77 100644 > > --- a/drivers/usb/host/xhci-mem.c > > +++ b/drivers/usb/host/xhci-mem.c > > @@ -1729,6 +1729,7 @@ struct xhci_command *xhci_alloc_command(struct xhci_hcd *xhci, > > } > > > > command->status = 0; > > + command->timeout = 0; > > INIT_LIST_HEAD(&command->cmd_list); > > return command; > > } > > diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c > > index 1dde53f6eb31..0bd19a1efdec 100644 > > --- a/drivers/usb/host/xhci-ring.c > > +++ b/drivers/usb/host/xhci-ring.c > > @@ -4301,7 +4301,8 @@ static int queue_command(struct xhci_hcd *xhci, struct xhci_command *cmd, > > /* if there are no other commands queued we start the timeout timer */ > > if (list_empty(&xhci->cmd_list)) { > > xhci->current_cmd = cmd; > > - xhci_mod_cmd_timer(xhci, XHCI_CMD_DEFAULT_TIMEOUT); > > + xhci_mod_cmd_timer(xhci, (cmd->timeout) ? cmd->timeout : > > + XHCI_CMD_DEFAULT_TIMEOUT); > > } > > > > list_add_tail(&cmd->cmd_list, &xhci->cmd_list); > > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c > > index e1b1b64a0723..1d088ceb2b74 100644 > > --- a/drivers/usb/host/xhci.c > > +++ b/drivers/usb/host/xhci.c > > @@ -4002,7 +4002,7 @@ int xhci_alloc_dev(struct usb_hcd *hcd, struct usb_device *udev) > > * SetAddress request to the device. > > */ > > static int xhci_setup_device(struct usb_hcd *hcd, struct usb_device *udev, > > - enum xhci_setup_dev setup) > > + enum xhci_setup_dev setup, int timeout) > > What is the units of timeout here? The timeout is in jiffies. Should we consider adding 'jiffies' in the variable name for clarity? I searched the kernel source code but couldn't find a reference for the unit of timeout in jiffies. Nevertheless, I will add a comment in patch v3 for clarification. > > > { > > const char *act = setup == SETUP_CONTEXT_ONLY ? "context" : "address"; > > unsigned long flags; > > @@ -4059,6 +4059,7 @@ static int xhci_setup_device(struct usb_hcd *hcd, struct usb_device *udev, > > } > > > > command->in_ctx = virt_dev->in_ctx; > > + command->timeout = timeout; > > > > slot_ctx = xhci_get_slot_ctx(xhci, virt_dev->in_ctx); > > ctrl_ctx = xhci_get_input_control_ctx(virt_dev->in_ctx); > > @@ -4185,14 +4186,14 @@ static int xhci_setup_device(struct usb_hcd *hcd, struct usb_device *udev, > > return ret; > > } > > > > -static int xhci_address_device(struct usb_hcd *hcd, struct usb_device *udev) > > +static int xhci_address_device(struct usb_hcd *hcd, struct usb_device *udev, int timeout) > > { > > - return xhci_setup_device(hcd, udev, SETUP_CONTEXT_ADDRESS); > > + return xhci_setup_device(hcd, udev, SETUP_CONTEXT_ADDRESS, timeout); > > } > > > > static int xhci_enable_device(struct usb_hcd *hcd, struct usb_device *udev) > > { > > - return xhci_setup_device(hcd, udev, SETUP_CONTEXT_ONLY); > > + return xhci_setup_device(hcd, udev, SETUP_CONTEXT_ONLY, 0); > > 0 is no timeout at all? Or max timeout? Where is this documented? > > And why is this only added to the xhci driver and not all other host > controllers? Because 'address_device' is only assigned in the xHCI driver, other host drivers (OHCI, EHCI) issue 'set_address' requests directly from 'hub.c' and utilize the timeout I have already updated that API in this patch to utilize timeout. > > > } > > > > /* > > diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h > > index 7e282b4522c0..ebdca8dd01c2 100644 > > --- a/drivers/usb/host/xhci.h > > +++ b/drivers/usb/host/xhci.h > > @@ -818,6 +818,7 @@ struct xhci_command { > > struct completion *completion; > > union xhci_trb *command_trb; > > struct list_head cmd_list; > > + int timeout; > > What is the units here. The timeout is in jiffies. Should we consider adding 'jiffies' in the variable name for clarity? I searched the kernel source code but couldn't find a reference for the unit of timeout in jiffies. Nevertheless, I will add a comment in patch v3 for clarification. > > > }; > > > > /* drop context bitmasks */ > > diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h > > index 61d4f0b793dc..b0fda87ad3a2 100644 > > --- a/include/linux/usb/hcd.h > > +++ b/include/linux/usb/hcd.h > > @@ -373,7 +373,8 @@ struct hc_driver { > > */ > > void (*reset_bandwidth)(struct usb_hcd *, struct usb_device *); > > /* Returns the hardware-chosen device address */ > > - int (*address_device)(struct usb_hcd *, struct usb_device *udev); > > + int (*address_device)(struct usb_hcd *, struct usb_device *udev, > > + int timeout); > > Again, units please. The timeout is in jiffies. Should we consider adding 'jiffies' in the variable name for clarity? I searched the kernel source code but couldn't find a reference for the unit of timeout in jiffies. Nevertheless, I will add a comment in patch v3 for clarification. > > > /* prepares the hardware to send commands to the device */ > > int (*enable_device)(struct usb_hcd *, struct usb_device *udev); > > /* Notifies the HCD after a hub descriptor is fetched. > > diff --git a/include/linux/usb/quirks.h b/include/linux/usb/quirks.h > > index eeb7c2157c72..0cb464e3eaf4 100644 > > --- a/include/linux/usb/quirks.h > > +++ b/include/linux/usb/quirks.h > > @@ -72,4 +72,7 @@ > > /* device has endpoints that should be ignored */ > > #define USB_QUIRK_ENDPOINT_IGNORE BIT(15) > > > > +/* short device address timeout */ > > +#define USB_QUIRK_SHORT_DEVICE_ADDR_TIMEOUT BIT(16) > > Don't you also need to have a Documentation/ update for the new > user/kernel api you are adding? > When you recommend Documentation, I assume you suggest to add the detailed comment in quirks.h to clear intention of the change. I will update it in patch v3. > thanks, > > greg k-h
On Mon, Oct 09, 2023 at 03:16:25PM -0400, Alan Stern wrote: > On Mon, Oct 09, 2023 at 06:14:02PM +0200, Hardik Gajjar wrote: > > Currently, the timeout for the set address command is fixed at > > 5 seconds in the xhci driver. This means the host waits up to 5 > > seconds to receive a response for the set_address command from > > the device. > > > > In the automotive context, most smartphone enumerations, including > > screen projection, should ideally complete within 3 seconds. > > Achieving this is impossible in scenarios where the set_address is > > not successful and waits for a timeout. > > What will you do about scenarios where the Set-Address completes very > quickly but the following Get-Device-Descriptor times out after 5 > seconds? Or any of the other transfers involved in device > initialization and enumeration? This issue occurs when the device first enumerates as full speed and then as a high-speed device. It appears that the set_address request is issued as soon as the device is detected as full speed. However, during the progress of the set_address request, the device changes its state from full speed to high speed, causing the set_address request to become stuck until it times out. Stress testing of USB devices indicates that the problem is specific to the set_address request. Other requests, such as device descriptor requests, consistently fail immediately with a protocol error in almost all cases. > > > The shortened address device timeout quirks provide the flexibility > > to align with a 3-second time limit in the event of errors. > > By swiftly triggering a failure response and swiftly initiating > > retry procedures, these quirks ensure efficient and rapid recovery, > > particularly in automotive contexts where rapid smartphone enumeration > > and screen projection are vital. > > > > The quirk will set the timeout to 500 ms from 5 seconds. > > > > To use the quirk, please write "vendor_id:product_id:p" to > > /sys/bus/usb/drivers/hub/module/parameter/quirks > > > > For example, > > echo "0x2c48:0x0132:p" > /sys/bus/usb/drivers/hub/module/parameter/quirks" > > > > Signed-off-by: Hardik Gajjar <hgajjar@de.adit-jv.com> > > --- > > changes since version 1: > > - implement quirk instead of new API in xhci driver > > --- > > drivers/usb/core/hub.c | 15 +++++++++++++-- > > drivers/usb/core/quirks.c | 3 +++ > > drivers/usb/host/xhci-mem.c | 1 + > > drivers/usb/host/xhci-ring.c | 3 ++- > > drivers/usb/host/xhci.c | 9 +++++---- > > drivers/usb/host/xhci.h | 1 + > > include/linux/usb/hcd.h | 3 ++- > > include/linux/usb/quirks.h | 3 +++ > > 8 files changed, 30 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c > > index 3c54b218301c..975449b03426 100644 > > --- a/drivers/usb/core/hub.c > > +++ b/drivers/usb/core/hub.c > > @@ -54,6 +54,9 @@ > > #define USB_TP_TRANSMISSION_DELAY_MAX 65535 /* ns */ > > #define USB_PING_RESPONSE_TIME 400 /* ns */ > > > > +#define USB_DEFAULT_ADDR_DEVICE_TIMEOUT (HZ * 5) /* 5000ms */ > > +#define USB_SHORT_ADDR_DEVICE_TIMEOUT 125 /* ~500ms */ > > That number, 125, is meaningless. It's in units of jiffies, which vary > from one system to another. If you want the timeout to be about 500 ms, > you should write it as (HZ / 2). Good Point, Thanks, I will update in patch v3 > > Alan Stern
On Tue, Oct 10, 2023 at 12:31:43PM +0200, Hardik Gajjar wrote: > On Mon, Oct 09, 2023 at 07:43:09PM +0200, Greg KH wrote: > > On Mon, Oct 09, 2023 at 06:14:02PM +0200, Hardik Gajjar wrote: > > > Currently, the timeout for the set address command is fixed at > > > 5 seconds in the xhci driver. This means the host waits up to 5 > > > seconds to receive a response for the set_address command from > > > the device. > > > > > > In the automotive context, most smartphone enumerations, including > > > screen projection, should ideally complete within 3 seconds. > > > Achieving this is impossible in scenarios where the set_address is > > > not successful and waits for a timeout. > > > > > > The shortened address device timeout quirks provide the flexibility > > > to align with a 3-second time limit in the event of errors. > > > By swiftly triggering a failure response and swiftly initiating > > > retry procedures, these quirks ensure efficient and rapid recovery, > > > particularly in automotive contexts where rapid smartphone enumeration > > > and screen projection are vital. > > > > So you have known-broken devices where you want a shorter error timeout? > > But you don't list those devices in this patch adding the quirk > > settings, shouldn't that be required, otherwise this looks like an > > unused quirk. > Yes, we have identified the device (hub) that is causing the issue. However, > I would prefer not to force this timeout globally. Instead, I can dynamically > control it by writing to the /sys/bus/usb/drivers/hub/module/parameter/quirks > file from init.rc (Android) during the boot-up process. Then add that device to the quirk list please so that everyone doesn't have to figure this out on their own for that broken device. That's why we have a quirk list in the kernel. > > > static int xhci_setup_device(struct usb_hcd *hcd, struct usb_device *udev, > > > - enum xhci_setup_dev setup) > > > + enum xhci_setup_dev setup, int timeout) > > > > What is the units of timeout here? > The timeout is in jiffies. Should we consider adding 'jiffies' in the variable name > for clarity? I searched the kernel source code but couldn't find a reference for the > unit of timeout in jiffies. > Nevertheless, I will add a comment in patch v3 for clarification. You should not pass around jiffies in the kernel in an int, please pass around a unit of time and then when you actually need to tell the kernel to sleep, you can use the time to convert to whatever unit the delay function needs. > > > +/* short device address timeout */ > > > +#define USB_QUIRK_SHORT_DEVICE_ADDR_TIMEOUT BIT(16) > > > > Don't you also need to have a Documentation/ update for the new > > user/kernel api you are adding? > > > When you recommend Documentation, I assume you suggest to add the > detailed comment in quirks.h to clear intention of the change. No, please document the info that you have in the changelog in someplace where people will know what flag to use in the module option. That's already documented for the other flags somewhere, right? thanks, greg k-h
On 9.10.2023 19.14, Hardik Gajjar wrote: > Currently, the timeout for the set address command is fixed at > 5 seconds in the xhci driver. This means the host waits up to 5 > seconds to receive a response for the set_address command from > the device. > > In the automotive context, most smartphone enumerations, including > screen projection, should ideally complete within 3 seconds. > Achieving this is impossible in scenarios where the set_address is > not successful and waits for a timeout. > > The shortened address device timeout quirks provide the flexibility > to align with a 3-second time limit in the event of errors. > By swiftly triggering a failure response and swiftly initiating > retry procedures, these quirks ensure efficient and rapid recovery, > particularly in automotive contexts where rapid smartphone enumeration > and screen projection are vital. > > The quirk will set the timeout to 500 ms from 5 seconds. > > To use the quirk, please write "vendor_id:product_id:p" to > /sys/bus/usb/drivers/hub/module/parameter/quirks > > For example, > echo "0x2c48:0x0132:p" > /sys/bus/usb/drivers/hub/module/parameter/quirks" > > Signed-off-by: Hardik Gajjar <hgajjar@de.adit-jv.com> > --- > changes since version 1: > - implement quirk instead of new API in xhci driver > --- > drivers/usb/core/hub.c | 15 +++++++++++++-- > drivers/usb/core/quirks.c | 3 +++ > drivers/usb/host/xhci-mem.c | 1 + > drivers/usb/host/xhci-ring.c | 3 ++- > drivers/usb/host/xhci.c | 9 +++++---- > drivers/usb/host/xhci.h | 1 + > include/linux/usb/hcd.h | 3 ++- > include/linux/usb/quirks.h | 3 +++ > 8 files changed, 30 insertions(+), 8 deletions(-) > > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c > index 3c54b218301c..975449b03426 100644 > --- a/drivers/usb/core/hub.c > +++ b/drivers/usb/core/hub.c > @@ -54,6 +54,9 @@ > #define USB_TP_TRANSMISSION_DELAY_MAX 65535 /* ns */ > #define USB_PING_RESPONSE_TIME 400 /* ns */ > > +#define USB_DEFAULT_ADDR_DEVICE_TIMEOUT (HZ * 5) /* 5000ms */ > +#define USB_SHORT_ADDR_DEVICE_TIMEOUT 125 /* ~500ms */ maybe use milliseconds directly define USB_DEFAULT_ADDR_DEVICE_TIMEOUT 5000 /* ms */ ... > + > /* Protect struct usb_device->state and ->children members > * Note: Both are also protected by ->dev.sem, except that ->state can > * change to USB_STATE_NOTATTACHED even when the semaphore isn't held. */ > @@ -4626,8 +4629,16 @@ EXPORT_SYMBOL_GPL(usb_ep0_reinit); > static int hub_set_address(struct usb_device *udev, int devnum) > { > int retval; > + int timeout = USB_DEFAULT_ADDR_DEVICE_TIMEOUT; > struct usb_hcd *hcd = bus_to_hcd(udev->bus); > > + struct usb_hub *hub = usb_hub_to_struct_hub(udev->parent); > + > + if (hub->hdev->quirks & USB_QUIRK_SHORT_DEVICE_ADDR_TIMEOUT) > + timeout = USB_SHORT_ADDR_DEVICE_TIMEOUT; > + > + dev_dbg(&udev->dev, "address_device timeout %d\n", timeout); > + > /* > * The host controller will choose the device address, > * instead of the core having chosen it earlier > @@ -4639,11 +4650,11 @@ static int hub_set_address(struct usb_device *udev, int devnum) > if (udev->state != USB_STATE_DEFAULT) > return -EINVAL; > if (hcd->driver->address_device) > - retval = hcd->driver->address_device(hcd, udev); > + retval = hcd->driver->address_device(hcd, udev, timeout); > else > retval = usb_control_msg(udev, usb_sndaddr0pipe(), > USB_REQ_SET_ADDRESS, 0, devnum, 0, > - NULL, 0, USB_CTRL_SET_TIMEOUT); > + NULL, 0, jiffies_to_msecs(timeout)); > if (retval == 0) { > update_devnum(udev, devnum); > /* Device now using proper address. */ > diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c > index 15e9bd180a1d..01ed26bd41f0 100644 > --- a/drivers/usb/core/quirks.c > +++ b/drivers/usb/core/quirks.c > @@ -138,6 +138,9 @@ static int quirks_param_set(const char *value, const struct kernel_param *kp) > case 'o': > flags |= USB_QUIRK_HUB_SLOW_RESET; > break; > + case 'p': > + flags |= USB_QUIRK_SHORT_DEVICE_ADDR_TIMEOUT; > + break; > /* Ignore unrecognized flag characters */ > } > } > diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c > index 8714ab5bf04d..492433fdac77 100644 > --- a/drivers/usb/host/xhci-mem.c > +++ b/drivers/usb/host/xhci-mem.c > @@ -1729,6 +1729,7 @@ struct xhci_command *xhci_alloc_command(struct xhci_hcd *xhci, > } > > command->status = 0; > + command->timeout = 0; This could be command->timeout = XHCI_CMD_DEFAULT_TIMEOUT_MS; For address device commands we then override it later in xhci_setup_device() The default xhci command timeout could be changed to 5000ms instead of 5 * HZ, so in xhci.h: /* Default xhci command timeout in milliseconds */ XHCI_CMD_DEFAULT_TIMEOUT_MS 5000 > INIT_LIST_HEAD(&command->cmd_list); > return command; > } > diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c > index 1dde53f6eb31..0bd19a1efdec 100644 > --- a/drivers/usb/host/xhci-ring.c > +++ b/drivers/usb/host/xhci-ring.c > @@ -4301,7 +4301,8 @@ static int queue_command(struct xhci_hcd *xhci, struct xhci_command *cmd, > /* if there are no other commands queued we start the timeout timer */ > if (list_empty(&xhci->cmd_list)) { > xhci->current_cmd = cmd; > - xhci_mod_cmd_timer(xhci, XHCI_CMD_DEFAULT_TIMEOUT); > + xhci_mod_cmd_timer(xhci, (cmd->timeout) ? cmd->timeout : > + XHCI_CMD_DEFAULT_TIMEOUT); xhci_mod_cmd_timer(xhci, cmd->timeout); We would then also need to convert ms to jiffies in xhci_mod_cmd_timer(): return mod_delayed_work(system_wq, &xhci->cmd_timer, msecs_to_jiffies(delay)); > } > > list_add_tail(&cmd->cmd_list, &xhci->cmd_list); > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c > index e1b1b64a0723..1d088ceb2b74 100644 > --- a/drivers/usb/host/xhci.c > +++ b/drivers/usb/host/xhci.c > @@ -4002,7 +4002,7 @@ int xhci_alloc_dev(struct usb_hcd *hcd, struct usb_device *udev) > * SetAddress request to the device. > */ > static int xhci_setup_device(struct usb_hcd *hcd, struct usb_device *udev, > - enum xhci_setup_dev setup) > + enum xhci_setup_dev setup, int timeout) Hmm, I know usb_control_msg() uses int for timeout, but unsigned int would seem more appropriate. Same for xhci_address_device() and xhci_enable_device() > { > const char *act = setup == SETUP_CONTEXT_ONLY ? "context" : "address"; > unsigned long flags; > @@ -4059,6 +4059,7 @@ static int xhci_setup_device(struct usb_hcd *hcd, struct usb_device *udev, > } > > command->in_ctx = virt_dev->in_ctx; > + command->timeout = timeout; > > slot_ctx = xhci_get_slot_ctx(xhci, virt_dev->in_ctx); > ctrl_ctx = xhci_get_input_control_ctx(virt_dev->in_ctx); > @@ -4185,14 +4186,14 @@ static int xhci_setup_device(struct usb_hcd *hcd, struct usb_device *udev, > return ret; > } > > -static int xhci_address_device(struct usb_hcd *hcd, struct usb_device *udev) > +static int xhci_address_device(struct usb_hcd *hcd, struct usb_device *udev, int timeout) > { > - return xhci_setup_device(hcd, udev, SETUP_CONTEXT_ADDRESS); > + return xhci_setup_device(hcd, udev, SETUP_CONTEXT_ADDRESS, timeout); > } > > static int xhci_enable_device(struct usb_hcd *hcd, struct usb_device *udev) > { > - return xhci_setup_device(hcd, udev, SETUP_CONTEXT_ONLY); > + return xhci_setup_device(hcd, udev, SETUP_CONTEXT_ONLY, 0); return xhci_setup_device(hcd, udev, SETUP_CONTEXT_ONLY, XHCI_CMD_DEFAULT_TIMEOUT_MS); > } > > /* > diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h > index 7e282b4522c0..ebdca8dd01c2 100644 > --- a/drivers/usb/host/xhci.h > +++ b/drivers/usb/host/xhci.h > @@ -818,6 +818,7 @@ struct xhci_command { > struct completion *completion; > union xhci_trb *command_trb; > struct list_head cmd_list; > + int timeout; unsigned int? Thanks Mathias
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index 3c54b218301c..975449b03426 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -54,6 +54,9 @@ #define USB_TP_TRANSMISSION_DELAY_MAX 65535 /* ns */ #define USB_PING_RESPONSE_TIME 400 /* ns */ +#define USB_DEFAULT_ADDR_DEVICE_TIMEOUT (HZ * 5) /* 5000ms */ +#define USB_SHORT_ADDR_DEVICE_TIMEOUT 125 /* ~500ms */ + /* Protect struct usb_device->state and ->children members * Note: Both are also protected by ->dev.sem, except that ->state can * change to USB_STATE_NOTATTACHED even when the semaphore isn't held. */ @@ -4626,8 +4629,16 @@ EXPORT_SYMBOL_GPL(usb_ep0_reinit); static int hub_set_address(struct usb_device *udev, int devnum) { int retval; + int timeout = USB_DEFAULT_ADDR_DEVICE_TIMEOUT; struct usb_hcd *hcd = bus_to_hcd(udev->bus); + struct usb_hub *hub = usb_hub_to_struct_hub(udev->parent); + + if (hub->hdev->quirks & USB_QUIRK_SHORT_DEVICE_ADDR_TIMEOUT) + timeout = USB_SHORT_ADDR_DEVICE_TIMEOUT; + + dev_dbg(&udev->dev, "address_device timeout %d\n", timeout); + /* * The host controller will choose the device address, * instead of the core having chosen it earlier @@ -4639,11 +4650,11 @@ static int hub_set_address(struct usb_device *udev, int devnum) if (udev->state != USB_STATE_DEFAULT) return -EINVAL; if (hcd->driver->address_device) - retval = hcd->driver->address_device(hcd, udev); + retval = hcd->driver->address_device(hcd, udev, timeout); else retval = usb_control_msg(udev, usb_sndaddr0pipe(), USB_REQ_SET_ADDRESS, 0, devnum, 0, - NULL, 0, USB_CTRL_SET_TIMEOUT); + NULL, 0, jiffies_to_msecs(timeout)); if (retval == 0) { update_devnum(udev, devnum); /* Device now using proper address. */ diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c index 15e9bd180a1d..01ed26bd41f0 100644 --- a/drivers/usb/core/quirks.c +++ b/drivers/usb/core/quirks.c @@ -138,6 +138,9 @@ static int quirks_param_set(const char *value, const struct kernel_param *kp) case 'o': flags |= USB_QUIRK_HUB_SLOW_RESET; break; + case 'p': + flags |= USB_QUIRK_SHORT_DEVICE_ADDR_TIMEOUT; + break; /* Ignore unrecognized flag characters */ } } diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index 8714ab5bf04d..492433fdac77 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -1729,6 +1729,7 @@ struct xhci_command *xhci_alloc_command(struct xhci_hcd *xhci, } command->status = 0; + command->timeout = 0; INIT_LIST_HEAD(&command->cmd_list); return command; } diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 1dde53f6eb31..0bd19a1efdec 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -4301,7 +4301,8 @@ static int queue_command(struct xhci_hcd *xhci, struct xhci_command *cmd, /* if there are no other commands queued we start the timeout timer */ if (list_empty(&xhci->cmd_list)) { xhci->current_cmd = cmd; - xhci_mod_cmd_timer(xhci, XHCI_CMD_DEFAULT_TIMEOUT); + xhci_mod_cmd_timer(xhci, (cmd->timeout) ? cmd->timeout : + XHCI_CMD_DEFAULT_TIMEOUT); } list_add_tail(&cmd->cmd_list, &xhci->cmd_list); diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index e1b1b64a0723..1d088ceb2b74 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -4002,7 +4002,7 @@ int xhci_alloc_dev(struct usb_hcd *hcd, struct usb_device *udev) * SetAddress request to the device. */ static int xhci_setup_device(struct usb_hcd *hcd, struct usb_device *udev, - enum xhci_setup_dev setup) + enum xhci_setup_dev setup, int timeout) { const char *act = setup == SETUP_CONTEXT_ONLY ? "context" : "address"; unsigned long flags; @@ -4059,6 +4059,7 @@ static int xhci_setup_device(struct usb_hcd *hcd, struct usb_device *udev, } command->in_ctx = virt_dev->in_ctx; + command->timeout = timeout; slot_ctx = xhci_get_slot_ctx(xhci, virt_dev->in_ctx); ctrl_ctx = xhci_get_input_control_ctx(virt_dev->in_ctx); @@ -4185,14 +4186,14 @@ static int xhci_setup_device(struct usb_hcd *hcd, struct usb_device *udev, return ret; } -static int xhci_address_device(struct usb_hcd *hcd, struct usb_device *udev) +static int xhci_address_device(struct usb_hcd *hcd, struct usb_device *udev, int timeout) { - return xhci_setup_device(hcd, udev, SETUP_CONTEXT_ADDRESS); + return xhci_setup_device(hcd, udev, SETUP_CONTEXT_ADDRESS, timeout); } static int xhci_enable_device(struct usb_hcd *hcd, struct usb_device *udev) { - return xhci_setup_device(hcd, udev, SETUP_CONTEXT_ONLY); + return xhci_setup_device(hcd, udev, SETUP_CONTEXT_ONLY, 0); } /* diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index 7e282b4522c0..ebdca8dd01c2 100644 --- a/drivers/usb/host/xhci.h +++ b/drivers/usb/host/xhci.h @@ -818,6 +818,7 @@ struct xhci_command { struct completion *completion; union xhci_trb *command_trb; struct list_head cmd_list; + int timeout; }; /* drop context bitmasks */ diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h index 61d4f0b793dc..b0fda87ad3a2 100644 --- a/include/linux/usb/hcd.h +++ b/include/linux/usb/hcd.h @@ -373,7 +373,8 @@ struct hc_driver { */ void (*reset_bandwidth)(struct usb_hcd *, struct usb_device *); /* Returns the hardware-chosen device address */ - int (*address_device)(struct usb_hcd *, struct usb_device *udev); + int (*address_device)(struct usb_hcd *, struct usb_device *udev, + int timeout); /* prepares the hardware to send commands to the device */ int (*enable_device)(struct usb_hcd *, struct usb_device *udev); /* Notifies the HCD after a hub descriptor is fetched. diff --git a/include/linux/usb/quirks.h b/include/linux/usb/quirks.h index eeb7c2157c72..0cb464e3eaf4 100644 --- a/include/linux/usb/quirks.h +++ b/include/linux/usb/quirks.h @@ -72,4 +72,7 @@ /* device has endpoints that should be ignored */ #define USB_QUIRK_ENDPOINT_IGNORE BIT(15) +/* short device address timeout */ +#define USB_QUIRK_SHORT_DEVICE_ADDR_TIMEOUT BIT(16) + #endif /* __LINUX_USB_QUIRKS_H */
Currently, the timeout for the set address command is fixed at 5 seconds in the xhci driver. This means the host waits up to 5 seconds to receive a response for the set_address command from the device. In the automotive context, most smartphone enumerations, including screen projection, should ideally complete within 3 seconds. Achieving this is impossible in scenarios where the set_address is not successful and waits for a timeout. The shortened address device timeout quirks provide the flexibility to align with a 3-second time limit in the event of errors. By swiftly triggering a failure response and swiftly initiating retry procedures, these quirks ensure efficient and rapid recovery, particularly in automotive contexts where rapid smartphone enumeration and screen projection are vital. The quirk will set the timeout to 500 ms from 5 seconds. To use the quirk, please write "vendor_id:product_id:p" to /sys/bus/usb/drivers/hub/module/parameter/quirks For example, echo "0x2c48:0x0132:p" > /sys/bus/usb/drivers/hub/module/parameter/quirks" Signed-off-by: Hardik Gajjar <hgajjar@de.adit-jv.com> --- changes since version 1: - implement quirk instead of new API in xhci driver --- drivers/usb/core/hub.c | 15 +++++++++++++-- drivers/usb/core/quirks.c | 3 +++ drivers/usb/host/xhci-mem.c | 1 + drivers/usb/host/xhci-ring.c | 3 ++- drivers/usb/host/xhci.c | 9 +++++---- drivers/usb/host/xhci.h | 1 + include/linux/usb/hcd.h | 3 ++- include/linux/usb/quirks.h | 3 +++ 8 files changed, 30 insertions(+), 8 deletions(-)