2023-02-21 10:31:20

by Ziyang Huang

[permalink] [raw]
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 <[email protected]>
---
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



2023-02-21 10:42:13

by Greg Kroah-Hartman

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

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 <[email protected]>
> ---
> Changes since v1
> - Use corrent name in Signed-off-by

Nope, still incorrect, please use your synopsys address.

thanks,

greg k-h

2023-02-21 12:33:50

by Ziyang Huang

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


在 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 <[email protected]>
>> ---
>> 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.



2023-02-21 13:48:56

by Greg Kroah-Hartman

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

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 <[email protected]>
> > > ---
> > > 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

2023-02-22 07:01:21

by Minas Harutyunyan

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

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 <[email protected]> wrote:
>From: Ziyang Huang <[email protected]>
>Sent: Tuesday, February 21, 2023 2:30 PM
>To: Minas Harutyunyan <[email protected]>
>Cc: [email protected]; [email protected];
>[email protected]; [email protected]; linux-
>[email protected]; Ziyang Huang <[email protected]>
>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 <[email protected]>
>---
>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


2023-03-01 11:26:50

by Fabrice Gasnier

[permalink] [raw]
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 <[email protected]>

Thanks & BR,
Fabrice

> Thanks,
> Minas
>
> On 2/21/2023 2:30 PM, Ziyang Huang <[email protected]> wrote:
>> From: Ziyang Huang <[email protected]>
>> Sent: Tuesday, February 21, 2023 2:30 PM
>> To: Minas Harutyunyan <[email protected]>
>> Cc: [email protected]; [email protected];
>> [email protected]; [email protected]; linux-
>> [email protected]; Ziyang Huang <[email protected]>
>> 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 <[email protected]>
>> ---
>> 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
>

2023-03-01 11:30:31

by Minas Harutyunyan

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

Hi Fabrice,

>-----Original Message-----
>From: Fabrice Gasnier <[email protected]>
>Sent: Wednesday, March 1, 2023 3:26 PM
>To: Minas Harutyunyan <[email protected]>; Ziyang Huang
><[email protected]>; [email protected]
>Cc: [email protected]; [email protected]; linux-
>[email protected]
>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 <[email protected]>
>
>Thanks & BR,
>Fabrice
>

Thank you very much.

Minas

2023-03-01 11:32:08

by Amelie Delaunay

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



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 <[email protected]>
>

Thanks Fabrice for testing!

You can also add my:
Reviewed-by: Amelie Delaunay <[email protected]>

Regards,
Amelie

> Thanks & BR,
> Fabrice
>
>> Thanks,
>> Minas
>>
>> On 2/21/2023 2:30 PM, Ziyang Huang <[email protected]> wrote:
>>> From: Ziyang Huang <[email protected]>
>>> Sent: Tuesday, February 21, 2023 2:30 PM
>>> To: Minas Harutyunyan <[email protected]>
>>> Cc: [email protected]; [email protected];
>>> [email protected]; [email protected]; linux-
>>> [email protected]; Ziyang Huang <[email protected]>
>>> 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 <[email protected]>
>>> ---
>>> 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
>>

2023-03-01 11:34:40

by Minas Harutyunyan

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

Hi Ziyang,

On 2/21/2023 2:30 PM, Ziyang Huang <[email protected]> wrote:
>From: Ziyang Huang <[email protected]>
>Sent: Tuesday, February 21, 2023 2:30 PM
>To: Minas Harutyunyan <[email protected]>
>Cc: [email protected]; [email protected];
>[email protected]; [email protected]; linux-
>[email protected]; Ziyang Huang <[email protected]>
>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 <[email protected]>

Reviewed-by: Amelie Delaunay <[email protected]>
Tested-by: Fabrice Gasnier <[email protected]>
Acked-by: Minas Harutyunyan <[email protected]>


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