[PATCH v4 0/2] ARM: sun8i-r40: Enable usb otg support

[PATCH v4 0/2] ARM: sun8i-r40: Enable usb otg support

[qianfanguijin]

From: qianfan Zhao <qianfanguijin@163.com>

History:
=======

v4(2022-05-18):
- Enable both musb and OHCI/EHCI support

Tests:
======

All test cases were tested on bananapi-m2-ultra.

1. USB DEVICE(ping test)

Enable usb gadget rndis network, ping m2u on ubuntu host:

➜  ~ ping 192.168.200.2
PING 192.168.200.2 (192.168.200.2) 56(84) bytes of data.
64 bytes from 192.168.200.2: icmp_seq=1 ttl=64 time=0.544 ms
64 bytes from 192.168.200.2: icmp_seq=2 ttl=64 time=0.269 ms
64 bytes from 192.168.200.2: icmp_seq=3 ttl=64 time=0.300 ms
64 bytes from 192.168.200.2: icmp_seq=4 ttl=64 time=0.295 ms
64 bytes from 192.168.200.2: icmp_seq=5 ttl=64 time=0.283 ms
64 bytes from 192.168.200.2: icmp_seq=6 ttl=64 time=0.226 ms
64 bytes from 192.168.200.2: icmp_seq=7 ttl=64 time=0.246 ms
64 bytes from 192.168.200.2: icmp_seq=8 ttl=64 time=0.204 ms
64 bytes from 192.168.200.2: icmp_seq=9 ttl=64 time=0.302 ms
64 bytes from 192.168.200.2: icmp_seq=10 ttl=64 time=0.249 ms
64 bytes from 192.168.200.2: icmp_seq=11 ttl=64 time=0.459 ms
64 bytes from 192.168.200.2: icmp_seq=12 ttl=64 time=0.232 ms
64 bytes from 192.168.200.2: icmp_seq=13 ttl=64 time=0.275 ms
64 bytes from 192.168.200.2: icmp_seq=14 ttl=64 time=0.243 ms

2. USB HOST(OHCI)

Connect an usb serial port on OTG port, nex t is the kernel log:

[   27.824137] usb 2-1: new full-speed USB device number 2 using ohci-platform
[   28.865504] cdc_acm 2-1:1.0: ttyACM0: USB ACM device
[   29.565509] cdc_acm 2-1:1.2: ttyACM1: USB ACM device

3. USB HOST(EHCI)

Connect an usb storage on OTG port, next is the kernel log:

[   17.754147] usb 1-1: new high-speed USB device number 2 using ehci-platform
[   17.955995] usb-storage 1-1:1.0: USB Mass Storage device detected
[   18.024497] scsi host1: usb-storage 1-1:1.0
[   19.035091] scsi 1:0:0:0: Direct-Access     General  USB Flash Disk   1.0  PQ: 0 ANSI: 2
[   19.049717] sd 1:0:0:0: [sda] 7831552 512-byte logical blocks: (4.01 GB/3.73 GiB)
[   19.060873] sd 1:0:0:0: [sda] Write Protect is off
[   19.071018] sd 1:0:0:0: [sda] No Caching mode page found
[   19.076437] sd 1:0:0:0: [sda] Assuming drive cache: write through
[   19.093566]  sda: sda1
[   19.103492] sd 1:0:0:0: [sda] Attached SCSI removable disk

issues:
=======

The system power often turned off when I plugged an usb device into the OTG port.
It's not clear why.

qianfan Zhao (2):
  ARM: dts: sun8i-r40: Add USB0_OTG/HOST support
  ARM: dts: bananapi-m2-ultra: Enable USB0_OTG and HOST support

 .../boot/dts/sun8i-r40-bananapi-m2-ultra.dts  | 39 +++++++++++++++++++
 arch/arm/boot/dts/sun8i-r40.dtsi              | 34 ++++++++++++++++
 2 files changed, 73 insertions(+)

-- 
2.25.1

[PATCH v4 1/2] ARM: dts: sun8i-r40: Add USB0_OTG/HOST support

[qianfanguijin]

From: qianfan Zhao <qianfanguijin@163.com>

The USB0 port of R40 is divided into two controllers, one is H3
compatibled MUSB device, another is OHCI/EHCI.

Signed-off-by: qianfan Zhao <qianfanguijin@163.com>
---
 arch/arm/boot/dts/sun8i-r40.dtsi | 34 ++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-r40.dtsi b/arch/arm/boot/dts/sun8i-r40.dtsi
index 212e19183484..ae48474fdefa 100644
--- a/arch/arm/boot/dts/sun8i-r40.dtsi
+++ b/arch/arm/boot/dts/sun8i-r40.dtsi
@@ -401,6 +401,21 @@ mmc3: mmc@1c12000 {
 			#size-cells = <0>;
 		};
 
