2019-11-17 03:36:20

by Ikjoon Jang

[permalink] [raw]
Subject: [PATCH 2/2] usb: overridable hub bInterval by device node

This patch enables hub device to override its own endpoint descriptor's
bInterval when the hub has a device node with "hub,interval" property,

Some existing hub devices have adjustable interval so the device is
allowed to use different bInterval. This is useful when the hub's default
bInterval is too big, so child device's waking up from autosuspend
takes much time.

Signed-off-by: Ikjoon Jang <[email protected]>
---
drivers/usb/core/config.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/drivers/usb/core/config.c b/drivers/usb/core/config.c
index 5f40117e68e7..234ca6124c98 100644
--- a/drivers/usb/core/config.c
+++ b/drivers/usb/core/config.c
@@ -6,6 +6,7 @@
#include <linux/usb.h>
#include <linux/usb/ch9.h>
#include <linux/usb/hcd.h>
+#include <linux/usb/of.h>
#include <linux/usb/quirks.h>
#include <linux/module.h>
#include <linux/slab.h>
@@ -257,6 +258,11 @@ static int usb_parse_endpoint(struct device *ddev, int cfgno, int inum,
memcpy(&endpoint->desc, d, n);
INIT_LIST_HEAD(&endpoint->urb_list);

+ /* device node property overrides bInterval */
+ if (unlikely(usb_of_has_combined_node(to_usb_device(ddev))))
+ of_property_read_u8(ddev->of_node, "hub, interval",
+ &d->bInterval);
+
/*
* Fix up bInterval values outside the legal range.
* Use 10 or 8 ms if no proper value can be guessed.
--
2.24.0.432.g9d3f5f5b63-goog


2019-11-17 07:20:04

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 2/2] usb: overridable hub bInterval by device node

On Sun, Nov 17, 2019 at 11:33:05AM +0800, Ikjoon Jang wrote:
> This patch enables hub device to override its own endpoint descriptor's
> bInterval when the hub has a device node with "hub,interval" property,
>
> Some existing hub devices have adjustable interval so the device is
> allowed to use different bInterval. This is useful when the hub's default
> bInterval is too big, so child device's waking up from autosuspend
> takes much time.
>
> Signed-off-by: Ikjoon Jang <[email protected]>
> ---
> drivers/usb/core/config.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/usb/core/config.c b/drivers/usb/core/config.c
> index 5f40117e68e7..234ca6124c98 100644
> --- a/drivers/usb/core/config.c
> +++ b/drivers/usb/core/config.c
> @@ -6,6 +6,7 @@
> #include <linux/usb.h>
> #include <linux/usb/ch9.h>
> #include <linux/usb/hcd.h>
> +#include <linux/usb/of.h>
> #include <linux/usb/quirks.h>
> #include <linux/module.h>
> #include <linux/slab.h>
> @@ -257,6 +258,11 @@ static int usb_parse_endpoint(struct device *ddev, int cfgno, int inum,
> memcpy(&endpoint->desc, d, n);
> INIT_LIST_HEAD(&endpoint->urb_list);
>
> + /* device node property overrides bInterval */
> + if (unlikely(usb_of_has_combined_node(to_usb_device(ddev))))

Only ever use likely/unlikely if you can measure the difference with a
benchmark. If not, then never use it as the compiler and CPU will
almost always get it right and more correct than you can.

And for code that has no performance issues/impact like this one, never
do it.

thanks,

greg k-h