diff mbox series

[v2,10/36] mtd: nand: Introduce the ECC engine abstraction

Message ID 20190304222841.13899-11-miquel.raynal@bootlin.com (mailing list archive)
State New, archived
Headers show
Series Introduce the generic ECC engine abstraction | expand

Commit Message

Miquel Raynal March 4, 2019, 10:28 p.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                 |   6 +
 drivers/mtd/nand/ecc/Makefile                |   3 +
 drivers/mtd/nand/ecc/engine.c                | 138 +++++++++++++++++++
 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                  |   4 +-
 drivers/mtd/nand/spi/macronix.c              |   6 +-
 drivers/mtd/nand/spi/toshiba.c               |   6 +-
 include/linux/mtd/nand.h                     |  82 ++++++++++-
 include/linux/mtd/spinand.h                  |   2 +-
 24 files changed, 327 insertions(+), 93 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 March 31, 2019, 12:10 p.m. UTC | #1
On Mon,  4 Mar 2019 23:28:15 +0100
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> 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                 |   6 +
>  drivers/mtd/nand/ecc/Makefile                |   3 +
>  drivers/mtd/nand/ecc/engine.c                | 138 +++++++++++++++++++
>  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                  |   4 +-
>  drivers/mtd/nand/spi/macronix.c              |   6 +-
>  drivers/mtd/nand/spi/toshiba.c               |   6 +-

Can we please split that in 3 patches:

1/ introduce the ECC framework
2/ convert spi nand
3/ convert raw nand

>  include/linux/mtd/nand.h                     |  82 ++++++++++-
>  include/linux/mtd/spinand.h                  |   2 +-
>  24 files changed, 327 insertions(+), 93 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
> 
> diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
> index d2ef8b89568e..75d0bd18b818 100644
> --- a/drivers/mtd/nand/Kconfig
> +++ b/drivers/mtd/nand/Kconfig
> @@ -6,5 +6,6 @@ config 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"
>  
>  endmenu
> 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..01439f66ecbf
> --- /dev/null
> +++ b/drivers/mtd/nand/ecc/Kconfig
> @@ -0,0 +1,6 @@
> +menu "ECC engine support"
> +
> +config MTD_NAND_ECC
> +	tristate
> +

There's already an MTD_NAND_ECC symbol defined in
drivers/mtd/nand/raw/Kconfig, plus I don't think we want ecc/engine.c
to be compiled as a module, but instead be embedded in nand-core.ko.
Not to mention that the name, engine.ko, is probably too generic.

> +endmenu
> diff --git a/drivers/mtd/nand/ecc/Makefile b/drivers/mtd/nand/ecc/Makefile
> new file mode 100644
> index 000000000000..c7367f834d81
> --- /dev/null
> +++ b/drivers/mtd/nand/ecc/Makefile
> @@ -0,0 +1,3 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +obj-$(CONFIG_MTD_NAND_ECC)		+= engine.o
Miquel Raynal May 3, 2019, 2:34 p.m. UTC | #2
Hi Boris,

Boris Brezillon <boris.brezillon@collabora.com> wrote on Sun, 31 Mar
2019 14:10:25 +0200:

> On Mon,  4 Mar 2019 23:28:15 +0100
> Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> 
> > 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                 |   6 +
> >  drivers/mtd/nand/ecc/Makefile                |   3 +
> >  drivers/mtd/nand/ecc/engine.c                | 138 +++++++++++++++++++
> >  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                  |   4 +-
> >  drivers/mtd/nand/spi/macronix.c              |   6 +-
> >  drivers/mtd/nand/spi/toshiba.c               |   6 +-  
> 
> Can we please split that in 3 patches:
> 
> 1/ introduce the ECC framework
> 2/ convert spi nand
> 3/ convert raw nand

Split in 2 patches:
1/ Introduce the ECC framework
2/ Change the eccreq parameter of the nand_device structure which
impacts both raw and SPI NAND frameworks.

> 
> >  include/linux/mtd/nand.h                     |  82 ++++++++++-
> >  include/linux/mtd/spinand.h                  |   2 +-
> >  24 files changed, 327 insertions(+), 93 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
> > 
> > diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
> > index d2ef8b89568e..75d0bd18b818 100644
> > --- a/drivers/mtd/nand/Kconfig
> > +++ b/drivers/mtd/nand/Kconfig
> > @@ -6,5 +6,6 @@ config 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"
> >  
> >  endmenu
> > 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..01439f66ecbf
> > --- /dev/null
> > +++ b/drivers/mtd/nand/ecc/Kconfig
> > @@ -0,0 +1,6 @@
> > +menu "ECC engine support"
> > +
> > +config MTD_NAND_ECC
> > +	tristate
> > +  
> 
> There's already an MTD_NAND_ECC symbol defined in
> drivers/mtd/nand/raw/Kconfig,

Didn't find any?

> plus I don't think we want ecc/engine.c
> to be compiled as a module, but instead be embedded in nand-core.ko.

