[PATCH] phy: stm32-usphyc: add mdelay(1) to fix timeout on some machines

[PATCH] phy: stm32-usphyc: add mdelay(1) to fix timeout on some machines

[Michael Grzeschik]

An mdelay of 1 seems to be necessary on some machines, since
the monsel status does not seem to be accurate. On rare occasions just
working with the phy after this pll check lead to no functional usb.
With this short mdelay this issue was not reported again.

Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
 drivers/phy/st/phy-stm32-usbphyc.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/phy/st/phy-stm32-usbphyc.c b/drivers/phy/st/phy-stm32-usbphyc.c
index 5bb9647b078f12..c452a0caceb9fa 100644
--- a/drivers/phy/st/phy-stm32-usbphyc.c
+++ b/drivers/phy/st/phy-stm32-usbphyc.c
@@ -353,6 +353,15 @@ static int stm32_usbphyc_phy_init(struct phy *phy)
 		goto pll_disable;
 	}
 
+	/* This mdelay seems to be necessary on some machines, since the
+	 * monsel status does not seem to be accurate. On rare occasions
+	 * just working with the phy after this pll check the usb
+	 * peripheral (e.g. on the dwc2) run into timeout issues and
+	 * leading to no functional usb. With this short mdelay this
+	 * issue was not reported again.
+	 */
+	mdelay(1);
+
 	usbphyc_phy->active = true;
 
 	return 0;
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] phy: stm32-usphyc: add mdelay(1) to fix timeout on some machines

[Fabrice Gasnier]

On 1/24/23 21:45, Michael Grzeschik wrote:
> An mdelay of 1 seems to be necessary on some machines, since

Hi Michael,

Could you precise on which board ?

> the monsel status does not seem to be accurate. On rare occasions just
> working with the phy after this pll check lead to no functional usb.

Could you elaborate on the symptoms (provide some error logs) ?

> With this short mdelay this issue was not reported again.
> 
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
> ---
>  drivers/phy/st/phy-stm32-usbphyc.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/phy/st/phy-stm32-usbphyc.c b/drivers/phy/st/phy-stm32-usbphyc.c
> index 5bb9647b078f12..c452a0caceb9fa 100644
> --- a/drivers/phy/st/phy-stm32-usbphyc.c
> +++ b/drivers/phy/st/phy-stm32-usbphyc.c
> @@ -353,6 +353,15 @@ static int stm32_usbphyc_phy_init(struct phy *phy)
>  		goto pll_disable;
>  	}
>  
> +	/* This mdelay seems to be necessary on some machines, since the
> +	 * monsel status does not seem to be accurate. On rare occasions
> +	 * just working with the phy after this pll check the usb
> +	 * peripheral (e.g. on the dwc2) run into timeout issues and
> +	 * leading to no functional usb. With this short mdelay this
> +	 * issue was not reported again.
> +	 */
> +	mdelay(1);
> +

The USBPHYC provides two PHY interfaces:
- PHY port#1 is for USBH
- PHY port#2 can be assigned to USBH or DWC2.

Adding some delay here, we'll hit twice this penalty (e.g. for USBH +
DWC2 or dual USBH) on all MP1 (MP13 & MP15) platforms.

Could you try to narrow down the issue by adding some debug log in:
- stm32_usbphyc_phy_init
- stm32_usbphyc_clk48_prepare

Best Regards,
Fabrice

>  	usbphyc_phy->active = true;
>  
>  	return 0;

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] phy: stm32-usphyc: add mdelay(1) to fix timeout on some machines

[Michael Grzeschik]

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

Hi Fabrice,

On Wed, Feb 01, 2023 at 05:20:11PM +0100, Fabrice Gasnier wrote:
>On 1/24/23 21:45, Michael Grzeschik wrote:
>> An mdelay of 1 seems to be necessary on some machines, since
>
>Hi Michael,
>
>Could you precise on which board ?

It was reproducible on one of our lxa-mc1 boards.

>> the monsel status does not seem to be accurate. On rare occasions just
>> working with the phy after this pll check lead to no functional usb.
>
>Could you elaborate on the symptoms (provide some error logs) ?

The symptom was, that the dwc2 controller was not resuming from polling
on the soft reset done bit and so ran into a timeout. (dwc2: GRSTCTL_CSFTRST(GRSTCTL_CSFTRST_DONE))

While debugging we found that the stm vendor uboot phy driver
was just adding an predefined wait of 200us after setting
the pll.

https://github.com/STMicroelectronics/u-boot/blob/v2021.10-stm32mp/drivers/phy/phy-stm32-usbphyc.c#L257

The comment for the PLL_INIT_TIME_US says that the value is based on the
possible max pll lock latency and an additional phy init delay:

  /* max 100 us for PLL lock and 100 us for PHY init */
  #define PLL_INIT_TIME_US	200

Reading the datasheet of the stm32mp1, the 100 us sure come from
tLOCK in Table 41. USB_PLL characteristics. (Page 159)

https://www.st.com/resource/en/datasheet/stm32mp157c.pdf


In the mainline Kernel we have the set of lock monitor
bits that are actually undefined in any register specs
we read so far.

  /* STM32_USBPHYC_MONITOR bit fields */
  #define STM32_USBPHYC_MON_OUT   GENMASK(3, 0)
  #define STM32_USBPHYC_MON_SEL   GENMASK(8, 4)
  #define STM32_USBPHYC_MON_SEL_LOCKP 0x1F
  #define STM32_USBPHYC_MON_OUT_LOCKP BIT(3)

So this will bring the maximum lock delay to its real
delay.

But for the additional phy init delay, we are unsure where the 100us are
coming from. So we could not really tell if the value makes sense at
all. The assumption was to increase the delay, just to make sure that
this will solve the issue for good.

>> With this short mdelay this issue was not reported again.
>>
>> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
>> ---
>>  drivers/phy/st/phy-stm32-usbphyc.c | 9 +++++++++
>>  1 file changed, 9 insertions(+)
>>
>> diff --git a/drivers/phy/st/phy-stm32-usbphyc.c b/drivers/phy/st/phy-stm32-usbphyc.c
>> index 5bb9647b078f12..c452a0caceb9fa 100644
>> --- a/drivers/phy/st/phy-stm32-usbphyc.c
>> +++ b/drivers/phy/st/phy-stm32-usbphyc.c
>> @@ -353,6 +353,15 @@ static int stm32_usbphyc_phy_init(struct phy *phy)
>>  		goto pll_disable;
>>  	}
>>
>> +	/* This mdelay seems to be necessary on some machines, since the
>> +	 * monsel status does not seem to be accurate. On rare occasions
>> +	 * just working with the phy after this pll check the usb
>> +	 * peripheral (e.g. on the dwc2) run into timeout issues and
>> +	 * leading to no functional usb. With this short mdelay this
>> +	 * issue was not reported again.
>> +	 */
>> +	mdelay(1);
>> +
>
>The USBPHYC provides two PHY interfaces:
>- PHY port#1 is for USBH
>- PHY port#2 can be assigned to USBH or DWC2.
>
>Adding some delay here, we'll hit twice this penalty (e.g. for USBH +
>DWC2 or dual USBH) on all MP1 (MP13 & MP15) platforms.
>
>Could you try to narrow down the issue by adding some debug log in:
>- stm32_usbphyc_phy_init
>- stm32_usbphyc_clk48_prepare

The whole issue is actually very easy to reproduce in the bootloader.
So we already mainlined this patch to the barebox bootloader.

So this patch is actually a port from the bootloader to make sure
that the kernel does not run into the issue as well.

I bet your team at st can figure out a more reasonable value for the
delay.

>>  	usbphyc_phy->active = true;
>>
>>  	return 0;
>

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: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] phy: stm32-usphyc: add mdelay(1) to fix timeout on some machines

[Fabrice Gasnier]

On 2/13/23 15:59, Michael Grzeschik wrote:
> Hi Fabrice,
> 
> On Wed, Feb 01, 2023 at 05:20:11PM +0100, Fabrice Gasnier wrote:
>> On 1/24/23 21:45, Michael Grzeschik wrote:
>>> An mdelay of 1 seems to be necessary on some machines, since
>>
>> Hi Michael,
>>
>> Could you precise on which board ?
> 
> It was reproducible on one of our lxa-mc1 boards.
> 
>>> the monsel status does not seem to be accurate. On rare occasions just
>>> working with the phy after this pll check lead to no functional usb.
>>
>> Could you elaborate on the symptoms (provide some error logs) ?
> 
> The symptom was, that the dwc2 controller was not resuming from polling
> on the soft reset done bit and so ran into a timeout. (dwc2:
> GRSTCTL_CSFTRST(GRSTCTL_CSFTRST_DONE))
> 
> While debugging we found that the stm vendor uboot phy driver
> was just adding an predefined wait of 200us after setting
> the pll.
> 
> https://github.com/STMicroelectronics/u-boot/blob/v2021.10-stm32mp/drivers/phy/phy-stm32-usbphyc.c#L257
> 
> The comment for the PLL_INIT_TIME_US says that the value is based on the
> possible max pll lock latency and an additional phy init delay:
> 
>  /* max 100 us for PLL lock and 100 us for PHY init */
>  #define PLL_INIT_TIME_US    200
> 
> Reading the datasheet of the stm32mp1, the 100 us sure come from
> tLOCK in Table 41. USB_PLL characteristics. (Page 159)
> 
> https://www.st.com/resource/en/datasheet/stm32mp157c.pdf
> 
> 
> In the mainline Kernel we have the set of lock monitor
> bits that are actually undefined in any register specs
> we read so far.
> 
>  /* STM32_USBPHYC_MONITOR bit fields */
>  #define STM32_USBPHYC_MON_OUT   GENMASK(3, 0)
>  #define STM32_USBPHYC_MON_SEL   GENMASK(8, 4)
>  #define STM32_USBPHYC_MON_SEL_LOCKP 0x1F
>  #define STM32_USBPHYC_MON_OUT_LOCKP BIT(3)
> 
> So this will bring the maximum lock delay to its real
> delay.


Hi Michael,

Thanks for all detailed explanation.
I asked some improvements in the documentations in this area.

> 
> But for the additional phy init delay, we are unsure where the 100us are
> coming from. So we could not really tell if the value makes sense at
> all. The assumption was to increase the delay, just to make sure that
> this will solve the issue for good.
> 
>>> With this short mdelay this issue was not reported again.
>>>
>>> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
>>> ---
>>>  drivers/phy/st/phy-stm32-usbphyc.c | 9 +++++++++
>>>  1 file changed, 9 insertions(+)
>>>
>>> diff --git a/drivers/phy/st/phy-stm32-usbphyc.c
>>> b/drivers/phy/st/phy-stm32-usbphyc.c
>>> index 5bb9647b078f12..c452a0caceb9fa 100644
>>> --- a/drivers/phy/st/phy-stm32-usbphyc.c
>>> +++ b/drivers/phy/st/phy-stm32-usbphyc.c
>>> @@ -353,6 +353,15 @@ static int stm32_usbphyc_phy_init(struct phy *phy)
>>>          goto pll_disable;
>>>      }
>>>
>>> +    /* This mdelay seems to be necessary on some machines, since the
>>> +     * monsel status does not seem to be accurate. On rare occasions
>>> +     * just working with the phy after this pll check the usb
>>> +     * peripheral (e.g. on the dwc2) run into timeout issues and
>>> +     * leading to no functional usb. With this short mdelay this
>>> +     * issue was not reported again.
>>> +     */
>>> +    mdelay(1);
>>> +
>>
>> The USBPHYC provides two PHY interfaces:
>> - PHY port#1 is for USBH
>> - PHY port#2 can be assigned to USBH or DWC2.
>>
>> Adding some delay here, we'll hit twice this penalty (e.g. for USBH +
>> DWC2 or dual USBH) on all MP1 (MP13 & MP15) platforms.
>>
>> Could you try to narrow down the issue by adding some debug log in:
>> - stm32_usbphyc_phy_init
>> - stm32_usbphyc_clk48_prepare
> 
> The whole issue is actually very easy to reproduce in the bootloader.
> So we already mainlined this patch to the barebox bootloader.
> 
> So this patch is actually a port from the bootloader to make sure
> that the kernel does not run into the issue as well.

Kernel and u-boot drivers are both registering:
- a clock provider
  clk_prepare ... -> stm32_usbphyc_clk48_prepare()
  Currently, there's no wait loop NOR delay here

- a PHY provider (2 PHYs, e.g. 2 ports)
  additional mdelay(1) will be hit twice, one time for DWC2, one for
EHCI/OHCI

Both in kernel and u-boot the clk_prepare() likely happens 1st, then
phy_init().

This is still unclear to me why an additional delay in PHY init, would
be necessary in your environment, with the kernel. Is there a real issue
in kernel ? Or is it more a caution, theoretical port from barebox ?

Still, If needed, I'd advice the delay to be applied only once, e.g. in
stm32_usbphyc_pll_enable(), just after PLLEN bit has been set (as in
u-boot). And rely on MON bits for each PHY init.

Regarding mdelay(): It would also be better to adopt usleep_range()
instead of busy loop.

 	stm32_usbphyc_set_bits(pll_reg, PLLEN);

+	/* Wait for maximum lock time */
+	usleep_range(200, 300);
+
 	return 0;

This way, delay will be hit upon clk_prepare, then polling the MON bits
in phy_init() may become a no-op. So, the delay would be hit only once
(instead of two times). If ever the phy_init() comes first in the
future, only the 1st call to one of the APIs will hit this delay.

Could you check if this solves the issue at your end ?

I had a look at barebox driver, same logic could apply there even if no
clock provider is registered.

> 
> I bet your team at st can figure out a more reasonable value for the
> delay.

I'd recommend 200µs (min) as in u-boot.

Regards,
Fabrice
> 
>>>      usbphyc_phy->active = true;
>>>
>>>      return 0;
>>
> 
> Regards,
> Michael
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2] phy: stm32-usphyc: add 200 to 300 us delay to fix timeout on some machines

[Michael Grzeschik]

An minimum udelay of 200 us seems to be necessary on some machines. After
the setup of the pll, which needs about 100 us to be locked there seem
to be additional 100 us to get the phy really functional. Without this
delay the usb runs not functional. With this additional short udelay
this issue was not reported again.

Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>

---
v1 -> v2: - changed the mdelay to udelay_range(200, 300), like suggested by fabrice
          - moved the delay to pll enable so it will only be triggered once

 drivers/phy/st/phy-stm32-usbphyc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/phy/st/phy-stm32-usbphyc.c b/drivers/phy/st/phy-stm32-usbphyc.c
index 5bb9647b078f12..dd469f57fba7eb 100644
--- a/drivers/phy/st/phy-stm32-usbphyc.c
+++ b/drivers/phy/st/phy-stm32-usbphyc.c
@@ -317,6 +317,9 @@ static int stm32_usbphyc_pll_enable(struct stm32_usbphyc *usbphyc)
 
 	stm32_usbphyc_set_bits(pll_reg, PLLEN);
 
+	/* Wait for maximum lock time */
+	usleep_range(200, 300);
+
 	return 0;
 
 reg_disable:
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2] phy: stm32-usphyc: add 200 to 300 us delay to fix timeout on some machines

[Fabrice Gasnier]

On 2/27/23 16:13, Michael Grzeschik wrote:
> An minimum udelay of 200 us seems to be necessary on some machines. After
> the setup of the pll, which needs about 100 us to be locked there seem
> to be additional 100 us to get the phy really functional. Without this
> delay the usb runs not functional. With this additional short udelay
> this issue was not reported again.
> 
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
> 

Hi Michael,

Thank you for the updates,

Fell free to add my:
Reviewed-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>

Best Regards,
Fabrice

> ---
> v1 -> v2: - changed the mdelay to udelay_range(200, 300), like suggested by fabrice
>           - moved the delay to pll enable so it will only be triggered once
> 
>  drivers/phy/st/phy-stm32-usbphyc.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/phy/st/phy-stm32-usbphyc.c b/drivers/phy/st/phy-stm32-usbphyc.c
> index 5bb9647b078f12..dd469f57fba7eb 100644
> --- a/drivers/phy/st/phy-stm32-usbphyc.c
> +++ b/drivers/phy/st/phy-stm32-usbphyc.c
> @@ -317,6 +317,9 @@ static int stm32_usbphyc_pll_enable(struct stm32_usbphyc *usbphyc)
>  
>  	stm32_usbphyc_set_bits(pll_reg, PLLEN);
>  
> +	/* Wait for maximum lock time */
> +	usleep_range(200, 300);
> +
>  	return 0;
>  
>  reg_disable:

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2] phy: stm32-usphyc: add 200 to 300 us delay to fix timeout on some machines

[Michael Grzeschik]

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

Hi Fabrice,

On Tue, Feb 28, 2023 at 06:28:21PM +0100, Fabrice Gasnier wrote:
>On 2/27/23 16:13, Michael Grzeschik wrote:
>> An minimum udelay of 200 us seems to be necessary on some machines. After
>> the setup of the pll, which needs about 100 us to be locked there seem
>> to be additional 100 us to get the phy really functional. Without this
>> delay the usb runs not functional. With this additional short udelay
>> this issue was not reported again.
>>
>> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
>>
>
>Hi Michael,
>
>Thank you for the updates,
>
>Fell free to add my:
>Reviewed-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>

Thanks!

Trhough which tree will this be picked?
Wil it be possible to add this to v6.3?

Regards,
Michael

>> ---
>> v1 -> v2: - changed the mdelay to udelay_range(200, 300), like suggested by fabrice
>>           - moved the delay to pll enable so it will only be triggered once
>>
>>  drivers/phy/st/phy-stm32-usbphyc.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/phy/st/phy-stm32-usbphyc.c b/drivers/phy/st/phy-stm32-usbphyc.c
>> index 5bb9647b078f12..dd469f57fba7eb 100644
>> --- a/drivers/phy/st/phy-stm32-usbphyc.c
>> +++ b/drivers/phy/st/phy-stm32-usbphyc.c
>> @@ -317,6 +317,9 @@ static int stm32_usbphyc_pll_enable(struct stm32_usbphyc *usbphyc)
>>
>>  	stm32_usbphyc_set_bits(pll_reg, PLLEN);
>>
>> +	/* Wait for maximum lock time */
>> +	usleep_range(200, 300);
>> +
>>  	return 0;
>>
>>  reg_disable:
>

-- 
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: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2] phy: stm32-usphyc: add 200 to 300 us delay to fix timeout on some machines

[Michael Grzeschik]

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

Gentle Ping!

On Fri, Mar 10, 2023 at 11:44:38AM +0100, Michael Grzeschik wrote:
>Hi Fabrice,
>
>On Tue, Feb 28, 2023 at 06:28:21PM +0100, Fabrice Gasnier wrote:
>>On 2/27/23 16:13, Michael Grzeschik wrote:
>>>An minimum udelay of 200 us seems to be necessary on some machines. After
>>>the setup of the pll, which needs about 100 us to be locked there seem
>>>to be additional 100 us to get the phy really functional. Without this
>>>delay the usb runs not functional. With this additional short udelay
>>>this issue was not reported again.
>>>
>>>Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
>>>
>>
>>Hi Michael,
>>
>>Thank you for the updates,
>>
>>Fell free to add my:
>>Reviewed-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
>
>Thanks!
>
>Through which tree will this be picked?
>Will it be possible to add this to v6.3?
>
>Regards,
>Michael
>
>>>---
>>>v1 -> v2: - changed the mdelay to udelay_range(200, 300), like suggested by fabrice
>>>          - moved the delay to pll enable so it will only be triggered once
>>>
>>> drivers/phy/st/phy-stm32-usbphyc.c | 3 +++
>>> 1 file changed, 3 insertions(+)
>>>
>>>diff --git a/drivers/phy/st/phy-stm32-usbphyc.c b/drivers/phy/st/phy-stm32-usbphyc.c
>>>index 5bb9647b078f12..dd469f57fba7eb 100644
>>>--- a/drivers/phy/st/phy-stm32-usbphyc.c
>>>+++ b/drivers/phy/st/phy-stm32-usbphyc.c
>>>@@ -317,6 +317,9 @@ static int stm32_usbphyc_pll_enable(struct stm32_usbphyc *usbphyc)
>>>
>>> 	stm32_usbphyc_set_bits(pll_reg, PLLEN);
>>>
>>>+	/* Wait for maximum lock time */
>>>+	usleep_range(200, 300);
>>>+
>>> 	return 0;
>>>
>>> reg_disable:
>>
>
>-- 
>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 |



>_______________________________________________
>linux-arm-kernel mailing list
>linux-arm-kernel@lists.infradead.org
>http://lists.infradead.org/mailman/listinfo/linux-arm-kernel


-- 
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: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2] phy: stm32-usphyc: add 200 to 300 us delay to fix timeout on some machines

[Michael Grzeschik]

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

Cc'ing: Greg Kroah-Hartman <gregkh@linuxfoundation.org>


On Mon, Mar 20, 2023 at 01:02:10PM +0100, Michael Grzeschik wrote:
>Gentle Ping!
>
>On Fri, Mar 10, 2023 at 11:44:38AM +0100, Michael Grzeschik wrote:
>>Hi Fabrice,
>>
>>On Tue, Feb 28, 2023 at 06:28:21PM +0100, Fabrice Gasnier wrote:
>>>On 2/27/23 16:13, Michael Grzeschik wrote:
>>>>An minimum udelay of 200 us seems to be necessary on some machines. After
>>>>the setup of the pll, which needs about 100 us to be locked there seem
>>>>to be additional 100 us to get the phy really functional. Without this
>>>>delay the usb runs not functional. With this additional short udelay
>>>>this issue was not reported again.
>>>>
>>>>Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
>>>>
>>>
>>>Hi Michael,
>>>
>>>Thank you for the updates,
>>>
>>>Fell free to add my:
>>>Reviewed-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
>>
>>Thanks!
>>
>>Through which tree will this be picked?
>>Will it be possible to add this to v6.3?

Hi Greg!

Since nobody seem to catch this, is it possible that you pick this?

Regards,
Michael

>>>>---
>>>>v1 -> v2: - changed the mdelay to udelay_range(200, 300), like suggested by fabrice
>>>>         - moved the delay to pll enable so it will only be triggered once
>>>>
>>>>drivers/phy/st/phy-stm32-usbphyc.c | 3 +++
>>>>1 file changed, 3 insertions(+)
>>>>
>>>>diff --git a/drivers/phy/st/phy-stm32-usbphyc.c b/drivers/phy/st/phy-stm32-usbphyc.c
>>>>index 5bb9647b078f12..dd469f57fba7eb 100644
>>>>--- a/drivers/phy/st/phy-stm32-usbphyc.c
>>>>+++ b/drivers/phy/st/phy-stm32-usbphyc.c
>>>>@@ -317,6 +317,9 @@ static int stm32_usbphyc_pll_enable(struct stm32_usbphyc *usbphyc)
>>>>
>>>>	stm32_usbphyc_set_bits(pll_reg, PLLEN);
>>>>
>>>>+	/* Wait for maximum lock time */
>>>>+	usleep_range(200, 300);
>>>>+
>>>>	return 0;
>>>>
>>>>reg_disable:
>>>
>>
>>-- 
>>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 |
>
>
>
>>_______________________________________________
>>linux-arm-kernel mailing list
>>linux-arm-kernel@lists.infradead.org
>>http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
>
>-- 
>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 |



>_______________________________________________
>linux-arm-kernel mailing list
>linux-arm-kernel@lists.infradead.org
>http://lists.infradead.org/mailman/listinfo/linux-arm-kernel


-- 
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: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2] phy: stm32-usphyc: add 200 to 300 us delay to fix timeout on some machines

[Greg KH]

