All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] usb: dwc2: Fix SET/CLEAR_FEATURE and GET_STATUS flows
@ 2019-12-16 17:14 Minas Harutyunyan
  2020-01-15 13:21 ` Minas Harutyunyan
  0 siblings, 1 reply; 4+ messages in thread
From: Minas Harutyunyan @ 2019-12-16 17:14 UTC (permalink / raw)
  To: Felipe Balbi, Greg Kroah-Hartman, Minas Harutyunyan, linux-usb
  Cc: John Youn, stable, Jack Mitchell

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:
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] usb: dwc2: Fix SET/CLEAR_FEATURE and GET_STATUS flows
  2019-12-16 17:14 [PATCH v2] usb: dwc2: Fix SET/CLEAR_FEATURE and GET_STATUS flows Minas Harutyunyan
@ 2020-01-15 13:21 ` Minas Harutyunyan
  2020-01-15 13:27   ` Felipe Balbi
  0 siblings, 1 reply; 4+ messages in thread
From: Minas Harutyunyan @ 2020-01-15 13:21 UTC (permalink / raw)
  To: Felipe Balbi, Greg Kroah-Hartman, linux-usb
  Cc: John Youn, stable, Jack Mitchell

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] usb: dwc2: Fix SET/CLEAR_FEATURE and GET_STATUS flows
  2020-01-15 13:21 ` Minas Harutyunyan
@ 2020-01-15 13:27   ` Felipe Balbi
  0 siblings, 0 replies; 4+ messages in thread
From: Felipe Balbi @ 2020-01-15 13:27 UTC (permalink / raw)
  To: Minas Harutyunyan, Greg Kroah-Hartman, linux-usb
  Cc: John Youn, stable, Jack Mitchell

[-- Attachment #1: Type: text/plain, Size: 861 bytes --]


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

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v2] usb: dwc2: Fix SET/CLEAR_FEATURE and GET_STATUS flows
@ 2020-01-21 10:17 Minas Harutyunyan
  0 siblings, 0 replies; 4+ messages in thread
From: Minas Harutyunyan @ 2020-01-21 10:17 UTC (permalink / raw)
  To: Felipe Balbi, Greg Kroah-Hartman, Minas Harutyunyan, linux-usb
  Cc: John Youn, Vardan Mikayelyan, Grigor Tovmasyan, stable, Jack Mitchell

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

 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:
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2020-01-21 10:17 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-16 17:14 [PATCH v2] usb: dwc2: Fix SET/CLEAR_FEATURE and GET_STATUS flows Minas Harutyunyan
2020-01-15 13:21 ` Minas Harutyunyan
2020-01-15 13:27   ` Felipe Balbi
2020-01-21 10:17 Minas Harutyunyan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.