+		usb_otg: usb@1c13000 {
+			compatible = "allwinner,sun8i-r40-musb",
+				     "allwinner,sun8i-h3-musb";
+			reg = <0x01c13000 0x0400>;
+			clocks = <&ccu CLK_BUS_OTG>;
+			resets = <&ccu RST_BUS_OTG>;
+			interrupts = <GIC_SPI 38 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "mc";
+			phys = <&usbphy 0>;
+			phy-names = "usb";
+			extcon = <&usbphy 0>;
+			dr_mode = "otg";
+			status = "disabled";
+		};
+
 		usbphy: phy@1c13400 {
 			compatible = "allwinner,sun8i-r40-usb-phy";
 			reg = <0x01c13400 0x14>,
@@ -427,6 +442,25 @@ usbphy: phy@1c13400 {
 			#phy-cells = <1>;
 		};
 
+		ehci0: usb@1c14000 {
+			compatible = "allwinner,sun8i-r40-ehci", "generic-ehci";
+			reg = <0x01c14000 0x100>;
+			interrupts = <GIC_SPI 39 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&ccu CLK_BUS_EHCI0>, <&ccu CLK_BUS_OHCI0>;
+			resets = <&ccu RST_BUS_EHCI0>, <&ccu RST_BUS_OHCI0>;
+			status = "disabled";
+		};
+
+		ohci0: usb@1c14400 {
+			compatible = "allwinner,sun8i-r40-ohci", "generic-ohci";
+			reg = <0x01c14400 0x100>;
+			interrupts = <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&ccu CLK_BUS_EHCI0>, <&ccu CLK_BUS_OHCI0>,
+				 <&ccu CLK_USB_OHCI0>;
+			resets = <&ccu RST_BUS_EHCI0>, <&ccu RST_BUS_OHCI0>;
+			status = "disabled";
+		};
+
 		crypto: crypto@1c15000 {
 			compatible = "allwinner,sun8i-r40-crypto";
 			reg = <0x01c15000 0x1000>;
-- 
2.25.1

[PATCH v4 2/2] ARM: dts: bananapi-m2-ultra: Enable USB0_OTG and HOST support

[qianfanguijin]

From: qianfan Zhao <qianfanguijin@163.com>

let USB0 work at OTG mode.

Signed-off-by: qianfan Zhao <qianfanguijin@163.com>
---
 .../boot/dts/sun8i-r40-bananapi-m2-ultra.dts  | 39 +++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-r40-bananapi-m2-ultra.dts b/arch/arm/boot/dts/sun8i-r40-bananapi-m2-ultra.dts
index 28197bbcb1d5..b3421e67967d 100644
--- a/arch/arm/boot/dts/sun8i-r40-bananapi-m2-ultra.dts
+++ b/arch/arm/boot/dts/sun8i-r40-bananapi-m2-ultra.dts
@@ -122,6 +122,10 @@ &de {
 	status = "okay";
 };
 
+&ehci0 {
+	status = "okay";
+};
+
 &ehci1 {
 	status = "okay";
 };
@@ -164,6 +168,7 @@ axp22x: pmic@34 {
 		reg = <0x34>;
 		interrupt-parent = <&nmi_intc>;
 		interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
+		x-powers,drive-vbus-en;
 	};
 };
 
@@ -199,6 +204,10 @@ &mmc2 {
 	status = "okay";
 };
 
+&ohci0 {
+	status = "okay";
+};
+
 &ohci1 {
 	status = "okay";
 };
@@ -216,6 +225,15 @@ &pio {
 	vcc-pe-supply = <&reg_eldo1>;
 	vcc-pf-supply = <&reg_dcdc1>;
 	vcc-pg-supply = <&reg_dldo1>;
+
+	/* USB0_DRVVBUS connected to both the PMIC.N_VBUSEN and PI13,
+	 * we chose PMIC.N_VBUSEN for control, so set the gpio as
+	 * input mode here.
+	 */
+	usb0_vbus_enable_gpio: usb0-vbus-enable-gpio {
+		pins = "PI13";
+		function = "gpio_in";
+	};
 };
 
 &reg_aldo2 {
@@ -298,6 +316,11 @@ &reg_dldo4 {
 	regulator-name = "vdd2v5-sata";
 };
 
+&reg_drivevbus {
+	regulator-name = "usb0-vbus";
+	status = "okay";
+};
+
 &reg_eldo3 {
 	regulator-min-microvolt = <1200000>;
 	regulator-max-microvolt = <1200000>;
@@ -333,7 +356,23 @@ bluetooth {
 	};
 };
 
+&usb_otg {
+	dr_mode = "otg";
+	status = "okay";
+};
+
+&usb_power_supply {
+	status = "okay";
+};
+
 &usbphy {
+	pinctrl-names = "default";
+	pinctrl-0 = <&usb0_vbus_enable_gpio>;
+
+	usb0_id_det-gpios = <&pio 8 4 GPIO_ACTIVE_HIGH>; /* PI4 */
+	usb0_vbus_det-gpios = <&pio 8 8 GPIO_ACTIVE_HIGH>; /* PI8 */
+	usb0_vbus_power-supply = <&usb_power_supply>;
+	usb0_vbus-supply = <&reg_drivevbus>;
 	usb1_vbus-supply = <&reg_vcc5v0>;
 	usb2_vbus-supply = <&reg_vcc5v0>;
 	status = "okay";
-- 
2.25.1

Re: [PATCH v4 0/2] ARM: sun8i-r40: Enable usb otg support

[qianfan]

在 2022/5/18 18:17, qianfanguijin@163.com 写道:
> From: qianfan Zhao <qianfanguijin@163.com>
>
> History:
> =======
>
> v4(2022-05-18):
> - Enable both musb and OHCI/EHCI support
>
> Tests:
> ======
>
> All test cases were tested on bananapi-m2-ultra.
>
> 1. USB DEVICE(ping test)
>
> Enable usb gadget rndis network, ping m2u on ubuntu host:
>
> ➜  ~ ping 192.168.200.2
> PING 192.168.200.2 (192.168.200.2) 56(84) bytes of data.
> 64 bytes from 192.168.200.2: icmp_seq=1 ttl=64 time=0.544 ms
> 64 bytes from 192.168.200.2: icmp_seq=2 ttl=64 time=0.269 ms
> 64 bytes from 192.168.200.2: icmp_seq=3 ttl=64 time=0.300 ms
> 64 bytes from 192.168.200.2: icmp_seq=4 ttl=64 time=0.295 ms
> 64 bytes from 192.168.200.2: icmp_seq=5 ttl=64 time=0.283 ms
> 64 bytes from 192.168.200.2: icmp_seq=6 ttl=64 time=0.226 ms
> 64 bytes from 192.168.200.2: icmp_seq=7 ttl=64 time=0.246 ms
> 64 bytes from 192.168.200.2: icmp_seq=8 ttl=64 time=0.204 ms
> 64 bytes from 192.168.200.2: icmp_seq=9 ttl=64 time=0.302 ms
> 64 bytes from 192.168.200.2: icmp_seq=10 ttl=64 time=0.249 ms
> 64 bytes from 192.168.200.2: icmp_seq=11 ttl=64 time=0.459 ms
> 64 bytes from 192.168.200.2: icmp_seq=12 ttl=64 time=0.232 ms
> 64 bytes from 192.168.200.2: icmp_seq=13 ttl=64 time=0.275 ms
> 64 bytes from 192.168.200.2: icmp_seq=14 ttl=64 time=0.243 ms
>
> 2. USB HOST(OHCI)
>
> Connect an usb serial port on OTG port, nex t is the kernel log:
>
> [   27.824137] usb 2-1: new full-speed USB device number 2 using ohci-platform
> [   28.865504] cdc_acm 2-1:1.0: ttyACM0: USB ACM device
> [   29.565509] cdc_acm 2-1:1.2: ttyACM1: USB ACM device
>
> 3. USB HOST(EHCI)
>
> Connect an usb storage on OTG port, next is the kernel log:
>
> [   17.754147] usb 1-1: new high-speed USB device number 2 using ehci-platform
> [   17.955995] usb-storage 1-1:1.0: USB Mass Storage device detected
> [   18.024497] scsi host1: usb-storage 1-1:1.0
> [   19.035091] scsi 1:0:0:0: Direct-Access     General  USB Flash Disk   1.0  PQ: 0 ANSI: 2
> [   19.049717] sd 1:0:0:0: [sda] 7831552 512-byte logical blocks: (4.01 GB/3.73 GiB)
> [   19.060873] sd 1:0:0:0: [sda] Write Protect is off
> [   19.071018] sd 1:0:0:0: [sda] No Caching mode page found
> [   19.076437] sd 1:0:0:0: [sda] Assuming drive cache: write through
> [   19.093566]  sda: sda1
> [   19.103492] sd 1:0:0:0: [sda] Attached SCSI removable disk
>
> issues:
> =======
>
> The system power often turned off when I plugged an usb device into the OTG port.
> It's not clear why.
It is not caused by software.

According to the schematic of M2U, there is a 100uF capacitor on the 
USBVBUS,
but noting on ACIN. The board is powered by ACIN when I test usb otg,
plugged an usb storage into the otg port, USB0 enter host mode and will
enable the USBVBUS power supply through N_VBUSEN. At this time due to the
limits of ACIN, the input voltage dropped and AXP enter shutdown protection
state.

This problem was sloved when I mounted a large capacitor on ACIN.
Both usb device and host stack can work fine now.
>
> qianfan Zhao (2):
>    ARM: dts: sun8i-r40: Add USB0_OTG/HOST support
>    ARM: dts: bananapi-m2-ultra: Enable USB0_OTG and HOST support
>
>   .../boot/dts/sun8i-r40-bananapi-m2-ultra.dts  | 39 +++++++++++++++++++
>   arch/arm/boot/dts/sun8i-r40.dtsi              | 34 ++++++++++++++++
>   2 files changed, 73 insertions(+)
>

Re: [PATCH v4 0/2] ARM: sun8i-r40: Enable usb otg support

[Evgeny Boger]

Hi qianfan,

As Allwinner A40i user, let me first thank you for your effort for 
making better upstream support for R40!

However, I would strongly suggest *not* to add USB support to one more 
Allwinner SoC in this particular way.
The problem is, this approach consists of a number of carefully crafted 
hacks in device tree to make current drivers work on Allwinner hardware 
without modification to the drivers.

a few examples:

1) please notice how ohci0 and ehci0 nodes do not contain reference to 
usb phy. It is done intentionally, otherwise EHCI will reset musb mode.
Of course omitting phy reference here is also completely breaking power 
cycling in case of usb error and otherwise messes with a power management.

2) one must always enable ohci, ehci and usb_otg nodes at the same time. 
If one forgets to enable ohci/ehci nodes while enabling usb_otg node, 
the system will silently fail to work as USB host.

3) For host-only mode we still have to enable usb_otg node despite no 
role switching is needed. That's because phy reference is missing in 
ehci/ohci, so the ehci/ohci driver won't enable the PHY.
Also I might be wrong, but I think phy won't be routed to ehci/ohci 
controllers is this case.

