diff mbox series

[RFC,16/27] mtd: nand: ecc: Let the software BCH ECC engine be a module

Message ID 20190221125806.28875-4-miquel.raynal@bootlin.com (mailing list archive)
State RFC
Headers show
Series None | expand

Commit Message

Miquel Raynal Feb. 21, 2019, 12:57 p.m. UTC
There is no reason to prevent the software BCH ECC engine
implementation to be compiled as a module, so change the 'bool' into a
'tristate'.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/mtd/nand/ecc/Kconfig           | 2 +-
 include/linux/mtd/nand-sw-bch-engine.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Comments

Adam Ford Feb. 21, 2019, 1:48 p.m. UTC | #1
On Thu, Feb 21, 2019 at 6:58 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> There is no reason to prevent the software BCH ECC engine
> implementation to be compiled as a module, so change the 'bool' into a
> 'tristate'.

If you're booting from nand and need the BCH engine, it seems to me
like you'd need it to be compiled into the kernel because you'd have
to mount the filesystem before loading the module unless you have a
initramfs.  If you need the BCH engine to mount the filesystem, it
creates a dependency issue.

Just my two cents.

adam
>
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/mtd/nand/ecc/Kconfig           | 2 +-
>  include/linux/mtd/nand-sw-bch-engine.h | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mtd/nand/ecc/Kconfig b/drivers/mtd/nand/ecc/Kconfig
> index 6ce9007e1d9b..e0106b3a7ec1 100644
> --- a/drivers/mtd/nand/ecc/Kconfig
> +++ b/drivers/mtd/nand/ecc/Kconfig
> @@ -12,7 +12,7 @@ config MTD_NAND_ECC_SW_HAMMING_SMC
>           The original Linux implementation had byte 0 and 1 swapped.
>
>  config MTD_NAND_ECC_SW_BCH
> -       bool "Software BCH ECC engine"
> +       tristate "Software BCH ECC engine"
>         select BCH
>         default n
>         help
> diff --git a/include/linux/mtd/nand-sw-bch-engine.h b/include/linux/mtd/nand-sw-bch-engine.h
> index d8abfc9fc288..5d0b98e34a3a 100644
> --- a/include/linux/mtd/nand-sw-bch-engine.h
> +++ b/include/linux/mtd/nand-sw-bch-engine.h
> @@ -33,7 +33,7 @@ struct ecc_sw_bch_conf {
>         unsigned char *eccmask;
>  };
>
> -#if defined(CONFIG_MTD_NAND_ECC_SW_BCH)
> +#if IS_ENABLED(CONFIG_MTD_NAND_ECC_SW_BCH)
>
>  int ecc_sw_bch_calculate(struct nand_device *nand, const unsigned char *buf,
>                          unsigned char *code);
> --
> 2.19.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Miquel Raynal Feb. 21, 2019, 2:02 p.m. UTC | #2
Hi Adam,

Adam Ford <aford173@gmail.com> wrote on Thu, 21 Feb 2019 07:48:46 -0600:

> On Thu, Feb 21, 2019 at 6:58 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >
> > There is no reason to prevent the software BCH ECC engine
> > implementation to be compiled as a module, so change the 'bool' into a
> > 'tristate'.  
> 
> If you're booting from nand and need the BCH engine, it seems to me
> like you'd need it to be compiled into the kernel because you'd have
> to mount the filesystem before loading the module unless you have a
> initramfs.  If you need the BCH engine to mount the filesystem, it
> creates a dependency issue.

That's true, I could mention this use case in the commit log. But the
action of choosing to compile it as a module is done by the user, so I
guess preventing any user to use it as a module is nonetheless too
restrictive?


Thanks,
Miquèl
Boris Brezillon Feb. 22, 2019, 2:24 p.m. UTC | #3
On Thu, 21 Feb 2019 15:02:24 +0100
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Hi Adam,
> 
> Adam Ford <aford173@gmail.com> wrote on Thu, 21 Feb 2019 07:48:46 -0600:
> 
> > On Thu, Feb 21, 2019 at 6:58 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:  
> > >
> > > There is no reason to prevent the software BCH ECC engine
> > > implementation to be compiled as a module, so change the 'bool' into a
> > > 'tristate'.    
> > 
> > If you're booting from nand and need the BCH engine, it seems to me
> > like you'd need it to be compiled into the kernel because you'd have
> > to mount the filesystem before loading the module unless you have a
> > initramfs.  If you need the BCH engine to mount the filesystem, it
> > creates a dependency issue.  
> 
> That's true, I could mention this use case in the commit log. But the
> action of choosing to compile it as a module is done by the user, so I
> guess preventing any user to use it as a module is nonetheless too
> restrictive?

I agree.
diff mbox series

Patch

diff --git a/drivers/mtd/nand/ecc/Kconfig b/drivers/mtd/nand/ecc/Kconfig
index 6ce9007e1d9b..e0106b3a7ec1 100644
--- a/drivers/mtd/nand/ecc/Kconfig
+++ b/drivers/mtd/nand/ecc/Kconfig
@@ -12,7 +12,7 @@  config MTD_NAND_ECC_SW_HAMMING_SMC
 	  The original Linux implementation had byte 0 and 1 swapped.
 
 config MTD_NAND_ECC_SW_BCH
-	bool "Software BCH ECC engine"
+	tristate "Software BCH ECC engine"
 	select BCH
 	default n
 	help
diff --git a/include/linux/mtd/nand-sw-bch-engine.h b/include/linux/mtd/nand-sw-bch-engine.h
index d8abfc9fc288..5d0b98e34a3a 100644
--- a/include/linux/mtd/nand-sw-bch-engine.h
+++ b/include/linux/mtd/nand-sw-bch-engine.h
@@ -33,7 +33,7 @@  struct ecc_sw_bch_conf {
 	unsigned char *eccmask;
 };
 
-#if defined(CONFIG_MTD_NAND_ECC_SW_BCH)
+#if IS_ENABLED(CONFIG_MTD_NAND_ECC_SW_BCH)
 
 int ecc_sw_bch_calculate(struct nand_device *nand, const unsigned char *buf,
 			 unsigned char *code);