How would you do that? I don't find the right way to embed
nand_ecc_engine.o directly in nandcore.o. The only solution I found was
to add nandcore-$(<cond>) += ecc/thing.o directly in
drivers/mtd/nand/Makefile but I don't think it is acceptable?

> Not to mention that the name, engine.ko, is probably too generic.

Renamed it nand_ecc_engine.ko for now.

Thanks,
Miquèl
Boris Brezillon May 3, 2019, 3:54 p.m. UTC | #3
On Fri, 3 May 2019 16:34:00 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Hi Boris,
> 
> Boris Brezillon <boris.brezillon@collabora.com> wrote on Sun, 31 Mar
> 2019 14:10:25 +0200:
> 
> > On Mon,  4 Mar 2019 23:28:15 +0100
> > Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >   
> > > 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                 |   6 +
> > >  drivers/mtd/nand/ecc/Makefile                |   3 +
> > >  drivers/mtd/nand/ecc/engine.c                | 138 +++++++++++++++++++
> > >  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                  |   4 +-
> > >  drivers/mtd/nand/spi/macronix.c              |   6 +-
> > >  drivers/mtd/nand/spi/toshiba.c               |   6 +-    
> > 
> > Can we please split that in 3 patches:
> > 
> > 1/ introduce the ECC framework
> > 2/ convert spi nand
> > 3/ convert raw nand  
> 
> Split in 2 patches:
> 1/ Introduce the ECC framework
> 2/ Change the eccreq parameter of the nand_device structure which
> impacts both raw and SPI NAND frameworks.
> 
> >   
> > >  include/linux/mtd/nand.h                     |  82 ++++++++++-
> > >  include/linux/mtd/spinand.h                  |   2 +-
> > >  24 files changed, 327 insertions(+), 93 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
> > > 
> > > diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
> > > index d2ef8b89568e..75d0bd18b818 100644
> > > --- a/drivers/mtd/nand/Kconfig
> > > +++ b/drivers/mtd/nand/Kconfig
> > > @@ -6,5 +6,6 @@ config 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"
> > >  
> > >  endmenu
> > > 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..01439f66ecbf
> > > --- /dev/null
> > > +++ b/drivers/mtd/nand/ecc/Kconfig
> > > @@ -0,0 +1,6 @@
> > > +menu "ECC engine support"
> > > +
> > > +config MTD_NAND_ECC
> > > +	tristate
> > > +    
> > 
> > There's already an MTD_NAND_ECC symbol defined in
> > drivers/mtd/nand/raw/Kconfig,  
> 
> Didn't find any?

There was one in drivers/mtd/nand/raw/Kconfig [1], but maybe it's gone
now.

> 
> > plus I don't think we want ecc/engine.c
> > to be compiled as a module, but instead be embedded in nand-core.ko.  
> 
> How would you do that? I don't find the right way to embed
> nand_ecc_engine.o directly in nandcore.o. The only solution I found was
> to add nandcore-$(<cond>) += ecc/thing.o directly in
> drivers/mtd/nand/Makefile but I don't think it is acceptable?

Or just move the core logic into drivers/mtd/nand/ecc.c and add

nandcore-$(<cond>) += ecc.o

to the Makefile.


> 
> > Not to mention that the name, engine.ko, is probably too generic.  
> 
> Renamed it nand_ecc_engine.ko for now.

Still think it's better to have the code embedded in nandcore.ko.

[1]https://elixir.bootlin.com/linux/v5.1-rc7/source/drivers/mtd/nand/raw/Kconfig#L1
Miquel Raynal May 6, 2019, 3:49 p.m. UTC | #4
Hi Boris,

Boris Brezillon <boris.brezillon@collabora.com> wrote on Fri, 3 May
2019 17:54:46 +0200:

