[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 = <®_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