| Message ID | 20200528113113.9166-16-miquel.raynal@bootlin.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Introduce the generic ECC engine abstraction | expand |
On Thu, 28 May 2020 13:31:10 +0200 Miquel Raynal <miquel.raynal@bootlin.com> wrote: > Create a generic ECC engine object. > > Later the ecc.c file will receive more generic code coming from > the raw NAND specific part. This is a base to instantiate ECC engine > objects. > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > --- > drivers/mtd/nand/Kconfig | 7 ++ > drivers/mtd/nand/Makefile | 2 + > drivers/mtd/nand/ecc.c | 138 ++++++++++++++++++++++++++++++++++++++ > include/linux/mtd/nand.h | 67 ++++++++++++++++++ > 4 files changed, 214 insertions(+) > create mode 100644 drivers/mtd/nand/ecc.c > > diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig > index c1a45b071165..a4478ffa279d 100644 > --- a/drivers/mtd/nand/Kconfig > +++ b/drivers/mtd/nand/Kconfig > @@ -9,4 +9,11 @@ source "drivers/mtd/nand/onenand/Kconfig" > source "drivers/mtd/nand/raw/Kconfig" > source "drivers/mtd/nand/spi/Kconfig" > > +menu "ECC engine support" > + > +config MTD_NAND_ECC > + bool > + > +endmenu > + > endmenu > diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile > index 7ecd80c0a66e..981372953b56 100644 > --- a/drivers/mtd/nand/Makefile > +++ b/drivers/mtd/nand/Makefile > @@ -6,3 +6,5 @@ obj-$(CONFIG_MTD_NAND_CORE) += nandcore.o > obj-y += onenand/ > obj-y += raw/ > obj-y += spi/ > + > +nandcore-$(CONFIG_MTD_NAND_ECC) += ecc.o > diff --git a/drivers/mtd/nand/ecc.c b/drivers/mtd/nand/ecc.c > new file mode 100644 > index 000000000000..e4f2b6fcbb12 > --- /dev/null > +++ b/drivers/mtd/nand/ecc.c > @@ -0,0 +1,138 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Generic Error-Correcting Code (ECC) engine > + * > + * Copyright (C) 2019 Macronix > + * Author: > + * Miquèl RAYNAL <miquel.raynal@bootlin.com> > + * > + * > + * This file describes the abstraction of any NAND ECC engine. It has been > + * designed to fit most cases, including parallel NANDs and SPI-NANDs. > + * > + * There are three main situations where instantiating this ECC engine makes > + * sense: > + * - "external": The ECC engine is outside the NAND pipeline, typically this I'm not sure why you put quotes around those names. > + * is a software ECC engine. One can also imagine a generic ^ or an hardware engine that's outside the NAND controller pipeline. You can the drop the "One can also imagine ..." since it's more than a theoretical use case, we already have a few engines that fall in this category. > + * hardware ECC engine which would be an IP itself. Interacting > + * with a SPI-NAND device without on-die ECC could be achieved ^can > + * thanks to the use of such external engine. But I think I would simply drop this last sentence. > + * - "pipelined": The ECC engine is inside the NAND pipeline, ie. on the > + * controller's side. This is the case of most of the raw NAND > + * controllers. These controllers usually embed an hardware ECC > + * engine which is managed thanks to the same register set as > + * the controller's. Again, I would drop the last sentence here. I think saying the ECC bytes are generated/data corrected on the fly when a page is written/read would be more useful. > + * - "ondie": The ECC engine is inside the NAND pipeline, on the chip's side. > + * Some NAND chips can correct themselves the data. The on-die > + * correction can be enabled, disabled and the status of the > + * correction after a read may be retrieved with a NAND command > + * (may be vendor specific). "The on-die correction can be enabled, disabled" -> this is true for any kind of ECC engine :P. > + * > + * Besides the initial setup and final cleanups, the interfaces are rather > + * simple: > + * - "prepare": Prepare an I/O request, check the ECC engine is enabled or ^if/whether > + * disabled as requested before the I/O. In case of software How about "Enable/disable the ECC engine based on the I/O request type." > + * correction, this step may involve to derive the ECC bytes and > + * place them in the OOB area before a write. This is also true for external hardware ECC engines. > + * - "finish": Finish an I/O request, check the status of the operation ie. > + * the data validity in case of a read (report to the upper layer > + * any bitflip/errors). It's all about correcting/reporting errors, right. Let's try to put that into simple words: "Correct the data in case of a read request and report the number of corrected bits/uncorrectable errors. Most likely empty for write operations, unless you have hardware specific stuff to do, like shutting down the engine to save some power" > + * > + * Both prepare/finish callbacks are supposed to enclose I/O request and will "The I/O request should be enclosed in a prepare()/finish() pair of calls" or "The prepare/finish call should surround the I/O request". > + * behave differently depending on the desired correction: ^requested I/O type > + * - "raw": Correction disabled > + * - "ecc": Correction enabled > + * > + * The request direction is impacting the logic as well: > + * - "read": Load data from the NAND chip > + * - "write": Store data in the NAND chip > + * > + * Mixing all this combinations together gives the following behavior. Mention that those are just examples, and drivers are free to add custom steps in their prepare/finish hooks. > + * > + * ["external" ECC engine] > + * - external + prepare + raw + read: do nothing > + * - external + finish + raw + read: do nothing > + * - external + prepare + raw + write: do nothing > + * - external + finish + raw + write: do nothing > + * - external + prepare + ecc + read: do nothing > + * - external + finish + ecc + read: calculate expected ECC bytes, extract > + * ECC bytes from OOB buffer, correct > + * and report any bitflip/error > + * - external + prepare + ecc + write: calculate ECC bytes and store them at > + * the right place in the OOB buffer based > + * on the OOB layout > + * - external + finish + ecc + write: do nothing > + * > + * ["pipelined" ECC engine] > + * - pipelined + prepare + raw + read: disable the controller's ECC engine if > + * activated > + * - pipelined + finish + raw + read: do nothing > + * - pipelined + prepare + raw + write: disable the controller's ECC engine if > + * activated > + * - pipelined + finish + raw + write: do nothing > + * - pipelined + prepare + ecc + read: enable the controller's ECC engine if > + * deactivated > + * - pipelined + finish + ecc + read: check the status, report any > + * error/bitflip > + * - pipelined + prepare + ecc + write: enable the controller's ECC engine if > + * deactivated > + * - pipelined + finish + ecc + write: do nothing > + * > + * ["ondie" ECC engine] > + * - ondie + prepare + raw + read: send commands to disable the on-chip ECC > + * engine if activated > + * - ondie + finish + raw + read: do nothing > + * - ondie + prepare + raw + write: send commands to disable the on-chip ECC > + * engine if activated > + * - ondie + finish + raw + write: do nothing > + * - ondie + prepare + ecc + read: send commands to enable the on-chip ECC > + * engine if deactivated > + * - ondie + finish + ecc + read: send commands to check the status, report > + * any error/bitflip > + * - ondie + prepare + ecc + write: send commands to enable the on-chip ECC > + * engine if deactivated > + * - ondie + finish + ecc + write: do nothing > + */ > + > +#include <linux/module.h> > +#include <linux/mtd/nand.h> > + Shouldn't we have kernel-docs for those functions? > +int nand_ecc_init_ctx(struct nand_device *nand) > +{ > + if (!nand->ecc.engine->ops->init_ctx) > + return 0; > + > + return nand->ecc.engine->ops->init_ctx(nand); > +} > +EXPORT_SYMBOL(nand_ecc_init_ctx); > + > +void nand_ecc_cleanup_ctx(struct nand_device *nand) > +{ > + if (nand->ecc.engine->ops->cleanup_ctx) > + nand->ecc.engine->ops->cleanup_ctx(nand); > +} > +EXPORT_SYMBOL(nand_ecc_cleanup_ctx); > + > +int nand_ecc_prepare_io_req(struct nand_device *nand, > + struct nand_page_io_req *req) > +{ > + if (!nand->ecc.engine->ops->prepare_io_req) > + return 0; > + > + return nand->ecc.engine->ops->prepare_io_req(nand, req); > +} > +EXPORT_SYMBOL(nand_ecc_prepare_io_req); > + > +int nand_ecc_finish_io_req(struct nand_device *nand, > + struct nand_page_io_req *req) > +{ > + if (!nand->ecc.engine->ops->finish_io_req) > + return 0; > + > + return nand->ecc.engine->ops->finish_io_req(nand, req); > +} > +EXPORT_SYMBOL(nand_ecc_finish_io_req); > + > +MODULE_LICENSE("GPL"); > +MODULE_AUTHOR("Miquel Raynal <miquel.raynal@bootlin.com>"); > +MODULE_DESCRIPTION("Generic ECC engine"); > diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h > index 2e9af24936cd..0be260fd2f86 100644 > --- a/include/linux/mtd/nand.h > +++ b/include/linux/mtd/nand.h > @@ -221,6 +221,73 @@ struct nand_ops { > bool (*isbad)(struct nand_device *nand, const struct nand_pos *pos); > }; > > +/** > + * struct nand_ecc_context - Context for the ECC engine > + * @conf: basic ECC engine parameters > + * @total: Total number of bytes used for storing ECC codes, this is used by Sometimes you start your description with an uppercase, sometimes not. > + * generic OOB layouts > + * @priv: ECC engine driver private data > + */ > +struct nand_ecc_context { > + struct nand_ecc_props conf; > + unsigned int total; > + void *priv; > +}; > + > +/** > + * struct nand_ecc_engine_ops - Generic ECC engine operations ^s/Generic// > + * @init_ctx: given a desired user configuration for the pointed NAND device, > + * requests the ECC engine driver to setup a configuration with > + * values it supports. > + * @cleanup_ctx: clean the context initialized by @init_ctx. > + * @prepare_io_req: is called before reading/writing a page to prepare the I/O > + * request to be performed with ECC correction. > + * @finish_io_req: is called after reading/writing a page to terminate the I/O > + * request and ensure proper ECC correction. > + */ > +struct nand_ecc_engine_ops { > + int (*init_ctx)(struct nand_device *nand); > + void (*cleanup_ctx)(struct nand_device *nand); > + int (*prepare_io_req)(struct nand_device *nand, > + struct nand_page_io_req *req); > + int (*finish_io_req)(struct nand_device *nand, > + struct nand_page_io_req *req); > +}; > + > +/** > + * struct nand_ecc_engine - Generic ECC engine abstraction for NAND devices ^s/Generic// > + * @ops: ECC engine operations > + */ > +struct nand_ecc_engine { > + struct nand_ecc_engine_ops *ops; > +}; > + > +int nand_ecc_init_ctx(struct nand_device *nand); > +void nand_ecc_cleanup_ctx(struct nand_device *nand); > +int nand_ecc_prepare_io_req(struct nand_device *nand, > + struct nand_page_io_req *req); > +int nand_ecc_finish_io_req(struct nand_device *nand, > + struct nand_page_io_req *req); > + > +/** > + * struct nand_ecc - High-level ECC object I think you can drop the "High-level" and just say "Information relative to the ECC" > + * @defaults: Default values, depend on the underlying subsystem > + * @requirements: ECC requirements from the NAND chip perspective > + * @user_conf: User desires in terms of ECC parameters > + * @ctx: ECC context for the ECC engine, derived from the device @requirements > + * the @user_conf and the @defaults > + * @ondie_engine: On-die ECC engine reference, if any > + * @engine: ECC engine actually bound > + */ > +struct nand_ecc { > + struct nand_ecc_props defaults; > + struct nand_ecc_props requirements; > + struct nand_ecc_props user_conf; > + struct nand_ecc_context ctx; > + struct nand_ecc_engine *ondie_engine; > + struct nand_ecc_engine *engine; > +}; > + > /** > * struct nand_device - NAND device > * @mtd: MTD instance attached to the NAND device
Hi Boris, Boris Brezillon <boris.brezillon@collabora.com> wrote on Thu, 28 May 2020 20:52:51 +0200: > On Thu, 28 May 2020 13:31:10 +0200 > Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > Create a generic ECC engine object. > > > > Later the ecc.c file will receive more generic code coming from > > the raw NAND specific part. This is a base to instantiate ECC engine > > objects. > > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > > --- > > drivers/mtd/nand/Kconfig | 7 ++ > > drivers/mtd/nand/Makefile | 2 + > > drivers/mtd/nand/ecc.c | 138 ++++++++++++++++++++++++++++++++++++++ > > include/linux/mtd/nand.h | 67 ++++++++++++++++++ > > 4 files changed, 214 insertions(+) > > create mode 100644 drivers/mtd/nand/ecc.c > > > > diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig > > index c1a45b071165..a4478ffa279d 100644 > > --- a/drivers/mtd/nand/Kconfig > > +++ b/drivers/mtd/nand/Kconfig > > @@ -9,4 +9,11 @@ source "drivers/mtd/nand/onenand/Kconfig" > > source "drivers/mtd/nand/raw/Kconfig" > > source "drivers/mtd/nand/spi/Kconfig" > > > > +menu "ECC engine support" > > + > > +config MTD_NAND_ECC > > + bool > > + > > +endmenu > > + > > endmenu > > diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile > > index 7ecd80c0a66e..981372953b56 100644 > > --- a/drivers/mtd/nand/Makefile > > +++ b/drivers/mtd/nand/Makefile > > @@ -6,3 +6,5 @@ obj-$(CONFIG_MTD_NAND_CORE) += nandcore.o > > obj-y += onenand/ > > obj-y += raw/ > > obj-y += spi/ > > + > > +nandcore-$(CONFIG_MTD_NAND_ECC) += ecc.o > > diff --git a/drivers/mtd/nand/ecc.c b/drivers/mtd/nand/ecc.c > > new file mode 100644 > > index 000000000000..e4f2b6fcbb12 > > --- /dev/null > > +++ b/drivers/mtd/nand/ecc.c > > @@ -0,0 +1,138 @@ > > +// SPDX-License-Identifier: GPL-2.0+ > > +/* > > + * Generic Error-Correcting Code (ECC) engine > > + * > > + * Copyright (C) 2019 Macronix > > + * Author: > > + * Miquèl RAYNAL <miquel.raynal@bootlin.com> > > + * > > + * > > + * This file describes the abstraction of any NAND ECC engine. It has been > > + * designed to fit most cases, including parallel NANDs and SPI-NANDs. > > + * > > + * There are three main situations where instantiating this ECC engine makes > > + * sense: > > + * - "external": The ECC engine is outside the NAND pipeline, typically this > > I'm not sure why you put quotes around those names. > > > + * is a software ECC engine. One can also imagine a generic > > ^ or an hardware > engine that's outside the NAND controller pipeline. > > You can the drop the "One can also imagine ..." since it's more than a > theoretical use case, we already have a few engines that fall in this > category. > > > + * hardware ECC engine which would be an IP itself. Interacting > > + * with a SPI-NAND device without on-die ECC could be achieved > > ^can > > > + * thanks to the use of such external engine. > > But I think I would simply drop this last sentence. > > > + * - "pipelined": The ECC engine is inside the NAND pipeline, ie. on the > > + * controller's side. This is the case of most of the raw NAND > > + * controllers. These controllers usually embed an hardware ECC > > + * engine which is managed thanks to the same register set as > > + * the controller's. > > Again, I would drop the last sentence here. I think saying the ECC > bytes are generated/data corrected on the fly when a page is > written/read would be more useful. > > > + * - "ondie": The ECC engine is inside the NAND pipeline, on the chip's side. > > + * Some NAND chips can correct themselves the data. The on-die > > + * correction can be enabled, disabled and the status of the > > + * correction after a read may be retrieved with a NAND command > > + * (may be vendor specific). > > "The on-die correction can be enabled, disabled" -> this is true for > any kind of ECC engine :P. > > > + * > > + * Besides the initial setup and final cleanups, the interfaces are rather > > + * simple: > > + * - "prepare": Prepare an I/O request, check the ECC engine is enabled or > > ^if/whether > > > + * disabled as requested before the I/O. In case of software > > How about "Enable/disable the ECC engine based on the I/O request type." > > > + * correction, this step may involve to derive the ECC bytes and > > + * place them in the OOB area before a write. > > This is also true for external hardware ECC engines. > > > + * - "finish": Finish an I/O request, check the status of the operation ie. > > + * the data validity in case of a read (report to the upper layer > > + * any bitflip/errors). > > It's all about correcting/reporting errors, right. Let's try to put > that into simple words: "Correct the data in case of a read request and > report the number of corrected bits/uncorrectable errors. Most likely > empty for write operations, unless you have hardware specific stuff to > do, like shutting down the engine to save some power" > > > + * > > + * Both prepare/finish callbacks are supposed to enclose I/O request and will > > "The I/O request should be enclosed in a prepare()/finish() pair of > calls" or "The prepare/finish call should surround the I/O request". > > > + * behave differently depending on the desired correction: > > ^requested I/O type > > > + * - "raw": Correction disabled > > + * - "ecc": Correction enabled > > + * > > + * The request direction is impacting the logic as well: > > + * - "read": Load data from the NAND chip > > + * - "write": Store data in the NAND chip > > + * > > + * Mixing all this combinations together gives the following behavior. > > Mention that those are just examples, and drivers are free to add > custom steps in their prepare/finish hooks. > > > + * > > + * ["external" ECC engine] > > + * - external + prepare + raw + read: do nothing > > + * - external + finish + raw + read: do nothing > > + * - external + prepare + raw + write: do nothing > > + * - external + finish + raw + write: do nothing > > + * - external + prepare + ecc + read: do nothing > > + * - external + finish + ecc + read: calculate expected ECC bytes, extract > > + * ECC bytes from OOB buffer, correct > > + * and report any bitflip/error > > + * - external + prepare + ecc + write: calculate ECC bytes and store them at > > + * the right place in the OOB buffer based > > + * on the OOB layout > > + * - external + finish + ecc + write: do nothing > > + * > > + * ["pipelined" ECC engine] > > + * - pipelined + prepare + raw + read: disable the controller's ECC engine if > > + * activated > > + * - pipelined + finish + raw + read: do nothing > > + * - pipelined + prepare + raw + write: disable the controller's ECC engine if > > + * activated > > + * - pipelined + finish + raw + write: do nothing > > + * - pipelined + prepare + ecc + read: enable the controller's ECC engine if > > + * deactivated > > + * - pipelined + finish + ecc + read: check the status, report any > > + * error/bitflip > > + * - pipelined + prepare + ecc + write: enable the controller's ECC engine if > > + * deactivated > > + * - pipelined + finish + ecc + write: do nothing > > + * > > + * ["ondie" ECC engine] > > + * - ondie + prepare + raw + read: send commands to disable the on-chip ECC > > + * engine if activated > > + * - ondie + finish + raw + read: do nothing > > + * - ondie + prepare + raw + write: send commands to disable the on-chip ECC > > + * engine if activated > > + * - ondie + finish + raw + write: do nothing > > + * - ondie + prepare + ecc + read: send commands to enable the on-chip ECC > > + * engine if deactivated > > + * - ondie + finish + ecc + read: send commands to check the status, report > > + * any error/bitflip > > + * - ondie + prepare + ecc + write: send commands to enable the on-chip ECC > > + * engine if deactivated > > + * - ondie + finish + ecc + write: do nothing > > + */ > > + > > +#include <linux/module.h> > > +#include <linux/mtd/nand.h> > > + > > Shouldn't we have kernel-docs for those functions? > > > +int nand_ecc_init_ctx(struct nand_device *nand) > > +{ > > + if (!nand->ecc.engine->ops->init_ctx) > > + return 0; > > + > > + return nand->ecc.engine->ops->init_ctx(nand); > > +} > > +EXPORT_SYMBOL(nand_ecc_init_ctx); > > + > > +void nand_ecc_cleanup_ctx(struct nand_device *nand) > > +{ > > + if (nand->ecc.engine->ops->cleanup_ctx) > > + nand->ecc.engine->ops->cleanup_ctx(nand); > > +} > > +EXPORT_SYMBOL(nand_ecc_cleanup_ctx); > > + > > +int nand_ecc_prepare_io_req(struct nand_device *nand, > > + struct nand_page_io_req *req) > > +{ > > + if (!nand->ecc.engine->ops->prepare_io_req) > > + return 0; > > + > > + return nand->ecc.engine->ops->prepare_io_req(nand, req); > > +} > > +EXPORT_SYMBOL(nand_ecc_prepare_io_req); > > + > > +int nand_ecc_finish_io_req(struct nand_device *nand, > > + struct nand_page_io_req *req) > > +{ > > + if (!nand->ecc.engine->ops->finish_io_req) > > + return 0; > > + > > + return nand->ecc.engine->ops->finish_io_req(nand, req); > > +} > > +EXPORT_SYMBOL(nand_ecc_finish_io_req); > > + > > +MODULE_LICENSE("GPL"); > > +MODULE_AUTHOR("Miquel Raynal <miquel.raynal@bootlin.com>"); > > +MODULE_DESCRIPTION("Generic ECC engine"); > > diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h > > index 2e9af24936cd..0be260fd2f86 100644 > > --- a/include/linux/mtd/nand.h > > +++ b/include/linux/mtd/nand.h > > @@ -221,6 +221,73 @@ struct nand_ops { > > bool (*isbad)(struct nand_device *nand, const struct nand_pos *pos); > > }; > > > > +/** > > + * struct nand_ecc_context - Context for the ECC engine > > + * @conf: basic ECC engine parameters > > + * @total: Total number of bytes used for storing ECC codes, this is used by > > Sometimes you start your description with an uppercase, sometimes not. > > > + * generic OOB layouts > > + * @priv: ECC engine driver private data > > + */ > > +struct nand_ecc_context { > > + struct nand_ecc_props conf; > > + unsigned int total; > > + void *priv; > > +}; > > + > > +/** > > + * struct nand_ecc_engine_ops - Generic ECC engine operations > > ^s/Generic// > > > + * @init_ctx: given a desired user configuration for the pointed NAND device, > > + * requests the ECC engine driver to setup a configuration with > > + * values it supports. > > + * @cleanup_ctx: clean the context initialized by @init_ctx. > > + * @prepare_io_req: is called before reading/writing a page to prepare the I/O > > + * request to be performed with ECC correction. > > + * @finish_io_req: is called after reading/writing a page to terminate the I/O > > + * request and ensure proper ECC correction. > > + */ > > +struct nand_ecc_engine_ops { > > + int (*init_ctx)(struct nand_device *nand); > > + void (*cleanup_ctx)(struct nand_device *nand); > > + int (*prepare_io_req)(struct nand_device *nand, > > + struct nand_page_io_req *req); > > + int (*finish_io_req)(struct nand_device *nand, > > + struct nand_page_io_req *req); > > +}; > > + > > +/** > > + * struct nand_ecc_engine - Generic ECC engine abstraction for NAND devices > > ^s/Generic// > > > + * @ops: ECC engine operations > > + */ > > +struct nand_ecc_engine { > > + struct nand_ecc_engine_ops *ops; > > +}; > > + > > +int nand_ecc_init_ctx(struct nand_device *nand); > > +void nand_ecc_cleanup_ctx(struct nand_device *nand); > > +int nand_ecc_prepare_io_req(struct nand_device *nand, > > + struct nand_page_io_req *req); > > +int nand_ecc_finish_io_req(struct nand_device *nand, > > + struct nand_page_io_req *req); > > + > > +/** > > + * struct nand_ecc - High-level ECC object > > I think you can drop the "High-level" and just say "Information > relative to the ECC" > > > + * @defaults: Default values, depend on the underlying subsystem > > + * @requirements: ECC requirements from the NAND chip perspective > > + * @user_conf: User desires in terms of ECC parameters > > + * @ctx: ECC context for the ECC engine, derived from the device @requirements > > + * the @user_conf and the @defaults > > + * @ondie_engine: On-die ECC engine reference, if any > > + * @engine: ECC engine actually bound > > + */ > > +struct nand_ecc { > > + struct nand_ecc_props defaults; > > + struct nand_ecc_props requirements; > > + struct nand_ecc_props user_conf; > > + struct nand_ecc_context ctx; > > + struct nand_ecc_engine *ondie_engine; > > + struct nand_ecc_engine *engine; > > +}; > > + > > /** > > * struct nand_device - NAND device > > * @mtd: MTD instance attached to the NAND device > All comments applied. Thanks, Miquèl
diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig index c1a45b071165..a4478ffa279d 100644 --- a/drivers/mtd/nand/Kconfig +++ b/drivers/mtd/nand/Kconfig @@ -9,4 +9,11 @@ source "drivers/mtd/nand/onenand/Kconfig" source "drivers/mtd/nand/raw/Kconfig" source "drivers/mtd/nand/spi/Kconfig" +menu "ECC engine support" + +config MTD_NAND_ECC + bool + +endmenu + endmenu diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile index 7ecd80c0a66e..981372953b56 100644 --- a/drivers/mtd/nand/Makefile +++ b/drivers/mtd/nand/Makefile @@ -6,3 +6,5 @@ obj-$(CONFIG_MTD_NAND_CORE) += nandcore.o obj-y += onenand/ obj-y += raw/ obj-y += spi/ + +nandcore-$(CONFIG_MTD_NAND_ECC) += ecc.o diff --git a/drivers/mtd/nand/ecc.c b/drivers/mtd/nand/ecc.c new file mode 100644 index 000000000000..e4f2b6fcbb12 --- /dev/null +++ b/drivers/mtd/nand/ecc.c @@ -0,0 +1,138 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Generic Error-Correcting Code (ECC) engine + * + * Copyright (C) 2019 Macronix + * Author: + * Miquèl RAYNAL <miquel.raynal@bootlin.com> + * + * + * This file describes the abstraction of any NAND ECC engine. It has been + * designed to fit most cases, including parallel NANDs and SPI-NANDs. + * + * There are three main situations where instantiating this ECC engine makes + * sense: + * - "external": The ECC engine is outside the NAND pipeline, typically this + * is a software ECC engine. One can also imagine a generic + * hardware ECC engine which would be an IP itself. Interacting + * with a SPI-NAND device without on-die ECC could be achieved + * thanks to the use of such external engine. + * - "pipelined": The ECC engine is inside the NAND pipeline, ie. on the + * controller's side. This is the case of most of the raw NAND + * controllers. These controllers usually embed an hardware ECC + * engine which is managed thanks to the same register set as + * the controller's. + * - "ondie": The ECC engine is inside the NAND pipeline, on the chip's side. + * Some NAND chips can correct themselves the data. The on-die + * correction can be enabled, disabled and the status of the + * correction after a read may be retrieved with a NAND command + * (may be vendor specific). + * + * Besides the initial setup and final cleanups, the interfaces are rather + * simple: + * - "prepare": Prepare an I/O request, check the ECC engine is enabled or + * disabled as requested before the I/O. In case of software + * correction, this step may involve to derive the ECC bytes and + * place them in the OOB area before a write. + * - "finish": Finish an I/O request, check the status of the operation ie. + * the data validity in case of a read (report to the upper layer + * any bitflip/errors). + * + * Both prepare/finish callbacks are supposed to enclose I/O request and will + * behave differently depending on the desired correction: + * - "raw": Correction disabled + * - "ecc": Correction enabled + * + * The request direction is impacting the logic as well: + * - "read": Load data from the NAND chip + * - "write": Store data in the NAND chip + * + * Mixing all this combinations together gives the following behavior. + * + * ["external" ECC engine] + * - external + prepare + raw + read: do nothing + * - external + finish + raw + read: do nothing + * - external + prepare + raw + write: do nothing + * - external + finish + raw + write: do nothing + * - external + prepare + ecc + read: do nothing + * - external + finish + ecc + read: calculate expected ECC bytes, extract + * ECC bytes from OOB buffer, correct + * and report any bitflip/error + * - external + prepare + ecc + write: calculate ECC bytes and store them at + * the right place in the OOB buffer based + * on the OOB layout + * - external + finish + ecc + write: do nothing + * + * ["pipelined" ECC engine] + * - pipelined + prepare + raw + read: disable the controller's ECC engine if + * activated + * - pipelined + finish + raw + read: do nothing + * - pipelined + prepare + raw + write: disable the controller's ECC engine if + * activated + * - pipelined + finish + raw + write: do nothing + * - pipelined + prepare + ecc + read: enable the controller's ECC engine if + * deactivated + * - pipelined + finish + ecc + read: check the status, report any + * error/bitflip + * - pipelined + prepare + ecc + write: enable the controller's ECC engine if + * deactivated + * - pipelined + finish + ecc + write: do nothing + * + * ["ondie" ECC engine] + * - ondie + prepare + raw + read: send commands to disable the on-chip ECC + * engine if activated + * - ondie + finish + raw + read: do nothing + * - ondie + prepare + raw + write: send commands to disable the on-chip ECC + * engine if activated + * - ondie + finish + raw + write: do nothing + * - ondie + prepare + ecc + read: send commands to enable the on-chip ECC + * engine if deactivated + * - ondie + finish + ecc + read: send commands to check the status, report + * any error/bitflip + * - ondie + prepare + ecc + write: send commands to enable the on-chip ECC + * engine if deactivated + * - ondie + finish + ecc + write: do nothing + */ + +#include <linux/module.h> +#include <linux/mtd/nand.h> + +int nand_ecc_init_ctx(struct nand_device *nand) +{ + if (!nand->ecc.engine->ops->init_ctx) + return 0; + + return nand->ecc.engine->ops->init_ctx(nand); +} +EXPORT_SYMBOL(nand_ecc_init_ctx); + +void nand_ecc_cleanup_ctx(struct nand_device *nand) +{ + if (nand->ecc.engine->ops->cleanup_ctx) + nand->ecc.engine->ops->cleanup_ctx(nand); +} +EXPORT_SYMBOL(nand_ecc_cleanup_ctx); + +int nand_ecc_prepare_io_req(struct nand_device *nand, + struct nand_page_io_req *req) +{ + if (!nand->ecc.engine->ops->prepare_io_req) + return 0; + + return nand->ecc.engine->ops->prepare_io_req(nand, req); +} +EXPORT_SYMBOL(nand_ecc_prepare_io_req); + +int nand_ecc_finish_io_req(struct nand_device *nand, + struct nand_page_io_req *req) +{ + if (!nand->ecc.engine->ops->finish_io_req) + return 0; + + return nand->ecc.engine->ops->finish_io_req(nand, req); +} +EXPORT_SYMBOL(nand_ecc_finish_io_req); + +MODULE_LICENSE("GPL"); +MODULE_AUTHOR("Miquel Raynal <miquel.raynal@bootlin.com>"); +MODULE_DESCRIPTION("Generic ECC engine"); diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h index 2e9af24936cd..0be260fd2f86 100644 --- a/include/linux/mtd/nand.h +++ b/include/linux/mtd/nand.h @@ -221,6 +221,73 @@ struct nand_ops { bool (*isbad)(struct nand_device *nand, const struct nand_pos *pos); }; +/** + * struct nand_ecc_context - Context for the ECC engine + * @conf: basic ECC engine parameters + * @total: Total number of bytes used for storing ECC codes, this is used by + * generic OOB layouts + * @priv: ECC engine driver private data + */ +struct nand_ecc_context { + struct nand_ecc_props conf; + unsigned int total; + void *priv; +}; + +/** + * struct nand_ecc_engine_ops - Generic ECC engine operations + * @init_ctx: given a desired user configuration for the pointed NAND device, + * requests the ECC engine driver to setup a configuration with + * values it supports. + * @cleanup_ctx: clean the context initialized by @init_ctx. + * @prepare_io_req: is called before reading/writing a page to prepare the I/O + * request to be performed with ECC correction. + * @finish_io_req: is called after reading/writing a page to terminate the I/O + * request and ensure proper ECC correction. + */ +struct nand_ecc_engine_ops { + int (*init_ctx)(struct nand_device *nand); + void (*cleanup_ctx)(struct nand_device *nand); + int (*prepare_io_req)(struct nand_device *nand, + struct nand_page_io_req *req); + int (*finish_io_req)(struct nand_device *nand, + struct nand_page_io_req *req); +}; + +/** + * struct nand_ecc_engine - Generic ECC engine abstraction for NAND devices + * @ops: ECC engine operations + */ +struct nand_ecc_engine { + struct nand_ecc_engine_ops *ops; +}; + +int nand_ecc_init_ctx(struct nand_device *nand); +void nand_ecc_cleanup_ctx(struct nand_device *nand); +int nand_ecc_prepare_io_req(struct nand_device *nand, + struct nand_page_io_req *req); +int nand_ecc_finish_io_req(struct nand_device *nand, + struct nand_page_io_req *req); + +/** + * struct nand_ecc - High-level ECC object + * @defaults: Default values, depend on the underlying subsystem + * @requirements: ECC requirements from the NAND chip perspective + * @user_conf: User desires in terms of ECC parameters + * @ctx: ECC context for the ECC engine, derived from the device @requirements + * the @user_conf and the @defaults + * @ondie_engine: On-die ECC engine reference, if any + * @engine: ECC engine actually bound + */ +struct nand_ecc { + struct nand_ecc_props defaults; + struct nand_ecc_props requirements; + struct nand_ecc_props user_conf; + struct nand_ecc_context ctx; + struct nand_ecc_engine *ondie_engine; + struct nand_ecc_engine *engine; +}; + /** * struct nand_device - NAND device * @mtd: MTD instance attached to the NAND device
Create a generic ECC engine object. Later the ecc.c file will receive more generic code coming from the raw NAND specific part. This is a base to instantiate ECC engine objects. Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> --- drivers/mtd/nand/Kconfig | 7 ++ drivers/mtd/nand/Makefile | 2 + drivers/mtd/nand/ecc.c | 138 ++++++++++++++++++++++++++++++++++++++ include/linux/mtd/nand.h | 67 ++++++++++++++++++ 4 files changed, 214 insertions(+) create mode 100644 drivers/mtd/nand/ecc.c