[U-Boot] [EXT] Re: [PATCH v2 4/7] mvebu: usb: xhci: Add support for VBUS controlled by GPIO

Kostya Porotchkin kostap at marvell.com
Mon Feb 6 13:26:34 UTC 2017


Hi, Marek,

I added a regulator to the board DTS and connected it to the USB port:

/{
	model = "MACCHIATOBin-8040";
	compatible = "marvell,armada8040-mcbin",
		     "marvell,armada8040";
...
...
	regulators {
		compatible = "simple-bus";
		#address-cells = <1>;
		#size-cells = <0>;

		reg_usb3h0_vbus: usb3-vbus0 {
			compatible = "regulator-fixed";
			pinctrl-names = "default";
			pinctrl-0 = <&cpm_xhci_vbus_pins>;
			regulator-name = "reg-usb3h0-vbus";
			regulator-min-microvolt = <5000000>;
			regulator-max-microvolt = <5000000>;
			startup-delay-us = <500000>;
			enable-active-high;
			regulator-always-on;
			gpio = <&cpm_gpio1 15 GPIO_ACTIVE_HIGH>; /* GPIO[47] */
		};
	};
};

&cps_usb3_0 {
	vcc-supply = <&reg_usb3h0_vbus>;
	status = "okay";
};

However the regulator is not activated when I issue "usb reset" command.
I can either call to " uclass_first_device(UCLASS_REGULATOR, &dev)" during arch early init stage,
or select the regulator with "regulator dev" command.

Only then the VBUS GPIO enables the USB power.

Is it the expected behavior or I am missing something in my configuration?

Thank you for your help
Kosta


