From mboxrd@z Thu Jan 1 00:00:00 1970 From: Heiner Kallweit Subject: Re: [PATCH net] net: phy: consider PHY_IGNORE_INTERRUPT in phy_start_aneg_priv Date: Fri, 20 Jul 2018 23:22:06 +0200 Message-ID: References: <013532d7-ed33-354b-81d9-f69ad534ebe3@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Cc: "netdev@vger.kernel.org" To: Florian Fainelli , Andrew Lunn , David Miller Return-path: Received: from mail-wm0-f67.google.com ([74.125.82.67]:37122 "EHLO mail-wm0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728167AbeGTWMS (ORCPT ); Fri, 20 Jul 2018 18:12:18 -0400 Received: by mail-wm0-f67.google.com with SMTP id a19-v6so10989126wmb.2 for ; Fri, 20 Jul 2018 14:22:14 -0700 (PDT) In-Reply-To: <013532d7-ed33-354b-81d9-f69ad534ebe3@gmail.com> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 20.07.2018 11:36, Florian Fainelli wrote: > > > On 07/18/2018 11:15 PM, Heiner Kallweit wrote: >> The situation described in the comment can occur also with >> PHY_IGNORE_INTERRUPT, therefore change the condition to include it. > > Yes indeed! You might want to locate the offending commit to provide a > fixes tag so this could be backported to stable trees. > Fixes: f555f34fdc58 ("net: phy: fix auto-negotiation stall due to unavailable interrupt") Is it sufficient this way or better re-submit the patch? > Also, for net-next, we may want to introduce a helper which checks for > phydev->irq != PHY_POLL that we can use consistently as a way to tell > that the conditions applies to either PHY_IGNORE_INTERRUPT or > phydev->irq is valid? > Yes, will put it on my agenda. Thanks for the feedback. Heiner >> >> Signed-off-by: Heiner Kallweit >> --- >> drivers/net/phy/phy.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c >> index d2baedc4..914fe8e6 100644 >> --- a/drivers/net/phy/phy.c >> +++ b/drivers/net/phy/phy.c >> @@ -519,7 +519,7 @@ static int phy_start_aneg_priv(struct phy_device *phydev, bool sync) >> * negotiation may already be done and aneg interrupt may not be >> * generated. >> */ >> - if (phy_interrupt_is_valid(phydev) && (phydev->state == PHY_AN)) { >> + if (phydev->irq != PHY_POLL && phydev->state == PHY_AN) { >> err = phy_aneg_done(phydev); >> if (err > 0) { >> trigger = true; >> >