From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kostya Porotchkin Date: Tue, 24 Jan 2017 14:57:44 +0000 Subject: [U-Boot] [EXT] Re: [PATCH v2 4/7] mvebu: usb: xhci: Add support for VBUS controlled by GPIO In-Reply-To: <0eae9dc5-5837-fcbe-0f31-7a9127830706@denx.de> References: <1483887132-21169-1-git-send-email-kostap@marvell.com> <1483887132-21169-5-git-send-email-kostap@marvell.com> <3170f8b3-60cf-868c-fde3-9b8e197d75d7@denx.de>, <0eae9dc5-5837-fcbe-0f31-7a9127830706@denx.de> Message-ID: <1485270039019.33080@marvell.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi, Marek, ________________________________________ From: Marek Vasut 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 >> >> 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 >> Cc: Stefan Roese >> Cc: Nadav Haklai >> Cc: Neta Zur Hershkovits >> Cc: Omri Itach >> Cc: Igal Liberman >> Cc: Haim Boot >> Cc: Hanna Hawa >> --- >> 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 = ; >> + 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