> -----Original Message-----
> From: U-Boot [mailto:u-boot-bounces at lists.denx.de] On Behalf Of Kostya
> Porotchkin
> Sent: Tuesday, January 24, 2017 16:58
> To: Marek Vasut; Stefan Roese; u-boot at lists.denx.de
> Cc: Haim Boot; Hanna Hawa; Omri Itach; Nadav Haklai; Neta Zur
> Hershkovits; Igal Liberman
> Subject: Re: [U-Boot] [EXT] Re: [PATCH v2 4/7] mvebu: usb: xhci: Add
> support for VBUS controlled by GPIO
> 
> Hi, Marek,
> ________________________________________
> From: Marek Vasut <marex at denx.de>
> Sent: Tuesday, January 24, 2017 16:11
> To: Stefan Roese; Kostya Porotchkin; u-boot at lists.denx.de
> Cc: Haim Boot; Hanna Hawa; Omri Itach; Nadav Haklai; Neta Zur
> Hershkovits; Igal Liberman
> Subject: [EXT] Re: [U-Boot] [PATCH v2 4/7] mvebu: usb: xhci: Add support
> for VBUS controlled by GPIO
> 
> ----------------------------------------------------------------------
> On 01/24/2017 02:53 PM, Stefan Roese wrote:
> > Hi Marek,
> >
> > (adding Marek as USB custodian to Cc)
> >
> > On 08.01.2017 15:52, kostap at marvell.com wrote:
> >> From: Konstantin Porotchkin <kostap at marvell.com>
> >>
> >> Add support for "marvell,vbus-gpio" property to mvebu XHCI host
> >> adapter driver.
> >> This option is valid when CONFIG_DM_GPIO=y
> >>
> >> Change-Id: I930b3ebe001e50ae8d5abe1f3c774bcdb1739e64
> >> Signed-off-by: Konstantin Porotchkin <kostap at marvell.com>
> >> Cc: Stefan Roese <sr at denx.de>
> >> Cc: Nadav Haklai <nadavh at marvell.com>
> >> Cc: Neta Zur Hershkovits <neta at marvell.com>
> >> Cc: Omri Itach <omrii at marvell.com>
> >> Cc: Igal Liberman <igall at marvell.com>
> >> Cc: Haim Boot <hayim at marvell.com>
> >> Cc: Hanna Hawa <hannah at marvell.com>
> >> ---
> >> Changes for v2:
> >> - Move VBUS GPIO support from board-specific function to mvebu XHCI
> >> driver
> >> - Increase delay after VBUS GPIO activation
> >>
> >>  doc/device-tree-bindings/usb/marvell.xhci-usb.txt | 29
> >> +++++++++++++++++++++++
> >>  drivers/usb/host/xhci-mvebu.c                     | 14 +++++++++++
> >>  2 files changed, 43 insertions(+)
> >>  create mode 100644 doc/device-tree-bindings/usb/marvell.xhci-usb.txt
> >>
> >> diff --git a/doc/device-tree-bindings/usb/marvell.xhci-usb.txt
> >> b/doc/device-tree-bindings/usb/marvell.xhci-usb.txt
> >> new file mode 100644
> >> index 0000000..b0a53ad
> >> --- /dev/null
> >> +++ b/doc/device-tree-bindings/usb/marvell.xhci-usb.txt
> >> @@ -0,0 +1,29 @@
> >> +Marvell SOC USB controllers
> >> +
> >> +This controller is integrated in Armada 3700/8K.
> >> +It uses the same properties as a generic XHCI host controller
> >> +
> >> +Required properties :
> >> + - compatible: should be one or more of:
> >> +   - "marvell,armada3700-xhci", "generic-xhci" for Armada 37xx SoCs
> >> +   - "marvell,armada-8k-xhci", "generic-xhci" for Armada A8K SoCs
> >> + - reg: should contain address and length of the standard XHCI
> >> +    register set for the device.
> >> + - interrupts: one XHCI interrupt should be described here.
> >> +
> >> +Optional properties:
> >> + - clocks: reference to a clock
> 
> >Reference clock are optional ?
> Actually the basis for this file was taken from kernel documentation and
> clocks are listed as "optional" here.
> 
> >> + - marvell,vbus-gpio : If present, specifies a gpio that needs to be
> >> +   activated for the bus to be powered.
> 
> >Shouldn't this be a regulator instead ?
> 
> If it is required, I can implement the regulator method.
> However as you mentioned the regulator core has to be extended for the
> ramp-up time support.
> This is kind of a shortcut, that enables immediate community board
> support.
> The same GPIO VBUS implementation exists in Marvell SDK.
> 
> >> +Example:
> >> +    cpm_usb3_0: usb3 at 500000 {
> >> +        compatible = "marvell,armada-8k-xhci",
> >> +                 "generic-xhci";
> >> +        reg = <0x500000 0x4000>;
> >> +        interrupts = <GIC_SPI 62 IRQ_TYPE_LEVEL_HIGH>;
> >> +        clocks = <&cpm_syscon0 1 22>;
> >> +        marvell,vbus-gpio = <&cpm_gpio1 15 GPIO_ACTIVE_HIGH>;
> >> +        status = "disabled";
> >> +    };
> >> +
> >> diff --git a/drivers/usb/host/xhci-mvebu.c
> >> b/drivers/usb/host/xhci-mvebu.c index 46eb937..64801e7 100644
> >> --- a/drivers/usb/host/xhci-mvebu.c
> >> +++ b/drivers/usb/host/xhci-mvebu.c
> >> @@ -45,6 +45,20 @@ static int xhci_usb_probe(struct udevice *dev)
> >>      struct mvebu_xhci *ctx = dev_get_priv(dev);
> >>      struct xhci_hcor *hcor;
> >>      int len;
> >> +#ifdef CONFIG_DM_GPIO
> >> +    struct gpio_desc vbus_gpio;
> >> +
> >> +    gpio_request_by_name(dev, "marvell,vbus-gpio", 0, &vbus_gpio,
> >> +                 GPIOD_IS_OUT);
> >> +
> >> +    if (dm_gpio_is_valid(&vbus_gpio)) {
> >> +        dm_gpio_set_value(&vbus_gpio, 1);
> >> +        /* Wait for the GPIO VBUS output set */
> >> +        mdelay(500);
> 
> >I think if the GPIO was instead a regulator-fixed, the regulator could
> >handle the delay with something like "ramp-up" delay. But I don't think
> >we do support the ramp-up delay yet ... maybe this can be easily added?
> 
> So I assume you suggest to drop this one and implement the VBUS through
> regulator GPIO?
> 
> >> +    }
> >> +#else
> >> +    debug("USB VBUS on GPIO support is missing\n"); #endif /*
> >> +CONFIG_DM_GPIO */
> >>
> >>      ctx->hcd = (struct xhci_hccr *)plat->hcd_base;
> >>      len = HC_LENGTH(xhci_readl(&ctx->hcd->cr_capbase));
> >>
> >
> > Marek, do you have some comments on this USB related patch? If not,
> > are you okay with me pushing it via the Marvell repository?
> 
> >I have one .
> 
> Thank you for your review!
> 
> Best Regards
> Konstantin Porotchkin
> 
> 
> --
> Best regards,
> Marek Vasut
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot


More information about the U-Boot mailing list