Discussion:
[PATCH 0/2] ARM: dts: sabrelite/nitrogen6x: fix USB PHY reset
Gary Bisson
2017-07-24 08:50:03 UTC
Permalink
Hi all,

This series is follow-up to those two patches:
https://patchwork.kernel.org/patch/9395531/
https://patchwork.kernel.org/patch/9395539/

At the time they were refused because a pwrseq library was supposed to
get in. Unfortunately that plan hasn't succeeded yet so re-submitting
the original two patches with the following modifications:
- Rebase on top of v4.13-rc2
- Reword the title to make it clear this is a fix
- Reword the changelog so it is clearly a temporary workaround

Let me know if you have any question.

Regards,
Gary

Gary Bisson (2):
ARM: dts: imx6qdl-sabrelite: fix USB PHY reset
ARM: dts: imx6qdl-nitrogen6x: fix USB PHY reset

arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi | 20 ++++++++++++++++++++
arch/arm/boot/dts/imx6qdl-sabrelite.dtsi | 20 ++++++++++++++++++++
2 files changed, 40 insertions(+)
--
2.11.0
Gary Bisson
2017-07-24 08:50:04 UTC
Permalink
Declared as a regulator since the driver doesn't have a reset-gpios
property for this.

This ensures that the PHY is woken up, not depending on the state the
second stage bootloader leaves the pin.

This is a workaround until a proper mechanism is provided to reset such
devices like the pwrseq library [1] for instance.

[1] https://lkml.org/lkml/2017/2/10/779

Signed-off-by: Gary Bisson <***@boundarydevices.com>
---
arch/arm/boot/dts/imx6qdl-sabrelite.dtsi | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)

diff --git a/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi b/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi
index afe7449c47da..7bd14e23afb8 100644
--- a/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi
@@ -123,6 +123,19 @@
regulator-max-microvolt = <2800000>;
regulator-always-on;
};
+
+ reg_usb_h1_vbus: ***@7 {
+ compatible = "regulator-fixed";
+ reg = <7>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_usbh1>;
+ regulator-name = "usb_h1_vbus";
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ gpio = <&gpio7 12 GPIO_ACTIVE_HIGH>;
+ enable-active-high;
+ regulator-always-on;
+ };
};

mipi_xclk: mipi_xclk {
@@ -610,6 +623,12 @@
;
};

+ pinctrl_usbh1: usbh1grp {
+ fsl,pins = <
+ MX6QDL_PAD_GPIO_17__GPIO7_IO12 0x030b0
+ >;
+ };
+
pinctrl_usbotg: usbotggrp {
fsl,pins = <
MX6QDL_PAD_GPIO_1__USB_OTG_ID 0x17059
@@ -705,6 +724,7 @@
};

&usbh1 {
+ vbus-supply = <&reg_usb_h1_vbus>;
status = "okay";
};
--
2.11.0
Fabio Estevam
2017-07-24 12:49:56 UTC
Permalink
Hi Gary,

On Mon, Jul 24, 2017 at 5:50 AM, Gary Bisson
+
+ compatible = "regulator-fixed";
+ reg = <7>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_usbh1>;
+ regulator-name = "usb_h1_vbus";
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ gpio = <&gpio7 12 GPIO_ACTIVE_HIGH>;
+ enable-active-high;
+ regulator-always-on;
As this regulator is controlled via GPIO I believe you should remove
the ' regulator-always-on' property.

Same applies to 2/2/
Gary Bisson
2017-07-24 12:52:57 UTC
Permalink
Hi Fabio,
Post by Fabio Estevam
Hi Gary,
On Mon, Jul 24, 2017 at 5:50 AM, Gary Bisson
+
+ compatible = "regulator-fixed";
+ reg = <7>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_usbh1>;
+ regulator-name = "usb_h1_vbus";
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ gpio = <&gpio7 12 GPIO_ACTIVE_HIGH>;
+ enable-active-high;
+ regulator-always-on;
As this regulator is controlled via GPIO I believe you should remove
the ' regulator-always-on' property.
Same applies to 2/2/
Good catch, I'll send a v2 right away.

Thanks,
Gary
Gary Bisson
2017-07-24 08:50:05 UTC
Permalink
Declared as a regulator since the driver doesn't have a reset-gpios
property for this.

This ensures that the PHY is woken up, not depending on the state the
second stage bootloader leaves the pin.

This is a workaround until a proper mechanism is provided to reset such
devices like the pwrseq library [1] for instance.

[1] https://lkml.org/lkml/2017/2/10/779

Signed-off-by: Gary Bisson <***@boundarydevices.com>
---
arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)

diff --git a/arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi b/arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi
index f22e5879340b..989376643cf6 100644
--- a/arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi
@@ -108,6 +108,19 @@
startup-delay-us = <70000>;
enable-active-high;
};
+
+ reg_usb_h1_vbus: ***@5 {
+ compatible = "regulator-fixed";
+ reg = <5>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_usbh1>;
+ regulator-name = "usb_h1_vbus";
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ gpio = <&gpio7 12 GPIO_ACTIVE_HIGH>;
+ enable-active-high;
+ regulator-always-on;
+ };
};

gpio-keys {
@@ -515,6 +528,12 @@
;
};

+ pinctrl_usbh1: usbh1grp {
+ fsl,pins = <
+ MX6QDL_PAD_GPIO_17__GPIO7_IO12 0x030b0
+ >;
+ };
+
pinctrl_usbotg: usbotggrp {
fsl,pins = <
MX6QDL_PAD_GPIO_1__USB_OTG_ID 0x17059
@@ -629,6 +648,7 @@
};

&usbh1 {
+ vbus-supply = <&reg_usb_h1_vbus>;
status = "okay";
};
--
2.11.0
Gary Bisson
2017-07-24 12:59:57 UTC
Permalink
Declared as a regulator since the driver doesn't have a reset-gpios
property for this.

This ensures that the PHY is woken up, not depending on the state the
second stage bootloader leaves the pin.

This is a workaround until a proper mechanism is provided to reset such
devices like the pwrseq library [1] for instance.

[1] https://lkml.org/lkml/2017/2/10/779

Signed-off-by: Gary Bisson <***@boundarydevices.com>
---
arch/arm/boot/dts/imx6qdl-sabrelite.dtsi | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)

diff --git a/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi b/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi
index afe7449c47da..756c5054f047 100644
--- a/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-sabrelite.dtsi
@@ -123,6 +123,18 @@
regulator-max-microvolt = <2800000>;
regulator-always-on;
};
+
+ reg_usb_h1_vbus: ***@7 {
+ compatible = "regulator-fixed";
+ reg = <7>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_usbh1>;
+ regulator-name = "usb_h1_vbus";
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ gpio = <&gpio7 12 GPIO_ACTIVE_HIGH>;
+ enable-active-high;
+ };
};

mipi_xclk: mipi_xclk {
@@ -610,6 +622,12 @@
;
};

+ pinctrl_usbh1: usbh1grp {
+ fsl,pins = <
+ MX6QDL_PAD_GPIO_17__GPIO7_IO12 0x030b0
+ >;
+ };
+
pinctrl_usbotg: usbotggrp {
fsl,pins = <
MX6QDL_PAD_GPIO_1__USB_OTG_ID 0x17059
@@ -705,6 +723,7 @@
};

&usbh1 {
+ vbus-supply = <&reg_usb_h1_vbus>;
status = "okay";
};
--
2.11.0
Fabio Estevam
2017-07-24 13:03:14 UTC
Permalink
On Mon, Jul 24, 2017 at 9:59 AM, Gary Bisson
Post by Gary Bisson
Declared as a regulator since the driver doesn't have a reset-gpios
property for this.
This ensures that the PHY is woken up, not depending on the state the
second stage bootloader leaves the pin.
This is a workaround until a proper mechanism is provided to reset such
devices like the pwrseq library [1] for instance.
[1] https://lkml.org/lkml/2017/2/10/779
Reviewed-by: Fabio Estevam <***@nxp.com>
Gary Bisson
2017-07-24 12:59:58 UTC
Permalink
Declared as a regulator since the driver doesn't have a reset-gpios
property for this.

This ensures that the PHY is woken up, not depending on the state the
second stage bootloader leaves the pin.

This is a workaround until a proper mechanism is provided to reset such
devices like the pwrseq library [1] for instance.

[1] https://lkml.org/lkml/2017/2/10/779

Signed-off-by: Gary Bisson <***@boundarydevices.com>
---
arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)

diff --git a/arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi b/arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi
index f22e5879340b..d309a4d0eb08 100644
--- a/arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi
@@ -108,6 +108,18 @@
startup-delay-us = <70000>;
enable-active-high;
};
+
+ reg_usb_h1_vbus: ***@5 {
+ compatible = "regulator-fixed";
+ reg = <5>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_usbh1>;
+ regulator-name = "usb_h1_vbus";
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ gpio = <&gpio7 12 GPIO_ACTIVE_HIGH>;
+ enable-active-high;
+ };
};

gpio-keys {
@@ -515,6 +527,12 @@
;
};

+ pinctrl_usbh1: usbh1grp {
+ fsl,pins = <
+ MX6QDL_PAD_GPIO_17__GPIO7_IO12 0x030b0
+ >;
+ };
+
pinctrl_usbotg: usbotggrp {
fsl,pins = <
MX6QDL_PAD_GPIO_1__USB_OTG_ID 0x17059
@@ -629,6 +647,7 @@
};

&usbh1 {
+ vbus-supply = <&reg_usb_h1_vbus>;
status = "okay";
};
--
2.11.0
Fabio Estevam
2017-07-24 13:04:08 UTC
Permalink
On Mon, Jul 24, 2017 at 9:59 AM, Gary Bisson
Post by Gary Bisson
Declared as a regulator since the driver doesn't have a reset-gpios
property for this.
This ensures that the PHY is woken up, not depending on the state the
second stage bootloader leaves the pin.
This is a workaround until a proper mechanism is provided to reset such
devices like the pwrseq library [1] for instance.
[1] https://lkml.org/lkml/2017/2/10/779
Reviewed-by: Fabio Estevam <***@nxp.com>
Shawn Guo
2017-07-26 02:39:54 UTC
Permalink
Post by Gary Bisson
ARM: dts: imx6qdl-sabrelite: fix USB PHY reset
ARM: dts: imx6qdl-nitrogen6x: fix USB PHY reset
Applied both, thanks.

Loading...