Skip to contentSkip to conversation

ARM: sunxi: Add support for USB OTG PHY

PATCH Superseded Roman Byshko 4 participants 14 messages Oct 27, 2014 – Oct 30, 2014

Subsystems (3)
Device tree bindings Arm/Allwinner sunXi SoC support Generic PHY framework

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Roman Byshko

author
#0 lore raw

cover ARM: sunxi: Add support for USB OTG PHY

Hi,

There is already support for two USB phys, which are used for USB host controllers on Allwinner SoC aka sunxi.

This patch series add support for the USB OTG phy (aka phy0).

The code has been tested on Allwinner A20 aka sun7i (Cubietruck).

Best regards,
Roman Byshko

Roman Byshko (5):
phy: sun4i: add support for USB phy0
ARM: dtsi: sunxi: add common VBUS regulator
ARM: sun7i: dtsi: add support for usbphy0
ARM: dts: sun7i: Cubietruck: override regulator pin
ARM: dts: sun7i: Cubietruck: add power supply regulator for USB OTG
VBUS

 arch/arm/boot/dts/sun7i-a20-cubietruck.dts     | 14 ++++++++++++++
 arch/arm/boot/dts/sun7i-a20.dtsi               |  4 ++--
 arch/arm/boot/dts/sunxi-common-regulators.dtsi | 19 +++++++++++++++++++
 drivers/phy/phy-sun4i-usb.c                    |  9 ++++++---
 4 files changed, 41 insertions(+), 5 deletions(-)
Signature · 1 line
2.1.2
author
#1 lore raw

PATCH 1/5 phy: sun4i: add support for USB phy0

The driver for sun4i USB phys currently supports only phy1 and phy2 which are used for USB host controllers. This patch adds support for USB phy0, which is used by the musb hdrc USB controller.

---
 drivers/phy/phy-sun4i-usb.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)
drivers/phy/phy-sun4i-usb.c +6 -3
index 0baf5ef..a24728b 100644
@@ -157,6 +157,10 @@ static int sun4i_usb_phy_init(struct phy *_phy)
 		return ret;
 	}
 
+	/* Regulation 45 ohms */
+	if (phy->index == 0)
+		sun4i_usb_phy_write(phy, PHY_RES45_CAL_EN, 0x01, 1);
+
 	/* Adjust PHY's magnitude and rate */
 	sun4i_usb_phy_write(phy, PHY_TX_AMPLITUDE_TUNE, 0x14, 5);
 

@@ -213,7 +217,7 @@ static struct phy *sun4i_usb_phy_xlate(struct device *dev,
 {
 	struct sun4i_usb_phy_data *data = dev_get_drvdata(dev);
 
-	if (WARN_ON(args->args[0] == 0 || args->args[0] >= data->num_phys))
+	if (args->args[0] >= data->num_phys)
 		return ERR_PTR(-ENODEV);
 
 	return data->phys[args->args[0]].phy;

@@ -255,8 +259,7 @@ static int sun4i_usb_phy_probe(struct platform_device *pdev)
 	if (IS_ERR(data->base))
 		return PTR_ERR(data->base);
 
-	/* Skip 0, 0 is the phy for otg which is not yet supported. */
-	for (i = 1; i < data->num_phys; i++) {
+	for (i = 0; i < data->num_phys; i++) {
 		struct sun4i_usb_phy *phy = data->phys + i;
 		char name[16];
Signature · 1 line
2.1.2
#2 lore raw

Re: [PATCH 1/5] phy: sun4i: add support for USB phy0

Hi Roman,

Thanks for your patches

Quoted 4 lines · Roman Byshko

The driver for sun4i USB phys currently supports only phy1 and phy2 which are used for USB host controllers. This patch adds support for USB phy0, which is used by the musb hdrc USB controller.

Like Andreas already pointed out, your signed-off-by is missing from all your patches.

Quoted 9 lines
---
 drivers/phy/phy-sun4i-usb.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)
drivers/phy/phy-sun4i-usb.c +0 -0
index 0baf5ef..a24728b 100644

drivers/phy/phy-sun4i-usb.c static int sun4i_usb_phy_init(struct phy *_phy) · line 162

Quoted 6 lines
drivers/phy/phy-sun4i-usb.c +3 -0
@@ -157,6 +157,10 @@ static int sun4i_usb_phy_init(struct phy *_phy)
 		return ret;
 	}
 
+	/* Regulation 45 ohms */
+	if (phy->index == 0)
+		sun4i_usb_phy_write(phy, PHY_RES45_CAL_EN, 0x01, 1);

What is this code supposed to do?

Some define for this bit and/or a better comment would be nice.

Maxime

Signature · 10 lines
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141028/eb9479e8/attachment.sig>
#3 lore raw

Re: [PATCH 1/5] phy: sun4i: add support for USB phy0

Quoted 31 lines · Maxime Ripard

Hi Roman,

Thanks for your patches

Quoted 4 lines · Roman Byshko

The driver for sun4i USB phys currently supports only phy1 and phy2 which are used for USB host controllers. This patch adds support for USB phy0, which is used by the musb hdrc USB controller.

Like Andreas already pointed out, your signed-off-by is missing from all your patches.

Quoted 9 lines
---
 drivers/phy/phy-sun4i-usb.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)
drivers/phy/phy-sun4i-usb.c +0 -0
index 0baf5ef..a24728b 100644

drivers/phy/phy-sun4i-usb.c static int sun4i_usb_phy_init(struct phy *_phy) · line 162

Quoted 6 lines
drivers/phy/phy-sun4i-usb.c +3 -0
@@ -157,6 +157,10 @@ static int sun4i_usb_phy_init(struct phy *_phy)
              return ret;
      }
 
