mtd: rawnand: ingenic: fix ingenic_ecc dependency

Message ID 20190617111110.2103786-1-arnd@arndb.de
State New
Headers show
Series
  • mtd: rawnand: ingenic: fix ingenic_ecc dependency
Related show

Commit Message

Arnd Bergmann June 17, 2019, 11:10 a.m.
The ecc code is called from the main ingenic_nand module, but the
Kconfig symbol gets selected by the dependent ones.

If the child drivers are loadable modules, this leads to a link
error:

drivers/mtd/nand/raw/ingenic/ingenic_nand.o: In function `ingenic_nand_remove':
ingenic_nand.c:(.text+0x1a1): undefined reference to `ingenic_ecc_release'
drivers/mtd/nand/raw/ingenic/ingenic_nand.o: In function `ingenic_nand_ecc_correct':
ingenic_nand.c:(.text+0x1fa): undefined reference to `ingenic_ecc_correct'
drivers/mtd/nand/raw/ingenic/ingenic_nand.o: In function `ingenic_nand_ecc_calculate':
ingenic_nand.c:(.text+0x255): undefined reference to `ingenic_ecc_calculate'
drivers/mtd/nand/raw/ingenic/ingenic_nand.o: In function `ingenic_nand_probe':
ingenic_nand.c:(.text+0x3ca): undefined reference to `of_ingenic_ecc_get'
ingenic_nand.c:(.text+0x685): undefined reference to `ingenic_ecc_release'

Rearrange this to have the ecc code linked the same way as the main
driver.

Fixes: 15de8c6efd0e ("mtd: rawnand: ingenic: Separate top-level and SoC specific code")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>

---
 drivers/mtd/nand/raw/ingenic/Kconfig  | 2 +-
 drivers/mtd/nand/raw/ingenic/Makefile | 5 ++++-
 2 files changed, 5 insertions(+), 2 deletions(-)

-- 
2.20.0

Comments

Paul Cercueil June 17, 2019, 11:24 a.m. | #1
Hi Arnd,


Le lun. 17 juin 2019 à 13:10, Arnd Bergmann <arnd@arndb.de> a écrit :
> The ecc code is called from the main ingenic_nand module, but the

> Kconfig symbol gets selected by the dependent ones.

> 

> If the child drivers are loadable modules, this leads to a link

> error:

> 

> drivers/mtd/nand/raw/ingenic/ingenic_nand.o: In function 

> `ingenic_nand_remove':

> ingenic_nand.c:(.text+0x1a1): undefined reference to 

> `ingenic_ecc_release'

> drivers/mtd/nand/raw/ingenic/ingenic_nand.o: In function 

> `ingenic_nand_ecc_correct':

> ingenic_nand.c:(.text+0x1fa): undefined reference to 

> `ingenic_ecc_correct'

> drivers/mtd/nand/raw/ingenic/ingenic_nand.o: In function 

> `ingenic_nand_ecc_calculate':

> ingenic_nand.c:(.text+0x255): undefined reference to 

> `ingenic_ecc_calculate'

> drivers/mtd/nand/raw/ingenic/ingenic_nand.o: In function 

> `ingenic_nand_probe':

> ingenic_nand.c:(.text+0x3ca): undefined reference to 

> `of_ingenic_ecc_get'

> ingenic_nand.c:(.text+0x685): undefined reference to 

> `ingenic_ecc_release'

> 

> Rearrange this to have the ecc code linked the same way as the main

> driver.


I think there's a better way to fix it, only in Kconfig.

* Add a bool symbol MTD_NAND_INGENIC_USE_HW_ECC
* Have the three ECC/BCH drivers select this symbol instead of
  MTD_NAND_INGENIC_ECC
* Add the following to the MTD_NAND_JZ4780 config option:
  "select MTD_NAND_INGENIC_ECC if MTD_NAND_INGENIC_USE_HW_ECC"


> Fixes: 15de8c6efd0e ("mtd: rawnand: ingenic: Separate top-level and 

> SoC specific code")

> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

> ---

>  drivers/mtd/nand/raw/ingenic/Kconfig  | 2 +-

>  drivers/mtd/nand/raw/ingenic/Makefile | 5 ++++-

>  2 files changed, 5 insertions(+), 2 deletions(-)

> 

> diff --git a/drivers/mtd/nand/raw/ingenic/Kconfig 

> b/drivers/mtd/nand/raw/ingenic/Kconfig

> index 19a96ce515c1..66b7cffdb0c2 100644

> --- a/drivers/mtd/nand/raw/ingenic/Kconfig

> +++ b/drivers/mtd/nand/raw/ingenic/Kconfig

> @@ -16,7 +16,7 @@ config MTD_NAND_JZ4780

>  if MTD_NAND_JZ4780

> 

>  config MTD_NAND_INGENIC_ECC

> -	tristate

> +	bool

> 

>  config MTD_NAND_JZ4740_ECC

>  	tristate "Hardware BCH support for JZ4740 SoC"

> diff --git a/drivers/mtd/nand/raw/ingenic/Makefile 

> b/drivers/mtd/nand/raw/ingenic/Makefile

> index 1ac4f455baea..5a55efc5d9bb 100644

> --- a/drivers/mtd/nand/raw/ingenic/Makefile

> +++ b/drivers/mtd/nand/raw/ingenic/Makefile

> @@ -2,7 +2,10 @@

>  obj-$(CONFIG_MTD_NAND_JZ4740) += jz4740_nand.o

>  obj-$(CONFIG_MTD_NAND_JZ4780) += ingenic_nand.o

> 

> -obj-$(CONFIG_MTD_NAND_INGENIC_ECC) += ingenic_ecc.o

> +ifdef CONFIG_MTD_NAND_INGENIC_ECC

> +obj-$(CONFIG_MTD_NAND_JZ4780) += ingenic_ecc.o

> +endif

> +

>  obj-$(CONFIG_MTD_NAND_JZ4740_ECC) += jz4740_ecc.o

>  obj-$(CONFIG_MTD_NAND_JZ4725B_BCH) += jz4725b_bch.o

>  obj-$(CONFIG_MTD_NAND_JZ4780_BCH) += jz4780_bch.o

> --

> 2.20.0

>
Arnd Bergmann June 17, 2019, 12:12 p.m. | #2
On Mon, Jun 17, 2019 at 1:24 PM Paul Cercueil <paul@crapouillou.net> wrote:

> I think there's a better way to fix it, only in Kconfig.

>

> * Add a bool symbol MTD_NAND_INGENIC_USE_HW_ECC

> * Have the three ECC/BCH drivers select this symbol instead of

>   MTD_NAND_INGENIC_ECC

> * Add the following to the MTD_NAND_JZ4780 config option:

>   "select MTD_NAND_INGENIC_ECC if MTD_NAND_INGENIC_USE_HW_ECC"


I don't see much difference to my approach here, but if you want
to submit that version with 'Reported-by: Arnd Bergmann <arnd@arndb.de>',
please do so.

Yet another option would be to use Makefile code to link both
files into one module, and remove the EXPORT_SYMBOL statements:

obj-$(CONFIG_MTD_NAND_JZ4780) += jz4780_nand.o
jz4780_nand-y += ingenic_nand.o
jz4780_nand-$(CONFIG_MTD_NAND_INGENIC_ECC) += ingenic_ecc.o

        Arnd
Miquel Raynal June 17, 2019, 12:16 p.m. | #3
Hello,

Arnd Bergmann <arnd@arndb.de> wrote on Mon, 17 Jun 2019 14:12:48 +0200:

> On Mon, Jun 17, 2019 at 1:24 PM Paul Cercueil <paul@crapouillou.net> wrote:

> 

> > I think there's a better way to fix it, only in Kconfig.

> >

> > * Add a bool symbol MTD_NAND_INGENIC_USE_HW_ECC

> > * Have the three ECC/BCH drivers select this symbol instead of

> >   MTD_NAND_INGENIC_ECC

> > * Add the following to the MTD_NAND_JZ4780 config option:

> >   "select MTD_NAND_INGENIC_ECC if MTD_NAND_INGENIC_USE_HW_ECC"  

> 

> I don't see much difference to my approach here, but if you want

> to submit that version with 'Reported-by: Arnd Bergmann <arnd@arndb.de>',

> please do so.

> 

> Yet another option would be to use Makefile code to link both

> files into one module, and remove the EXPORT_SYMBOL statements:

> 

> obj-$(CONFIG_MTD_NAND_JZ4780) += jz4780_nand.o

> jz4780_nand-y += ingenic_nand.o

> jz4780_nand-$(CONFIG_MTD_NAND_INGENIC_ECC) += ingenic_ecc.o

> 


I personally have a preference for this one.

Thanks,
Miquèl
Miquel Raynal June 27, 2019, 4:40 p.m. | #4
Hi Paul,

Miquel Raynal <miquel.raynal@bootlin.com> wrote on Mon, 17 Jun 2019
14:16:59 +0200:

> Hello,

> 

> Arnd Bergmann <arnd@arndb.de> wrote on Mon, 17 Jun 2019 14:12:48 +0200:

> 

> > On Mon, Jun 17, 2019 at 1:24 PM Paul Cercueil <paul@crapouillou.net> wrote:

> >   

> > > I think there's a better way to fix it, only in Kconfig.

> > >

> > > * Add a bool symbol MTD_NAND_INGENIC_USE_HW_ECC

> > > * Have the three ECC/BCH drivers select this symbol instead of

> > >   MTD_NAND_INGENIC_ECC

> > > * Add the following to the MTD_NAND_JZ4780 config option:

> > >   "select MTD_NAND_INGENIC_ECC if MTD_NAND_INGENIC_USE_HW_ECC"    

> > 

> > I don't see much difference to my approach here, but if you want

> > to submit that version with 'Reported-by: Arnd Bergmann <arnd@arndb.de>',

> > please do so.

> > 

> > Yet another option would be to use Makefile code to link both

> > files into one module, and remove the EXPORT_SYMBOL statements:

> > 

> > obj-$(CONFIG_MTD_NAND_JZ4780) += jz4780_nand.o

> > jz4780_nand-y += ingenic_nand.o

> > jz4780_nand-$(CONFIG_MTD_NAND_INGENIC_ECC) += ingenic_ecc.o

> >   

> 

> I personally have a preference for this one.


Would you mind sending the above change? I forgot about it but I would
like to queue it for the next release. Preferably the last version Arnd
proposed.


Thanks,
Miquèl
Paul Cercueil June 28, 2019, 7:53 p.m. | #5
Le jeu. 27 juin 2019 à 18:40, Miquel Raynal 
<miquel.raynal@bootlin.com> a écrit :
> Hi Paul,

> 

> Miquel Raynal <miquel.raynal@bootlin.com> wrote on Mon, 17 Jun 2019

> 14:16:59 +0200:

> 

>>  Hello,

>> 

>>  Arnd Bergmann <arnd@arndb.de> wrote on Mon, 17 Jun 2019 14:12:48 

>> +0200:

>> 

>>  > On Mon, Jun 17, 2019 at 1:24 PM Paul Cercueil 

>> <paul@crapouillou.net> wrote:

>>  >

>>  > > I think there's a better way to fix it, only in Kconfig.

>>  > >

>>  > > * Add a bool symbol MTD_NAND_INGENIC_USE_HW_ECC

>>  > > * Have the three ECC/BCH drivers select this symbol instead of

>>  > >   MTD_NAND_INGENIC_ECC

>>  > > * Add the following to the MTD_NAND_JZ4780 config option:

>>  > >   "select MTD_NAND_INGENIC_ECC if MTD_NAND_INGENIC_USE_HW_ECC"

>>  >

>>  > I don't see much difference to my approach here, but if you want

>>  > to submit that version with 'Reported-by: Arnd Bergmann 

>> <arnd@arndb.de>',

>>  > please do so.

>>  >

>>  > Yet another option would be to use Makefile code to link both

>>  > files into one module, and remove the EXPORT_SYMBOL statements:

>>  >

>>  > obj-$(CONFIG_MTD_NAND_JZ4780) += jz4780_nand.o

>>  > jz4780_nand-y += ingenic_nand.o

>>  > jz4780_nand-$(CONFIG_MTD_NAND_INGENIC_ECC) += ingenic_ecc.o

>>  >

>> 

>>  I personally have a preference for this one.

> 

> Would you mind sending the above change? I forgot about it but I would

> like to queue it for the next release. Preferably the last version 

> Arnd

> proposed.


It does change the module name from 'ingenic_nand' to 'jz4780_nand', 
though.
That's not really ideal...

> 

> Thanks,

> Miquèl
Arnd Bergmann July 1, 2019, 3:01 p.m. | #6
On Fri, Jun 28, 2019 at 9:53 PM Paul Cercueil <paul@crapouillou.net> wrote:
> Le jeu. 27 juin 2019 à 18:40, Miquel Raynal

> <miquel.raynal@bootlin.com> a écrit :


> > Miquel Raynal <miquel.raynal@bootlin.com> wrote on Mon, 17 Jun 2019

> > 14:16:59 +0200:

> >>  I personally have a preference for this one.

> >

> > Would you mind sending the above change? I forgot about it but I would

> > like to queue it for the next release. Preferably the last version

> > Arnd

> > proposed.

>

> It does change the module name from 'ingenic_nand' to 'jz4780_nand',

> though.

> That's not really ideal...


Any other suggestion for the name? If the module keeps getting called
ingeneric_nand.ko, then the source file has to get renamed to something
else instead.

      Arnd

Patch

diff --git a/drivers/mtd/nand/raw/ingenic/Kconfig b/drivers/mtd/nand/raw/ingenic/Kconfig
index 19a96ce515c1..66b7cffdb0c2 100644
--- a/drivers/mtd/nand/raw/ingenic/Kconfig
+++ b/drivers/mtd/nand/raw/ingenic/Kconfig
@@ -16,7 +16,7 @@  config MTD_NAND_JZ4780
 if MTD_NAND_JZ4780
 
 config MTD_NAND_INGENIC_ECC
-	tristate
+	bool
 
 config MTD_NAND_JZ4740_ECC
 	tristate "Hardware BCH support for JZ4740 SoC"
diff --git a/drivers/mtd/nand/raw/ingenic/Makefile b/drivers/mtd/nand/raw/ingenic/Makefile
index 1ac4f455baea..5a55efc5d9bb 100644
--- a/drivers/mtd/nand/raw/ingenic/Makefile
+++ b/drivers/mtd/nand/raw/ingenic/Makefile
@@ -2,7 +2,10 @@ 
 obj-$(CONFIG_MTD_NAND_JZ4740) += jz4740_nand.o
 obj-$(CONFIG_MTD_NAND_JZ4780) += ingenic_nand.o
 
-obj-$(CONFIG_MTD_NAND_INGENIC_ECC) += ingenic_ecc.o
+ifdef CONFIG_MTD_NAND_INGENIC_ECC
+obj-$(CONFIG_MTD_NAND_JZ4780) += ingenic_ecc.o
+endif
+
 obj-$(CONFIG_MTD_NAND_JZ4740_ECC) += jz4740_ecc.o
 obj-$(CONFIG_MTD_NAND_JZ4725B_BCH) += jz4725b_bch.o
 obj-$(CONFIG_MTD_NAND_JZ4780_BCH) += jz4780_bch.o