diff mbox series

[RFC,17/27] mtd: nand: ecc: Let the software Hamming ECC engine be unselected

Message ID 20190221125806.28875-5-miquel.raynal@bootlin.com
State RFC
Headers show
Series None | expand

Commit Message

Miquel Raynal Feb. 21, 2019, 12:57 p.m. UTC
There is no reason to always embed the software Hamming ECC engine
implementation. By default it is, but we can let the user decide.

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

Comments

Boris Brezillon Feb. 21, 2019, 1:20 p.m. UTC | #1
On Thu, 21 Feb 2019 13:57:56 +0100
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> There is no reason to always embed the software Hamming ECC engine
> implementation. By default it is, but we can let the user decide.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/mtd/nand/ecc/Kconfig               | 10 +++++-
>  drivers/mtd/nand/raw/Kconfig               |  2 +-
>  include/linux/mtd/nand-sw-hamming-engine.h | 38 ++++++++++++++++++++++
>  3 files changed, 48 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/nand/ecc/Kconfig b/drivers/mtd/nand/ecc/Kconfig
> index e0106b3a7ec1..ff20e621ffef 100644
> --- a/drivers/mtd/nand/ecc/Kconfig
> +++ b/drivers/mtd/nand/ecc/Kconfig
> @@ -1,7 +1,15 @@
>  menu "ECC engine support"
>  
>  config MTD_NAND_ECC_SW_HAMMING
> -	tristate
> +	tristate "Software Hamming ECC engine"
> +	default y

Same as for the NAND_CORE stuff, let users this option when they need
it instead of having a "default y". Haven't made my mind yet on whether
this option should be visible to users or not. I guess it could be with
the new infrastructure, but it's probably too early in the patch series
to change that.

> +	help
> +	  This enables support for software Hamming error
> +	  correction. This correction can correct up to 1 bit error
> +	  per chunk and detect up to 2 bit errors. While it used to be
> +	  widely used with old parts, newer NAND chips usually require
> +	  more strength correction and in this case BCH or RS will be
> +	  preferred.
>
Miquel Raynal Feb. 21, 2019, 1:35 p.m. UTC | #2
Hi Boris,

Boris Brezillon <bbrezillon@kernel.org> wrote on Thu, 21 Feb 2019
14:20:02 +0100:

> On Thu, 21 Feb 2019 13:57:56 +0100
> Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> 
> > There is no reason to always embed the software Hamming ECC engine
> > implementation. By default it is, but we can let the user decide.
> > 
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> >  drivers/mtd/nand/ecc/Kconfig               | 10 +++++-
> >  drivers/mtd/nand/raw/Kconfig               |  2 +-
> >  include/linux/mtd/nand-sw-hamming-engine.h | 38 ++++++++++++++++++++++
> >  3 files changed, 48 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/mtd/nand/ecc/Kconfig b/drivers/mtd/nand/ecc/Kconfig
> > index e0106b3a7ec1..ff20e621ffef 100644
> > --- a/drivers/mtd/nand/ecc/Kconfig
> > +++ b/drivers/mtd/nand/ecc/Kconfig
> > @@ -1,7 +1,15 @@
> >  menu "ECC engine support"
> >  
> >  config MTD_NAND_ECC_SW_HAMMING
> > -	tristate
> > +	tristate "Software Hamming ECC engine"
> > +	default y  
> 
> Same as for the NAND_CORE stuff, let users this option when they need
> it instead of having a "default y". Haven't made my mind yet on whether
> this option should be visible to users or not. I guess it could be with
> the new infrastructure, but it's probably too early in the patch series
> to change that.

This one is different.

Before the series: the software Hamming ECC algorithm is part of the
'NAND package'. There is no way to ignore it, it *will* be part of your
binary (or module).

After the series: I just give the user the possibility to deselect
this option. But having 'default y' is mandatory here to avoid breaking
current defconfigs.

> 
> > +	help
> > +	  This enables support for software Hamming error
> > +	  correction. This correction can correct up to 1 bit error
> > +	  per chunk and detect up to 2 bit errors. While it used to be
> > +	  widely used with old parts, newer NAND chips usually require
> > +	  more strength correction and in this case BCH or RS will be
> > +	  preferred.
> >    


Thanks,
Miquèl
Boris Brezillon Feb. 21, 2019, 1:41 p.m. UTC | #3
On Thu, 21 Feb 2019 14:35:39 +0100
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Hi Boris,
> 
> Boris Brezillon <bbrezillon@kernel.org> wrote on Thu, 21 Feb 2019
> 14:20:02 +0100:
> 
> > On Thu, 21 Feb 2019 13:57:56 +0100
> > Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >   
> > > There is no reason to always embed the software Hamming ECC engine
> > > implementation. By default it is, but we can let the user decide.
> > > 
> > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > > ---
> > >  drivers/mtd/nand/ecc/Kconfig               | 10 +++++-
> > >  drivers/mtd/nand/raw/Kconfig               |  2 +-
> > >  include/linux/mtd/nand-sw-hamming-engine.h | 38 ++++++++++++++++++++++
> > >  3 files changed, 48 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/mtd/nand/ecc/Kconfig b/drivers/mtd/nand/ecc/Kconfig
> > > index e0106b3a7ec1..ff20e621ffef 100644
> > > --- a/drivers/mtd/nand/ecc/Kconfig
> > > +++ b/drivers/mtd/nand/ecc/Kconfig
> > > @@ -1,7 +1,15 @@
> > >  menu "ECC engine support"
> > >  
> > >  config MTD_NAND_ECC_SW_HAMMING
> > > -	tristate
> > > +	tristate "Software Hamming ECC engine"
> > > +	default y    
> > 
> > Same as for the NAND_CORE stuff, let users this option when they need
> > it instead of having a "default y". Haven't made my mind yet on whether
> > this option should be visible to users or not. I guess it could be with
> > the new infrastructure, but it's probably too early in the patch series
> > to change that.  
> 
> This one is different.
> 
> Before the series: the software Hamming ECC algorithm is part of the
> 'NAND package'. There is no way to ignore it, it *will* be part of your
> binary (or module).
> 
> After the series: I just give the user the possibility to deselect
> this option. But having 'default y' is mandatory here to avoid breaking
> current defconfigs.

Okay, then maybe

	default y if MTD_RAW_NAND

And I think you should select the option in MTD_NAND_NDFC instead of
adding a depends on. 

> 
> >   
> > > +	help
> > > +	  This enables support for software Hamming error
> > > +	  correction. This correction can correct up to 1 bit error
> > > +	  per chunk and detect up to 2 bit errors. While it used to be
> > > +	  widely used with old parts, newer NAND chips usually require
> > > +	  more strength correction and in this case BCH or RS will be
> > > +	  preferred.
> > >      
> 
> 
> Thanks,
> Miquèl
Miquel Raynal Feb. 21, 2019, 1:46 p.m. UTC | #4
Hi Boris,

Boris Brezillon <bbrezillon@kernel.org> wrote on Thu, 21 Feb 2019
14:41:48 +0100:

> On Thu, 21 Feb 2019 14:35:39 +0100
> Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> 
> > Hi Boris,
> > 
> > Boris Brezillon <bbrezillon@kernel.org> wrote on Thu, 21 Feb 2019
> > 14:20:02 +0100:
> >   
> > > On Thu, 21 Feb 2019 13:57:56 +0100
> > > Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > >     
> > > > There is no reason to always embed the software Hamming ECC engine
> > > > implementation. By default it is, but we can let the user decide.
> > > > 
> > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > > > ---
> > > >  drivers/mtd/nand/ecc/Kconfig               | 10 +++++-
> > > >  drivers/mtd/nand/raw/Kconfig               |  2 +-
> > > >  include/linux/mtd/nand-sw-hamming-engine.h | 38 ++++++++++++++++++++++
> > > >  3 files changed, 48 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/drivers/mtd/nand/ecc/Kconfig b/drivers/mtd/nand/ecc/Kconfig
> > > > index e0106b3a7ec1..ff20e621ffef 100644
> > > > --- a/drivers/mtd/nand/ecc/Kconfig
> > > > +++ b/drivers/mtd/nand/ecc/Kconfig
> > > > @@ -1,7 +1,15 @@
> > > >  menu "ECC engine support"
> > > >  
> > > >  config MTD_NAND_ECC_SW_HAMMING
> > > > -	tristate
> > > > +	tristate "Software Hamming ECC engine"
> > > > +	default y      
> > > 
> > > Same as for the NAND_CORE stuff, let users this option when they need
> > > it instead of having a "default y". Haven't made my mind yet on whether
> > > this option should be visible to users or not. I guess it could be with
> > > the new infrastructure, but it's probably too early in the patch series
> > > to change that.    
> > 
> > This one is different.
> > 
> > Before the series: the software Hamming ECC algorithm is part of the
> > 'NAND package'. There is no way to ignore it, it *will* be part of your
> > binary (or module).
> > 
> > After the series: I just give the user the possibility to deselect
> > this option. But having 'default y' is mandatory here to avoid breaking
> > current defconfigs.  
> 
> Okay, then maybe
> 
> 	default y if MTD_RAW_NAND

Right, this is more accurate.

> 
> And I think you should select the option in MTD_NAND_NDFC instead of
> adding a depends on. 

