Re: [PATCH v3 1/3] phy-sun4i-usb: Fix sun8i_r40_cfg

[Evgeny Boger <boger@wirenboard.com>]

Hi,

I don't think that's the case.

According to my tests on custom Allwinner A40i board, the USB0 PHY works 
exactly as expected from its current configuration.

I.e. there are working ehci0/ohci0 at 1c14000/1c14400, which are working 
only if USB0 PHY is routed to EHCI/OHCI. In turn, musb stops working if 
we route USB0 to EHCI/OHCI.

Also, .enable_pmu_unk1 is needed as well.

So, the real issue here is that our current usb phy driver expects 
_both_ usbotg and ehci0+ohci0 nodes to be present and enabled so 
.phy0_dual_route works properly [1]. Attached please find the dtsi patch.

By the way, don't we need to improve PHY dual route handling?

I think the current behaviour is quite confusing. As one can see, USB on 
affected Allwinner SoCs doesn't work properly if only musb is enabled, 
as PHY is routed to non-initialized EHCI/OHCI. Surprisingly, USB won't 
also work if only ehci0/ohci0 are enabled, as usb phy is not referenced 
in ehci0/ohci0 nodes and thus won't be properly initialized. Musb's host 
controller is also exposed to the system, although it's never really 
used. I'm also afraid that current behaviour messes up with OTG dual 
role state machine, which is rarely used nowadays, but still.


[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=3ecc25e12f0e210d56fcca110a8144e50db05905

diff --git a/arch/arm/boot/dts/sun8i-r40.dtsi 
b/arch/arm/boot/dts/sun8i-r40.dtsi
index d5ad3b9efd12..98f598805867 100644
--- a/arch/arm/boot/dts/sun8i-r40.dtsi
+++ b/arch/arm/boot/dts/sun8i-r40.dtsi
@@ -363,6 +363,20 @@ mmc3: mmc@1c12000 {
                         #size-cells = <0>;
                 };

+               usb_otg: usb@1c13000 {
+                       compatible = "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>,
@@ -421,6 +435,25 @@ ahci: sata@1c18000 {
                         status = "disabled";
                 };

+               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>;
+                       resets = <&ccu RST_BUS_EHCI0>;
+                       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_OHCI0>,
+                                        <&ccu CLK_USB_OHCI0>;
+                       resets = <&ccu RST_BUS_OHCI0>;
+                       status = "disabled";
+               };
+
                 ehci1: usb@1c19000 {
                         compatible = "allwinner,sun8i-r40-ehci", 
"generic-ehci";
                         reg = <0x01c19000 0x100>;