linux-sunxi.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
From: Evgeny Boger <boger@wirenboard.com>
To: qianfanguijin@163.com
Cc: andre.przywara@arm.com, jernej.skrabec@gmail.com,
	linux-sunxi@lists.linux.dev, mripard@kernel.org, wens@csie.org,
	Icenowy Zheng <icenowy@aosc.xyz>
Subject: Re: [PATCH v3 1/3] phy-sun4i-usb: Fix sun8i_r40_cfg
Date: Sat, 10 Jul 2021 20:06:41 +0300	[thread overview]
Message-ID: <8021df40-f2dc-a2d5-7d6c-ad6e248c6835@wirenboard.com> (raw)
In-Reply-To: <20210705033104.7824-1-qianfanguijin@163.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>;


      parent reply	other threads:[~2021-07-10 17:06 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-05  3:31 qianfanguijin
2021-07-05  3:31 ` [PATCH v3 2/3] ARM: dts: sun8i: r40: Add usb_otg device node qianfanguijin
2021-07-05  3:31 ` [PATCH v3 3/3] ARM: dts: sun8i: r40: bananapi-m2-ultra: Enable usb_otg qianfanguijin
2021-07-10 17:06 ` Evgeny Boger [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8021df40-f2dc-a2d5-7d6c-ad6e248c6835@wirenboard.com \
    --to=boger@wirenboard.com \
    --cc=andre.przywara@arm.com \
    --cc=icenowy@aosc.xyz \
    --cc=jernej.skrabec@gmail.com \
    --cc=linux-sunxi@lists.linux.dev \
    --cc=mripard@kernel.org \
    --cc=qianfanguijin@163.com \
    --cc=wens@csie.org \
    --subject='Re: [PATCH v3 1/3] phy-sun4i-usb: Fix sun8i_r40_cfg' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
on how to clone and mirror all data and code used for this inbox