Ack.

> 
> >   
> > >     
> > > > +	help
> > > > +	  This enables support for software Hamming error
> > > > +	  correction. This correction can correct up to 1 bit error
> > > > +	  per chunk and detect up to 2 bit errors. While it used to be
> > > > +	  widely used with old parts, newer NAND chips usually require
> > > > +	  more strength correction and in this case BCH or RS will be
> > > > +	  preferred.
> > > >        
> > 
> > 
> > Thanks,
> > Miquèl  
> 




Thanks,
Miquèl
diff mbox series

Patch

diff --git a/drivers/mtd/nand/ecc/Kconfig b/drivers/mtd/nand/ecc/Kconfig
index e0106b3a7ec1..ff20e621ffef 100644
--- a/drivers/mtd/nand/ecc/Kconfig
+++ b/drivers/mtd/nand/ecc/Kconfig
@@ -1,7 +1,15 @@ 
 menu "ECC engine support"
 
 config MTD_NAND_ECC_SW_HAMMING
-	tristate
+	tristate "Software Hamming ECC engine"
+	default y
+	help
+	  This enables support for software Hamming error
+	  correction. This correction can correct up to 1 bit error
+	  per chunk and detect up to 2 bit errors. While it used to be
+	  widely used with old parts, newer NAND chips usually require
+	  more strength correction and in this case BCH or RS will be
+	  preferred.
 
 config MTD_NAND_ECC_SW_HAMMING_SMC
 	bool "NAND ECC Smart Media byte order"
diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig
index 995fa78bdedb..76a2a8493b1f 100644
--- a/drivers/mtd/nand/raw/Kconfig
+++ b/drivers/mtd/nand/raw/Kconfig
@@ -1,6 +1,5 @@ 
 menuconfig MTD_RAW_NAND
 	tristate "Raw/Parallel NAND Device Support"
-	select MTD_NAND_ECC_SW_HAMMING
 	help
 	  This enables support for accessing all type of raw/parallel
 	  NAND flash devices. For further information see
@@ -69,6 +68,7 @@  config MTD_NAND_AU1550
 config MTD_NAND_NDFC
 	tristate "IBM/MCC 4xx NAND controller"
 	depends on 4xx
+	depends on MTD_NAND_ECC_SW_HAMMING
 	select MTD_NAND_ECC_SW_HAMMING_SMC
 	help
 	  NDFC Nand Flash Controllers are integrated in IBM/AMCC's 4xx SoCs
diff --git a/include/linux/mtd/nand-sw-hamming-engine.h b/include/linux/mtd/nand-sw-hamming-engine.h
index c50da3a255b7..378ba46f7e1d 100644
--- a/include/linux/mtd/nand-sw-hamming-engine.h
+++ b/include/linux/mtd/nand-sw-hamming-engine.h
@@ -30,6 +30,8 @@  struct ecc_sw_hamming_conf {
 	unsigned int sm_order;
 };
 
+#if IS_ENABLED(CONFIG_MTD_NAND_ECC_SW_HAMMING)
+
 int __ecc_sw_hamming_calculate(const unsigned char *buf,
 			       unsigned int step_size,
 			       unsigned char *code, bool sm_order);
@@ -42,4 +44,40 @@  int __ecc_sw_hamming_correct(unsigned char *buf, unsigned char *read_ecc,
 int ecc_sw_hamming_correct(struct nand_device *nand, unsigned char *buf,
 			   unsigned char *read_ecc, unsigned char *calc_ecc);
 
+#else /* !CONFIG_MTD_NAND_ECC_SW_HAMMING */
+
+static inline int __ecc_sw_hamming_calculate(const unsigned char *buf,
+					     unsigned int step_size,
+					     unsigned char *code,
+					     bool sm_order)
+{
+	return -ENOTSUPP;
+}
+
+static inline int ecc_sw_hamming_calculate(struct nand_device *nand,
+					   const unsigned char *buf,
+					   unsigned char *code)
+{
+	return -ENOTSUPP;
+}
+
+static inline int __ecc_sw_hamming_correct(unsigned char *buf,
+					   unsigned char *read_ecc,
+					   unsigned char *calc_ecc,
+					   unsigned int step_size,
+					   bool sm_order)
+{
+	return -ENOTSUPP;
+}
+
+static inline int ecc_sw_hamming_correct(struct nand_device *nand,
+					 unsigned char *buf,
+					 unsigned char *read_ecc,
+					 unsigned char *calc_ecc)
+{
+	return -ENOTSUPP;
+}
+
+#endif /* CONFIG_MTD_NAND_ECC_SW_HAMMING */
+
 #endif /* __MTD_NAND_ECC_SW_HAMMING_H__ */