diff mbox series

[v2] mtd: nand: ecc: Rework Kconfig dependencies

Message ID 20220131145032.17614-1-miquel.raynal@bootlin.com
State Accepted
Headers show
Series [v2] mtd: nand: ecc: Rework Kconfig dependencies | expand

Commit Message

Miquel Raynal Jan. 31, 2022, 2:50 p.m. UTC 
Unlike "depends on", "select" does not enforce any type (y or m), which
can lead to the following situation:
* SPI_MXIC=y expects the NAND symbols to be built statically
* SPI_MXIC depends on MTD_NAND_ECC
* MTD_NAND_ECC selects MTD_NAND_CORE
In this case MTD_NAND_CORE=m is "valid" but will trigger 'undefined
reference' link errors.

The cleanest way to handle the situation is to use a "depends on"
between MTD_NAND_ECC and MTD_NAND_CORE to avoid such situations.

While at modifying the MTD_NAND_ECC symbol, fix the spacing.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Pratyush Yadav <p.yadav@ti.com>
---

Changes in v2:
* Pratyush Ack.
* Stopped quoting a missing error log. Mentioned the 'undefined
  reference' link error instead.

 drivers/mtd/nand/Kconfig | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Comments

Miquel Raynal Jan. 31, 2022, 4:22 p.m. UTC | #1 
On Mon, 2022-01-31 at 14:50:32 UTC, Miquel Raynal wrote:
> Unlike "depends on", "select" does not enforce any type (y or m), which
> can lead to the following situation:
> * SPI_MXIC=y expects the NAND symbols to be built statically
> * SPI_MXIC depends on MTD_NAND_ECC
> * MTD_NAND_ECC selects MTD_NAND_CORE
> In this case MTD_NAND_CORE=m is "valid" but will trigger 'undefined
> reference' link errors.
> 
> The cleanest way to handle the situation is to use a "depends on"
> between MTD_NAND_ECC and MTD_NAND_CORE to avoid such situations.
> 
> While at modifying the MTD_NAND_ECC symbol, fix the spacing.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> Reviewed-by: Pratyush Yadav <p.yadav@ti.com>

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git spi-mem-ecc.

Miquel
Miquel Raynal Feb. 2, 2022, 2:47 p.m. UTC | #2 
Hello,

miquel.raynal@bootlin.com wrote on Mon, 31 Jan 2022 17:22:56 +0100:

> On Mon, 2022-01-31 at 14:50:32 UTC, Miquel Raynal wrote:
> > Unlike "depends on", "select" does not enforce any type (y or m), which
> > can lead to the following situation:
> > * SPI_MXIC=y expects the NAND symbols to be built statically
> > * SPI_MXIC depends on MTD_NAND_ECC
> > * MTD_NAND_ECC selects MTD_NAND_CORE
> > In this case MTD_NAND_CORE=m is "valid" but will trigger 'undefined
> > reference' link errors.
> > 
> > The cleanest way to handle the situation is to use a "depends on"
> > between MTD_NAND_ECC and MTD_NAND_CORE to avoid such situations.
> > 
> > While at modifying the MTD_NAND_ECC symbol, fix the spacing.
> > 
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > Reviewed-by: Pratyush Yadav <p.yadav@ti.com>  
> 
> Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git spi-mem-ecc.

Unfortunately this patch break other users (like sm_ftl) which are
specific cases which I forgot to take into account when writing this
change. I've found another way to handle the situation, see the patch
sent in parallel, v11 of the last change of the spi-mem-ecc series.

Thanks,
Miquèl
diff mbox series

Patch

diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index 8431292ff49d..1b5f5bac3e6e 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -12,13 +12,13 @@  source "drivers/mtd/nand/spi/Kconfig"
 menu "ECC engine support"
 
 config MTD_NAND_ECC
-       bool
-       select MTD_NAND_CORE
+	bool
+	depends on MTD_NAND_CORE
 
 config MTD_NAND_ECC_SW_HAMMING
 	bool "Software Hamming ECC engine"
+	depends on MTD_NAND_ECC
 	default y if MTD_RAW_NAND
-	select MTD_NAND_ECC
 	help
 	  This enables support for software Hamming error
 	  correction. This correction can correct up to 1 bit error
@@ -37,8 +37,8 @@  config MTD_NAND_ECC_SW_HAMMING_SMC
 
 config MTD_NAND_ECC_SW_BCH
 	bool "Software BCH ECC engine"
+	depends on MTD_NAND_ECC
 	select BCH
-	select MTD_NAND_ECC
 	default n
 	help
 	  This enables support for software BCH error correction. Binary BCH
@@ -48,7 +48,7 @@  config MTD_NAND_ECC_SW_BCH
 
 config MTD_NAND_ECC_MXIC
 	bool "Macronix external hardware ECC engine"
-	select MTD_NAND_ECC
+	depends on MTD_NAND_ECC
 	help
 	  This enables support for the hardware ECC engine from Macronix.