| Message ID | 20210115134239.126152-1-marex@denx.de |
|---|---|
| State | New |
| Headers | show |
| Series | [net-next] net: ks8851: Fix mixed module/builtin build | expand |
On Fri, Jan 15, 2021 at 02:42:39PM +0100, Marek Vasut 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 including the ks8851_common.c in both ks8851_spi.c and > ks8851_par.c. The DEBUG macro is defined in ks8851_common.c, so it > does not have to be defined again. DEBUG should not be defined for production code. So i would remove it altogether. There is kconfig'ury you can use to make them both the same. But i'm not particularly good with it. Andrew
On 1/15/21 4:00 PM, Andrew Lunn wrote: > On Fri, Jan 15, 2021 at 02:42:39PM +0100, Marek Vasut 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 including the ks8851_common.c in both ks8851_spi.c and >> ks8851_par.c. The DEBUG macro is defined in ks8851_common.c, so it >> does not have to be defined again. > > DEBUG should not be defined for production code. So i would remove it > altogether. > > There is kconfig'ury you can use to make them both the same. But i'm > not particularly good with it. We had discussion about this module/builtin topic in ks8851 before, so I was hoping someone might provide a better suggestion.
On Fri, Jan 15, 2021 at 04:05:57PM +0100, Marek Vasut wrote: > On 1/15/21 4:00 PM, Andrew Lunn wrote: > > On Fri, Jan 15, 2021 at 02:42:39PM +0100, Marek Vasut 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 including the ks8851_common.c in both ks8851_spi.c and > > > ks8851_par.c. The DEBUG macro is defined in ks8851_common.c, so it > > > does not have to be defined again. > > > > DEBUG should not be defined for production code. So i would remove it > > altogether. > > > > There is kconfig'ury you can use to make them both the same. But i'm > > not particularly good with it. > > We had discussion about this module/builtin topic in ks8851 before, so I was > hoping someone might provide a better suggestion. Try Arnd Bergmann. He is good with this sort of thing. Andrew
Hi Marek,
I love your patch! Yet something to improve:
[auto build test ERROR on net-next/master]
url: https://github.com/0day-ci/linux/commits/Marek-Vasut/net-ks8851-Fix-mixed-module-builtin-build/20210115-215024
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 1d9f03c0a15fa01aa14fb295cbc1236403fceb0b
config: riscv-allyesconfig (attached as .config)
compiler: riscv64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/e11965ba009b3247ecbbb87730c724405eae8753
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Marek-Vasut/net-ks8851-Fix-mixed-module-builtin-build/20210115-215024
git checkout e11965ba009b3247ecbbb87730c724405eae8753
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=riscv
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
riscv64-linux-ld: drivers/net/ethernet/micrel/ks8851_par.o: in function `ks8851_remove_common':
>> (.text+0x469a): multiple definition of `ks8851_remove_common'; drivers/net/ethernet/micrel/ks8851_spi.o:(.text+0x47a4): first defined here
riscv64-linux-ld: drivers/net/ethernet/micrel/ks8851_par.o: in function `ks8851_probe_common':
>> (.text+0x360c): multiple definition of `ks8851_probe_common'; drivers/net/ethernet/micrel/ks8851_spi.o:(.text+0x3570): first defined here
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
On 1/15/21 10:36 PM, Arnd Bergmann wrote: > On Fri, Jan 15, 2021 at 6:24 PM Heiner Kallweit <hkallweit1@gmail.com> wrote: >> On 15.01.2021 16:59, Andrew Lunn wrote: >>> On Fri, Jan 15, 2021 at 04:05:57PM +0100, Marek Vasut wrote: >>>> On 1/15/21 4:00 PM, Andrew Lunn wrote: >>>>> On Fri, Jan 15, 2021 at 02:42:39PM +0100, Marek Vasut 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 including the ks8851_common.c in both ks8851_spi.c and >>>>>> ks8851_par.c. The DEBUG macro is defined in ks8851_common.c, so it >>>>>> does not have to be defined again. >>>>> >>>>> DEBUG should not be defined for production code. So i would remove it >>>>> altogether. >>>>> >>>>> There is kconfig'ury you can use to make them both the same. But i'm >>>>> not particularly good with it. >>>> >>>> We had discussion about this module/builtin topic in ks8851 before, so I was >>>> hoping someone might provide a better suggestion. >>> >>> Try Arnd Bergmann. He is good with this sort of thing. >>> >> I'd say make ks8851_common.c a separate module. Then, if one of SPI / PAR >> is built in, ks8851_common needs to be built in too. To do so you'd have >> export all symbols from ks8851_common that you want to use in SPI /PAR. > > Yes, that should work, as long the common module does not reference > any symbols from the other two modules (it normally wouldn't), and all > globals in the common one are exported. > > You can also link everything into a single module, but then you have > to deal with registering two device_driver structures from a single > init function, which would undo some of cleanup. Maybe just passing THIS_MODULE around is even better way to do it, I CCed you on the V2, [PATCH net-next V2] net: ks8851: Fix mixed module/builtin build .
diff --git a/drivers/net/ethernet/micrel/Makefile b/drivers/net/ethernet/micrel/Makefile index 5cc00d22c708..1aedfe0ecaf1 100644 --- a/drivers/net/ethernet/micrel/Makefile +++ b/drivers/net/ethernet/micrel/Makefile @@ -5,7 +5,7 @@ obj-$(CONFIG_KS8842) += ks8842.o obj-$(CONFIG_KS8851) += ks8851.o -ks8851-objs = ks8851_common.o ks8851_spi.o +ks8851-objs = ks8851_spi.o obj-$(CONFIG_KS8851_MLL) += ks8851_mll.o -ks8851_mll-objs = ks8851_common.o ks8851_par.o +ks8851_mll-objs = ks8851_par.o obj-$(CONFIG_KSZ884X_PCI) += ksz884x.o diff --git a/drivers/net/ethernet/micrel/ks8851_par.c b/drivers/net/ethernet/micrel/ks8851_par.c index 3bab0cb2b1a5..e9131cc8a57b 100644 --- a/drivers/net/ethernet/micrel/ks8851_par.c +++ b/drivers/net/ethernet/micrel/ks8851_par.c @@ -8,7 +8,7 @@ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt -#define DEBUG +#include "ks8851_common.c" #include <linux/interrupt.h> #include <linux/module.h> diff --git a/drivers/net/ethernet/micrel/ks8851_spi.c b/drivers/net/ethernet/micrel/ks8851_spi.c index 4ec7f1615977..53779d66f747 100644 --- a/drivers/net/ethernet/micrel/ks8851_spi.c +++ b/drivers/net/ethernet/micrel/ks8851_spi.c @@ -8,7 +8,7 @@ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt -#define DEBUG +#include "ks8851_common.c" #include <linux/interrupt.h> #include <linux/module.h>
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 including the ks8851_common.c in both ks8851_spi.c and ks8851_par.c. The DEBUG macro is defined in ks8851_common.c, so it does not have to be defined again. Signed-off-by: Marek Vasut <marex@denx.de> Cc: Andrew Lunn <andrew@lunn.ch> Cc: Heiner Kallweit <hkallweit1@gmail.com> Cc: Jakub Kicinski <kuba@kernel.org> Cc: Lukas Wunner <lukas@wunner.de> --- NOTE: Maybe there is a better way? --- drivers/net/ethernet/micrel/Makefile | 4 ++-- drivers/net/ethernet/micrel/ks8851_par.c | 2 +- drivers/net/ethernet/micrel/ks8851_spi.c | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-)