Hi Marek,
On 29 November 2016 at 19:00, Marek Vasut marex@denx.de wrote:
On 11/29/2016 09:46 AM, Kever Yang wrote:
Hi Marek,
On 11/26/2016 12:46 AM, Marek Vasut wrote:
On 11/24/2016 08:29 AM, 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
Changes in v2: None
drivers/usb/host/dwc2.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+)
diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c index d08879d..8292aa8 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" @@ -55,6 +56,8 @@ DEFINE_ALIGN_BUFFER(uint8_t, status_buffer_addr, DWC2_STATUS_BUF_SIZE, static struct dwc2_priv local; #endif +static struct udevice *g_dwc2_udev;
Can we avoid the static global var ?
I don't want to use this global var either, but I don't know how to get this udev structure without this var, do you have any good suggestion? BTW, of_container() is not available to find the udevice structure with dwc2_priv pointer.
But you can ie. store the udevice in priv ? Simon , any hints ?
I think it can just be a parameter. I sent a v3 patch to show how to do it.
- /*
*/
- DWC2 IP interface
@@ -159,6 +162,29 @@ static void dwc_otg_core_reset(struct dwc2_core_regs *regs) mdelay(100); } +static int dwc_vbus_supply_init(void) +{ +#if defined(CONFIG_DM_USB) && defined(CONFIG_DM_REGULATOR) && \
- !defined(CONFIG_SPL_BUILD)
Why does this not work for SPL ?
We do not enable the USB driver in SPL, in this case I can just drop the CONFIG_SPL_BUILD because the driver in not compiled in SPL.
Correct
btw, I'd rather see this as:
#if FOO function bar() { ... } #else static inline function bar() { return 0; } #endif
- struct udevice *vbus_supply;
- int ret;
- ret = device_get_supply_regulator(g_dwc2_udev, "vbus-supply",
&vbus_supply);Is this compatible with the Linux DWC2 DT bindings ?
This compatible in kernel is in in phy dts node, the dwc2 driver and phy driver in U-Boot are both very different from kernel now, so I chose a place which is reasonable to enable the vbus.
DT is driver and OS agnostic, so if there is already agreed-upon binding for specifying external VBUS to DWC2, we should use it. Why would that be a problem ?
You can bring in the DT node, then use special code to find it. But I looked in the linux kernel and cannot find vbus-supply. Are you going to send that patch to the kernel at some point?
- if (ret) {
debug("%s: No vbus supply\n", g_dwc2_udev->name);return 0;- }
- ret = regulator_set_enable(vbus_supply, true);
Shouldn't the regulator be enabled when we enable VBUS for a port instead of here ? And where is it disabled ?
I add this function just after the controller enable the port power bit, isn't it? I didn't found anywhere the driver disable the vbus power, we do not need it in U-Boot?
So we leave it enabled and pass the hardware to kernel in some weird state ? That's a bit odd, isn't it. dwc2_lowlevel_stop() might be a good place to disable the regulator ...
-- Best regards, Marek Vasut
Regards, Simon