diff mbox series

[RFC,03/27] mtd: nand: Introduce the ECC engine abstraction

Message ID 20190221100216.25255-4-miquel.raynal@bootlin.com (mailing list archive)
State RFC
Headers show
Series Introduce the generic ECC engine abstraction | expand

Commit Message

Miquel Raynal Feb. 21, 2019, 10:01 a.m. UTC
Create a generic ECC engine object.

Later the ecc/engine.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                     |   1 +
 drivers/mtd/nand/Makefile                    |   1 +
 drivers/mtd/nand/ecc/Kconfig                 |   3 +
 drivers/mtd/nand/ecc/Makefile                |   3 +
 drivers/mtd/nand/ecc/engine.c                | 134 +++++++++++++++++++
 drivers/mtd/nand/raw/atmel/nand-controller.c |   9 +-
 drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c   |  12 +-
 drivers/mtd/nand/raw/marvell_nand.c          |   7 +-
 drivers/mtd/nand/raw/mtk_nand.c              |   4 +-
 drivers/mtd/nand/raw/nand_base.c             |  17 +--
 drivers/mtd/nand/raw/nand_esmt.c             |  11 +-
 drivers/mtd/nand/raw/nand_hynix.c            |  41 +++---
 drivers/mtd/nand/raw/nand_jedec.c            |   4 +-
 drivers/mtd/nand/raw/nand_micron.c           |  14 +-
 drivers/mtd/nand/raw/nand_onfi.c             |   8 +-
 drivers/mtd/nand/raw/nand_samsung.c          |  19 +--
 drivers/mtd/nand/raw/nand_toshiba.c          |  11 +-
 drivers/mtd/nand/raw/sunxi_nand.c            |   5 +-
 drivers/mtd/nand/raw/tegra_nand.c            |   9 +-
 drivers/mtd/nand/spi/core.c                  |   8 +-
 drivers/mtd/nand/spi/macronix.c              |   6 +-
 drivers/mtd/nand/spi/toshiba.c               |   6 +-
 include/linux/mtd/nand.h                     | 112 ++++++++++++++--
 include/linux/mtd/spinand.h                  |   4 +-
 24 files changed, 349 insertions(+), 100 deletions(-)
 create mode 100644 drivers/mtd/nand/ecc/Kconfig
 create mode 100644 drivers/mtd/nand/ecc/Makefile
 create mode 100644 drivers/mtd/nand/ecc/engine.c

Comments

Boris Brezillon Feb. 21, 2019, 11:16 a.m. UTC | #1
On Thu, 21 Feb 2019 11:01:52 +0100
Miquel Raynal <miquel.raynal@bootlin.com> wrote:


> diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
> index 30f0fb02abe2..9e3b018d0b83 100644
> --- a/include/linux/mtd/nand.h
> +++ b/include/linux/mtd/nand.h
> @@ -82,8 +82,14 @@ struct nand_pos {
>  	unsigned int page;
>  };
>  
> +enum nand_page_io_req_type {
> +	NAND_PAGE_READ = 0,
> +	NAND_PAGE_WRITE,
> +};
> +
>  /**
>   * struct nand_page_io_req - NAND I/O request object
> + * @type: the type of page I/O: read or write
>   * @pos: the position this I/O request is targeting
>   * @dataoffs: the offset within the page
>   * @datalen: number of data bytes to read from/write to this page
> @@ -99,6 +105,7 @@ struct nand_pos {
>   * specific commands/operations.
>   */
>  struct nand_page_io_req {
> +	enum nand_page_io_req_type type;

Can you add the reqtype enum and type field (+ patch the iterator
helpers) in a separate patch?

>  	struct nand_pos pos;
>  	unsigned int dataoffs;
>  	unsigned int datalen;
> @@ -116,13 +123,35 @@ struct nand_page_io_req {
>  };
>  
>  /**
> - * struct nand_ecc_req - NAND ECC requirements
> + * struct nand_ecc_conf - NAND ECC configuration
> + * @strength: ECC strength
> + * @step_size: Number of bytes per step
> + * @total: Total number of bytes used for storing ECC codes, this is used by
> + *         generic OOB layouts
> + */
> +struct nand_ecc_conf {

Please do the s/nand_ecc_req/nand_ecc_conf/ in a separate patch.

> +	unsigned int strength;
> +	unsigned int step_size;
> +	unsigned int total;

Do we really need to add this total field here? Looks like something
that should be kept private to the ECC engine implementation.

> +};
> +
> +/**
> + * struct nand_ecc_user_conf - User desired ECC configuration
> + * @mode: ECC mode
> + * @algo: ECC algorithm
>   * @strength: ECC strength
>   * @step_size: ECC step/block size
> + * @maximize: ECC parameters must be maximized depending on the device
> + *            capabilities
> + * @flags: User flags
>   */
> -struct nand_ecc_req {
> +struct nand_ecc_user_conf {
> +	int mode;

We should definitely name that one differently ('provider' maybe).

> +	unsigned int algo;
>  	unsigned int strength;
>  	unsigned int step_size;
> +	unsigned int maximize;
> +	unsigned int flags;

maximize could be a flag.

>  };
>  
>  #define NAND_ECCREQ(str, stp) { .strength = (str), .step_size = (stp) }
> @@ -157,11 +186,76 @@ struct nand_ops {
>  	bool (*isbad)(struct nand_device *nand, const struct nand_pos *pos);
>  };
Boris Brezillon Feb. 25, 2019, 6:55 p.m. UTC | #2
On Thu, 21 Feb 2019 11:01:52 +0100
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> +
> +/**
> + * 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 {

We might want to add a

	void (*put_engine)(struct nand_ecc_engine *engine);

here if we want the nanddev cleanup path to be generic.
This hook would be implemented by drivers where the ECC engine object is
refcounted (typically the case for HW ECC engines shared by the raw NAND
controller and the SPI controller).

Alternatively, you can just add one nand_put_xxx_ecc_engine() func per
engine class (SW, ondie and HW).

> +	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,
> +			      void *oobbuf);
> +	int (*finish_io_req)(struct nand_device *nand,
> +			     struct nand_page_io_req *req,
> +			     void *oobbuf);
> +};
Miquel Raynal Feb. 27, 2019, 9:26 a.m. UTC | #3
Hi Boris,

Boris Brezillon <bbrezillon@kernel.org> wrote on Thu, 21 Feb 2019
12:16:25 +0100:

> On Thu, 21 Feb 2019 11:01:52 +0100
> Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> 
> 
> > diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
> > index 30f0fb02abe2..9e3b018d0b83 100644
> > --- a/include/linux/mtd/nand.h
> > +++ b/include/linux/mtd/nand.h
> > @@ -82,8 +82,14 @@ struct nand_pos {
> >  	unsigned int page;
> >  };
> >  
> > +enum nand_page_io_req_type {
> > +	NAND_PAGE_READ = 0,
> > +	NAND_PAGE_WRITE,
> > +};
> > +
> >  /**
> >   * struct nand_page_io_req - NAND I/O request object
> > + * @type: the type of page I/O: read or write
> >   * @pos: the position this I/O request is targeting
> >   * @dataoffs: the offset within the page
> >   * @datalen: number of data bytes to read from/write to this page
> > @@ -99,6 +105,7 @@ struct nand_pos {
> >   * specific commands/operations.
> >   */
> >  struct nand_page_io_req {
> > +	enum nand_page_io_req_type type;  
> 
> Can you add the reqtype enum and type field (+ patch the iterator
> helpers) in a separate patch?

Sure.

> 
> >  	struct nand_pos pos;
> >  	unsigned int dataoffs;
> >  	unsigned int datalen;
> > @@ -116,13 +123,35 @@ struct nand_page_io_req {
> >  };
> >  
> >  /**
> > - * struct nand_ecc_req - NAND ECC requirements
> > + * struct nand_ecc_conf - NAND ECC configuration
> > + * @strength: ECC strength
> > + * @step_size: Number of bytes per step
> > + * @total: Total number of bytes used for storing ECC codes, this is used by
> > + *         generic OOB layouts
> > + */
> > +struct nand_ecc_conf {  
> 
> Please do the s/nand_ecc_req/nand_ecc_conf/ in a separate patch.

Ok.

> 
> > +	unsigned int strength;
> > +	unsigned int step_size;
> > +	unsigned int total;  
> 
> Do we really need to add this total field here? Looks like something
> that should be kept private to the ECC engine implementation.

It was initially private, but I realized it was needed by generic parts
(including for instance the generic OOB layouts) so it could not be
made private.

I just moved the 'total' entry out of the struct nand_ecc_conf and
moved it in the struct nand_ecc_ctx. It is still public but not in the
very generic "conf" structure anymore.

> 
> > +};
> > +
> > +/**
> > + * struct nand_ecc_user_conf - User desired ECC configuration
> > + * @mode: ECC mode
> > + * @algo: ECC algorithm
> >   * @strength: ECC strength
> >   * @step_size: ECC step/block size
> > + * @maximize: ECC parameters must be maximized depending on the device
> > + *            capabilities
> > + * @flags: User flags
> >   */
> > -struct nand_ecc_req {
> > +struct nand_ecc_user_conf {
> > +	int mode;  
> 
> We should definitely name that one differently ('provider' maybe).

Changed to provider.

> 
> > +	unsigned int algo;
> >  	unsigned int strength;
> >  	unsigned int step_size;
> > +	unsigned int maximize;
> > +	unsigned int flags;  
> 
> maximize could be a flag.

It is now.

> 
> >  };
> >  
> >  #define NAND_ECCREQ(str, stp) { .strength = (str), .step_size = (stp) }
> > @@ -157,11 +186,76 @@ struct nand_ops {
> >  	bool (*isbad)(struct nand_device *nand, const struct nand_pos *pos);
> >  };  
> 


Thanks,
Miquèl
Boris Brezillon Feb. 27, 2019, 9:47 a.m. UTC | #4
On Wed, 27 Feb 2019 10:26:42 +0100
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> >   
> > > +	unsigned int strength;
> > > +	unsigned int step_size;
> > > +	unsigned int total;    
> > 
> > Do we really need to add this total field here? Looks like something
> > that should be kept private to the ECC engine implementation.  
> 
> It was initially private, but I realized it was needed by generic parts
> (including for instance the generic OOB layouts) so it could not be
> made private.
> 
> I just moved the 'total' entry out of the struct nand_ecc_conf and
> moved it in the struct nand_ecc_ctx. It is still public but not in the
> very generic "conf" structure anymore.

Ack.
Miquel Raynal Feb. 27, 2019, 1:56 p.m. UTC | #5
Hi Boris,

Boris Brezillon <bbrezillon@kernel.org> wrote on Mon, 25 Feb 2019
19:55:43 +0100:

> On Thu, 21 Feb 2019 11:01:52 +0100
> Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> 
> > +
> > +/**
> > + * 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 {  
> 
> We might want to add a
> 
> 	void (*put_engine)(struct nand_ecc_engine *engine);
> 
> here if we want the nanddev cleanup path to be generic.
> This hook would be implemented by drivers where the ECC engine object is
> refcounted (typically the case for HW ECC engines shared by the raw NAND
> controller and the SPI controller).
> 
> Alternatively, you can just add one nand_put_xxx_ecc_engine() func per
> engine class (SW, ondie and HW).

Can't this be handled in the init/cleanup_ctx() path directly?

Furthermore if this is just a hook to do reference counting.


Thanks,
Miquèl
Boris Brezillon Feb. 27, 2019, 2:06 p.m. UTC | #6
On Wed, 27 Feb 2019 14:56:07 +0100
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Hi Boris,
> 
> Boris Brezillon <bbrezillon@kernel.org> wrote on Mon, 25 Feb 2019
> 19:55:43 +0100:
> 
> > On Thu, 21 Feb 2019 11:01:52 +0100
> > Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >   
> > > +
> > > +/**
> > > + * 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 {    
> > 
> > We might want to add a
> > 
> > 	void (*put_engine)(struct nand_ecc_engine *engine);
> > 
> > here if we want the nanddev cleanup path to be generic.
> > This hook would be implemented by drivers where the ECC engine object is
> > refcounted (typically the case for HW ECC engines shared by the raw NAND
> > controller and the SPI controller).
> > 
> > Alternatively, you can just add one nand_put_xxx_ecc_engine() func per
> > engine class (SW, ondie and HW).  
> 
> Can't this be handled in the init/cleanup_ctx() path directly?

You really have to get the reference before init_ctx() otherwise the
engine might disappear between your get() and init() call, and, to keep
things symmetric, I think it's best to handle the put() outside the
cleanup_ctx() path.

> 
> Furthermore if this is just a hook to do reference counting.

Well, what this put() does depends on the class of engine. For SW and
on-die ECC it can be a NOOP (that's true only if you keep the approach
where you have a single instance shared by everyone for SW-based ECC
engines).
For HW-controller-side ECC engines, you'll have to call device_get() on
the parent device in your nand_get_hw_ecc_engine() function while you
hold the lock protecting the ECC engine list. And device_put() will be
called in nand_put_hw_ecc_engine().
Miquel Raynal Feb. 27, 2019, 2:19 p.m. UTC | #7
Hi Boris,

Boris Brezillon <boris.brezillon@collabora.com> wrote on Wed, 27 Feb
2019 15:06:33 +0100:

> On Wed, 27 Feb 2019 14:56:07 +0100
> Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> 
> > Hi Boris,
> > 
> > Boris Brezillon <bbrezillon@kernel.org> wrote on Mon, 25 Feb 2019
> > 19:55:43 +0100:
> >   
> > > On Thu, 21 Feb 2019 11:01:52 +0100
> > > Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > >     
> > > > +
> > > > +/**
> > > > + * 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 {      
> > > 
> > > We might want to add a
> > > 
> > > 	void (*put_engine)(struct nand_ecc_engine *engine);
> > > 
> > > here if we want the nanddev cleanup path to be generic.
> > > This hook would be implemented by drivers where the ECC engine object is
> > > refcounted (typically the case for HW ECC engines shared by the raw NAND
> > > controller and the SPI controller).
> > > 
> > > Alternatively, you can just add one nand_put_xxx_ecc_engine() func per
> > > engine class (SW, ondie and HW).    
> > 
> > Can't this be handled in the init/cleanup_ctx() path directly?  
> 
> You really have to get the reference before init_ctx() otherwise the
> engine might disappear between your get() and init() call, and, to keep
> things symmetric, I think it's best to handle the put() outside the
> cleanup_ctx() path.
> 
> > 
> > Furthermore if this is just a hook to do reference counting.  
> 
> Well, what this put() does depends on the class of engine. For SW and
> on-die ECC it can be a NOOP (that's true only if you keep the approach
> where you have a single instance shared by everyone for SW-based ECC
> engines).
> For HW-controller-side ECC engines, you'll have to call device_get() on
> the parent device in your nand_get_hw_ecc_engine() function while you
> hold the lock protecting the ECC engine list. And device_put() will be
> called in nand_put_hw_ecc_engine().


I see.

Then I prefer keeping the logic in the core, not in the engine driver
and propose a

        void nand_ecc_put_engine(struct nand_ecc_engine *engine)

which will do nothing for on-die/sw engines and drop the reference for
hw engines. I will also rename the "find_ecc_engine" to "get_engine" so
that the call to the "put" helper has more meaning.


Thanks,
Miquèl
Boris Brezillon Feb. 27, 2019, 2:28 p.m. UTC | #8
On Wed, 27 Feb 2019 15:19:57 +0100
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Hi Boris,
> 
> Boris Brezillon <boris.brezillon@collabora.com> wrote on Wed, 27 Feb
> 2019 15:06:33 +0100:
> 
> > On Wed, 27 Feb 2019 14:56:07 +0100
> > Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >   
> > > Hi Boris,
> > > 
> > > Boris Brezillon <bbrezillon@kernel.org> wrote on Mon, 25 Feb 2019
> > > 19:55:43 +0100:
> > >     
> > > > On Thu, 21 Feb 2019 11:01:52 +0100
> > > > Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > > >       
> > > > > +
> > > > > +/**
> > > > > + * 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 {        
> > > > 
> > > > We might want to add a
> > > > 
> > > > 	void (*put_engine)(struct nand_ecc_engine *engine);
> > > > 
> > > > here if we want the nanddev cleanup path to be generic.
> > > > This hook would be implemented by drivers where the ECC engine object is
> > > > refcounted (typically the case for HW ECC engines shared by the raw NAND
> > > > controller and the SPI controller).
> > > > 
> > > > Alternatively, you can just add one nand_put_xxx_ecc_engine() func per
> > > > engine class (SW, ondie and HW).      
> > > 
> > > Can't this be handled in the init/cleanup_ctx() path directly?    
> > 
> > You really have to get the reference before init_ctx() otherwise the
> > engine might disappear between your get() and init() call, and, to keep
> > things symmetric, I think it's best to handle the put() outside the
> > cleanup_ctx() path.
> >   
> > > 
> > > Furthermore if this is just a hook to do reference counting.    
> > 
> > Well, what this put() does depends on the class of engine. For SW and
> > on-die ECC it can be a NOOP (that's true only if you keep the approach
> > where you have a single instance shared by everyone for SW-based ECC
> > engines).
> > For HW-controller-side ECC engines, you'll have to call device_get() on
> > the parent device in your nand_get_hw_ecc_engine() function while you
> > hold the lock protecting the ECC engine list. And device_put() will be
> > called in nand_put_hw_ecc_engine().  
> 
> 
> I see.
> 
> Then I prefer keeping the logic in the core, not in the engine driver
> and propose a
> 
>         void nand_ecc_put_engine(struct nand_ecc_engine *engine)
> 
> which will do nothing for on-die/sw engines and drop the reference for
> hw engines. I will also rename the "find_ecc_engine" to "get_engine" so
> that the call to the "put" helper has more meaning.

Ack for most of it. One thing I'd like to clarify: it's probably better
to have a separate function called nand_ecc_put_hw_engine() which you'll
call from nand_ecc_put_engine() when you're dealing with an
HW ECC engine rather than calling put_device() directly from
nand_ecc_put_engine(). This way you keep the code for HW ECC engine
well isolated.

Same goes for the nand_ecc_get_engine() path, just delegate to
nand_ecc_get_hw_engine() when ->provider == HW_ECC.
Miquel Raynal Feb. 27, 2019, 2:34 p.m. UTC | #9
> > > > Furthermore if this is just a hook to do reference counting.      
> > > 
> > > Well, what this put() does depends on the class of engine. For SW and
> > > on-die ECC it can be a NOOP (that's true only if you keep the approach
> > > where you have a single instance shared by everyone for SW-based ECC
> > > engines).
> > > For HW-controller-side ECC engines, you'll have to call device_get() on
> > > the parent device in your nand_get_hw_ecc_engine() function while you
> > > hold the lock protecting the ECC engine list. And device_put() will be
> > > called in nand_put_hw_ecc_engine().    
> > 
> > 
> > I see.
> > 
> > Then I prefer keeping the logic in the core, not in the engine driver
> > and propose a
> > 
> >         void nand_ecc_put_engine(struct nand_ecc_engine *engine)
> > 
> > which will do nothing for on-die/sw engines and drop the reference for
> > hw engines. I will also rename the "find_ecc_engine" to "get_engine" so
> > that the call to the "put" helper has more meaning.  
> 
> Ack for most of it. One thing I'd like to clarify: it's probably better
> to have a separate function called nand_ecc_put_hw_engine() which you'll
> call from nand_ecc_put_engine() when you're dealing with an
> HW ECC engine rather than calling put_device() directly from
> nand_ecc_put_engine(). This way you keep the code for HW ECC engine
> well isolated.
> 
> Same goes for the nand_ecc_get_engine() path, just delegate to
> nand_ecc_get_hw_engine() when ->provider == HW_ECC.

Ack.
diff mbox series

Patch

diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index e8d26a715922..bc0c26fcb190 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -10,5 +10,6 @@  if MTD_NAND_CORE
 source "drivers/mtd/nand/onenand/Kconfig"
 source "drivers/mtd/nand/raw/Kconfig"
 source "drivers/mtd/nand/spi/Kconfig"
+source "drivers/mtd/nand/ecc/Kconfig"
 
 endif
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index 7ecd80c0a66e..9772e781534d 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -6,3 +6,4 @@  obj-$(CONFIG_MTD_NAND_CORE) += nandcore.o
 obj-y	+= onenand/
 obj-y	+= raw/
 obj-y	+= spi/
+obj-y	+= ecc/
diff --git a/drivers/mtd/nand/ecc/Kconfig b/drivers/mtd/nand/ecc/Kconfig
new file mode 100644
index 000000000000..66c396dcbfbd
--- /dev/null
+++ b/drivers/mtd/nand/ecc/Kconfig
@@ -0,0 +1,3 @@ 
+menu "ECC engine support"
+
+endmenu
diff --git a/drivers/mtd/nand/ecc/Makefile b/drivers/mtd/nand/ecc/Makefile
new file mode 100644
index 000000000000..6a42577ba424
--- /dev/null
+++ b/drivers/mtd/nand/ecc/Makefile
@@ -0,0 +1,3 @@ 
+# SPDX-License-Identifier: GPL-2.0
+
+obj-$(CONFIG_MTD_NAND_CORE) += engine.o
diff --git a/drivers/mtd/nand/ecc/engine.c b/drivers/mtd/nand/ecc/engine.c
new file mode 100644
index 000000000000..e3d8bb092e2a
--- /dev/null
+++ b/drivers/mtd/nand/ecc/engine.c
@@ -0,0 +1,134 @@ 
+// 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);
+}
+
+void nand_ecc_cleanup_ctx(struct nand_device *nand)
+{
+	if (nand->ecc.engine->ops->cleanup_ctx)
+		nand->ecc.engine->ops->cleanup_ctx(nand);
+}
+
+int nand_ecc_prepare_io_req(struct nand_device *nand,
+			    struct nand_page_io_req *req, void *oobbuf)
+{
+	if (!nand->ecc.engine->ops->prepare_io_req)
+		return 0;
+
+	return nand->ecc.engine->ops->prepare_io_req(nand, req, oobbuf);
+}
+
+int nand_ecc_finish_io_req(struct nand_device *nand,
+			   struct nand_page_io_req *req, void *oobbuf)
+{
+	if (!nand->ecc.engine->ops->finish_io_req)
+		return 0;
+
+	return nand->ecc.engine->ops->finish_io_req(nand, req, oobbuf);
+}
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Miquel Raynal <miquel.raynal@bootlin.com>");
+MODULE_DESCRIPTION("Generic ECC engine");
diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c b/drivers/mtd/nand/raw/atmel/nand-controller.c
index 9867e9115399..3dacaa352a58 100644
--- a/drivers/mtd/nand/raw/atmel/nand-controller.c
+++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
@@ -1039,6 +1039,7 @@  static int atmel_hsmc_nand_pmecc_read_page_raw(struct nand_chip *chip,
 
 static int atmel_nand_pmecc_init(struct nand_chip *chip)
 {
+	struct nand_ecc_conf *requirements = &chip->base.ecc.requirements;
 	struct mtd_info *mtd = nand_to_mtd(chip);
 	struct atmel_nand *nand = to_atmel_nand(chip);
 	struct atmel_nand_controller *nc;
@@ -1068,15 +1069,15 @@  static int atmel_nand_pmecc_init(struct nand_chip *chip)
 		req.ecc.strength = ATMEL_PMECC_MAXIMIZE_ECC_STRENGTH;
 	else if (chip->ecc.strength)
 		req.ecc.strength = chip->ecc.strength;
-	else if (chip->base.eccreq.strength)
-		req.ecc.strength = chip->base.eccreq.strength;
+	else if (requirements->strength)
+		req.ecc.strength = requirements->strength;
 	else
 		req.ecc.strength = ATMEL_PMECC_MAXIMIZE_ECC_STRENGTH;
 
 	if (chip->ecc.size)
 		req.ecc.sectorsize = chip->ecc.size;
-	else if (chip->base.eccreq.step_size)
-		req.ecc.sectorsize = chip->base.eccreq.step_size;
+	else if (requirements->step_size)
+		req.ecc.sectorsize = requirements->step_size;
 	else
 		req.ecc.sectorsize = ATMEL_PMECC_SECTOR_SIZE_AUTO;
 
diff --git a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
index dbefb6bac5c9..1d0f556129de 100644
--- a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
@@ -204,8 +204,8 @@  static int set_geometry_by_ecc_info(struct gpmi_nand_data *this,
 	default:
 		dev_err(this->dev,
 			"unsupported nand chip. ecc bits : %d, ecc size : %d\n",
-			chip->base.eccreq.strength,
-			chip->base.eccreq.step_size);
+			chip->base.ecc.requirements.strength,
+			chip->base.ecc.requirements.step_size);
 		return -EINVAL;
 	}
 	geo->ecc_chunk_size = ecc_step;
@@ -418,13 +418,13 @@  int common_nfc_set_geometry(struct gpmi_nand_data *this)
 
 	if ((of_property_read_bool(this->dev->of_node, "fsl,use-minimum-ecc"))
 				|| legacy_set_geometry(this)) {
-		if (!(chip->base.eccreq.strength > 0 &&
-		      chip->base.eccreq.step_size > 0))
+		if (!(chip->base.ecc.requirements.strength > 0 &&
+		      chip->base.ecc.requirements.step_size > 0))
 			return -EINVAL;
 
 		return set_geometry_by_ecc_info(this,
-						chip->base.eccreq.strength,
-						chip->base.eccreq.step_size);
+						chip->base.ecc.requirements.strength,
+						chip->base.ecc.requirements.step_size);
 	}
 
 	return 0;
diff --git a/drivers/mtd/nand/raw/marvell_nand.c b/drivers/mtd/nand/raw/marvell_nand.c
index d21e808bb075..4d0d3c34ac12 100644
--- a/drivers/mtd/nand/raw/marvell_nand.c
+++ b/drivers/mtd/nand/raw/marvell_nand.c
@@ -2244,13 +2244,14 @@  static int marvell_nand_ecc_init(struct mtd_info *mtd,
 				 struct nand_ecc_ctrl *ecc)
 {
 	struct nand_chip *chip = mtd_to_nand(mtd);
+	struct nand_ecc_conf *requirements = &chip->base.ecc.requirements;
 	struct marvell_nfc *nfc = to_marvell_nfc(chip->controller);
 	int ret;
 
 	if (ecc->mode != NAND_ECC_NONE && (!ecc->size || !ecc->strength)) {
-		if (chip->base.eccreq.step_size && chip->base.eccreq.strength) {
-			ecc->size = chip->base.eccreq.step_size;
-			ecc->strength = chip->base.eccreq.strength;
+		if (requirements->step_size && requirements->strength) {
+			ecc->size = requirements->step_size;
+			ecc->strength = requirements->strength;
 		} else {
 			dev_info(nfc->dev,
 				 "No minimum ECC strength, using 1b/512B\n");
diff --git a/drivers/mtd/nand/raw/mtk_nand.c b/drivers/mtd/nand/raw/mtk_nand.c
index bfb89aca4155..b66eb96b7d49 100644
--- a/drivers/mtd/nand/raw/mtk_nand.c
+++ b/drivers/mtd/nand/raw/mtk_nand.c
@@ -1197,8 +1197,8 @@  static int mtk_nfc_ecc_init(struct device *dev, struct mtd_info *mtd)
 	/* if optional dt settings not present */
 	if (!nand->ecc.size || !nand->ecc.strength) {
 		/* use datasheet requirements */
-		nand->ecc.strength = nand->base.eccreq.strength;
-		nand->ecc.size = nand->base.eccreq.step_size;
+		nand->ecc.strength = nand->base.ecc.requirements.strength;
+		nand->ecc.size = nand->base.ecc.requirements.step_size;
 
 		/*
 		 * align eccstrength and eccsize
diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 13efa206c7e6..82ac620bbdac 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -4592,8 +4592,8 @@  static bool find_full_id_nand(struct nand_chip *chip,
 					   memorg->pagesize *
 					   memorg->pages_per_eraseblock);
 		chip->options |= type->options;
-		chip->base.eccreq.strength = NAND_ECC_STRENGTH(type);
-		chip->base.eccreq.step_size = NAND_ECC_STEP(type);
+		chip->base.ecc.requirements.strength = NAND_ECC_STRENGTH(type);
+		chip->base.ecc.requirements.step_size = NAND_ECC_STEP(type);
 		chip->onfi_timing_mode_default =
 					type->onfi_timing_mode_default;
 
@@ -5266,8 +5266,8 @@  nand_match_ecc_req(struct nand_chip *chip,
 {
 	struct mtd_info *mtd = nand_to_mtd(chip);
 	const struct nand_ecc_step_info *stepinfo;
-	int req_step = chip->base.eccreq.step_size;
-	int req_strength = chip->base.eccreq.strength;
+	int req_step = chip->base.ecc.requirements.step_size;
+	int req_strength = chip->base.ecc.requirements.strength;
 	int req_corr, step_size, strength, nsteps, ecc_bytes, ecc_bytes_total;
 	int best_step, best_strength, best_ecc_bytes;
 	int best_ecc_bytes_total = INT_MAX;
@@ -5458,9 +5458,10 @@  static bool nand_ecc_strength_good(struct nand_chip *chip)
 {
 	struct mtd_info *mtd = nand_to_mtd(chip);
 	struct nand_ecc_ctrl *ecc = &chip->ecc;
+	struct nand_ecc_conf *requirements = &chip->base.ecc.requirements;
 	int corr, ds_corr;
 
-	if (ecc->size == 0 || chip->base.eccreq.step_size == 0)
+	if (ecc->size == 0 || requirements->step_size == 0)
 		/* Not enough information */
 		return true;
 
@@ -5469,10 +5470,10 @@  static bool nand_ecc_strength_good(struct nand_chip *chip)
 	 * the correction density.
 	 */
 	corr = (mtd->writesize * ecc->strength) / ecc->size;
-	ds_corr = (mtd->writesize * chip->base.eccreq.strength) /
-		  chip->base.eccreq.step_size;
+	ds_corr = (mtd->writesize * requirements->strength) /
+		  requirements->step_size;
 
-	return corr >= ds_corr && ecc->strength >= chip->base.eccreq.strength;
+	return corr >= ds_corr && ecc->strength >= requirements->strength;
 }
 
 static int rawnand_erase(struct nand_device *nand, const struct nand_pos *pos)
diff --git a/drivers/mtd/nand/raw/nand_esmt.c b/drivers/mtd/nand/raw/nand_esmt.c
index 3de5e89482f5..c3fc85f77ff8 100644
--- a/drivers/mtd/nand/raw/nand_esmt.c
+++ b/drivers/mtd/nand/raw/nand_esmt.c
@@ -10,24 +10,25 @@ 
 
 static void esmt_nand_decode_id(struct nand_chip *chip)
 {
+	struct nand_ecc_conf *requirements = &chip->base.ecc.requirements;
 	nand_decode_ext_id(chip);
 
 	/* Extract ECC requirements from 5th id byte. */
 	if (chip->id.len >= 5 && nand_is_slc(chip)) {
-		chip->base.eccreq.step_size = 512;
+		requirements->step_size = 512;
 		switch (chip->id.data[4] & 0x3) {
 		case 0x0:
-			chip->base.eccreq.strength = 4;
+			requirements->strength = 4;
 			break;
 		case 0x1:
-			chip->base.eccreq.strength = 2;
+			requirements->strength = 2;
 			break;
 		case 0x2:
-			chip->base.eccreq.strength = 1;
+			requirements->strength = 1;
 			break;
 		default:
 			WARN(1, "Could not get ECC info");
-			chip->base.eccreq.step_size = 0;
+			requirements->step_size = 0;
 			break;
 		}
 	}
diff --git a/drivers/mtd/nand/raw/nand_hynix.c b/drivers/mtd/nand/raw/nand_hynix.c
index 821d221b83eb..3c05991e3445 100644
--- a/drivers/mtd/nand/raw/nand_hynix.c
+++ b/drivers/mtd/nand/raw/nand_hynix.c
@@ -504,34 +504,35 @@  static void hynix_nand_extract_oobsize(struct nand_chip *chip,
 static void hynix_nand_extract_ecc_requirements(struct nand_chip *chip,
 						bool valid_jedecid)
 {
+	struct nand_ecc_conf *requirements = &chip->base.ecc.requirements;
 	u8 ecc_level = (chip->id.data[4] >> 4) & 0x7;
 
 	if (valid_jedecid) {
 		/* Reference: H27UCG8T2E datasheet */
-		chip->base.eccreq.step_size = 1024;
+		requirements->step_size = 1024;
 
 		switch (ecc_level) {
 		case 0:
-			chip->base.eccreq.step_size = 0;
-			chip->base.eccreq.strength = 0;
+			requirements->step_size = 0;
+			requirements->strength = 0;
 			break;
 		case 1:
-			chip->base.eccreq.strength = 4;
+			requirements->strength = 4;
 			break;
 		case 2:
-			chip->base.eccreq.strength = 24;
+			requirements->strength = 24;
 			break;
 		case 3:
-			chip->base.eccreq.strength = 32;
+			requirements->strength = 32;
 			break;
 		case 4:
-			chip->base.eccreq.strength = 40;
+			requirements->strength = 40;
 			break;
 		case 5:
-			chip->base.eccreq.strength = 50;
+			requirements->strength = 50;
 			break;
 		case 6:
-			chip->base.eccreq.strength = 60;
+			requirements->strength = 60;
 			break;
 		default:
 			/*
@@ -552,14 +553,14 @@  static void hynix_nand_extract_ecc_requirements(struct nand_chip *chip,
 		if (nand_tech < 3) {
 			/* > 26nm, reference: H27UBG8T2A datasheet */
 			if (ecc_level < 5) {
-				chip->base.eccreq.step_size = 512;
-				chip->base.eccreq.strength = 1 << ecc_level;
+				requirements->step_size = 512;
+				requirements->strength = 1 << ecc_level;
 			} else if (ecc_level < 7) {
 				if (ecc_level == 5)
-					chip->base.eccreq.step_size = 2048;
+					requirements->step_size = 2048;
 				else
-					chip->base.eccreq.step_size = 1024;
-				chip->base.eccreq.strength = 24;
+					requirements->step_size = 1024;
+				requirements->strength = 24;
 			} else {
 				/*
 				 * We should never reach this case, but if that
@@ -572,14 +573,14 @@  static void hynix_nand_extract_ecc_requirements(struct nand_chip *chip,
 		} else {
 			/* <= 26nm, reference: H27UBG8T2B datasheet */
 			if (!ecc_level) {
-				chip->base.eccreq.step_size = 0;
-				chip->base.eccreq.strength = 0;
+				requirements->step_size = 0;
+				requirements->strength = 0;
 			} else if (ecc_level < 5) {
-				chip->base.eccreq.step_size = 512;
-				chip->base.eccreq.strength = 1 << (ecc_level - 1);
+				requirements->step_size = 512;
+				requirements->strength = 1 << (ecc_level - 1);
 			} else {
-				chip->base.eccreq.step_size = 1024;
-				chip->base.eccreq.strength = 24 +
+				requirements->step_size = 1024;
+				requirements->strength = 24 +
 							(8 * (ecc_level - 5));
 			}
 		}
diff --git a/drivers/mtd/nand/raw/nand_jedec.c b/drivers/mtd/nand/raw/nand_jedec.c
index 9b540e76f84f..46afa25abc70 100644
--- a/drivers/mtd/nand/raw/nand_jedec.c
+++ b/drivers/mtd/nand/raw/nand_jedec.c
@@ -110,8 +110,8 @@  int nand_jedec_detect(struct nand_chip *chip)
 	ecc = &p->ecc_info[0];
 
 	if (ecc->codeword_size >= 9) {
-		chip->base.eccreq.strength = ecc->ecc_bits;
-		chip->base.eccreq.step_size = 1 << ecc->codeword_size;
+		chip->base.ecc.requirements.strength = ecc->ecc_bits;
+		chip->base.ecc.requirements.step_size = 1 << ecc->codeword_size;
 	} else {
 		pr_warn("Invalid codeword size\n");
 	}
diff --git a/drivers/mtd/nand/raw/nand_micron.c b/drivers/mtd/nand/raw/nand_micron.c
index 7a2cef02eacd..dd2358a01f54 100644
--- a/drivers/mtd/nand/raw/nand_micron.c
+++ b/drivers/mtd/nand/raw/nand_micron.c
@@ -379,6 +379,7 @@  enum {
  */
 static int micron_supports_on_die_ecc(struct nand_chip *chip)
 {
+	struct nand_ecc_conf *requirements = &chip->base.ecc.requirements;
 	u8 id[5];
 	int ret;
 
@@ -391,7 +392,7 @@  static int micron_supports_on_die_ecc(struct nand_chip *chip)
 	/*
 	 * We only support on-die ECC of 4/512 or 8/512
 	 */
-	if  (chip->base.eccreq.strength != 4 && chip->base.eccreq.strength != 8)
+	if  (requirements->strength != 4 && requirements->strength != 8)
 		return MICRON_ON_DIE_UNSUPPORTED;
 
 	/* 0x2 means on-die ECC is available. */
@@ -424,7 +425,7 @@  static int micron_supports_on_die_ecc(struct nand_chip *chip)
 	/*
 	 * We only support on-die ECC of 4/512 or 8/512
 	 */
-	if  (chip->base.eccreq.strength != 4 && chip->base.eccreq.strength != 8)
+	if  (requirements->strength != 4 && requirements->strength != 8)
 		return MICRON_ON_DIE_UNSUPPORTED;
 
 	return MICRON_ON_DIE_SUPPORTED;
@@ -432,6 +433,7 @@  static int micron_supports_on_die_ecc(struct nand_chip *chip)
 
 static int micron_nand_init(struct nand_chip *chip)
 {
+	struct nand_ecc_conf *requirements = &chip->base.ecc.requirements;
 	struct mtd_info *mtd = nand_to_mtd(chip);
 	struct micron_nand *micron;
 	int ondie;
@@ -479,7 +481,7 @@  static int micron_nand_init(struct nand_chip *chip)
 		 * That's not needed for 8-bit ECC, because the status expose
 		 * a better approximation of the number of bitflips in a page.
 		 */
-		if (chip->base.eccreq.strength == 4) {
+		if (requirements->strength == 4) {
 			micron->ecc.rawbuf = kmalloc(mtd->writesize +
 						     mtd->oobsize,
 						     GFP_KERNEL);
@@ -489,16 +491,16 @@  static int micron_nand_init(struct nand_chip *chip)
 			}
 		}
 
-		if (chip->base.eccreq.strength == 4)
+		if (requirements->strength == 4)
 			mtd_set_ooblayout(mtd,
 					  &micron_nand_on_die_4_ooblayout_ops);
 		else
 			mtd_set_ooblayout(mtd,
 					  &micron_nand_on_die_8_ooblayout_ops);
 
-		chip->ecc.bytes = chip->base.eccreq.strength * 2;
+		chip->ecc.bytes = requirements->strength * 2;
 		chip->ecc.size = 512;
-		chip->ecc.strength = chip->base.eccreq.strength;
+		chip->ecc.strength = requirements->strength;
 		chip->ecc.algo = NAND_ECC_BCH;
 		chip->ecc.read_page = micron_nand_read_page_on_die_ecc;
 		chip->ecc.write_page = micron_nand_write_page_on_die_ecc;
diff --git a/drivers/mtd/nand/raw/nand_onfi.c b/drivers/mtd/nand/raw/nand_onfi.c
index 0b879bd0a68c..9d21b47ebef1 100644
--- a/drivers/mtd/nand/raw/nand_onfi.c
+++ b/drivers/mtd/nand/raw/nand_onfi.c
@@ -94,8 +94,8 @@  static int nand_flash_detect_ext_param_page(struct nand_chip *chip,
 		goto ext_out;
 	}
 
-	chip->base.eccreq.strength = ecc->ecc_bits;
-	chip->base.eccreq.step_size = 1 << ecc->codeword_size;
+	chip->base.ecc.requirements.strength = ecc->ecc_bits;
+	chip->base.ecc.requirements.step_size = 1 << ecc->codeword_size;
 	ret = 0;
 
 ext_out:
@@ -252,8 +252,8 @@  int nand_onfi_detect(struct nand_chip *chip)
 		chip->options |= NAND_BUSWIDTH_16;
 
 	if (p->ecc_bits != 0xff) {
-		chip->base.eccreq.strength = p->ecc_bits;
-		chip->base.eccreq.step_size = 512;
+		chip->base.ecc.requirements.strength = p->ecc_bits;
+		chip->base.ecc.requirements.step_size = 512;
 	} else if (onfi_version >= 21 &&
 		(le16_to_cpu(p->features) & ONFI_FEATURE_EXT_PARAM_PAGE)) {
 
diff --git a/drivers/mtd/nand/raw/nand_samsung.c b/drivers/mtd/nand/raw/nand_samsung.c
index f7d7041b6213..4874ba33db15 100644
--- a/drivers/mtd/nand/raw/nand_samsung.c
+++ b/drivers/mtd/nand/raw/nand_samsung.c
@@ -19,6 +19,7 @@ 
 
 static void samsung_nand_decode_id(struct nand_chip *chip)
 {
+	struct nand_ecc_conf *requirements = &chip->base.ecc.requirements;
 	struct mtd_info *mtd = nand_to_mtd(chip);
 	struct nand_memory_organization *memorg;
 
@@ -80,23 +81,23 @@  static void samsung_nand_decode_id(struct nand_chip *chip)
 		/* Extract ECC requirements from 5th id byte*/
 		extid = (chip->id.data[4] >> 4) & 0x07;
 		if (extid < 5) {
-			chip->base.eccreq.step_size = 512;
-			chip->base.eccreq.strength = 1 << extid;
+			requirements->step_size = 512;
+			requirements->strength = 1 << extid;
 		} else {
-			chip->base.eccreq.step_size = 1024;
+			requirements->step_size = 1024;
 			switch (extid) {
 			case 5:
-				chip->base.eccreq.strength = 24;
+				requirements->strength = 24;
 				break;
 			case 6:
-				chip->base.eccreq.strength = 40;
+				requirements->strength = 40;
 				break;
 			case 7:
-				chip->base.eccreq.strength = 60;
+				requirements->strength = 60;
 				break;
 			default:
 				WARN(1, "Could not decode ECC info");
-				chip->base.eccreq.step_size = 0;
+				requirements->step_size = 0;
 			}
 		}
 	} else {
@@ -106,8 +107,8 @@  static void samsung_nand_decode_id(struct nand_chip *chip)
 			switch (chip->id.data[1]) {
 			/* K9F4G08U0D-S[I|C]B0(T00) */
 			case 0xDC:
-				chip->base.eccreq.step_size = 512;
-				chip->base.eccreq.strength = 1;
+				requirements->step_size = 512;
+				requirements->strength = 1;
 				break;
 
 			/* K9F1G08U0E 21nm chips do not support subpage write */
diff --git a/drivers/mtd/nand/raw/nand_toshiba.c b/drivers/mtd/nand/raw/nand_toshiba.c
index 13f9632f1cb4..f00d958d724f 100644
--- a/drivers/mtd/nand/raw/nand_toshiba.c
+++ b/drivers/mtd/nand/raw/nand_toshiba.c
@@ -100,6 +100,7 @@  static void toshiba_nand_benand_init(struct nand_chip *chip)
 
 static void toshiba_nand_decode_id(struct nand_chip *chip)
 {
+	struct nand_ecc_conf *requirements = &chip->base.ecc.requirements;
 	struct mtd_info *mtd = nand_to_mtd(chip);
 	struct nand_memory_organization *memorg;
 
@@ -130,20 +131,20 @@  static void toshiba_nand_decode_id(struct nand_chip *chip)
 	 *  - 24nm: 8 bit ECC for each 512Byte is required.
 	 */
 	if (chip->id.len >= 6 && nand_is_slc(chip)) {
-		chip->base.eccreq.step_size = 512;
+		requirements->step_size = 512;
 		switch (chip->id.data[5] & 0x7) {
 		case 0x4:
-			chip->base.eccreq.strength = 1;
+			requirements->strength = 1;
 			break;
 		case 0x5:
-			chip->base.eccreq.strength = 4;
+			requirements->strength = 4;
 			break;
 		case 0x6:
-			chip->base.eccreq.strength = 8;
+			requirements->strength = 8;
 			break;
 		default:
 			WARN(1, "Could not get ECC info");
-			chip->base.eccreq.step_size = 0;
+			requirements->step_size = 0;
 			break;
 		}
 	}
diff --git a/drivers/mtd/nand/raw/sunxi_nand.c b/drivers/mtd/nand/raw/sunxi_nand.c
index 69599854fdd6..43b22564da10 100644
--- a/drivers/mtd/nand/raw/sunxi_nand.c
+++ b/drivers/mtd/nand/raw/sunxi_nand.c
@@ -1807,6 +1807,7 @@  static void sunxi_nand_ecc_cleanup(struct nand_ecc_ctrl *ecc)
 
 static int sunxi_nand_attach_chip(struct nand_chip *nand)
 {
+	struct nand_ecc_conf *requirements = &nand->base.ecc.requirements;
 	struct mtd_info *mtd = nand_to_mtd(nand);
 	struct nand_ecc_ctrl *ecc = &nand->ecc;
 	struct device_node *np = nand_get_flash_node(nand);
@@ -1821,8 +1822,8 @@  static int sunxi_nand_attach_chip(struct nand_chip *nand)
 	nand->options |= NAND_SUBPAGE_READ;
 
 	if (!ecc->size) {
-		ecc->size = nand->base.eccreq.step_size;
-		ecc->strength = nand->base.eccreq.strength;
+		ecc->size = requirements->step_size;
+		ecc->strength = requirements->strength;
 	}
 
 	if (!ecc->size || !ecc->strength)
diff --git a/drivers/mtd/nand/raw/tegra_nand.c b/drivers/mtd/nand/raw/tegra_nand.c
index 3cc9a4c41443..31d547d96795 100644
--- a/drivers/mtd/nand/raw/tegra_nand.c
+++ b/drivers/mtd/nand/raw/tegra_nand.c
@@ -853,7 +853,7 @@  static int tegra_nand_get_strength(struct nand_chip *chip, const int *strength,
 		} else {
 			strength_sel = strength[i];
 
-			if (strength_sel < chip->base.eccreq.strength)
+			if (strength_sel < chip->base.ecc.requirements.strength)
 				continue;
 		}
 
@@ -906,6 +906,7 @@  static int tegra_nand_select_strength(struct nand_chip *chip, int oobsize)
 static int tegra_nand_attach_chip(struct nand_chip *chip)
 {
 	struct tegra_nand_controller *ctrl = to_tegra_ctrl(chip->controller);
+	struct nand_ecc_conf *requirements = &chip->base.ecc.requirements;
 	struct tegra_nand_chip *nand = to_tegra_chip(chip);
 	struct mtd_info *mtd = nand_to_mtd(chip);
 	int bits_per_step;
@@ -917,9 +918,9 @@  static int tegra_nand_attach_chip(struct nand_chip *chip)
 	chip->ecc.mode = NAND_ECC_HW;
 	chip->ecc.size = 512;
 	chip->ecc.steps = mtd->writesize / chip->ecc.size;
-	if (chip->base.eccreq.step_size != 512) {
+	if (requirements->step_size != 512) {
 		dev_err(ctrl->dev, "Unsupported step size %d\n",
-			chip->base.eccreq.step_size);
+			requirements->step_size);
 		return -EINVAL;
 	}
 
@@ -950,7 +951,7 @@  static int tegra_nand_attach_chip(struct nand_chip *chip)
 		if (ret < 0) {
 			dev_err(ctrl->dev,
 				"No valid strength found, minimum %d\n",
-				chip->base.eccreq.strength);
+				requirements->strength);
 			return ret;
 		}
 
diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
index ed5e340dff51..ab41b9434d87 100644
--- a/drivers/mtd/nand/spi/core.c
+++ b/drivers/mtd/nand/spi/core.c
@@ -480,7 +480,7 @@  static int spinand_check_ecc_status(struct spinand_device *spinand, u8 status)
 		 * fixed, so let's return the maximum possible value so that
 		 * wear-leveling layers move the data immediately.
 		 */
-		return nand->eccreq.strength;
+		return nand->ecc.ctx.conf.strength;
 
 	case STATUS_ECC_UNCOR_ERROR:
 		return -EBADMSG;
@@ -558,7 +558,7 @@  static int spinand_mtd_read(struct mtd_info *mtd, loff_t from,
 
 	mutex_lock(&spinand->lock);
 
-	nanddev_io_for_each_page(nand, from, ops, &iter) {
+	nanddev_io_for_each_page(nand, NAND_PAGE_READ, from, ops, &iter) {
 		ret = spinand_select_target(spinand, iter.req.pos.target);
 		if (ret)
 			break;
@@ -606,7 +606,7 @@  static int spinand_mtd_write(struct mtd_info *mtd, loff_t to,
 
 	mutex_lock(&spinand->lock);
 
-	nanddev_io_for_each_page(nand, to, ops, &iter) {
+	nanddev_io_for_each_page(nand, NAND_PAGE_WRITE, to, ops, &iter) {
 		ret = spinand_select_target(spinand, iter.req.pos.target);
 		if (ret)
 			break;
@@ -869,7 +869,7 @@  int spinand_match_and_init(struct spinand_device *spinand,
 			continue;
 
 		nand->memorg = table[i].memorg;
-		nand->eccreq = table[i].eccreq;
+		nand->ecc.requirements = table[i].eccreq;
 		spinand->eccinfo = table[i].eccinfo;
 		spinand->flags = table[i].flags;
 		spinand->select_target = table[i].select_target;
diff --git a/drivers/mtd/nand/spi/macronix.c b/drivers/mtd/nand/spi/macronix.c
index c6300d9d63f9..ef71312966a2 100644
--- a/drivers/mtd/nand/spi/macronix.c
+++ b/drivers/mtd/nand/spi/macronix.c
@@ -78,10 +78,10 @@  static int mx35lf1ge4ab_ecc_get_status(struct spinand_device *spinand,
 		 * data around if it's not necessary.
 		 */
 		if (mx35lf1ge4ab_get_eccsr(spinand, &eccsr))
-			return nand->eccreq.strength;
+			return nand->ecc.ctx.conf.strength;
 
-		if (WARN_ON(eccsr > nand->eccreq.strength || !eccsr))
-			return nand->eccreq.strength;
+		if (WARN_ON(eccsr > nand->ecc.ctx.conf.strength || !eccsr))
+			return nand->ecc.ctx.conf.strength;
 
 		return eccsr;
 
diff --git a/drivers/mtd/nand/spi/toshiba.c b/drivers/mtd/nand/spi/toshiba.c
index 00ddab08e6c6..4ac336d3a304 100644
--- a/drivers/mtd/nand/spi/toshiba.c
+++ b/drivers/mtd/nand/spi/toshiba.c
@@ -77,12 +77,12 @@  static int tc58cvg2s0h_ecc_get_status(struct spinand_device *spinand,
 		 * data around if it's not necessary.
 		 */
 		if (spi_mem_exec_op(spinand->spimem, &op))
-			return nand->eccreq.strength;
+			return nand->ecc.ctx.conf.strength;
 
 		mbf >>= 4;
 
-		if (WARN_ON(mbf > nand->eccreq.strength || !mbf))
-			return nand->eccreq.strength;
+		if (WARN_ON(mbf > nand->ecc.ctx.conf.strength || !mbf))
+			return nand->ecc.ctx.conf.strength;
 
 		return mbf;
 
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h
index 30f0fb02abe2..9e3b018d0b83 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -82,8 +82,14 @@  struct nand_pos {
 	unsigned int page;
 };
 
+enum nand_page_io_req_type {
+	NAND_PAGE_READ = 0,
+	NAND_PAGE_WRITE,
+};
+
 /**
  * struct nand_page_io_req - NAND I/O request object
+ * @type: the type of page I/O: read or write
  * @pos: the position this I/O request is targeting
  * @dataoffs: the offset within the page
  * @datalen: number of data bytes to read from/write to this page
@@ -99,6 +105,7 @@  struct nand_pos {
  * specific commands/operations.
  */
 struct nand_page_io_req {
+	enum nand_page_io_req_type type;
 	struct nand_pos pos;
 	unsigned int dataoffs;
 	unsigned int datalen;
@@ -116,13 +123,35 @@  struct nand_page_io_req {
 };
 
 /**
- * struct nand_ecc_req - NAND ECC requirements
+ * struct nand_ecc_conf - NAND ECC configuration
+ * @strength: ECC strength
+ * @step_size: Number of bytes per step
+ * @total: Total number of bytes used for storing ECC codes, this is used by
+ *         generic OOB layouts
+ */
+struct nand_ecc_conf {
+	unsigned int strength;
+	unsigned int step_size;
+	unsigned int total;
+};
+
+/**
+ * struct nand_ecc_user_conf - User desired ECC configuration
+ * @mode: ECC mode
+ * @algo: ECC algorithm
  * @strength: ECC strength
  * @step_size: ECC step/block size
+ * @maximize: ECC parameters must be maximized depending on the device
+ *            capabilities
+ * @flags: User flags
  */
-struct nand_ecc_req {
+struct nand_ecc_user_conf {
+	int mode;
+	unsigned int algo;
 	unsigned int strength;
 	unsigned int step_size;
+	unsigned int maximize;
+	unsigned int flags;
 };
 
 #define NAND_ECCREQ(str, stp) { .strength = (str), .step_size = (stp) }
@@ -157,11 +186,76 @@  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
+ * @priv: ECC engine driver private data
+ */
+struct nand_ecc_context {
+	struct nand_ecc_conf conf;
+	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,
+			      void *oobbuf);
+	int (*finish_io_req)(struct nand_device *nand,
+			     struct nand_page_io_req *req,
+			     void *oobbuf);
+};
+
+/**
+ * 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, void *oobbuf);
+int nand_ecc_finish_io_req(struct nand_device *nand,
+			   struct nand_page_io_req *req, void *oobbuf);
+
+/**
+ * struct nand_ecc - High-level ECC object
+ *
+ * @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
+ *       and @user_conf
+ * @engine: ECC engine
+ */
+struct nand_ecc {
+	struct nand_ecc_conf requirements;
+	struct nand_ecc_user_conf user_conf;
+	struct nand_ecc_context ctx;
+	struct nand_ecc_engine *engine;
+};
+
 /**
  * struct nand_device - NAND device
  * @mtd: MTD instance attached to the NAND device
  * @memorg: memory layout
- * @eccreq: ECC requirements
  * @rowconv: position to row address converter
  * @bbt: bad block table info
  * @ops: NAND operations attached to the NAND device
@@ -169,8 +263,8 @@  struct nand_ops {
  * Generic NAND object. Specialized NAND layers (raw NAND, SPI NAND, OneNAND)
  * should declare their own NAND object embedding a nand_device struct (that's
  * how inheritance is done).
- * struct_nand_device->memorg and struct_nand_device->eccreq should be filled
- * at device detection time to reflect the NAND device
+ * struct_nand_device->memorg and struct_nand_device->ecc.ctx.conf should
+ * be filled at device detection time to reflect the NAND device
  * capabilities/requirements. Once this is done nanddev_init() can be called.
  * It will take care of converting NAND information into MTD ones, which means
  * the specialized NAND layers should never manually tweak
@@ -179,7 +273,7 @@  struct nand_ops {
 struct nand_device {
 	struct mtd_info mtd;
 	struct nand_memory_organization memorg;
-	struct nand_ecc_req eccreq;
+	struct nand_ecc ecc;
 	struct nand_row_converter rowconv;
 	struct nand_bbt bbt;
 	const struct nand_ops *ops;
@@ -624,11 +718,13 @@  static inline void nanddev_pos_next_page(struct nand_device *nand,
  * layer.
  */
 static inline void nanddev_io_iter_init(struct nand_device *nand,
+					enum nand_page_io_req_type reqtype,
 					loff_t offs, struct mtd_oob_ops *req,
 					struct nand_io_iter *iter)
 {
 	struct mtd_info *mtd = nanddev_to_mtd(nand);
 
+	iter->req.type = reqtype;
 	iter->req.mode = req->mode;
 	iter->req.dataoffs = nanddev_offs_to_pos(nand, offs, &iter->req.pos);
 	iter->req.ooboffs = req->ooboffs;
@@ -698,8 +794,8 @@  static inline bool nanddev_io_iter_end(struct nand_device *nand,
  *
  * Should be used for iterate over pages that are contained in an MTD request.
  */
-#define nanddev_io_for_each_page(nand, start, req, iter)		\
-	for (nanddev_io_iter_init(nand, start, req, iter);		\
+#define nanddev_io_for_each_page(nand, type, start, req, iter)		\
+	for (nanddev_io_iter_init(nand, type, start, req, iter);	\
 	     !nanddev_io_iter_end(nand, iter);				\
 	     nanddev_io_iter_next_page(nand, iter))
 
diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h
index b92e2aa955b6..75a28dc79a9c 100644
--- a/include/linux/mtd/spinand.h
+++ b/include/linux/mtd/spinand.h
@@ -168,7 +168,7 @@  struct spinand_id {
  *	    match, 0 if the manufacturer ID does not match and a negative
  *	    error code otherwise. When true is returned, the core assumes
  *	    that properties of the NAND chip (spinand->base.memorg and
- *	    spinand->base.eccreq) have been filled
+ *	    spinand->base.ecc.ctx.conf) have been filled
  * @init: initialize a SPI NAND device
  * @cleanup: cleanup a SPI NAND device
  *
@@ -263,7 +263,7 @@  struct spinand_info {
 	u8 devid;
 	u32 flags;
 	struct nand_memory_organization memorg;
-	struct nand_ecc_req eccreq;
+	struct nand_ecc_conf eccreq;
 	struct spinand_ecc_info eccinfo;
 	struct {
 		const struct spinand_op_variants *read_cache;