4) musb host controller is initialized and present to hardware though 
never actually used

To summarize, not only the resulting device tree is not describing the 
hardware properly, it is creating device tree configuration which will 
be very hard to support in future, once proper driver support is in place.


At Wiren Board kernel tree we tried to untangle this issue [1-6]. 
Unfortunately I didn't have time to prepare it for kernel submission 
yet, but I think I better submit it as RFC to get a feedback from you 
and others.


[1] 
https://github.com/wirenboard/linux/commit/359abbbd86ddff4d3c61179c882c286de32bb089
[2] 
https://github.com/wirenboard/linux/commit/6327f9d7972c21b229fb83457fdde643b31553f9
[3] 
https://github.com/wirenboard/linux/commit/f01f4c66758bde460a4d8c5b54ecee3b585c0232
[4] 
https://github.com/wirenboard/linux/commit/c27598ad601e5a46f624b73412a531d6f1f63d37
[5] 
https://github.com/wirenboard/linux/commit/5796d6eebb86b32a3751b2038b63af46f94954b3
[6] 
https://github.com/wirenboard/linux/commit/0928a675d875f9c2849fd3a9888f718bbb673bda


-- 
Kind regards,
Evgeny Boger
CTO @ Wiren Board
+49 3046690053
https://wirenboard.com/

Re: [PATCH v4 0/2] ARM: sun8i-r40: Enable usb otg support

[qianfan]

在 2022/5/20 4:54, Evgeny Boger 写道:
> Hi qianfan,
>
> As Allwinner A40i user, let me first thank you for your effort for 
> making better upstream support for R40!
>
> However, I would strongly suggest *not* to add USB support to one more 
> Allwinner SoC in this particular way.
> The problem is, this approach consists of a number of carefully 
> crafted hacks in device tree to make current drivers work on Allwinner 
> hardware without modification to the drivers.
>
> a few examples:
>
> 1) please notice how ohci0 and ehci0 nodes do not contain reference to 
> usb phy. It is done intentionally, otherwise EHCI will reset musb mode.
> Of course omitting phy reference here is also completely breaking 
> power cycling in case of usb error and otherwise messes with a power 
> management.
>
> 2) one must always enable ohci, ehci and usb_otg nodes at the same 
> time. If one forgets to enable ohci/ehci nodes while enabling usb_otg 
> node, the system will silently fail to work as USB host.
>
> 3) For host-only mode we still have to enable usb_otg node despite no 
> role switching is needed. That's because phy reference is missing in 
> ehci/ohci, so the ehci/ohci driver won't enable the PHY.
> Also I might be wrong, but I think phy won't be routed to ehci/ohci 
> controllers is this case.
>
> 4) musb host controller is initialized and present to hardware though 
> never actually used
>
> To summarize, not only the resulting device tree is not describing the 
> hardware properly, it is creating device tree configuration which will 
> be very hard to support in future, once proper driver support is in 
> place.
PHY setting is did in MUSB driver, so we need enable MUSB regardless of 
host mode.

