Re: [U-Boot] [PATCH 1/3] ARM: tegra: fix USB ULPI PHY reset signal inversion confusion

[Simon Glass]

On 15 September 2016 at 12:19, Stephen Warren  wrote:
> From: Stephen Warren 
>
> USB ULPI PHY reset signals are typically active low. Consequently, they
> should be marked as GPIO_ACTIVE_LOW in device tree, and indeed they are in
> the Linux kernel DTs, and in DT properties that U-Boot doesn't yet use.
> However, in DT properties that U-Boot does use, the value has been set to
> 0 (== GPIO_ACTIVE_HIGH) to work around a bug in U-Boot.
>
> This change fixes the DT to correctly represent the HW, and fixes the
> Tegra USB driver to cope with the fact that dm_gpio_set_value() internally
> handles any inversions implied by the DT value GPIO_ACTIVE_LOW.
>
> Cc: Marcel Ziswiler 
> Signed-off-by: Stephen Warren 
> ---
>  arch/arm/dts/tegra20-colibri.dts |  3 ++-
>  arch/arm/dts/tegra20-harmony.dts |  3 ++-
>  drivers/usb/host/ehci-tegra.c| 13 +++--
>  3 files changed, 15 insertions(+), 4 deletions(-)

Reviewed-by: Simon Glass 
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 1/3] ARM: tegra: fix USB ULPI PHY reset signal inversion confusion

[Stephen Warren]

From: Stephen Warren 

USB ULPI PHY reset signals are typically active low. Consequently, they
should be marked as GPIO_ACTIVE_LOW in device tree, and indeed they are in
the Linux kernel DTs, and in DT properties that U-Boot doesn't yet use.
However, in DT properties that U-Boot does use, the value has been set to
0 (== GPIO_ACTIVE_HIGH) to work around a bug in U-Boot.

This change fixes the DT to correctly represent the HW, and fixes the
Tegra USB driver to cope with the fact that dm_gpio_set_value() internally
handles any inversions implied by the DT value GPIO_ACTIVE_LOW.

Cc: Marcel Ziswiler 
Signed-off-by: Stephen Warren 
---
 arch/arm/dts/tegra20-colibri.dts |  3 ++-
 arch/arm/dts/tegra20-harmony.dts |  3 ++-
 drivers/usb/host/ehci-tegra.c| 13 +++--
 3 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/arch/arm/dts/tegra20-colibri.dts b/arch/arm/dts/tegra20-colibri.dts
index a291d93c7d01..777f63e5bdb6 100644
--- a/arch/arm/dts/tegra20-colibri.dts
+++ b/arch/arm/dts/tegra20-colibri.dts
@@ -39,7 +39,8 @@
usb@c5004000 {
statuc = "okay";
/* VBUS_LAN */
-   nvidia,phy-reset-gpio = < TEGRA_GPIO(V, 1) 
GPIO_ACTIVE_HIGH>;
+   nvidia,phy-reset-gpio = < TEGRA_GPIO(V, 1)
+   GPIO_ACTIVE_LOW>;
nvidia,vbus-gpio = < TEGRA_GPIO(BB, 1) GPIO_ACTIVE_HIGH>;
};
 
diff --git a/arch/arm/dts/tegra20-harmony.dts b/arch/arm/dts/tegra20-harmony.dts
index cace74339483..5aec150b5e61 100644
--- a/arch/arm/dts/tegra20-harmony.dts
+++ b/arch/arm/dts/tegra20-harmony.dts
@@ -626,7 +626,8 @@
 
usb@c5004000 {
status = "okay";
-   nvidia,phy-reset-gpio = < TEGRA_GPIO(V, 1) 0>;
+   nvidia,phy-reset-gpio = < TEGRA_GPIO(V, 1)
+   GPIO_ACTIVE_LOW>;
};
 
usb-phy@c5004000 {
diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c
index 31d54ab285bf..c10597861873 100644
--- a/drivers/usb/host/ehci-tegra.c
+++ b/drivers/usb/host/ehci-tegra.c
@@ -600,9 +600,18 @@ static int init_ulpi_usb_controller(struct fdt_usb *config,
 
/* reset ULPI phy */
if (dm_gpio_is_valid(>phy_reset_gpio)) {
-   dm_gpio_set_value(>phy_reset_gpio, 0);
-   mdelay(5);
+   /*
+* This GPIO is typically active-low, and marked as such in
+* device tree. dm_gpio_set_value() takes this into account
+* and inverts the value we pass here if required. In other
+* words, this first call logically asserts the reset signal,
+* which typically results in driving the physical GPIO low,
+* and the second call logically de-asserts the reset signal,
+* which typically results in driver the GPIO high.
+*/
dm_gpio_set_value(>phy_reset_gpio, 1);
+   mdelay(5);
+   dm_gpio_set_value(>phy_reset_gpio, 0);
}
 
/* Reset the usb controller */
-- 
2.9.3

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot