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@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.