[net-next,V2] net: ks8851: Fix mixed module/builtin build

When either the SPI or PAR variant is compiled as module AND the other
variant is compiled as built-in, the following build error occurs:

arm-linux-gnueabi-ld: drivers/net/ethernet/micrel/ks8851_common.o: in function `ks8851_probe_common':
ks8851_common.c:(.text+0x1564): undefined reference to `__this_module'

Fix this by passing THIS_MODULE as argument to ks8851_probe_common(),
ks8851_register_mdiobus(), and ultimately __mdiobus_register() in the
ks8851_common.c.

Fixes: ef3631220d2b ("net: ks8851: Register MDIO bus and the internal PHY")
Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Heiner Kallweit <hkallweit1@gmail.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Lukas Wunner <lukas@wunner.de>
---
V2: Pass the THIS_MODULE into ks8851_common.c
---
 drivers/net/ethernet/micrel/ks8851.h        | 2 +-
 drivers/net/ethernet/micrel/ks8851_common.c | 9 +++++----
 drivers/net/ethernet/micrel/ks8851_par.c    | 2 +-
 drivers/net/ethernet/micrel/ks8851_spi.c    | 2 +-
 4 files changed, 8 insertions(+), 7 deletions(-)

On Sat, Jan 16, 2021 at 5:48 PM Marek Vasut <marex@denx.de> wrote:
>
> When either the SPI or PAR variant is compiled as module AND the other
> variant is compiled as built-in, the following build error occurs:
>
> arm-linux-gnueabi-ld: drivers/net/ethernet/micrel/ks8851_common.o: in function `ks8851_probe_common':
> ks8851_common.c:(.text+0x1564): undefined reference to `__this_module'
>
> Fix this by passing THIS_MODULE as argument to ks8851_probe_common(),
> ks8851_register_mdiobus(), and ultimately __mdiobus_register() in the
> ks8851_common.c.
>
> Fixes: ef3631220d2b ("net: ks8851: Register MDIO bus and the internal PHY")
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Heiner Kallweit <hkallweit1@gmail.com>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: Lukas Wunner <lukas@wunner.de>

I don't really like this version, as it does not actually solve the problem of
linking the same object file into both vmlinux and a loadable module, which
can have all kinds of side-effects besides that link failure you saw.

If you want to avoid exporting all those symbols, a simpler hack would
be to '#include "ks8851_common.c" from each of the two files, which
then always duplicates the contents (even when both are built-in), but
at least builds the file the correct way.

       Arnd

On 1/16/21 6:04 PM, Arnd Bergmann wrote:
> On Sat, Jan 16, 2021 at 5:48 PM Marek Vasut <marex@denx.de> wrote:
>>
>> When either the SPI or PAR variant is compiled as module AND the other
>> variant is compiled as built-in, the following build error occurs:
>>
>> arm-linux-gnueabi-ld: drivers/net/ethernet/micrel/ks8851_common.o: in function `ks8851_probe_common':
>> ks8851_common.c:(.text+0x1564): undefined reference to `__this_module'
>>
>> Fix this by passing THIS_MODULE as argument to ks8851_probe_common(),
>> ks8851_register_mdiobus(), and ultimately __mdiobus_register() in the
>> ks8851_common.c.
>>
>> Fixes: ef3631220d2b ("net: ks8851: Register MDIO bus and the internal PHY")
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> Cc: Andrew Lunn <andrew@lunn.ch>
>> Cc: Arnd Bergmann <arnd@arndb.de>
>> Cc: Heiner Kallweit <hkallweit1@gmail.com>
>> Cc: Jakub Kicinski <kuba@kernel.org>
>> Cc: Lukas Wunner <lukas@wunner.de>
> 
> I don't really like this version, as it does not actually solve the problem of
> linking the same object file into both vmlinux and a loadable module, which
> can have all kinds of side-effects besides that link failure you saw.
> 
> If you want to avoid exporting all those symbols, a simpler hack would
> be to '#include "ks8851_common.c" from each of the two files, which
> then always duplicates the contents (even when both are built-in), but
> at least builds the file the correct way.

That's the same as V1, isn't it ?

On Sat, Jan 16, 2021 at 6:56 PM Marek Vasut <marex@denx.de> wrote:
> On 1/16/21 6:04 PM, Arnd Bergmann wrote:
> > On Sat, Jan 16, 2021 at 5:48 PM Marek Vasut <marex@denx.de> wrote:
>
> > I don't really like this version, as it does not actually solve the problem of
> > linking the same object file into both vmlinux and a loadable module, which
> > can have all kinds of side-effects besides that link failure you saw.
> >
> > If you want to avoid exporting all those symbols, a simpler hack would
> > be to '#include "ks8851_common.c" from each of the two files, which
> > then always duplicates the contents (even when both are built-in), but
> > at least builds the file the correct way.
>
> That's the same as V1, isn't it ?

Ah, I had not actually looked at the original submission, but yes, that
was slightly better than v2, provided you make all symbols static to
avoid the new link error.

I still think that having three modules and exporting the symbols from
the common part as Heiner Kallweit suggested would be the best
way to do it.

        Arnd

On Sat, Jan 16, 2021 at 10:41 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
> >>

> >> It seems unlikely that a system uses both, the parallel *and* the SPI

> >> variant of the ks8851.  So the additional memory necessary because of

> >> code duplication wouldn't matter in practice.

> >

> > I have a board with both options populated on my desk, sorry.

>

> Making the common part a separate module shouldn't be that hard.

> AFAICS it would just take:

> - export 4 functions from common

> - extend Kconfig

> - extend Makefile

> One similar configuration that comes to my mind and could be used as

> template is SPI_FSL_LIB.


There is no need to even change Kconfig, just simplify the Makefile to

obj-$(CONFIG_KS8851) += ks8851_common.o ks8851_spi.o
obj-$(CONFIG_KS8851_MLL) += ks8851_common.o ks8851_par.o

This will do the right thing and build ks8851_common.ko into
vmlinux if at least one of the two front-ends is built-in, and
otherwise build it at a loadable module if there is another
module using it.

         Arnd