+     /* Regulation 45 ohms */
+     if (phy->index == 0)
+             sun4i_usb_phy_write(phy, PHY_RES45_CAL_EN, 0x01, 1);

What is this code supposed to do?

Quoted 3 lines

Some define for this bit and/or a better comment would be nice.

From Allwinner's sources: "Enable/Disable USB res45 Calibration"

which I think refers to the internal 45 ohm termination resistors for the USB data lines. But I'm not an expert on USB hardware.

author
#4 lore raw

Re: [PATCH 1/5] phy: sun4i: add support for USB phy0

Hi all,

sorry for missing Signed-offs, they will be added, of course.

Quoted 4 lines
Quoted 4 lines
Quoted 3 lines
drivers/phy/phy-sun4i-usb.c +3 -0
+     /* Regulation 45 ohms */
+     if (phy->index == 0)
+             sun4i_usb_phy_write(phy, PHY_RES45_CAL_EN, 0x01, 1);
Quoted 8 lines
Quoted 3 lines

What is this code supposed to do?

Some define for this bit and/or a better comment would be nice.

From Allwinner's sources: "Enable/Disable USB res45 Calibration"

which I think refers to the internal 45 ohm termination resistors for the USB data lines. But I'm not an expert on USB hardware.

Same with me. I will however change the comment to make it more informative.

Thanks for your review,
Roman

#5 lore raw

Re: [PATCH 1/5] phy: sun4i: add support for USB phy0

In reply to Chen-Yu Tsai

Hi,

Quoted 33 lines · Chen-Yu Tsai
Quoted 31 lines · Maxime Ripard

Hi Roman,

Thanks for your patches

Quoted 4 lines · Roman Byshko

The driver for sun4i USB phys currently supports only phy1 and phy2 which are used for USB host controllers. This patch adds support for USB phy0, which is used by the musb hdrc USB controller.

Like Andreas already pointed out, your signed-off-by is missing from all your patches.

Quoted 9 lines
---
 drivers/phy/phy-sun4i-usb.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)
drivers/phy/phy-sun4i-usb.c +0 -0
index 0baf5ef..a24728b 100644

drivers/phy/phy-sun4i-usb.c static int sun4i_usb_phy_init(struct phy *_phy) · line 162

Quoted 6 lines
drivers/phy/phy-sun4i-usb.c +3 -0
@@ -157,6 +157,10 @@ static int sun4i_usb_phy_init(struct phy *_phy)
              return ret;
      }
 
+     /* Regulation 45 ohms */
+     if (phy->index == 0)
+             sun4i_usb_phy_write(phy, PHY_RES45_CAL_EN, 0x01, 1);

What is this code supposed to do?

Quoted 6 lines
Quoted 1 line

Some define for this bit and/or a better comment would be nice.

From Allwinner's sources: "Enable/Disable USB res45 Calibration"

which I think refers to the internal 45 ohm termination resistors for the USB data lines. But I'm not an expert on USB hardware.

That would make a great comment ;)

Maxime

Signature · 10 lines
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141030/1f9e45b7/attachment.sig>
author
#6 lore raw

PATCH 2/5 ARM: dtsi: sunxi: add common VBUS regulator

