Received-SPF: pass (google.com: domain of linux-kernel+bounces-89535-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Sat, 2 Mar 2024 17:25:45 +0000
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Heiner Kallweit <hkallweit1@gmail.com>
Cc: Oleksij Rempel <o.rempel@pengutronix.de>, Wei Fang <wei.fang@nxp.com>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Andrew Lunn <andrew@lunn.ch>,
	Florian Fainelli <florian.fainelli@broadcom.com>,
	kernel@pengutronix.de, linux-kernel@vger.kernel.org,
	netdev@vger.kernel.org, Shenwei Wang <shenwei.wang@nxp.com>,
	Clark Wang <xiaoning.wang@nxp.com>,
	NXP Linux Team <linux-imx@nxp.com>
Subject: Re: [PATCH net-next v8 3/8] net: phy: Add helper to set EEE Clock
 stop enable bit
Message-ID: <ZeNhGURTEfzwhikL@shell.armlinux.org.uk>
References: <20240301100153.927743-1-o.rempel@pengutronix.de>
 <20240301100153.927743-4-o.rempel@pengutronix.de>
 <d550b591-cd83-4ac6-8fd5-f5e0e2ad71d9@gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <d550b591-cd83-4ac6-8fd5-f5e0e2ad71d9@gmail.com>
Sender: Russell King (Oracle) <linux@armlinux.org.uk>

On Sat, Mar 02, 2024 at 06:16:34PM +0100, Heiner Kallweit wrote:
> On 01.03.2024 11:01, Oleksij Rempel wrote:
> > From: Andrew Lunn <andrew@lunn.ch>
> > 
> > The MAC driver can request that the PHY stops the clock during EEE
> > LPI. This has normally been does as part of phy_init_eee(), however
> > that function is overly complex and often wrongly used. Add a
> > standalone helper, to aid removing phy_init_eee().
> > 
> > Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> > Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
> > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > ---
> >  drivers/net/phy/phy.c | 20 ++++++++++++++++++++
> >  include/linux/phy.h   |  1 +
> >  2 files changed, 21 insertions(+)
> > 
> > diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> > index 2bc0a7d51c63f..ab18b0d9beb47 100644
> > --- a/drivers/net/phy/phy.c
> > +++ b/drivers/net/phy/phy.c
> > @@ -1579,6 +1579,26 @@ void phy_mac_interrupt(struct phy_device *phydev)
> >  }
> >  EXPORT_SYMBOL(phy_mac_interrupt);
> >  
> > +/**
> > + * phy_eee_clk_stop_enable - Clock should stop during LIP
> > + * @phydev: target phy_device struct
> > + *
> > + * Description: Program the MMD register 3.0 setting the "Clock stop enable"
> > + * bit.
> > + */
> > +int phy_eee_clk_stop_enable(struct phy_device *phydev)
> > +{
> > +	int ret;
> > +
> > +	mutex_lock(&phydev->lock);
> > +	ret = phy_set_bits_mmd(phydev, MDIO_MMD_PCS, MDIO_CTRL1,
> > +			       MDIO_PCS_CTRL1_CLKSTOP_EN);
> > +	mutex_unlock(&phydev->lock);
> > +
> > +	return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(phy_eee_clk_stop_enable);
> > +
> I don't see a user of this function in the series.
> Based on the commit description, wouldn't it be better to
> make this patch part of a future series removing
> phy_init_eee()?

That depends who is going to do that work. If it's individual driver
maintainers, then I think we want this to go in along with this series
so that we don't end up with individual driver maintainers having to
carry this patch, and submissions ending up with multiple copies of
this patch or depending on other maintainers submissions.

On the other hand, if someone is going to go through all the network
drivers and update them as one series, then it probably makes more
sense to move this to that series.

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