I know your's point, OHCI/EHCI need do more works to init USBPHY, it 
shoule be able to work
in dependently, but I don't have the ability to deal with these things 
right now, I need
learn more things about OHCI/EHCI, that's a long-term goal.

So now I need to make the whole usb work and do some tests as much as 
possible,
hoping to merge this patch into master. Some other optimizations can be 
made later.

Thanks for yours guide.
>
>
> At Wiren Board kernel tree we tried to untangle this issue [1-6]. 
> Unfortunately I didn't have time to prepare it for kernel submission 
> yet, but I think I better submit it as RFC to get a feedback from you 
> and others.
>
>
> [1] 
> https://github.com/wirenboard/linux/commit/359abbbd86ddff4d3c61179c882c286de32bb089
> [2] 
> https://github.com/wirenboard/linux/commit/6327f9d7972c21b229fb83457fdde643b31553f9
> [3] 
> https://github.com/wirenboard/linux/commit/f01f4c66758bde460a4d8c5b54ecee3b585c0232
> [4] 
> https://github.com/wirenboard/linux/commit/c27598ad601e5a46f624b73412a531d6f1f63d37
> [5] 
> https://github.com/wirenboard/linux/commit/5796d6eebb86b32a3751b2038b63af46f94954b3
> [6] 
> https://github.com/wirenboard/linux/commit/0928a675d875f9c2849fd3a9888f718bbb673bda
>
>

Re: [PATCH v4 0/2] ARM: sun8i-r40: Enable usb otg support

[Evgeny Boger]

Hi,

my point is not that your patch is good enough.

My point is that it would be very difficult to maintain backward 
compatibility with this device tree once proper driver support is 
implemented.


On 5/21/22 07:26, qianfan wrote:
>
>
> 在 2022/5/20 4:54, Evgeny Boger 写道:
>> Hi qianfan,
>>
>> As Allwinner A40i user, let me first thank you for your effort for 
>> making better upstream support for R40!
>>
>> However, I would strongly suggest *not* to add USB support to one 
>> more Allwinner SoC in this particular way.
>> The problem is, this approach consists of a number of carefully 
>> crafted hacks in device tree to make current drivers work on 
>> Allwinner hardware without modification to the drivers.
>>
>> a few examples:
>>
>> 1) please notice how ohci0 and ehci0 nodes do not contain reference 
>> to usb phy. It is done intentionally, otherwise EHCI will reset musb 
>> mode.
>> Of course omitting phy reference here is also completely breaking 
>> power cycling in case of usb error and otherwise messes with a power 
>> management.
>>
>> 2) one must always enable ohci, ehci and usb_otg nodes at the same 
>> time. If one forgets to enable ohci/ehci nodes while enabling usb_otg 
>> node, the system will silently fail to work as USB host.
>>
>> 3) For host-only mode we still have to enable usb_otg node despite no 
>> role switching is needed. That's because phy reference is missing in 
>> ehci/ohci, so the ehci/ohci driver won't enable the PHY.
>> Also I might be wrong, but I think phy won't be routed to ehci/ohci 
>> controllers is this case.
>>
>> 4) musb host controller is initialized and present to hardware though 
>> never actually used
>>
>> To summarize, not only the resulting device tree is not describing 
>> the hardware properly, it is creating device tree configuration which 
>> will be very hard to support in future, once proper driver support is 
>> in place.
> PHY setting is did in MUSB driver, so we need enable MUSB regardless 
> of host mode.
>
> I know your's point, OHCI/EHCI need do more works to init USBPHY, it 
> shoule be able to work
> in dependently, but I don't have the ability to deal with these things 
> right now, I need
> learn more things about OHCI/EHCI, that's a long-term goal.
>
> So now I need to make the whole usb work and do some tests as much as 
> possible,
> hoping to merge this patch into master. Some other optimizations can 
> be made later.
>
> Thanks for yours guide.
>>
>>
>> At Wiren Board kernel tree we tried to untangle this issue [1-6]. 
>> Unfortunately I didn't have time to prepare it for kernel submission 
>> yet, but I think I better submit it as RFC to get a feedback from you 
>> and others.
>>
>>
>> [1] 
>> https://github.com/wirenboard/linux/commit/359abbbd86ddff4d3c61179c882c286de32bb089
>> [2] 
>> https://github.com/wirenboard/linux/commit/6327f9d7972c21b229fb83457fdde643b31553f9
>> [3] 
>> https://github.com/wirenboard/linux/commit/f01f4c66758bde460a4d8c5b54ecee3b585c0232
>> [4] 
>> https://github.com/wirenboard/linux/commit/c27598ad601e5a46f624b73412a531d6f1f63d37
>> [5] 
>> https://github.com/wirenboard/linux/commit/5796d6eebb86b32a3751b2038b63af46f94954b3
>> [6] 
>> https://github.com/wirenboard/linux/commit/0928a675d875f9c2849fd3a9888f718bbb673bda
>>
>>
>


-- 
Kind regards,
Evgeny Boger
CTO @ Wiren Board
+49 3046690053
https://wirenboard.com/

Re: [PATCH v4 0/2] ARM: sun8i-r40: Enable usb otg support

[Icenowy Zheng]

在 2022-05-19星期四的 23:54 +0300，Evgeny Boger写道：
> Hi qianfan,
> 
> As Allwinner A40i user, let me first thank you for your effort for 
> making better upstream support for R40!
> 
> However, I would strongly suggest *not* to add USB support to one
> more 
> Allwinner SoC in this particular way.
> The problem is, this approach consists of a number of carefully
> crafted 
> hacks in device tree to make current drivers work on Allwinner
> hardware 
> without modification to the drivers.
> 
> a few examples:
> 
> 1) please notice how ohci0 and ehci0 nodes do not contain reference
> to 
> usb phy. It is done intentionally, otherwise EHCI will reset musb
> mode.
> Of course omitting phy reference here is also completely breaking
> power 
> cycling in case of usb error and otherwise messes with a power
> management.
> 
> 2) one must always enable ohci, ehci and usb_otg nodes at the same
> time. 
> If one forgets to enable ohci/ehci nodes while enabling usb_otg node,
> the system will silently fail to work as USB host.
> 
> 3) For host-only mode we still have to enable usb_otg node despite no
> role switching is needed. That's because phy reference is missing in 
> ehci/ohci, so the ehci/ohci driver won't enable the PHY.
> Also I might be wrong, but I think phy won't be routed to ehci/ohci 
> controllers is this case.
> 
> 4) musb host controller is initialized and present to hardware though
> never actually used
> 
> To summarize, not only the resulting device tree is not describing
> the 
> hardware properly, it is creating device tree configuration which
> will 
> be very hard to support in future, once proper driver support is in
> place.
> 
> 
> At Wiren Board kernel tree we tried to untangle this issue [1-6]. 
> Unfortunately I didn't have time to prepare it for kernel submission 
> yet, but I think I better submit it as RFC to get a feedback from you
> and others.

Please cc me in the following patches because it's me who makes the
whole dual route thing hacky.

> 
> 
> [1] 
> https://github.com/wirenboard/linux/commit/359abbbd86ddff4d3c61179c882c286de32bb089
> [2] 
> https://github.com/wirenboard/linux/commit/6327f9d7972c21b229fb83457fdde643b31553f9
> [3] 
> https://github.com/wirenboard/linux/commit/f01f4c66758bde460a4d8c5b54ecee3b585c0232
> [4] 
> https://github.com/wirenboard/linux/commit/c27598ad601e5a46f624b73412a531d6f1f63d37
> [5] 
> https://github.com/wirenboard/linux/commit/5796d6eebb86b32a3751b2038b63af46f94954b3
> [6] 
> https://github.com/wirenboard/linux/commit/0928a675d875f9c2849fd3a9888f718bbb673bda
> 
> 

Re: [PATCH v4 0/2] ARM: sun8i-r40: Enable usb otg support

[Icenowy Zheng]

在 2022-05-18星期三的 18:17 +0800，qianfanguijin@163.com写道：
> From: qianfan Zhao <qianfanguijin@163.com>
> 
> History:
> =======
> 
> v4(2022-05-18):
> - Enable both musb and OHCI/EHCI support
> 
> Tests:
> ======
> 
> All test cases were tested on bananapi-m2-ultra.
> 
> 1. USB DEVICE(ping test)
> 
> Enable usb gadget rndis network, ping m2u on ubuntu host:

Interestingly musb previous totally fail when I initially work on R40.
Maybe some phy-sun4i-usb patches fixed it by accident?

> 
> ➜  ~ ping 192.168.200.2
> PING 192.168.200.2 (192.168.200.2) 56(84) bytes of data.
> 64 bytes from 192.168.200.2: icmp_seq=1 ttl=64 time=0.544 ms
> 64 bytes from 192.168.200.2: icmp_seq=2 ttl=64 time=0.269 ms
> 64 bytes from 192.168.200.2: icmp_seq=3 ttl=64 time=0.300 ms
> 64 bytes from 192.168.200.2: icmp_seq=4 ttl=64 time=0.295 ms
> 64 bytes from 192.168.200.2: icmp_seq=5 ttl=64 time=0.283 ms
> 64 bytes from 192.168.200.2: icmp_seq=6 ttl=64 time=0.226 ms
> 64 bytes from 192.168.200.2: icmp_seq=7 ttl=64 time=0.246 ms
> 64 bytes from 192.168.200.2: icmp_seq=8 ttl=64 time=0.204 ms
> 64 bytes from 192.168.200.2: icmp_seq=9 ttl=64 time=0.302 ms
> 64 bytes from 192.168.200.2: icmp_seq=10 ttl=64 time=0.249 ms
> 64 bytes from 192.168.200.2: icmp_seq=11 ttl=64 time=0.459 ms
> 64 bytes from 192.168.200.2: icmp_seq=12 ttl=64 time=0.232 ms
> 64 bytes from 192.168.200.2: icmp_seq=13 ttl=64 time=0.275 ms
> 64 bytes from 192.168.200.2: icmp_seq=14 ttl=64 time=0.243 ms
> 
> 2. USB HOST(OHCI)
> 
> Connect an usb serial port on OTG port, nex t is the kernel log:
> 
> [   27.824137] usb 2-1: new full-speed USB device number 2 using
> ohci-platform
> [   28.865504] cdc_acm 2-1:1.0: ttyACM0: USB ACM device
> [   29.565509] cdc_acm 2-1:1.2: ttyACM1: USB ACM device
> 
> 3. USB HOST(EHCI)
> 
> Connect an usb storage on OTG port, next is the kernel log:
> 
> [   17.754147] usb 1-1: new high-speed USB device number 2 using
> ehci-platform
> [   17.955995] usb-storage 1-1:1.0: USB Mass Storage device detected
> [   18.024497] scsi host1: usb-storage 1-1:1.0
> [   19.035091] scsi 1:0:0:0: Direct-Access     General  USB Flash
> Disk   1.0  PQ: 0 ANSI: 2
> [   19.049717] sd 1:0:0:0: [sda] 7831552 512-byte logical blocks:
> (4.01 GB/3.73 GiB)
> [   19.060873] sd 1:0:0:0: [sda] Write Protect is off
> [   19.071018] sd 1:0:0:0: [sda] No Caching mode page found
> [   19.076437] sd 1:0:0:0: [sda] Assuming drive cache: write through
> [   19.093566]  sda: sda1
> [   19.103492] sd 1:0:0:0: [sda] Attached SCSI removable disk
> 
> issues:
> =======
> 
> The system power often turned off when I plugged an usb device into
> the OTG port.
> It's not clear why.
> 
> qianfan Zhao (2):
>   ARM: dts: sun8i-r40: Add USB0_OTG/HOST support
>   ARM: dts: bananapi-m2-ultra: Enable USB0_OTG and HOST support
> 
>  .../boot/dts/sun8i-r40-bananapi-m2-ultra.dts  | 39
> +++++++++++++++++++
>  arch/arm/boot/dts/sun8i-r40.dtsi              | 34 ++++++++++++++++
>  2 files changed, 73 insertions(+)
> 