In reply to Roman Byshko

Until now the regulator nodes for powering USB VBUS existed only for the two host controllers. Now the regulator is added for USB OTG too.

---
 arch/arm/boot/dts/sunxi-common-regulators.dtsi | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
arch/arm/boot/dts/sunxi-common-regulators.dtsi +19 -0
index c9c5b10..f521672 100644
@@ -21,6 +21,13 @@
 				allwinner,pull = <0>;
 			};
 
+			usb0_vbus_pin_a: usb0_vbus_pin at 0 {
+				allwinner,pins = "PB9";
+				allwinner,function = "gpio_out";
+				allwinner,drive = <0>;
+				allwinner,pull = <0>;
+			};
+
 			usb1_vbus_pin_a: usb1_vbus_pin at 0 {
 				allwinner,pins = "PH6";
 				allwinner,function = "gpio_out";

@@ -49,6 +56,18 @@
 		status = "disabled";
 	};
 
+	reg_usb0_vbus: usb0-vbus {
+		compatible = "regulator-fixed";
+		pinctrl-names = "default";
+		pinctrl-0 = <&usb0_vbus_pin_a>;
+		regulator-name = "usb0-vbus";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		enable-active-high;
+		gpio = <&pio 1 9 0>;
+		status = "disabled";
+	};
+
 	reg_usb1_vbus: usb1-vbus {
 		compatible = "regulator-fixed";
 		pinctrl-names = "default";
Signature · 1 line
2.1.2
#7 lore raw

Re: [PATCH 2/5] ARM: dtsi: sunxi: add common VBUS regulator

Quoted 30 lines · Roman Byshko

Until now the regulator nodes for powering USB VBUS existed only for the two host controllers. Now the regulator is added for USB OTG too.

---
 arch/arm/boot/dts/sunxi-common-regulators.dtsi | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
arch/arm/boot/dts/sunxi-common-regulators.dtsi +8 -0
index c9c5b10..f521672 100644
@@ -21,6 +21,13 @@
 				allwinner,pull = <0>;
 			};
 
+			usb0_vbus_pin_a: usb0_vbus_pin at 0 {
+				allwinner,pins = "PB9";
+				allwinner,function = "gpio_out";
+				allwinner,drive = <0>;
+				allwinner,pull = <0>;
+			};
+
 			usb1_vbus_pin_a: usb1_vbus_pin at 0 {
 				allwinner,pins = "PH6";
 				allwinner,function = "gpio_out";

@@ -49,6 +56,18 @@
 		status = "disabled";
 	};
 
+	reg_usb0_vbus: usb0-vbus {

arch/arm/boot/dts/sunxi-common-regulators.dtsi line 69

Quoted 10 lines
arch/arm/boot/dts/sunxi-common-regulators.dtsi +10 -0
+		compatible = "regulator-fixed";
+		pinctrl-names = "default";
+		pinctrl-0 = <&usb0_vbus_pin_a>;
+		regulator-name = "usb0-vbus";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		enable-active-high;
+		gpio = <&pio 1 9 0>;
+		status = "disabled";
+	};

We're using these regulators because a huuuge majority of the boards out there, regardless of their SoC, are using the same GPIO to drive the same regulator. Is this the case here? What board have you looked at?

Thanks,
Maxime

Signature · 10 lines
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141028/644628e5/attachment.sig>
author
#8 lore raw

Re: [PATCH 2/5] ARM: dtsi: sunxi: add common VBUS regulator

Hi Maxime,

thank you for the review!

Quoted 30 lines · Maxime Ripard
Quoted 29 lines · Roman Byshko

Until now the regulator nodes for powering USB VBUS existed only for the two host controllers. Now the regulator is added for USB OTG too.

---
 arch/arm/boot/dts/sunxi-common-regulators.dtsi | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
arch/arm/boot/dts/sunxi-common-regulators.dtsi +7 -0
index c9c5b10..f521672 100644
@@ -21,6 +21,13 @@
                              allwinner,pull = <0>;
                      };
 
+                     usb0_vbus_pin_a: usb0_vbus_pin at 0 {
+                             allwinner,pins = "PB9";
+                             allwinner,function = "gpio_out";
+                             allwinner,drive = <0>;
+                             allwinner,pull = <0>;
+                     };
+
                      usb1_vbus_pin_a: usb1_vbus_pin at 0 {
                              allwinner,pins = "PH6";
                              allwinner,function = "gpio_out";

@@ -49,6 +56,18 @@
              status = "disabled";
      };
