When resuming from sleep, if there is a time lag from link-down to link-up
due to auto-negotiation, the phy status has been still PHY_NOLINK, so
WARN_ON dump occurs in mdio_bus_phy_resume(). For example, UniPhier AVE
ethernet takes about a few seconds to link up after resuming.
To avoid this issue, should remove PHY_NOLINK the WARN_ON conditions.
Signed-off-by: Kunihiko Hayashi <[email protected]>
---
drivers/net/phy/phy_device.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 57849ac0384e..c647d027bb5d 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -318,12 +318,12 @@ static __maybe_unused int mdio_bus_phy_resume(struct device *dev)
phydev->suspended_by_mdio_bus = 0;
/* If we managed to get here with the PHY state machine in a state
- * neither PHY_HALTED, PHY_READY nor PHY_UP, this is an indication
- * that something went wrong and we should most likely be using
- * MAC managed PM, but we are not.
+ * neither PHY_HALTED, PHY_READY, PHY_UP nor PHY_NOLINK, this is an
+ * indication that something went wrong and we should most likely
+ * be using MAC managed PM, but we are not.
*/
WARN_ON(phydev->state != PHY_HALTED && phydev->state != PHY_READY &&
- phydev->state != PHY_UP);
+ phydev->state != PHY_UP && phydev->state != PHY_NOLINK);
ret = phy_init_hw(phydev);
if (ret < 0)
--
2.25.1
On 21.10.2022 09:41, Kunihiko Hayashi wrote:
> When resuming from sleep, if there is a time lag from link-down to link-up
> due to auto-negotiation, the phy status has been still PHY_NOLINK, so
> WARN_ON dump occurs in mdio_bus_phy_resume(). For example, UniPhier AVE
> ethernet takes about a few seconds to link up after resuming.
>
That autoneg takes some time is normal. If this would actually the root
cause then basically every driver should be affected. But it's not.
> To avoid this issue, should remove PHY_NOLINK the WARN_ON conditions.
>
> Signed-off-by: Kunihiko Hayashi <[email protected]>
> ---
> drivers/net/phy/phy_device.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index 57849ac0384e..c647d027bb5d 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -318,12 +318,12 @@ static __maybe_unused int mdio_bus_phy_resume(struct device *dev)
> phydev->suspended_by_mdio_bus = 0;
>
> /* If we managed to get here with the PHY state machine in a state
> - * neither PHY_HALTED, PHY_READY nor PHY_UP, this is an indication
> - * that something went wrong and we should most likely be using
> - * MAC managed PM, but we are not.
> + * neither PHY_HALTED, PHY_READY, PHY_UP nor PHY_NOLINK, this is an
> + * indication that something went wrong and we should most likely
> + * be using MAC managed PM, but we are not.
> */
Did you read the comment you're changing? ave_resume() calls phy_resume(),
so you should follow the advice in the comment.
> WARN_ON(phydev->state != PHY_HALTED && phydev->state != PHY_READY &&
> - phydev->state != PHY_UP);
> + phydev->state != PHY_UP && phydev->state != PHY_NOLINK);
>
> ret = phy_init_hw(phydev);
> if (ret < 0)
Hi Heiner,
Thank you for your comment.
On 2022/10/21 17:38, Heiner Kallweit wrote:
> On 21.10.2022 09:41, Kunihiko Hayashi wrote:
>> When resuming from sleep, if there is a time lag from link-down to link-up
>> due to auto-negotiation, the phy status has been still PHY_NOLINK, so
>> WARN_ON dump occurs in mdio_bus_phy_resume(). For example, UniPhier AVE
>> ethernet takes about a few seconds to link up after resuming.
>>
> That autoneg takes some time is normal. If this would actually the root
> cause then basically every driver should be affected. But it's not.
Although the auto-neg should happen normally, I'm not sure about other
platforms.
>> To avoid this issue, should remove PHY_NOLINK the WARN_ON conditions.
>>
>> Signed-off-by: Kunihiko Hayashi <[email protected]>
>> ---
>> drivers/net/phy/phy_device.c | 8 ++++----
>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
>> index 57849ac0384e..c647d027bb5d 100644
>> --- a/drivers/net/phy/phy_device.c
>> +++ b/drivers/net/phy/phy_device.c
>> @@ -318,12 +318,12 @@ static __maybe_unused int mdio_bus_phy_resume(struct
>> device *dev)
>> phydev->suspended_by_mdio_bus = 0;
>>
>> /* If we managed to get here with the PHY state machine in a state
>> - * neither PHY_HALTED, PHY_READY nor PHY_UP, this is an indication
>> - * that something went wrong and we should most likely be using
>> - * MAC managed PM, but we are not.
>> + * neither PHY_HALTED, PHY_READY, PHY_UP nor PHY_NOLINK, this is an
>> + * indication that something went wrong and we should most likely
>> + * be using MAC managed PM, but we are not.
>> */
>
> Did you read the comment you're changing? ave_resume() calls phy_resume(),
> so you should follow the advice in the comment.
I understand something is wrong with "PHY_NOLINK" here, and need to investigate
the root cause of the phy state issue.
Thank you,
---
Best Regards
Kunihiko Hayashi
On 2022/10/21 20:12, Heiner Kallweit wrote:
> On 21.10.2022 11:35, Kunihiko Hayashi wrote:
>> Hi Heiner,
>>
>> Thank you for your comment.
>>
>> On 2022/10/21 17:38, Heiner Kallweit wrote:
>>> On 21.10.2022 09:41, Kunihiko Hayashi wrote:
>>>> When resuming from sleep, if there is a time lag from link-down to
>>>> link-up
>>>> due to auto-negotiation, the phy status has been still PHY_NOLINK, so
>>>> WARN_ON dump occurs in mdio_bus_phy_resume(). For example, UniPhier AVE
>>>> ethernet takes about a few seconds to link up after resuming.
>>>>
>>> That autoneg takes some time is normal. If this would actually the root
>>> cause then basically every driver should be affected. But it's not.
>>
>> Although the auto-neg should happen normally, I'm not sure about other
>> platforms.
>>
>>>> To avoid this issue, should remove PHY_NOLINK the WARN_ON conditions.
>>>>
>>>> Signed-off-by: Kunihiko Hayashi <[email protected]>
>>>> ---
>>>> drivers/net/phy/phy_device.c | 8 ++++----
>>>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
>>>> index 57849ac0384e..c647d027bb5d 100644
>>>> --- a/drivers/net/phy/phy_device.c
>>>> +++ b/drivers/net/phy/phy_device.c
>>>> @@ -318,12 +318,12 @@ static __maybe_unused int
>>>> mdio_bus_phy_resume(struct
>>>> device *dev)
>>>> phydev->suspended_by_mdio_bus = 0;
>>>>
>>>> /* If we managed to get here with the PHY state machine in a state
>>>> - * neither PHY_HALTED, PHY_READY nor PHY_UP, this is an indication
>>>> - * that something went wrong and we should most likely be using
>>>> - * MAC managed PM, but we are not.
>>>> + * neither PHY_HALTED, PHY_READY, PHY_UP nor PHY_NOLINK, this is an
>>>> + * indication that something went wrong and we should most likely
>>>> + * be using MAC managed PM, but we are not.
>>>> */
>>>
>>> Did you read the comment you're changing? ave_resume() calls
>>> phy_resume(),
>>> so you should follow the advice in the comment.
>>
>> I understand something is wrong with "PHY_NOLINK" here, and need to
>> investigate
>> the root cause of the phy state issue.
>>
> Best look at how phydev->mac_managed_pm is used in phylib and by MAC
> drivers.
Thank you for the clue!
I'll try the flag and check the behavior of MAC/PHY.
Thank you,
---
Best Regards
Kunihiko Hayashi
On 21.10.2022 11:35, Kunihiko Hayashi wrote:
> Hi Heiner,
>
> Thank you for your comment.
>
> On 2022/10/21 17:38, Heiner Kallweit wrote:
>> On 21.10.2022 09:41, Kunihiko Hayashi wrote:
>>> When resuming from sleep, if there is a time lag from link-down to link-up
>>> due to auto-negotiation, the phy status has been still PHY_NOLINK, so
>>> WARN_ON dump occurs in mdio_bus_phy_resume(). For example, UniPhier AVE
>>> ethernet takes about a few seconds to link up after resuming.
>>>
>> That autoneg takes some time is normal. If this would actually the root
>> cause then basically every driver should be affected. But it's not.
>
> Although the auto-neg should happen normally, I'm not sure about other
> platforms.
>
>>> To avoid this issue, should remove PHY_NOLINK the WARN_ON conditions.
>>>
>>> Signed-off-by: Kunihiko Hayashi <[email protected]>
>>> ---
>>> drivers/net/phy/phy_device.c | 8 ++++----
>>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
>>> index 57849ac0384e..c647d027bb5d 100644
>>> --- a/drivers/net/phy/phy_device.c
>>> +++ b/drivers/net/phy/phy_device.c
>>> @@ -318,12 +318,12 @@ static __maybe_unused int mdio_bus_phy_resume(struct
>>> device *dev)
>>> phydev->suspended_by_mdio_bus = 0;
>>>
>>> /* If we managed to get here with the PHY state machine in a state
>>> - * neither PHY_HALTED, PHY_READY nor PHY_UP, this is an indication
>>> - * that something went wrong and we should most likely be using
>>> - * MAC managed PM, but we are not.
>>> + * neither PHY_HALTED, PHY_READY, PHY_UP nor PHY_NOLINK, this is an
>>> + * indication that something went wrong and we should most likely
>>> + * be using MAC managed PM, but we are not.
>>> */
>>
>> Did you read the comment you're changing? ave_resume() calls phy_resume(),
>> so you should follow the advice in the comment.
>
> I understand something is wrong with "PHY_NOLINK" here, and need to investigate
> the root cause of the phy state issue.
>
Best look at how phydev->mac_managed_pm is used in phylib and by MAC drivers.
> Thank you,
>
> ---
> Best Regards
> Kunihiko Hayashi