Re: [PATCH] usb: gadget/legacy/ether: assume saner default power mode

Felipe Balbi
2021-09-17T05:32:01Z

Hi,

Andreas Bauer <andreas.bauer.nexus@gmail.com> writes:

> I propose the included patch to the OTG USB ethernet gadget code:
>
> Report default of bus powered and 500mA bMaxPower consumption.
>
> Reason:
>
> The USB spec requires all devices consuming more than 100mA from
> the bus to report in this manner. Devices such as Rpi Zero can operate
> in this mode and will then falsly report being self-powered when they
> are not.
>
> The more sane default is to report being 500mA bus-powered than being
> self-powered
>
> This patch was born from an issue within the Raspberry Pi Zero ecosystem,
> therefore I would invite comment regarding other usage of this OTG code.

Have a look at other commits to get examples of how to write commits for
upstream. You're missing your Signed-off-by, adding extra unnecessary
spaces and making your signature show up in the commit log. All of
these are described in kernel documentation this
(https://www.kernel.org/doc/html/latest/process/submitting-patches.html)
is a good starting point.

> $ diff -u linux-5.14.2/drivers/usb/gadget/legacy/ether.c.orig linux-5.14.2/drivers/usb/gadget/legacy/ether.c
> --- linux-5.14.2/drivers/usb/gadget/legacy/ether.c.orig 2021-09-08 13:52:41.000000000 +0700
> +++ linux-5.14.2/drivers/usb/gadget/legacy/ether.c 2021-09-16 21:25:06.782958554 +0700
> @@ -296,7 +296,8 @@
> /* .label = f(hardware) */
> .bConfigurationValue = 1,
> /* .iConfiguration = DYNAMIC */
> - .bmAttributes = USB_CONFIG_ATT_SELFPOWER,
> + .bmAttributes = 0, /* bus powered implied */
> + .bMaxPower = 250, /* 500mA in 2mA units */

right, your "sane" default now prevents this gadget driver from working
behind bus powered hubs. Considering that a linux-based device is likely
to have its own battery, this is not really a sane default.

The default value of "self powered", seems much saner ;-)

--
balbi