Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1606369 > unrolled thread

[PATCH v2 1/2] net: phy: Fix PHY AN done state machine for interrupt driven PHYs

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v2 1/2] net: phy: Fix PHY AN done state machine for interrupt driven PHYs Roger Quadros <rogerq@ti.com> - 2017-03-22 12:10 +0100
    Re: [PATCH v2 1/2] net: phy: Fix PHY AN done state machine for  interrupt driven PHYs Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2017-03-23 11:00 +0100
      Re: [PATCH v2 1/2] net: phy: Fix PHY AN done state machine for  interrupt driven PHYs Roger Quadros <rogerq@ti.com> - 2017-03-27 14:00 +0200
    [PATCH v3 1/2] net: phy: Fix PHY AN done state machine for interrupt  driven PHYs Roger Quadros <rogerq@ti.com> - 2017-03-27 14:10 +0200
      Re: [PATCH v3 1/2] net: phy: Fix PHY AN done state machine for  interrupt driven PHYs Roger Quadros <rogerq@ti.com> - 2017-03-28 12:10 +0200
      RE: [PATCH v3 1/2] net: phy: Fix PHY AN done state machine for  interrupt driven PHYs Madalin-Cristian Bucur <madalin.bucur@nxp.com> - 2017-03-30 15:10 +0200
      Re: [PATCH v3 1/2] net: phy: Fix PHY AN done state machine for  interrupt driven PHYs Florian Fainelli <f.fainelli@gmail.com> - 2017-03-30 22:10 +0200
        Re: [PATCH v3 1/2] net: phy: Fix PHY AN done state machine for  interrupt driven PHYs Roger Quadros <rogerq@ti.com> - 2017-03-31 11:30 +0200

#1606369 — [PATCH v2 1/2] net: phy: Fix PHY AN done state machine for interrupt driven PHYs

he ethernet link on an interrupt driven PHY was not coming up if the
ethernet cable was plugged before the ethernet interface was brought up.

The PHY state machine seems to be stuck from RUNNING to AN state
with no new interrupts from the PHY. So it doesn't know when the
PHY Auto-negotiation has been completed and doesn't transition to RUNNING
state with ANEG done thus netif_carrier_on() is never called.

NOTE: genphy_config_aneg() will not restart PHY Auto-negotiation of
advertisement parameters didn't change.

Fix this by scheduling the PHY state machine in phy_start_aneg().

Fixes: 3c293f4e08b5 ("net: phy: Trigger state machine on state change and not polling.")
Cc: stable <stable@vger.kernel.org> # v4.9+
Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 drivers/net/phy/phy.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 1be69d8..49dedf8 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -630,6 +630,10 @@ int phy_start_aneg(struct phy_device *phydev)
 
 out_unlock:
 	mutex_unlock(&phydev->lock);
+	if (!err && phy_interrupt_is_valid(phydev))
+		queue_delayed_work(system_power_efficient_wq,
+				   &phydev->state_queue, HZ);
+
 	return err;
 }
 EXPORT_SYMBOL(phy_start_aneg);
-- 
2.7.4

[toc] | [next] | [standalone]

#1607309 — Re: [PATCH v2 1/2] net: phy: Fix PHY AN done state machine for interrupt driven PHYs

Hello!

On 3/22/2017 2:02 PM, Roger Quadros wrote:

> he ethernet link on an interrupt driven PHY was not coming up if the

    s/he/The/?

> ethernet cable was plugged before the ethernet interface was brought up.

    Also, my spell checker trips on "ethernet", perhaps should be capitalized?

> The PHY state machine seems to be stuck from RUNNING to AN state
> with no new interrupts from the PHY. So it doesn't know when the
> PHY Auto-negotiation has been completed and doesn't transition to RUNNING
> state with ANEG done thus netif_carrier_on() is never called.
>
> NOTE: genphy_config_aneg() will not restart PHY Auto-negotiation of
> advertisement parameters didn't change.
>
> Fix this by scheduling the PHY state machine in phy_start_aneg().
>
> Fixes: 3c293f4e08b5 ("net: phy: Trigger state machine on state change and not polling.")
> Cc: stable <stable@vger.kernel.org> # v4.9+
> Signed-off-by: Roger Quadros <rogerq@ti.com>
[...]

MBR, Sergei

[toc] | [prev] | [next] | [standalone]

#1609759 — Re: [PATCH v2 1/2] net: phy: Fix PHY AN done state machine for interrupt driven PHYs

On 23/03/17 11:52, Sergei Shtylyov wrote:
> Hello!
> 
> On 3/22/2017 2:02 PM, Roger Quadros wrote:
> 
>> he ethernet link on an interrupt driven PHY was not coming up if the
> 
>    s/he/The/?
> 
>> ethernet cable was plugged before the ethernet interface was brought up.
> 
>    Also, my spell checker trips on "ethernet", perhaps should be capitalized?

Thanks. I'll fix both issues.

> 
>> The PHY state machine seems to be stuck from RUNNING to AN state
>> with no new interrupts from the PHY. So it doesn't know when the
>> PHY Auto-negotiation has been completed and doesn't transition to RUNNING
>> state with ANEG done thus netif_carrier_on() is never called.
>>
>> NOTE: genphy_config_aneg() will not restart PHY Auto-negotiation of
>> advertisement parameters didn't change.
>>
>> Fix this by scheduling the PHY state machine in phy_start_aneg().
>>
>> Fixes: 3c293f4e08b5 ("net: phy: Trigger state machine on state change and not polling.")
>> Cc: stable <stable@vger.kernel.org> # v4.9+
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
> [...]
> 
> MBR, Sergei
> 

cheers,
-roger

[toc] | [prev] | [next] | [standalone]

#1609761 — [PATCH v3 1/2] net: phy: Fix PHY AN done state machine for interrupt driven PHYs

The Ethernet link on an interrupt driven PHY was not coming up if the
Ethernet cable was plugged before the Ethernet interface was brought up.

The PHY state machine seems to be stuck from RUNNING to AN state
with no new interrupts from the PHY. So it doesn't know when the
PHY Auto-negotiation has been completed and doesn't transition to RUNNING
state with ANEG done thus netif_carrier_on() is never called.

NOTE: genphy_config_aneg() will not restart PHY Auto-negotiation of
advertisement parameters didn't change.

Fix this by scheduling the PHY state machine in phy_start_aneg().
There is no way of knowing in phy.c whether auto-negotiation was
restarted or not by the PHY driver so we just wait for the next
poll/interrupt to update the PHY state machine.

Fixes: 3c293f4e08b5 ("net: phy: Trigger state machine on state change and not polling.")
Cc: stable <stable@vger.kernel.org> # v4.9+
Signed-off-by: Roger Quadros <rogerq@ti.com>
---
v3: Fix typo in commit message

 drivers/net/phy/phy.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 1be69d8..49dedf8 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -630,6 +630,10 @@ int phy_start_aneg(struct phy_device *phydev)
 
 out_unlock:
 	mutex_unlock(&phydev->lock);
+	if (!err && phy_interrupt_is_valid(phydev))
+		queue_delayed_work(system_power_efficient_wq,
+				   &phydev->state_queue, HZ);
+
 	return err;
 }
 EXPORT_SYMBOL(phy_start_aneg);
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1610519 — Re: [PATCH v3 1/2] net: phy: Fix PHY AN done state machine for interrupt driven PHYs

+Andrew Davis & Sekhar.

Hi,

Andrew Davis posted a few comments offline which I'm clarifying here.

On 27/03/17 14:59, Roger Quadros wrote:
> The Ethernet link on an interrupt driven PHY was not coming up if the
> Ethernet cable was plugged before the Ethernet interface was brought up.
> 
> The PHY state machine seems to be stuck from RUNNING to AN state
> with no new interrupts from the PHY. So it doesn't know when the
> PHY Auto-negotiation has been completed and doesn't transition to RUNNING
> state with ANEG done thus netif_carrier_on() is never called.
> 
> NOTE: genphy_config_aneg() will not restart PHY Auto-negotiation of
> advertisement parameters didn't change.

Is phy->config_aneg expected to *always* restart auto-negotiation even if
advertisement parameters didn't change?
If so then we'll need to fix genphy_config_aneg().

> 
> Fix this by scheduling the PHY state machine in phy_start_aneg().
> There is no way of knowing in phy.c whether auto-negotiation was
> restarted or not by the PHY driver so we just wait for the next
> poll/interrupt to update the PHY state machine.
> 
> Fixes: 3c293f4e08b5 ("net: phy: Trigger state machine on state change and not polling.")
> Cc: stable <stable@vger.kernel.org> # v4.9+
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
> v3: Fix typo in commit message
> 
>  drivers/net/phy/phy.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index 1be69d8..49dedf8 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -630,6 +630,10 @@ int phy_start_aneg(struct phy_device *phydev)
>  
>  out_unlock:
>  	mutex_unlock(&phydev->lock);
> +	if (!err && phy_interrupt_is_valid(phydev))
> +		queue_delayed_work(system_power_efficient_wq,
> +				   &phydev->state_queue, HZ);
> +
>  	return err;
>  }
>  EXPORT_SYMBOL(phy_start_aneg);
> 