> On Fri, 3 May 2019 16:34:00 +0200
> Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> 
> > Hi Boris,
> > 
> > Boris Brezillon <boris.brezillon@collabora.com> wrote on Sun, 31 Mar
> > 2019 14:10:25 +0200:
> >   
> > > On Mon,  4 Mar 2019 23:28:15 +0100
> > > Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > >     
> > > > 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                 |   6 +
> > > >  drivers/mtd/nand/ecc/Makefile                |   3 +
> > > >  drivers/mtd/nand/ecc/engine.c                | 138 +++++++++++++++++++
> > > >  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                  |   4 +-
> > > >  drivers/mtd/nand/spi/macronix.c              |   6 +-
> > > >  drivers/mtd/nand/spi/toshiba.c               |   6 +-      
> > > 
> > > Can we please split that in 3 patches:
> > > 
> > > 1/ introduce the ECC framework
> > > 2/ convert spi nand
> > > 3/ convert raw nand    
> > 
> > Split in 2 patches:
> > 1/ Introduce the ECC framework
> > 2/ Change the eccreq parameter of the nand_device structure which
> > impacts both raw and SPI NAND frameworks.
> >   
> > >     
> > > >  include/linux/mtd/nand.h                     |  82 ++++++++++-
> > > >  include/linux/mtd/spinand.h                  |   2 +-
> > > >  24 files changed, 327 insertions(+), 93 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
> > > > 
> > > > diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
> > > > index d2ef8b89568e..75d0bd18b818 100644
> > > > --- a/drivers/mtd/nand/Kconfig
> > > > +++ b/drivers/mtd/nand/Kconfig
> > > > @@ -6,5 +6,6 @@ config 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"
> > > >  
> > > >  endmenu
> > > > 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..01439f66ecbf
> > > > --- /dev/null
> > > > +++ b/drivers/mtd/nand/ecc/Kconfig
> > > > @@ -0,0 +1,6 @@
> > > > +menu "ECC engine support"
> > > > +
> > > > +config MTD_NAND_ECC
> > > > +	tristate
> > > > +      
> > > 
> > > There's already an MTD_NAND_ECC symbol defined in
> > > drivers/mtd/nand/raw/Kconfig,    
> > 
> > Didn't find any?  
> 
> There was one in drivers/mtd/nand/raw/Kconfig [1], but maybe it's gone
> now.
> 
> >   
> > > plus I don't think we want ecc/engine.c
> > > to be compiled as a module, but instead be embedded in nand-core.ko.    
> > 
> > How would you do that? I don't find the right way to embed
> > nand_ecc_engine.o directly in nandcore.o. The only solution I found was
> > to add nandcore-$(<cond>) += ecc/thing.o directly in
> > drivers/mtd/nand/Makefile but I don't think it is acceptable?  
> 
> Or just move the core logic into drivers/mtd/nand/ecc.c and add
> 
> nandcore-$(<cond>) += ecc.o
> 
> to the Makefile.
> 
> 
> >   
> > > Not to mention that the name, engine.ko, is probably too generic.    
> > 
> > Renamed it nand_ecc_engine.ko for now.  
> 
> Still think it's better to have the code embedded in nandcore.ko.
> 
> [1]https://elixir.bootlin.com/linux/v5.1-rc7/source/drivers/mtd/nand/raw/Kconfig#L1

What is there currently was not what I was looking for.

But nevermind, I moved all code to the nand/ directory, dropping the
ecc/ subdir. Now ECC code is called ecc-engine.o and is embedded in
nandcore.ko.


Thanks,
Miquèl
diff mbox series

Patch

diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index d2ef8b89568e..75d0bd18b818 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -6,5 +6,6 @@  config 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"
 
 endmenu
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..01439f66ecbf
--- /dev/null
+++ b/drivers/mtd/nand/ecc/Kconfig
@@ -0,0 +1,6 @@ 
+menu "ECC engine support"
+
+config MTD_NAND_ECC
+	tristate
+
+endmenu
diff --git a/drivers/mtd/nand/ecc/Makefile b/drivers/mtd/nand/ecc/Makefile
new file mode 100644
index 000000000000..c7367f834d81
--- /dev/null
+++ b/drivers/mtd/nand/ecc/Makefile
@@ -0,0 +1,3 @@ 
+# SPDX-License-Identifier: GPL-2.0
+
+obj-$(CONFIG_MTD_NAND_ECC)		+= engine.o
diff --git a/drivers/mtd/nand/ecc/engine.c b/drivers/mtd/nand/ecc/engine.c
new file mode 100644
index 000000000000..4c7943ddf2cc
--- /dev/null
+++ b/drivers/mtd/nand/ecc/engine.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, void *oobbuf)
+{
+	if (!nand->ecc.engine->ops->prepare_io_req)
+		return 0;
+
+	return nand->ecc.engine->ops->prepare_io_req(nand, req, oobbuf);
+}
+EXPORT_SYMBOL(nand_ecc_prepare_io_req);
+
+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);
+}
+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/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 f4e85e56a041..b12ca291c7e3 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 9ee192585854..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;
@@ -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 78cf905083c9..45caaa489085 100644
--- a/include/linux/mtd/nand.h
+++ b/include/linux/mtd/nand.h
@@ -124,12 +124,18 @@  struct nand_page_io_req {
 
 /**
  * struct nand_ecc_conf - NAND ECC configuration
+ * @provider: ECC engine provider type
+ * @algo: ECC algorithm (if relevant)
  * @strength: ECC strength
  * @step_size: Number of bytes per step
+ * @flags: Misc properties
  */
 struct nand_ecc_conf {
+	unsigned int provider;
+	unsigned int algo;
 	unsigned int strength;
 	unsigned int step_size;
+	unsigned int flags;
 };
 
 #define NAND_ECCREQ(str, stp) { .strength = (str), .step_size = (stp) }
@@ -164,11 +170,79 @@  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_conf 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,
+			      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
+ * @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_conf defaults;
+	struct nand_ecc_conf requirements;
+	struct nand_ecc_conf 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
  * @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
@@ -176,8 +250,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
@@ -186,7 +260,7 @@  struct nand_ops {
 struct nand_device {
 	struct mtd_info mtd;
 	struct nand_memory_organization memorg;
-	struct nand_ecc_conf eccreq;
+	struct nand_ecc ecc;
 	struct nand_row_converter rowconv;
 	struct nand_bbt bbt;
 	const struct nand_ops *ops;
diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h
index 3008de54f958..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
  *