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

[Ramon Fried]

On Tue, May 24, 2022 at 3:08 PM Vladimir Oltean <vladimir.oltean at nxp.com> wrote:
>
> 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.
Nack.
Let's fix the root problem, phy_write_mmd/phy_read_mmd should have
never been static inline.
They are too big for that.
I'll create a fix for that.