diff mbox series

arm64: dts: verdin-imx8mm: add otg2 pd to usbphy

Message ID 20220722075600.10943-1-dev@pschenker.ch
State New
Headers show
Series arm64: dts: verdin-imx8mm: add otg2 pd to usbphy | expand

Commit Message

Philippe Schenker July 22, 2022, 7:55 a.m. UTC
From: Philippe Schenker <philippe.schenker@toradex.com>

The Verdin iMX8M Mini System on Module does not have VBUS signal
connected on Verdin USB_2 (usbotg2). On Verdin Development board this is
no problem, as we have connected a USB-Hub that is always connected.

However, if Verdin USB_2 is desired to be used as a single USB-Host port
the chipidea driver does not detect if a USB device is plugged into this
port, due to runtime pm shutting down the PHY.

Add the power-domain &pgc_otg2 to &usbphynop2 in order to detect
plugging events and enumerate the usb device.

Fixes: 6a57f224f734 ("arm64: dts: freescale: add initial support for verdin imx8m mini")
Signed-off-by: Philippe Schenker <philippe.schenker@toradex.com>

---

 arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi | 1 +
 1 file changed, 1 insertion(+)

Comments

Frieder Schrempf Aug. 1, 2022, 12:14 p.m. UTC | #1
+CC: Li Jun, Jacky Bai, Lucas Stach

Hi Philippe,

Am 22.07.22 um 09:55 schrieb Philippe Schenker:
> From: Philippe Schenker <philippe.schenker@toradex.com>
> 
> The Verdin iMX8M Mini System on Module does not have VBUS signal
> connected on Verdin USB_2 (usbotg2). On Verdin Development board this is
> no problem, as we have connected a USB-Hub that is always connected.
> 
> However, if Verdin USB_2 is desired to be used as a single USB-Host port
> the chipidea driver does not detect if a USB device is plugged into this
> port, due to runtime pm shutting down the PHY.
> 
> Add the power-domain &pgc_otg2 to &usbphynop2 in order to detect
> plugging events and enumerate the usb device.
> 
> Fixes: 6a57f224f734 ("arm64: dts: freescale: add initial support for verdin imx8m mini")
> Signed-off-by: Philippe Schenker <philippe.schenker@toradex.com>

I'm probably having the same issue on our hardware. There was a previous
attempt to fix this globally for all the i.MX8MM boards here: [1].

Unfortunately this didn't seem to work as intended in my case (see
discussion for that patch). Looking at your patch I wonder if not having
the vcc-supply for the usbphynop causes problems in my case. Do you
happen to know the effect of adding the regulator here? I don't see this
in any other i.MX8MM board devicetree.

Could you test Li's patch instead of this board specific fix and see if
it works for you? On your hardware, do you have an always-on device on
the usbotg1 port? If yes, does the detection on the usbotg2 port still
work if the usbotg1 port is disabled in the devicetree?

Thanks
Frieder

[1]
https://lore.kernel.org/linux-arm-kernel/f4879eed-79a7-3a1a-8dd0-c1a6ed367f34@kontron.de

> 
> ---
> 
>  arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi
> index eafa88d980b3..197da74837ca 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi
> @@ -737,6 +737,7 @@ &usbphynop1 {
>  };
>  
>  &usbphynop2 {
> +	power-domains = <&pgc_otg2>;
>  	vcc-supply = <&reg_vdd_3v3>;
>  };
>
Marcel Ziswiler Aug. 3, 2022, 9:26 p.m. UTC | #2
Hi Frieder

As Philippe is about to go on his well-deserved vacation he asked my to help answering your questions. As I was
also involved in the initial analysis of this issue I am happy to help.

On Mon, 2022-08-01 at 14:14 +0200, Frieder Schrempf wrote:
> +CC: Li Jun, Jacky Bai, Lucas Stach
> 
> Hi Philippe,
> 
> Am 22.07.22 um 09:55 schrieb Philippe Schenker:
> > From: Philippe Schenker <philippe.schenker@toradex.com>
> > 
> > The Verdin iMX8M Mini System on Module does not have VBUS

It's actually the ID pin we are talking about. VBUS is connected just fine.

