All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH v3 0/5] usb: host: dwc2: use driver model for PHY and CLOCK
@ 2019-11-12  9:42 Patrick Delaunay
  2019-11-12  9:42 ` [U-Boot] [PATCH v3 1/5] dm: clk: add stub for clk_disable_bulk when CONFIG_CLK is desactivated Patrick Delaunay
                   ` (5 more replies)
  0 siblings, 6 replies; 12+ messages in thread
From: Patrick Delaunay @ 2019-11-12  9:42 UTC (permalink / raw)
  To: u-boot


In this serie I update the DWC2 host driver to use the device tree
information and the associated PHY and CLOCK drivers when they are
available.

CI-Travis build is OK for all target after V3:
https://travis-ci.org/patrickdelaunay/u-boot/builds/609496187

In V2, I cause the warnings for some boards:
drivers/usb/host/built-in.o: In function `dwc2_usb_remove':
drivers/usb/host/dwc2.c:1441: undefined reference to `clk_disable_bulk'

I test this serie on stm32mp157c-ev1 board, with PHY and CLK
support

The U-CLASS are provided by:
- PHY by USBPHYC driver = ./drivers/phy/phy-stm32-usbphyc.c
- CLOCK by RCC clock driver = drivers/clk/clk_stm32mp1.c
- RESET by RCC reset driver = drivers/reset/stm32-reset.c

And I activate the configuration
+CONFIG_USB_DWC2=y

PS: it is not the default configuration to avoid conflict with gadget
    driver

To solve a binding issue, I also deactivate the gadget support:
by default only one driver is bound to theusbotg_hs node with "snps,dwc2"
compatible, and today it is the device one (the first in the driver list).

I also need to deactivate hnp-srp support with:

&usbotg_hs {
	/* need to disable ONLY for HOST support */
	hnp-srp-disable;
};

WARNING: OTG with device or host support is not correctly handle by DWC2
         driver (see example for dynamic OTG role in DWC3 driver).

The tests executed on the stm32mp157c-ev1 target:

STM32MP> usb start
starting USB...
Bus usb-otg at 49000000: USB DWC2
Bus usbh-ehci at 5800d000: USB EHCI 1.00
scanning bus usb-otg at 49000000 for devices... 2 USB Device(s) found
scanning bus usbh-ehci at 5800d000 for devices... 3 USB Device(s) found
       scanning usb for storage devices... 2 Storage Device(s) found
STM32MP> usb tree
USB device tree:
  1  Hub (480 Mb/s, 0mA)
  |   U-Boot Root Hub
  |
  +-2  Mass Storage (480 Mb/s, 300mA)
       Verbatim STORE N GO 070731C8ACD7EE97

  1  Hub (480 Mb/s, 0mA)
  |  u-boot EHCI Host Controller
  |
  +-2  Hub (480 Mb/s, 2mA)
    |
    +-3  Mass Storage (480 Mb/s, 500mA)
         Generic  USB Storage

STM32MP> ls usb 0
<DIR>       4096 .
<DIR>       4096 ..
<DIR>      16384 lost+found
<DIR>       4096 record
         1490212 xipImage
        21058006 vmlinux

STM32MP> load usb 0 0xC0000000 vmlinux
21058006 bytes read in 10851 ms (1.9 MiB/s)


Changes in v3:
- Add stub for clk_disable_bulk

Changes in v2:
- update dev_err
- update commit message
- change dev_err to dev_dbg for PHY function call
- treat dwc2_shutdown_phy error
- add clk_disable_bulk in dwc2_usb_remove

Patrick Delaunay (5):
  dm: clk: add stub for clk_disable_bulk when CONFIG_CLK is desactivated
  usb: host: dwc2: add phy support
  usb: host: dwc2: add clk support
  usb: host: dwc2: force reset assert
  usb: host: dwc2: add trace to have clean usb start

 drivers/usb/host/dwc2.c | 100 +++++++++++++++++++++++++++++++++++++++-
 include/clk.h           |   4 ++
 2 files changed, 103 insertions(+), 1 deletion(-)

-- 
2.17.1

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH v3 1/5] dm: clk: add stub for clk_disable_bulk when CONFIG_CLK is desactivated
  2019-11-12  9:42 [U-Boot] [PATCH v3 0/5] usb: host: dwc2: use driver model for PHY and CLOCK Patrick Delaunay
@ 2019-11-12  9:42 ` Patrick Delaunay
  2019-11-12 10:16   ` Jean-Jacques Hiblot
  2019-11-12 10:40   ` Simon Goldschmidt
  2019-11-12  9:42 ` [U-Boot] [PATCH v3 2/5] usb: host: dwc2: add phy support Patrick Delaunay
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 12+ messages in thread
From: Patrick Delaunay @ 2019-11-12  9:42 UTC (permalink / raw)
  To: u-boot

Add stub for clk_disable_bulk() when CONFIG_CLK is desactivated.

That avoid compilation issue (undefined reference to
`clk_disable_bulk') for code:

clk_disable_bulk(&priv->clks);
clk_release_bulk(&priv->clks);

Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
---

Changes in v3:
- Add stub for clk_disable_bulk

Changes in v2: None

 include/clk.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/clk.h b/include/clk.h
index a5ee53d94a..6f0b0fe4bc 100644
--- a/include/clk.h
+++ b/include/clk.h
@@ -379,7 +379,11 @@ int clk_disable(struct clk *clk);
  *		by clk_get_bulk().
  * @return zero on success, or -ve error code.
  */
+ #if CONFIG_IS_ENABLED(CLK)
 int clk_disable_bulk(struct clk_bulk *bulk);
+#else
+inline int clk_disable_bulk(struct clk_bulk *bulk) { return 0; }
+#endif
 
 /**
  * clk_is_match - check if two clk's point to the same hardware clock
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH v3 2/5] usb: host: dwc2: add phy support
  2019-11-12  9:42 [U-Boot] [PATCH v3 0/5] usb: host: dwc2: use driver model for PHY and CLOCK Patrick Delaunay
  2019-11-12  9:42 ` [U-Boot] [PATCH v3 1/5] dm: clk: add stub for clk_disable_bulk when CONFIG_CLK is desactivated Patrick Delaunay
@ 2019-11-12  9:42 ` Patrick Delaunay
  2019-11-12  9:42 ` [U-Boot] [PATCH v3 3/5] usb: host: dwc2: add clk support Patrick Delaunay
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Patrick Delaunay @ 2019-11-12  9:42 UTC (permalink / raw)
  To: u-boot

Use generic phy to initialize the PHY associated to the
DWC2 device and available in the device tree.

This patch don't added dependency because when CONFIG_PHY
is not activated, the generic PHY function are stubbed.

Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
---

Changes in v3: None
Changes in v2:
- update dev_err
- update commit message
- change dev_err to dev_dbg for PHY function call
- treat dwc2_shutdown_phy error

 drivers/usb/host/dwc2.c | 66 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c
index 350d820a6e..cb2b381eb6 100644
--- a/drivers/usb/host/dwc2.c
+++ b/drivers/usb/host/dwc2.c
@@ -7,6 +7,7 @@
 #include <common.h>
 #include <dm.h>
 #include <errno.h>
+#include <generic-phy.h>
 #include <usb.h>
 #include <malloc.h>
 #include <memalign.h>
@@ -35,6 +36,7 @@ struct dwc2_priv {
 #ifdef CONFIG_DM_REGULATOR
 	struct udevice *vbus_supply;
 #endif
+	struct phy phy;
 #else
 	uint8_t *aligned_buffer;
 	uint8_t *status_buffer;
@@ -1320,13 +1322,71 @@ static int dwc2_usb_ofdata_to_platdata(struct udevice *dev)
 	return 0;
 }
 
+static int dwc2_setup_phy(struct udevice *dev)
+{
+	struct dwc2_priv *priv = dev_get_priv(dev);
+	int ret;
+
+	ret = generic_phy_get_by_index(dev, 0, &priv->phy);
+	if (ret) {
+		if (ret != -ENOENT) {
+			dev_err(dev, "Failed to get USB PHY: %d.\n", ret);
+			return ret;
+		}
+		return 0;
+	}
+
+	ret = generic_phy_init(&priv->phy);
+	if (ret) {
+		dev_dbg(dev, "Failed to init USB PHY: %d.\n", ret);
+		return ret;
+	}
+
+	ret = generic_phy_power_on(&priv->phy);
+	if (ret) {
+		dev_dbg(dev, "Failed to power on USB PHY: %d.\n", ret);
+		generic_phy_exit(&priv->phy);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int dwc2_shutdown_phy(struct udevice *dev)
+{
+	struct dwc2_priv *priv = dev_get_priv(dev);
+	int ret;
+
+	if (!generic_phy_valid(&priv->phy))
+		return 0;
+
+	ret = generic_phy_power_off(&priv->phy);
+	if (ret) {
+		dev_dbg(dev, "Failed to power off USB PHY: %d.\n", ret);
+		return ret;
+	}
+
+	ret = generic_phy_exit(&priv->phy);
+	if (ret) {
+		dev_dbg(dev, "Failed to power off USB PHY: %d.\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
 static int dwc2_usb_probe(struct udevice *dev)
 {
 	struct dwc2_priv *priv = dev_get_priv(dev);
 	struct usb_bus_priv *bus_priv = dev_get_uclass_priv(dev);
+	int ret;
 
 	bus_priv->desc_before_addr = true;
 
+	ret = dwc2_setup_phy(dev);
+	if (ret)
+		return ret;
+
 	return dwc2_init_common(dev, priv);
 }
 
@@ -1339,6 +1399,12 @@ static int dwc2_usb_remove(struct udevice *dev)
 	if (ret)
 		return ret;
 
+	ret = dwc2_shutdown_phy(dev);
+	if (ret) {
+		dev_dbg(dev, "Failed to shutdown USB PHY: %d.\n", ret);
+		return ret;
+	}
+
 	dwc2_uninit_common(priv->regs);
 
 	reset_release_bulk(&priv->resets);
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH v3 3/5] usb: host: dwc2: add clk support
  2019-11-12  9:42 [U-Boot] [PATCH v3 0/5] usb: host: dwc2: use driver model for PHY and CLOCK Patrick Delaunay
  2019-11-12  9:42 ` [U-Boot] [PATCH v3 1/5] dm: clk: add stub for clk_disable_bulk when CONFIG_CLK is desactivated Patrick Delaunay
  2019-11-12  9:42 ` [U-Boot] [PATCH v3 2/5] usb: host: dwc2: add phy support Patrick Delaunay
@ 2019-11-12  9:42 ` Patrick Delaunay
  2019-11-12  9:42 ` [U-Boot] [PATCH v3 4/5] usb: host: dwc2: force reset assert Patrick Delaunay
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Patrick Delaunay @ 2019-11-12  9:42 UTC (permalink / raw)
  To: u-boot

Add support for clock with driver model.

This patch don't added dependency because when CONFIG_CLK
is not activated the clk function are stubbed.

Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
---

Changes in v3: None
Changes in v2: None

 drivers/usb/host/dwc2.c | 30 +++++++++++++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c
index cb2b381eb6..9a00bea24f 100644
--- a/drivers/usb/host/dwc2.c
+++ b/drivers/usb/host/dwc2.c
@@ -5,13 +5,14 @@
  */
 
 #include <common.h>
+#include <clk.h>
 #include <dm.h>
 #include <errno.h>
 #include <generic-phy.h>
-#include <usb.h>
 #include <malloc.h>
 #include <memalign.h>
 #include <phys2bus.h>
+#include <usb.h>
 #include <usbroothubdes.h>
 #include <wait_bit.h>
 #include <asm/io.h>
@@ -37,6 +38,7 @@ struct dwc2_priv {
 	struct udevice *vbus_supply;
 #endif
 	struct phy phy;
+	struct clk_bulk clks;
 #else
 	uint8_t *aligned_buffer;
 	uint8_t *status_buffer;
@@ -1375,6 +1377,26 @@ static int dwc2_shutdown_phy(struct udevice *dev)
 	return 0;
 }
 
+static int dwc2_clk_init(struct udevice *dev)
+{
+	struct dwc2_priv *priv = dev_get_priv(dev);
+	int ret;
+
+	ret = clk_get_bulk(dev, &priv->clks);
+	if (ret == -ENOSYS || ret == -ENOENT)
+		return 0;
+	if (ret)
+		return ret;
+
+	ret = clk_enable_bulk(&priv->clks);
+	if (ret) {
+		clk_release_bulk(&priv->clks);
+		return ret;
+	}
+
+	return 0;
+}
+
 static int dwc2_usb_probe(struct udevice *dev)
 {
 	struct dwc2_priv *priv = dev_get_priv(dev);
@@ -1383,6 +1405,10 @@ static int dwc2_usb_probe(struct udevice *dev)
 
 	bus_priv->desc_before_addr = true;
 
+	ret = dwc2_clk_init(dev);
+	if (ret)
+		return ret;
+
 	ret = dwc2_setup_phy(dev);
 	if (ret)
 		return ret;
@@ -1408,6 +1434,8 @@ static int dwc2_usb_remove(struct udevice *dev)
 	dwc2_uninit_common(priv->regs);
 
 	reset_release_bulk(&priv->resets);
+	clk_disable_bulk(&priv->clks);
+	clk_release_bulk(&priv->clks);
 
 	return 0;
 }
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH v3 4/5] usb: host: dwc2: force reset assert
  2019-11-12  9:42 [U-Boot] [PATCH v3 0/5] usb: host: dwc2: use driver model for PHY and CLOCK Patrick Delaunay
                   ` (2 preceding siblings ...)
  2019-11-12  9:42 ` [U-Boot] [PATCH v3 3/5] usb: host: dwc2: add clk support Patrick Delaunay
@ 2019-11-12  9:42 ` Patrick Delaunay
  2019-11-12  9:42 ` [U-Boot] [PATCH v3 5/5] usb: host: dwc2: add trace to have clean usb start Patrick Delaunay
  2020-02-14 13:34 ` [PATCH v3 0/5] usb: host: dwc2: use driver model for PHY and CLOCK Patrick DELAUNAY
  5 siblings, 0 replies; 12+ messages in thread
From: Patrick Delaunay @ 2019-11-12  9:42 UTC (permalink / raw)
  To: u-boot

Assert reset before deassert in dwc2_reset;
this patch solve issues when the DWC2 registers are already
initialized with value incompatible with host mode.

Force a hardware reset of the IP reset all the DWC2 registers at
default value, the host driver start with a clean state
(Core Soft reset doen in dwc_otg_core_reset is not enought
 to reset all register).

The error can occurs in U-Boot when DWC2 device gadget driver
force device mode (called by ums or dfu command, before to execute
the usb start command).

Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
---

Changes in v3: None
Changes in v2:
- add clk_disable_bulk in dwc2_usb_remove

 drivers/usb/host/dwc2.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c
index 9a00bea24f..870b06459e 100644
--- a/drivers/usb/host/dwc2.c
+++ b/drivers/usb/host/dwc2.c
@@ -1149,6 +1149,8 @@ static int dwc2_reset(struct udevice *dev)
 			return ret;
 	}
 
+	/* force reset to clear all IP register */
+	reset_assert_bulk(&priv->resets);
 	ret = reset_deassert_bulk(&priv->resets);
 	if (ret) {
 		reset_release_bulk(&priv->resets);
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH v3 5/5] usb: host: dwc2: add trace to have clean usb start
  2019-11-12  9:42 [U-Boot] [PATCH v3 0/5] usb: host: dwc2: use driver model for PHY and CLOCK Patrick Delaunay
                   ` (3 preceding siblings ...)
  2019-11-12  9:42 ` [U-Boot] [PATCH v3 4/5] usb: host: dwc2: force reset assert Patrick Delaunay
@ 2019-11-12  9:42 ` Patrick Delaunay
  2020-02-14 13:34 ` [PATCH v3 0/5] usb: host: dwc2: use driver model for PHY and CLOCK Patrick DELAUNAY
  5 siblings, 0 replies; 12+ messages in thread
From: Patrick Delaunay @ 2019-11-12  9:42 UTC (permalink / raw)
  To: u-boot

Solve issue for the display of "usb start" command on stm32mp1
because one carriage return is missing in DWC2 probe.

STM32MP> usb start
starting USB...
Bus usb-otg at 49000000:    Bus usbh-ehci at 5800d000:   USB EHCI 1.00

after the patch:

STM32MP> usb start
starting USB...
Bus usb-otg at 49000000: USB DWC2
Bus usbh-ehci at 5800d000: USB EHCI 1.00

Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
---

Changes in v3: None
Changes in v2: None

 drivers/usb/host/dwc2.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c
index 870b06459e..2450dff9ac 100644
--- a/drivers/usb/host/dwc2.c
+++ b/drivers/usb/host/dwc2.c
@@ -1217,6 +1217,8 @@ static int dwc2_init_common(struct udevice *dev, struct dwc2_priv *priv)
 	if (readl(&regs->gintsts) & DWC2_GINTSTS_CURMODE_HOST)
 		mdelay(1000);
 
+	printf("USB DWC2\n");
+
 	return 0;
 }
 
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH v3 1/5] dm: clk: add stub for clk_disable_bulk when CONFIG_CLK is desactivated
  2019-11-12  9:42 ` [U-Boot] [PATCH v3 1/5] dm: clk: add stub for clk_disable_bulk when CONFIG_CLK is desactivated Patrick Delaunay
@ 2019-11-12 10:16   ` Jean-Jacques Hiblot
  2019-11-12 15:47     ` Patrick DELAUNAY
  2019-11-12 10:40   ` Simon Goldschmidt
  1 sibling, 1 reply; 12+ messages in thread
From: Jean-Jacques Hiblot @ 2019-11-12 10:16 UTC (permalink / raw)
  To: u-boot

Hi Patrick,

On 12/11/2019 10:42, Patrick Delaunay wrote:
> Add stub for clk_disable_bulk() when CONFIG_CLK is desactivated.
>
> That avoid compilation issue (undefined reference to
> `clk_disable_bulk') for code:
>
> clk_disable_bulk(&priv->clks);
> clk_release_bulk(&priv->clks);
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> ---
>
> Changes in v3:
> - Add stub for clk_disable_bulk
>
> Changes in v2: None
>
>   include/clk.h | 4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/include/clk.h b/include/clk.h
> index a5ee53d94a..6f0b0fe4bc 100644
> --- a/include/clk.h
> +++ b/include/clk.h
> @@ -379,7 +379,11 @@ int clk_disable(struct clk *clk);
>    *		by clk_get_bulk().
>    * @return zero on success, or -ve error code.
>    */
> + #if CONFIG_IS_ENABLED(CLK)
>   int clk_disable_bulk(struct clk_bulk *bulk);
> +#else
> +inline int clk_disable_bulk(struct clk_bulk *bulk) { return 0; }
> +#endif

Maybe this could be done for all clk operations ?

JJ

>   
>   /**
>    * clk_is_match - check if two clk's point to the same hardware clock

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH v3 1/5] dm: clk: add stub for clk_disable_bulk when CONFIG_CLK is desactivated
  2019-11-12  9:42 ` [U-Boot] [PATCH v3 1/5] dm: clk: add stub for clk_disable_bulk when CONFIG_CLK is desactivated Patrick Delaunay
  2019-11-12 10:16   ` Jean-Jacques Hiblot
@ 2019-11-12 10:40   ` Simon Goldschmidt
  2019-11-20 10:04     ` Patrick DELAUNAY
  1 sibling, 1 reply; 12+ messages in thread
From: Simon Goldschmidt @ 2019-11-12 10:40 UTC (permalink / raw)
  To: u-boot

Patrick Delaunay <patrick.delaunay@st.com> schrieb am Di., 12. Nov. 2019,
10:42:

> Add stub for clk_disable_bulk() when CONFIG_CLK is desactivated.
>
> That avoid compilation issue (undefined reference to
> `clk_disable_bulk') for code:
>
> clk_disable_bulk(&priv->clks);
> clk_release_bulk(&priv->clks);
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> ---
>
> Changes in v3:
> - Add stub for clk_disable_bulk
>
> Changes in v2: None
>
>  include/clk.h | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/include/clk.h b/include/clk.h
> index a5ee53d94a..6f0b0fe4bc 100644
> --- a/include/clk.h
> +++ b/include/clk.h
> @@ -379,7 +379,11 @@ int clk_disable(struct clk *clk);
>   *             by clk_get_bulk().
>   * @return zero on success, or -ve error code.
>   */
> + #if CONFIG_IS_ENABLED(CLK)
>  int clk_disable_bulk(struct clk_bulk *bulk);
> +#else
> +inline int clk_disable_bulk(struct clk_bulk *bulk) { return 0; }
> +#endif
>

Doing this inline at this place seems quite different than what is done for
the other functions?

Regards,
Simon


>  /**
>   * clk_is_match - check if two clk's point to the same hardware clock
> --
> 2.17.1
>
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH v3 1/5] dm: clk: add stub for clk_disable_bulk when CONFIG_CLK is desactivated
  2019-11-12 10:16   ` Jean-Jacques Hiblot
@ 2019-11-12 15:47     ` Patrick DELAUNAY
  0 siblings, 0 replies; 12+ messages in thread
From: Patrick DELAUNAY @ 2019-11-12 15:47 UTC (permalink / raw)
  To: u-boot

Hi Jean-Jacques,


> From: Jean-Jacques Hiblot <jjhiblot@ti.com>
> Sent: mardi 12 novembre 2019 11:17
> 
> Hi Patrick,
> 
> On 12/11/2019 10:42, Patrick Delaunay wrote:
> > Add stub for clk_disable_bulk() when CONFIG_CLK is desactivated.
> >
> > That avoid compilation issue (undefined reference to
> > `clk_disable_bulk') for code:
> >
> > clk_disable_bulk(&priv->clks);
> > clk_release_bulk(&priv->clks);
> >
> > Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> > ---
> >
> > Changes in v3:
> > - Add stub for clk_disable_bulk
> >
> > Changes in v2: None
> >
> >   include/clk.h | 4 ++++
> >   1 file changed, 4 insertions(+)
> >
> > diff --git a/include/clk.h b/include/clk.h index
> > a5ee53d94a..6f0b0fe4bc 100644
> > --- a/include/clk.h
> > +++ b/include/clk.h
> > @@ -379,7 +379,11 @@ int clk_disable(struct clk *clk);
> >    *		by clk_get_bulk().
> >    * @return zero on success, or -ve error code.
> >    */
> > + #if CONFIG_IS_ENABLED(CLK)
> >   int clk_disable_bulk(struct clk_bulk *bulk);
> > +#else
> > +inline int clk_disable_bulk(struct clk_bulk *bulk) { return 0; }
> > +#endif
> 
> Maybe this could be done for all clk operations ?

I think about, but after reflection

1/ stub already exist for :
clk_get_by_index
clk_get_bulk
clk_get_by_name
clk_release_all

=> just inline , return -ENOSYS

2/ clk_release_bulk inline call for clk_release_all

3/ other function (clk_request, clk_free, clk_get_rate, clk_enable, clk_disable) 
    should be not used as "clk" parameter is never valid / available if CONFIG_CLK is not activited

4/ the only remaining case is 

	int clk_disable_bulk(struct clk_bulk *bulk);

	=> clk_get_bulk return -ENOSYS but normally this information is not keept by caller....

	On error bulk.count = 0, and for me clk_disable_bulk(bulk wthou count = 0) is valid even if CONFIG_CLK is disable....

So I decide to limit the patch to this function to minimize the impacts
also because the 2020.01 windows is closed.

Moreover  I have not board to test CONFIG_CLK disabled.

But I agree : it is more clear a have a stub for other function which can be used 
	including clk_valid

=> I can propose a 2nd separate patch with this proposal if it is required.

> JJ

Regards 
Patrick

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [U-Boot] [PATCH v3 1/5] dm: clk: add stub for clk_disable_bulk when CONFIG_CLK is desactivated
  2019-11-12 10:40   ` Simon Goldschmidt
@ 2019-11-20 10:04     ` Patrick DELAUNAY
  0 siblings, 0 replies; 12+ messages in thread
From: Patrick DELAUNAY @ 2019-11-20 10:04 UTC (permalink / raw)
  To: u-boot

Hi Simon

>De : Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
>Envoyé : mardi 12 novembre 2019 11:40
>
>
>Patrick Delaunay <patrick.delaunay@st.com> schrieb am Di., 12. Nov. 2019, 10:42:
>Add stub for clk_disable_bulk() when CONFIG_CLK is desactivated.
>
>That avoid compilation issue (undefined reference to
>`clk_disable_bulk') for code:
>
>clk_disable_bulk(&priv->clks);
>clk_release_bulk(&priv->clks);
>
>Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
>---
>
>Changes in v3:
>- Add stub for clk_disable_bulk
>
>Changes in v2: None
>
> include/clk.h | 4 ++++
> 1 file changed, 4 insertions(+)
>
>diff --git a/include/clk.h b/include/clk.h
>index a5ee53d94a..6f0b0fe4bc 100644
>--- a/include/clk.h
>+++ b/include/clk.h
>@@ -379,7 +379,11 @@ int clk_disable(struct clk *clk);
>  *             by clk_get_bulk().
>  * @return zero on success, or -ve error code.
>  */
>+ #if CONFIG_IS_ENABLED(CLK)
> int clk_disable_bulk(struct clk_bulk *bulk);
>+#else
>+inline int clk_disable_bulk(struct clk_bulk *bulk) { return 0; }
>+#endif
>
>Doing this inline at this place seems quite different than what is done for the other functions?

which functions ?

I see:

static inline int clk_get_by_index(struct udevice *dev, int index,
   struct clk *clk)
{
return -ENOSYS;
}

static inline int clk_set_defaults(struct udevice *dev)
{
return 0;
}

But I agree, that it is a simplified stub....

Disable clk bulk if OK only if bulk.count ==0 but if should be always
the case if CONFIG_CLK is disabled (as clk_get_bulk return -ENOSYS).

A more complete (better ?) stub is :

inline int clk_disable_bulk(struct clk_bulk *bulk) {
if (bulk && bulk->count == 0) return 0;
else return -ENOSYS;
}

I think about a other solution:

inline int clk_disable_bulk(struct clk_bulk *bulk) {
return -ENOSYS;
}

But that cause issue if the return value is tested by caller.

ret = clk_disable_bulk(bulk)
if (ret)
            return ret;

>Regards,
>Simon

Regards
Patrick

>
> /**
>  * clk_is_match - check if two clk's point to the same hardware clock
>--
>2.17.1
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v3 0/5] usb: host: dwc2: use driver model for PHY and CLOCK
  2019-11-12  9:42 [U-Boot] [PATCH v3 0/5] usb: host: dwc2: use driver model for PHY and CLOCK Patrick Delaunay
                   ` (4 preceding siblings ...)
  2019-11-12  9:42 ` [U-Boot] [PATCH v3 5/5] usb: host: dwc2: add trace to have clean usb start Patrick Delaunay
@ 2020-02-14 13:34 ` Patrick DELAUNAY
  2020-02-14 18:29   ` Marek Vasut
  5 siblings, 1 reply; 12+ messages in thread
From: Patrick DELAUNAY @ 2020-02-14 13:34 UTC (permalink / raw)
  To: u-boot

Hi,

> From: Patrick DELAUNAY <patrick.delaunay@st.com>
> Sent: mardi 12 novembre 2019 10:42
> To: u-boot at lists.denx.de
> Cc: ley.foon.tan at intel.com; b.galvani at gmail.com;
> simon.k.r.goldschmidt at gmail.com; Patrick DELAUNAY
> <patrick.delaunay@st.com>; Jagan Teki <jagan@amarulasolutions.com>; Jean-
> Jacques Hiblot <jjhiblot@ti.com>; Lokesh Vutla <lokeshvutla@ti.com>; Lukasz
> Majewski <lukma@denx.de>; Marek Vasut <marex@denx.de>; Michal Suchanek
> <msuchanek@suse.de>; Peng Fan <peng.fan@nxp.com>; Sekhar Nori
> <nsekhar@ti.com>; Simon Glass <sjg@chromium.org>; Sven Schwermer
> <sven@svenschwermer.de>; U-Boot STM32 <uboot-stm32@st-md-
> mailman.stormreply.com>
> Subject: [PATCH v3 0/5] usb: host: dwc2: use driver model for PHY and CLOCK
> Importance: High
> 
> 
> In this serie I update the DWC2 host driver to use the device tree information and
> the associated PHY and CLOCK drivers when they are available.
> 
> CI-Travis build is OK for all target after V3:
> https://travis-ci.org/patrickdelaunay/u-boot/builds/609496187
> 
> In V2, I cause the warnings for some boards:
> drivers/usb/host/built-in.o: In function `dwc2_usb_remove':
> drivers/usb/host/dwc2.c:1441: undefined reference to `clk_disable_bulk'
> 
> I test this serie on stm32mp157c-ev1 board, with PHY and CLK support
> 
> The U-CLASS are provided by:
> - PHY by USBPHYC driver = ./drivers/phy/phy-stm32-usbphyc.c
> - CLOCK by RCC clock driver = drivers/clk/clk_stm32mp1.c
> - RESET by RCC reset driver = drivers/reset/stm32-reset.c
> 
> And I activate the configuration
> +CONFIG_USB_DWC2=y
> 
> PS: it is not the default configuration to avoid conflict with gadget
>     driver
> 
> To solve a binding issue, I also deactivate the gadget support:
> by default only one driver is bound to theusbotg_hs node with "snps,dwc2"
> compatible, and today it is the device one (the first in the driver list).
> 
> I also need to deactivate hnp-srp support with:
> 
> &usbotg_hs {
> 	/* need to disable ONLY for HOST support */
> 	hnp-srp-disable;
> };
> 
> WARNING: OTG with device or host support is not correctly handle by DWC2
>          driver (see example for dynamic OTG role in DWC3 driver).
> 
> The tests executed on the stm32mp157c-ev1 target:
> 
> STM32MP> usb start
> starting USB...
> Bus usb-otg at 49000000: USB DWC2
> Bus usbh-ehci at 5800d000: USB EHCI 1.00
> scanning bus usb-otg at 49000000 for devices... 2 USB Device(s) found scanning
> bus usbh-ehci at 5800d000 for devices... 3 USB Device(s) found
>        scanning usb for storage devices... 2 Storage Device(s) found
> STM32MP> usb tree
> USB device tree:
>   1  Hub (480 Mb/s, 0mA)
>   |   U-Boot Root Hub
>   |
>   +-2  Mass Storage (480 Mb/s, 300mA)
>        Verbatim STORE N GO 070731C8ACD7EE97
> 
>   1  Hub (480 Mb/s, 0mA)
>   |  u-boot EHCI Host Controller
>   |
>   +-2  Hub (480 Mb/s, 2mA)
>     |
>     +-3  Mass Storage (480 Mb/s, 500mA)
>          Generic  USB Storage
> 
> STM32MP> ls usb 0
> <DIR>       4096 .
> <DIR>       4096 ..
> <DIR>      16384 lost+found
> <DIR>       4096 record
>          1490212 xipImage
>         21058006 vmlinux
> 
> STM32MP> load usb 0 0xC0000000 vmlinux
> 21058006 bytes read in 10851 ms (1.9 MiB/s)
> 
> 
> Changes in v3:
> - Add stub for clk_disable_bulk
> 
> Changes in v2:
> - update dev_err
> - update commit message
> - change dev_err to dev_dbg for PHY function call
> - treat dwc2_shutdown_phy error
> - add clk_disable_bulk in dwc2_usb_remove
> 
> Patrick Delaunay (5):
>   dm: clk: add stub for clk_disable_bulk when CONFIG_CLK is desactivated
>   usb: host: dwc2: add phy support
>   usb: host: dwc2: add clk support
>   usb: host: dwc2: force reset assert
>   usb: host: dwc2: add trace to have clean usb start
> 
>  drivers/usb/host/dwc2.c | 100
> +++++++++++++++++++++++++++++++++++++++-
>  include/clk.h           |   4 ++
>  2 files changed, 103 insertions(+), 1 deletion(-)
> 
> --
> 2.17.1

Any comments on this serie or I need to rebase it and resend the serie ?

http://patchwork.ozlabs.org/project/uboot/list/?series=142257

Sorry for the delay...
I miss the previous merge windows for v2020.01 and now I think it also the case for v2020.04 !

The previous version v2 was almost ready
http://patchwork.ozlabs.org/project/uboot/list/?series=141678&state=*

In v3 I only solve the compilation issue on some board with patch 1/5
(= http://patchwork.ozlabs.org/patch/1193397/)

PS: for other remarks I create a separate serie:
       http://patchwork.ozlabs.org/project/uboot/list/?series=142260

Regards

Patrick

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v3 0/5] usb: host: dwc2: use driver model for PHY and CLOCK
  2020-02-14 13:34 ` [PATCH v3 0/5] usb: host: dwc2: use driver model for PHY and CLOCK Patrick DELAUNAY
@ 2020-02-14 18:29   ` Marek Vasut
  0 siblings, 0 replies; 12+ messages in thread
From: Marek Vasut @ 2020-02-14 18:29 UTC (permalink / raw)
  To: u-boot

On 2/14/20 2:34 PM, Patrick DELAUNAY wrote:
> Hi,

[...]

>>  drivers/usb/host/dwc2.c | 100
>> +++++++++++++++++++++++++++++++++++++++-
>>  include/clk.h           |   4 ++
>>  2 files changed, 103 insertions(+), 1 deletion(-)
>>
>> --
>> 2.17.1
> 
> Any comments on this serie or I need to rebase it and resend the serie ?
> 
> http://patchwork.ozlabs.org/project/uboot/list/?series=142257
> 
> Sorry for the delay...
> I miss the previous merge windows for v2020.01 and now I think it also the case for v2020.04 !
Please rebase and repost, thanks.

[...]

-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2020-02-14 18:29 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-12  9:42 [U-Boot] [PATCH v3 0/5] usb: host: dwc2: use driver model for PHY and CLOCK Patrick Delaunay
2019-11-12  9:42 ` [U-Boot] [PATCH v3 1/5] dm: clk: add stub for clk_disable_bulk when CONFIG_CLK is desactivated Patrick Delaunay
2019-11-12 10:16   ` Jean-Jacques Hiblot
2019-11-12 15:47     ` Patrick DELAUNAY
2019-11-12 10:40   ` Simon Goldschmidt
2019-11-20 10:04     ` Patrick DELAUNAY
2019-11-12  9:42 ` [U-Boot] [PATCH v3 2/5] usb: host: dwc2: add phy support Patrick Delaunay
2019-11-12  9:42 ` [U-Boot] [PATCH v3 3/5] usb: host: dwc2: add clk support Patrick Delaunay
2019-11-12  9:42 ` [U-Boot] [PATCH v3 4/5] usb: host: dwc2: force reset assert Patrick Delaunay
2019-11-12  9:42 ` [U-Boot] [PATCH v3 5/5] usb: host: dwc2: add trace to have clean usb start Patrick Delaunay
2020-02-14 13:34 ` [PATCH v3 0/5] usb: host: dwc2: use driver model for PHY and CLOCK Patrick DELAUNAY
2020-02-14 18:29   ` Marek Vasut

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.