Re: [PATCH V3 6/8] ARM: dts: sun8i: v3s: add EHCI and OHCI to v3s dts

On Fri, 25 Aug 2023 10:58:00 -0500
Chris Morgan [off-list ref] wrote:

Hi Chris,

On Thu, Aug 24, 2023 at 10:21:50PM +0100, Andre Przywara wrote:

quoted

On Wed, 23 Aug 2023 16:25:52 -0500
Chris Morgan [off-list ref] wrote:

Hi Chris,
  

quoted

From: Chris Morgan <redacted>

Add the EHCI and OHCI controller to the Allwinner v3s to support using
USB in host mode.  

Alright, so was it really that easy? I was afraid we left out host mode
for a reason back then ....  

Can't speak to that, but it is working for me as both a host and a
gadget.

quoted

quoted

Signed-off-by: Chris Morgan <redacted>
---
 arch/arm/boot/dts/allwinner/sun8i-v3s.dtsi | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/arch/arm/boot/dts/allwinner/sun8i-v3s.dtsi b/arch/arm/boot/dts/allwinner/sun8i-v3s.dtsi
index c87476ea31e2..eb63dd274305 100644
--- a/arch/arm/boot/dts/allwinner/sun8i-v3s.dtsi
+++ b/arch/arm/boot/dts/allwinner/sun8i-v3s.dtsi

@@ -319,6 +319,25 @@ usbphy: phy@1c19400 {
 			#phy-cells = <1>;
 		};
 
+		ehci: usb@1c1a000 {
+			compatible = "allwinner,sun8i-v3s-ehci", "generic-ehci";
+			reg = <0x01c1a000 0x100>;
+			interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&ccu CLK_BUS_EHCI0>, <&ccu CLK_BUS_OHCI0>;
+			resets = <&ccu RST_BUS_EHCI0>, <&ccu RST_BUS_OHCI0>;  

Please add the link the PHY here:
			phys = <&usbphy 0>;
			phy-names = "usb";  

usbphy 0 doesn't work, but if I do phys = <&usbphy 1>; for here and the
OHCI node it does work. Is that expected/acceptable?

Well, that means it doesn't work, because the V3s has only one PHY, so
"1" is invalid, and will either be ignored or rejected (any hints in
dmesg?)

By "doesn't work"
I mean the device is always in HOST mode if I use 0 and switches
correctly (the extcon shows HOST=0 when unplugged or plugged into my
computer, and shows HOST=1 if I plug in a device which I assume is
the desired behavior).

You mean the latter part was with <usbphy 1>, and that's the same
behaviour as without the property?
That would mean that this part is still broken - I think somewhere in
the PHY driver. But this is a generic issue, and not specific to the
V3s, and your patch is just revealing it.

I will try to have a look in the next few days, to find the real
culprit.

Cheers,
Andre

quoted

quoted

+			status = "disabled";
+		};
+
+		ohci: usb@1c1a400 {
+			compatible = "allwinner,sun8i-v3s-ohci", "generic-ohci";
+			reg = <0x01c1a400 0x100>;
+			interrupts = <GIC_SPI 73 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>;  

... and here.

That would be definitely the right thing to do, but please check
whether it still works, especially OTG operation.

The addresses, clocks, resets and interrupts match the manual.

Cheers,
Andre.

  

quoted

+			status = "disabled";
+		};
+
 		ccu: clock@1c20000 {
 			compatible = "allwinner,sun8i-v3s-ccu";
 			reg = <0x01c20000 0x400>;  

Thank you,
Chris