[PATCH v4 0/5] usb: host: dwc2: use driver model for PHY and CLOCK
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 availables.
The V4 is rebased on latest master (v2020.04-rc2). CI-Tavis build is OK: https://travis-ci.org/patrickdelaunay/u-boot/builds/651479714
NB: CI-Travis build was OK for all target after V3: https://travis-ci.org/patrickdelaunay/u-boot/builds/609496187 As 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 the usbotg_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@49000000: USB DWC2 Bus usbh-ehci@5800d000: USB EHCI 1.00 scanning bus usb-otg@49000000 for devices... 2 USB Device(s) found scanning bus usbh-ehci@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)
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 v4: - Add stub for all functions using 'struct clk' or 'struct clk_bulk' after remarks on v3
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 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 | 101 ++++++++++++++++++++++++++++++++++------ 2 files changed, 187 insertions(+), 14 deletions(-)
Add stub for functions clk_...() when CONFIG_CLK is desactivated.
This patch avoids compilation issues for driver using these API without protection (#if CONFIG_IS_ENABLED(CLK))
For example, before this patch we have 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 v4: - Add stub for all functions using 'struct clk' or 'struct clk_bulk' after remarks on v3
Changes in v3: - Add stub for clk_disable_bulk
Changes in v2: None
include/clk.h | 101 +++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 88 insertions(+), 13 deletions(-)
diff --git a/include/clk.h b/include/clk.h index 3336301815..1fb415ddc8 100644 --- a/include/clk.h +++ b/include/clk.h @@ -312,6 +312,7 @@ static inline int clk_release_bulk(struct clk_bulk *bulk) return clk_release_all(bulk->clks, bulk->count); }
+#if CONFIG_IS_ENABLED(CLK) /** * clk_request - Request a clock by provider-specific ID. * @@ -433,19 +434,6 @@ int clk_disable_bulk(struct clk_bulk *bulk); */ bool clk_is_match(const struct clk *p, const struct clk *q);
-int soc_clk_dump(void); - -/** - * clk_valid() - check if clk is valid - * - * @clk: the clock to check - * @return true if valid, or false - */ -static inline bool clk_valid(struct clk *clk) -{ - return clk && !!clk->dev; -} - /** * clk_get_by_id() - Get the clock by its ID * @@ -465,6 +453,93 @@ int clk_get_by_id(ulong id, struct clk **clkp); * @return true on binded, or false on no */ bool clk_dev_binded(struct clk *clk); + +#else /* CONFIG_IS_ENABLED(CLK) */ + +static inline int clk_request(struct udevice *dev, struct clk *clk) +{ + return -ENOSYS; +} + +static inline int clk_free(struct clk *clk) +{ + return -ENOSYS; +} + +static inline ulong clk_get_rate(struct clk *clk) +{ + return -ENOSYS; +} + +static inline struct clk *clk_get_parent(struct clk *clk) +{ + return (struct clk *)-ENOSYS; +} + +static inline long long clk_get_parent_rate(struct clk *clk) +{ + return -ENOSYS; +} + +static inline ulong clk_set_rate(struct clk *clk, ulong rate) +{ + return -ENOSYS; +} + +static inline int clk_set_parent(struct clk *clk, struct clk *parent) +{ + return -ENOSYS; +} + +static inline int clk_enable(struct clk *clk) +{ + return -ENOSYS; +} + +static inline int clk_enable_bulk(struct clk_bulk *bulk) +{ + return bulk && bulk->count == 0 ? 0 : -ENOSYS; +} + +static inline int clk_disable(struct clk *clk) +{ + return -ENOSYS; +} + +static inline int clk_disable_bulk(struct clk_bulk *bulk) +{ + return bulk && bulk->count == 0 ? 0 : -ENOSYS; +} + +static inline bool clk_is_match(const struct clk *p, const struct clk *q) +{ + return false; +} + +static inline int clk_get_by_id(ulong id, struct clk **clkp) +{ + return -ENOSYS; +} + +static inline bool clk_dev_binded(struct clk *clk) +{ + return false; +} +#endif /* CONFIG_IS_ENABLED(CLK) */ + +/** + * clk_valid() - check if clk is valid + * + * @clk: the clock to check + * @return true if valid, or false + */ +static inline bool clk_valid(struct clk *clk) +{ + return clk && !!clk->dev; +} + +int soc_clk_dump(void); + #endif
#define clk_prepare_enable(clk) clk_enable(clk)
Am 18.02.2020 um 09:34 schrieb Patrick Delaunay:
Add stub for functions clk_...() when CONFIG_CLK is desactivated.
This patch avoids compilation issues for driver using these API without protection (#if CONFIG_IS_ENABLED(CLK))
For example, before this patch we have 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 v4:
- Add stub for all functions using 'struct clk' or 'struct clk_bulk' after remarks on v3
Changes in v3:
- Add stub for clk_disable_bulk
Changes in v2: None
include/clk.h | 101 +++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 88 insertions(+), 13 deletions(-)
diff --git a/include/clk.h b/include/clk.h index 3336301815..1fb415ddc8 100644 --- a/include/clk.h +++ b/include/clk.h @@ -312,6 +312,7 @@ static inline int clk_release_bulk(struct clk_bulk *bulk) return clk_release_all(bulk->clks, bulk->count); }
+#if CONFIG_IS_ENABLED(CLK) /**
- clk_request - Request a clock by provider-specific ID.
@@ -433,19 +434,6 @@ int clk_disable_bulk(struct clk_bulk *bulk); */ bool clk_is_match(const struct clk *p, const struct clk *q);
-int soc_clk_dump(void);
-/**
- clk_valid() - check if clk is valid
- @clk: the clock to check
- @return true if valid, or false
- */
-static inline bool clk_valid(struct clk *clk) -{
- return clk && !!clk->dev;
-}
/**
- clk_get_by_id() - Get the clock by its ID
@@ -465,6 +453,93 @@ int clk_get_by_id(ulong id, struct clk **clkp);
- @return true on binded, or false on no
*/ bool clk_dev_binded(struct clk *clk);
+#else /* CONFIG_IS_ENABLED(CLK) */
+static inline int clk_request(struct udevice *dev, struct clk *clk) +{
- return -ENOSYS;
+}
+static inline int clk_free(struct clk *clk) +{
- return -ENOSYS;
+}
+static inline ulong clk_get_rate(struct clk *clk) +{
- return -ENOSYS;
+}
+static inline struct clk *clk_get_parent(struct clk *clk) +{
- return (struct clk *)-ENOSYS;
This should use ERR_PTR() to care for platforms defining CONFIG_ERR_PTR_OFFSET.
+}
+static inline long long clk_get_parent_rate(struct clk *clk) +{
- return -ENOSYS;
+}
+static inline ulong clk_set_rate(struct clk *clk, ulong rate) +{
- return -ENOSYS;
+}
+static inline int clk_set_parent(struct clk *clk, struct clk *parent) +{
- return -ENOSYS;
+}
+static inline int clk_enable(struct clk *clk) +{
- return -ENOSYS;
+}
+static inline int clk_enable_bulk(struct clk_bulk *bulk) +{
- return bulk && bulk->count == 0 ? 0 : -ENOSYS;
For this test to work, someone would need to set bulk->count to 0. This is normally done by clk_get_bulk(), but you defined it to only return an error.
I guess it works for you because all clk_bulk objects you use are from the heap (which is currently zeroed out in U-Boot) or if they are on the stack, you have if/else code that doesn't bring you here. Still it seems wrong.
Regards, Simon
+}
+static inline int clk_disable(struct clk *clk) +{
- return -ENOSYS;
+}
+static inline int clk_disable_bulk(struct clk_bulk *bulk) +{
- return bulk && bulk->count == 0 ? 0 : -ENOSYS;
+}
+static inline bool clk_is_match(const struct clk *p, const struct clk *q) +{
- return false;
+}
+static inline int clk_get_by_id(ulong id, struct clk **clkp) +{
- return -ENOSYS;
+}
+static inline bool clk_dev_binded(struct clk *clk) +{
- return false;
+} +#endif /* CONFIG_IS_ENABLED(CLK) */
+/**
- clk_valid() - check if clk is valid
- @clk: the clock to check
- @return true if valid, or false
- */
+static inline bool clk_valid(struct clk *clk) +{
- return clk && !!clk->dev;
+}
+int soc_clk_dump(void);
#endif
#define clk_prepare_enable(clk) clk_enable(clk)
Hi Simon,
From: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com Sent: mercredi 4 mars 2020 20:49
Am 18.02.2020 um 09:34 schrieb Patrick Delaunay:
Add stub for functions clk_...() when CONFIG_CLK is desactivated.
This patch avoids compilation issues for driver using these API without protection (#if CONFIG_IS_ENABLED(CLK))
For example, before this patch we have 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 v4:
- Add stub for all functions using 'struct clk' or 'struct clk_bulk' after remarks on v3
Changes in v3:
- Add stub for clk_disable_bulk
Changes in v2: None
include/clk.h | 101 +++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 88 insertions(+), 13 deletions(-)
diff --git a/include/clk.h b/include/clk.h index 3336301815..1fb415ddc8 100644 --- a/include/clk.h +++ b/include/clk.h @@ -312,6 +312,7 @@ static inline int clk_release_bulk(struct clk_bulk *bulk) return clk_release_all(bulk->clks, bulk->count); }
+#if CONFIG_IS_ENABLED(CLK) /**
- clk_request - Request a clock by provider-specific ID.
@@ -433,19 +434,6 @@ int clk_disable_bulk(struct clk_bulk *bulk); */ bool clk_is_match(const struct clk *p, const struct clk *q);
-int soc_clk_dump(void);
-/**
- clk_valid() - check if clk is valid
- @clk: the clock to check
- @return true if valid, or false
- */
-static inline bool clk_valid(struct clk *clk) -{
- return clk && !!clk->dev;
-}
/**
- clk_get_by_id() - Get the clock by its ID
@@ -465,6 +453,93 @@ int clk_get_by_id(ulong id, struct clk **clkp);
- @return true on binded, or false on no
*/ bool clk_dev_binded(struct clk *clk);
+#else /* CONFIG_IS_ENABLED(CLK) */
+static inline int clk_request(struct udevice *dev, struct clk *clk) {
- return -ENOSYS;
+}
+static inline int clk_free(struct clk *clk) {
- return -ENOSYS;
+}
+static inline ulong clk_get_rate(struct clk *clk) {
- return -ENOSYS;
+}
+static inline struct clk *clk_get_parent(struct clk *clk) {
- return (struct clk *)-ENOSYS;
This should use ERR_PTR() to care for platforms defining CONFIG_ERR_PTR_OFFSET.
Yes I take the point for V5.
+}
+static inline long long clk_get_parent_rate(struct clk *clk) {
- return -ENOSYS;
+}
+static inline ulong clk_set_rate(struct clk *clk, ulong rate) {
- return -ENOSYS;
+}
+static inline int clk_set_parent(struct clk *clk, struct clk *parent) +{
- return -ENOSYS;
+}
+static inline int clk_enable(struct clk *clk) {
- return -ENOSYS;
+}
+static inline int clk_enable_bulk(struct clk_bulk *bulk) {
- return bulk && bulk->count == 0 ? 0 : -ENOSYS;
For this test to work, someone would need to set bulk->count to 0. This is normally done by clk_get_bulk(), but you defined it to only return an error.
I guess it works for you because all clk_bulk objects you use are from the heap (which is currently zeroed out in U-Boot) or if they are on the stack, you have if/else code that doesn't bring you here. Still it seems wrong.
Yes exactly, it is working for me as the bulk was in private date, so zero allocated.
I will update on V5
Regards, Simon
Thanks for the review
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 v4: None 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 e4efaf1e59..5e7ffaddd9 100644 --- a/drivers/usb/host/dwc2.c +++ b/drivers/usb/host/dwc2.c @@ -8,6 +8,7 @@ #include <cpu_func.h> #include <dm.h> #include <errno.h> +#include <generic-phy.h> #include <usb.h> #include <malloc.h> #include <memalign.h> @@ -37,6 +38,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; @@ -1322,13 +1324,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); }
@@ -1341,6 +1401,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);
Am 18.02.2020 um 09:35 schrieb Patrick Delaunay:
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 v4: None 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 e4efaf1e59..5e7ffaddd9 100644 --- a/drivers/usb/host/dwc2.c +++ b/drivers/usb/host/dwc2.c @@ -8,6 +8,7 @@ #include <cpu_func.h> #include <dm.h> #include <errno.h> +#include <generic-phy.h> #include <usb.h> #include <malloc.h> #include <memalign.h> @@ -37,6 +38,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; @@ -1322,13 +1324,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) {
Could you invert this logic and add a comment like "no PHY" or something?
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))
A comment saying that this is for platforms without a phy driver would be nice.
Other than that: Reviewed-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com
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);
}
@@ -1341,6 +1401,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);
-----Original Message----- From: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com Sent: mercredi 4 mars 2020 20:52 To: Patrick DELAUNAY patrick.delaunay@st.com; u-boot@lists.denx.de Cc: ley.foon.tan@intel.com; b.galvani@gmail.com; Daniel Schwierzeck daniel.schwierzeck@gmail.com; Marek Vasut marex@denx.de; Michal Suchanek msuchanek@suse.de; Simon Glass sjg@chromium.org; U-Boot STM32 uboot-stm32@st-md-mailman.stormreply.com Subject: Re: [PATCH v4 2/5] usb: host: dwc2: add phy support Importance: High
Am 18.02.2020 um 09:35 schrieb Patrick Delaunay:
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 v4: None 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 e4efaf1e59..5e7ffaddd9 100644 --- a/drivers/usb/host/dwc2.c +++ b/drivers/usb/host/dwc2.c @@ -8,6 +8,7 @@ #include <cpu_func.h> #include <dm.h> #include <errno.h> +#include <generic-phy.h> #include <usb.h> #include <malloc.h> #include <memalign.h> @@ -37,6 +38,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; @@ -1322,13 +1324,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) {Could you invert this logic and add a comment like "no PHY" or something?
Yes in V5, it is more clear
ret = generic_phy_get_by_index(dev, 0, &priv->phy); if (ret) { if (ret == -ENOENT) return 0; /* no PHY, nothing to do */ dev_err(dev, "Failed to get USB PHY: %d.\n", ret); return ret; }
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))
A comment saying that this is for platforms without a phy driver would be nice.
Yes in V5:
static int dwc2_shutdown_phy(struct udevice *dev) { struct dwc2_priv *priv = dev_get_priv(dev); int ret;
/* PHY is not valid when generic_phy_get_by_index() = -ENOENT */ if (!generic_phy_valid(&priv->phy)) return 0; /* no PHY, nothing to do */
Other than that: Reviewed-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com
Thanks Regards
Patrick
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 v4: None 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 5e7ffaddd9..d56d0e61b5 100644 --- a/drivers/usb/host/dwc2.c +++ b/drivers/usb/host/dwc2.c @@ -5,14 +5,15 @@ */
#include <common.h> +#include <clk.h> #include <cpu_func.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> @@ -39,6 +40,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; @@ -1377,6 +1379,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); @@ -1385,6 +1407,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; @@ -1410,6 +1436,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; }
Am 18.02.2020 um 09:35 schrieb Patrick Delaunay:
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
Reviewed-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com
Changes in v4: None 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 5e7ffaddd9..d56d0e61b5 100644 --- a/drivers/usb/host/dwc2.c +++ b/drivers/usb/host/dwc2.c @@ -5,14 +5,15 @@ */
#include <common.h> +#include <clk.h> #include <cpu_func.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> @@ -39,6 +40,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; @@ -1377,6 +1379,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); @@ -1385,6 +1407,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;
@@ -1410,6 +1436,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;
}
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 v4: None 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 d56d0e61b5..f53913cde4 100644 --- a/drivers/usb/host/dwc2.c +++ b/drivers/usb/host/dwc2.c @@ -1151,6 +1151,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);
Solve issue for the display of "usb start" command on stm32mp1 because one carriage return is missing in DWC2 probe.
Before the patch:
STM32MP> usb start starting USB... Bus usb-otg@49000000: Bus usbh-ehci@5800d000: USB EHCI 1.00
after the patch:
STM32MP> usb start starting USB... Bus usb-otg@49000000: USB DWC2 Bus usbh-ehci@5800d000: USB EHCI 1.00
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com ---
Changes in v4: None 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 f53913cde4..8cc395037d 100644 --- a/drivers/usb/host/dwc2.c +++ b/drivers/usb/host/dwc2.c @@ -1219,6 +1219,8 @@ static int dwc2_init_common(struct udevice *dev, struct dwc2_priv *priv) if (readl(®s->gintsts) & DWC2_GINTSTS_CURMODE_HOST) mdelay(1000);
+ printf("USB DWC2\n"); + return 0; }
On 2/18/20 9:34 AM, Patrick Delaunay wrote:
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 availables.
The V4 is rebased on latest master (v2020.04-rc2). CI-Tavis build is OK: https://travis-ci.org/patrickdelaunay/u-boot/builds/651479714
NB: CI-Travis build was OK for all target after V3: https://travis-ci.org/patrickdelaunay/u-boot/builds/609496187 As 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
Simon, can you test this on SOCFPGA ?
[...]
On Tue, Feb 18, 2020 at 6:53 PM Marek Vasut marex@denx.de wrote:
On 2/18/20 9:34 AM, Patrick Delaunay wrote:
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 availables.
The V4 is rebased on latest master (v2020.04-rc2). CI-Tavis build is OK: https://travis-ci.org/patrickdelaunay/u-boot/builds/651479714
NB: CI-Travis build was OK for all target after V3: https://travis-ci.org/patrickdelaunay/u-boot/builds/609496187 As 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
Simon, can you test this on SOCFPGA ?
I can test if it probes, but I don't have anything running on that USB port the socfpga_socrates board has. Would that be enought to test?
Regards, Simon
[...]
Am 19.02.2020 um 08:27 schrieb Simon Goldschmidt:
On Tue, Feb 18, 2020 at 6:53 PM Marek Vasut marex@denx.de wrote:
On 2/18/20 9:34 AM, Patrick Delaunay wrote:
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 availables.
The V4 is rebased on latest master (v2020.04-rc2). CI-Tavis build is OK: https://travis-ci.org/patrickdelaunay/u-boot/builds/651479714
NB: CI-Travis build was OK for all target after V3: https://travis-ci.org/patrickdelaunay/u-boot/builds/609496187 As 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
Simon, can you test this on SOCFPGA ?
I can test if it probes, but I don't have anything running on that USB port the socfpga_socrates board has. Would that be enought to test?
Tested the whole series on socfpga_socrates by instantiating the driver. Shows the same behaviour as before (I still have no OTG cable to test attaching a storage device).
Regards, Simon
Regards, Simon
[...]
participants (4)
-
Marek Vasut -
Patrick DELAUNAY -
Patrick Delaunay
-
Simon Goldschmidt