Re: [U-Boot] [PATCH v2 1/4] usb: dwc2: add support for external vbus supply

[Kever Yang]

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.


+
  /*
   * 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.


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.


+       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?

Thanks,
- Kever

+       if (ret) {
+               error("Error enabling vbus supply\n");
+               return ret;
+       }
+#endif
+       return 0;
+}
+
  /*
   * This function initializes the DWC_otg controller registers for
   * host mode.
@@ -248,6 +274,8 @@ static void dwc_otg_core_host_init(struct dwc2_core_regs 
*regs)
                        writel(hprt0, &regs->hprt0);
                }
        }
+
+       dwc_vbus_supply_init();
  }
  
  /*
@@ -1194,6 +1222,7 @@ static int dwc2_usb_ofdata_to_platdata(struct udevice 
*dev)
        const void *prop;
        fdt_addr_t addr;
  
+	g_dwc2_udev = dev;
        addr = dev_get_addr(dev);
        if (addr == FDT_ADDR_T_NONE)
                return -EINVAL;




_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot