From mboxrd@z Thu Jan 1 00:00:00 1970 From: Heiner Kallweit Subject: Re: [PATCH net-next 1/2] net: phy: replace PHY_HAS_INTERRUPT with a check for config_intr and ack_interrupt Date: Thu, 8 Nov 2018 23:40:38 +0100 Message-ID: References: <193e3970-8ce2-1221-357a-7b7f9f6aea76@gmail.com> <1ad7d0ef-7e7e-93b2-3b3c-3c4704b3b0cc@gmail.com> <96112482-1293-0276-8bcc-44bf1beabd59@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: "netdev@vger.kernel.org" To: Florian Fainelli , Andrew Lunn , David Miller Return-path: Received: from mail-wr1-f68.google.com ([209.85.221.68]:41583 "EHLO mail-wr1-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727771AbeKIIS0 (ORCPT ); Fri, 9 Nov 2018 03:18:26 -0500 Received: by mail-wr1-f68.google.com with SMTP id v18-v6so3645986wrt.8 for ; Thu, 08 Nov 2018 14:40:44 -0800 (PST) In-Reply-To: <96112482-1293-0276-8bcc-44bf1beabd59@gmail.com> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 08.11.2018 23:33, Florian Fainelli wrote: > On 11/8/18 1:55 PM, Heiner Kallweit wrote: >> Flag PHY_HAS_INTERRUPT is used only here for this small check. I think >> using interrupts isn't possible if a driver defines neither >> config_intr nor ack_interrupts callback. So we can replace checking >> flag PHY_HAS_INTERRUPT with checking for these callbacks. >> >> Signed-off-by: Heiner Kallweit >> --- >> drivers/net/phy/phy_device.c | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c >> index d165a2c82..33e51b955 100644 >> --- a/drivers/net/phy/phy_device.c >> +++ b/drivers/net/phy/phy_device.c >> @@ -2104,8 +2104,9 @@ static int phy_probe(struct device *dev) >> /* Disable the interrupt if the PHY doesn't support it >> * but the interrupt is still a valid one >> */ >> - if (!(phydrv->flags & PHY_HAS_INTERRUPT) && >> - phy_interrupt_is_valid(phydev)) >> + if (!phydrv->config_intr && >> + !phydrv->ack_interrupt && >> + phy_interrupt_is_valid(phydev)) >> phydev->irq = PHY_POLL; > > I would introduce an inline helper function which checks for > drv->config_intr and config_ack_interrupt, that way if we ever have to > introduce an additional function in the future, we just update the > helper to check for that. > > Other than that, LGTM > OK, will add a helper and remove PHY_HAS_INTERRUPT from all drivers.