mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH 1/5] usb: dwc2: Make dwc2_wait_bit_set wait all bits in mask
@ 2021-03-12 16:24 Jules Maselbas
  2021-03-12 16:24 ` [PATCH 2/5] usb: dwc2: Replace pr_info with dev_warn Jules Maselbas
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Jules Maselbas @ 2021-03-12 16:24 UTC (permalink / raw)
  To: barebox; +Cc: Jules Maselbas

The dwc2_wait_bit_set() wasn't waiting for all bits in the mask to be
set but was waiting for only one bit set a mask.
This is not the expected behavior as stated in the function's comment.

The function dwc2_wait_bit_set() is always called with a mask of only
one bit.

Signed-off-by: Jules Maselbas <jmaselbas@kalray.eu>
---
 drivers/usb/dwc2/core.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 780efb262..fa9d6ae95 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -532,7 +532,7 @@ static inline int dwc2_wait_bit_set(struct dwc2 *dwc2, u32 offset, u32 mask,
 			    u32 timeout)
 {
 	return wait_on_timeout(timeout * USECOND,
-			dwc2_readl(dwc2, offset) & mask);
+			(dwc2_readl(dwc2, offset) & mask) == mask);
 }
 
 /**
-- 
2.17.1



_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH 2/5] usb: dwc2: Replace pr_info with dev_warn
  2021-03-12 16:24 [PATCH 1/5] usb: dwc2: Make dwc2_wait_bit_set wait all bits in mask Jules Maselbas
@ 2021-03-12 16:24 ` Jules Maselbas
  2021-03-12 16:24 ` [PATCH 3/5] usb: dwc2: Rework dwc2_wait_for_mode to use wait_on_timeout Jules Maselbas
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Jules Maselbas @ 2021-03-12 16:24 UTC (permalink / raw)
  To: barebox; +Cc: Jules Maselbas

There is no reason to not use dev_warn (as done in Linux), this might
have been unwanted changes made when porting the driver to barebox.

Signed-off-by: Jules Maselbas <jmaselbas@kalray.eu>
---
 drivers/usb/dwc2/core.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
index cc5729ed9..8ec77d68a 100644
--- a/drivers/usb/dwc2/core.c
+++ b/drivers/usb/dwc2/core.c
@@ -696,8 +696,8 @@ int dwc2_core_reset(struct dwc2 *dwc2)
 	/* Wait for AHB master IDLE state. */
 	ret = dwc2_wait_bit_set(dwc2, GRSTCTL, GRSTCTL_AHBIDLE, 10000);
 	if (ret) {
-		pr_info("%s: Timeout! Waiting for AHB master IDLE state\n",
-			__func__);
+		dwc2_warn(dwc2, "%s: Timeout! Waiting for AHB master IDLE state\n",
+				__func__);
 		return ret;
 	}
 
@@ -728,7 +728,8 @@ int dwc2_core_reset(struct dwc2 *dwc2)
 
 	ret = dwc2_wait_bit_clear(dwc2, GRSTCTL, GRSTCTL_CSFTRST, 10000);
 	if (ret) {
-		pr_info("%s: Timeout! Waiting for Core Soft Reset\n", __func__);
+		dwc2_warn(dwc2, "%s: Timeout! Waiting for Core Soft Reset\n",
+				__func__);
 		return ret;
 	}
 
-- 
2.17.1



_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH 3/5] usb: dwc2: Rework dwc2_wait_for_mode to use wait_on_timeout
  2021-03-12 16:24 [PATCH 1/5] usb: dwc2: Make dwc2_wait_bit_set wait all bits in mask Jules Maselbas
  2021-03-12 16:24 ` [PATCH 2/5] usb: dwc2: Replace pr_info with dev_warn Jules Maselbas
@ 2021-03-12 16:24 ` Jules Maselbas
  2021-03-12 16:24 ` [PATCH 4/5] usb: dwc2: Move wait_for_mode and iddig_filter_enabled to core Jules Maselbas
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Jules Maselbas @ 2021-03-12 16:24 UTC (permalink / raw)
  To: barebox; +Cc: Jules Maselbas

Cc: Michael Grzeschik <m.grzeschik@pengutronix.de>
Signed-off-by: Jules Maselbas <jmaselbas@kalray.eu>
---
 drivers/usb/dwc2/gadget.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index b8ec37be7..3fc7099d7 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -1,7 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0+
 #include <dma.h>
 #include <usb/gadget.h>
-#include <linux/iopoll.h>
 #include "dwc2.h"
 
 #define to_dwc2 gadget_to_dwc2