Re: [PATCH v4 0/2] ARM: sun8i-r40: Enable usb otg support

[qianfan]

在 2022/5/23 21:11, Icenowy Zheng 写道:
> 在 2022-05-18星期三的 18:17 +0800，qianfanguijin@163.com写道：
>> From: qianfan Zhao <qianfanguijin@163.com>
>>
>> History:
>> =======
>>
>> v4(2022-05-18):
>> - Enable both musb and OHCI/EHCI support
>>
>> Tests:
>> ======
>>
>> All test cases were tested on bananapi-m2-ultra.
>>
>> 1. USB DEVICE(ping test)
>>
>> Enable usb gadget rndis network, ping m2u on ubuntu host:
> Interestingly musb previous totally fail when I initially work on R40.
> Maybe some phy-sun4i-usb patches fixed it by accident?
Hi, could you please try this patch again?
>
>> ➜  ~ ping 192.168.200.2
>> PING 192.168.200.2 (192.168.200.2) 56(84) bytes of data.
>> 64 bytes from 192.168.200.2: icmp_seq=1 ttl=64 time=0.544 ms
>> 64 bytes from 192.168.200.2: icmp_seq=2 ttl=64 time=0.269 ms
>> 64 bytes from 192.168.200.2: icmp_seq=3 ttl=64 time=0.300 ms
>> 64 bytes from 192.168.200.2: icmp_seq=4 ttl=64 time=0.295 ms
>> 64 bytes from 192.168.200.2: icmp_seq=5 ttl=64 time=0.283 ms
>> 64 bytes from 192.168.200.2: icmp_seq=6 ttl=64 time=0.226 ms
>> 64 bytes from 192.168.200.2: icmp_seq=7 ttl=64 time=0.246 ms
>> 64 bytes from 192.168.200.2: icmp_seq=8 ttl=64 time=0.204 ms
>> 64 bytes from 192.168.200.2: icmp_seq=9 ttl=64 time=0.302 ms
>> 64 bytes from 192.168.200.2: icmp_seq=10 ttl=64 time=0.249 ms
>> 64 bytes from 192.168.200.2: icmp_seq=11 ttl=64 time=0.459 ms
>> 64 bytes from 192.168.200.2: icmp_seq=12 ttl=64 time=0.232 ms
>> 64 bytes from 192.168.200.2: icmp_seq=13 ttl=64 time=0.275 ms
>> 64 bytes from 192.168.200.2: icmp_seq=14 ttl=64 time=0.243 ms
>>
>> 2. USB HOST(OHCI)
>>
>> Connect an usb serial port on OTG port, nex t is the kernel log:
>>
>> [   27.824137] usb 2-1: new full-speed USB device number 2 using
>> ohci-platform
>> [   28.865504] cdc_acm 2-1:1.0: ttyACM0: USB ACM device
>> [   29.565509] cdc_acm 2-1:1.2: ttyACM1: USB ACM device
>>
>> 3. USB HOST(EHCI)
>>
>> Connect an usb storage on OTG port, next is the kernel log:
>>
>> [   17.754147] usb 1-1: new high-speed USB device number 2 using
>> ehci-platform
>> [   17.955995] usb-storage 1-1:1.0: USB Mass Storage device detected
>> [   18.024497] scsi host1: usb-storage 1-1:1.0
>> [   19.035091] scsi 1:0:0:0: Direct-Access     General  USB Flash
>> Disk   1.0  PQ: 0 ANSI: 2
>> [   19.049717] sd 1:0:0:0: [sda] 7831552 512-byte logical blocks:
>> (4.01 GB/3.73 GiB)
>> [   19.060873] sd 1:0:0:0: [sda] Write Protect is off
>> [   19.071018] sd 1:0:0:0: [sda] No Caching mode page found
>> [   19.076437] sd 1:0:0:0: [sda] Assuming drive cache: write through
>> [   19.093566]  sda: sda1
>> [   19.103492] sd 1:0:0:0: [sda] Attached SCSI removable disk
>>
>> issues:
>> =======
>>
>> The system power often turned off when I plugged an usb device into
>> the OTG port.
>> It's not clear why.
>>
>> qianfan Zhao (2):
>>    ARM: dts: sun8i-r40: Add USB0_OTG/HOST support
>>    ARM: dts: bananapi-m2-ultra: Enable USB0_OTG and HOST support
>>
>>   .../boot/dts/sun8i-r40-bananapi-m2-ultra.dts  | 39
>> +++++++++++++++++++
>>   arch/arm/boot/dts/sun8i-r40.dtsi              | 34 ++++++++++++++++
>>   2 files changed, 73 insertions(+)
>>

Re: [PATCH v4 1/2] ARM: dts: sun8i-r40: Add USB0_OTG/HOST support

[Samuel Holland]

Hi Qianfan,

On 5/18/22 5:17 AM, qianfanguijin@163.com wrote:
> From: qianfan Zhao <qianfanguijin@163.com>
> 
> The USB0 port of R40 is divided into two controllers, one is H3
> compatibled MUSB device, another is OHCI/EHCI.

typo: compatible

