[PATCH 0/2] usb: dwc2: gadget: fix stm32mp1 hang on barebox shutdown

[PATCH 0/2] usb: dwc2: gadget: fix stm32mp1 hang on barebox shutdown

[Ahmad Fatoum]

This applies on top of Jules' and Michael's latest dwc2 series.
With them, the dwc2 is usable in gadget mode on the stm32mp157c-dk2, but
boot/reset didn't work as barebox froze on shutdown.
This is fixed here.

Ahmad Fatoum (2):
  fixup! usb: dwc2: Add support for optional usb phy
  regulator: stm32-pwr: don't propagate regulator turn-off to supply

 drivers/regulator/stm32-pwr.c |  6 ++----
 drivers/usb/dwc2/dwc2.c       | 13 +++++++------
 2 files changed, 9 insertions(+), 10 deletions(-)

-- 
2.29.2


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

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

[Ahmad Fatoum]

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@5a006000.of: 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@pengutronix.de/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.

Cc: Jules Maselbas <jmaselbas@kalray.eu>
Signed-off-by: Ahmad Fatoum <ahmad@a3f.at>
---
 drivers/usb/dwc2/dwc2.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/dwc2/dwc2.c b/drivers/usb/dwc2/dwc2.c
index 65b92b542efc..8f7440471c31 100644
--- a/drivers/usb/dwc2/dwc2.c
+++ b/drivers/usb/dwc2/dwc2.c
@@ -78,12 +78,13 @@ static int dwc2_probe(struct device_d *dev)
 		goto clk_disable;
 	}
 
-	ret = phy_init(dwc2->phy);
+	ret = phy_power_on(dwc2->phy);
 	if (ret)
 		goto clk_disable;
-	ret = phy_power_on(dwc2->phy);
+
+	ret = phy_init(dwc2->phy);
 	if (ret)
-		goto err_phy_power;
+		goto phy_power_off;
 
 	ret = dwc2_check_core_version(dwc2);
 	if (ret)
@@ -119,9 +120,9 @@ static int dwc2_probe(struct device_d *dev)
 
 	return 0;
 error:
-	phy_power_off(dwc2->phy);
-err_phy_power:
 	phy_exit(dwc2->phy);
+phy_power_off:
+	phy_power_off(dwc2->phy);
 clk_disable:
 	clk_disable(dwc2->clk);
 clk_put:
@@ -139,8 +140,8 @@ static void dwc2_remove(struct device_d *dev)
 	dwc2_host_uninit(dwc2);
 	dwc2_gadget_uninit(dwc2);
 
-	phy_power_off(dwc2->phy);
 	phy_exit(dwc2->phy);
+	phy_power_off(dwc2->phy);
 }
 
 static const struct of_device_id dwc2_platform_dt_ids[] = {
-- 
2.29.2


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

[PATCH 2/2] regulator: stm32-pwr: don't propagate regulator turn-off to supply

[Ahmad Fatoum]

Linux walks up the regulator supply chain in the core code, which we
don't do in barebox. Instead the stm32-pwr driver did enable/disable its
parent manually. This is problematic, because it doesn't respect
the regulator-always-on flag the parent supply may have, leading to
prohibited disabling of regulators. Work around this by dropping the
regulator disable altogether and just enable the parent supply
unconditionally.

When we gain proper regulator supply chain handling, this patch can be
reverted.

Signed-off-by: Ahmad Fatoum <ahmad@a3f.at>
---
 drivers/regulator/stm32-pwr.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/regulator/stm32-pwr.c b/drivers/regulator/stm32-pwr.c
index 7df8074ba78e..4cb46b081abb 100644
--- a/drivers/regulator/stm32-pwr.c
+++ b/drivers/regulator/stm32-pwr.c
@@ -86,8 +86,6 @@ static int stm32_pwr_reg_enable(struct regulator_dev *rdev)
 	int ret;
 	u32 val;
 
-	regulator_enable(priv->supply);
-
 	val = readl(priv->base + REG_PWR_CR3);
 	val |= rdev->desc->enable_mask;
 	writel(val, priv->base + REG_PWR_CR3);
@@ -120,8 +118,6 @@ static int stm32_pwr_reg_disable(struct regulator_dev *rdev)
 		dev_err(rdev->dev, "%s: regulator disable timed out!\n",
 			desc->name);
 
-	regulator_disable(priv->supply);
-
 	return ret;
 }
 
@@ -194,6 +190,8 @@ static int stm32_pwr_regulator_probe(struct device_d *dev)
 				desc->name, ret);
 			goto release_region;
 		}
+
+		regulator_enable(priv->supply);
 	}
 
 	return 0;
-- 
2.29.2


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

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

[Sascha Hauer]

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@5a006000.of: 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@pengutronix.de/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.

Sascha

-- 
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 |

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

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

[Michael Grzeschik]

[-- Attachment #1.1: Type: text/plain, Size: 2185 bytes --]

Hi Ahmad!

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@5a006000.of: 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@pengutronix.de/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@49000000.of: dwc2_core_reset: Timeout! Waiting for Core Soft Reset
ERROR: dwc2 49000000.usb-otg@49000000.of: 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?
How can I reproduce the case this was fixing in the first place?

Rergards,
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 #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 149 bytes --]

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

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

[Ahmad Fatoum]

Hello Michael,

On 21.03.22 23:17, Michael Grzeschik wrote:
> Hi Ahmad!
> 
> 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@5a006000.of: 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@pengutronix.de/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@49000000.of: dwc2_core_reset: Timeout! Waiting for Core Soft Reset
> ERROR: dwc2 49000000.usb-otg@49000000.of: 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.

> 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.

Cheers,
Ahmad

> 
> Rergards,
> Michael
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox


-- 
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 |

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

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

[Michael Grzeschik]

[-- Attachment #1.1: Type: text/plain, Size: 2944 bytes --]

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@5a006000.of: 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@pengutronix.de/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@49000000.of: dwc2_core_reset: Timeout! Waiting for Core Soft Reset
>> ERROR: dwc2 49000000.usb-otg@49000000.of: 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.

>> 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.

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 #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 149 bytes --]

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

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

[Ahmad Fatoum]

Hello Michael,

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@5a006000.of: 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@pengutronix.de/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@49000000.of: dwc2_core_reset: Timeout! Waiting for Core Soft Reset
>>> ERROR: dwc2 49000000.usb-otg@49000000.of: 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?

Cheers,
Ahmad

> 
> 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 |

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

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

[Michael Grzeschik]

[-- Attachment #1.1: Type: text/plain, Size: 3548 bytes --]



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@5a006000.of: 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@pengutronix.de/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@49000000.of: dwc2_core_reset: Timeout! Waiting for Core Soft Reset
>>>> ERROR: dwc2 49000000.usb-otg@49000000.of: 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 #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 149 bytes --]

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

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

[Jules Maselbas]

Hi,

> > > > > > > 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@49000000.of: dwc2_core_reset: Timeout! Waiting for Core Soft Reset
> > > > > ERROR: dwc2 49000000.usb-otg@49000000.of: 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.

In linux driver dwc2 (and dwc3) call phy_poweron then phy_init, which I
believe is not the right order, it should be init -> poweron.
The thing is that's a two part issue, first the usb drivers using phy
must use the correct call order, secondly the phy drivers must implement
the correct "flow"







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