[PATCH] phy: dp83867: add dp83867_{read,write}_mmd helpers

[Vladimir Oltean]

On Tue, May 24, 2022 at 01:31:41PM +0200, Rasmus Villemoes wrote:
> On 19/05/2022 16.38, Vladimir Oltean wrote:
> > Hi Rasmus,
> > 
> > On Tue, May 17, 2022 at 04:27:06PM +0200, Rasmus Villemoes wrote:
> >> Since the phy_{read,write}_mmd functions are static inlines using
> >> other static inline functions, they cause code using them to explode.
> >>
> >> Defining local wrappers cuts the size of the generated code by 50%:
> >>
> >> $ size drivers/net/phy/dp83867.o.{before,after}
> >>    text    data     bss     dec     hex filename
> >>    4873     112       0    4985    1379 drivers/net/phy/dp83867.o.before
> >>    2413     112       0    2525     9dd drivers/net/phy/dp83867.o.after
> >>
> >> Of course, most of that improvement could also be had by making the
> >> phy_*_mmd functions out-of-line, and they probably should be, but this
>    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> >> still has the advantage of avoiding passing the DP83867_DEVADDR
> >> argument at all call sites, which allows lines to be unwrapped (and
> >> probably also gives a little .text reduction by itself).
> >>
> >> Signed-off-by: Rasmus Villemoes <rasmus.villemoes at prevas.dk>
> >> ---
> > 
> > Have you considered making phy_read_mmd() and phy_write_mmd() non-inline?
> > There are few users, but it looks like they would all benefit from this.
> 
> Yes, I wrote precisely that in the commit message. But the problem with
> that is finding some TU to put them in which is guaranteed to be built
> and included by all current users. This localized change gives just
> about the same benefit in .text, plus the line unwrapping. And nothing
> prevents somebody later from figuring out a proper place to put
> out-of-line versions of these phy_*_mmd functions.

I don't see why the translation unit you mention cannot be drivers/net/phy/phy.c.
For phy_read_mmd() and phy_write_mmd() this is even pretty easy to see,
as all the callers are either PHY drivers or cmd/mdio.c which itself
depends on CONFIG_PHYLIB.

The phy_read() and phy_write() calls themselves can also be probably be
uninlined as a further exercise, but I didn't request that.

But yeah, ok, whatever.