[PATCH v2] usb: dwc2: drd: fix inconsistent mode if role-switch-default-mode="host"

[PATCH v2] usb: dwc2: drd: fix inconsistent mode if role-switch-default-mode="host"

[Ziyang Huang]

Some boards might use USB-A female connector for USB ports, however,
the port could be connected to a dual-mode USB controller, making it
also behaves as a peripheral device if male-to-male cable is connected.

In this case, the dts looks like this:

	&usb0 {
		status = "okay";
		dr_mode = "otg";
		usb-role-switch;
		role-switch-default-mode = "host";
	};

After boot, dwc2_ovr_init() sets GOTGCTL to GOTGCTL_AVALOVAL and call
dwc2_force_mode() with parameter host=false, which causes inconsistent
mode - The hardware is in peripheral mode while the kernel status is
in host mode.

What we can do now is to call dwc2_drd_role_sw_set() to switch to
device mode, and everything should work just fine now, even switching
back to none(default) mode afterwards.

Fixes: e14acb876985 ("usb: dwc2: drd: add role-switch-default-node support")
Signed-off-by: Ziyang Huang <hzyitc@outlook.com>
---
Changes since v1
- Use corrent name in Signed-off-by

 drivers/usb/dwc2/drd.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/dwc2/drd.c b/drivers/usb/dwc2/drd.c
index d8d6493bc457..a8605b02115b 100644
--- a/drivers/usb/dwc2/drd.c
+++ b/drivers/usb/dwc2/drd.c
@@ -35,7 +35,8 @@ static void dwc2_ovr_init(struct dwc2_hsotg *hsotg)
 
 	spin_unlock_irqrestore(&hsotg->lock, flags);
 
-	dwc2_force_mode(hsotg, (hsotg->dr_mode == USB_DR_MODE_HOST));
+	dwc2_force_mode(hsotg, (hsotg->dr_mode == USB_DR_MODE_HOST) ||
+				(hsotg->role_sw_default_mode == USB_DR_MODE_HOST));
 }
 
 static int dwc2_ovr_avalid(struct dwc2_hsotg *hsotg, bool valid)
-- 
2.34.1

Re: [PATCH v2] usb: dwc2: drd: fix inconsistent mode if role-switch-default-mode="host"

[Greg KH]

On Tue, Feb 21, 2023 at 06:30:04PM +0800, Ziyang Huang wrote:
> Some boards might use USB-A female connector for USB ports, however,
> the port could be connected to a dual-mode USB controller, making it
> also behaves as a peripheral device if male-to-male cable is connected.
> 
> In this case, the dts looks like this:
> 
> 	&usb0 {
> 		status = "okay";
> 		dr_mode = "otg";
> 		usb-role-switch;
> 		role-switch-default-mode = "host";
> 	};
> 
> After boot, dwc2_ovr_init() sets GOTGCTL to GOTGCTL_AVALOVAL and call
> dwc2_force_mode() with parameter host=false, which causes inconsistent
> mode - The hardware is in peripheral mode while the kernel status is
> in host mode.
> 
> What we can do now is to call dwc2_drd_role_sw_set() to switch to
> device mode, and everything should work just fine now, even switching
> back to none(default) mode afterwards.
> 
> Fixes: e14acb876985 ("usb: dwc2: drd: add role-switch-default-node support")
> Signed-off-by: Ziyang Huang <hzyitc@outlook.com>
> ---
> Changes since v1
> - Use corrent name in Signed-off-by

Nope, still incorrect, please use your synopsys address.

thanks,

greg k-h

Re: [PATCH v2] usb: dwc2: drd: fix inconsistent mode if role-switch-default-mode="host"

[Ziyang Huang]

在 2023/2/21 18:41, Greg KH 写道:
> On Tue, Feb 21, 2023 at 06:30:04PM +0800, Ziyang Huang wrote:
>> Some boards might use USB-A female connector for USB ports, however,
>> the port could be connected to a dual-mode USB controller, making it
>> also behaves as a peripheral device if male-to-male cable is connected.
>>
>> In this case, the dts looks like this:
>>
>> 	&usb0 {
>> 		status = "okay";
>> 		dr_mode = "otg";
>> 		usb-role-switch;
>> 		role-switch-default-mode = "host";
>> 	};
>>
>> After boot, dwc2_ovr_init() sets GOTGCTL to GOTGCTL_AVALOVAL and call
>> dwc2_force_mode() with parameter host=false, which causes inconsistent
>> mode - The hardware is in peripheral mode while the kernel status is
>> in host mode.
>>
>> What we can do now is to call dwc2_drd_role_sw_set() to switch to
>> device mode, and everything should work just fine now, even switching
>> back to none(default) mode afterwards.
>>
>> Fixes: e14acb876985 ("usb: dwc2: drd: add role-switch-default-node support")
>> Signed-off-by: Ziyang Huang <hzyitc@outlook.com>
>> ---
>> Changes since v1
>> - Use corrent name in Signed-off-by
> Nope, still incorrect, please use your synopsys address.
>
> thanks,
>
> greg k-h


Oh, I'm not a Synopsys employee but a free developer. This is my first 
time submitting a kernel patch, please excuse me. Thank you.

Re: [PATCH v2] usb: dwc2: drd: fix inconsistent mode if role-switch-default-mode="host"

[Greg KH]

On Tue, Feb 21, 2023 at 08:33:32PM +0800, Ziyang Huang wrote:
> 
> 在 2023/2/21 18:41, Greg KH 写道:
> > On Tue, Feb 21, 2023 at 06:30:04PM +0800, Ziyang Huang wrote:
> > > Some boards might use USB-A female connector for USB ports, however,
> > > the port could be connected to a dual-mode USB controller, making it
> > > also behaves as a peripheral device if male-to-male cable is connected.
> > > 
> > > In this case, the dts looks like this:
> > > 
> > > 	&usb0 {
> > > 		status = "okay";
> > > 		dr_mode = "otg";
> > > 		usb-role-switch;
> > > 		role-switch-default-mode = "host";
> > > 	};
> > > 
> > > After boot, dwc2_ovr_init() sets GOTGCTL to GOTGCTL_AVALOVAL and call
> > > dwc2_force_mode() with parameter host=false, which causes inconsistent
> > > mode - The hardware is in peripheral mode while the kernel status is
> > > in host mode.
> > > 
> > > What we can do now is to call dwc2_drd_role_sw_set() to switch to
> > > device mode, and everything should work just fine now, even switching
> > > back to none(default) mode afterwards.
> > > 
> > > Fixes: e14acb876985 ("usb: dwc2: drd: add role-switch-default-node support")
> > > Signed-off-by: Ziyang Huang <hzyitc@outlook.com>
> > > ---
> > > Changes since v1
> > > - Use corrent name in Signed-off-by
> > Nope, still incorrect, please use your synopsys address.
> > 
> > thanks,
> > 
> > greg k-h
> 
> 
> Oh, I'm not a Synopsys employee but a free developer. This is my first time
> submitting a kernel patch, please excuse me. Thank you.

Ah, my fault, sorry, I saw the synopsys email on the to: line and
thought it was you.  Nevermind then, sorry, this will be reviewed once
6.3-rc1 is out.

thanks,

greg k-h

RE: [PATCH v2] usb: dwc2: drd: fix inconsistent mode if role-switch-default-mode="host"

[Minas Harutyunyan]

Hi Amelie,

Could you please review and test this patch on your setup.
Doesn't broke anything.

Thanks,
Minas

On 2/21/2023 2:30 PM, Ziyang Huang <hzyitc@outlook.com> wrote:
>From: Ziyang Huang <hzyitc@outlook.com>
>Sent: Tuesday, February 21, 2023 2:30 PM
>To: Minas Harutyunyan <hminas@synopsys.com>
>Cc: gregkh@linuxfoundation.org; fabrice.gasnier@foss.st.com;
>amelie.delaunay@foss.st.com; linux-usb@vger.kernel.org; linux-
>kernel@vger.kernel.org; Ziyang Huang <hzyitc@outlook.com>
>Subject: [PATCH v2] usb: dwc2: drd: fix inconsistent mode if role-switch-
>default-mode="host"
>
>Some boards might use USB-A female connector for USB ports, however, the
>port could be connected to a dual-mode USB controller, making it also
>behaves as a peripheral device if male-to-male cable is connected.
>
>In this case, the dts looks like this:
>
>	&usb0 {
>		status = "okay";
>		dr_mode = "otg";
>		usb-role-switch;
>		role-switch-default-mode = "host";
>	};
>
>After boot, dwc2_ovr_init() sets GOTGCTL to GOTGCTL_AVALOVAL and call
>dwc2_force_mode() with parameter host=false, which causes inconsistent mode
>- The hardware is in peripheral mode while the kernel status is in host
>mode.
>
>What we can do now is to call dwc2_drd_role_sw_set() to switch to device
>mode, and everything should work just fine now, even switching back to
>none(default) mode afterwards.
>
>Fixes: e14acb876985 ("usb: dwc2: drd: add role-switch-default-node support")
>Signed-off-by: Ziyang Huang <hzyitc@outlook.com>
>---
>Changes since v1
>- Use corrent name in Signed-off-by
>
> drivers/usb/dwc2/drd.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/usb/dwc2/drd.c b/drivers/usb/dwc2/drd.c index
>d8d6493bc457..a8605b02115b 100644
>--- a/drivers/usb/dwc2/drd.c
>+++ b/drivers/usb/dwc2/drd.c
>@@ -35,7 +35,8 @@ static void dwc2_ovr_init(struct dwc2_hsotg *hsotg)
>
> 	spin_unlock_irqrestore(&hsotg->lock, flags);
>
>-	dwc2_force_mode(hsotg, (hsotg->dr_mode == USB_DR_MODE_HOST));
>+	dwc2_force_mode(hsotg, (hsotg->dr_mode == USB_DR_MODE_HOST) ||
>+				(hsotg->role_sw_default_mode == USB_DR_MODE_HOST));
> }
>
> static int dwc2_ovr_avalid(struct dwc2_hsotg *hsotg, bool valid)
>--
>2.34.1

Re: [PATCH v2] usb: dwc2: drd: fix inconsistent mode if role-switch-default-mode="host"

[Fabrice Gasnier]

On 2/22/23 08:00, Minas Harutyunyan wrote:
> Hi Amelie,
> 
> Could you please review and test this patch on your setup.
> Doesn't broke anything.
> 

Hi Minas,

I tested at my end on behalf of Amelie.
Of course I let her possibly comment.

This doesn't break anything on existing boards. I also tested the
example described by Ziyang in the commit message.

You can add my:
Tested-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>

Thanks & BR,
Fabrice

> Thanks,
> Minas
> 
> On 2/21/2023 2:30 PM, Ziyang Huang <hzyitc@outlook.com> wrote:
>> From: Ziyang Huang <hzyitc@outlook.com>
>> Sent: Tuesday, February 21, 2023 2:30 PM
>> To: Minas Harutyunyan <hminas@synopsys.com>
>> Cc: gregkh@linuxfoundation.org; fabrice.gasnier@foss.st.com;
>> amelie.delaunay@foss.st.com; linux-usb@vger.kernel.org; linux-
>> kernel@vger.kernel.org; Ziyang Huang <hzyitc@outlook.com>
>> Subject: [PATCH v2] usb: dwc2: drd: fix inconsistent mode if role-switch-
>> default-mode="host"
>>
>> Some boards might use USB-A female connector for USB ports, however, the
>> port could be connected to a dual-mode USB controller, making it also
>> behaves as a peripheral device if male-to-male cable is connected.
>>
>> In this case, the dts looks like this:
>>
>> 	&usb0 {
>> 		status = "okay";
>> 		dr_mode = "otg";
>> 		usb-role-switch;
>> 		role-switch-default-mode = "host";
>> 	};
>>
>> After boot, dwc2_ovr_init() sets GOTGCTL to GOTGCTL_AVALOVAL and call
>> dwc2_force_mode() with parameter host=false, which causes inconsistent mode
>> - The hardware is in peripheral mode while the kernel status is in host
>> mode.
>>
>> What we can do now is to call dwc2_drd_role_sw_set() to switch to device
>> mode, and everything should work just fine now, even switching back to
>> none(default) mode afterwards.
>>
>> Fixes: e14acb876985 ("usb: dwc2: drd: add role-switch-default-node support")
>> Signed-off-by: Ziyang Huang <hzyitc@outlook.com>
>> ---
>> Changes since v1
>> - Use corrent name in Signed-off-by
>>
>> drivers/usb/dwc2/drd.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/dwc2/drd.c b/drivers/usb/dwc2/drd.c index
>> d8d6493bc457..a8605b02115b 100644
>> --- a/drivers/usb/dwc2/drd.c
>> +++ b/drivers/usb/dwc2/drd.c
>> @@ -35,7 +35,8 @@ static void dwc2_ovr_init(struct dwc2_hsotg *hsotg)
>>
>> 	spin_unlock_irqrestore(&hsotg->lock, flags);
>>
>> -	dwc2_force_mode(hsotg, (hsotg->dr_mode == USB_DR_MODE_HOST));
>> +	dwc2_force_mode(hsotg, (hsotg->dr_mode == USB_DR_MODE_HOST) ||
>> +				(hsotg->role_sw_default_mode == USB_DR_MODE_HOST));
>> }
>>
>> static int dwc2_ovr_avalid(struct dwc2_hsotg *hsotg, bool valid)
>> --
>> 2.34.1
> 

RE: [PATCH v2] usb: dwc2: drd: fix inconsistent mode if role-switch-default-mode="host"

[Minas Harutyunyan]

Hi Fabrice,

>-----Original Message-----
>From: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
>Sent: Wednesday, March 1, 2023 3:26 PM
>To: Minas Harutyunyan <hminas@synopsys.com>; Ziyang Huang
><hzyitc@outlook.com>; amelie.delaunay@foss.st.com
>Cc: gregkh@linuxfoundation.org; linux-usb@vger.kernel.org; linux-
>kernel@vger.kernel.org
>Subject: Re: [PATCH v2] usb: dwc2: drd: fix inconsistent mode if role-
>switch-default-mode="host"
>
>On 2/22/23 08:00, Minas Harutyunyan wrote:
>> Hi Amelie,
>>
>> Could you please review and test this patch on your setup.
>> Doesn't broke anything.
>>
>
>Hi Minas,
>
>I tested at my end on behalf of Amelie.
>Of course I let her possibly comment.
>
>This doesn't break anything on existing boards. I also tested the example
>described by Ziyang in the commit message.
>
>You can add my:
>Tested-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
>
>Thanks & BR,
>Fabrice
>

Thank you very much.

Minas

Re: [PATCH v2] usb: dwc2: drd: fix inconsistent mode if role-switch-default-mode="host"

[Amelie Delaunay]

On 3/1/23 12:26, Fabrice Gasnier wrote:
> On 2/22/23 08:00, Minas Harutyunyan wrote:
>> Hi Amelie,
>>
>> Could you please review and test this patch on your setup.
>> Doesn't broke anything.
>>
> 
> Hi Minas,
> 
> I tested at my end on behalf of Amelie.
> Of course I let her possibly comment.
> 
> This doesn't break anything on existing boards. I also tested the
> example described by Ziyang in the commit message.
> 
> You can add my:
> Tested-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
> 

Thanks Fabrice for testing!

You can also add my:
Reviewed-by: Amelie Delaunay <amelie.delaunay@foss.st.com>

Regards,
Amelie

> Thanks & BR,
> Fabrice
> 
>> Thanks,
>> Minas
>>
>> On 2/21/2023 2:30 PM, Ziyang Huang <hzyitc@outlook.com> wrote:
>>> From: Ziyang Huang <hzyitc@outlook.com>
>>> Sent: Tuesday, February 21, 2023 2:30 PM
>>> To: Minas Harutyunyan <hminas@synopsys.com>
>>> Cc: gregkh@linuxfoundation.org; fabrice.gasnier@foss.st.com;
>>> amelie.delaunay@foss.st.com; linux-usb@vger.kernel.org; linux-
>>> kernel@vger.kernel.org; Ziyang Huang <hzyitc@outlook.com>
>>> Subject: [PATCH v2] usb: dwc2: drd: fix inconsistent mode if role-switch-
>>> default-mode="host"
>>>
>>> Some boards might use USB-A female connector for USB ports, however, the
>>> port could be connected to a dual-mode USB controller, making it also
>>> behaves as a peripheral device if male-to-male cable is connected.
>>>
>>> In this case, the dts looks like this:
>>>
>>> 	&usb0 {
>>> 		status = "okay";
>>> 		dr_mode = "otg";
>>> 		usb-role-switch;
>>> 		role-switch-default-mode = "host";
>>> 	};
>>>
>>> After boot, dwc2_ovr_init() sets GOTGCTL to GOTGCTL_AVALOVAL and call
>>> dwc2_force_mode() with parameter host=false, which causes inconsistent mode
>>> - The hardware is in peripheral mode while the kernel status is in host
>>> mode.
>>>
>>> What we can do now is to call dwc2_drd_role_sw_set() to switch to device
>>> mode, and everything should work just fine now, even switching back to
>>> none(default) mode afterwards.
>>>
>>> Fixes: e14acb876985 ("usb: dwc2: drd: add role-switch-default-node support")
>>> Signed-off-by: Ziyang Huang <hzyitc@outlook.com>
>>> ---
>>> Changes since v1
>>> - Use corrent name in Signed-off-by
>>>
>>> drivers/usb/dwc2/drd.c | 3 ++-
>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/usb/dwc2/drd.c b/drivers/usb/dwc2/drd.c index
>>> d8d6493bc457..a8605b02115b 100644
>>> --- a/drivers/usb/dwc2/drd.c
>>> +++ b/drivers/usb/dwc2/drd.c
>>> @@ -35,7 +35,8 @@ static void dwc2_ovr_init(struct dwc2_hsotg *hsotg)
>>>
>>> 	spin_unlock_irqrestore(&hsotg->lock, flags);
>>>
>>> -	dwc2_force_mode(hsotg, (hsotg->dr_mode == USB_DR_MODE_HOST));
>>> +	dwc2_force_mode(hsotg, (hsotg->dr_mode == USB_DR_MODE_HOST) ||
>>> +				(hsotg->role_sw_default_mode == USB_DR_MODE_HOST));
>>> }
>>>
>>> static int dwc2_ovr_avalid(struct dwc2_hsotg *hsotg, bool valid)
>>> --
>>> 2.34.1
>>

