[U-Boot] [PATCH v4 00/11] rk3328: add support of usb host and gadget
Add support of usb host and gadget function for rk3328.
Changes in v4: - Splite patch [U-boot,v3,04/10] into two patches - Define set vbus as empty function if the macros aren't set - Prepare a mask and set capabilities in GUSBCFG register once
Changes in v3: - Revert change of macro definition in dwc2 driver - Support host mode without HNP/SRP capability through DTS
Changes in v2: - Add commit messages - Split patch [U-boot,7/8] into two patches - Use fixed regulator to control vbus instead of gpio
Meng Dongyang (11): configs: rk3328: add support for usb and config ehci and ohci driver rockchip: dts: rk3328: add ehci and ohci node and enable host0 port configs: rk3328: config xhci controller usb: host: xhci-rockchip: use fixed regulator to control vbus usb: host: xhci-rockchip: add support for rk3328 rockchip: dts: rk3328: support and enable xhci configs: rk3328: enable dwc2 driver and config for fastboot usb: dwc2: force to host mode if not support HNP/SRP rk3328: board: add support of dwc2 gadget rockchip: dts: rk3328: support and enable dwc2 rockchip: dts: rk3399: control vbus of typec by fixed regulator
arch/arm/dts/rk3328-evb.dts | 36 ++++++++++++++++++++++++++++ arch/arm/dts/rk3328.dtsi | 35 +++++++++++++++++++++++++++ arch/arm/dts/rk3399-evb.dts | 16 +++++++++++-- board/rockchip/evb_rk3328/evb-rk3328.c | 43 +++++++++++++++++++++++++++++++--- configs/evb-rk3328_defconfig | 25 ++++++++++++++++++++ drivers/usb/host/dwc2.c | 19 ++++++++++++--- drivers/usb/host/xhci-rockchip.c | 43 +++++++++++++++++++++++++++------- include/configs/rk3328_common.h | 9 +++++++ 8 files changed, 209 insertions(+), 17 deletions(-)
Add defconfig for usb and ehci and ohci controller, config maximal number of ports of the root hub for ohci driver.
Signed-off-by: Meng Dongyang daniel.meng@rock-chips.com Reviewed-by: Simon Glass sjg@chromium.org ---
Changes in v4: None Changes in v3: None Changes in v2: None
configs/evb-rk3328_defconfig | 8 ++++++++ include/configs/rk3328_common.h | 3 +++ 2 files changed, 11 insertions(+)
diff --git a/configs/evb-rk3328_defconfig b/configs/evb-rk3328_defconfig index 96241f6..a4312b2 100644 --- a/configs/evb-rk3328_defconfig +++ b/configs/evb-rk3328_defconfig @@ -31,3 +31,11 @@ CONFIG_SYS_NS16550=y CONFIG_SYSRESET=y CONFIG_USE_TINY_PRINTF=y CONFIG_ERRNO_STR=y +CONFIG_USB=y +CONFIG_DM_USB=y +CONFIG_CMD_USB=y +CONFIG_USB_STORAGE=y +CONFIG_USB_EHCI_HCD=y +CONFIG_USB_EHCI_GENERIC=y +CONFIG_USB_OHCI_HCD=y +CONFIG_USB_OHCI_GENERIC=y diff --git a/include/configs/rk3328_common.h b/include/configs/rk3328_common.h index b0dcd48..96b71c3 100644 --- a/include/configs/rk3328_common.h +++ b/include/configs/rk3328_common.h @@ -61,4 +61,7 @@
#endif
+/* rockchip ohci host driver */ +#define CONFIG_USB_OHCI_NEW +#define CONFIG_SYS_USB_OHCI_MAX_ROOT_PORTS 1 #endif
Add dts node for ehci and ohci controller, enable the controllers.
Signed-off-by: Meng Dongyang daniel.meng@rock-chips.com Reviewed-by: Simon Glass sjg@chromium.org ---
Changes in v4: None Changes in v3: None Changes in v2: None
arch/arm/dts/rk3328-evb.dts | 8 ++++++++ arch/arm/dts/rk3328.dtsi | 14 ++++++++++++++ 2 files changed, 22 insertions(+)
diff --git a/arch/arm/dts/rk3328-evb.dts b/arch/arm/dts/rk3328-evb.dts index 01794ed..9920935 100644 --- a/arch/arm/dts/rk3328-evb.dts +++ b/arch/arm/dts/rk3328-evb.dts @@ -43,3 +43,11 @@ pinctrl-0 = <&emmc_clk &emmc_cmd &emmc_bus8>; status = "okay"; }; + +&usb_host0_ehci { + status = "okay"; +}; + +&usb_host0_ohci { + status = "okay"; +}; diff --git a/arch/arm/dts/rk3328.dtsi b/arch/arm/dts/rk3328.dtsi index 8a98ee3..e1219c3 100644 --- a/arch/arm/dts/rk3328.dtsi +++ b/arch/arm/dts/rk3328.dtsi @@ -446,6 +446,20 @@ status = "disabled"; };
+ usb_host0_ehci: usb@ff5c0000 { + compatible = "generic-ehci"; + reg = <0x0 0xff5c0000 0x0 0x10000>; + interrupts = <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>; + status = "disabled"; + }; + + usb_host0_ohci: usb@ff5d0000 { + compatible = "generic-ohci"; + reg = <0x0 0xff5d0000 0x0 0x10000>; + interrupts = <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>; + status = "disabled"; + }; + sdmmc_ext: rksdmmc@ff5f0000 { compatible = "rockchip,rk3328-dw-mshc", "rockchip,rk3288-dw-mshc"; reg = <0x0 0xff5f0000 0x0 0x4000>;
Add config of max root ports and add config to enable xhci controller.
Signed-off-by: Meng Dongyang daniel.meng@rock-chips.com Reviewed-by: Simon Glass sjg@chromium.org ---
Changes in v4: None Changes in v3: None Changes in v2: None
configs/evb-rk3328_defconfig | 3 +++ include/configs/rk3328_common.h | 3 +++ 2 files changed, 6 insertions(+)
diff --git a/configs/evb-rk3328_defconfig b/configs/evb-rk3328_defconfig index a4312b2..57aee02 100644 --- a/configs/evb-rk3328_defconfig +++ b/configs/evb-rk3328_defconfig @@ -39,3 +39,6 @@ CONFIG_USB_EHCI_HCD=y CONFIG_USB_EHCI_GENERIC=y CONFIG_USB_OHCI_HCD=y CONFIG_USB_OHCI_GENERIC=y +CONFIG_USB_XHCI_DWC3=y +CONFIG_USB_XHCI_HCD=y +CONFIG_USB_XHCI_RK=y diff --git a/include/configs/rk3328_common.h b/include/configs/rk3328_common.h index 96b71c3..cc6c890 100644 --- a/include/configs/rk3328_common.h +++ b/include/configs/rk3328_common.h @@ -64,4 +64,7 @@ /* rockchip ohci host driver */ #define CONFIG_USB_OHCI_NEW #define CONFIG_SYS_USB_OHCI_MAX_ROOT_PORTS 1 + +/* xhci host */ +#define CONFIG_SYS_USB_XHCI_MAX_ROOT_PORTS 2 #endif
Use fixed regulator to control the voltage of vbus and turn off vbus when usb stop.
Signed-off-by: Meng Dongyang daniel.meng@rock-chips.com ---
Changes in v4: - Splited from patch [Uboot,v3,04/10] - Define set vbus as empty function if the macros aren't set
Changes in v3: None Changes in v2: - Use fixed regulator to control vbus instead of gpio
drivers/usb/host/xhci-rockchip.c | 41 +++++++++++++++++++++++++++++++--------- 1 file changed, 32 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/host/xhci-rockchip.c b/drivers/usb/host/xhci-rockchip.c index f559830..dc9cd56 100644 --- a/drivers/usb/host/xhci-rockchip.c +++ b/drivers/usb/host/xhci-rockchip.c @@ -11,10 +11,10 @@ #include <malloc.h> #include <usb.h> #include <watchdog.h> -#include <asm/gpio.h> #include <linux/errno.h> #include <linux/compat.h> #include <linux/usb/dwc3.h> +#include <power/regulator.h>
#include "xhci.h"
@@ -23,7 +23,7 @@ DECLARE_GLOBAL_DATA_PTR; struct rockchip_xhci_platdata { fdt_addr_t hcd_base; fdt_addr_t phy_base; - struct gpio_desc vbus_gpio; + struct udevice *vbus_supply; };
/* @@ -66,15 +66,37 @@ static int xhci_usb_ofdata_to_platdata(struct udevice *dev) return -ENXIO; }
- /* Vbus gpio */ - ret = gpio_request_by_name(dev, "rockchip,vbus-gpio", 0, - &plat->vbus_gpio, GPIOD_IS_OUT); +#if defined(CONFIG_DM_USB) && defined(CONFIG_DM_REGULATOR) + /* Vbus regulator */ + ret = device_get_supply_regulator(dev, "vbus-supply", + &plat->vbus_supply); if (ret) - debug("rockchip,vbus-gpio node missing!"); + debug("Can't get vbus supply\n"); +#endif
return 0; }
+#if defined(CONFIG_DM_USB) && defined(CONFIG_DM_REGULATOR) +static int rockchip_xhci_set_vbus(struct rockchip_xhci_platdata *plat, + bool value) +{ + int ret = 0; + + ret = regulator_set_enable(plat->vbus_supply, value); + if (ret) + debug("XHCI: Failed to set vbus supply\n"); + + return ret; +} +#else +static int rockchip_xhci_set_vbus(struct rockchip_xhci_platdata *plat, + bool value) +{ + return 0; +} +#endif + /* * rockchip_dwc3_phy_setup() - Configure USB PHY Interface of DWC3 Core * @dwc: Pointer to our controller context structure @@ -153,9 +175,7 @@ static int xhci_usb_probe(struct udevice *dev) hcor = (struct xhci_hcor *)((uint64_t)ctx->hcd + HC_LENGTH(xhci_readl(&ctx->hcd->cr_capbase)));
- /* setup the Vbus gpio here */ - if (dm_gpio_is_valid(&plat->vbus_gpio)) - dm_gpio_set_value(&plat->vbus_gpio, 1); + rockchip_xhci_set_vbus(plat, true);
ret = rockchip_xhci_core_init(ctx, dev); if (ret) { @@ -168,6 +188,7 @@ static int xhci_usb_probe(struct udevice *dev)
static int xhci_usb_remove(struct udevice *dev) { + struct rockchip_xhci_platdata *plat = dev_get_platdata(dev); struct rockchip_xhci *ctx = dev_get_priv(dev); int ret;
@@ -178,6 +199,8 @@ static int xhci_usb_remove(struct udevice *dev) if (ret) return ret;
+ rockchip_xhci_set_vbus(plat, false); + return 0; }
On 06/08/2017 09:31 AM, Meng Dongyang wrote:
Use fixed regulator to control the voltage of vbus and turn off vbus when usb stop.
Signed-off-by: Meng Dongyang daniel.meng@rock-chips.com
Changes in v4:
- Splited from patch [Uboot,v3,04/10]
- Define set vbus as empty function if the macros aren't set
Changes in v3: None Changes in v2:
- Use fixed regulator to control vbus instead of gpio
drivers/usb/host/xhci-rockchip.c | 41 +++++++++++++++++++++++++++++++--------- 1 file changed, 32 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/host/xhci-rockchip.c b/drivers/usb/host/xhci-rockchip.c index f559830..dc9cd56 100644 --- a/drivers/usb/host/xhci-rockchip.c +++ b/drivers/usb/host/xhci-rockchip.c @@ -11,10 +11,10 @@ #include <malloc.h> #include <usb.h> #include <watchdog.h> -#include <asm/gpio.h> #include <linux/errno.h> #include <linux/compat.h> #include <linux/usb/dwc3.h> +#include <power/regulator.h>
#include "xhci.h"
@@ -23,7 +23,7 @@ DECLARE_GLOBAL_DATA_PTR; struct rockchip_xhci_platdata { fdt_addr_t hcd_base; fdt_addr_t phy_base;
- struct gpio_desc vbus_gpio;
- struct udevice *vbus_supply;
};
/* @@ -66,15 +66,37 @@ static int xhci_usb_ofdata_to_platdata(struct udevice *dev) return -ENXIO; }
- /* Vbus gpio */
- ret = gpio_request_by_name(dev, "rockchip,vbus-gpio", 0,
&plat->vbus_gpio, GPIOD_IS_OUT);+#if defined(CONFIG_DM_USB) && defined(CONFIG_DM_REGULATOR)
- /* Vbus regulator */
- ret = device_get_supply_regulator(dev, "vbus-supply",
&plat->vbus_supply);
debug("rockchip,vbus-gpio node missing!");
debug("Can't get vbus supply\n");
VBUS in caps
+#endif
return 0; }
+#if defined(CONFIG_DM_USB) && defined(CONFIG_DM_REGULATOR) +static int rockchip_xhci_set_vbus(struct rockchip_xhci_platdata *plat,
bool value)+{
- int ret = 0;
You don't need to init ret, it's always set right below.
- ret = regulator_set_enable(plat->vbus_supply, value);
- if (ret)
debug("XHCI: Failed to set vbus supply\n");
That shouldn't be debug, that's a printf() because it's actually a failure. Or error() I guess ...
- return ret;
+} +#else +static int rockchip_xhci_set_vbus(struct rockchip_xhci_platdata *plat,
bool value)+{
- return 0;
+} +#endif
/*
- rockchip_dwc3_phy_setup() - Configure USB PHY Interface of DWC3 Core
- @dwc: Pointer to our controller context structure
@@ -153,9 +175,7 @@ static int xhci_usb_probe(struct udevice *dev) hcor = (struct xhci_hcor *)((uint64_t)ctx->hcd + HC_LENGTH(xhci_readl(&ctx->hcd->cr_capbase)));
- /* setup the Vbus gpio here */
- if (dm_gpio_is_valid(&plat->vbus_gpio))
dm_gpio_set_value(&plat->vbus_gpio, 1);
- rockchip_xhci_set_vbus(plat, true);
What about handling the return value ?
ret = rockchip_xhci_core_init(ctx, dev); if (ret) { @@ -168,6 +188,7 @@ static int xhci_usb_probe(struct udevice *dev)
static int xhci_usb_remove(struct udevice *dev) {
- struct rockchip_xhci_platdata *plat = dev_get_platdata(dev); struct rockchip_xhci *ctx = dev_get_priv(dev); int ret;
@@ -178,6 +199,8 @@ static int xhci_usb_remove(struct udevice *dev) if (ret) return ret;
- rockchip_xhci_set_vbus(plat, false);
Handle return value
return 0; }
On 2017/6/8 21:17, Marek Vasut wrote:
On 06/08/2017 09:31 AM, Meng Dongyang wrote:
Use fixed regulator to control the voltage of vbus and turn off vbus when usb stop.
Signed-off-by: Meng Dongyang daniel.meng@rock-chips.com
Changes in v4:
- Splited from patch [Uboot,v3,04/10]
- Define set vbus as empty function if the macros aren't set
Changes in v3: None Changes in v2:
Use fixed regulator to control vbus instead of gpio
drivers/usb/host/xhci-rockchip.c | 41 +++++++++++++++++++++++++++++++--------- 1 file changed, 32 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/host/xhci-rockchip.c b/drivers/usb/host/xhci-rockchip.c index f559830..dc9cd56 100644 --- a/drivers/usb/host/xhci-rockchip.c +++ b/drivers/usb/host/xhci-rockchip.c @@ -11,10 +11,10 @@ #include <malloc.h> #include <usb.h> #include <watchdog.h> -#include <asm/gpio.h> #include <linux/errno.h> #include <linux/compat.h> #include <linux/usb/dwc3.h> +#include <power/regulator.h>
#include "xhci.h"
@@ -23,7 +23,7 @@ DECLARE_GLOBAL_DATA_PTR; struct rockchip_xhci_platdata { fdt_addr_t hcd_base; fdt_addr_t phy_base;
- struct gpio_desc vbus_gpio;
struct udevice *vbus_supply; };
/*
@@ -66,15 +66,37 @@ static int xhci_usb_ofdata_to_platdata(struct udevice *dev) return -ENXIO; }
- /* Vbus gpio */
- ret = gpio_request_by_name(dev, "rockchip,vbus-gpio", 0,
&plat->vbus_gpio, GPIOD_IS_OUT);+#if defined(CONFIG_DM_USB) && defined(CONFIG_DM_REGULATOR)
- /* Vbus regulator */
- ret = device_get_supply_regulator(dev, "vbus-supply",
&plat->vbus_supply);
debug("rockchip,vbus-gpio node missing!");
debug("Can't get vbus supply\n");VBUS in caps
+#endif
return 0; }
+#if defined(CONFIG_DM_USB) && defined(CONFIG_DM_REGULATOR) +static int rockchip_xhci_set_vbus(struct rockchip_xhci_platdata *plat,
bool value)+{
- int ret = 0;
You don't need to init ret, it's always set right below.
- ret = regulator_set_enable(plat->vbus_supply, value);
- if (ret)
debug("XHCI: Failed to set vbus supply\n");That shouldn't be debug, that's a printf() because it's actually a failure. Or error() I guess ...
Considering the case vbus is always on, it is right with no vbus regulator. So maybe it's not an error actually.
- return ret;
+} +#else +static int rockchip_xhci_set_vbus(struct rockchip_xhci_platdata *plat,
bool value)+{
- return 0;
+} +#endif
- /*
- rockchip_dwc3_phy_setup() - Configure USB PHY Interface of DWC3 Core
- @dwc: Pointer to our controller context structure
@@ -153,9 +175,7 @@ static int xhci_usb_probe(struct udevice *dev) hcor = (struct xhci_hcor *)((uint64_t)ctx->hcd + HC_LENGTH(xhci_readl(&ctx->hcd->cr_capbase)));
- /* setup the Vbus gpio here */
- if (dm_gpio_is_valid(&plat->vbus_gpio))
dm_gpio_set_value(&plat->vbus_gpio, 1);
- rockchip_xhci_set_vbus(plat, true);
What about handling the return value ?
The return value can be ignored when vbus is always on. So is it enough just print message in the rockchip_xhci_set_vbus function?
ret = rockchip_xhci_core_init(ctx, dev); if (ret) { @@ -168,6 +188,7 @@ static int xhci_usb_probe(struct udevice *dev)
static int xhci_usb_remove(struct udevice *dev) {
- struct rockchip_xhci_platdata *plat = dev_get_platdata(dev); struct rockchip_xhci *ctx = dev_get_priv(dev); int ret;
@@ -178,6 +199,8 @@ static int xhci_usb_remove(struct udevice *dev) if (ret) return ret;
- rockchip_xhci_set_vbus(plat, false);
Handle return value
return 0; }
On 06/09/2017 09:49 AM, rock-chips(daniel.meng) wrote:
On 2017/6/8 21:17, Marek Vasut wrote:
On 06/08/2017 09:31 AM, Meng Dongyang wrote:
Use fixed regulator to control the voltage of vbus and turn off vbus when usb stop.
Signed-off-by: Meng Dongyang daniel.meng@rock-chips.com
Changes in v4:
- Splited from patch [Uboot,v3,04/10]
- Define set vbus as empty function if the macros aren't set
Changes in v3: None Changes in v2:
Use fixed regulator to control vbus instead of gpio
drivers/usb/host/xhci-rockchip.c | 41
+++++++++++++++++++++++++++++++--------- 1 file changed, 32 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/host/xhci-rockchip.c b/drivers/usb/host/xhci-rockchip.c index f559830..dc9cd56 100644 --- a/drivers/usb/host/xhci-rockchip.c +++ b/drivers/usb/host/xhci-rockchip.c @@ -11,10 +11,10 @@ #include <malloc.h> #include <usb.h> #include <watchdog.h> -#include <asm/gpio.h> #include <linux/errno.h> #include <linux/compat.h> #include <linux/usb/dwc3.h> +#include <power/regulator.h> #include "xhci.h" @@ -23,7 +23,7 @@ DECLARE_GLOBAL_DATA_PTR; struct rockchip_xhci_platdata { fdt_addr_t hcd_base; fdt_addr_t phy_base;
- struct gpio_desc vbus_gpio;
- struct udevice *vbus_supply; }; /*
@@ -66,15 +66,37 @@ static int xhci_usb_ofdata_to_platdata(struct udevice *dev) return -ENXIO; }
- /* Vbus gpio */
- ret = gpio_request_by_name(dev, "rockchip,vbus-gpio", 0,
&plat->vbus_gpio, GPIOD_IS_OUT);+#if defined(CONFIG_DM_USB) && defined(CONFIG_DM_REGULATOR)
- /* Vbus regulator */
- ret = device_get_supply_regulator(dev, "vbus-supply",
&plat->vbus_supply); if (ret)
debug("rockchip,vbus-gpio node missing!");
debug("Can't get vbus supply\n");VBUS in caps
+#endif return 0; } +#if defined(CONFIG_DM_USB) && defined(CONFIG_DM_REGULATOR) +static int rockchip_xhci_set_vbus(struct rockchip_xhci_platdata *plat,
bool value)+{
- int ret = 0;
You don't need to init ret, it's always set right below.
- ret = regulator_set_enable(plat->vbus_supply, value);
- if (ret)
debug("XHCI: Failed to set vbus supply\n");That shouldn't be debug, that's a printf() because it's actually a failure. Or error() I guess ...
Considering the case vbus is always on, it is right with no vbus regulator. So maybe it's not an error actually.
Regulator failed to turn on, that's an error, right ? Why would VBUS be always on ? You can just add a GPIO-regulator into the DT and then the VBUS is no longer always on.
- return ret;
+} +#else +static int rockchip_xhci_set_vbus(struct rockchip_xhci_platdata *plat,
bool value)+{
- return 0;
+} +#endif
- /*
- rockchip_dwc3_phy_setup() - Configure USB PHY Interface of DWC3
Core
- @dwc: Pointer to our controller context structure
@@ -153,9 +175,7 @@ static int xhci_usb_probe(struct udevice *dev) hcor = (struct xhci_hcor *)((uint64_t)ctx->hcd + HC_LENGTH(xhci_readl(&ctx->hcd->cr_capbase)));
- /* setup the Vbus gpio here */
- if (dm_gpio_is_valid(&plat->vbus_gpio))
dm_gpio_set_value(&plat->vbus_gpio, 1);
- rockchip_xhci_set_vbus(plat, true);
What about handling the return value ?
The return value can be ignored when vbus is always on. So is it enough just print message in the rockchip_xhci_set_vbus function?
See above, why is VBUS always on
ret = rockchip_xhci_core_init(ctx, dev); if (ret) {@@ -168,6 +188,7 @@ static int xhci_usb_probe(struct udevice *dev) static int xhci_usb_remove(struct udevice *dev) {
- struct rockchip_xhci_platdata *plat = dev_get_platdata(dev); struct rockchip_xhci *ctx = dev_get_priv(dev); int ret; @@ -178,6 +199,8 @@ static int xhci_usb_remove(struct udevice *dev) if (ret) return ret;
- rockchip_xhci_set_vbus(plat, false);
Handle return value
return 0;}
On 2017/6/9 16:22, Marek Vasut wrote:
On 06/09/2017 09:49 AM, rock-chips(daniel.meng) wrote:
On 2017/6/8 21:17, Marek Vasut wrote:
On 06/08/2017 09:31 AM, Meng Dongyang wrote:
Use fixed regulator to control the voltage of vbus and turn off vbus when usb stop.
Signed-off-by: Meng Dongyang daniel.meng@rock-chips.com
Changes in v4:
- Splited from patch [Uboot,v3,04/10]
- Define set vbus as empty function if the macros aren't set
Changes in v3: None Changes in v2:
Use fixed regulator to control vbus instead of gpio
drivers/usb/host/xhci-rockchip.c | 41
+++++++++++++++++++++++++++++++--------- 1 file changed, 32 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/host/xhci-rockchip.c b/drivers/usb/host/xhci-rockchip.c index f559830..dc9cd56 100644 --- a/drivers/usb/host/xhci-rockchip.c +++ b/drivers/usb/host/xhci-rockchip.c @@ -11,10 +11,10 @@ #include <malloc.h> #include <usb.h> #include <watchdog.h> -#include <asm/gpio.h> #include <linux/errno.h> #include <linux/compat.h> #include <linux/usb/dwc3.h> +#include <power/regulator.h> #include "xhci.h" @@ -23,7 +23,7 @@ DECLARE_GLOBAL_DATA_PTR; struct rockchip_xhci_platdata { fdt_addr_t hcd_base; fdt_addr_t phy_base;
- struct gpio_desc vbus_gpio;
- struct udevice *vbus_supply; }; /*
@@ -66,15 +66,37 @@ static int xhci_usb_ofdata_to_platdata(struct udevice *dev) return -ENXIO; }
- /* Vbus gpio */
- ret = gpio_request_by_name(dev, "rockchip,vbus-gpio", 0,
&plat->vbus_gpio, GPIOD_IS_OUT);+#if defined(CONFIG_DM_USB) && defined(CONFIG_DM_REGULATOR)
- /* Vbus regulator */
- ret = device_get_supply_regulator(dev, "vbus-supply",
&plat->vbus_supply); if (ret)
debug("rockchip,vbus-gpio node missing!");
debug("Can't get vbus supply\n");VBUS in caps
change the print message? debug("Can't get VBUS regulator\n") ?
+#endif return 0; } +#if defined(CONFIG_DM_USB) && defined(CONFIG_DM_REGULATOR) +static int rockchip_xhci_set_vbus(struct rockchip_xhci_platdata *plat,
bool value)+{
- int ret = 0;
You don't need to init ret, it's always set right below.
OK, I will correct
- ret = regulator_set_enable(plat->vbus_supply, value);
- if (ret)
debug("XHCI: Failed to set vbus supply\n");That shouldn't be debug, that's a printf() because it's actually a failure. Or error() I guess ...
Considering the case vbus is always on, it is right with no vbus regulator. So maybe it's not an error actually.
Regulator failed to turn on, that's an error, right ? Why would VBUS be always on ? You can just add a GPIO-regulator into the DT and then the VBUS is no longer always on.
It's certainly right adding a GPIO-regulator into the DT. But when it's designed as a host only port, the VBUS would be fixed to on by hardware and there is no gpio for it. In this case, setting regulator will return fail, but it's also a right case. So I think it can be treated as a debug or warning message.
- return ret;
+} +#else +static int rockchip_xhci_set_vbus(struct rockchip_xhci_platdata *plat,
bool value)+{
- return 0;
+} +#endif
- /*
- rockchip_dwc3_phy_setup() - Configure USB PHY Interface of DWC3
Core * @dwc: Pointer to our controller context structure @@ -153,9 +175,7 @@ static int xhci_usb_probe(struct udevice *dev) hcor = (struct xhci_hcor *)((uint64_t)ctx->hcd + HC_LENGTH(xhci_readl(&ctx->hcd->cr_capbase)));
- /* setup the Vbus gpio here */
- if (dm_gpio_is_valid(&plat->vbus_gpio))
dm_gpio_set_value(&plat->vbus_gpio, 1);
- rockchip_xhci_set_vbus(plat, true);
What about handling the return value ?
The return value can be ignored when vbus is always on. So is it enough just print message in the rockchip_xhci_set_vbus function?
See above, why is VBUS always on
answer above
ret = rockchip_xhci_core_init(ctx, dev); if (ret) {@@ -168,6 +188,7 @@ static int xhci_usb_probe(struct udevice *dev) static int xhci_usb_remove(struct udevice *dev) {
- struct rockchip_xhci_platdata *plat = dev_get_platdata(dev); struct rockchip_xhci *ctx = dev_get_priv(dev); int ret; @@ -178,6 +199,8 @@ static int xhci_usb_remove(struct udevice *dev) if (ret) return ret;
- rockchip_xhci_set_vbus(plat, false);
Handle return value
I think print message may be is enough.
return 0;}
On 09 Jun 2017, at 16:01, rock-chips(daniel.meng) daniel.meng@rock-chips.com wrote:
On 2017/6/9 16:22, Marek Vasut wrote:
On 06/09/2017 09:49 AM, rock-chips(daniel.meng) wrote:
On 2017/6/8 21:17, Marek Vasut wrote:
On 06/08/2017 09:31 AM, Meng Dongyang wrote:
Use fixed regulator to control the voltage of vbus and turn off vbus when usb stop.
Signed-off-by: Meng Dongyang daniel.meng@rock-chips.com
Changes in v4:
- Splited from patch [Uboot,v3,04/10]
- Define set vbus as empty function if the macros aren't set
Changes in v3: None Changes in v2:
Use fixed regulator to control vbus instead of gpio
drivers/usb/host/xhci-rockchip.c | 41
+++++++++++++++++++++++++++++++--------- 1 file changed, 32 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/host/xhci-rockchip.c b/drivers/usb/host/xhci-rockchip.c index f559830..dc9cd56 100644 --- a/drivers/usb/host/xhci-rockchip.c +++ b/drivers/usb/host/xhci-rockchip.c @@ -11,10 +11,10 @@ #include <malloc.h> #include <usb.h> #include <watchdog.h> -#include <asm/gpio.h> #include <linux/errno.h> #include <linux/compat.h> #include <linux/usb/dwc3.h> +#include <power/regulator.h> #include "xhci.h" @@ -23,7 +23,7 @@ DECLARE_GLOBAL_DATA_PTR; struct rockchip_xhci_platdata { fdt_addr_t hcd_base; fdt_addr_t phy_base;
- struct gpio_desc vbus_gpio;
- struct udevice *vbus_supply; }; /*
@@ -66,15 +66,37 @@ static int xhci_usb_ofdata_to_platdata(struct udevice *dev) return -ENXIO; }
- /* Vbus gpio */
- ret = gpio_request_by_name(dev, "rockchip,vbus-gpio", 0,
&plat->vbus_gpio, GPIOD_IS_OUT);+#if defined(CONFIG_DM_USB) && defined(CONFIG_DM_REGULATOR)
- /* Vbus regulator */
- ret = device_get_supply_regulator(dev, "vbus-supply",
&plat->vbus_supply); if (ret)
debug("rockchip,vbus-gpio node missing!");
debug("Can't get vbus supply\n");VBUS in caps
change the print message? debug("Can't get VBUS regulator\n") ?
+#endif return 0; } +#if defined(CONFIG_DM_USB) && defined(CONFIG_DM_REGULATOR) +static int rockchip_xhci_set_vbus(struct rockchip_xhci_platdata *plat,
bool value)+{
- int ret = 0;
You don't need to init ret, it's always set right below.
OK, I will correct
- ret = regulator_set_enable(plat->vbus_supply, value);
- if (ret)
debug("XHCI: Failed to set vbus supply\n");That shouldn't be debug, that's a printf() because it's actually a failure. Or error() I guess ...
Considering the case vbus is always on, it is right with no vbus regulator. So maybe it's not an error actually.
Regulator failed to turn on, that's an error, right ? Why would VBUS be always on ? You can just add a GPIO-regulator into the DT and then the VBUS is no longer always on.
It's certainly right adding a GPIO-regulator into the DT. But when it's designed as a host only port, the VBUS would be fixed to on by hardware and there is no gpio for it. In this case, setting regulator will return fail, but it's also a right case. So I think it can be treated as a debug or warning message.
Not necessarily: we sometimes put a USB hub on our modules and turn it on and off via the vbus_supply when enabling/disabling the USB controller.
In fact we do just that for the RK3399-Q7. See https://patchwork.ozlabs.org/patch/769256/ https://patchwork.ozlabs.org/patch/769256/.
- return ret;
+} +#else +static int rockchip_xhci_set_vbus(struct rockchip_xhci_platdata *plat,
bool value)+{
- return 0;
+} +#endif
- /*
- rockchip_dwc3_phy_setup() - Configure USB PHY Interface of DWC3
Core
- @dwc: Pointer to our controller context structure
@@ -153,9 +175,7 @@ static int xhci_usb_probe(struct udevice *dev) hcor = (struct xhci_hcor *)((uint64_t)ctx->hcd + HC_LENGTH(xhci_readl(&ctx->hcd->cr_capbase)));
- /* setup the Vbus gpio here */
- if (dm_gpio_is_valid(&plat->vbus_gpio))
dm_gpio_set_value(&plat->vbus_gpio, 1);
- rockchip_xhci_set_vbus(plat, true);
What about handling the return value ?
The return value can be ignored when vbus is always on. So is it enough just print message in the rockchip_xhci_set_vbus function?
See above, why is VBUS always on
answer above
ret = rockchip_xhci_core_init(ctx, dev); if (ret) {@@ -168,6 +188,7 @@ static int xhci_usb_probe(struct udevice *dev) static int xhci_usb_remove(struct udevice *dev) {
- struct rockchip_xhci_platdata *plat = dev_get_platdata(dev); struct rockchip_xhci *ctx = dev_get_priv(dev); int ret; @@ -178,6 +199,8 @@ static int xhci_usb_remove(struct udevice *dev) if (ret) return ret;
- rockchip_xhci_set_vbus(plat, false);
Handle return value
I think print message may be is enough.
return 0;}
U-Boot mailing list U-Boot@lists.denx.de mailto:U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot https://lists.denx.de/listinfo/u-boot
On 06/09/2017 04:01 PM, rock-chips(daniel.meng) wrote:
On 2017/6/9 16:22, Marek Vasut wrote:
On 06/09/2017 09:49 AM, rock-chips(daniel.meng) wrote:
On 2017/6/8 21:17, Marek Vasut wrote:
On 06/08/2017 09:31 AM, Meng Dongyang wrote:
Use fixed regulator to control the voltage of vbus and turn off vbus when usb stop.
Signed-off-by: Meng Dongyang daniel.meng@rock-chips.com
Changes in v4:
- Splited from patch [Uboot,v3,04/10]
- Define set vbus as empty function if the macros aren't set
Changes in v3: None Changes in v2:
Use fixed regulator to control vbus instead of gpio
drivers/usb/host/xhci-rockchip.c | 41
+++++++++++++++++++++++++++++++--------- 1 file changed, 32 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/host/xhci-rockchip.c b/drivers/usb/host/xhci-rockchip.c index f559830..dc9cd56 100644 --- a/drivers/usb/host/xhci-rockchip.c +++ b/drivers/usb/host/xhci-rockchip.c @@ -11,10 +11,10 @@ #include <malloc.h> #include <usb.h> #include <watchdog.h> -#include <asm/gpio.h> #include <linux/errno.h> #include <linux/compat.h> #include <linux/usb/dwc3.h> +#include <power/regulator.h> #include "xhci.h" @@ -23,7 +23,7 @@ DECLARE_GLOBAL_DATA_PTR; struct rockchip_xhci_platdata { fdt_addr_t hcd_base; fdt_addr_t phy_base;
- struct gpio_desc vbus_gpio;
- struct udevice *vbus_supply; }; /*
@@ -66,15 +66,37 @@ static int xhci_usb_ofdata_to_platdata(struct udevice *dev) return -ENXIO; }
- /* Vbus gpio */
- ret = gpio_request_by_name(dev, "rockchip,vbus-gpio", 0,
&plat->vbus_gpio, GPIOD_IS_OUT);+#if defined(CONFIG_DM_USB) && defined(CONFIG_DM_REGULATOR)
- /* Vbus regulator */
- ret = device_get_supply_regulator(dev, "vbus-supply",
&plat->vbus_supply); if (ret)
debug("rockchip,vbus-gpio node missing!");
debug("Can't get vbus supply\n");VBUS in caps
change the print message? debug("Can't get VBUS regulator\n") ?
Yes
+#endif return 0; } +#if defined(CONFIG_DM_USB) && defined(CONFIG_DM_REGULATOR) +static int rockchip_xhci_set_vbus(struct rockchip_xhci_platdata *plat,
bool value)+{
- int ret = 0;
You don't need to init ret, it's always set right below.
OK, I will correct
- ret = regulator_set_enable(plat->vbus_supply, value);
- if (ret)
debug("XHCI: Failed to set vbus supply\n");That shouldn't be debug, that's a printf() because it's actually a failure. Or error() I guess ...
Considering the case vbus is always on, it is right with no vbus regulator. So maybe it's not an error actually.
Regulator failed to turn on, that's an error, right ? Why would VBUS be always on ? You can just add a GPIO-regulator into the DT and then the VBUS is no longer always on.
It's certainly right adding a GPIO-regulator into the DT. But when it's designed as a host only port, the VBUS would be fixed to on by hardware and there is no gpio for it.
Ugh, what ? You can certainly have a host port which does support switching VBUS on/off. In fact, that's the majority case. You can just toggle VBUS and reenumerate the whole bus, that's normal and majority of the USB controller driver(s) do it.
In this case, setting regulator will return fail, but it's also a right case.
No, if something fails, it's definitely not right. If you use dummy regulator on your hardware because your hardware doesn't support VBUS switching (because it's sub-par), that's fine, but setting the regulator on/off must not fail in that case.
So I think it can be treated as a debug or warning message.
No, this is an error.
- return ret;
+} +#else +static int rockchip_xhci_set_vbus(struct rockchip_xhci_platdata *plat,
bool value)+{
- return 0;
+} +#endif
- /*
- rockchip_dwc3_phy_setup() - Configure USB PHY Interface of DWC3
Core * @dwc: Pointer to our controller context structure @@ -153,9 +175,7 @@ static int xhci_usb_probe(struct udevice *dev) hcor = (struct xhci_hcor *)((uint64_t)ctx->hcd + HC_LENGTH(xhci_readl(&ctx->hcd->cr_capbase)));
- /* setup the Vbus gpio here */
- if (dm_gpio_is_valid(&plat->vbus_gpio))
dm_gpio_set_value(&plat->vbus_gpio, 1);
- rockchip_xhci_set_vbus(plat, true);
What about handling the return value ?
The return value can be ignored when vbus is always on. So is it enough just print message in the rockchip_xhci_set_vbus function?
See above, why is VBUS always on
answer above
ret = rockchip_xhci_core_init(ctx, dev); if (ret) {@@ -168,6 +188,7 @@ static int xhci_usb_probe(struct udevice *dev) static int xhci_usb_remove(struct udevice *dev) {
- struct rockchip_xhci_platdata *plat = dev_get_platdata(dev); struct rockchip_xhci *ctx = dev_get_priv(dev); int ret; @@ -178,6 +199,8 @@ static int xhci_usb_remove(struct udevice *dev) if (ret) return ret;
- rockchip_xhci_set_vbus(plat, false);
Handle return value
I think print message may be is enough.
No, printing a message and ignoring error is NOT enough, the error must be propagated.
return 0;}
On 06/08/2017 09:31 AM, Meng Dongyang wrote:
Add support of usb host and gadget function for rk3328.
Why did I receive half of the series as a series (that is, cover letter and patches IRT to that) and half as separate emails ?
Changes in v4:
- Splite patch [U-boot,v3,04/10] into two patches
- Define set vbus as empty function if the macros aren't set
- Prepare a mask and set capabilities in GUSBCFG register once
Changes in v3:
- Revert change of macro definition in dwc2 driver
- Support host mode without HNP/SRP capability through DTS
Changes in v2:
- Add commit messages
- Split patch [U-boot,7/8] into two patches
- Use fixed regulator to control vbus instead of gpio
Meng Dongyang (11): configs: rk3328: add support for usb and config ehci and ohci driver rockchip: dts: rk3328: add ehci and ohci node and enable host0 port configs: rk3328: config xhci controller usb: host: xhci-rockchip: use fixed regulator to control vbus usb: host: xhci-rockchip: add support for rk3328 rockchip: dts: rk3328: support and enable xhci configs: rk3328: enable dwc2 driver and config for fastboot usb: dwc2: force to host mode if not support HNP/SRP rk3328: board: add support of dwc2 gadget rockchip: dts: rk3328: support and enable dwc2 rockchip: dts: rk3399: control vbus of typec by fixed regulator
arch/arm/dts/rk3328-evb.dts | 36 ++++++++++++++++++++++++++++ arch/arm/dts/rk3328.dtsi | 35 +++++++++++++++++++++++++++ arch/arm/dts/rk3399-evb.dts | 16 +++++++++++-- board/rockchip/evb_rk3328/evb-rk3328.c | 43 +++++++++++++++++++++++++++++++--- configs/evb-rk3328_defconfig | 25 ++++++++++++++++++++ drivers/usb/host/dwc2.c | 19 ++++++++++++--- drivers/usb/host/xhci-rockchip.c | 43 +++++++++++++++++++++++++++------- include/configs/rk3328_common.h | 9 +++++++ 8 files changed, 209 insertions(+), 17 deletions(-)
On 2017/6/8 20:11, Marek Vasut wrote:
On 06/08/2017 09:31 AM, Meng Dongyang wrote:
Add support of usb host and gadget function for rk3328.
Why did I receive half of the series as a series (that is, cover letter and patches IRT to that) and half as separate emails ?
Because there is a limit to the number of letter one time.
Changes in v4:
- Splite patch [U-boot,v3,04/10] into two patches
- Define set vbus as empty function if the macros aren't set
- Prepare a mask and set capabilities in GUSBCFG register once
Changes in v3:
- Revert change of macro definition in dwc2 driver
- Support host mode without HNP/SRP capability through DTS
Changes in v2:
- Add commit messages
- Split patch [U-boot,7/8] into two patches
- Use fixed regulator to control vbus instead of gpio
Meng Dongyang (11): configs: rk3328: add support for usb and config ehci and ohci driver rockchip: dts: rk3328: add ehci and ohci node and enable host0 port configs: rk3328: config xhci controller usb: host: xhci-rockchip: use fixed regulator to control vbus usb: host: xhci-rockchip: add support for rk3328 rockchip: dts: rk3328: support and enable xhci configs: rk3328: enable dwc2 driver and config for fastboot usb: dwc2: force to host mode if not support HNP/SRP rk3328: board: add support of dwc2 gadget rockchip: dts: rk3328: support and enable dwc2 rockchip: dts: rk3399: control vbus of typec by fixed regulator
arch/arm/dts/rk3328-evb.dts | 36 ++++++++++++++++++++++++++++ arch/arm/dts/rk3328.dtsi | 35 +++++++++++++++++++++++++++ arch/arm/dts/rk3399-evb.dts | 16 +++++++++++-- board/rockchip/evb_rk3328/evb-rk3328.c | 43 +++++++++++++++++++++++++++++++--- configs/evb-rk3328_defconfig | 25 ++++++++++++++++++++ drivers/usb/host/dwc2.c | 19 ++++++++++++--- drivers/usb/host/xhci-rockchip.c | 43 +++++++++++++++++++++++++++------- include/configs/rk3328_common.h | 9 +++++++ 8 files changed, 209 insertions(+), 17 deletions(-)
On 06/09/2017 05:49 AM, rock-chips(daniel.meng) wrote:
On 2017/6/8 20:11, Marek Vasut wrote:
On 06/08/2017 09:31 AM, Meng Dongyang wrote:
Add support of usb host and gadget function for rk3328.
Why did I receive half of the series as a series (that is, cover letter and patches IRT to that) and half as separate emails ?
Because there is a limit to the number of letter one time.
You can use gmail to work that around if your corporate email is crap. You can also use foo.bar+rockchips@gmail.com with foo.bar@gmail.com to differentiate traffic.
Changes in v4:
- Splite patch [U-boot,v3,04/10] into two patches
- Define set vbus as empty function if the macros aren't set
- Prepare a mask and set capabilities in GUSBCFG register once
Changes in v3:
- Revert change of macro definition in dwc2 driver
- Support host mode without HNP/SRP capability through DTS
Changes in v2:
- Add commit messages
- Split patch [U-boot,7/8] into two patches
- Use fixed regulator to control vbus instead of gpio
Meng Dongyang (11): configs: rk3328: add support for usb and config ehci and ohci driver rockchip: dts: rk3328: add ehci and ohci node and enable host0 port configs: rk3328: config xhci controller usb: host: xhci-rockchip: use fixed regulator to control vbus usb: host: xhci-rockchip: add support for rk3328 rockchip: dts: rk3328: support and enable xhci configs: rk3328: enable dwc2 driver and config for fastboot usb: dwc2: force to host mode if not support HNP/SRP rk3328: board: add support of dwc2 gadget rockchip: dts: rk3328: support and enable dwc2 rockchip: dts: rk3399: control vbus of typec by fixed regulator
arch/arm/dts/rk3328-evb.dts | 36 ++++++++++++++++++++++++++++ arch/arm/dts/rk3328.dtsi | 35 +++++++++++++++++++++++++++ arch/arm/dts/rk3399-evb.dts | 16 +++++++++++-- board/rockchip/evb_rk3328/evb-rk3328.c | 43 +++++++++++++++++++++++++++++++--- configs/evb-rk3328_defconfig | 25 ++++++++++++++++++++ drivers/usb/host/dwc2.c | 19 ++++++++++++--- drivers/usb/host/xhci-rockchip.c | 43 +++++++++++++++++++++++++++------- include/configs/rk3328_common.h | 9 +++++++ 8 files changed, 209 insertions(+), 17 deletions(-)
participants (4)
-
Dr. Philipp Tomsich -
Marek Vasut -
Meng Dongyang -
rock-chips(daniel.meng)