On Fri, 26 Nov 2021 12:39:19 +0100
Miquel Raynal <miquel.raynal@xxxxxxxxxxx> wrote:
> In order for pipelined ECC engines to be able to enable/disable the ECC
> engine only when needed and avoid races when future parallel-operations
> will be supported, we need to provide the information about the use of
> the ECC engine in the direct mapping hooks. As direct mapping
> configurations are meant to be static, it is best to create two new
> mappings: one for regular 'raw' accesses and one for accesses involving
> correction. It is up to the driver to use or not the new ECC enable
> boolean contained in the spi-mem operation.
>
> Signed-off-by: Miquel Raynal <miquel.raynal@xxxxxxxxxxx>
> ---
> drivers/mtd/nand/spi/core.c | 28 ++++++++++++++++++++++++++--
> include/linux/mtd/spinand.h | 2 ++
> include/linux/spi/spi-mem.h | 3 +++
I'd split that patch in 2: one adding the ecc_en field to spi_mem_op
and the other one using it in spinand.
> 3 files changed, 31 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
> index 7027c09925e2..10ccffb6bf0d 100644
> --- a/drivers/mtd/nand/spi/core.c
> +++ b/drivers/mtd/nand/spi/core.c
> @@ -381,7 +381,10 @@ static int spinand_read_from_cache_op(struct spinand_device *spinand,
> }
> }
>
> - rdesc = spinand->dirmaps[req->pos.plane].rdesc;
> + if (req->mode == MTD_OPS_RAW)
> + rdesc = spinand->dirmaps[req->pos.plane].rdesc;
> + else
> + rdesc = spinand->dirmaps[req->pos.plane].rdesc_ecc;
>
> while (nbytes) {
> ret = spi_mem_dirmap_read(rdesc, column, nbytes, buf);
> @@ -452,7 +455,10 @@ static int spinand_write_to_cache_op(struct spinand_device *spinand,
> req->ooblen);
> }
>
> - wdesc = spinand->dirmaps[req->pos.plane].wdesc;
> + if (req->mode == MTD_OPS_RAW)
> + wdesc = spinand->dirmaps[req->pos.plane].wdesc;
> + else
> + wdesc = spinand->dirmaps[req->pos.plane].wdesc_ecc;
>
> while (nbytes) {
> ret = spi_mem_dirmap_write(wdesc, column, nbytes, buf);
> @@ -866,6 +872,24 @@ static int spinand_create_dirmap(struct spinand_device *spinand,
>
> spinand->dirmaps[plane].rdesc = desc;
>
> + info.op_tmpl = *spinand->op_templates.update_cache;
> + info.op_tmpl.ecc_en = true;
> + desc = devm_spi_mem_dirmap_create(&spinand->spimem->spi->dev,
> + spinand->spimem, &info);
> + if (IS_ERR(desc))
> + return PTR_ERR(desc);
> +
> + spinand->dirmaps[plane].wdesc_ecc = desc;
> +
> + info.op_tmpl = *spinand->op_templates.read_cache;
> + info.op_tmpl.ecc_en = true;
> + desc = devm_spi_mem_dirmap_create(&spinand->spimem->spi->dev,
> + spinand->spimem, &info);
> + if (IS_ERR(desc))
> + return PTR_ERR(desc);
> +
> + spinand->dirmaps[plane].rdesc_ecc = desc;
> +
Direct mappings are not free (they might reserve a piece of MMIO
address space depending on the spi-mem controller implementation), so
I'd recommend creating those mapping only when strictly needed, that
is, when dealing with a pipelined ECC.
> return 0;
> }
>
> diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h
> index 6988956b8492..3aa28240a77f 100644
> --- a/include/linux/mtd/spinand.h
> +++ b/include/linux/mtd/spinand.h
> @@ -389,6 +389,8 @@ struct spinand_info {
> struct spinand_dirmap {
> struct spi_mem_dirmap_desc *wdesc;
> struct spi_mem_dirmap_desc *rdesc;
> + struct spi_mem_dirmap_desc *wdesc_ecc;
> + struct spi_mem_dirmap_desc *rdesc_ecc;
> };
>
> /**
> diff --git a/include/linux/spi/spi-mem.h b/include/linux/spi/spi-mem.h
> index 85e2ff7b840d..3be594be24c0 100644
> --- a/include/linux/spi/spi-mem.h
> +++ b/include/linux/spi/spi-mem.h
> @@ -94,6 +94,7 @@ enum spi_mem_data_dir {
> * operation does not involve transferring data
> * @data.buf.in: input buffer (must be DMA-able)
> * @data.buf.out: output buffer (must be DMA-able)
> + * @ecc_en: error correction is required
> */
> struct spi_mem_op {
> struct {
> @@ -126,6 +127,8 @@ struct spi_mem_op {
> const void *out;
> } buf;
> } data;
> +
> + bool ecc_en;
> };
>
> #define SPI_MEM_OP(__cmd, __addr, __dummy, __data) \
