| Message ID | 38a93a8deedd76dbc22bb1e41b4fc998f3750c95.1576516371.git.hminas@synopsys.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] usb: dwc2: Fix SET/CLEAR_FEATURE and GET_STATUS flows | expand |
Hi Felipe, On 12/16/2019 9:14 PM, Minas Harutyunyan wrote: > SET/CLEAR_FEATURE for Remote Wakeup allowance not handled correctly. > GET_STATUS handling provided not correct data on DATA Stage. > Issue seen when gadget's dr_mode set to "otg" mode and connected > to MacOS. > Both are fixed and tested using USBCV Ch.9 tests. > > Signed-off-by: Minas Harutyunyan <hminas@synopsys.com> > Fixes: fa389a6d7726 ("usb: dwc2: gadget: Add remote_wakeup_allowed flag") > Tested-by: Jack Mitchell <ml@embed.me.uk> > Cc: stable@vger.kernel.org > --- > Changes in v2: > - Add Fixes tag > - Add Tested-by tag > > Signed-off-by: Minas Harutyunyan <hminas@synopsys.com> > --- > drivers/usb/dwc2/gadget.c | 28 ++++++++++++++++------------ > 1 file changed, 16 insertions(+), 12 deletions(-) > > diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c > index 6be10e496e10..3a6176c22371 100644 > --- a/drivers/usb/dwc2/gadget.c > +++ b/drivers/usb/dwc2/gadget.c > @@ -1632,6 +1632,7 @@ static int dwc2_hsotg_process_req_status(struct dwc2_hsotg *hsotg, > struct dwc2_hsotg_ep *ep0 = hsotg->eps_out[0]; > struct dwc2_hsotg_ep *ep; > __le16 reply; > + u16 status; > int ret; > > dev_dbg(hsotg->dev, "%s: USB_REQ_GET_STATUS\n", __func__); > @@ -1643,11 +1644,10 @@ static int dwc2_hsotg_process_req_status(struct dwc2_hsotg *hsotg, > > switch (ctrl->bRequestType & USB_RECIP_MASK) { > case USB_RECIP_DEVICE: > - /* > - * bit 0 => self powered > - * bit 1 => remote wakeup > - */ > - reply = cpu_to_le16(0); > + status = 1 << USB_DEVICE_SELF_POWERED; > + status |= hsotg->remote_wakeup_allowed << > + USB_DEVICE_REMOTE_WAKEUP; > + reply = cpu_to_le16(status); > break; > > case USB_RECIP_INTERFACE: > @@ -1758,7 +1758,10 @@ static int dwc2_hsotg_process_req_feature(struct dwc2_hsotg *hsotg, > case USB_RECIP_DEVICE: > switch (wValue) { > case USB_DEVICE_REMOTE_WAKEUP: > - hsotg->remote_wakeup_allowed = 1; > + if (set) > + hsotg->remote_wakeup_allowed = 1; > + else > + hsotg->remote_wakeup_allowed = 0; > break; > > case USB_DEVICE_TEST_MODE: > @@ -1768,16 +1771,17 @@ static int dwc2_hsotg_process_req_feature(struct dwc2_hsotg *hsotg, > return -EINVAL; > > hsotg->test_mode = wIndex >> 8; > - ret = dwc2_hsotg_send_reply(hsotg, ep0, NULL, 0); > - if (ret) { > - dev_err(hsotg->dev, > - "%s: failed to send reply\n", __func__); > - return ret; > - } > break; > default: > return -ENOENT; > } > + > + ret = dwc2_hsotg_send_reply(hsotg, ep0, NULL, 0); > + if (ret) { > + dev_err(hsotg->dev, > + "%s: failed to send reply\n", __func__); > + return ret; > + } > break; > > case USB_RECIP_ENDPOINT: > What about this patch? Thanks, Minas
Hi, Minas Harutyunyan <Minas.Harutyunyan@synopsys.com> writes: >> @@ -1768,16 +1771,17 @@ static int dwc2_hsotg_process_req_feature(struct dwc2_hsotg *hsotg, >> return -EINVAL; >> >> hsotg->test_mode = wIndex >> 8; >> - ret = dwc2_hsotg_send_reply(hsotg, ep0, NULL, 0); >> - if (ret) { >> - dev_err(hsotg->dev, >> - "%s: failed to send reply\n", __func__); >> - return ret; >> - } >> break; >> default: >> return -ENOENT; >> } >> + >> + ret = dwc2_hsotg_send_reply(hsotg, ep0, NULL, 0); >> + if (ret) { >> + dev_err(hsotg->dev, >> + "%s: failed to send reply\n", __func__); >> + return ret; >> + } >> break; >> >> case USB_RECIP_ENDPOINT: >> > > What about this patch? See: https://lore.kernel.org/linux-usb/875zhd6pw0.fsf@kernel.org/T/#u
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c index 6be10e496e10..3a6176c22371 100644 --- a/drivers/usb/dwc2/gadget.c +++ b/drivers/usb/dwc2/gadget.c @@ -1632,6 +1632,7 @@ static int dwc2_hsotg_process_req_status(struct dwc2_hsotg *hsotg, struct dwc2_hsotg_ep *ep0 = hsotg->eps_out[0]; struct dwc2_hsotg_ep *ep; __le16 reply; + u16 status; int ret; dev_dbg(hsotg->dev, "%s: USB_REQ_GET_STATUS\n", __func__); @@ -1643,11 +1644,10 @@ static int dwc2_hsotg_process_req_status(struct dwc2_hsotg *hsotg, switch (ctrl->bRequestType & USB_RECIP_MASK) { case USB_RECIP_DEVICE: - /* - * bit 0 => self powered - * bit 1 => remote wakeup - */ - reply = cpu_to_le16(0); + status = 1 << USB_DEVICE_SELF_POWERED; + status |= hsotg->remote_wakeup_allowed << + USB_DEVICE_REMOTE_WAKEUP; + reply = cpu_to_le16(status); break; case USB_RECIP_INTERFACE: @@ -1758,7 +1758,10 @@ static int dwc2_hsotg_process_req_feature(struct dwc2_hsotg *hsotg, case USB_RECIP_DEVICE: switch (wValue) { case USB_DEVICE_REMOTE_WAKEUP: - hsotg->remote_wakeup_allowed = 1; + if (set) + hsotg->remote_wakeup_allowed = 1; + else + hsotg->remote_wakeup_allowed = 0; break; case USB_DEVICE_TEST_MODE: @@ -1768,16 +1771,17 @@ static int dwc2_hsotg_process_req_feature(struct dwc2_hsotg *hsotg, return -EINVAL; hsotg->test_mode = wIndex >> 8; - ret = dwc2_hsotg_send_reply(hsotg, ep0, NULL, 0); - if (ret) { - dev_err(hsotg->dev, - "%s: failed to send reply\n", __func__); - return ret; - } break; default: return -ENOENT; } + + ret = dwc2_hsotg_send_reply(hsotg, ep0, NULL, 0); + if (ret) { + dev_err(hsotg->dev, + "%s: failed to send reply\n", __func__); + return ret; + } break; case USB_RECIP_ENDPOINT: