On 2019-10-23 00:49, Felipe Balbi wrote:
Hi,
Jack Pham <jackp@xxxxxxxxxxxxxx> 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.
The round up causes 900 on the input side to be greater than 900 when
doing the
reverse, i.e. multiplication by 8.
Alternatively we could just do a normal integer division here
(effectively
round down).
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@xxxxxxxxxxxxxx>
---
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?
Right, but now on the host it will do the reverse calculation, i.e.
113*8 == 904mA.
For a root port this would be greater than it's maximum power budget of
900mA and
would result in not selecting the config.
