| Message ID | 20190221125806.28875-5-miquel.raynal@bootlin.com (mailing list archive) |
|---|---|
| State | RFC |
| Headers | show |
| Series | None | expand |
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. >
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
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
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 --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__ */
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(-)