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

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

[Amelie Delaunay]

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

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

[Amelie Delaunay]

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 <amelie.delaunay@foss.st.com>
---
 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

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

[Minas Harutyunyan]

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 <amelie.delaunay@foss.st.com>

Acked-by: Minas Harutyunyan <Minas.Harutyunyan@synopsys.com>

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

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

[Amelie Delaunay]

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 <amelie.delaunay@foss.st.com>
---
 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 <amelie.delaunay@st.com>
  */
 
+#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

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

[Minas Harutyunyan]

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 <amelie.delaunay@foss.st.com>

Acked-by: Minas Harutyunyan <Minas.Harutyunyan@synopsys.com>

> ---
>   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 <amelie.delaunay@st.com>
>    */
>   
> +#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");
> 

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

[Amelie Delaunay]

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 <amelie.delaunay@foss.st.com>
---
 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

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

[Minas Harutyunyan]

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 <amelie.delaunay@foss.st.com>

Acked-by: Minas Harutyunyan <Minas.Harutyunyan@synopsys.com>

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