Quoted 16 lines

arch/arm/boot/dts/sunxi-common-regulators.dtsi line 69

Quoted 11 lines
arch/arm/boot/dts/sunxi-common-regulators.dtsi +11 -0
+     reg_usb0_vbus: usb0-vbus {
+             compatible = "regulator-fixed";
+             pinctrl-names = "default";
+             pinctrl-0 = <&usb0_vbus_pin_a>;
+             regulator-name = "usb0-vbus";
+             regulator-min-microvolt = <5000000>;
+             regulator-max-microvolt = <5000000>;
+             enable-active-high;
+             gpio = <&pio 1 9 0>;
+             status = "disabled";
+     };

We're using these regulators because a huuuge majority of the boards out there, regardless of their SoC, are using the same GPIO to drive the same regulator. Is this the case here? What board have you looked at?

Currently I'm developing and testing for Cubietruck. I also have a few other boards (with different SoC), I will add corresponding DT nodes for them later.

As for GPIO which is used to drive the USB0 VBUS pin.

$ git remote show origin
* remote origin
  Fetch URL: https://github.com/linux-sunxi/sunxi-boards.git
  Push  URL: https://github.com/linux-sunxi/sunxi-boards.git
...
$ grep -A7 usbc0 -r . | grep usb_drv_vbus_gpio | grep port | sort |
cut -d: -f2 | cut -d "<" -f1 | sort | uniq -c
      1 PA16
      1 PA17
     67 PB09
      4 PB10
      1 PB9
      1 PC17
      1 PD25
      4 PG11
     21 PG12
      1 PG13
      1 PH17
      4 power4

So PB09 is most often used. PG12 is used on (alsmost always) A13. So propose to make PB09 default, then override it in sun5i-a13.dtsi and later in board dts file if needed.

Cubietruck uses PH17. That is why I override PB09 in its dts file.

Sounds good?

Best,
Roman

Quoted 7 lines

Thanks,
Maxime

--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

#9 lore raw

Re: [PATCH 2/5] ARM: dtsi: sunxi: add common VBUS regulator

Hi,

Quoted 79 lines · Roman Byshko

Hi Maxime,

thank you for the review!

Quoted 30 lines · Maxime Ripard
Quoted 29 lines · Roman Byshko

Until now the regulator nodes for powering USB VBUS existed only for the two host controllers. Now the regulator is added for USB OTG too.

---
 arch/arm/boot/dts/sunxi-common-regulators.dtsi | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
arch/arm/boot/dts/sunxi-common-regulators.dtsi +7 -0
index c9c5b10..f521672 100644
@@ -21,6 +21,13 @@
                              allwinner,pull = <0>;
                      };
 
+                     usb0_vbus_pin_a: usb0_vbus_pin at 0 {
+                             allwinner,pins = "PB9";
+                             allwinner,function = "gpio_out";
+                             allwinner,drive = <0>;
+                             allwinner,pull = <0>;
+                     };
+
                      usb1_vbus_pin_a: usb1_vbus_pin at 0 {
                              allwinner,pins = "PH6";
                              allwinner,function = "gpio_out";

@@ -49,6 +56,18 @@
              status = "disabled";
      };
Quoted 16 lines

arch/arm/boot/dts/sunxi-common-regulators.dtsi line 69

Quoted 11 lines
arch/arm/boot/dts/sunxi-common-regulators.dtsi +11 -0
+     reg_usb0_vbus: usb0-vbus {
+             compatible = "regulator-fixed";
+             pinctrl-names = "default";
+             pinctrl-0 = <&usb0_vbus_pin_a>;
+             regulator-name = "usb0-vbus";
+             regulator-min-microvolt = <5000000>;
+             regulator-max-microvolt = <5000000>;
+             enable-active-high;
+             gpio = <&pio 1 9 0>;
+             status = "disabled";
+     };

We're using these regulators because a huuuge majority of the boards out there, regardless of their SoC, are using the same GPIO to drive the same regulator. Is this the case here? What board have you looked at?

Currently I'm developing and testing for Cubietruck. I also have a few other boards (with different SoC), I will add corresponding DT nodes for them later.

As for GPIO which is used to drive the USB0 VBUS pin.

$ git remote show origin
* remote origin
  Fetch URL: https://github.com/linux-sunxi/sunxi-boards.git
  Push  URL: https://github.com/linux-sunxi/sunxi-boards.git
