[mtd:spi-mem-ecc 30/30] ld.lld: error: undefined symbol: nand_ecc_unregister_on_host_hw_engine

[mtd:spi-mem-ecc 30/30] ld.lld: error: undefined symbol: nand_ecc_unregister_on_host_hw_engine

[kernel test robot]

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git spi-mem-ecc
head:   6d0fadec1de4434fce145b374ef25c665357fa60
commit: 6d0fadec1de4434fce145b374ef25c665357fa60 [30/30] spi: mxic: Add support for pipelined ECC operations
config: hexagon-randconfig-r001-20220130 (https://download.01.org/0day-ci/archive/20220201/202202011308.a6RlPiGp-lkp@intel.com/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 6b1e844b69f15bb7dffaf9365cd2b355d2eb7579)
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://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git/commit/?id=6d0fadec1de4434fce145b374ef25c665357fa60
        git remote add mtd https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git
        git fetch --no-tags mtd spi-mem-ecc
        git checkout 6d0fadec1de4434fce145b374ef25c665357fa60
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash

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 >>):

>> ld.lld: error: undefined symbol: nand_ecc_unregister_on_host_hw_engine
   >>> referenced by spi-mxic.c
   >>>               spi/spi-mxic.o:(mxic_spi_remove) in archive drivers/built-in.a
   >>> referenced by spi-mxic.c
   >>>               spi/spi-mxic.o:(mxic_spi_remove) in archive drivers/built-in.a

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

Re: [mtd:spi-mem-ecc 30/30] ld.lld: error: undefined symbol: nand_ecc_unregister_on_host_hw_engine

[Miquel Raynal]

Hi Mark,

lkp@intel.com wrote on Tue, 1 Feb 2022 13:28:45 +0800:

> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git spi-mem-ecc
> head:   6d0fadec1de4434fce145b374ef25c665357fa60
> commit: 6d0fadec1de4434fce145b374ef25c665357fa60 [30/30] spi: mxic: Add support for pipelined ECC operations
> config: hexagon-randconfig-r001-20220130 (https://download.01.org/0day-ci/archive/20220201/202202011308.a6RlPiGp-lkp@intel.com/config)
> compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 6b1e844b69f15bb7dffaf9365cd2b355d2eb7579)
> 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://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git/commit/?id=6d0fadec1de4434fce145b374ef25c665357fa60
>         git remote add mtd https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git
>         git fetch --no-tags mtd spi-mem-ecc
>         git checkout 6d0fadec1de4434fce145b374ef25c665357fa60
>         # save the config file to linux build tree
>         mkdir build_dir
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash
> 
> 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 >>):
> 
> >> ld.lld: error: undefined symbol: nand_ecc_unregister_on_host_hw_engine  
>    >>> referenced by spi-mxic.c
>    >>>               spi/spi-mxic.o:(mxic_spi_remove) in archive drivers/built-in.a
>    >>> referenced by spi-mxic.c
>    >>>               spi/spi-mxic.o:(mxic_spi_remove) in archive drivers/built-in.a  

I've failed to prevent faulty configurations with regular depends
on/select keywords, so I've come with a new solution which received a
successful build coverage test from the 0-day robots.

In order to still be able to use the spi controller driver (=y) while
mtd is =m or =n, I need to add an IS_REACHABLE() check in a couple of
headers. This way we can just imply the right MTD symbols from the
SPI_MXIC Kconfig entry.

I'll send a v11 with these changes which seem to rather cleanly address
all situations. I don't think I can do without these extra checks as
select/imply will not propagate the real value of the Kconfig symbol
(y/m) like depends on does.

Thanks,
Miquèl

Re: [mtd:spi-mem-ecc 30/30] ld.lld: error: undefined symbol: nand_ecc_unregister_on_host_hw_engine

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 574 bytes --]

On Wed, Feb 02, 2022 at 03:45:04PM +0100, Miquel Raynal wrote:

> I've failed to prevent faulty configurations with regular depends
> on/select keywords, so I've come with a new solution which received a
> successful build coverage test from the 0-day robots.

> In order to still be able to use the spi controller driver (=y) while
> mtd is =m or =n, I need to add an IS_REACHABLE() check in a couple of
> headers. This way we can just imply the right MTD symbols from the
> SPI_MXIC Kconfig entry.

Isn't this just a case where we shouldn't allow MTD to be built modular?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [mtd:spi-mem-ecc 30/30] ld.lld: error: undefined symbol: nand_ecc_unregister_on_host_hw_engine

[Miquel Raynal]

broonie@kernel.org wrote on Wed, 2 Feb 2022 15:20:05 +0000:

> On Wed, Feb 02, 2022 at 03:45:04PM +0100, Miquel Raynal wrote:
> 
> > I've failed to prevent faulty configurations with regular depends
> > on/select keywords, so I've come with a new solution which received a
> > successful build coverage test from the 0-day robots.  
> 
> > In order to still be able to use the spi controller driver (=y) while
> > mtd is =m or =n, I need to add an IS_REACHABLE() check in a couple of
> > headers. This way we can just imply the right MTD symbols from the
> > SPI_MXIC Kconfig entry.  
> 
> Isn't this just a case where we shouldn't allow MTD to be built modular?

How would you do that in a nice Kconfig way?

Re: [mtd:spi-mem-ecc 30/30] ld.lld: error: undefined symbol: nand_ecc_unregister_on_host_hw_engine

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 773 bytes --]

