[U-Boot] [RESEND PATCH v4] usb: dwc2: add support for external vbus supply
Some board do not use the dwc2 internal VBUS_DRV signal, but use a gpio pin to enable the 5.0V VBUS power, add interface to enable the power in dwc2 driver.
Signed-off-by: Kever Yang kever.yang@rock-chips.com Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v4: - Drop no use code comment by Marek.
Changes in v3: - Drop use of static variable
drivers/usb/host/dwc2.c | 37 ++++++++++++++++++++++++++++++++----- 1 file changed, 32 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c index d253b94..5ac602e 100644 --- a/drivers/usb/host/dwc2.c +++ b/drivers/usb/host/dwc2.c @@ -15,6 +15,7 @@ #include <usbroothubdes.h> #include <wait_bit.h> #include <asm/io.h> +#include <power/regulator.h>
#include "dwc2.h"
@@ -159,6 +160,27 @@ static void dwc_otg_core_reset(struct dwc2_core_regs *regs) mdelay(100); }
+static int dwc_vbus_supply_init(struct udevice *dev) +{ +#if defined(CONFIG_DM_USB) && defined(CONFIG_DM_REGULATOR) + struct udevice *vbus_supply; + int ret; + + ret = device_get_supply_regulator(dev, "vbus-supply", &vbus_supply); + if (ret) { + debug("%s: No vbus supply\n", dev->name); + return 0; + } + + ret = regulator_set_enable(vbus_supply, true); + if (ret) { + error("Error enabling vbus supply\n"); + return ret; + } +#endif + return 0; +} + /* * This function initializes the DWC_otg controller registers for * host mode. @@ -167,10 +189,12 @@ static void dwc_otg_core_reset(struct dwc2_core_regs *regs) * request queues. Host channels are reset to ensure that they are ready for * performing transfers. * + * @param dev USB Device (NULL if driver model is not being used) * @param regs Programming view of DWC_otg controller * */ -static void dwc_otg_core_host_init(struct dwc2_core_regs *regs) +static void dwc_otg_core_host_init(struct udevice *dev, + struct dwc2_core_regs *regs) { uint32_t nptxfifosize = 0; uint32_t ptxfifosize = 0; @@ -248,6 +272,9 @@ static void dwc_otg_core_host_init(struct dwc2_core_regs *regs) writel(hprt0, ®s->hprt0); } } + + if (dev) + dwc_vbus_supply_init(dev); }
/* @@ -1048,7 +1075,7 @@ int _submit_int_msg(struct dwc2_priv *priv, struct usb_device *dev, } }
-static int dwc2_init_common(struct dwc2_priv *priv) +static int dwc2_init_common(struct udevice *dev, struct dwc2_priv *priv) { struct dwc2_core_regs *regs = priv->regs; uint32_t snpsid; @@ -1070,7 +1097,7 @@ static int dwc2_init_common(struct dwc2_priv *priv) #endif
dwc_otg_core_init(priv); - dwc_otg_core_host_init(regs); + dwc_otg_core_host_init(dev, regs);
clrsetbits_le32(®s->hprt0, DWC2_HPRT0_PRTENA | DWC2_HPRT0_PRTCONNDET | DWC2_HPRT0_PRTENCHNG | @@ -1143,7 +1170,7 @@ int usb_lowlevel_init(int index, enum usb_init_type init, void **controller) if (board_usb_init(index, USB_INIT_HOST)) return -1;
- return dwc2_init_common(priv); + return dwc2_init_common(NULL, priv); }
int usb_lowlevel_stop(int index) @@ -1214,7 +1241,7 @@ static int dwc2_usb_probe(struct udevice *dev)
bus_priv->desc_before_addr = true;
- return dwc2_init_common(priv); + return dwc2_init_common(dev, priv); }
static int dwc2_usb_remove(struct udevice *dev)
On 03/06/2017 01:54 PM, Kever Yang wrote:
Some board do not use the dwc2 internal VBUS_DRV signal, but use a gpio pin to enable the 5.0V VBUS power, add interface to enable the power in dwc2 driver.
Signed-off-by: Kever Yang kever.yang@rock-chips.com Signed-off-by: Simon Glass sjg@chromium.org
Changes in v4:
- Drop no use code comment by Marek.
Changes in v3:
- Drop use of static variable
drivers/usb/host/dwc2.c | 37 ++++++++++++++++++++++++++++++++----- 1 file changed, 32 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c index d253b94..5ac602e 100644 --- a/drivers/usb/host/dwc2.c +++ b/drivers/usb/host/dwc2.c @@ -15,6 +15,7 @@ #include <usbroothubdes.h> #include <wait_bit.h> #include <asm/io.h> +#include <power/regulator.h>
#include "dwc2.h"
@@ -159,6 +160,27 @@ static void dwc_otg_core_reset(struct dwc2_core_regs *regs) mdelay(100); }
Nit:
add the ifdef around the function, ie.
#ifdef (.....) static foo() { do stuff } #else static foo() { return 0 } #endif
The rest is great, thanks!
+static int dwc_vbus_supply_init(struct udevice *dev) +{ +#if defined(CONFIG_DM_USB) && defined(CONFIG_DM_REGULATOR)
- struct udevice *vbus_supply;
- int ret;
- ret = device_get_supply_regulator(dev, "vbus-supply", &vbus_supply);
- if (ret) {
debug("%s: No vbus supply\n", dev->name);return 0;- }
- ret = regulator_set_enable(vbus_supply, true);
- if (ret) {
error("Error enabling vbus supply\n");return ret;- }
+#endif
- return 0;
+}
/*
- This function initializes the DWC_otg controller registers for
- host mode.
@@ -167,10 +189,12 @@ static void dwc_otg_core_reset(struct dwc2_core_regs *regs)
- request queues. Host channels are reset to ensure that they are ready for
- performing transfers.
*/
- @param dev USB Device (NULL if driver model is not being used)
- @param regs Programming view of DWC_otg controller
-static void dwc_otg_core_host_init(struct dwc2_core_regs *regs) +static void dwc_otg_core_host_init(struct udevice *dev,
struct dwc2_core_regs *regs){ uint32_t nptxfifosize = 0; uint32_t ptxfifosize = 0; @@ -248,6 +272,9 @@ static void dwc_otg_core_host_init(struct dwc2_core_regs *regs) writel(hprt0, ®s->hprt0); } }
- if (dev)
dwc_vbus_supply_init(dev);}
/* @@ -1048,7 +1075,7 @@ int _submit_int_msg(struct dwc2_priv *priv, struct usb_device *dev, } }
-static int dwc2_init_common(struct dwc2_priv *priv) +static int dwc2_init_common(struct udevice *dev, struct dwc2_priv *priv) { struct dwc2_core_regs *regs = priv->regs; uint32_t snpsid; @@ -1070,7 +1097,7 @@ static int dwc2_init_common(struct dwc2_priv *priv) #endif
dwc_otg_core_init(priv);
- dwc_otg_core_host_init(regs);
dwc_otg_core_host_init(dev, regs);
clrsetbits_le32(®s->hprt0, DWC2_HPRT0_PRTENA | DWC2_HPRT0_PRTCONNDET | DWC2_HPRT0_PRTENCHNG |
@@ -1143,7 +1170,7 @@ int usb_lowlevel_init(int index, enum usb_init_type init, void **controller) if (board_usb_init(index, USB_INIT_HOST)) return -1;
- return dwc2_init_common(priv);
- return dwc2_init_common(NULL, priv);
}
int usb_lowlevel_stop(int index) @@ -1214,7 +1241,7 @@ static int dwc2_usb_probe(struct udevice *dev)
bus_priv->desc_before_addr = true;
- return dwc2_init_common(priv);
- return dwc2_init_common(dev, priv);
}
static int dwc2_usb_remove(struct udevice *dev)
Hi Marek,
On 03/07/2017 10:55 AM, Marek Vasut wrote:
On 03/06/2017 01:54 PM, Kever Yang wrote:
Some board do not use the dwc2 internal VBUS_DRV signal, but use a gpio pin to enable the 5.0V VBUS power, add interface to enable the power in dwc2 driver.
Signed-off-by: Kever Yang kever.yang@rock-chips.com Signed-off-by: Simon Glass sjg@chromium.org
Changes in v4:
- Drop no use code comment by Marek.
Changes in v3:
- Drop use of static variable
drivers/usb/host/dwc2.c | 37 ++++++++++++++++++++++++++++++++----- 1 file changed, 32 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c index d253b94..5ac602e 100644 --- a/drivers/usb/host/dwc2.c +++ b/drivers/usb/host/dwc2.c @@ -15,6 +15,7 @@ #include <usbroothubdes.h> #include <wait_bit.h> #include <asm/io.h> +#include <power/regulator.h>
#include "dwc2.h"
@@ -159,6 +160,27 @@ static void dwc_otg_core_reset(struct dwc2_core_regs *regs) mdelay(100); }
Nit:
add the ifdef around the function, ie.
I don't understand what to do here, we already have "#if defined(CONFIG_DM_USB) && defined(CONFIG_DM_REGULATOR) " in the function, do you mean move this out of the function or do you need add any other MACRO?
Thanks, - Kever
#ifdef (.....) static foo() { do stuff } #else static foo() { return 0 } #endif
The rest is great, thanks!
+static int dwc_vbus_supply_init(struct udevice *dev) +{ +#if defined(CONFIG_DM_USB) && defined(CONFIG_DM_REGULATOR)
- struct udevice *vbus_supply;
- int ret;
- ret = device_get_supply_regulator(dev, "vbus-supply",
&vbus_supply);
- if (ret) {
debug("%s: No vbus supply\n", dev->name);return 0;- }
- ret = regulator_set_enable(vbus_supply, true);
- if (ret) {
error("Error enabling vbus supply\n");return ret;- }
+#endif
- return 0;
+}
/*
- This function initializes the DWC_otg controller registers for
- host mode.
@@ -167,10 +189,12 @@ static void dwc_otg_core_reset(struct dwc2_core_regs *regs)
- request queues. Host channels are reset to ensure that they are
ready for
- performing transfers.
*/
- @param dev USB Device (NULL if driver model is not being used)
- @param regs Programming view of DWC_otg controller
-static void dwc_otg_core_host_init(struct dwc2_core_regs *regs) +static void dwc_otg_core_host_init(struct udevice *dev,
struct dwc2_core_regs *regs){ uint32_t nptxfifosize = 0; uint32_t ptxfifosize = 0; @@ -248,6 +272,9 @@ static void dwc_otg_core_host_init(struct dwc2_core_regs *regs) writel(hprt0, ®s->hprt0); } }
- if (dev)
dwc_vbus_supply_init(dev);}
/* @@ -1048,7 +1075,7 @@ int _submit_int_msg(struct dwc2_priv *priv, struct usb_device *dev, } }
-static int dwc2_init_common(struct dwc2_priv *priv) +static int dwc2_init_common(struct udevice *dev, struct dwc2_priv *priv) { struct dwc2_core_regs *regs = priv->regs; uint32_t snpsid; @@ -1070,7 +1097,7 @@ static int dwc2_init_common(struct dwc2_priv *priv) #endif
dwc_otg_core_init(priv);
- dwc_otg_core_host_init(regs);
dwc_otg_core_host_init(dev, regs);
clrsetbits_le32(®s->hprt0, DWC2_HPRT0_PRTENA | DWC2_HPRT0_PRTCONNDET | DWC2_HPRT0_PRTENCHNG |
@@ -1143,7 +1170,7 @@ int usb_lowlevel_init(int index, enum usb_init_type init, void **controller) if (board_usb_init(index, USB_INIT_HOST)) return -1;
- return dwc2_init_common(priv);
- return dwc2_init_common(NULL, priv);
}
int usb_lowlevel_stop(int index) @@ -1214,7 +1241,7 @@ static int dwc2_usb_probe(struct udevice *dev)
bus_priv->desc_before_addr = true;
- return dwc2_init_common(priv);
- return dwc2_init_common(dev, priv);
}
static int dwc2_usb_remove(struct udevice *dev)
On 03/07/2017 07:50 AM, Kever Yang wrote:
Hi Marek,
On 03/07/2017 10:55 AM, Marek Vasut wrote:
On 03/06/2017 01:54 PM, Kever Yang wrote:
Some board do not use the dwc2 internal VBUS_DRV signal, but use a gpio pin to enable the 5.0V VBUS power, add interface to enable the power in dwc2 driver.
Signed-off-by: Kever Yang kever.yang@rock-chips.com Signed-off-by: Simon Glass sjg@chromium.org
Changes in v4:
- Drop no use code comment by Marek.
Changes in v3:
- Drop use of static variable
drivers/usb/host/dwc2.c | 37 ++++++++++++++++++++++++++++++++----- 1 file changed, 32 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c index d253b94..5ac602e 100644 --- a/drivers/usb/host/dwc2.c +++ b/drivers/usb/host/dwc2.c @@ -15,6 +15,7 @@ #include <usbroothubdes.h> #include <wait_bit.h> #include <asm/io.h> +#include <power/regulator.h>
#include "dwc2.h"
@@ -159,6 +160,27 @@ static void dwc_otg_core_reset(struct dwc2_core_regs *regs) mdelay(100); }
Nit:
add the ifdef around the function, ie.
I don't understand what to do here, we already have "#if defined(CONFIG_DM_USB) && defined(CONFIG_DM_REGULATOR) " in the function, do you mean move this out of the function or do you need add any other MACRO?
The example of what you should do is right below
Thanks,
- Kever
#ifdef (.....) static foo() { do stuff } #else static foo() { return 0 } #endif
here ^
The rest is great, thanks!
+static int dwc_vbus_supply_init(struct udevice *dev) +{ +#if defined(CONFIG_DM_USB) && defined(CONFIG_DM_REGULATOR)
- struct udevice *vbus_supply;
- int ret;
- ret = device_get_supply_regulator(dev, "vbus-supply",
&vbus_supply);
- if (ret) {
debug("%s: No vbus supply\n", dev->name);return 0;- }
- ret = regulator_set_enable(vbus_supply, true);
- if (ret) {
error("Error enabling vbus supply\n");return ret;- }
+#endif
- return 0;
+}
/*
- This function initializes the DWC_otg controller registers for
- host mode.
@@ -167,10 +189,12 @@ static void dwc_otg_core_reset(struct dwc2_core_regs *regs)
- request queues. Host channels are reset to ensure that they are
ready for
- performing transfers.
*/
- @param dev USB Device (NULL if driver model is not being used)
- @param regs Programming view of DWC_otg controller
-static void dwc_otg_core_host_init(struct dwc2_core_regs *regs) +static void dwc_otg_core_host_init(struct udevice *dev,
struct dwc2_core_regs *regs){ uint32_t nptxfifosize = 0; uint32_t ptxfifosize = 0; @@ -248,6 +272,9 @@ static void dwc_otg_core_host_init(struct dwc2_core_regs *regs) writel(hprt0, ®s->hprt0); } }
- if (dev)
dwc_vbus_supply_init(dev);}
/* @@ -1048,7 +1075,7 @@ int _submit_int_msg(struct dwc2_priv *priv, struct usb_device *dev, } }
-static int dwc2_init_common(struct dwc2_priv *priv) +static int dwc2_init_common(struct udevice *dev, struct dwc2_priv *priv) { struct dwc2_core_regs *regs = priv->regs; uint32_t snpsid; @@ -1070,7 +1097,7 @@ static int dwc2_init_common(struct dwc2_priv *priv) #endif
dwc_otg_core_init(priv);
- dwc_otg_core_host_init(regs);
dwc_otg_core_host_init(dev, regs);
clrsetbits_le32(®s->hprt0, DWC2_HPRT0_PRTENA | DWC2_HPRT0_PRTCONNDET | DWC2_HPRT0_PRTENCHNG |
@@ -1143,7 +1170,7 @@ int usb_lowlevel_init(int index, enum usb_init_type init, void **controller) if (board_usb_init(index, USB_INIT_HOST)) return -1;
- return dwc2_init_common(priv);
- return dwc2_init_common(NULL, priv);
}
int usb_lowlevel_stop(int index) @@ -1214,7 +1241,7 @@ static int dwc2_usb_probe(struct udevice *dev)
bus_priv->desc_before_addr = true;
- return dwc2_init_common(priv);
- return dwc2_init_common(dev, priv);
}
static int dwc2_usb_remove(struct udevice *dev)
Hi Marek,
On 03/10/2017 10:16 AM, Marek Vasut wrote:
On 03/07/2017 07:50 AM, Kever Yang wrote:
Hi Marek,
On 03/07/2017 10:55 AM, Marek Vasut wrote:
On 03/06/2017 01:54 PM, Kever Yang wrote:
Some board do not use the dwc2 internal VBUS_DRV signal, but use a gpio pin to enable the 5.0V VBUS power, add interface to enable the power in dwc2 driver.
Signed-off-by: Kever Yang kever.yang@rock-chips.com Signed-off-by: Simon Glass sjg@chromium.org
Changes in v4:
- Drop no use code comment by Marek.
Changes in v3:
Drop use of static variable
drivers/usb/host/dwc2.c | 37 ++++++++++++++++++++++++++++++++----- 1 file changed, 32 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c index d253b94..5ac602e 100644 --- a/drivers/usb/host/dwc2.c +++ b/drivers/usb/host/dwc2.c @@ -15,6 +15,7 @@ #include <usbroothubdes.h> #include <wait_bit.h> #include <asm/io.h> +#include <power/regulator.h>
#include "dwc2.h"
@@ -159,6 +160,27 @@ static void dwc_otg_core_reset(struct dwc2_core_regs *regs) mdelay(100); }
Nit:
add the ifdef around the function, ie.
I don't understand what to do here, we already have "#if defined(CONFIG_DM_USB) && defined(CONFIG_DM_REGULATOR)" in the function, do you mean move this out of the function or do you need add any other MACRO?
The example of what you should do is right below
Thanks for your example, I just don't understand why do you think add the ifdef around the function is better than inside the function? Does this a rule in U-Boot?
Thanks, - Kever
Thanks,
- Kever
#ifdef (.....) static foo() { do stuff } #else static foo() { return 0 } #endif
here ^
The rest is great, thanks!
+static int dwc_vbus_supply_init(struct udevice *dev) +{ +#if defined(CONFIG_DM_USB) && defined(CONFIG_DM_REGULATOR)
- struct udevice *vbus_supply;
- int ret;
- ret = device_get_supply_regulator(dev, "vbus-supply",
&vbus_supply);
- if (ret) {
debug("%s: No vbus supply\n", dev->name);return 0;- }
- ret = regulator_set_enable(vbus_supply, true);
- if (ret) {
error("Error enabling vbus supply\n");return ret;- }
+#endif
- return 0;
+}
- /*
- This function initializes the DWC_otg controller registers for
- host mode.
@@ -167,10 +189,12 @@ static void dwc_otg_core_reset(struct dwc2_core_regs *regs)
- request queues. Host channels are reset to ensure that they are
ready for
- performing transfers.
*/
- @param dev USB Device (NULL if driver model is not being used)
- @param regs Programming view of DWC_otg controller
-static void dwc_otg_core_host_init(struct dwc2_core_regs *regs) +static void dwc_otg_core_host_init(struct udevice *dev,
struct dwc2_core_regs *regs)@@ -248,6 +272,9 @@ static void dwc_otg_core_host_init(struct dwc2_core_regs *regs) writel(hprt0, ®s->hprt0); } }
if (dev)
dwc_vbus_supply_init(dev);}
/*
@@ -1048,7 +1075,7 @@ int _submit_int_msg(struct dwc2_priv *priv, struct usb_device *dev, } }
-static int dwc2_init_common(struct dwc2_priv *priv) +static int dwc2_init_common(struct udevice *dev, struct dwc2_priv *priv) { struct dwc2_core_regs *regs = priv->regs; uint32_t snpsid; @@ -1070,7 +1097,7 @@ static int dwc2_init_common(struct dwc2_priv *priv) #endif
dwc_otg_core_init(priv);
- dwc_otg_core_host_init(regs);
dwc_otg_core_host_init(dev, regs);
clrsetbits_le32(®s->hprt0, DWC2_HPRT0_PRTENA | DWC2_HPRT0_PRTCONNDET | DWC2_HPRT0_PRTENCHNG |
@@ -1143,7 +1170,7 @@ int usb_lowlevel_init(int index, enum usb_init_type init, void **controller) if (board_usb_init(index, USB_INIT_HOST)) return -1;
- return dwc2_init_common(priv);
return dwc2_init_common(NULL, priv); }
int usb_lowlevel_stop(int index)
@@ -1214,7 +1241,7 @@ static int dwc2_usb_probe(struct udevice *dev)
bus_priv->desc_before_addr = true;
- return dwc2_init_common(priv);
return dwc2_init_common(dev, priv); }
static int dwc2_usb_remove(struct udevice *dev)
On 03/10/2017 05:02 AM, Kever Yang wrote:
Hi Marek,
On 03/10/2017 10:16 AM, Marek Vasut wrote:
On 03/07/2017 07:50 AM, Kever Yang wrote:
Hi Marek,
On 03/07/2017 10:55 AM, Marek Vasut wrote:
On 03/06/2017 01:54 PM, Kever Yang wrote:
Some board do not use the dwc2 internal VBUS_DRV signal, but use a gpio pin to enable the 5.0V VBUS power, add interface to enable the power in dwc2 driver.
Signed-off-by: Kever Yang kever.yang@rock-chips.com Signed-off-by: Simon Glass sjg@chromium.org
Changes in v4:
- Drop no use code comment by Marek.
Changes in v3:
Drop use of static variable
drivers/usb/host/dwc2.c | 37 ++++++++++++++++++++++++++++++++----- 1 file changed, 32 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c index d253b94..5ac602e 100644 --- a/drivers/usb/host/dwc2.c +++ b/drivers/usb/host/dwc2.c @@ -15,6 +15,7 @@ #include <usbroothubdes.h> #include <wait_bit.h> #include <asm/io.h> +#include <power/regulator.h>
#include "dwc2.h"
@@ -159,6 +160,27 @@ static void dwc_otg_core_reset(struct dwc2_core_regs *regs) mdelay(100); }
Nit:
add the ifdef around the function, ie.
I don't understand what to do here, we already have "#if defined(CONFIG_DM_USB) && defined(CONFIG_DM_REGULATOR)" in the function, do you mean move this out of the function or do you need add any other MACRO?
The example of what you should do is right below
Thanks for your example, I just don't understand why do you think add the ifdef around the function is better than inside the function? Does this a rule in U-Boot?
It's more readable, you can clearly tell that the whole function is conditionally compiled in. (and it's consistent with the rest of the codebase)
participants (2)
-
Kever Yang -
Marek Vasut