From mboxrd@z Thu Jan 1 00:00:00 1970 From: Heiner Kallweit Subject: Re: [PATCH net] net: phy: fix phy_start to consider PHY_IGNORE_INTERRUPT Date: Wed, 7 Feb 2018 20:39:20 +0100 Message-ID: References: <80518b40-3467-6e1b-7ee8-6e860afde049@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-wr0-f193.google.com ([209.85.128.193]:44921 "EHLO mail-wr0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754048AbeBGTjZ (ORCPT ); Wed, 7 Feb 2018 14:39:25 -0500 Received: by mail-wr0-f193.google.com with SMTP id v31so2295874wrc.11 for ; Wed, 07 Feb 2018 11:39:25 -0800 (PST) In-Reply-To: Sender: netdev-owner@vger.kernel.org List-ID: Am 07.02.2018 um 20:34 schrieb Florian Fainelli: > > > On 02/07/2018 11:26 AM, Heiner Kallweit wrote: >> Am 07.02.2018 um 20:06 schrieb Florian Fainelli: >>> >>> >>> On 02/07/2018 10:44 AM, Heiner Kallweit wrote: >>>> This condition wasn't adjusted when PHY_IGNORE_INTERRUPT (-2) was added >>>> long ago. In case of PHY_IGNORE_INTERRUPT the MAC interrupt indicates >>>> also PHY state changes and we should do what the symbol says. >>> >>> Do you use phy_enable_interrupts() to configure how the PHY interrupts >>> will be flowing through the Ethernet MAC? >>> >> No. And I'm not sure I understand your question correctly. > > No wonder, my question does not make sense, I read the test wrong. > >> The change applies the same behavior as e.g. in phy_connect_direct() >> where phy_start_interrupts() is called only if phy_dev->irq > 0. > > Not enough coffee, your change is fine, could you consider using > phy_interrupt_is_valid() instead for this test? > Sure. I was considering this already however wasn't sure because currently both ways to check for a valid interrupt are used in phylib. >> >>>> >>>> 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 f3313a129..50ed35a45 100644 >>>> --- a/drivers/net/phy/phy.c >>>> +++ b/drivers/net/phy/phy.c >>>> @@ -822,7 +822,7 @@ void phy_start(struct phy_device *phydev) >>>> phy_resume(phydev); >>>> >>>> /* make sure interrupts are re-enabled for the PHY */ >>>> - if (phydev->irq != PHY_POLL) { >>>> + if (phydev->irq > 0) { >>>> err = phy_enable_interrupts(phydev); >>>> if (err < 0) >>>> break; >>>> >>> >> >