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.y...@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 ? >> >>> + >>> /* >>> * 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 ? >>> + 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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot