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

[Minas Harutyunyan <Minas.Harutyunyan@synopsys.com>]

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