2021-10-05 09:55:23

by Amelie Delaunay

[permalink] [raw]
Subject: [PATCH 0/3] usb: dwc2: drd: fix some issues

This patchset fixes initialization issues seen while dr_mode != "otg"
and resets previous session before setting the new one.

Amelie Delaunay (3):
usb: dwc2: drd: fix dwc2_force_mode call in dwc2_ovr_init
usb: dwc2: drd: fix dwc2_drd_role_sw_set when clock could be disabled
usb: dwc2: drd: reset current session before setting the new one

drivers/usb/dwc2/drd.c | 24 ++++++++++++++++++++++--
1 file changed, 22 insertions(+), 2 deletions(-)

--
2.25.1


2021-10-05 09:55:24

by Amelie Delaunay

[permalink] [raw]
Subject: [PATCH 1/3] usb: dwc2: drd: fix dwc2_force_mode call in dwc2_ovr_init

Instead of forcing the role to Device, check the dr_mode configuration.
If the core is Host only, force the mode to Host, this to avoid the
dwc2_force_mode warning:
WARNING: CPU: 1 PID: 21 at drivers/usb/dwc2/core.c:615 dwc2_drd_init+0x104/0x17c

When forcing mode to Host, dwc2_force_mode may sleep the time the host
role is applied. To avoid sleeping while atomic context, move the call
to dwc2_force_mode after spin_unlock_irqrestore. It is safe, as
interrupts are not yet unmasked here.

Fixes: 17f934024e84 ("usb: dwc2: override PHY input signals with usb role switch support")
Signed-off-by: Amelie Delaunay <[email protected]>
---
drivers/usb/dwc2/drd.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/dwc2/drd.c b/drivers/usb/dwc2/drd.c
index 2d4176f5788e..80eae88d76dd 100644
--- a/drivers/usb/dwc2/drd.c
+++ b/drivers/usb/dwc2/drd.c
@@ -25,9 +25,9 @@ static void dwc2_ovr_init(struct dwc2_hsotg *hsotg)
gotgctl &= ~(GOTGCTL_BVALOVAL | GOTGCTL_AVALOVAL | GOTGCTL_VBVALOVAL);
dwc2_writel(hsotg, gotgctl, GOTGCTL);

- dwc2_force_mode(hsotg, false);
-
spin_unlock_irqrestore(&hsotg->lock, flags);
+
+ dwc2_force_mode(hsotg, (hsotg->dr_mode == USB_DR_MODE_HOST));
}

static int dwc2_ovr_avalid(struct dwc2_hsotg *hsotg, bool valid)
--
2.25.1

2021-10-05 09:55:38

by Amelie Delaunay

[permalink] [raw]
Subject: [PATCH 3/3] usb: dwc2: drd: reset current session before setting the new one

If role is changed without the "none" step, A- and B- valid session could
be set at the same time. It is an issue.
This patch resets A-session if role switch sets B-session, and resets
B-session if role switch sets A-session.
Then, it is possible to change the role without the "none" step.

Fixes: 17f934024e84 ("usb: dwc2: override PHY input signals with usb role switch support")
Signed-off-by: Amelie Delaunay <[email protected]>
---
drivers/usb/dwc2/drd.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/usb/dwc2/drd.c b/drivers/usb/dwc2/drd.c
index 99672360f34b..aa6eb76f64dd 100644
--- a/drivers/usb/dwc2/drd.c
+++ b/drivers/usb/dwc2/drd.c
@@ -40,6 +40,7 @@ static int dwc2_ovr_avalid(struct dwc2_hsotg *hsotg, bool valid)
(!valid && !(gotgctl & GOTGCTL_ASESVLD)))
return -EALREADY;

+ gotgctl &= ~GOTGCTL_BVALOVAL;
if (valid)
gotgctl |= GOTGCTL_AVALOVAL | GOTGCTL_VBVALOVAL;
else
@@ -58,6 +59,7 @@ static int dwc2_ovr_bvalid(struct dwc2_hsotg *hsotg, bool valid)
(!valid && !(gotgctl & GOTGCTL_BSESVLD)))
return -EALREADY;

+ gotgctl &= ~GOTGCTL_AVALOVAL;
if (valid)
gotgctl |= GOTGCTL_BVALOVAL | GOTGCTL_VBVALOVAL;
else
--
2.25.1

2021-10-05 09:57:16

by Amelie Delaunay

[permalink] [raw]
Subject: [PATCH 2/3] usb: dwc2: drd: fix dwc2_drd_role_sw_set when clock could be disabled

