On Wed, Nov 01, 2023 at 04:55:19PM -0300, Luiz Angelo Daros de Luca wrote:

Hi Vladimir,

quoted

realtek-mdio is an MDIO driver while realtek-smi is a platform driver
implementing a bitbang protocol. They might never be used together in
a system to share RAM and not even installed together in small
systems. If I do need to share the code, I would just link it twice.
Would something like this be acceptable?

drivers/net/dsa/realtek/Makefile
-obj-$(CONFIG_NET_DSA_REALTEK_MDIO)     += realtek-mdio.o
-obj-$(CONFIG_NET_DSA_REALTEK_SMI)      += realtek-smi.o
+obj-$(CONFIG_NET_DSA_REALTEK_MDIO)     += realtek-mdio.o realtek_common.o
+obj-$(CONFIG_NET_DSA_REALTEK_SMI)      += realtek-smi.o realtek_common.o

You cannot link realtek_common.o into multiple .ko files. It generates
build warnings and it is being phased out.
https://patchwork.kernel.org/project/netdevbpf/cover/20221119225650.1044591-1-alobakin@pm.me/

Just a follow up.

It is not that simple to include a .c file into an existing single
file module. It looks like you need to rename the original file as all
linked objects must not conflict with the module name. The kernel
build seems to create a new object file for each module. Is there a
clearer way? I think #include a common .c file would not be
acceptable.

I tested your shared module suggestion. It is the clearest one but it
also increased the overall size quite a bit. Even linking two objects
seems to use the double of space used by the functions alone. These
are some values (mips)

drivers/net/dsa/realtek/realtek_common.o  5744  without exports
drivers/net/dsa/realtek/realtek_common.o 33472  exporting the two reset functions (assert/deassert)

drivers/net/dsa/realtek/realtek-mdio.o   18756  without the reset funcs (to be used as a module)
drivers/net/dsa/realtek/realtek-mdio.o   20480  including the realtek_common.c (#include <realtek_common.c>)
drivers/net/dsa/realtek/realtek-mdio.o   22696  linking the realtek_common.o

drivers/net/dsa/realtek/realtek-smi.o    30712  without the reset funcs (to be used as a module)
drivers/net/dsa/realtek/realtek-smi.o    34604  linking the realtek_common.o

drivers/net/dsa/realtek/realtek-mdio.ko  28800  without the reset funcs (it will use realtek_common.ko)
drivers/net/dsa/realtek/realtek-mdio.ko  32736  linking the realtek_common.o

drivers/net/dsa/realtek/realtek-smi.ko   40708  without the reset funcs (it will use realtek_common.ko)
drivers/net/dsa/realtek/realtek-smi.ko   44612  linking the realtek_common.o

In summary, we get about 1.5kb of code with the extra functions,
almost 4kb if we link a common object containing the functions and
33kb if we use a module for those two functions.

I can go with any option. I just need to know which one would be
accepted to update my patches.
1) keep duplicated functions on each file

Disadvantage: need to update the same functions twice, it becomes
possible for them to diverge, increases maintenance difficulty.

2) share the code including the .c on both

Including a .c file with a preprocessor #include is fragile, has to be
placed very carefully, etc. So it is also a time bomb from a maintenance
PoV. Maybe a header with static inline function definitions would be
worth considering, although I don't think it's common practice to do
this.

3) share the code linking a common object in both modules (and
renaming existing .c files)

Sharing object files is being phased out.

4) create a new module used by both modules.

I am suspicious of the numbers you provided above. What needs to be
compared is the reduction in size of realtek_mdio.ko and realtek_smi.ko,
compared to the size of the new realtek_common.ko. Also, this starts
being more and more worthwhile as more code goes into realtek_common.ko,
and I also mentioned a common probe/remove/shutdown as being viable
candidates. Looking at your figures, I'm not sure at which ones I need
to look in order to compute that metric.

The devices that would use this driver have very restricted storage
space. Every kbyte counts.

Well, in that case you could compile the code as built into the kernel,
and the module overhead would go away.