Re: [PATCH 1/2] fixup! usb: dwc2: Add support for optional usb phy

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On Tue, Mar 22, 2022 at 09:25:12AM +0100, Ahmad Fatoum wrote:
On 22.03.22 09:19, Michael Grzeschik wrote:
On Tue, Mar 22, 2022 at 06:35:22AM +0100, Ahmad Fatoum wrote:
On 21.03.22 23:17, Michael Grzeschik wrote:
On Wed, Jan 06, 2021 at 11:05:43AM +0100, Sascha Hauer wrote:
On Mon, Dec 21, 2020 at 01:32:50AM +0100, Ahmad Fatoum wrote:
Linux doesn't seem to enforce a fixed order between phy_init and
phy_power_on. The Linux dwc2 driver does power_on and then phy_init,
which is the inverse of what barebox is currently doing.

The PHYs normally used with dwc2 are written with this in mind.
For example, our stm32-usbphyc driver fails to disable:

  ERROR: stm32-usbphyc 5a006000.usbphyc@xxxxxxxxxxx: PLL not reset
  ERROR: phy1: phy exit failed --> -5

Because Linux does exit -> power_off, but barebox does power_off ->
exit.

Issue was raised upstream:
https://lore.kernel.org/lkml/6cd01e79-fdc0-3bd4-32b5-a85142533f8a@xxxxxxxxxxxxxx/T/#t

Until this is settled, swap the order to follow what Linux does.
This is suboptimal, because it means controller drivers have different
order of the operations and that you can't combine arbitrary PHYs and
controllers, but it seems unlikely we will support combinations that
aren't supported by Linux in the first place anyway.

This is valuable information and I don't want it to be lost, so instead
of applying this as a fixup I rewrote the subject to:

usb: dwc2: swap order of phy_init and phy_power_on to what Linux does

and applied it as a separate patch.

With this patch applied, some stm32mp1 do seem to timeout when
addressing the core.

WARNING: dwc2 49000000.usb-otg@xxxxxxxxxxx: dwc2_core_reset: Timeout! Waiting for Core Soft Reset
ERROR: dwc2 49000000.usb-otg@xxxxxxxxxxx: probe failed: Connection timed out

When I revert this patch. Everything works fine again.

Is it possible that we can revert it in mainline now?

Since 0e37f94fbe1b ("phy: stm32: sync with upstream"), the phy-stm32-usbphyc now
has no poweron/poweroff callbacks, so a revert seems to only slightly change
timing. I'd suggest you dig some more.

That is not true. phy_power_on also enables the phy_regulator. Changing
this order has the effect that the regulator is enabled after or before
phy_init call.

Oh, good point.

How can I reproduce the case this was fixing in the first place?

The case was that barebox calls poweron/init in a different order than Linux.
This point is moot now though because poweron is no longer provided by the
driver.

Still not true, though.

Isn't this the same order Linux uses? Powering on regulator and then phy_init?
Why does it lead to issues in barebox, but not in Linux?

I have no clue so far. Especially as this seems to have implications on
the platform I am working on but not on the dk2. Although both use the
same st,stpmic1 node vdd_usb for the phy_regulator.

Regards,
Michael

--
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Attachment: signature.asc
Description: PGP signature

_______________________________________________
barebox mailing list
barebox@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/barebox

[Index of Archives]     [Linux Embedded]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux