On Tue, Apr 22, 2025, Prashanth K wrote:
> Currently when the host sends GET_STATUS request for an interface,
> we use get_status callbacks to set/clear remote wakeup capability
> of that interface. And if get_status callback isn't present for
> that interface, then we assume its remote wakeup capability based
> on bmAttributes.
>
> Now consider a scenario, where we have a USB configuration with
> multiple interfaces (say ECM + ADB), here ECM is remote wakeup
> capable and as of now ADB isn't. And bmAttributes will indicate
> the device as wakeup capable. With the current implementation,
> when host sends GET_STATUS request for both interfaces, we will
> set FUNC_RW_CAP for both. This results in USB3 CV Chapter 9.15
> (Function Remote Wakeup Test) failures as host expects remote
> wakeup from both interfaces.
>
> The above scenario is just an example, and the failure can be
> observed if we use configuration with any interface except ECM.
> Hence avoid configuring remote wakeup capability from composite
> driver based on bmAttributes, instead use get_status callbacks
> and let the function drivers decide this.
>
> Cc: stable@xxxxxxxxxx
> Fixes: 481c225c4802 ("usb: gadget: Handle function suspend feature selector")
> Signed-off-by: Prashanth K <prashanth.k@xxxxxxxxxxxxxxxx>
> ---
> drivers/usb/gadget/composite.c | 12 +++++-------
> 1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
> index 869ad99afb48..8dbc132a505e 100644
> --- a/drivers/usb/gadget/composite.c
> +++ b/drivers/usb/gadget/composite.c
> @@ -2011,15 +2011,13 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
>
> if (f->get_status) {
> status = f->get_status(f);
> +
> if (status < 0)
> break;
> - } else {
> - /* Set D0 and D1 bits based on func wakeup capability */
> - if (f->config->bmAttributes & USB_CONFIG_ATT_WAKEUP) {
> - status |= USB_INTRF_STAT_FUNC_RW_CAP;
> - if (f->func_wakeup_armed)
> - status |= USB_INTRF_STAT_FUNC_RW;
> - }
> +
> + /* if D5 is not set, then device is not wakeup capable */
> + if (!(f->config->bmAttributes & USB_CONFIG_ATT_WAKEUP))
> + status &= ~(USB_INTRF_STAT_FUNC_RW_CAP | USB_INTRF_STAT_FUNC_RW);
> }
>
> put_unaligned_le16(status & 0x0000ffff, req->buf);
> --
> 2.25.1
>
Reviewed-by: Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx>
Thanks,
Thinh
