Re: [RFC,v3 5/5] mtd: spinand: skip set/get oob data bytes when interleaved case

Miquel Raynal
2021-11-09T12:06:03Z

Hi Xiangsheng,

Has we discussed a while ago:

...it is possible that I did not test with MTD_OPS_AUTO_OOB
recently and indeed the ECC bytes will missing in this case. In
the write path, maybe the ->prepare_io hooks should spread the
user data following req->mode in req.oobbuf before computing
the codes. Similar logic should be applied to the read path.

Can you please add a patch for this situation in your next iteration?
It does not look like this situation has been handled.

xiangsheng.hou@mediatek.com wrote on Fri, 22 Oct 2021 10:40:21 +0800:

> For syndrome layout, ECC/free byte in oob layout and main
> data are interleaved. For this case, it is better to set/get
> oob data bytes in ECC engine.

I don't understand the last sentence

>
> For MTK ECC engine, although it can auto place data as sector +
> oob free + oob ecc for one page in pipelined. However, the bad
> mark will be not fit with nand spec. Therefore, there have bad
> mark swap operation in ecc engine.
>
> But, the swap opeation(between bbm 0xff with 1byte main data) will
> lead to more one byte than oobavailable.

Sorry but again, I don't understand what you mean.

> Set oob databytes after
> bad mark swap will lead to lost one oob free byte.

We don't care about free OOB bytes really.

>
> Therefore, just try to modify the spi nand framework for review.
> And this may be common for the interleaved case.

I don't get how falling back to a memcpy will solve your problem? Can
you please provide an anscii figure or something more visual to let us
understand?

>
> Signed-off-by: Xiangsheng Hou <xiangsheng.hou@mediatek.com>
> ---
> drivers/mtd/nand/spi/core.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
> index 2c8685f1f2fa..32a4707982c5 100644
> --- a/drivers/mtd/nand/spi/core.c
> +++ b/drivers/mtd/nand/spi/core.c
> @@ -401,7 +401,8 @@ static int spinand_read_from_cache_op(struct spinand_device *spinand,
> req->datalen);
>
> if (req->ooblen) {
> - if (req->mode == MTD_OPS_AUTO_OOB)
> + if (req->mode == MTD_OPS_AUTO_OOB &&
> + nand->ecc.user_conf.placement != NAND_ECC_PLACEMENT_INTERLEAVED)
> mtd_ooblayout_get_databytes(mtd, req->oobbuf.in,
> spinand->oobbuf,
> req->ooboffs,
> @@ -442,7 +443,8 @@ static int spinand_write_to_cache_op(struct spinand_device *spinand,
> req->datalen);
>
> if (req->ooblen) {
> - if (req->mode == MTD_OPS_AUTO_OOB)
> + if (req->mode == MTD_OPS_AUTO_OOB &&
> + nand->ecc.user_conf.placement != NAND_ECC_PLACEMENT_INTERLEAVED)
> mtd_ooblayout_set_databytes(mtd, req->oobbuf.out,
> spinand->oobbuf,
> req->ooboffs,

Thanks,
Miquèl