In case of USB_DR_MODE_PERIPHERAL, the OTG clock is disabled at the end of
the probe (it is not the case if USB_DR_MODE_HOST or USB_DR_MODE_OTG).
The clock is then enabled on udc_start.
If dwc2_drd_role_sw_set is called before udc_start (it is the case if the
usb cable is plugged at boot), GOTGCTL and GUSBCFG registers cannot be
read/written, so session cannot be overridden.
To avoid this case, check the ll_hw_enabled value and enable the clock if
it is available, and disable it after the override.

Fixes: 17f934024e84 ("usb: dwc2: override PHY input signals with usb role switch support")
Signed-off-by: Amelie Delaunay <[email protected]>
---
drivers/usb/dwc2/drd.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)

diff --git a/drivers/usb/dwc2/drd.c b/drivers/usb/dwc2/drd.c
index 80eae88d76dd..99672360f34b 100644
--- a/drivers/usb/dwc2/drd.c
+++ b/drivers/usb/dwc2/drd.c
@@ -7,6 +7,7 @@
* Author(s): Amelie Delaunay <[email protected]>
*/

+#include <linux/clk.h>
#include <linux/iopoll.h>
#include <linux/platform_device.h>
#include <linux/usb/role.h>
@@ -86,6 +87,20 @@ static int dwc2_drd_role_sw_set(struct usb_role_switch *sw, enum usb_role role)
}
#endif

+ /*
+ * In case of USB_DR_MODE_PERIPHERAL, clock is disabled at the end of
+ * the probe and enabled on udc_start.
+ * If role-switch set is called before the udc_start, we need to enable
+ * the clock to read/write GOTGCTL and GUSBCFG registers to override
+ * mode and sessions. It is the case if cable is plugged at boot.
+ */
+ if (!hsotg->ll_hw_enabled && hsotg->clk) {
+ int ret = clk_prepare_enable(hsotg->clk);
+
+ if (ret)
+ return ret;
+ }
+
spin_lock_irqsave(&hsotg->lock, flags);

if (role == USB_ROLE_HOST) {
@@ -110,6 +125,9 @@ static int dwc2_drd_role_sw_set(struct usb_role_switch *sw, enum usb_role role)
/* This will raise a Connector ID Status Change Interrupt */
dwc2_force_mode(hsotg, role == USB_ROLE_HOST);

+ if (!hsotg->ll_hw_enabled && hsotg->clk)
+ clk_disable_unprepare(hsotg->clk);
+
dev_dbg(hsotg->dev, "%s-session valid\n",
role == USB_ROLE_NONE ? "No" :
role == USB_ROLE_HOST ? "A" : "B");
--
2.25.1

2021-10-15 19:40:46

by Minas Harutyunyan

[permalink] [raw]
Subject: Re: [PATCH 3/3] usb: dwc2: drd: reset current session before setting the new one

On 10/5/2021 1:53 PM, Amelie Delaunay wrote:
> If role is changed without the "none" step, A- and B- valid session could
> be set at the same time. It is an issue.
> This patch resets A-session if role switch sets B-session, and resets
> B-session if role switch sets A-session.
> Then, it is possible to change the role without the "none" step.
>
> Fixes: 17f934024e84 ("usb: dwc2: override PHY input signals with usb role switch support")
> Signed-off-by: Amelie Delaunay <[email protected]>

Acked-by: Minas Harutyunyan <[email protected]>

> ---
> drivers/usb/dwc2/drd.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/usb/dwc2/drd.c b/drivers/usb/dwc2/drd.c
> index 99672360f34b..aa6eb76f64dd 100644
> --- a/drivers/usb/dwc2/drd.c
> +++ b/drivers/usb/dwc2/drd.c
> @@ -40,6 +40,7 @@ static int dwc2_ovr_avalid(struct dwc2_hsotg *hsotg, bool valid)
> (!valid && !(gotgctl & GOTGCTL_ASESVLD)))
> return -EALREADY;
>
> + gotgctl &= ~GOTGCTL_BVALOVAL;
> if (valid)
> gotgctl |= GOTGCTL_AVALOVAL | GOTGCTL_VBVALOVAL;
> else
> @@ -58,6 +59,7 @@ static int dwc2_ovr_bvalid(struct dwc2_hsotg *hsotg, bool valid)
> (!valid && !(gotgctl & GOTGCTL_BSESVLD)))
> return -EALREADY;
>
> + gotgctl &= ~GOTGCTL_AVALOVAL;
> if (valid)
> gotgctl |= GOTGCTL_BVALOVAL | GOTGCTL_VBVALOVAL;
> else
>

2021-10-15 19:40:58

by Minas Harutyunyan

[permalink] [raw]
Subject: Re: [PATCH 1/3] usb: dwc2: drd: fix dwc2_force_mode call in dwc2_ovr_init

