Re: [PATCH v2] usb: dwc2: Fix SET/CLEAR_FEATURE and GET_STATUS flows

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