@@ -2668,13 +2667,13 @@ static int dwc2_eps_alloc(struct dwc2 *dwc2)
  */
 static void dwc2_wait_for_mode(struct dwc2 *dwc2, bool host_mode)
 {
-	int val, ret;
+	unsigned int timeout = 110 * USECOND;
+	int ret;
 
 	dev_vdbg(dwc2->dev, "Waiting for %s mode\n",
 		 host_mode ? "host" : "device");
 
-	ret = readx_poll_timeout(dwc2_is_host_mode, dwc2, val,
-			val == host_mode, 110 * USEC_PER_MSEC);
+	ret = wait_on_timeout(timeout, dwc2_is_host_mode(dwc2) == host_mode);
 	if (ret)
 		dev_err(dwc2->dev, "%s: Couldn't set %s mode\n",
 				 __func__, host_mode ? "host" : "device");
-- 
2.17.1



_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH 4/5] usb: dwc2: Move wait_for_mode and iddig_filter_enabled to core
  2021-03-12 16:24 [PATCH 1/5] usb: dwc2: Make dwc2_wait_bit_set wait all bits in mask Jules Maselbas
  2021-03-12 16:24 ` [PATCH 2/5] usb: dwc2: Replace pr_info with dev_warn Jules Maselbas
  2021-03-12 16:24 ` [PATCH 3/5] usb: dwc2: Rework dwc2_wait_for_mode to use wait_on_timeout Jules Maselbas
