[PATCH] ARM: dts: imx51-babbage: Fix ULPI PHY reset modelling

[PATCH] ARM: dts: imx51-babbage: Fix ULPI PHY reset modelling

[Fabio Estevam]

GPIO2_5 is the reset GPIO for the USB3317 ULPI PHY. Instead of modelling it as
a regulator, the correct approach is to use the 'reset_gpios' property of the
"usb-nop-xceiv" node.

GPIO1_7 is the reset GPIO for the USB2517 USB hub. As we currently don't have
dt bindings to describe a HUB reset, let's keep using the regulator approach.

Rename the regulator to 'reg_hub_reset' to better describe its function and bind
it with the USB host1 port instead.

USB host support has been introduced by commit 9bf206a9d13be3 ("ARM: dts:
imx51-babbage: Add USB Host1 support"), which landed in 3.16 and it seems that
USB has only been functional due to previous bootloader initialization.

With this patch applied we can get USB host to work without relying on the 
bootloader.

Cc: <stable@vger.kernel.org> # 3.16+
Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
 arch/arm/boot/dts/imx51-babbage.dts | 22 +++++-----------------
 1 file changed, 5 insertions(+), 17 deletions(-)

diff --git a/arch/arm/boot/dts/imx51-babbage.dts b/arch/arm/boot/dts/imx51-babbage.dts
index 56569ce..649befe 100644
--- a/arch/arm/boot/dts/imx51-babbage.dts
+++ b/arch/arm/boot/dts/imx51-babbage.dts
@@ -127,24 +127,12 @@
 		#address-cells = <1>;
 		#size-cells = <0>;
 
-		reg_usbh1_vbus: regulator at 0 {
-			compatible = "regulator-fixed";
-			pinctrl-names = "default";
-			pinctrl-0 = <&pinctrl_usbh1reg>;
-			reg = <0>;
-			regulator-name = "usbh1_vbus";
-			regulator-min-microvolt = <5000000>;
-			regulator-max-microvolt = <5000000>;
-			gpio = <&gpio2 5 GPIO_ACTIVE_HIGH>;
-			enable-active-high;
-		};
-
-		reg_usbotg_vbus: regulator at 1 {
+		reg_hub_reset: regulator at 0 {
 			compatible = "regulator-fixed";
 			pinctrl-names = "default";
 			pinctrl-0 = <&pinctrl_usbotgreg>;
-			reg = <1>;
-			regulator-name = "usbotg_vbus";
+			reg = <0>;
+			regulator-name = "hub_reset";
 			regulator-min-microvolt = <5000000>;
 			regulator-max-microvolt = <5000000>;
 			gpio = <&gpio1 7 GPIO_ACTIVE_HIGH>;
@@ -176,6 +164,7 @@
 			reg = <0>;
 			clocks = <&clks IMX5_CLK_DUMMY>;
 			clock-names = "main_clk";
+			reset-gpios = <&gpio2 5 GPIO_ACTIVE_LOW>;
 		};
 	};
 };
@@ -419,7 +408,7 @@
 &usbh1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_usbh1>;
-	vbus-supply = <&reg_usbh1_vbus>;
+	vbus-supply = <&reg_hub_reset>;
 	fsl,usbphy = <&usbh1phy>;
 	phy_type = "ulpi";
 	status = "okay";
@@ -429,7 +418,6 @@
 	dr_mode = "otg";
 	disable-over-current;
 	phy_type = "utmi_wide";
-	vbus-supply = <&reg_usbotg_vbus>;
 	status = "okay";
 };
 
-- 
1.9.1

[PATCH] ARM: dts: imx51-babbage: Fix ULPI PHY reset modelling

[Shawn Guo]

On Mon, Jan 05, 2015 at 03:19:08PM -0200, Fabio Estevam wrote:
> GPIO2_5 is the reset GPIO for the USB3317 ULPI PHY. Instead of modelling it as
> a regulator, the correct approach is to use the 'reset_gpios' property of the
> "usb-nop-xceiv" node.
> 
> GPIO1_7 is the reset GPIO for the USB2517 USB hub. As we currently don't have
> dt bindings to describe a HUB reset, let's keep using the regulator approach.
> 
> Rename the regulator to 'reg_hub_reset' to better describe its function and bind
> it with the USB host1 port instead.
> 
> USB host support has been introduced by commit 9bf206a9d13be3 ("ARM: dts:
> imx51-babbage: Add USB Host1 support"), which landed in 3.16 and it seems that
> USB has only been functional due to previous bootloader initialization.
> 
> With this patch applied we can get USB host to work without relying on the 
> bootloader.
> 
> Cc: <stable@vger.kernel.org> # 3.16+
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>

Applied, thanks.

[PATCH] ARM: dts: imx51-babbage: Fix ULPI PHY reset modelling

[Lucas Stach]

Am Dienstag, den 06.01.2015, 20:43 +0800 schrieb Shawn Guo:
> On Mon, Jan 05, 2015 at 03:19:08PM -0200, Fabio Estevam wrote:
> > GPIO2_5 is the reset GPIO for the USB3317 ULPI PHY. Instead of modelling it as
> > a regulator, the correct approach is to use the 'reset_gpios' property of the
> > "usb-nop-xceiv" node.
> > 
> > GPIO1_7 is the reset GPIO for the USB2517 USB hub. As we currently don't have
> > dt bindings to describe a HUB reset, let's keep using the regulator approach.
> > 
> > Rename the regulator to 'reg_hub_reset' to better describe its function and bind
> > it with the USB host1 port instead.
> > 
> > USB host support has been introduced by commit 9bf206a9d13be3 ("ARM: dts:
> > imx51-babbage: Add USB Host1 support"), which landed in 3.16 and it seems that
> > USB has only been functional due to previous bootloader initialization.
> > 
> > With this patch applied we can get USB host to work without relying on the 
> > bootloader.
> > 
> > Cc: <stable@vger.kernel.org> # 3.16+
> > Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> 
> Applied, thanks.
> 
For the record:
I'm not happy with this patch. It still abuses the regulator API for
something completely unrelated and this may well break again in the
future. But given that it fixes an actual regression and you already
sent a pull including this I'm okay to keep it.

We still need a proper fix to handle ULPI resets to get rid of this
hackery.

Regards,
Lucas

-- 
Pengutronix e.K.             | Lucas Stach                 |
Industrial Linux Solutions   | http://www.pengutronix.de/  |

[PATCH] ARM: dts: imx51-babbage: Fix ULPI PHY reset modelling

[Shawn Guo]

On Tue, Jan 06, 2015 at 02:36:05PM +0100, Lucas Stach wrote:
> Am Dienstag, den 06.01.2015, 20:43 +0800 schrieb Shawn Guo:
> > On Mon, Jan 05, 2015 at 03:19:08PM -0200, Fabio Estevam wrote:
> > > GPIO2_5 is the reset GPIO for the USB3317 ULPI PHY. Instead of modelling it as
> > > a regulator, the correct approach is to use the 'reset_gpios' property of the
> > > "usb-nop-xceiv" node.
> > > 
> > > GPIO1_7 is the reset GPIO for the USB2517 USB hub. As we currently don't have
> > > dt bindings to describe a HUB reset, let's keep using the regulator approach.
> > > 
> > > Rename the regulator to 'reg_hub_reset' to better describe its function and bind
> > > it with the USB host1 port instead.
> > > 
> > > USB host support has been introduced by commit 9bf206a9d13be3 ("ARM: dts:
> > > imx51-babbage: Add USB Host1 support"), which landed in 3.16 and it seems that
> > > USB has only been functional due to previous bootloader initialization.
> > > 
> > > With this patch applied we can get USB host to work without relying on the 
> > > bootloader.
> > > 
> > > Cc: <stable@vger.kernel.org> # 3.16+
> > > Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> > 
> > Applied, thanks.
> > 
> For the record:
> I'm not happy with this patch. It still abuses the regulator API for
> something completely unrelated and this may well break again in the
> future. But given that it fixes an actual regression and you already
> sent a pull including this I'm okay to keep it.
> 
> We still need a proper fix to handle ULPI resets to get rid of this
> hackery.

Yeah, at least this fix patch removes one user of the regulator hackery.

Shawn

[PATCH] ARM: dts: imx51-babbage: Fix ULPI PHY reset modelling

[Fabio Estevam]

On Tue, Jan 6, 2015 at 11:36 AM, Lucas Stach <l.stach@pengutronix.de> wrote:

> For the record:
> I'm not happy with this patch. It still abuses the regulator API for
> something completely unrelated and this may well break again in the
> future. But given that it fixes an actual regression and you already
> sent a pull including this I'm okay to keep it.
>
> We still need a proper fix to handle ULPI resets to get rid of this
> hackery.

Actually the ULPI PHY reset is properly handled now. What is missing
is the USB Hub reset to be properly handled.

According to the USB maintainer, Alan Stern, a proper solution has not
come up yet:
http://www.spinics.net/lists/linux-usb/msg97483.html

When a proper USB hub reset solution shows up, we can certainly use
it, but for now let's have USB host functional by using the regulator
API hack.

[PATCH] ARM: dts: imx51-babbage: Fix ULPI PHY reset modelling

[Lucas Stach]

Am Dienstag, den 06.01.2015, 11:47 -0200 schrieb Fabio Estevam:
> On Tue, Jan 6, 2015 at 11:36 AM, Lucas Stach <l.stach@pengutronix.de> wrote:
> 
> > For the record:
> > I'm not happy with this patch. It still abuses the regulator API for
> > something completely unrelated and this may well break again in the
> > future. But given that it fixes an actual regression and you already
> > sent a pull including this I'm okay to keep it.
> >
> > We still need a proper fix to handle ULPI resets to get rid of this
> > hackery.
> 
> Actually the ULPI PHY reset is properly handled now. What is missing
> is the USB Hub reset to be properly handled.
> 
No it is not. A regulator isn't the proper abstraction for a reset at
all. Even if you (ab)use the regulator drivers ability to toggle a reset
GPIO.
A ULPI PHY reset needs to be modeled as a GPIO reset attached to USB
controller node. There is no other way to describe the hardware
accurately.

Regards,
Lucas

-- 
Pengutronix e.K.             | Lucas Stach                 |
Industrial Linux Solutions   | http://www.pengutronix.de/  |

[PATCH] ARM: dts: imx51-babbage: Fix ULPI PHY reset modelling

[Fabio Estevam]

On Tue, Jan 6, 2015 at 11:54 AM, Lucas Stach <l.stach@pengutronix.de> wrote:

>> Actually the ULPI PHY reset is properly handled now. What is missing
>> is the USB Hub reset to be properly handled.
>>
> No it is not. A regulator isn't the proper abstraction for a reset at
> all. Even if you (ab)use the regulator drivers ability to toggle a reset
> GPIO.
> A ULPI PHY reset needs to be modeled as a GPIO reset attached to USB
> controller node. There is no other way to describe the hardware
> accurately.

The ULPI PHY is modelled as a GPIO reset via reset-gpios property.
This is documented at
Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt:

- reset-gpios: Should specify the GPIO for reset.

This is the property we use right now for resetting the ULPI PHY.

[PATCH] ARM: dts: imx51-babbage: Fix ULPI PHY reset modelling

[Lucas Stach]

Am Dienstag, den 06.01.2015, 11:59 -0200 schrieb Fabio Estevam:
> On Tue, Jan 6, 2015 at 11:54 AM, Lucas Stach <l.stach@pengutronix.de> wrote:
> 
> >> Actually the ULPI PHY reset is properly handled now. What is missing
> >> is the USB Hub reset to be properly handled.
> >>
> > No it is not. A regulator isn't the proper abstraction for a reset at
> > all. Even if you (ab)use the regulator drivers ability to toggle a reset
> > GPIO.
> > A ULPI PHY reset needs to be modeled as a GPIO reset attached to USB
> > controller node. There is no other way to describe the hardware
> > accurately.
> 
> The ULPI PHY is modelled as a GPIO reset via reset-gpios property.
> This is documented at
> Documentation/devicetree/bindings/usb/usb-nop-xceiv.txt:
> 
> - reset-gpios: Should specify the GPIO for reset.
> 
> This is the property we use right now for resetting the ULPI PHY.

Right. I misread that part of the patch. Sorry about that.

Regards,
Lucas 

-- 
Pengutronix e.K.             | Lucas Stach                 |
Industrial Linux Solutions   | http://www.pengutronix.de/  |