There is still room for optimization for interrupt driven PHYs as I still
see a delay of 1 second between "ifconfig ethx up" and link status coming up
if cable was already plugged in. In fact if Auto-negotiation was already completed
and not required to be restarted, the PHY state machine should have move from
AN to RUNNING instantly without expecting a PHY interrupt.

How can we get rid of the unnecessary delay in the case where auto-negotiation
is not restarted?
Should we check for phy_aneg_done() immediately after issuing a phy_start_aneg()
in phy_state_machine() and switch from PHY_AN to PHY_RUNNING?

This should avoid the need to re-schedule the state machine in phy_start_angeg().

cheers,
-roger

[toc] | [prev] | [next] | [standalone]

#1613046 — RE: [PATCH v3 1/2] net: phy: Fix PHY AN done state machine for interrupt driven PHYs

On March 27, 2017 2:59 PM, Roger Quadros wrote:
> The Ethernet link on an interrupt driven PHY was not coming up if the
> Ethernet cable was plugged before the Ethernet interface was brought up.
> 
> The PHY state machine seems to be stuck from RUNNING to AN state
> with no new interrupts from the PHY. So it doesn't know when the
> PHY Auto-negotiation has been completed and doesn't transition to RUNNING
> state with ANEG done thus netif_carrier_on() is never called.
> 
> NOTE: genphy_config_aneg() will not restart PHY Auto-negotiation of
> advertisement parameters didn't change.
> 
> Fix this by scheduling the PHY state machine in phy_start_aneg().
> There is no way of knowing in phy.c whether auto-negotiation was
> restarted or not by the PHY driver so we just wait for the next
> poll/interrupt to update the PHY state machine.
> 
> Fixes: 3c293f4e08b5 ("net: phy: Trigger state machine on state change and
> not polling.")
> Cc: stable <stable@vger.kernel.org> # v4.9+
> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
> v3: Fix typo in commit message
> 
>  drivers/net/phy/phy.c | 4 ++++
>  1 file changed, 4 insertions(+)

Tested-by: Madalin Bucur <madalin.bucur@nxp.com>

[toc] | [prev] | [next] | [standalone]

#1613442 — Re: [PATCH v3 1/2] net: phy: Fix PHY AN done state machine for interrupt driven PHYs

On 03/27/2017 04:59 AM, Roger Quadros wrote:
> The Ethernet link on an interrupt driven PHY was not coming up if the
> Ethernet cable was plugged before the Ethernet interface was brought up.
> 
> The PHY state machine seems to be stuck from RUNNING to AN state
> with no new interrupts from the PHY. So it doesn't know when the
> PHY Auto-negotiation has been completed and doesn't transition to RUNNING
> state with ANEG done thus netif_carrier_on() is never called.
> 
> NOTE: genphy_config_aneg() will not restart PHY Auto-negotiation of
> advertisement parameters didn't change.
> 
> Fix this by scheduling the PHY state machine in phy_start_aneg().
> There is no way of knowing in phy.c whether auto-negotiation was
> restarted or not by the PHY driver so we just wait for the next
> poll/interrupt to update the PHY state machine.
> 
> Fixes: 3c293f4e08b5 ("net: phy: Trigger state machine on state change and not polling.")
> Cc: stable <stable@vger.kernel.org> # v4.9+
> Signed-off-by: Roger Quadros <rogerq@ti.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

[toc] | [prev] | [next] | [standalone]

#1613794 — Re: [PATCH v3 1/2] net: phy: Fix PHY AN done state machine for interrupt driven PHYs

Florian,

On 30/03/17 23:02, Florian Fainelli wrote:
> On 03/27/2017 04:59 AM, Roger Quadros wrote:
>> The Ethernet link on an interrupt driven PHY was not coming up if the
>> Ethernet cable was plugged before the Ethernet interface was brought up.
>>
>> The PHY state machine seems to be stuck from RUNNING to AN state
>> with no new interrupts from the PHY. So it doesn't know when the
>> PHY Auto-negotiation has been completed and doesn't transition to RUNNING
>> state with ANEG done thus netif_carrier_on() is never called.
>>
>> NOTE: genphy_config_aneg() will not restart PHY Auto-negotiation of
>> advertisement parameters didn't change.
>>
>> Fix this by scheduling the PHY state machine in phy_start_aneg().
>> There is no way of knowing in phy.c whether auto-negotiation was
>> restarted or not by the PHY driver so we just wait for the next
>> poll/interrupt to update the PHY state machine.
>>
>> Fixes: 3c293f4e08b5 ("net: phy: Trigger state machine on state change and not polling.")
>> Cc: stable <stable@vger.kernel.org> # v4.9+
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
> 
> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
> 
Thanks for the review, but there are a still few unanswered questions in the parallel thread.
Can you please clarify those first before this patch gets picked? Thanks.

cheers,
-roger

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web