From mboxrd@z Thu Jan 1 00:00:00 1970 From: Florian Fainelli 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 14:33:37 -0800 Message-ID: <96112482-1293-0276-8bcc-44bf1beabd59@gmail.com> References: <193e3970-8ce2-1221-357a-7b7f9f6aea76@gmail.com> <1ad7d0ef-7e7e-93b2-3b3c-3c4704b3b0cc@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: "netdev@vger.kernel.org" To: Heiner Kallweit , Andrew Lunn , David Miller Return-path: Received: from mail-pg1-f196.google.com ([209.85.215.196]:42224 "EHLO mail-pg1-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727203AbeKIILZ (ORCPT ); Fri, 9 Nov 2018 03:11:25 -0500 Received: by mail-pg1-f196.google.com with SMTP id i4-v6so9469096pgq.9 for ; Thu, 08 Nov 2018 14:33:46 -0800 (PST) In-Reply-To: <1ad7d0ef-7e7e-93b2-3b3c-3c4704b3b0cc@gmail.com> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: 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 -- Florian