diff mbox series

usb: dwc2: Fix SET/CLEAR_FEATURE and GET_STATUS flows

Message ID f8af9e4b892a8d005f0ae3d42401fee532f44a27.1574938826.git.hminas@synopsys.com (mailing list archive)
State Superseded
Headers show
Series usb: dwc2: Fix SET/CLEAR_FEATURE and GET_STATUS flows | expand

Commit Message

Minas Harutyunyan Nov. 28, 2019, 11:21 a.m. UTC
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>
---
 drivers/usb/dwc2/gadget.c | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

Comments

Felipe Balbi Dec. 10, 2019, 12:53 p.m. UTC | #1
Hi,

Minas Harutyunyan <Minas.Harutyunyan@synopsys.com> writes:

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

do you want to add a Fixes tag here?
Minas Harutyunyan Dec. 10, 2019, 2:19 p.m. UTC | #2
Hi Felipe,

On 12/10/2019 4:53 PM, Felipe Balbi wrote:
> 
> Hi,
> 
> Minas Harutyunyan <Minas.Harutyunyan@synopsys.com> writes:
> 
>> 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>
> 
> do you want to add a Fixes tag here?

Fixes: fa389a6d7726 ("usb: dwc2: gadget: Add remote_wakeup_allowed flag")

Thanks,
Minas

>
Minas Harutyunyan Dec. 13, 2019, 11:54 a.m. UTC | #3
Hi Felipe,

On 12/10/2019 6:19 PM, Minas Harutyunyan wrote:
> Hi Felipe,
> 
> On 12/10/2019 4:53 PM, Felipe Balbi wrote:
>>
>> Hi,
>>
>> Minas Harutyunyan <Minas.Harutyunyan@synopsys.com> writes:
>>
>>> 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>
>>
>> do you want to add a Fixes tag here?
> 
> Fixes: fa389a6d7726 ("usb: dwc2: gadget: Add remote_wakeup_allowed flag")
> 
Got tested tag by issue reported.

Tested-By: Jack Mitchell <ml@embed.me.uk>


Should I resubmit patch as v2 with added "Fixes" and "Tested-by" tags.

> Thanks,
> Minas
> 
>>
John Keeping Feb. 4, 2020, 11:45 a.m. UTC | #4
Hi Minas,

On Fri, 13 Dec 2019 11:54:11 +0000
Minas Harutyunyan <Minas.Harutyunyan@synopsys.com> wrote:
> On 12/10/2019 6:19 PM, Minas Harutyunyan wrote:
> > On 12/10/2019 4:53 PM, Felipe Balbi wrote:  
> >> Minas Harutyunyan <Minas.Harutyunyan@synopsys.com> writes:
> >>  
> >>> 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>  
> >>
> >> do you want to add a Fixes tag here?  
> > 
> > Fixes: fa389a6d7726 ("usb: dwc2: gadget: Add remote_wakeup_allowed flag")
> >   
> Got tested tag by issue reported.
> 
> Tested-By: Jack Mitchell <ml@embed.me.uk>
> 
> Should I resubmit patch as v2 with added "Fixes" and "Tested-by" tags.

Was this patch ever picked up?  I don't see it in current next.

Given Felipe's message [1], I guess it might have got lost.

Would you mind resending?

[1] https://lore.kernel.org/linux-usb/875zhd6pw0.fsf@kernel.org/


Thanks,
John
Minas Harutyunyan Feb. 4, 2020, 11:50 a.m. UTC | #5
Hi John,


On 2/4/2020 3:45 PM, John Keeping wrote:
> Hi Minas,
> 
> On Fri, 13 Dec 2019 11:54:11 +0000
> Minas Harutyunyan <Minas.Harutyunyan@synopsys.com> wrote:
>> On 12/10/2019 6:19 PM, Minas Harutyunyan wrote:
>>> On 12/10/2019 4:53 PM, Felipe Balbi wrote:
>>>> Minas Harutyunyan <Minas.Harutyunyan@synopsys.com> writes:
>>>>   
>>>>> 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>
>>>>
>>>> do you want to add a Fixes tag here?
>>>
>>> Fixes: fa389a6d7726 ("usb: dwc2: gadget: Add remote_wakeup_allowed flag")
>>>    
>> Got tested tag by issue reported.
>>
>> Tested-By: Jack Mitchell <ml@embed.me.uk>
>>
>> Should I resubmit patch as v2 with added "Fixes" and "Tested-by" tags.
> 
> Was this patch ever picked up?  I don't see it in current next.
> 
Yes, it picked up by Felipe. It under testing/fixes

https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git/commit/?h=testing/fixes&id=6de1e301b9cf8eb023357586ab8b02edda07320b

Thanks,
Minas

> Given Felipe's message [1], I guess it might have got lost.
> 
> Would you mind resending?
> 
> [1] https://urldefense.proofpoint.com/v2/url?u=https-3A__lore.kernel.org_linux-2Dusb_875zhd6pw0.fsf-40kernel.org_&d=DwICAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=6z9Al9FrHR_ZqbbtSAsD16pvOL2S3XHxQnSzq8kusyI&m=D4RlPf0cgyzMpc-hKhI1giEWZJGcxNJFJWC5xjDVyoI&s=pw7tlVwt7OpHtYj_WpGG3K9WTodfF6FUWvyHoJ4__co&e=
> 
> 
> Thanks,
> John
>
John Keeping Feb. 4, 2020, 3:04 p.m. UTC | #6
On Tue, 4 Feb 2020 11:50:12 +0000
Minas Harutyunyan <Minas.Harutyunyan@synopsys.com> wrote:

> On 2/4/2020 3:45 PM, John Keeping wrote:
> > On Fri, 13 Dec 2019 11:54:11 +0000
> > Minas Harutyunyan <Minas.Harutyunyan@synopsys.com> wrote:  
> >> On 12/10/2019 6:19 PM, Minas Harutyunyan wrote:  
> >>> On 12/10/2019 4:53 PM, Felipe Balbi wrote:  
> >>>> Minas Harutyunyan <Minas.Harutyunyan@synopsys.com> writes:
> >>>>     
> >>>>> 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>  
> >>>>
> >>>> do you want to add a Fixes tag here?  
> >>>
> >>> Fixes: fa389a6d7726 ("usb: dwc2: gadget: Add remote_wakeup_allowed flag")
> >>>      
> >> Got tested tag by issue reported.
> >>
> >> Tested-By: Jack Mitchell <ml@embed.me.uk>
> >>
> >> Should I resubmit patch as v2 with added "Fixes" and "Tested-by" tags.  
> > 
> > Was this patch ever picked up?  I don't see it in current next.
> >   
> Yes, it picked up by Felipe. It under testing/fixes
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git/commit/?h=testing/fixes&id=6de1e301b9cf8eb023357586ab8b02edda07320b

Thanks!  I'll try to remember to look there next time.


Sorry for the noise,
John
diff mbox series

Patch

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: