[PATCH] net: phylib: fix interrupts re-enablement in phy_start

[PATCH] net: phylib: fix interrupts re-enablement in phy_start

[shh.xie]

From: Shaohui Xie <Shaohui.Xie@nxp.com>

If phy was suspended and is starting, current driver always enable
phy's interrupts, if phy works in polling, phy can raise unexpected
interrupt which will not be handled, the interrupt will block system
enter suspend again. So interrupts should only be re-enabled if phy
works in interrupt.

Signed-off-by: Shaohui Xie <Shaohui.Xie@nxp.com>
---
 drivers/net/phy/phy.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 6f221c8..baa5ecb 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -871,9 +871,11 @@ void phy_start(struct phy_device *phydev)
 		break;
 	case PHY_HALTED:
 		/* make sure interrupts are re-enabled for the PHY */
-		err = phy_enable_interrupts(phydev);
-		if (err < 0)
-			break;
+		if (phydev->irq != PHY_POLL) {
+			err = phy_enable_interrupts(phydev);
+			if (err < 0)
+				break;
+		}
 
 		phydev->state = PHY_RESUMING;
 		do_resume = true;
-- 
2.1.0.27.g96db324

Re: [PATCH] net: phylib: fix interrupts re-enablement in phy_start

[Florian Fainelli]

On 05/10/2016 02:42 AM, shh.xie@gmail.com wrote:
> From: Shaohui Xie <Shaohui.Xie@nxp.com>
> 
> If phy was suspended and is starting, current driver always enable
> phy's interrupts, if phy works in polling, phy can raise unexpected
> interrupt which will not be handled, the interrupt will block system
> enter suspend again. So interrupts should only be re-enabled if phy
> works in interrupt.

Your explanation makes sense. The commit message could you use some
improvements like s/phy/PHY/ and "works in interrupt mode", but that is
not a huge thing.

> 
> Signed-off-by: Shaohui Xie <Shaohui.Xie@nxp.com>

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

> ---
>  drivers/net/phy/phy.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index 6f221c8..baa5ecb 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -871,9 +871,11 @@ void phy_start(struct phy_device *phydev)
>  		break;
>  	case PHY_HALTED:
>  		/* make sure interrupts are re-enabled for the PHY */
> -		err = phy_enable_interrupts(phydev);
> -		if (err < 0)
> -			break;
> +		if (phydev->irq != PHY_POLL) {
> +			err = phy_enable_interrupts(phydev);
> +			if (err < 0)
> +				break;
> +		}
>  
>  		phydev->state = PHY_RESUMING;
>  		do_resume = true;
> 


-- 
Florian

Re: [PATCH] net: phylib: fix interrupts re-enablement in phy_start

[David Miller]

From: <shh.xie@gmail.com>
Date: Tue, 10 May 2016 17:42:26 +0800

> From: Shaohui Xie <Shaohui.Xie@nxp.com>
> 
> If phy was suspended and is starting, current driver always enable
> phy's interrupts, if phy works in polling, phy can raise unexpected
> interrupt which will not be handled, the interrupt will block system
> enter suspend again. So interrupts should only be re-enabled if phy
> works in interrupt.
> 
> Signed-off-by: Shaohui Xie <Shaohui.Xie@nxp.com>

Applied.

RE: [PATCH] net: phylib: fix interrupts re-enablement in phy_start

[Shaohui Xie]

> -----Original Message-----
> From: Florian Fainelli [mailto:f.fainelli@gmail.com]
> Sent: Wednesday, May 11, 2016 2:25 AM
> To: shh.xie@gmail.com; netdev@vger.kernel.org; davem@davemloft.net
> Cc: Shaohui Xie <shaohui.xie@nxp.com>; Andrew Lunn <andrew@lunn.ch>
> Subject: Re: [PATCH] net: phylib: fix interrupts re-enablement in phy_start
> 
> On 05/10/2016 02:42 AM, shh.xie@gmail.com wrote:
> > From: Shaohui Xie <Shaohui.Xie@nxp.com>
> >
> > If phy was suspended and is starting, current driver always enable
> > phy's interrupts, if phy works in polling, phy can raise unexpected
> > interrupt which will not be handled, the interrupt will block system
> > enter suspend again. So interrupts should only be re-enabled if phy
> > works in interrupt.
> 
> Your explanation makes sense. The commit message could you use some
> improvements like s/phy/PHY/ and "works in interrupt mode", but that is not a
> huge thing.
[S.H] Thank you for the comment.

I should make the commit message more precise and will make improvement in
future patch submission.

Regards,
Shaohui