> > signal
> > connected on Verdin USB_2 (usbotg2). On Verdin Development board this is
> > no problem, as we have connected a USB-Hub that is always connected.
> > 
> > However, if Verdin USB_2 is desired to be used as a single USB-Host port
> > the chipidea driver does not detect if a USB device is plugged into this
> > port, due to runtime pm shutting down the PHY.
> > 
> > Add the power-domain &pgc_otg2 to &usbphynop2 in order to detect
> > plugging events and enumerate the usb device.
> > 
> > Fixes: 6a57f224f734 ("arm64: dts: freescale: add initial support for verdin imx8m mini")
> > Signed-off-by: Philippe Schenker <philippe.schenker@toradex.com>
> 
> I'm probably having the same issue on our hardware.

Does your hardware also NOT connect the ID pin (sorry, Philippe initially confused it with the VBUS pin)?

BTW: According to a lengthy discussion under NDA with NXP the ID pin would need connecting even for host only
operation but our current PCB does really not allow for this to be easily done even just for verifying NXP's
claim.

> There was a previous
> attempt to fix this globally for all the i.MX8MM boards here: [1].
> 
> Unfortunately this didn't seem to work as intended in my case (see
> discussion for that patch). Looking at your patch I wonder if not having
> the vcc-supply for the usbphynop causes problems in my case. Do you
> happen to know the effect of adding the regulator here? I don't see this
> in any other i.MX8MM board devicetree.
> 
> Could you test Li's patch instead of this board specific fix and see if
> it works for you?

I read that and doing some more testing I just realised that detection does indeed not seem to work on usbotg1
neither, even though that one has the ID pin connected just fine! So it indeed seems to be a more generic
issue.

Okay, for me Li's patch actually fixed that detection issue on usbotg1. Yeah! That is on one of our carrier
boards either Dahlia or the Verdin development board where usbotg1 goes to a regular fully OTG resp.
device/host-switchable USB-C port (with ID and VBUS pins being connected). usbotg2 connects to an always
connected USB hub, so no detection issues ever there as that hub handles it just fine.

Now the issue Philippe's patch addresses is on a different custom carrier board where usbotg1 is a device-only
micro-USB port while usbotg2 is a regular USB-A host-only port. There, unfortunately, Li's patch does not fix
the issue. However, as stated above, according to NXP that ID pin would really need to be connected. So
basically this patch here from Philippe addresses an issue with our broken hardware to make detection work
regardless (by basically indirectly disabling auto-suspend).

> On your hardware, do you have an always-on device on
> the usbotg1 port?

No, as mentioned above, our Dahlia and Verdin development board have a regular fully OTG resp. device/host-
switchable USB-C port. While that custom carrier board has a device only micro-USB port.

> If yes, does the detection on the usbotg2 port still
> work if the usbotg1 port is disabled in the devicetree?

I believe this might be a separate issue recently also reported to us by a customer. However, most likely that
is/was on downstream and maybe even on a different i.MX 8 series based module, not quite sure. Let me check...

Anyway, for us on that custom carrier board detection does not even work in the regular case with usbotg1 being
available (albeit micro-USB device-mode only). Unfortunately, we don't have any carrier board where the full
range of USB tests could easily be conducted.

> Thanks
> Frieder

Thank you!

Cheers

Marcel

> [1]
> https://lore.kernel.org/linux-arm-kernel/f4879eed-79a7-3a1a-8dd0-c1a6ed367f34@kontron.de
> 
> > 
> > ---
> > 
> >  arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-
> > verdin.dtsi
> > index eafa88d980b3..197da74837ca 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi
> > @@ -737,6 +737,7 @@ &usbphynop1 {
> >  };
> >  
> >  &usbphynop2 {
> > +       power-domains = <&pgc_otg2>;
> >         vcc-supply = <&reg_vdd_3v3>;
> >  };
Marcel Ziswiler Aug. 18, 2022, 8:31 p.m. UTC | #3
Hi Frieder

