Re: [PATCH net-next v6 2/7] net: ethtool: plumb PHY stats to PHY drivers

[Oleksij Rempel <o.rempel@pengutronix.de>]

On Thu, Jan 09, 2025 at 08:07:58AM -0800, Jakub Kicinski wrote:
> On Thu,  9 Jan 2025 10:44:52 +0100 Oleksij Rempel wrote:
> > +static inline void phy_ethtool_get_phy_stats(struct phy_device *phydev,
> > +					struct ethtool_eth_phy_stats *phy_stats,
> > +					struct ethtool_phy_stats *phydev_stats)
> > +{
> > +	ASSERT_RTNL();
> > +
> > +	if (!phylib_stubs)
> > +		return;
> > +
> > +	phylib_stubs->get_phy_stats(phydev, phy_stats, phydev_stats);
> > +}
> > +
> > +static inline void phy_ethtool_get_link_ext_stats(struct phy_device *phydev,
> > +				    struct ethtool_link_ext_stats *link_stats)
> > +{
> > +	ASSERT_RTNL();
> > +
> > +	if (!phylib_stubs)
> > +		return;
> > +
> > +	phylib_stubs->get_link_ext_stats(phydev, link_stats);
> > +}
> 
> So we traded on set of static inlines for another?
> What's wrong with adding a C source which is always built in?
> Like drivers/net/phy/stubs.c, maybe call it drivers/net/phy/accessors.c
> or drivers/net/phy/helpers.c

I chose the current stubs approach based on existing examples like
hw_timestamps. Any implementation, including the current one, will have
zero kernel size impact because each function is only used once. While
moving them to a C source file is an option, it doesn't seem necessary
given the current usage pattern. Do we really want to spend more time on
this for something that won’t impact functionality or size? :)

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |