Re: [PATCH 2/2] usb: gadget: composite: Support more than 500mA MaxPower

[Felipe Balbi <balbi@kernel.org>]

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


Hi,

Jack Pham <jackp@codeaurora.org> writes:
> USB 3.x SuperSpeed peripherals can draw up to 900mA of VBUS power
> when in configured state. However, if a configuration wanting to
> take advantage of this is added with MaxPower greater than 500
> (currently possible if using a ConfigFS gadget) the composite
> driver fails to accommodate this for a couple reasons:
>
>  - usb_gadget_vbus_draw() when called from set_config() and
>    composite_resume() will be passed the MaxPower value without
>    regard for the current connection speed, resulting in a
>    violation for USB 2.0 since the max is 500mA.
>
>  - the bMaxPower of the configuration descriptor would be
>    incorrectly encoded, again if the connection speed is only
>    at USB 2.0 or below, likely wrapping around UINT8_MAX since
>    the 2mA multiplier corresponds to a maximum of 610mA.
>
> Fix these by adding checks against the current gadget->speed
> when the c->MaxPower value is used and appropriately limit
> based on whether it is currently at a low-/full-/high- or super-
> speed connection.
>
> Incidentally, 900 is not divisble by 8, so even for super-speed
> the bMaxPower neds to be capped at 896mA, otherwise due to the
                ^^^^
                needs

Why do you need to cap it? DIV_ROUND_UP() should still work.

> round-up division a MaxPower of 900 will result in an encoded
> value of 904mA and many host stacks (including Linux and Windows)
> of a root port will reject the configuration.
>
> N.B. USB 3.2 Gen N x 2 allows for up to 1500mA but there doesn't
> seem to be any any peripheral controller supported by Linux that
> does two lane operation, so for now keeping the clamp at 900
> should be fine.
>
> Signed-off-by: Jack Pham <jackp@codeaurora.org>
> ---
>  drivers/usb/gadget/composite.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
> index e1db94d1fe2e..92ce3018f482 100644
> --- a/drivers/usb/gadget/composite.c
> +++ b/drivers/usb/gadget/composite.c
> @@ -438,9 +438,10 @@ static u8 encode_bMaxPower(enum usb_device_speed speed,
>  	if (!val)
>  		return 0;
>  	if (speed < USB_SPEED_SUPER)
> -		return DIV_ROUND_UP(val, 2);
> +		return DIV_ROUND_UP(min(val, 500U), 2);
>  	else
> -		return DIV_ROUND_UP(val, 8);
> +		/* USB 3.x supports 900mA, but that isn't divisible by 8... */
> +		return DIV_ROUND_UP(min(val, 896U), 8);

DIV_ROUND_UP(896, 8) = 112
DIV_ROUND_UP(900, 8) = 113

Why value do you want here?

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]