[SRU,G/raspi,F/raspi] net: lan78xx: Ack pending PHY ints when resetting

From: Phil Elwell <phil@raspberrypi.com>

BugLink: https://bugs.launchpad.net/bugs/1890487

lan78xx_link_reset explicitly clears the MAC's view of the PHY's IRQ
status. In doing so it potentially leaves the PHY with a pending
interrupt that will never be acknowledged, at which point no further
interrupts will be generated.

Avoid the problem by acknowledging any pending PHY interrupt after
clearing the MAC's status bit.

See: https://github.com/raspberrypi/linux/issues/2937

Signed-off-by: Phil Elwell <phil@raspberrypi.com>

(cherry picked from commit 44c11fa2063f00994cbce6708097f521f0b2c493 rpi-5.4.y)
Signed-off-by: Juerg Haefliger <juergh@canonical.com>
---
 drivers/net/usb/lan78xx.c | 3 +++
 1 file changed, 3 insertions(+)

On 16/12/2020 12:26, Juerg Haefliger wrote:
> From: Phil Elwell <phil@raspberrypi.com>
> 
> BugLink: https://bugs.launchpad.net/bugs/1890487
> 
> lan78xx_link_reset explicitly clears the MAC's view of the PHY's IRQ
> status. In doing so it potentially leaves the PHY with a pending
> interrupt that will never be acknowledged, at which point no further
> interrupts will be generated.
> 
> Avoid the problem by acknowledging any pending PHY interrupt after
> clearing the MAC's status bit.
> 
> See: https://github.com/raspberrypi/linux/issues/2937
> 
> Signed-off-by: Phil Elwell <phil@raspberrypi.com>
> 
> (cherry picked from commit 44c11fa2063f00994cbce6708097f521f0b2c493 rpi-5.4.y)
> Signed-off-by: Juerg Haefliger <juergh@canonical.com>
> ---
>  drivers/net/usb/lan78xx.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
> index ad93e7f28dde..dcd68781e730 100644
> --- a/drivers/net/usb/lan78xx.c
> +++ b/drivers/net/usb/lan78xx.c
> @@ -1181,6 +1181,9 @@ static int lan78xx_link_reset(struct lan78xx_net *dev)
>  	if (unlikely(ret < 0))
>  		return -EIO;
>  
> +	/* Acknowledge any pending PHY interrupt, lest it be the last */
> +	phy_read(phydev, LAN88XX_INT_STS);
> +
>  	phy_read_status(phydev);
>  
>  	if (!phydev->link && dev->link_on) {
> 

I don't see a SRU template for this in the bug. Once that's been added
I'll re-review.

Colin

On Wed, 16 Dec 2020 12:41:51 +0000
Colin Ian King <colin.king@canonical.com> wrote:

> On 16/12/2020 12:26, Juerg Haefliger wrote:
> > From: Phil Elwell <phil@raspberrypi.com>
> > 
> > BugLink: https://bugs.launchpad.net/bugs/1890487
> > 
> > lan78xx_link_reset explicitly clears the MAC's view of the PHY's IRQ
> > status. In doing so it potentially leaves the PHY with a pending
> > interrupt that will never be acknowledged, at which point no further
> > interrupts will be generated.
> > 
> > Avoid the problem by acknowledging any pending PHY interrupt after
> > clearing the MAC's status bit.
> > 
> > See: https://github.com/raspberrypi/linux/issues/2937
> > 
> > Signed-off-by: Phil Elwell <phil@raspberrypi.com>
> > 
> > (cherry picked from commit 44c11fa2063f00994cbce6708097f521f0b2c493 rpi-5.4.y)
> > Signed-off-by: Juerg Haefliger <juergh@canonical.com>
> > ---
> >  drivers/net/usb/lan78xx.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
> > index ad93e7f28dde..dcd68781e730 100644
> > --- a/drivers/net/usb/lan78xx.c
> > +++ b/drivers/net/usb/lan78xx.c
> > @@ -1181,6 +1181,9 @@ static int lan78xx_link_reset(struct lan78xx_net *dev)
> >  	if (unlikely(ret < 0))
> >  		return -EIO;
> >  
> > +	/* Acknowledge any pending PHY interrupt, lest it be the last */
> > +	phy_read(phydev, LAN88XX_INT_STS);
> > +
> >  	phy_read_status(phydev);
> >  
> >  	if (!phydev->link && dev->link_on) {
> >   
> 
> I don't see a SRU template for this in the bug. Once that's been added
> I'll re-review.

Done. Thanks for pointing that out.

...Juerg

 
> Colin

On 16.12.20 13:26, Juerg Haefliger wrote:
> From: Phil Elwell <phil@raspberrypi.com>
> 
> BugLink: https://bugs.launchpad.net/bugs/1890487
> 
> lan78xx_link_reset explicitly clears the MAC's view of the PHY's IRQ
> status. In doing so it potentially leaves the PHY with a pending
> interrupt that will never be acknowledged, at which point no further
> interrupts will be generated.
> 
> Avoid the problem by acknowledging any pending PHY interrupt after
> clearing the MAC's status bit.
> 
> See: https://github.com/raspberrypi/linux/issues/2937
> 
> Signed-off-by: Phil Elwell <phil@raspberrypi.com>
> 
> (cherry picked from commit 44c11fa2063f00994cbce6708097f521f0b2c493 rpi-5.4.y)
> Signed-off-by: Juerg Haefliger <juergh@canonical.com>

Acked-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com>

> ---
>   drivers/net/usb/lan78xx.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
> index ad93e7f28dde..dcd68781e730 100644
> --- a/drivers/net/usb/lan78xx.c
> +++ b/drivers/net/usb/lan78xx.c
> @@ -1181,6 +1181,9 @@ static int lan78xx_link_reset(struct lan78xx_net *dev)
>   	if (unlikely(ret < 0))
>   		return -EIO;
>   
> +	/* Acknowledge any pending PHY interrupt, lest it be the last */
> +	phy_read(phydev, LAN88XX_INT_STS);
> +
>   	phy_read_status(phydev);
>   
>   	if (!phydev->link && dev->link_on) {
>

On 2020-12-16 13:26:57 , Juerg Haefliger wrote:
> From: Phil Elwell <phil@raspberrypi.com>
> 
> BugLink: https://bugs.launchpad.net/bugs/1890487
> 
> lan78xx_link_reset explicitly clears the MAC's view of the PHY's IRQ
> status. In doing so it potentially leaves the PHY with a pending
> interrupt that will never be acknowledged, at which point no further
> interrupts will be generated.
> 
> Avoid the problem by acknowledging any pending PHY interrupt after
> clearing the MAC's status bit.
> 
> See: https://github.com/raspberrypi/linux/issues/2937
> 
> Signed-off-by: Phil Elwell <phil@raspberrypi.com>
> 
> (cherry picked from commit 44c11fa2063f00994cbce6708097f521f0b2c493 rpi-5.4.y)
> Signed-off-by: Juerg Haefliger <juergh@canonical.com>
Acked-by: Kelsey Skunberg <kelsey.skunberg@canonical.com>

> ---
>  drivers/net/usb/lan78xx.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
> index ad93e7f28dde..dcd68781e730 100644
> --- a/drivers/net/usb/lan78xx.c
> +++ b/drivers/net/usb/lan78xx.c
> @@ -1181,6 +1181,9 @@ static int lan78xx_link_reset(struct lan78xx_net *dev)
>  	if (unlikely(ret < 0))
>  		return -EIO;
>  
> +	/* Acknowledge any pending PHY interrupt, lest it be the last */
> +	phy_read(phydev, LAN88XX_INT_STS);
> +
>  	phy_read_status(phydev);
>  
>  	if (!phydev->link && dev->link_on) {
> -- 
> 2.25.1
> 
> 
> -- 
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

On 16.12.20 13:26, Juerg Haefliger wrote:
> From: Phil Elwell <phil@raspberrypi.com>
> 
> BugLink: https://bugs.launchpad.net/bugs/1890487
> 
> lan78xx_link_reset explicitly clears the MAC's view of the PHY's IRQ
> status. In doing so it potentially leaves the PHY with a pending
> interrupt that will never be acknowledged, at which point no further
> interrupts will be generated.
> 
> Avoid the problem by acknowledging any pending PHY interrupt after
> clearing the MAC's status bit.
> 
> See: https://github.com/raspberrypi/linux/issues/2937
> 
> Signed-off-by: Phil Elwell <phil@raspberrypi.com>
> 
> (cherry picked from commit 44c11fa2063f00994cbce6708097f521f0b2c493 rpi-5.4.y)
> Signed-off-by: Juerg Haefliger <juergh@canonical.com>
> ---
>   drivers/net/usb/lan78xx.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
> index ad93e7f28dde..dcd68781e730 100644
> --- a/drivers/net/usb/lan78xx.c
> +++ b/drivers/net/usb/lan78xx.c
> @@ -1181,6 +1181,9 @@ static int lan78xx_link_reset(struct lan78xx_net *dev)
>   	if (unlikely(ret < 0))
>   		return -EIO;
>   
> +	/* Acknowledge any pending PHY interrupt, lest it be the last */
> +	phy_read(phydev, LAN88XX_INT_STS);
> +
>   	phy_read_status(phydev);
>   
>   	if (!phydev->link && dev->link_on) {
> 

Applied to focal/linux-raspi and groovy/linux-raspi.

Thanks,
Kleber