[PATCH v1] net: phy: fix auto-negotiation stall due to unavailable interrupt

[PATCH v1] net: phy: fix auto-negotiation stall due to unavailable interrupt

[Alexander Kochetkov]

Hello Florian, Roger and Madalin!

This is slightly modified version of previous patch[1].

It take into account that phy_start_aneg() may be called from
differend places not only from PHY state machine. So the patch
use phy_trigger_machine() to schedule link state update
correctly.

I borrow some text from Roger's commit message.

Added 'Cc: stable <stable@vger.kernel.org> tag'
because the problem exist on 4.9 and 4.10 branches.

Don't know what commit brake the code, so I don't add
Fixes tag.

Roger, could you please test this one patch?

Madalin, I found, that you tested Roger’s patch[2] and this
one patch will get upstream instead of patch[2]. Could you
please test it?

Regards,
Alexander.

[1] http://patchwork.ozlabs.org/patch/743773/
[2] https://lkml.org/lkml/2017/3/30/517

Alexander Kochetkov (1):
  net: phy: fix auto-negotiation stall due to unavailable interrupt

 drivers/net/phy/phy.c |   40 ++++++++++++++++++++++++++++++++++++----
 include/linux/phy.h   |    1 +
 2 files changed, 37 insertions(+), 4 deletions(-)

-- 
1.7.9.5

[PATCH v1] net: phy: fix auto-negotiation stall due to unavailable interrupt

[Alexander Kochetkov]

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 patch trigger PHY state machine to update link state if PHY was requested to
do auto-negotiation and auto-negotiation complete flag already set.

During power-up cycle the PHY do auto-negotiation, generate interrupt and set
auto-negotiation complete flag. Interrupt is handled by PHY state machine but
doesn't update link state because PHY is in PHY_READY state. After some time
MAC bring up, start and request PHY to do auto-negotiation. If there are no new
settings to advertise genphy_config_aneg() doesn't start PHY auto-negotiation.
PHY continue to stay in auto-negotiation complete state and doesn't fire
interrupt. At the same time PHY state machine expect that PHY started
auto-negotiation and is waiting for interrupt from PHY and it won't get it.

Signed-off-by: Alexander Kochetkov <al.kochet@gmail.com>
Cc: stable <stable@vger.kernel.org> # v4.9+
---
 drivers/net/phy/phy.c |   40 ++++++++++++++++++++++++++++++++++++----
 include/linux/phy.h   |    1 +
 2 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 7cc1b7d..2d9975b 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -591,16 +591,18 @@ int phy_mii_ioctl(struct phy_device *phydev, struct ifreq *ifr, int cmd)
 EXPORT_SYMBOL(phy_mii_ioctl);
 
 /**
- * phy_start_aneg - start auto-negotiation for this PHY device
+ * phy_start_aneg_priv - start auto-negotiation for this PHY device
  * @phydev: the phy_device struct
+ * @sync: indicate whether we should wait for the workqueue cancelation
  *
  * Description: Sanitizes the settings (if we're not autonegotiating
  *   them), and then calls the driver's config_aneg function.
  *   If the PHYCONTROL Layer is operating, we change the state to
  *   reflect the beginning of Auto-negotiation or forcing.
  */
-int phy_start_aneg(struct phy_device *phydev)
+static int phy_start_aneg_priv(struct phy_device *phydev, bool sync)
 {
+	bool trigger = 0;
 	int err;
 
 	mutex_lock(&phydev->lock);
@@ -625,10 +627,40 @@ int phy_start_aneg(struct phy_device *phydev)
 		}
 	}
 
+	/* Re-schedule a PHY state machine to check PHY status because
+	 * negotiation may already be done and aneg interrupt may not be
+	 * generated.
+	 */
+	if (phy_interrupt_is_valid(phydev) && (phydev->state == PHY_AN)) {
+		err = phy_aneg_done(phydev);
+		if (err > 0) {
+			trigger = true;
+			err = 0;
+		}
+	}
+
 out_unlock:
 	mutex_unlock(&phydev->lock);
+
+	if (trigger)
+		phy_trigger_machine(phydev, sync);
+
 	return err;
 }
+
+/**
+ * phy_start_aneg - start auto-negotiation for this PHY device
+ * @phydev: the phy_device struct
+ *
+ * Description: Sanitizes the settings (if we're not autonegotiating
+ *   them), and then calls the driver's config_aneg function.
+ *   If the PHYCONTROL Layer is operating, we change the state to
+ *   reflect the beginning of Auto-negotiation or forcing.
+ */
+int phy_start_aneg(struct phy_device *phydev)
+{
+	return phy_start_aneg_priv(phydev, true);
+}
 EXPORT_SYMBOL(phy_start_aneg);
 
 /**
@@ -656,7 +688,7 @@ void phy_start_machine(struct phy_device *phydev)
  *   state machine runs.
  */
 
-static void phy_trigger_machine(struct phy_device *phydev, bool sync)
+void phy_trigger_machine(struct phy_device *phydev, bool sync)
 {
 	if (sync)
 		cancel_delayed_work_sync(&phydev->state_queue);
@@ -1151,7 +1183,7 @@ void phy_state_machine(struct work_struct *work)
 	mutex_unlock(&phydev->lock);
 
 	if (needs_aneg)
-		err = phy_start_aneg(phydev);
+		err = phy_start_aneg_priv(phydev, false);
 	else if (do_suspend)
 		phy_suspend(phydev);
 
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 7fc1105..b19ae66 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -840,6 +840,7 @@ int phy_drivers_register(struct phy_driver *new_driver, int n,
 void phy_mac_interrupt(struct phy_device *phydev, int new_link);
 void phy_start_machine(struct phy_device *phydev);
 void phy_stop_machine(struct phy_device *phydev);
+void phy_trigger_machine(struct phy_device *phydev, bool sync);
 int phy_ethtool_sset(struct phy_device *phydev, struct ethtool_cmd *cmd);
 int phy_ethtool_gset(struct phy_device *phydev, struct ethtool_cmd *cmd);
 int phy_ethtool_ksettings_get(struct phy_device *phydev,
-- 
1.7.9.5

Re: [PATCH v1] net: phy: fix auto-negotiation stall due to unavailable interrupt

[Alexander Kochetkov]

Hi, Alexandre!

Just found that you've fixed similar problem for Micrel PHY:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=99f81afc139c6edd14d77a91ee91685a414a1c66

Could you please test if my patch[1] fix your problem?

As reverting your patch will speedup MAC startup time for boards with Micrel PHY
on 3~5 sec (auto-negotiation time[2]). Could you check that also?

Regards,
Alexander.

[1] https://lkml.org/lkml/2017/4/20/357
[2] http://www.ieee802.org/3/af/public/jan02/brown_1_0102.pdf


> 20 апр. 2017 г., в 14:00, Alexander Kochetkov <al.kochet@gmail.com> написал(а):
> 
> 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 patch trigger PHY state machine to update link state if PHY was requested to
> do auto-negotiation and auto-negotiation complete flag already set.
> 
> During power-up cycle the PHY do auto-negotiation, generate interrupt and set
> auto-negotiation complete flag. Interrupt is handled by PHY state machine but
> doesn't update link state because PHY is in PHY_READY state. After some time
> MAC bring up, start and request PHY to do auto-negotiation. If there are no new
> settings to advertise genphy_config_aneg() doesn't start PHY auto-negotiation.
> PHY continue to stay in auto-negotiation complete state and doesn't fire
> interrupt. At the same time PHY state machine expect that PHY started
> auto-negotiation and is waiting for interrupt from PHY and it won't get it.
> 
> Signed-off-by: Alexander Kochetkov <al.kochet@gmail.com>
> Cc: stable <stable@vger.kernel.org> # v4.9+
> ---
> drivers/net/phy/phy.c |   40 ++++++++++++++++++++++++++++++++++++----
> include/linux/phy.h   |    1 +
> 2 files changed, 37 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index 7cc1b7d..2d9975b 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -591,16 +591,18 @@ int phy_mii_ioctl(struct phy_device *phydev, struct ifreq *ifr, int cmd)
> EXPORT_SYMBOL(phy_mii_ioctl);
> 
> /**
> - * phy_start_aneg - start auto-negotiation for this PHY device
> + * phy_start_aneg_priv - start auto-negotiation for this PHY device
>  * @phydev: the phy_device struct
> + * @sync: indicate whether we should wait for the workqueue cancelation
>  *
>  * Description: Sanitizes the settings (if we're not autonegotiating
>  *   them), and then calls the driver's config_aneg function.
>  *   If the PHYCONTROL Layer is operating, we change the state to
>  *   reflect the beginning of Auto-negotiation or forcing.
>  */
> -int phy_start_aneg(struct phy_device *phydev)
> +static int phy_start_aneg_priv(struct phy_device *phydev, bool sync)
> {
> +	bool trigger = 0;
> 	int err;
> 
> 	mutex_lock(&phydev->lock);
> @@ -625,10 +627,40 @@ int phy_start_aneg(struct phy_device *phydev)
> 		}
> 	}
> 
> +	/* Re-schedule a PHY state machine to check PHY status because
> +	 * negotiation may already be done and aneg interrupt may not be
> +	 * generated.
> +	 */
> +	if (phy_interrupt_is_valid(phydev) && (phydev->state == PHY_AN)) {
> +		err = phy_aneg_done(phydev);
> +		if (err > 0) {
> +			trigger = true;
> +			err = 0;
> +		}
> +	}
> +
> out_unlock:
> 	mutex_unlock(&phydev->lock);
> +
> +	if (trigger)
> +		phy_trigger_machine(phydev, sync);
> +
> 	return err;
> }
> +
> +/**
> + * phy_start_aneg - start auto-negotiation for this PHY device
> + * @phydev: the phy_device struct
> + *
> + * Description: Sanitizes the settings (if we're not autonegotiating
> + *   them), and then calls the driver's config_aneg function.
> + *   If the PHYCONTROL Layer is operating, we change the state to
> + *   reflect the beginning of Auto-negotiation or forcing.
> + */
> +int phy_start_aneg(struct phy_device *phydev)
> +{
> +	return phy_start_aneg_priv(phydev, true);
> +}
> EXPORT_SYMBOL(phy_start_aneg);
> 
> /**
> @@ -656,7 +688,7 @@ void phy_start_machine(struct phy_device *phydev)
>  *   state machine runs.
>  */
> 
> -static void phy_trigger_machine(struct phy_device *phydev, bool sync)
> +void phy_trigger_machine(struct phy_device *phydev, bool sync)
> {
> 	if (sync)
> 		cancel_delayed_work_sync(&phydev->state_queue);
> @@ -1151,7 +1183,7 @@ void phy_state_machine(struct work_struct *work)
> 	mutex_unlock(&phydev->lock);
> 
> 	if (needs_aneg)
> -		err = phy_start_aneg(phydev);
> +		err = phy_start_aneg_priv(phydev, false);
> 	else if (do_suspend)
> 		phy_suspend(phydev);
> 
> diff --git a/include/linux/phy.h b/include/linux/phy.h
> index 7fc1105..b19ae66 100644
> --- a/include/linux/phy.h
> +++ b/include/linux/phy.h
> @@ -840,6 +840,7 @@ int phy_drivers_register(struct phy_driver *new_driver, int n,
> void phy_mac_interrupt(struct phy_device *phydev, int new_link);
> void phy_start_machine(struct phy_device *phydev);
> void phy_stop_machine(struct phy_device *phydev);
> +void phy_trigger_machine(struct phy_device *phydev, bool sync);
> int phy_ethtool_sset(struct phy_device *phydev, struct ethtool_cmd *cmd);
> int phy_ethtool_gset(struct phy_device *phydev, struct ethtool_cmd *cmd);
> int phy_ethtool_ksettings_get(struct phy_device *phydev,
> -- 
> 1.7.9.5
> 

Re: [PATCH v1] net: phy: fix auto-negotiation stall due to unavailable interrupt

[Roger Quadros]

On 20/04/17 14:00, Alexander Kochetkov 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 patch trigger PHY state machine to update link state if PHY was requested to
> do auto-negotiation and auto-negotiation complete flag already set.
> 
> During power-up cycle the PHY do auto-negotiation, generate interrupt and set
> auto-negotiation complete flag. Interrupt is handled by PHY state machine but
> doesn't update link state because PHY is in PHY_READY state. After some time
> MAC bring up, start and request PHY to do auto-negotiation. If there are no new
> settings to advertise genphy_config_aneg() doesn't start PHY auto-negotiation.
> PHY continue to stay in auto-negotiation complete state and doesn't fire
> interrupt. At the same time PHY state machine expect that PHY started
> auto-negotiation and is waiting for interrupt from PHY and it won't get it.
> 
> Signed-off-by: Alexander Kochetkov <al.kochet@gmail.com>
> Cc: stable <stable@vger.kernel.org> # v4.9+

Tested-by: Roger Quadros <rogerq@ti.com>

I think the following commit broke functionality with interrupt driven PHYs
3c293f4e08b5 ("net: phy: Trigger state machine on state change and not polling.")

cheers,
-roger

> ---
>  drivers/net/phy/phy.c |   40 ++++++++++++++++++++++++++++++++++++----
>  include/linux/phy.h   |    1 +
>  2 files changed, 37 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index 7cc1b7d..2d9975b 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -591,16 +591,18 @@ int phy_mii_ioctl(struct phy_device *phydev, struct ifreq *ifr, int cmd)
>  EXPORT_SYMBOL(phy_mii_ioctl);
>  
>  /**
> - * phy_start_aneg - start auto-negotiation for this PHY device
> + * phy_start_aneg_priv - start auto-negotiation for this PHY device
>   * @phydev: the phy_device struct
> + * @sync: indicate whether we should wait for the workqueue cancelation
>   *
>   * Description: Sanitizes the settings (if we're not autonegotiating
>   *   them), and then calls the driver's config_aneg function.
>   *   If the PHYCONTROL Layer is operating, we change the state to
>   *   reflect the beginning of Auto-negotiation or forcing.
>   */
> -int phy_start_aneg(struct phy_device *phydev)
> +static int phy_start_aneg_priv(struct phy_device *phydev, bool sync)
>  {
> +	bool trigger = 0;
>  	int err;
>  
>  	mutex_lock(&phydev->lock);
> @@ -625,10 +627,40 @@ int phy_start_aneg(struct phy_device *phydev)
>  		}
>  	}
>  
> +	/* Re-schedule a PHY state machine to check PHY status because
> +	 * negotiation may already be done and aneg interrupt may not be
> +	 * generated.
> +	 */
> +	if (phy_interrupt_is_valid(phydev) && (phydev->state == PHY_AN)) {
> +		err = phy_aneg_done(phydev);
> +		if (err > 0) {
> +			trigger = true;
> +			err = 0;
> +		}
> +	}
> +
>  out_unlock:
>  	mutex_unlock(&phydev->lock);
> +
> +	if (trigger)
> +		phy_trigger_machine(phydev, sync);
> +
>  	return err;
>  }
> +
> +/**
> + * phy_start_aneg - start auto-negotiation for this PHY device
> + * @phydev: the phy_device struct
> + *
> + * Description: Sanitizes the settings (if we're not autonegotiating
> + *   them), and then calls the driver's config_aneg function.
> + *   If the PHYCONTROL Layer is operating, we change the state to
> + *   reflect the beginning of Auto-negotiation or forcing.
> + */
> +int phy_start_aneg(struct phy_device *phydev)
> +{
> +	return phy_start_aneg_priv(phydev, true);
> +}
>  EXPORT_SYMBOL(phy_start_aneg);
>  
>  /**
> @@ -656,7 +688,7 @@ void phy_start_machine(struct phy_device *phydev)
>   *   state machine runs.
>   */
>  
> -static void phy_trigger_machine(struct phy_device *phydev, bool sync)
> +void phy_trigger_machine(struct phy_device *phydev, bool sync)
>  {
>  	if (sync)
>  		cancel_delayed_work_sync(&phydev->state_queue);
> @@ -1151,7 +1183,7 @@ void phy_state_machine(struct work_struct *work)
>  	mutex_unlock(&phydev->lock);
>  
>  	if (needs_aneg)
> -		err = phy_start_aneg(phydev);
> +		err = phy_start_aneg_priv(phydev, false);
>  	else if (do_suspend)
>  		phy_suspend(phydev);
>  
> diff --git a/include/linux/phy.h b/include/linux/phy.h
> index 7fc1105..b19ae66 100644
> --- a/include/linux/phy.h
> +++ b/include/linux/phy.h
> @@ -840,6 +840,7 @@ int phy_drivers_register(struct phy_driver *new_driver, int n,
>  void phy_mac_interrupt(struct phy_device *phydev, int new_link);
>  void phy_start_machine(struct phy_device *phydev);
>  void phy_stop_machine(struct phy_device *phydev);
> +void phy_trigger_machine(struct phy_device *phydev, bool sync);
>  int phy_ethtool_sset(struct phy_device *phydev, struct ethtool_cmd *cmd);
>  int phy_ethtool_gset(struct phy_device *phydev, struct ethtool_cmd *cmd);
>  int phy_ethtool_ksettings_get(struct phy_device *phydev,
> 

Re: [PATCH v1] net: phy: fix auto-negotiation stall due to unavailable interrupt

[Alexander Kochetkov]

> 21 апр. 2017 г., в 17:18, Roger Quadros <rogerq@ti.com> написал(а):
> 
> I think the following commit broke functionality with interrupt driven PHYs
> 3c293f4e08b5 ("net: phy: Trigger state machine on state change and not polling.")

Probably this one[1] broke, according to Alexandre’s commit[2].
And it was since Nov 16 2015. But it was hidden by some other commits.

For Roger problem became visible after 3c293f4e08b5 ("net: phy:
Trigger state machine on state change and not polling.»),

For my problem became visible after 529ed1275263 ("net: phy: phy drivers
should not set SUPPORTED_[Asym_]Pause»). As commit 529ed1275263 
removed SUPPORTED_Pause flag from PHY advertising property and
genphy_config_aneg() began to skip PHY auto-negotiation.

Alexander.

[1] Fixes: 321beec5047a (net: phy: Use interrupts when available in NOLINK state)
     https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=321beec5047af83db90c88114b7e664b156f49fe
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=99f81afc139c6edd14d77a91ee91685a414a1c66

Re: [PATCH v1] net: phy: fix auto-negotiation stall due to unavailable interrupt

[David Miller]

From: Alexander Kochetkov <al.kochet@gmail.com>
Date: Thu, 20 Apr 2017 14:00:04 +0300

> 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 patch trigger PHY state machine to update link state if PHY was requested to
> do auto-negotiation and auto-negotiation complete flag already set.
> 
> During power-up cycle the PHY do auto-negotiation, generate interrupt and set
> auto-negotiation complete flag. Interrupt is handled by PHY state machine but
> doesn't update link state because PHY is in PHY_READY state. After some time
> MAC bring up, start and request PHY to do auto-negotiation. If there are no new
> settings to advertise genphy_config_aneg() doesn't start PHY auto-negotiation.
> PHY continue to stay in auto-negotiation complete state and doesn't fire
> interrupt. At the same time PHY state machine expect that PHY started
> auto-negotiation and is waiting for interrupt from PHY and it won't get it.
> 
> Signed-off-by: Alexander Kochetkov <al.kochet@gmail.com>
> Cc: stable <stable@vger.kernel.org> # v4.9+

So... what are we doing here?

My understanding is that this should fix the same problem that commit
99f81afc139c6edd14d77a91ee91685a414a1c66 ("phy: micrel: Disable auto
negotiation on startup") fixed and that this micrel commit should thus
be reverted to improve MAC startup times which regressed.

Florian, any guidance?

Re: [PATCH v1] net: phy: fix auto-negotiation stall due to unavailable interrupt

[Alexander Kochetkov]

Hello David!

> 25 апр. 2017 г., в 17:36, David Miller <davem@davemloft.net> написал(а):
> 
> So... what are we doing here?
> 
> My understanding is that this should fix the same problem that commit
> 99f81afc139c6edd14d77a91ee91685a414a1c66 ("phy: micrel: Disable auto
> negotiation on startup") fixed and that this micrel commit should thus
> be reverted to improve MAC startup times which regressed.
> 
> Florian, any guidance?

Yes, this should be done.

I aksed Alexandre to check if 99f81afc139c6edd14d77a91ee91685a414a1c66 ("phy: micrel: Disable auto
negotiation on startup») can be reverted, and he answered what it may do that
sometime this/next week.

Alexander.

Re: [PATCH v1] net: phy: fix auto-negotiation stall due to unavailable interrupt

[Alexandre Belloni]

Hi,

On 25/04/2017 at 18:25:30 +0300, Alexander Kochetkov wrote:
> Hello David!
> 
> > 25 апр. 2017 г., в 17:36, David Miller <davem@davemloft.net> написал(а):
> > 
> > So... what are we doing here?
> > 
> > My understanding is that this should fix the same problem that commit
> > 99f81afc139c6edd14d77a91ee91685a414a1c66 ("phy: micrel: Disable auto
> > negotiation on startup") fixed and that this micrel commit should thus
> > be reverted to improve MAC startup times which regressed.
> > 
> > Florian, any guidance?
> 
> Yes, this should be done.
> 
> I aksed Alexandre to check if 99f81afc139c6edd14d77a91ee91685a414a1c66 ("phy: micrel: Disable auto
> negotiation on startup») can be reverted, and he answered what it may do that
> sometime this/next week.
> 

Yes, it can be safely reverted after Alexander's patch. I had to test on
v4.7 because we are not using interrupts on those boards since v4.8
(another issue to be fixed).

As Florian pointed out, at the time I sent my patch, I didn't have time
to investigate whether this was affecting other phys, see
https://lkml.org/lkml/2016/2/26/766

I can send the revert or you can do it.

-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH v1] net: phy: fix auto-negotiation stall due to unavailable interrupt

[David Miller]

From: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Date: Tue, 25 Apr 2017 22:09:11 +0200

> Hi,
> 
> On 25/04/2017 at 18:25:30 +0300, Alexander Kochetkov wrote:
>> Hello David!
>> 
>> > 25 апр. 2017 г., в 17:36, David Miller <davem@davemloft.net> написал(а):
>> > 
>> > So... what are we doing here?
>> > 
>> > My understanding is that this should fix the same problem that commit
>> > 99f81afc139c6edd14d77a91ee91685a414a1c66 ("phy: micrel: Disable auto
>> > negotiation on startup") fixed and that this micrel commit should thus
>> > be reverted to improve MAC startup times which regressed.
>> > 
>> > Florian, any guidance?
>> 
>> Yes, this should be done.
>> 
>> I aksed Alexandre to check if 99f81afc139c6edd14d77a91ee91685a414a1c66 ("phy: micrel: Disable auto
>> negotiation on startup») can be reverted, and he answered what it may do that
>> sometime this/next week.
>> 
> 
> Yes, it can be safely reverted after Alexander's patch. I had to test on
> v4.7 because we are not using interrupts on those boards since v4.8
> (another issue to be fixed).
> 
> As Florian pointed out, at the time I sent my patch, I didn't have time
> to investigate whether this was affecting other phys, see
> https://lkml.org/lkml/2016/2/26/766
> 
> I can send the revert or you can do it.

I can take care of it, thanks for testing.

Re: [PATCH v1] net: phy: fix auto-negotiation stall due to unavailable interrupt

[Florian Fainelli]

On 04/25/2017 01:26 PM, David Miller wrote:
> From: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> Date: Tue, 25 Apr 2017 22:09:11 +0200
> 
>> Hi,
>>
>> On 25/04/2017 at 18:25:30 +0300, Alexander Kochetkov wrote:
>>> Hello David!
>>>
>>>> 25 апр. 2017 г., в 17:36, David Miller <davem@davemloft.net> написал(а):
>>>>
>>>> So... what are we doing here?
>>>>
>>>> My understanding is that this should fix the same problem that commit
>>>> 99f81afc139c6edd14d77a91ee91685a414a1c66 ("phy: micrel: Disable auto
>>>> negotiation on startup") fixed and that this micrel commit should thus
>>>> be reverted to improve MAC startup times which regressed.
>>>>
>>>> Florian, any guidance?
>>>
>>> Yes, this should be done.
>>>
>>> I aksed Alexandre to check if 99f81afc139c6edd14d77a91ee91685a414a1c66 ("phy: micrel: Disable auto
>>> negotiation on startup») can be reverted, and he answered what it may do that
>>> sometime this/next week.
>>>
>>
>> Yes, it can be safely reverted after Alexander's patch. I had to test on
>> v4.7 because we are not using interrupts on those boards since v4.8
>> (another issue to be fixed).
>>
>> As Florian pointed out, at the time I sent my patch, I didn't have time
>> to investigate whether this was affecting other phys, see
>> https://lkml.org/lkml/2016/2/26/766
>>
>> I can send the revert or you can do it.
> 
> I can take care of it, thanks for testing.

Thanks! Can you add the following Fixes tag:

Fixes: 321beec5047a (net: phy: Use interrupts when available in NOLINK
state)

BTW, can you have the netdev patchwork instance automatically accepted
Fixes: tag sent as replies to patches? (just like Acked-by, Reviewed-by
and so on are already accepted)?
-- 
Florian

Re: [PATCH v1] net: phy: fix auto-negotiation stall due to unavailable interrupt

[Alexandre Belloni]

On 25/04/2017 at 22:09:11 +0200, Alexandre Belloni wrote:
> Hi,
> 
> On 25/04/2017 at 18:25:30 +0300, Alexander Kochetkov wrote:
> > Hello David!
> > 
> > > 25 апр. 2017 г., в 17:36, David Miller <davem@davemloft.net> написал(а):
> > > 
> > > So... what are we doing here?
> > > 
> > > My understanding is that this should fix the same problem that commit
> > > 99f81afc139c6edd14d77a91ee91685a414a1c66 ("phy: micrel: Disable auto
> > > negotiation on startup") fixed and that this micrel commit should thus
> > > be reverted to improve MAC startup times which regressed.
> > > 
> > > Florian, any guidance?
> > 
> > Yes, this should be done.
> > 
> > I aksed Alexandre to check if 99f81afc139c6edd14d77a91ee91685a414a1c66 ("phy: micrel: Disable auto
> > negotiation on startup») can be reverted, and he answered what it may do that
> > sometime this/next week.
> > 
> 
> Yes, it can be safely reverted after Alexander's patch. I had to test on
> v4.7 because we are not using interrupts on those boards since v4.8
> (another issue to be fixed).
> 
> As Florian pointed out, at the time I sent my patch, I didn't have time
> to investigate whether this was affecting other phys, see
> https://lkml.org/lkml/2016/2/26/766
> 
> I can send the revert or you can do it.
> 

I've now tested on linux-next after fixing phy interrupts in the macb
driver and this also fixes the issue I was trying to solve with:
https://lkml.org/lkml/2016/4/15/786

-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH v1] net: phy: fix auto-negotiation stall due to unavailable interrupt

[David Miller]

From: Alexander Kochetkov <al.kochet@gmail.com>
Date: Thu, 20 Apr 2017 14:00:04 +0300

> 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 patch trigger PHY state machine to update link state if PHY was requested to
> do auto-negotiation and auto-negotiation complete flag already set.
> 
> During power-up cycle the PHY do auto-negotiation, generate interrupt and set
> auto-negotiation complete flag. Interrupt is handled by PHY state machine but
> doesn't update link state because PHY is in PHY_READY state. After some time
> MAC bring up, start and request PHY to do auto-negotiation. If there are no new
> settings to advertise genphy_config_aneg() doesn't start PHY auto-negotiation.
> PHY continue to stay in auto-negotiation complete state and doesn't fire
> interrupt. At the same time PHY state machine expect that PHY started
> auto-negotiation and is waiting for interrupt from PHY and it won't get it.
> 
> Signed-off-by: Alexander Kochetkov <al.kochet@gmail.com>
> Cc: stable <stable@vger.kernel.org> # v4.9+

Applied, and I reverted the micrel commit too.

Thanks.