@ 2021-03-12 16:24 ` Jules Maselbas
  2021-03-12 16:24 ` [PATCH 5/5] usb: dwc2: Rework wait for host mode during core reset Jules Maselbas
  2021-03-15  8:51 ` [PATCH 1/5] usb: dwc2: Make dwc2_wait_bit_set wait all bits in mask Sascha Hauer
  4 siblings, 0 replies; 6+ messages in thread
From: Jules Maselbas @ 2021-03-12 16:24 UTC (permalink / raw)
  To: barebox; +Cc: Jules Maselbas

Move functions dwc2_wait_for_mode() and dwc2_iddig_filter_enabled()
into core.c so they can be used during dwc2_core_init().

Cc: Michael Grzeschik <m.grzeschik@pengutronix.de>
Signed-off-by: Jules Maselbas <jmaselbas@kalray.eu>
---
 drivers/usb/dwc2/core.c   | 53 +++++++++++++++++++++++++++++++++++++++
 drivers/usb/dwc2/dwc2.h   |  3 +++
 drivers/usb/dwc2/gadget.c | 53 ---------------------------------------
 3 files changed, 56 insertions(+), 53 deletions(-)

diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
index 8ec77d68a..24efe5a41 100644
--- a/drivers/usb/dwc2/core.c
+++ b/drivers/usb/dwc2/core.c
@@ -681,6 +681,59 @@ int dwc2_get_dr_mode(struct dwc2 *dwc2)
 	return 0;
 }
 
+/**
+ * dwc2_wait_for_mode() - Waits for the controller mode.
+ * @dwc2:	Programming view of the DWC_otg controller.
+ * @host_mode:	If true, waits for host mode, otherwise device mode.
+ */
+void dwc2_wait_for_mode(struct dwc2 *dwc2, bool host_mode)
+{
+	unsigned int timeout = 110 * USECOND;
+	int ret;
+
+	dev_vdbg(dwc2->dev, "Waiting for %s mode\n",
+		 host_mode ? "host" : "device");
+
+	ret = wait_on_timeout(timeout, dwc2_is_host_mode(dwc2) == host_mode);
+	if (ret)
+		dev_err(dwc2->dev, "%s: Couldn't set %s mode\n",
+				 __func__, host_mode ? "host" : "device");
+
+	dev_vdbg(dwc2->dev, "%s mode set\n",
+		 host_mode ? "Host" : "Device");
+}
+
+/**
+ * dwc2_iddig_filter_enabled() - Returns true if the IDDIG debounce
+ * filter is enabled.
+ *
+ * @hsotg: Programming view of DWC_otg controller
+ */
+bool dwc2_iddig_filter_enabled(struct dwc2 *dwc2)
+{
+	u32 gsnpsid;
+	u32 ghwcfg4;
+
+	/* Check if core configuration includes the IDDIG filter. */
+	ghwcfg4 = dwc2_readl(dwc2, GHWCFG4);
+	if (!(ghwcfg4 & GHWCFG4_IDDIG_FILT_EN))
+		return false;
+
+	/*
+	 * Check if the IDDIG debounce filter is bypassed. Available
+	 * in core version >= 3.10a.
+	 */
+	gsnpsid = dwc2_readl(dwc2, GSNPSID);
+	if (gsnpsid >= DWC2_CORE_REV_3_10a) {
+		u32 gotgctl = dwc2_readl(dwc2, GOTGCTL);
+
+		if (gotgctl & GOTGCTL_DBNCE_FLTR_BYPASS)
+			return false;
+	}
+
+	return true;
+}
+
 /*
  * Do core a soft reset of the core.  Be careful with this because it
  * resets all the internal state machines of the core.
diff --git a/drivers/usb/dwc2/dwc2.h b/drivers/usb/dwc2/dwc2.h
index b9fdda850..31827d4c7 100644
--- a/drivers/usb/dwc2/dwc2.h
+++ b/drivers/usb/dwc2/dwc2.h
@@ -25,6 +25,9 @@ int dwc2_get_dr_mode(struct dwc2 *dwc2);
 int dwc2_core_reset(struct dwc2 *dwc2);
 void dwc2_core_init(struct dwc2 *dwc2);
 
+void dwc2_wait_for_mode(struct dwc2 *dwc2, bool host_mode);
+bool dwc2_iddig_filter_enabled(struct dwc2 *dwc2);
+
 /* Host functions */
 #ifdef CONFIG_USB_DWC2_HOST
 int dwc2_submit_roothub(struct dwc2 *dwc2, struct usb_device *dev,
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 3fc7099d7..3df5d7352 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -2660,59 +2660,6 @@ static int dwc2_eps_alloc(struct dwc2 *dwc2)
 	return 0;
 }
 
-/**
- * dwc2_wait_for_mode() - Waits for the controller mode.
- * @dwc2:	Programming view of the DWC_otg controller.
- * @host_mode:	If true, waits for host mode, otherwise device mode.
- */
-static void dwc2_wait_for_mode(struct dwc2 *dwc2, bool host_mode)
-{
-	unsigned int timeout = 110 * USECOND;
-	int ret;
-
-	dev_vdbg(dwc2->dev, "Waiting for %s mode\n",
-		 host_mode ? "host" : "device");
-
-	ret = wait_on_timeout(timeout, dwc2_is_host_mode(dwc2) == host_mode);
-	if (ret)
-		dev_err(dwc2->dev, "%s: Couldn't set %s mode\n",
-				 __func__, host_mode ? "host" : "device");
-
-	dev_vdbg(dwc2->dev, "%s mode set\n",
-		 host_mode ? "Host" : "Device");
-}
-
-/**
- * dwc2_iddig_filter_enabled() - Returns true if the IDDIG debounce
- * filter is enabled.
- *
- * @hsotg: Programming view of DWC_otg controller
- */
-static bool dwc2_iddig_filter_enabled(struct dwc2 *dwc2)
-{
-	u32 gsnpsid;
-	u32 ghwcfg4;
-
-	/* Check if core configuration includes the IDDIG filter. */
-	ghwcfg4 = dwc2_readl(dwc2, GHWCFG4);
-	if (!(ghwcfg4 & GHWCFG4_IDDIG_FILT_EN))
-		return false;
-
-	/*
-	 * Check if the IDDIG debounce filter is bypassed. Available
-	 * in core version >= 3.10a.
-	 */
-	gsnpsid = dwc2_readl(dwc2, GSNPSID);
-	if (gsnpsid >= DWC2_CORE_REV_3_10a) {
-		u32 gotgctl = dwc2_readl(dwc2, GOTGCTL);
-
-		if (gotgctl & GOTGCTL_DBNCE_FLTR_BYPASS)
-			return false;
-	}
-
-	return true;
-}
-
 /**
  * dwc2_force_mode() - Force the mode of the controller.
  *
-- 
2.17.1



_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH 5/5] usb: dwc2: Rework wait for host mode during core reset
  2021-03-12 16:24 [PATCH 1/5] usb: dwc2: Make dwc2_wait_bit_set wait all bits in mask Jules Maselbas
                   ` (2 preceding siblings ...)
  2021-03-12 16:24 ` [PATCH 4/5] usb: dwc2: Move wait_for_mode and iddig_filter_enabled to core Jules Maselbas
@ 2021-03-12 16:24 ` Jules Maselbas
  2021-03-15  8:51 ` [PATCH 1/5] usb: dwc2: Make dwc2_wait_bit_set wait all bits in mask Sascha Hauer
  4 siblings, 0 replies; 6+ messages in thread
From: Jules Maselbas @ 2021-03-12 16:24 UTC (permalink / raw)
  To: barebox; +Cc: Jules Maselbas

Replace the unconditional delay of 100ms during core_reset by
a call to dwc2_wait_for_mode() when iddig filter is enabled,
as done in Linux.

The uses of mdelay comes from the porting of u-boot's driver.

Cc: Michael Grzeschik <m.grzeschik@pengutronix.de>
Signed-off-by: Jules Maselbas <jmaselbas@kalray.eu>
---
 drivers/usb/dwc2/core.c | 26 +++++++++++---------------
 1 file changed, 11 insertions(+), 15 deletions(-)

diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
index 24efe5a41..5d04a07b0 100644
--- a/drivers/usb/dwc2/core.c
+++ b/drivers/usb/dwc2/core.c
@@ -764,16 +764,16 @@ int dwc2_core_reset(struct dwc2 *dwc2)
 	 * Determine whether we will go back into host mode after a
 	 * reset and account for this delay after the reset.
 	 */
-	{
-	u32 gotgctl = dwc2_readl(dwc2, GOTGCTL);
-	u32 gusbcfg = dwc2_readl(dwc2, GUSBCFG);
-
-	if (!(gotgctl & GOTGCTL_CONID_B) ||
-	    (gusbcfg & GUSBCFG_FORCEHOSTMODE)) {
-		dwc2_dbg(dwc2, "HOST MODE\n");
-		wait_for_host_mode = true;
-	}
+	if (dwc2_iddig_filter_enabled(dwc2)) {
+		u32 gotgctl = dwc2_readl(dwc2, GOTGCTL);
+		u32 gusbcfg = dwc2_readl(dwc2, GUSBCFG);
+
+		if (!(gotgctl & GOTGCTL_CONID_B) ||
+		    (gusbcfg & GUSBCFG_FORCEHOSTMODE)) {
+			wait_for_host_mode = true;
+		}
 	}
+
 	/* Core Soft Reset */
 	greset = dwc2_readl(dwc2, GRSTCTL);
 	greset |= GRSTCTL_CSFTRST;
@@ -786,12 +786,8 @@ int dwc2_core_reset(struct dwc2 *dwc2)
 		return ret;
 	}
 
-	/*
-	 * Wait for core to come out of reset.
-	 * NOTE: This long sleep is _very_ important, otherwise the core will
-	 *       not stay in host mode after a connector ID change!
-	 */
-	mdelay(100);
+	if (wait_for_host_mode)
+		dwc2_wait_for_mode(dwc2, wait_for_host_mode);
 
 	return 0;
 }
-- 
2.17.1



_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH 1/5] usb: dwc2: Make dwc2_wait_bit_set wait all bits in mask
  2021-03-12 16:24 [PATCH 1/5] usb: dwc2: Make dwc2_wait_bit_set wait all bits in mask Jules Maselbas
                   ` (3 preceding siblings ...)
  2021-03-12 16:24 ` [PATCH 5/5] usb: dwc2: Rework wait for host mode during core reset Jules Maselbas
@ 2021-03-15  8:51 ` Sascha Hauer
  4 siblings, 0 replies; 6+ messages in thread