RE: [PATCH v2] usb: dwc2: drd: fix inconsistent mode if role-switch-default-mode="host"

[Minas Harutyunyan]

Hi Ziyang,

On 2/21/2023 2:30 PM, Ziyang Huang <hzyitc@outlook.com> wrote:
>From: Ziyang Huang <hzyitc@outlook.com>
>Sent: Tuesday, February 21, 2023 2:30 PM
>To: Minas Harutyunyan <hminas@synopsys.com>
>Cc: gregkh@linuxfoundation.org; fabrice.gasnier@foss.st.com;
>amelie.delaunay@foss.st.com; linux-usb@vger.kernel.org; linux-
>kernel@vger.kernel.org; Ziyang Huang <hzyitc@outlook.com>
>Subject: [PATCH v2] usb: dwc2: drd: fix inconsistent mode if role-switch-
>default-mode="host"
>
>Some boards might use USB-A female connector for USB ports, however, the
>port could be connected to a dual-mode USB controller, making it also
>behaves as a peripheral device if male-to-male cable is connected.
>
>In this case, the dts looks like this:
>
>	&usb0 {
>		status = "okay";
>		dr_mode = "otg";
>		usb-role-switch;
>		role-switch-default-mode = "host";
>	};
>
>After boot, dwc2_ovr_init() sets GOTGCTL to GOTGCTL_AVALOVAL and call
>dwc2_force_mode() with parameter host=false, which causes inconsistent mode
>- The hardware is in peripheral mode while the kernel status is in host
>mode.
>
>What we can do now is to call dwc2_drd_role_sw_set() to switch to device
>mode, and everything should work just fine now, even switching back to
>none(default) mode afterwards.
>
>Fixes: e14acb876985 ("usb: dwc2: drd: add role-switch-default-node support")
>Signed-off-by: Ziyang Huang <hzyitc@outlook.com>

Reviewed-by: Amelie Delaunay <amelie.delaunay@foss.st.com>
Tested-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
Acked-by: Minas Harutyunyan <hminas@synopsys.com>


Thanks,
Minas


>---
>Changes since v1
>- Use corrent name in Signed-off-by
>
> drivers/usb/dwc2/drd.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/usb/dwc2/drd.c b/drivers/usb/dwc2/drd.c index
>d8d6493bc457..a8605b02115b 100644
>--- a/drivers/usb/dwc2/drd.c
>+++ b/drivers/usb/dwc2/drd.c
>@@ -35,7 +35,8 @@ static void dwc2_ovr_init(struct dwc2_hsotg *hsotg)
>
> 	spin_unlock_irqrestore(&hsotg->lock, flags);
>
>-	dwc2_force_mode(hsotg, (hsotg->dr_mode == USB_DR_MODE_HOST));
>+	dwc2_force_mode(hsotg, (hsotg->dr_mode == USB_DR_MODE_HOST) ||
>+				(hsotg->role_sw_default_mode == USB_DR_MODE_HOST));
> }
>
> static int dwc2_ovr_avalid(struct dwc2_hsotg *hsotg, bool valid)
>--
>2.34.1