...
$ grep -A7 usbc0 -r . | grep usb_drv_vbus_gpio | grep port | sort |
cut -d: -f2 | cut -d "<" -f1 | sort | uniq -c
      1 PA16
      1 PA17
     67 PB09
      4 PB10
      1 PB9
      1 PC17
      1 PD25
      4 PG11
     21 PG12
      1 PG13
      1 PH17
      4 power4
Quoted 7 lines

So PB09 is most often used. PG12 is used on (alsmost always) A13. So propose to make PB09 default, then override it in sun5i-a13.dtsi and later in board dts file if needed.

Cubietruck uses PH17. That is why I override PB09 in its dts file.

Sounds good?

It does :)

Thanks!
Maxime

Signature · 10 lines
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141030/70cc0a7b/attachment.sig>
author
#10 lore raw

PATCH 3/5 ARM: sun7i: dtsi: add support for usbphy0

In reply to Roman Byshko
---
 arch/arm/boot/dts/sun7i-a20.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
arch/arm/boot/dts/sun7i-a20.dtsi +2 -2
index 82097c9..f787ccb 100644
@@ -552,8 +552,8 @@
 			reg-names = "phy_ctrl", "pmu1", "pmu2";
 			clocks = <&usb_clk 8>;
 			clock-names = "usb_phy";
-			resets = <&usb_clk 1>, <&usb_clk 2>;
-			reset-names = "usb1_reset", "usb2_reset";
+			resets = <&usb_clk 0>, <&usb_clk 1>, <&usb_clk 2>;
+			reset-names = "usb0_reset", "usb1_reset", "usb2_reset";
 			status = "disabled";
 		};
Signature · 1 line
2.1.2
author
#11 lore raw

PATCH 4/5 ARM: dts: sun7i: Cubietruck: override regulator pin

In reply to Roman Byshko

Cubietruck uses different pin for the USB OTG VBUS that is why we override the one defined in sunxi-common-regulators.dtsi

---
 arch/arm/boot/dts/sun7i-a20-cubietruck.dts | 13 +++++++++++++
 1 file changed, 13 insertions(+)
arch/arm/boot/dts/sun7i-a20-cubietruck.dts +13 -0
index a6c1a3c..1682307 100644
@@ -92,6 +92,13 @@
 				allwinner,drive = <0>;
 				allwinner,pull = <0>;
 			};
+
+			usb0_vbus_pin_a: usb0_vbus_pin at 0 {
+				allwinner,pins = "PH17";
+				allwinner,function = "gpio_out";
+				allwinner,drive = <0>;
+				allwinner,pull = <0>;
+			};
 		};
 
 		pwm: pwm at 01c20e00 {

@@ -185,6 +192,12 @@
 		status = "okay";
 	};
 
+	reg_usb0_vbus: usb0-vbus {
+		pinctrl-0 = <&usb0_vbus_pin_a>;
+		gpio = <&pio 7 17 0>;
+		status = "okay";
+	};
+
 	reg_usb1_vbus: usb1-vbus {
 		status = "okay";
 	};
Signature · 1 line
2.1.2
author
#12 lore raw

PATCH 5/5 ARM: dts: sun7i: Cubietruck: add power supply regulator for USB OTG VBUS

In reply to Roman Byshko
---
 arch/arm/boot/dts/sun7i-a20-cubietruck.dts | 1 +
 1 file changed, 1 insertion(+)
arch/arm/boot/dts/sun7i-a20-cubietruck.dts +1 -0
index 1682307..a281d25 100644
@@ -40,6 +40,7 @@
 		};
 
 		usbphy: phy at 01c13400 {
+			usb0_vbus-supply = <&reg_usb0_vbus>;
 			usb1_vbus-supply = <&reg_usb1_vbus>;
 			usb2_vbus-supply = <&reg_usb2_vbus>;
 			status = "okay";
Signature · 1 line
2.1.2
#13 lore raw
In reply to Roman Byshko

Hi Roman,

Am 27.10.2014 um 22:43 schrieb Roman Byshko:

Quoted 6 lines

There is already support for two USB phys, which are used for USB host controllers on Allwinner SoC aka sunxi.

This patch series add support for the USB OTG phy (aka phy0).

The code has been tested on Allwinner A20 aka sun7i (Cubietruck).

All these patches are lacking a Signed-off-by.

Regards,
Andreas

Signature · 2 lines
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N?rnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imend?rffer; HRB 16746 AG N?rnberg