> 
> Signed-off-by: qianfan Zhao <qianfanguijin@163.com>
> ---
>  arch/arm/boot/dts/sun8i-r40.dtsi | 34 ++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/sun8i-r40.dtsi b/arch/arm/boot/dts/sun8i-r40.dtsi
> index 212e19183484..ae48474fdefa 100644
> --- a/arch/arm/boot/dts/sun8i-r40.dtsi
> +++ b/arch/arm/boot/dts/sun8i-r40.dtsi
> @@ -401,6 +401,21 @@ mmc3: mmc@1c12000 {
>  			#size-cells = <0>;
>  		};
>  
> +		usb_otg: usb@1c13000 {
> +			compatible = "allwinner,sun8i-r40-musb",

This compatible string needs to be documented in the binding[0] before you can
use it.

[0]: Documentation/devicetree/bindings/usb/allwinner,sun4i-a10-musb.yaml

> +				     "allwinner,sun8i-h3-musb";
> +			reg = <0x01c13000 0x0400>;
> +			clocks = <&ccu CLK_BUS_OTG>;
> +			resets = <&ccu RST_BUS_OTG>;
> +			interrupts = <GIC_SPI 38 IRQ_TYPE_LEVEL_HIGH>;
> +			interrupt-names = "mc";
> +			phys = <&usbphy 0>;
> +			phy-names = "usb";
> +			extcon = <&usbphy 0>;
> +			dr_mode = "otg";
> +			status = "disabled";
> +		};
> +
>  		usbphy: phy@1c13400 {
>  			compatible = "allwinner,sun8i-r40-usb-phy";
>  			reg = <0x01c13400 0x14>,
> @@ -427,6 +442,25 @@ usbphy: phy@1c13400 {
>  			#phy-cells = <1>;
>  		};
>  
> +		ehci0: usb@1c14000 {
> +			compatible = "allwinner,sun8i-r40-ehci", "generic-ehci";
> +			reg = <0x01c14000 0x100>;
> +			interrupts = <GIC_SPI 39 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&ccu CLK_BUS_EHCI0>, <&ccu CLK_BUS_OHCI0>;
> +			resets = <&ccu RST_BUS_EHCI0>, <&ccu RST_BUS_OHCI0>;
> +			status = "disabled";
> +		};
> +
> +		ohci0: usb@1c14400 {
> +			compatible = "allwinner,sun8i-r40-ohci", "generic-ohci";
> +			reg = <0x01c14400 0x100>;
> +			interrupts = <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&ccu CLK_BUS_EHCI0>, <&ccu CLK_BUS_OHCI0>,
> +				 <&ccu CLK_USB_OHCI0>;
> +			resets = <&ccu RST_BUS_EHCI0>, <&ccu RST_BUS_OHCI0>;

Are you sure the OHCI device requires the EHCI clocks/resets? Usually it is only
the other way around.

Regards,
Samuel

> +			status = "disabled";
> +		};
> +
>  		crypto: crypto@1c15000 {
>  			compatible = "allwinner,sun8i-r40-crypto";
>  			reg = <0x01c15000 0x1000>;
> 

Re: [PATCH v4 2/2] ARM: dts: bananapi-m2-ultra: Enable USB0_OTG and HOST support

[Samuel Holland]

Hi Qianfan,

On 5/18/22 5:17 AM, qianfanguijin@163.com wrote:
> From: qianfan Zhao <qianfanguijin@163.com>
> 
> let USB0 work at OTG mode.
> 
> Signed-off-by: qianfan Zhao <qianfanguijin@163.com>
> ---
>  .../boot/dts/sun8i-r40-bananapi-m2-ultra.dts  | 39 +++++++++++++++++++
>  1 file changed, 39 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/sun8i-r40-bananapi-m2-ultra.dts b/arch/arm/boot/dts/sun8i-r40-bananapi-m2-ultra.dts
> index 28197bbcb1d5..b3421e67967d 100644
> --- a/arch/arm/boot/dts/sun8i-r40-bananapi-m2-ultra.dts
> +++ b/arch/arm/boot/dts/sun8i-r40-bananapi-m2-ultra.dts
> @@ -122,6 +122,10 @@ &de {
>  	status = "okay";
>  };
>  
> +&ehci0 {
> +	status = "okay";
> +};
> +
>  &ehci1 {
>  	status = "okay";
>  };
> @@ -164,6 +168,7 @@ axp22x: pmic@34 {
>  		reg = <0x34>;
>  		interrupt-parent = <&nmi_intc>;
>  		interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
> +		x-powers,drive-vbus-en;
>  	};
>  };
>  
> @@ -199,6 +204,10 @@ &mmc2 {
>  	status = "okay";
>  };
>  
> +&ohci0 {
> +	status = "okay";
> +};
> +
>  &ohci1 {
>  	status = "okay";
>  };
> @@ -216,6 +225,15 @@ &pio {
>  	vcc-pe-supply = <&reg_eldo1>;
>  	vcc-pf-supply = <&reg_dcdc1>;
>  	vcc-pg-supply = <&reg_dldo1>;
> +
> +	/* USB0_DRVVBUS connected to both the PMIC.N_VBUSEN and PI13,
> +	 * we chose PMIC.N_VBUSEN for control, so set the gpio as
> +	 * input mode here.
> +	 */

Toggling a GPIO is going to be more efficient and have lower latency than a PMIC
register write over I2C. (And there are several comments in the USB PHY driver
about latency being important.) So I would prefer to model this as a
GPIO-controlled regulator, and leave N_VBUSEN as an input.

Regards,
Samuel

> +	usb0_vbus_enable_gpio: usb0-vbus-enable-gpio {
> +		pins = "PI13";
> +		function = "gpio_in";
> +	};
>  };
>  
>  &reg_aldo2 {
> @@ -298,6 +316,11 @@ &reg_dldo4 {
>  	regulator-name = "vdd2v5-sata";
>  };
>  
> +&reg_drivevbus {
> +	regulator-name = "usb0-vbus";
> +	status = "okay";
> +};
> +
>  &reg_eldo3 {
>  	regulator-min-microvolt = <1200000>;
>  	regulator-max-microvolt = <1200000>;
> @@ -333,7 +356,23 @@ bluetooth {
>  	};
>  };
>  
> +&usb_otg {
> +	dr_mode = "otg";
> +	status = "okay";
> +};
> +
> +&usb_power_supply {
> +	status = "okay";
> +};
> +
>  &usbphy {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&usb0_vbus_enable_gpio>;
> +
> +	usb0_id_det-gpios = <&pio 8 4 GPIO_ACTIVE_HIGH>; /* PI4 */
> +	usb0_vbus_det-gpios = <&pio 8 8 GPIO_ACTIVE_HIGH>; /* PI8 */
> +	usb0_vbus_power-supply = <&usb_power_supply>;
> +	usb0_vbus-supply = <&reg_drivevbus>;
>  	usb1_vbus-supply = <&reg_vcc5v0>;
>  	usb2_vbus-supply = <&reg_vcc5v0>;
>  	status = "okay";
> 

Re: [PATCH v4 0/2] ARM: sun8i-r40: Enable usb otg support

[Samuel Holland]

Hi Evgeny, Qianfan,

On 5/21/22 6:10 AM, Evgeny Boger wrote:
> my point is not that your patch is good enough.
> 
> My point is that it would be very difficult to maintain backward compatibility
> with this device tree once proper driver support is implemented.

Any solution to the problems below will have to maintain backward compatibility
with the current bindings anyway, for all of the other SoCs that are affected.
It is no more difficult to apply that same solution to R40. So I do not see a
benefit to delaying this series.

> On 5/21/22 07:26, qianfan wrote:
>>
>>
>> 在 2022/5/20 4:54, Evgeny Boger 写道:
>>> Hi qianfan,
>>>
>>> As Allwinner A40i user, let me first thank you for your effort for making
>>> better upstream support for R40!
>>>
>>> However, I would strongly suggest *not* to add USB support to one more
>>> Allwinner SoC in this particular way.
>>> The problem is, this approach consists of a number of carefully crafted hacks
>>> in device tree to make current drivers work on Allwinner hardware without
>>> modification to the drivers.
>>>
>>> a few examples:
>>>
>>> 1) please notice how ohci0 and ehci0 nodes do not contain reference to usb
>>> phy. It is done intentionally, otherwise EHCI will reset musb mode.
>>> Of course omitting phy reference here is also completely breaking power
>>> cycling in case of usb error and otherwise messes with a power management.
>>>
>>> 2) one must always enable ohci, ehci and usb_otg nodes at the same time. If
>>> one forgets to enable ohci/ehci nodes while enabling usb_otg node, the system
>>> will silently fail to work as USB host.
>>>
>>> 3) For host-only mode we still have to enable usb_otg node despite no role
>>> switching is needed. That's because phy reference is missing in ehci/ohci, so
>>> the ehci/ohci driver won't enable the PHY.
>>> Also I might be wrong, but I think phy won't be routed to ehci/ohci
>>> controllers is this case.
>>>
>>> 4) musb host controller is initialized and present to hardware though never
>>> actually used
>>>
>>> To summarize, not only the resulting device tree is not describing the
>>> hardware properly, it is creating device tree configuration which will be
>>> very hard to support in future, once proper driver support is in place.

