Hi, Marek,
________________________________________
From: Marek Vasut <ma...@denx.de>
Sent: Tuesday, January 24, 2017 16:11
To: Stefan Roese; Kostya Porotchkin; u-boot@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, kos...@marvell.com wrote:
>> From: Konstantin Porotchkin <kos...@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 <kos...@marvell.com>
>> Cc: Stefan Roese <s...@denx.de>
>> Cc: Nadav Haklai <nad...@marvell.com>
>> Cc: Neta Zur Hershkovits <n...@marvell.com>
>> Cc: Omri Itach <om...@marvell.com>
>> Cc: Igal Liberman <ig...@marvell.com>
>> Cc: Haim Boot <ha...@marvell.com>
>> Cc: Hanna Hawa <han...@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@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@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