On 10/5/2021 1:53 PM, Amelie Delaunay wrote:
> Instead of forcing the role to Device, check the dr_mode configuration.
> If the core is Host only, force the mode to Host, this to avoid the
> dwc2_force_mode warning:
> WARNING: CPU: 1 PID: 21 at drivers/usb/dwc2/core.c:615 dwc2_drd_init+0x104/0x17c
>
> When forcing mode to Host, dwc2_force_mode may sleep the time the host
> role is applied. To avoid sleeping while atomic context, move the call
> to dwc2_force_mode after spin_unlock_irqrestore. It is safe, as
> interrupts are not yet unmasked here.
>
> Fixes: 17f934024e84 ("usb: dwc2: override PHY input signals with usb role switch support")
> Signed-off-by: Amelie Delaunay <[email protected]>

Acked-by: Minas Harutyunyan <[email protected]>

> ---
> drivers/usb/dwc2/drd.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/dwc2/drd.c b/drivers/usb/dwc2/drd.c
> index 2d4176f5788e..80eae88d76dd 100644
> --- a/drivers/usb/dwc2/drd.c
> +++ b/drivers/usb/dwc2/drd.c
> @@ -25,9 +25,9 @@ static void dwc2_ovr_init(struct dwc2_hsotg *hsotg)
> gotgctl &= ~(GOTGCTL_BVALOVAL | GOTGCTL_AVALOVAL | GOTGCTL_VBVALOVAL);
> dwc2_writel(hsotg, gotgctl, GOTGCTL);
>
> - dwc2_force_mode(hsotg, false);
> -
> spin_unlock_irqrestore(&hsotg->lock, flags);
> +
> + dwc2_force_mode(hsotg, (hsotg->dr_mode == USB_DR_MODE_HOST));
> }
>
> static int dwc2_ovr_avalid(struct dwc2_hsotg *hsotg, bool valid)
>

2021-10-15 19:40:59

by Minas Harutyunyan

[permalink] [raw]
Subject: Re: [PATCH 2/3] usb: dwc2: drd: fix dwc2_drd_role_sw_set when clock could be disabled

On 10/5/2021 1:53 PM, Amelie Delaunay wrote:
> In case of USB_DR_MODE_PERIPHERAL, the OTG clock is disabled at the end of
> the probe (it is not the case if USB_DR_MODE_HOST or USB_DR_MODE_OTG).
> The clock is then enabled on udc_start.
> If dwc2_drd_role_sw_set is called before udc_start (it is the case if the
> usb cable is plugged at boot), GOTGCTL and GUSBCFG registers cannot be
> read/written, so session cannot be overridden.
> To avoid this case, check the ll_hw_enabled value and enable the clock if
> it is available, and disable it after the override.
>
> Fixes: 17f934024e84 ("usb: dwc2: override PHY input signals with usb role switch support")
> Signed-off-by: Amelie Delaunay <[email protected]>

Acked-by: Minas Harutyunyan <[email protected]>

> ---
> drivers/usb/dwc2/drd.c | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/drivers/usb/dwc2/drd.c b/drivers/usb/dwc2/drd.c
> index 80eae88d76dd..99672360f34b 100644
> --- a/drivers/usb/dwc2/drd.c
> +++ b/drivers/usb/dwc2/drd.c
> @@ -7,6 +7,7 @@
> * Author(s): Amelie Delaunay <[email protected]>
> */
>
> +#include <linux/clk.h>
> #include <linux/iopoll.h>
> #include <linux/platform_device.h>
> #include <linux/usb/role.h>
> @@ -86,6 +87,20 @@ static int dwc2_drd_role_sw_set(struct usb_role_switch *sw, enum usb_role role)
> }
> #endif
>
> + /*
> + * In case of USB_DR_MODE_PERIPHERAL, clock is disabled at the end of
> + * the probe and enabled on udc_start.
> + * If role-switch set is called before the udc_start, we need to enable
> + * the clock to read/write GOTGCTL and GUSBCFG registers to override
> + * mode and sessions. It is the case if cable is plugged at boot.
> + */
> + if (!hsotg->ll_hw_enabled && hsotg->clk) {
> + int ret = clk_prepare_enable(hsotg->clk);
> +
> + if (ret)
> + return ret;
> + }
> +
> spin_lock_irqsave(&hsotg->lock, flags);
>
> if (role == USB_ROLE_HOST) {
> @@ -110,6 +125,9 @@ static int dwc2_drd_role_sw_set(struct usb_role_switch *sw, enum usb_role role)
> /* This will raise a Connector ID Status Change Interrupt */
> dwc2_force_mode(hsotg, role == USB_ROLE_HOST);
>
> + if (!hsotg->ll_hw_enabled && hsotg->clk)
> + clk_disable_unprepare(hsotg->clk);
> +
> dev_dbg(hsotg->dev, "%s-session valid\n",
> role == USB_ROLE_NONE ? "No" :
> role == USB_ROLE_HOST ? "A" : "B");
>