On Fri, Mar 31, 2023 at 02:06:27PM +0200, Michael Grzeschik wrote:
> Cc'ing: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> 
> 
> On Mon, Mar 20, 2023 at 01:02:10PM +0100, Michael Grzeschik wrote:
> > Gentle Ping!
> > 
> > On Fri, Mar 10, 2023 at 11:44:38AM +0100, Michael Grzeschik wrote:
> > > Hi Fabrice,
> > > 
> > > On Tue, Feb 28, 2023 at 06:28:21PM +0100, Fabrice Gasnier wrote:
> > > > On 2/27/23 16:13, Michael Grzeschik wrote:
> > > > > An minimum udelay of 200 us seems to be necessary on some machines. After
> > > > > the setup of the pll, which needs about 100 us to be locked there seem
> > > > > to be additional 100 us to get the phy really functional. Without this
> > > > > delay the usb runs not functional. With this additional short udelay
> > > > > this issue was not reported again.
> > > > > 
> > > > > Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
> > > > > 
> > > > 
> > > > Hi Michael,
> > > > 
> > > > Thank you for the updates,
> > > > 
> > > > Fell free to add my:
> > > > Reviewed-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
> > > 
> > > Thanks!
> > > 
> > > Through which tree will this be picked?
> > > Will it be possible to add this to v6.3?
> 
> Hi Greg!
> 
> Since nobody seem to catch this, is it possible that you pick this?

What is "this"?  The change to the following file:

> > > > > ---
> > > > > v1 -> v2: - changed the mdelay to udelay_range(200, 300), like suggested by fabrice
> > > > >         - moved the delay to pll enable so it will only be triggered once
> > > > > 
> > > > > drivers/phy/st/phy-stm32-usbphyc.c | 3 +++
> > > > > 1 file changed, 3 insertions(+)

That one?

I'm not anywhere on the maintainer path for it:

$ ./scripts/get_maintainer.pl drivers/phy/st/phy-stm32-usbphyc.c
Vinod Koul <vkoul@kernel.org> (supporter:GENERIC PHY FRAMEWORK,commit_signer:3/3=100%)
Kishon Vijay Abraham I <kishon@kernel.org> (supporter:GENERIC PHY FRAMEWORK)
Maxime Coquelin <mcoquelin.stm32@gmail.com> (maintainer:ARM/STM32 ARCHITECTURE)
Alexandre Torgue <alexandre.torgue@foss.st.com> (maintainer:ARM/STM32 ARCHITECTURE)
Philipp Zabel <p.zabel@pengutronix.de> (maintainer:RESET CONTROLLER FRAMEWORK)
Liam Girdwood <lgirdwood@gmail.com> (supporter:VOLTAGE AND CURRENT REGULATOR FRAMEWORK)
Mark Brown <broonie@kernel.org> (supporter:VOLTAGE AND CURRENT REGULATOR FRAMEWORK)
Amelie Delaunay <amelie.delaunay@foss.st.com> (commit_signer:2/3=67%)
Dan Carpenter <error27@gmail.com> (commit_signer:2/3=67%,authored:2/3=67%,added_lines:4/7=57%)
Fabrice Gasnier <fabrice.gasnier@foss.st.com> (commit_signer:1/3=33%,authored:1/3=33%,added_lines:3/7=43%,removed_lines:1/1=100%)
linux-phy@lists.infradead.org (open list:GENERIC PHY FRAMEWORK)
linux-stm32@st-md-mailman.stormreply.com (moderated list:ARM/STM32 ARCHITECTURE)
linux-arm-kernel@lists.infradead.org (moderated list:ARM/STM32 ARCHITECTURE)
linux-kernel@vger.kernel.org (open list)


What happened to the maintainers involved here?

thanks,

greg k-h

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2] phy: stm32-usphyc: add 200 to 300 us delay to fix timeout on some machines

[Vinod Koul]

On 31-03-23, 15:19, Greg KH wrote:
> On Fri, Mar 31, 2023 at 02:06:27PM +0200, Michael Grzeschik wrote:
> > Cc'ing: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > 
> > 
> > On Mon, Mar 20, 2023 at 01:02:10PM +0100, Michael Grzeschik wrote:
> > > Gentle Ping!
> > > 
> > > On Fri, Mar 10, 2023 at 11:44:38AM +0100, Michael Grzeschik wrote:
> > > > Hi Fabrice,
> > > > 
> > > > On Tue, Feb 28, 2023 at 06:28:21PM +0100, Fabrice Gasnier wrote:
> > > > > On 2/27/23 16:13, Michael Grzeschik wrote:
> > > > > > An minimum udelay of 200 us seems to be necessary on some machines. After
> > > > > > the setup of the pll, which needs about 100 us to be locked there seem
> > > > > > to be additional 100 us to get the phy really functional. Without this
> > > > > > delay the usb runs not functional. With this additional short udelay
> > > > > > this issue was not reported again.
> > > > > > 
> > > > > > Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
> > > > > > 
> > > > > 
> > > > > Hi Michael,
> > > > > 
> > > > > Thank you for the updates,
> > > > > 
> > > > > Fell free to add my:
> > > > > Reviewed-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
> > > > 
> > > > Thanks!
> > > > 
> > > > Through which tree will this be picked?
> > > > Will it be possible to add this to v6.3?
> > 
> > Hi Greg!
> > 
> > Since nobody seem to catch this, is it possible that you pick this?
> 
> What is "this"?  The change to the following file:
> 
> > > > > > ---
> > > > > > v1 -> v2: - changed the mdelay to udelay_range(200, 300), like suggested by fabrice
> > > > > >         - moved the delay to pll enable so it will only be triggered once
> > > > > > 
> > > > > > drivers/phy/st/phy-stm32-usbphyc.c | 3 +++
> > > > > > 1 file changed, 3 insertions(+)
> 
> That one?
> 
> I'm not anywhere on the maintainer path for it:
> 
> $ ./scripts/get_maintainer.pl drivers/phy/st/phy-stm32-usbphyc.c
> Vinod Koul <vkoul@kernel.org> (supporter:GENERIC PHY FRAMEWORK,commit_signer:3/3=100%)
> Kishon Vijay Abraham I <kishon@kernel.org> (supporter:GENERIC PHY FRAMEWORK)
> Maxime Coquelin <mcoquelin.stm32@gmail.com> (maintainer:ARM/STM32 ARCHITECTURE)
> Alexandre Torgue <alexandre.torgue@foss.st.com> (maintainer:ARM/STM32 ARCHITECTURE)
> Philipp Zabel <p.zabel@pengutronix.de> (maintainer:RESET CONTROLLER FRAMEWORK)
> Liam Girdwood <lgirdwood@gmail.com> (supporter:VOLTAGE AND CURRENT REGULATOR FRAMEWORK)
> Mark Brown <broonie@kernel.org> (supporter:VOLTAGE AND CURRENT REGULATOR FRAMEWORK)
> Amelie Delaunay <amelie.delaunay@foss.st.com> (commit_signer:2/3=67%)
> Dan Carpenter <error27@gmail.com> (commit_signer:2/3=67%,authored:2/3=67%,added_lines:4/7=57%)
> Fabrice Gasnier <fabrice.gasnier@foss.st.com> (commit_signer:1/3=33%,authored:1/3=33%,added_lines:3/7=43%,removed_lines:1/1=100%)
> linux-phy@lists.infradead.org (open list:GENERIC PHY FRAMEWORK)
> linux-stm32@st-md-mailman.stormreply.com (moderated list:ARM/STM32 ARCHITECTURE)
> linux-arm-kernel@lists.infradead.org (moderated list:ARM/STM32 ARCHITECTURE)
> linux-kernel@vger.kernel.org (open list)
> 
> 
> What happened to the maintainers involved here?

That would be me, sorry to have missed this one. I should be able to
review and do the needful shortly


-- 
~Vinod

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2] phy: stm32-usphyc: add 200 to 300 us delay to fix timeout on some machines

[Vinod Koul]

On 27-02-23, 16:13, Michael Grzeschik wrote:
> An minimum udelay of 200 us seems to be necessary on some machines. After
> the setup of the pll, which needs about 100 us to be locked there seem
> to be additional 100 us to get the phy really functional. Without this
> delay the usb runs not functional. With this additional short udelay
> this issue was not reported again.

Applied, thanks

-- 
~Vinod

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel