Re: [PATCH 3/3] spi: spi-mem: MediaTek: Add SPI NAND Flash interface driver for MediaTek MT7622

Re: [PATCH 3/3] spi: spi-mem: MediaTek: Add SPI NAND Flash interface driver for MediaTek MT7622

[Boris Brezillon]

+Mark (the SPI maintainer, please remember to Cc him next time you
send a SPI related patch).

Hi Xiangsheng,

On Wed, 12 Sep 2018 09:43:32 +0800
Xiangsheng Hou <xiangsheng.hou-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:

Please add a commit message here.

> Signed-off-by: Xiangsheng Hou <xiangsheng.hou-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> ---
>  drivers/spi/Kconfig        |   10 +
>  drivers/spi/Makefile       |    1 +
>  drivers/spi/spi-mtk-snfi.c |  918 ++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 929 insertions(+)
>  create mode 100644 drivers/spi/spi-mtk-snfi.c
> 
> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
> index 671d078..9f94921 100644
> --- a/drivers/spi/Kconfig
> +++ b/drivers/spi/Kconfig
> @@ -389,6 +389,16 @@ config SPI_MT65XX
>  	  say Y or M here.If you are not sure, say N.
>  	  SPI drivers for Mediatek MT65XX and MT81XX series ARM SoCs.
>  
> +config SPI_MTK_SNFI
> +	tristate "MediaTek SPI NAND interface"
> +	select MTD_SPI_NAND
> +	help
> +	  This selects the MediaTek(R) SPI NAND FLASH interface (SNFI)
> +	  driver, which could be found on MT7622 Soc, say Y or M here.
> +	  If you are not sure, say N. This driver use SPI NAND FLASH
> +          on-die ECC.

Should be aligned on the previous line.

> +	  Note Parallel Nand and SPI NAND is alternative on MediaTek SoCs.

There's a fundamental issue with this driver: spi-mem was designed to be
memory agnostic, and you seem to have a SPI controller that supports
only SPI NANDs. Is that correct, or is it just that you developed the
driver with SPI NANDs in  mind?

> +
>  config SPI_NUC900
>  	tristate "Nuvoton NUC900 series SPI"
>  	depends on ARCH_W90X900

> +static int mtk_snfc_read(struct spi_mem *mem,
> +			 const struct spi_mem_op *op)
> +{
> +	struct mtk_snfc *snfc = spi_controller_get_devdata(mem->spi->master);
> +	struct spinand_device *spinand = spi_mem_get_drvdata(mem);

Just one example of things I don't want to see in spi-mem drivers.
Clearly you're breaking the layering we're trying to enforce:

		       MTD
		------------------
		spi-nor | spinand
		------------------
		     spi-mem
		------------------
		       spi

spi-mem should no know nothing about the memory it's manipulating, all
it should do is execute SPI memory operations.

Don't know what's possible to do with your controller, and maybe it's
not able to execute random SPI memory operations, but in this case we
should consider a different solution to support this driver.

Do you have a public datasheet I can look at?

Regards,

Boris

> +	struct mtd_info *mtd = spinand_to_mtd(spinand);
> +	u32 sectors = mtd->writesize / snfc->caps->nand_sec_size;
> +	u32 reg, len, col_addr = 0;
> +	dma_addr_t dma_addr;
> +	int dummy_cycle, ret;
> +
> +	len = sectors * (snfc->caps->nand_sec_size +
> +			  snfc->spare_per_sector);
> +	dma_addr = dma_map_single(snfc->dev, snfc->buffer,
> +				  len, DMA_FROM_DEVICE);
> +	ret = dma_mapping_error(snfc->dev, dma_addr);
> +	if (ret) {
> +		dev_err(snfc->dev, "dma mapping error\n");
> +		return -EINVAL;
> +	}
> +
> +	/* set Read cache command and dummy cycle */
> +	dummy_cycle = (op->dummy.nbytes << 3) >> (ffs(op->dummy.buswidth) - 1);
> +	reg |= ((op->cmd.opcode & RD_CMD_MASK) |
> +	       (dummy_cycle << RD_DUMMY_SHIFT));
> +	writel(reg, snfc->regs + SNFI_RD_CTL2);
> +
> +	writel((col_addr & RD_ADDR_MASK), snfc->regs + SNFI_RD_CTL3);
> +
> +	reg = readl_relaxed(snfc->regs + SNFI_MISC_CTL);
> +	reg |= RD_CUSTOM_EN;
> +	reg &= ~(RD_MODE_MASK | WR_X4_EN);
> +
> +	/* set data and addr buswidth */
> +	if (op->data.buswidth == 4)
> +		reg |= RD_MODE_X4;
> +	else if (op->data.buswidth == 2)
> +		reg |= RD_MODE_X2;
> +
> +	if (op->addr.buswidth == 4 || op->addr.buswidth == 2)
> +		reg |= RD_QDUAL_IO;
> +	writel(reg, snfc->regs + SNFI_MISC_CTL);
> +
> +	writel(len, snfc->regs + SNFI_MISC_CTL2);
> +	writew((sectors << CON_SEC_SHIFT), snfc->regs + NFI_CON);
> +
> +	reg = CNFG_READ_EN | CNFG_DMA_BURST_EN | CNFG_DMA | CNFG_OP_CUST;
> +	writew(reg, snfc->regs + NFI_CNFG);
> +
> +	writel(lower_32_bits(dma_addr), snfc->regs + NFI_STRADDR);
> +	readw_relaxed(snfc->regs + NFI_INTR_STA);
> +	writew(INTR_AHB_DONE_EN, snfc->regs + NFI_INTR_EN);
> +
> +	init_completion(&snfc->done);
> +
> +	/* set dummy command to trigger NFI enter custom mode */
> +	writew(NAND_CMD_DUMMYREAD, snfc->regs + NFI_CMD);
> +	reg = readl_relaxed(snfc->regs + NFI_CON) | CON_BRD;
> +	writew(reg, snfc->regs + NFI_CON);
> +
> +	ret = wait_for_completion_timeout(&snfc->done, msecs_to_jiffies(500));
> +	if (!ret) {
> +		dev_err(snfc->dev, "read ahb done timeout\n");
> +		writew(0, snfc->regs + NFI_INTR_EN);
> +		ret = -ETIMEDOUT;
> +		goto out;
> +	}
> +
> +	ret = readl_poll_timeout_atomic(snfc->regs + NFI_BYTELEN, reg,
> +					ADDRCNTR_SEC(reg) >= sectors, 10,
> +					MTK_TIMEOUT);
> +	if (ret < 0) {
> +		dev_err(snfc->dev, "polling read byte len timeout\n");
> +		ret = -EIO;
> +	}
> +
> +out:
> +	dma_unmap_single(snfc->dev, dma_addr, len, DMA_FROM_DEVICE);
> +	reg = readl_relaxed(snfc->regs + SNFI_MISC_CTL);
> +	reg &= ~RD_CUSTOM_EN;
> +	writel(reg, snfc->regs + SNFI_MISC_CTL);
> +
> +	return ret;
> +}

Re: [PATCH 3/3] spi: spi-mem: MediaTek: Add SPI NAND Flash interface driver for MediaTek MT7622

[Boris Brezillon]

On Fri, 26 Oct 2018 13:46:36 +0800
Xiangsheng Hou <xiangsheng.hou-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:

> Hi Boris,
> 
> On Tue, 2018-10-23 at 07:52 +0200, Boris Brezillon wrote:
> > +Mark (the SPI maintainer, please remember to Cc him next time you
> > send a SPI related patch).
> > 
> > Hi Xiangsheng,
> > 
> > On Wed, 12 Sep 2018 09:43:32 +0800
> > Xiangsheng Hou <xiangsheng.hou-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> > 
> > Please add a commit message here.
> >   
> > > Signed-off-by: Xiangsheng Hou <xiangsheng.hou-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> > > ---
> > >  drivers/spi/Kconfig        |   10 +
> > >  drivers/spi/Makefile       |    1 +
> > >  drivers/spi/spi-mtk-snfi.c |  918 ++++++++++++++++++++++++++++++++++++++++++++
> > >  3 files changed, 929 insertions(+)
> > >  create mode 100644 drivers/spi/spi-mtk-snfi.c
> > > 
> > > diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
> > > index 671d078..9f94921 100644
> > > --- a/drivers/spi/Kconfig
> > > +++ b/drivers/spi/Kconfig
> > > @@ -389,6 +389,16 @@ config SPI_MT65XX
> > >  	  say Y or M here.If you are not sure, say N.
> > >  	  SPI drivers for Mediatek MT65XX and MT81XX series ARM SoCs.
> > >  
> > > +config SPI_MTK_SNFI
> > > +	tristate "MediaTek SPI NAND interface"
> > > +	select MTD_SPI_NAND
> > > +	help
> > > +	  This selects the MediaTek(R) SPI NAND FLASH interface (SNFI)
> > > +	  driver, which could be found on MT7622 Soc, say Y or M here.
> > > +	  If you are not sure, say N. This driver use SPI NAND FLASH
> > > +          on-die ECC.  
> > 
> > Should be aligned on the previous line.
> >   
> > > +	  Note Parallel Nand and SPI NAND is alternative on MediaTek SoCs.  
> > 
> > There's a fundamental issue with this driver: spi-mem was designed to be
> > memory agnostic, and you seem to have a SPI controller that supports
> > only SPI NANDs. Is that correct, or is it just that you developed the
> > driver with SPI NANDs in  mind?
> >   
> Yes, this driver supports only SPI NANDs.
> Actually, Mediatek's controller is designed for NAND specifically, which
> can support SPI NANDs and PARALLEL NANDs,and this driver is just for SPI
> NANDs.

Hm, I'm not so sure about that (I might be wrong though), it seems you
can send any command you want, not only SPI NAND related ones. Maybe the
controller is optimized for SPI NANDs but can do all kind of SPI
transfers.

> 
> > > +
> > >  config SPI_NUC900
> > >  	tristate "Nuvoton NUC900 series SPI"
> > >  	depends on ARCH_W90X900  
> >   
> > > +static int mtk_snfc_read(struct spi_mem *mem,
> > > +			 const struct spi_mem_op *op)
> > > +{
> > > +	struct mtk_snfc *snfc = spi_controller_get_devdata(mem->spi->master);
> > > +	struct spinand_device *spinand = spi_mem_get_drvdata(mem);  
> > 
> > Just one example of things I don't want to see in spi-mem drivers.
> > Clearly you're breaking the layering we're trying to enforce:
> > 
> > 		       MTD
> > 		------------------
> > 		spi-nor | spinand
> > 		------------------
> > 		     spi-mem
> > 		------------------
> > 		       spi
> > 
> > spi-mem should no know nothing about the memory it's manipulating, all
> > it should do is execute SPI memory operations.
> > 
> > Don't know what's possible to do with your controller, and maybe it's
> > not able to execute random SPI memory operations, but in this case we
> > should consider a different solution to support this driver.
> > 
> > Do you have a public datasheet I can look at?
> >   
> We do not have a public datasheet for Mediatek controller currently.

Unfortunately, there's not much I can do without a clear understanding
of how the controller works.

Re: [PATCH 3/3] spi: spi-mem: MediaTek: Add SPI NAND Flash interface driver for MediaTek MT7622

[Chuanhong Guo]

Hi Boris!

I'm about to pick this driver up and start upstream it in the future.
So I'm answering
your questions below and would like to get your further suggestions.

On Fri, Oct 26, 2018 at 2:09 PM Boris Brezillon
<boris.brezillon-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org> wrote:
> > > There's a fundamental issue with this driver: spi-mem was designed to be
> > > memory agnostic, and you seem to have a SPI controller that supports
> > > only SPI NANDs. Is that correct, or is it just that you developed the
> > > driver with SPI NANDs in  mind?
> > >
> > Yes, this driver supports only SPI NANDs.
> > Actually, Mediatek's controller is designed for NAND specifically, which
> > can support SPI NANDs and PARALLEL NANDs,and this driver is just for SPI
> > NANDs.
>
> Hm, I'm not so sure about that (I might be wrong though), it seems you
> can send any command you want, not only SPI NAND related ones. Maybe the
> controller is optimized for SPI NANDs but can do all kind of SPI
> transfers.

You are correct here. This controller can perform generic spi-mem operations,
and it has special routines for page cache R/W that utilize controller's ECC
functionality. I think the purpose of this is to provide better ECC capability
for some SPI NANDs with worse ECC algorithm on chip.

> > > Don't know what's possible to do with your controller, and maybe it's
> > > not able to execute random SPI memory operations, but in this case we
> > > should consider a different solution to support this driver.
> > >
> > > Do you have a public datasheet I can look at?
> > >
> > We do not have a public datasheet for Mediatek controller currently.
>
> Unfortunately, there's not much I can do without a clear understanding
> of how the controller works.
>

I found a public datasheet [0] on wiki page for Banana Pi R64 [1].
Register description is available under "NAND flash interface" section
starting at page 592.
There's a hackier version of this driver in OpenWrt [2] which checks
opcode and use controller routine for page cache R/W.

ECC part of this controller can also be used as a standalone ECC
algorithm and perform ECC operations on data provided by CPU.
But I think if it's implement this way, we wasted the page cache
R/W routines in this controller.

I have two other initial thoughts:
1. abstract some kind of ECC functionality in spi-mem interface
    I haven't really learned ECC stuff so I don't know whether this is
    possible and what kind of argument we needs for it.
2. modify SPI-NAND core to add support for special SPI-NAND controller.
    This limits controller's ability and adds extra burden for future extention
    of SPI-NAND framework.

Which way would you prefer and do you have other suggestions?

[0] https://drive.google.com/file/d/1cW8KQmmVpwDGmBd48KNQes9CRn7FEgBb/view?usp=sharing
[1] http://wiki.banana-pi.org/Banana_Pi_BPI-R64#Documents
[2] https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=target/linux/mediatek/patches-4.19/0306-spi-spi-mem-MediaTek-Add-SPI-NAND-Flash-interface-dr.patch;h=2370925372f69aed0566339a4808056580e88837;hb=HEAD
-- 
Regards,
Chuanhong Guo

Re: [PATCH 3/3] spi: spi-mem: MediaTek: Add SPI NAND Flash interface driver for MediaTek MT7622

[Chuanhong Guo]

Hi!

On Wed, Mar 11, 2020 at 4:18 PM Boris Brezillon
<boris.brezillon-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org> wrote:
>
> +Miquel who worked on the ECC engine abstraction [3] recently.
>
> Hello Chuanhong,
>
> On Wed, 11 Mar 2020 15:35:43 +0800
> Chuanhong Guo <gch981213-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
> > Hi Boris!
> >
> > [resend to you because of the wrong mail address in previous one.]
> >
> > I'm about to pick this driver up and start upstream it in the future.
> > So I'm answering
> > your questions below and would like to get your further suggestions.
> >
> > On Fri, Oct 26, 2018 at 2:09 PM Boris Brezillon
> > <boris.brezillon-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org> wrote:
> > > > > There's a fundamental issue with this driver: spi-mem was designed to be
> > > > > memory agnostic, and you seem to have a SPI controller that supports
> > > > > only SPI NANDs. Is that correct, or is it just that you developed the
> > > > > driver with SPI NANDs in  mind?
> > > > >
> > > > Yes, this driver supports only SPI NANDs.
> > > > Actually, Mediatek's controller is designed for NAND specifically, which
> > > > can support SPI NANDs and PARALLEL NANDs,and this driver is just for SPI
> > > > NANDs.
> > >
> > > Hm, I'm not so sure about that (I might be wrong though), it seems you
> > > can send any command you want, not only SPI NAND related ones. Maybe the
> > > controller is optimized for SPI NANDs but can do all kind of SPI
> > > transfers.
> >
> > You are correct here. This controller can perform generic spi-mem operations,
> > and it has special routines for page cache R/W that utilize controller's ECC
> > functionality.
>
> Sounds similar to the way the MXIC controller works, and that's
> actually what Miquel is trying to support with his ECC engine
> abstraction series [3].
>
> > I think the purpose of this is to provide better ECC capability
> > for some SPI NANDs with worse ECC algorithm on chip.
>
> Yep, or make it faster. Actually the reason doesn't matter, I think
> we all agree that we'll have to support external ECC for SPI NANDs at
> some point, hence the work Miquel has been doing.
>
> >
> > > > > Don't know what's possible to do with your controller, and maybe it's
> > > > > not able to execute random SPI memory operations, but in this case we
> > > > > should consider a different solution to support this driver.
> > > > >
> > > > > Do you have a public datasheet I can look at?
> > > > >
> > > > We do not have a public datasheet for Mediatek controller currently.
> > >
> > > Unfortunately, there's not much I can do without a clear understanding
> > > of how the controller works.
> > >
> >
> > I found a public datasheet [0] on wiki page for Banana Pi R64 [1].
> > Register description is available under "NAND flash interface" section
> > starting at page 592.
> > There's a hackier version of this driver in OpenWrt [2] which checks
> > opcode and use controller routine for page cache R/W.
> >
> > ECC part of this controller can also be used as a standalone ECC
> > algorithm and perform ECC operations on data provided by CPU.

The solution I'm referring to here is:
1. read uncorrected data to host directly from SPI NAND
2. start an ECC correction separately

> > But I think if it's implement this way, we wasted the page cache
> > R/W routines in this controller.
>
> Oh, you probably don't want the page cache to be active anyway. When
> the framework reads a NAND page, it also checks the number of ECC
> errors, if the page is held in some internal cache, you won't see
> the evolution of this number. Note that the FS should do some caching,
> so caching things at the HW level is probably useless.

It doesn't cache anything in controller. The R/W routine I refer to is:
When we issue a request to read page cache on SPI NAND:
1. host programs a DMA-able memory area for receiving data.
2. controller reads the page cache from SPI NAND
3. controller get the data and start ECC correction
4. corrected data will be sent back to host via DMA
5. host could check ECC status

writing of page cache goes similarly.
There's no need for a separated ECC request comparing to previous
one.

>
> >
> > I have two other initial thoughts:
> > 1. abstract some kind of ECC functionality in spi-mem interface
> >     I haven't really learned ECC stuff so I don't know whether this is
> >     possible and what kind of argument we needs for it.
>
> Nope, spi-mem should stay focused on SPI transfers, nothing
> memory-specific should leak there.
>
> > 2. modify SPI-NAND core to add support for special SPI-NAND controller.
> >     This limits controller's ability and adds extra burden for future extention
> >     of SPI-NAND framework.
>
> That doesn't work either as some ECC engines are shared between the
> raw NAND and spi-mem IPs.
>
> >
> > Which way would you prefer and do you have other suggestions?
>
> See [3]. I think you can already base your work on Miquel's series, but
> maybe he has a more up-to-date version to share. I'll let you sync with
> him.

Thanks. I'll check it out.

>
> Regards,
>
> Boris
>
> P.S.: the discussion should probably happen in the open. There's
> nothing confidential here, so I'd recommend continuing the discussion
> on [3] or creating a new thread with the MTD ML in Cc.

I did send it to the public list by replying to that 2-year-old email. It has
your old bootlin address and was rejected so I send it again to you
only :)

>
> >
> > [0] https://drive.google.com/file/d/1cW8KQmmVpwDGmBd48KNQes9CRn7FEgBb/view?usp=sharing
> > [1] http://wiki.banana-pi.org/Banana_Pi_BPI-R64#Documents
> > [2] https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=target/linux/mediatek/patches-4.19/0306-spi-spi-mem-MediaTek-Add-SPI-NAND-Flash-interface-dr.patch;h=2370925372f69aed0566339a4808056580e88837;hb=HEAD
>
> [3]https://lore.kernel.org/linux-mtd/20190919193141.7865-1-miquel.raynal-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org/
>

-- 
Regards,
Chuanhong Guo

Re: [PATCH 3/3] spi: spi-mem: MediaTek: Add SPI NAND Flash interface driver for MediaTek MT7622

[Miquel Raynal]

Hi Chuanhong,

Chuanhong Guo <gch981213-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote on Wed, 11 Mar 2020 17:15:38
+0800:

> Hi!
> 
> On Wed, Mar 11, 2020 at 4:18 PM Boris Brezillon
> <boris.brezillon-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org> wrote:
> >
> > +Miquel who worked on the ECC engine abstraction [3] recently.
> >
> > Hello Chuanhong,
> >
> > On Wed, 11 Mar 2020 15:35:43 +0800
> > Chuanhong Guo <gch981213-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >  
> > > Hi Boris!
> > >
> > > [resend to you because of the wrong mail address in previous one.]
> > >
> > > I'm about to pick this driver up and start upstream it in the future.
> > > So I'm answering
> > > your questions below and would like to get your further suggestions.
> > >
> > > On Fri, Oct 26, 2018 at 2:09 PM Boris Brezillon
> > > <boris.brezillon-LDxbnhwyfcJBDgjK7y7TUQ@public.gmane.org> wrote:  
> > > > > > There's a fundamental issue with this driver: spi-mem was designed to be
> > > > > > memory agnostic, and you seem to have a SPI controller that supports
> > > > > > only SPI NANDs. Is that correct, or is it just that you developed the
> > > > > > driver with SPI NANDs in  mind?
> > > > > >  
> > > > > Yes, this driver supports only SPI NANDs.
> > > > > Actually, Mediatek's controller is designed for NAND specifically, which
> > > > > can support SPI NANDs and PARALLEL NANDs,and this driver is just for SPI
> > > > > NANDs.  
> > > >
> > > > Hm, I'm not so sure about that (I might be wrong though), it seems you
> > > > can send any command you want, not only SPI NAND related ones. Maybe the
> > > > controller is optimized for SPI NANDs but can do all kind of SPI
> > > > transfers.  
> > >
> > > You are correct here. This controller can perform generic spi-mem operations,
> > > and it has special routines for page cache R/W that utilize controller's ECC
> > > functionality.  
> >
> > Sounds similar to the way the MXIC controller works, and that's
> > actually what Miquel is trying to support with his ECC engine
> > abstraction series [3].
> >  
> > > I think the purpose of this is to provide better ECC capability
> > > for some SPI NANDs with worse ECC algorithm on chip.  
> >
> > Yep, or make it faster. Actually the reason doesn't matter, I think
> > we all agree that we'll have to support external ECC for SPI NANDs at
> > some point, hence the work Miquel has been doing.
> >  
> > >  
> > > > > > Don't know what's possible to do with your controller, and maybe it's
> > > > > > not able to execute random SPI memory operations, but in this case we
> > > > > > should consider a different solution to support this driver.
> > > > > >
> > > > > > Do you have a public datasheet I can look at?
> > > > > >  
> > > > > We do not have a public datasheet for Mediatek controller currently.  
> > > >
> > > > Unfortunately, there's not much I can do without a clear understanding
> > > > of how the controller works.
> > > >  
> > >
> > > I found a public datasheet [0] on wiki page for Banana Pi R64 [1].
> > > Register description is available under "NAND flash interface" section
> > > starting at page 592.
> > > There's a hackier version of this driver in OpenWrt [2] which checks
> > > opcode and use controller routine for page cache R/W.
> > >
> > > ECC part of this controller can also be used as a standalone ECC
> > > algorithm and perform ECC operations on data provided by CPU.  
> 
> The solution I'm referring to here is:
> 1. read uncorrected data to host directly from SPI NAND
> 2. start an ECC correction separately
> 
> > > But I think if it's implement this way, we wasted the page cache
> > > R/W routines in this controller.  
> >
> > Oh, you probably don't want the page cache to be active anyway. When
> > the framework reads a NAND page, it also checks the number of ECC
> > errors, if the page is held in some internal cache, you won't see
> > the evolution of this number. Note that the FS should do some caching,
> > so caching things at the HW level is probably useless.  
> 
> It doesn't cache anything in controller. The R/W routine I refer to is:
> When we issue a request to read page cache on SPI NAND:
> 1. host programs a DMA-able memory area for receiving data.
> 2. controller reads the page cache from SPI NAND
> 3. controller get the data and start ECC correction
> 4. corrected data will be sent back to host via DMA
> 5. host could check ECC status
> 
> writing of page cache goes similarly.
> There's no need for a separated ECC request comparing to previous
> one.
> 
> >  
> > >
> > > I have two other initial thoughts:
> > > 1. abstract some kind of ECC functionality in spi-mem interface
> > >     I haven't really learned ECC stuff so I don't know whether this is
> > >     possible and what kind of argument we needs for it.  
> >
> > Nope, spi-mem should stay focused on SPI transfers, nothing
> > memory-specific should leak there.
> >  
> > > 2. modify SPI-NAND core to add support for special SPI-NAND controller.
> > >     This limits controller's ability and adds extra burden for future extention
> > >     of SPI-NAND framework.  
> >
> > That doesn't work either as some ECC engines are shared between the
> > raw NAND and spi-mem IPs.
> >  
> > >
> > > Which way would you prefer and do you have other suggestions?  
> >
> > See [3]. I think you can already base your work on Miquel's series, but
> > maybe he has a more up-to-date version to share. I'll let you sync with
> > him.  

I am actively working on it, this series is adding an "ECC engine
framework" that could potentially fit any architecture. I am currently
working with a Macronix external ECC engine, I will "soon" send a new
version of it, I'll copy you.

Thanks,
Miquèl