From: Sascha Hauer @ 2021-03-15  8:51 UTC (permalink / raw)
  To: Jules Maselbas; +Cc: barebox

On Fri, Mar 12, 2021 at 05:24:28PM +0100, Jules Maselbas wrote:
> The dwc2_wait_bit_set() wasn't waiting for all bits in the mask to be
> set but was waiting for only one bit set a mask.
> This is not the expected behavior as stated in the function's comment.
> 
> The function dwc2_wait_bit_set() is always called with a mask of only
> one bit.
> 
> Signed-off-by: Jules Maselbas <jmaselbas@kalray.eu>
> ---
>  drivers/usb/dwc2/core.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied, thanks

Sascha

> 
> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
> index 780efb262..fa9d6ae95 100644
> --- a/drivers/usb/dwc2/core.h
> +++ b/drivers/usb/dwc2/core.h
> @@ -532,7 +532,7 @@ static inline int dwc2_wait_bit_set(struct dwc2 *dwc2, u32 offset, u32 mask,
>  			    u32 timeout)
>  {
>  	return wait_on_timeout(timeout * USECOND,
> -			dwc2_readl(dwc2, offset) & mask);
> +			(dwc2_readl(dwc2, offset) & mask) == mask);
>  }
>  
>  /**
> -- 
> 2.17.1
> 
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-03-15  8:53 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-12 16:24 [PATCH 1/5] usb: dwc2: Make dwc2_wait_bit_set wait all bits in mask Jules Maselbas
2021-03-12 16:24 ` [PATCH 2/5] usb: dwc2: Replace pr_info with dev_warn Jules Maselbas
2021-03-12 16:24 ` [PATCH 3/5] usb: dwc2: Rework dwc2_wait_for_mode to use wait_on_timeout Jules Maselbas
2021-03-12 16:24 ` [PATCH 4/5] usb: dwc2: Move wait_for_mode and iddig_filter_enabled to core Jules Maselbas
2021-03-12 16:24 ` [PATCH 5/5] usb: dwc2: Rework wait for host mode during core reset Jules Maselbas
2021-03-15  8:51 ` [PATCH 1/5] usb: dwc2: Make dwc2_wait_bit_set wait all bits in mask Sascha Hauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox