Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 3 Jan 2023 10:27:20 +0000
From:   "Russell King (Oracle)" <linux@armlinux.org.uk>
To:     Florian Fainelli <f.fainelli@gmail.com>
Cc:     Michael Walle <michael@walle.cc>, Xu Liang <lxu@maxlinear.com>,
        Andrew Lunn <andrew@lunn.ch>,
        Heiner Kallweit <hkallweit1@gmail.com>,
        "David S . Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH RFC net-next v2 1/2] net: phy: allow a phy to opt-out of
 interrupt handling
Message-ID: <Y7QDCJyyJQBoaGl4@shell.armlinux.org.uk>
References: <20221228164008.1653348-1-michael@walle.cc>
 <20221228164008.1653348-2-michael@walle.cc>
 <f547b3b9-4c8f-b370-471a-0a7b5f025e50@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <f547b3b9-4c8f-b370-471a-0a7b5f025e50@gmail.com>
Sender: Russell King (Oracle) <linux@armlinux.org.uk>
Precedence: bulk

On Wed, Dec 28, 2022 at 08:49:35AM -0800, Florian Fainelli wrote:
> 
> 
> On 12/28/2022 8:40 AM, Michael Walle wrote:
> > Until now, it is not possible for a PHY driver to disable interrupts
> > during runtime. If a driver offers the .config_intr() as well as the
> > .handle_interrupt() ops, it is eligible for interrupt handling.
> > Introduce a new flag for the dev_flags property of struct phy_device, which
> > can be set by PHY driver to skip interrupt setup and fall back to polling
> > mode.
> > 
> > At the moment, this is used for the MaxLinear PHY which has broken
> > interrupt handling and there is a need to disable interrupts in some
> > cases.
> > 
> > Signed-off-by: Michael Walle <michael@walle.cc>
> > ---
> >   drivers/net/phy/phy_device.c | 7 +++++++
> >   include/linux/phy.h          | 2 ++
> >   2 files changed, 9 insertions(+)
> > 
> > diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> > index 716870a4499c..e4562859ac00 100644
> > --- a/drivers/net/phy/phy_device.c
> > +++ b/drivers/net/phy/phy_device.c
> > @@ -1487,6 +1487,13 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
> >   	phydev->interrupts = PHY_INTERRUPT_DISABLED;
> > +	/* PHYs can request to use poll mode even though they have an
> > +	 * associated interrupt line. This could be the case if they
> > +	 * detect a broken interrupt handling.
> > +	 */
> > +	if (phydev->dev_flags & PHY_F_NO_IRQ)
> > +		phydev->irq = PHY_POLL;
> 
> Cannot you achieve the same thing with the PHY driver mangling phydev->irq
> to a negative value, or is that too later already by the time your phy
> driver's probe function is running?
> 
> > +
> >   	/* Port is set to PORT_TP by default and the actual PHY driver will set
> >   	 * it to different value depending on the PHY configuration. If we have
> >   	 * the generic PHY driver we can't figure it out, thus set the old
> > diff --git a/include/linux/phy.h b/include/linux/phy.h
> > index 71eeb4e3b1fd..f1566c7e47a8 100644
> > --- a/include/linux/phy.h
> > +++ b/include/linux/phy.h
> > @@ -82,6 +82,8 @@ extern const int phy_10gbit_features_array[1];
> >   #define PHY_POLL_CABLE_TEST	0x00000004
> >   #define MDIO_DEVICE_IS_PHY	0x80000000
> > +#define PHY_F_NO_IRQ		0x80000000
> 
> Kudos for using the appropriate namespace for dev_flags :)

But eww for placement.

PHY_IS_INTERNAL, PHY_RST_AFTER_CLK_EN, PHY_POLL_CABLE_TEST and
MDIO_DEVICE_IS_PHY are all used for the MDIO driver's flags
member.

This new flag is used for the .dev_flags of phy_device - I feel
that it should be separated from the above definitions. I also
think it could do with a comment, because it's not obvious for
future changes that PHY_F_NO_IRQ is used with .dev_flags.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!