| Message ID | 20190919193141.7865-4-miquel.raynal@bootlin.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Introduce the generic ECC engine abstraction | expand |
On Thu, 19 Sep 2019 21:31:03 +0200 Miquel Raynal <miquel.raynal@bootlin.com> wrote: > There is currently a confusion between the ECC type/mode/provider > (eg. hardware, software, on-die or none) and the in-bad/out-of-band ^in-band > layout which is only described for hardware engines (OOB first, > syndrome). It's not really about in-band/out-of-band data placement (though it also has an impact on it since free OOB bytes are sometimes protected by ECC or placed next to it), more ECC bytes placement. > > Create a new enumeration to describe this placement. > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > --- > drivers/mtd/nand/raw/nand_base.c | 5 +++++ > include/linux/mtd/rawnand.h | 12 ++++++++++++ > 2 files changed, 17 insertions(+) > > diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c > index e6c483ec191a..74e9289e931c 100644 > --- a/drivers/mtd/nand/raw/nand_base.c > +++ b/drivers/mtd/nand/raw/nand_base.c > @@ -4842,6 +4842,11 @@ static const char * const nand_ecc_modes[] = { > [NAND_ECC_ON_DIE] = "on-die", > }; > > +static const char * const nand_ecc_engine_oob_placement[] = { ^nand_ecc_placement ? > + [NAND_ECC_SYNDROME_OOB_PLACEMENT] = "hw_syndrome", > + [NAND_ECC_OOB_FIRST_PLACEMENT] = "hw_oob_first", Since this is something you introduce, I'd recommend to change the naming here: s/NAND_ECC_SYNDROME_OOB_PLACEMENT/NAND_ECC_PLACEMENT_INTERLEAVED/ s/hw_syndrome/ecc-interleaved/ s/NAND_ECC_OOB_FIRST_PLACEMENT/NAND_ECC_PLACEMENT_FIRST/ s/hw_oob_first/ecc-first/ > +}; > + > static int of_get_nand_ecc_mode(struct device_node *np) > { > const char *pm; > diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h > index ccdc0c314acc..89f964816f2c 100644 > --- a/include/linux/mtd/rawnand.h > +++ b/include/linux/mtd/rawnand.h > @@ -93,6 +93,18 @@ enum nand_ecc_mode { > NAND_ECC_ON_DIE, > }; > > +/** > + * enum nand_ecc_engine_oob_placement - NAND ECC engine OOB placement > + * @NAND_ECC_DEFAULT_OOB_PLACEMENT: Standard layout, or not specified Maybe describe what the standard layout is (ECC placed at the end), and mention that "not specified/default" means the driver can decide to put the ECC/free-OOB bytes where he wants. > + * @NAND_ECC_SYNDROME_OOB_PLACEMENT: Syndrome layout (interlaced) > + * @NAND_ECC_OOB_FIRST_PLACEMENT: Free OOB bytes first > + */ > +enum nand_ecc_engine_oob_placement { > + NAND_ECC_DEFAULT_OOB_PLACEMENT, > + NAND_ECC_SYNDROME_OOB_PLACEMENT, > + NAND_ECC_OOB_FIRST_PLACEMENT, > +}; > + > enum nand_ecc_algo { > NAND_ECC_UNKNOWN, > NAND_ECC_HAMMING,
On Sat, 12 Oct 2019 11:02:09 +0200 Boris Brezillon <boris.brezillon@collabora.com> wrote: > On Thu, 19 Sep 2019 21:31:03 +0200 > Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > There is currently a confusion between the ECC type/mode/provider > > (eg. hardware, software, on-die or none) and the in-bad/out-of-band > > ^in-band > > > layout which is only described for hardware engines (OOB first, > > syndrome). > > It's not really about in-band/out-of-band data placement (though it > also has an impact on it since free OOB bytes are sometimes protected > by ECC or placed next to it), more ECC bytes placement. > > > > > Create a new enumeration to describe this placement. > > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > > --- > > drivers/mtd/nand/raw/nand_base.c | 5 +++++ > > include/linux/mtd/rawnand.h | 12 ++++++++++++ > > 2 files changed, 17 insertions(+) > > > > diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c > > index e6c483ec191a..74e9289e931c 100644 > > --- a/drivers/mtd/nand/raw/nand_base.c > > +++ b/drivers/mtd/nand/raw/nand_base.c > > @@ -4842,6 +4842,11 @@ static const char * const nand_ecc_modes[] = { > > [NAND_ECC_ON_DIE] = "on-die", > > }; > > > > +static const char * const nand_ecc_engine_oob_placement[] = { > > ^nand_ecc_placement ? > > > + [NAND_ECC_SYNDROME_OOB_PLACEMENT] = "hw_syndrome", > > + [NAND_ECC_OOB_FIRST_PLACEMENT] = "hw_oob_first", > > Since this is something you introduce, I'd recommend to change the > naming here: > > s/NAND_ECC_SYNDROME_OOB_PLACEMENT/NAND_ECC_PLACEMENT_INTERLEAVED/ > s/hw_syndrome/ecc-interleaved/ I realize the "ecc-" prefix is unneeded, so maybe just "interleaved" and "first". > s/NAND_ECC_OOB_FIRST_PLACEMENT/NAND_ECC_PLACEMENT_FIRST/ > s/hw_oob_first/ecc-first/ > > > +}; > > + > > static int of_get_nand_ecc_mode(struct device_node *np) > > { > > const char *pm; > > diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h > > index ccdc0c314acc..89f964816f2c 100644 > > --- a/include/linux/mtd/rawnand.h > > +++ b/include/linux/mtd/rawnand.h > > @@ -93,6 +93,18 @@ enum nand_ecc_mode { > > NAND_ECC_ON_DIE, > > }; > > > > +/** > > + * enum nand_ecc_engine_oob_placement - NAND ECC engine OOB placement > > + * @NAND_ECC_DEFAULT_OOB_PLACEMENT: Standard layout, or not specified > > Maybe describe what the standard layout is (ECC placed at the end), and > mention that "not specified/default" means the driver can decide to put > the ECC/free-OOB bytes where he wants. > > > + * @NAND_ECC_SYNDROME_OOB_PLACEMENT: Syndrome layout (interlaced) > > + * @NAND_ECC_OOB_FIRST_PLACEMENT: Free OOB bytes first > > + */ > > +enum nand_ecc_engine_oob_placement { > > + NAND_ECC_DEFAULT_OOB_PLACEMENT, > > + NAND_ECC_SYNDROME_OOB_PLACEMENT, > > + NAND_ECC_OOB_FIRST_PLACEMENT, > > +}; > > + > > enum nand_ecc_algo { > > NAND_ECC_UNKNOWN, > > NAND_ECC_HAMMING, >
diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c index e6c483ec191a..74e9289e931c 100644 --- a/drivers/mtd/nand/raw/nand_base.c +++ b/drivers/mtd/nand/raw/nand_base.c @@ -4842,6 +4842,11 @@ static const char * const nand_ecc_modes[] = { [NAND_ECC_ON_DIE] = "on-die", }; +static const char * const nand_ecc_engine_oob_placement[] = { + [NAND_ECC_SYNDROME_OOB_PLACEMENT] = "hw_syndrome", + [NAND_ECC_OOB_FIRST_PLACEMENT] = "hw_oob_first", +}; + static int of_get_nand_ecc_mode(struct device_node *np) { const char *pm; diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h index ccdc0c314acc..89f964816f2c 100644 --- a/include/linux/mtd/rawnand.h +++ b/include/linux/mtd/rawnand.h @@ -93,6 +93,18 @@ enum nand_ecc_mode { NAND_ECC_ON_DIE, }; +/** + * enum nand_ecc_engine_oob_placement - NAND ECC engine OOB placement + * @NAND_ECC_DEFAULT_OOB_PLACEMENT: Standard layout, or not specified + * @NAND_ECC_SYNDROME_OOB_PLACEMENT: Syndrome layout (interlaced) + * @NAND_ECC_OOB_FIRST_PLACEMENT: Free OOB bytes first + */ +enum nand_ecc_engine_oob_placement { + NAND_ECC_DEFAULT_OOB_PLACEMENT, + NAND_ECC_SYNDROME_OOB_PLACEMENT, + NAND_ECC_OOB_FIRST_PLACEMENT, +}; + enum nand_ecc_algo { NAND_ECC_UNKNOWN, NAND_ECC_HAMMING,
There is currently a confusion between the ECC type/mode/provider (eg. hardware, software, on-die or none) and the in-bad/out-of-band layout which is only described for hardware engines (OOB first, syndrome). Create a new enumeration to describe this placement. Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> --- drivers/mtd/nand/raw/nand_base.c | 5 +++++ include/linux/mtd/rawnand.h | 12 ++++++++++++ 2 files changed, 17 insertions(+)