On Thu, 2022-08-04 at 11:57 +0200, Frieder Schrempf wrote:
> Hi Marcel,
> 
> Am 03.08.22 um 23:26 schrieb Marcel Ziswiler:
> > Hi Frieder
> > 
> > As Philippe is about to go on his well-deserved vacation he asked my to help answering your questions. As I
> > was
> > also involved in the initial analysis of this issue I am happy to help.
> 
> Thanks for following up on this!

Welcome.

Philippe now modified one of our carrier boards to just route OTG2 straight to a USB-A receptacle rather than
going through a USB hub. That way I was now able to at least test without an always connected OTG2.

> > On Mon, 2022-08-01 at 14:14 +0200, Frieder Schrempf wrote:
> > > +CC: Li Jun, Jacky Bai, Lucas Stach
> > > 
> > > Hi Philippe,
> > > 
> > > Am 22.07.22 um 09:55 schrieb Philippe Schenker:
> > > > From: Philippe Schenker <philippe.schenker@toradex.com>
> > > > 
> > > > The Verdin iMX8M Mini System on Module does not have VBUS
> > 
> > It's actually the ID pin we are talking about. VBUS is connected just fine.

Philippe posted an updated patch here [1].

> > > > signal
> > > > connected on Verdin USB_2 (usbotg2). On Verdin Development board this is
> > > > no problem, as we have connected a USB-Hub that is always connected.
> > > > 
> > > > However, if Verdin USB_2 is desired to be used as a single USB-Host port
> > > > the chipidea driver does not detect if a USB device is plugged into this
> > > > port, due to runtime pm shutting down the PHY.
> > > > 
> > > > Add the power-domain &pgc_otg2 to &usbphynop2 in order to detect
> > > > plugging events and enumerate the usb device.
> > > > 
> > > > Fixes: 6a57f224f734 ("arm64: dts: freescale: add initial support for verdin imx8m mini")
> > > > Signed-off-by: Philippe Schenker <philippe.schenker@toradex.com>
> > > 
> > > I'm probably having the same issue on our hardware.
> > 
> > Does your hardware also NOT connect the ID pin (sorry, Philippe initially confused it with the VBUS pin)?
> 
> We have both (same SoM):
> 
> Kontron BL i.MX8MM:
> * USB1 connected to Micro-B Port with ID pin (full OTG)
> * USB2 connected to always-on onboard Ethernet adapter
> 
> Customer Board:
> * USB1 connected to USB-A Port, ID-Pin floating (host only)
> * USB2 connected to always-on onboard HUB
> 
> > 
> > BTW: According to a lengthy discussion under NDA with NXP the ID pin would need connecting even for host
> > only
> > operation but our current PCB does really not allow for this to be easily done even just for verifying
> > NXP's
> > claim.
> 
> Hm, that's news to me. So far I can't remember having problems with
> designs that use the OTG port in host-only mode and leave the ID pin
> unconnected.

Yes, the same with us and, remember, we are not a hundred percent convinced that what NXP told us here is
really the truth. However, we are unable to validate that claim right now.

> > > There was a previous
> > > attempt to fix this globally for all the i.MX8MM boards here: [1].
> > > 
> > > Unfortunately this didn't seem to work as intended in my case (see
> > > discussion for that patch). Looking at your patch I wonder if not having
> > > the vcc-supply for the usbphynop causes problems in my case. Do you
> > > happen to know the effect of adding the regulator here? I don't see this
> > > in any other i.MX8MM board devicetree.
> > > 
> > > Could you test Li's patch instead of this board specific fix and see if
> > > it works for you?
> > 
> > I read that and doing some more testing I just realised that detection does indeed not seem to work on
> > usbotg1
> > neither, even though that one has the ID pin connected just fine! So it indeed seems to be a more generic
> > issue.
> 
> Yes, I think this is unrelated to the ID pin.

With that new modified carrier board I can confirm that neither OTG1 nor OTG2 work unless a USB device is
connected during boot. Any later auto-detection fails.

> > Okay, for me Li's patch actually fixed that detection issue on usbotg1. Yeah! That is on one of our carrier
> > boards either Dahlia or the Verdin development board where usbotg1 goes to a regular fully OTG resp.
> > device/host-switchable USB-C port (with ID and VBUS pins being connected). usbotg2 connects to an always
> > connected USB hub, so no detection issues ever there as that hub handles it just fine.
> 
> This is similar to the hw setup on our BL i.MX8MM board and your results
> match with mine in that Li's patch *appears* to fix the issue. In fact
> when you disable usbotg2 (status = "disabled") where the hub is
> connected you will probably see, that detection on usbotg1 stops working
> again which tells us there's something wrong with a resource that is
> shared by the two ports (clock, reset, power-domain, etc.)

Yes, I agree. Despite Li's patch and all genpds being on auto-detection still fails:

root@verdin-imx8mm-07028378:~# cat /sys/kernel/debug/pm_genpd/usb-otg1/current_state
on
root@verdin-imx8mm-07028378:~# cat /sys/kernel/debug/pm_genpd/hsiomix/current_state
on
root@verdin-imx8mm-07028378:~# cat /sys/kernel/debug/pm_genpd/usb-otg2/current_state
on

> > Now the issue Philippe's patch addresses is on a different custom carrier board where usbotg1 is a device-
> > only
> > micro-USB port while usbotg2 is a regular USB-A host-only port. There, unfortunately, Li's patch does not
> > fix
> > the issue.
> 
> I think this is due to the fact that on this hw there's no always-on
> device on the other port. So whatever shared resource is needed for
> autodetection to work gets disabled when all ports are autosuspended.

Yes, exactly.

> > However, as stated above, according to NXP that ID pin would really need to be connected. So
> > basically this patch here from Philippe addresses an issue with our broken hardware to make detection work
> > regardless (by basically indirectly disabling auto-suspend).
> 
> So if Phillipe's patch fixes the issue for your customer board, but Li's
> patch doesn't, this means that the HSIOMIX needs to be kept enabled
> while suspending the port!?

Yes, but even that seems not enough (see above sysfs genpd state dump).


> This is strange because I think I tried this on my board and it still
> failed as soon as the other port was not unsuspended/enabled anymore.
> 
> And I don't see how it's related to the missing ID pin connection.

Yeah, well, we also don't really see that.

> > > On your hardware, do you have an always-on device on
> > > the usbotg1 port?
> > 
> > No, as mentioned above, our Dahlia and Verdin development board have a regular fully OTG resp. device/host-
> > switchable USB-C port. While that custom carrier board has a device only micro-USB port.
> 
> I was asking because of what I explained above. If the "other" port is
> not kept out of autosuspend by some always-on device, than Li's fix
> doesn't work for some reason.

Agreed.


> > > If yes, does the detection on the usbotg2 port still
> > > work if the usbotg1 port is disabled in the devicetree?
> > 
> > I believe this might be a separate issue recently also reported to us by a customer. However, most likely
> > that
> > is/was on downstream and maybe even on a different i.MX 8 series based module, not quite sure. Let me
> > check...
> 
> For me it looks like this is exactly what this issue is about and what
> prevents us from applying Li's patch as a global fix for the
> autodetection issue.

Yes, agreed. Fact is that somehow this all works in NXP's downstream stuff even in their latest 5.15.32_2.0.0
(which BTW requires a later downstream ATF where they likely hacked something in, at least with the previous
ATF version it just hangs at USB initialization during boot) we just tried. So please NXP could you reveal what
exactly is going on here? Thanks!

> > Anyway, for us on that custom carrier board detection does not even work in the regular case with usbotg1
> > being
> > available (albeit micro-USB device-mode only). Unfortunately, we don't have any carrier board where the
> > full
> > range of USB tests could easily be conducted

At least we now have one where there aren't any always on USB devices. But OTG2 does neither have its ID pin
exposed nor is it really device/host switchable.

> Thanks
> Frieder

[1] https://lore.kernel.org/all/20220811140738.96348-1-dev@pschenker.ch/

Cheers

Marcel
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi b/arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi
index eafa88d980b3..197da74837ca 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm-verdin.dtsi
@@ -737,6 +737,7 @@  &usbphynop1 {
 };
 
 &usbphynop2 {
+	power-domains = <&pgc_otg2>;
 	vcc-supply = <&reg_vdd_3v3>;
 };