All of these issues can be fixed either with no DT changes, or with only
backward-compatible changes like adding PHY references.

>> PHY setting is did in MUSB driver, so we need enable MUSB regardless of host
>> mode.
>>
>> I know your's point, OHCI/EHCI need do more works to init USBPHY, it shoule be
>> able to work
>> in dependently, but I don't have the ability to deal with these things right
>> now, I need
>> learn more things about OHCI/EHCI, that's a long-term goal.
>>
>> So now I need to make the whole usb work and do some tests as much as possible,
>> hoping to merge this patch into master. Some other optimizations can be made
>> later.

I am fine with merging this series, with the existing binding, and without
increasing the scope of the changes. It is still an improvement over the current
situation.

Regards,
Samuel

>> Thanks for yours guide.
>>>
>>>
>>> At Wiren Board kernel tree we tried to untangle this issue [1-6].
>>> Unfortunately I didn't have time to prepare it for kernel submission yet, but
>>> I think I better submit it as RFC to get a feedback from you and others.
>>>
>>>
>>> [1]
>>> https://github.com/wirenboard/linux/commit/359abbbd86ddff4d3c61179c882c286de32bb089
>>>
>>> [2]
>>> https://github.com/wirenboard/linux/commit/6327f9d7972c21b229fb83457fdde643b31553f9
>>>
>>> [3]
>>> https://github.com/wirenboard/linux/commit/f01f4c66758bde460a4d8c5b54ecee3b585c0232
>>>
>>> [4]
>>> https://github.com/wirenboard/linux/commit/c27598ad601e5a46f624b73412a531d6f1f63d37
>>>
>>> [5]
>>> https://github.com/wirenboard/linux/commit/5796d6eebb86b32a3751b2038b63af46f94954b3
>>>
>>> [6]
>>> https://github.com/wirenboard/linux/commit/0928a675d875f9c2849fd3a9888f718bbb673bda
>>>
>>>
>>>
>>
> 
> 

Re: [PATCH v4 0/2] ARM: sun8i-r40: Enable usb otg support

[Samuel Holland]

On 5/23/22 8:11 AM, Icenowy Zheng wrote:
> 在 2022-05-18星期三的 18:17 +0800，qianfanguijin@163.com写道：
>> From: qianfan Zhao <qianfanguijin@163.com>
>>
>> History:
>> =======
>>
>> v4(2022-05-18):
>> - Enable both musb and OHCI/EHCI support
>>
>> Tests:
>> ======
>>
>> All test cases were tested on bananapi-m2-ultra.
>>
>> 1. USB DEVICE(ping test)
>>
>> Enable usb gadget rndis network, ping m2u on ubuntu host:
> 
> Interestingly musb previous totally fail when I initially work on R40.
> Maybe some phy-sun4i-usb patches fixed it by accident?

I tested this series on my BPi M2U, and both host and device mode work fine. So
indeed whatever bug was there must have been fixed. Possibly it was e6f32efb1b12
("phy: sun4i-usb: Make sure to disable PHY0 passby for peripheral mode").

Tested-by: Samuel Holland <samuel@sholland.org>