On Wed, Feb 02, 2022 at 04:34:52PM +0100, Miquel Raynal wrote:
> > On Wed, Feb 02, 2022 at 03:45:04PM +0100, Miquel Raynal wrote:

> > > I've failed to prevent faulty configurations with regular depends
> > > on/select keywords, so I've come with a new solution which received a
> > > successful build coverage test from the 0-day robots.  

> > > In order to still be able to use the spi controller driver (=y) while
> > > mtd is =m or =n, I need to add an IS_REACHABLE() check in a couple of
> > > headers. This way we can just imply the right MTD symbols from the
> > > SPI_MXIC Kconfig entry.  

> > Isn't this just a case where we shouldn't allow MTD to be built modular?

> How would you do that in a nice Kconfig way?

depends on MTD=y if SPI_MXC=y

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [mtd:spi-mem-ecc 30/30] ld.lld: error: undefined symbol: nand_ecc_unregister_on_host_hw_engine

[Miquel Raynal]

Hi Mark,

broonie@kernel.org wrote on Wed, 2 Feb 2022 16:15:35 +0000:

> On Wed, Feb 02, 2022 at 04:34:52PM +0100, Miquel Raynal wrote:
> > > On Wed, Feb 02, 2022 at 03:45:04PM +0100, Miquel Raynal wrote:  
> 
> > > > I've failed to prevent faulty configurations with regular depends
> > > > on/select keywords, so I've come with a new solution which received a
> > > > successful build coverage test from the 0-day robots.    
> 
> > > > In order to still be able to use the spi controller driver (=y) while
> > > > mtd is =m or =n, I need to add an IS_REACHABLE() check in a couple of
> > > > headers. This way we can just imply the right MTD symbols from the
> > > > SPI_MXIC Kconfig entry.    
> 
> > > Isn't this just a case where we shouldn't allow MTD to be built modular?  
> 
> > How would you do that in a nice Kconfig way?  
> 
> depends on MTD=y if SPI_MXC=y

In this case I believe we should also add

depends on MTD=m if SPI_MXC=m ?

Anyway, this would force building the ECC support (and MTD...) even
though we don't need it in most cases.

My idea was to give people the right to only select SPI_MXIC without
really caring about MTD/ECC support at all because this is IMHO a
valid use case. We would then save a few kiB of extra MTD fat.

What do you think?

Thanks,
Miquèl

Re: [mtd:spi-mem-ecc 30/30] ld.lld: error: undefined symbol: nand_ecc_unregister_on_host_hw_engine

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1012 bytes --]

On Wed, Feb 02, 2022 at 06:35:00PM +0100, Miquel Raynal wrote:

> > depends on MTD=y if SPI_MXC=y

> In this case I believe we should also add

> depends on MTD=m if SPI_MXC=m ?

It doesn't specifically need MTD to be a module so just a straight
dependency should be fine I guess.

> Anyway, this would force building the ECC support (and MTD...) even
> though we don't need it in most cases.

> My idea was to give people the right to only select SPI_MXIC without
> really caring about MTD/ECC support at all because this is IMHO a
> valid use case. We would then save a few kiB of extra MTD fat.

Is that something that people actually do - does this controller get
used without the MTD functionality?  Most of these controllers seem to
be really bad generic SPI controllers that would rarely get used for
anything other than MTD devices, if this one is a useful generic
controller your approach makes more sense although I do worry about
people getting noticably worse performance if they don't build MTD in.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [mtd:spi-mem-ecc 30/30] ld.lld: error: undefined symbol: nand_ecc_unregister_on_host_hw_engine

[Miquel Raynal]

Hi Mark,

broonie@kernel.org wrote on Wed, 2 Feb 2022 18:04:35 +0000:

> On Wed, Feb 02, 2022 at 06:35:00PM +0100, Miquel Raynal wrote:
> 
> > > depends on MTD=y if SPI_MXC=y  
> 
> > In this case I believe we should also add  
> 
> > depends on MTD=m if SPI_MXC=m ?  
> 
> It doesn't specifically need MTD to be a module so just a straight
> dependency should be fine I guess.
> 
> > Anyway, this would force building the ECC support (and MTD...) even
> > though we don't need it in most cases.  
> 
> > My idea was to give people the right to only select SPI_MXIC without
> > really caring about MTD/ECC support at all because this is IMHO a
> > valid use case. We would then save a few kiB of extra MTD fat.  
> 
> Is that something that people actually do - does this controller get
> used without the MTD functionality?  Most of these controllers seem to
> be really bad generic SPI controllers that would rarely get used for
> anything other than MTD devices, if this one is a useful generic
> controller your approach makes more sense although I do worry about
> people getting noticably worse performance if they don't build MTD in.

This one in particular really is a SPI controller, and can really be
used without MTD at all. I don't know how good are the performances
with it but it is sure that there is no performance hit when the MTD
stack is not pulled in.

Also, for the example I mentioned CONFIG_MTD but in fact I simplified a
bit the situation as this driver is actually implying
CONFIG_MTD_NAND_ECC which selects CONFIG_MTD_NAND_CORE.

CONFIG_MTD_NAND_CORE is a subset of MTD. Building this subset
really is not needed when playing with eg. a SPI-NOR. And even with a
SPI-NAND which would require CONFIG_MTD_NAND_CORE anyway, it would not
be necessarily to have CONFIG_MTD_NAND_ECC enabled because most NANDs
have on-die support for it and thus do not need extra code to handle
bitflips.

That is the reason why I felt a bit puzzled in the first place by the
need to pull all MTD in a build which would select this SPI controller
driver.

Thanks,
Miquèl