diff mbox series

[v7,07/14] spi: spi-mem: Add an ecc_en parameter to the spi_mem_op structure

Message ID 20211217161654.367782-8-miquel.raynal@bootlin.com
State New
Headers show
Series External ECC engines & Macronix support | expand

Commit Message

Miquel Raynal Dec. 17, 2021, 4:16 p.m. UTC
Soon the SPI-NAND core will need a way to request a SPI controller to
enable ECC support for a given operation. This is because of the
pipelined integration of certain ECC engines, which are directly managed
by the SPI controller itself.

Introduce a spi_mem_op additional field for this purpose: ecc_en.

So far this field is left unset and checked to be false by all
the SPI controller drivers in their ->supports_op() hook, as they all
call spi_mem_default_supports_op().

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/spi/spi-mem.c       | 5 +++++
 include/linux/spi/spi-mem.h | 5 +++++
 2 files changed, 10 insertions(+)

Comments

Pratyush Yadav Dec. 20, 2021, 7:02 p.m. UTC | #1
On 17/12/21 05:16PM, Miquel Raynal wrote:
> Soon the SPI-NAND core will need a way to request a SPI controller to
> enable ECC support for a given operation. This is because of the
> pipelined integration of certain ECC engines, which are directly managed
> by the SPI controller itself.
> 
> Introduce a spi_mem_op additional field for this purpose: ecc_en.
> 
> So far this field is left unset and checked to be false by all
> the SPI controller drivers in their ->supports_op() hook, as they all
> call spi_mem_default_supports_op().
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> ---
>  drivers/spi/spi-mem.c       | 5 +++++
>  include/linux/spi/spi-mem.h | 5 +++++
>  2 files changed, 10 insertions(+)
> 
> diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
> index cfe1c99db5f3..94758e7e747d 100644
> --- a/drivers/spi/spi-mem.c
> +++ b/drivers/spi/spi-mem.c
> @@ -178,6 +178,11 @@ bool spi_mem_default_supports_op(struct spi_mem *mem,
>  			return false;
>  	}
>  
> +	if (op->ecc_en) {
> +		if (!spi_mem_controller_is_capable(ctlr, ecc))
> +			return false;
> +	}
> +
>  	return spi_mem_check_buswidth(mem, op);
>  }
>  EXPORT_SYMBOL_GPL(spi_mem_default_supports_op);
> diff --git a/include/linux/spi/spi-mem.h b/include/linux/spi/spi-mem.h
> index d7787c8f3746..e9238a858109 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;

ECC should only concern the data phase right? Would it make more sense 
to move this field under data?

Anyway, I don't know much about NAND or ECC so either way,

Acked-by: Pratyush Yadav <p.yadav@ti.com>

>  };
>  
>  #define SPI_MEM_OP(__cmd, __addr, __dummy, __data)		\
> @@ -223,9 +226,11 @@ static inline void *spi_mem_get_drvdata(struct spi_mem *mem)
>  /**
>   * struct spi_controller_mem_caps - SPI memory controller capabilities
>   * @dtr: Supports DTR operations
> + * @ecc: Supports operations with error correction
>   */
>  struct spi_controller_mem_caps {
>  	bool dtr;
> +	bool ecc;
>  };
>  
>  #define spi_mem_controller_is_capable(ctlr, cap)		\
> -- 
> 2.27.0
>
Miquel Raynal Dec. 21, 2021, 5:37 p.m. UTC | #2
Hi Pratyush,

p.yadav@ti.com wrote on Tue, 21 Dec 2021 00:32:29 +0530:

> On 17/12/21 05:16PM, Miquel Raynal wrote:
> > Soon the SPI-NAND core will need a way to request a SPI controller to
> > enable ECC support for a given operation. This is because of the
> > pipelined integration of certain ECC engines, which are directly managed
> > by the SPI controller itself.
> > 
> > Introduce a spi_mem_op additional field for this purpose: ecc_en.
> > 
> > So far this field is left unset and checked to be false by all
> > the SPI controller drivers in their ->supports_op() hook, as they all
> > call spi_mem_default_supports_op().
> > 
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> >  drivers/spi/spi-mem.c       | 5 +++++
> >  include/linux/spi/spi-mem.h | 5 +++++
> >  2 files changed, 10 insertions(+)
> > 
> > diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
> > index cfe1c99db5f3..94758e7e747d 100644
> > --- a/drivers/spi/spi-mem.c
> > +++ b/drivers/spi/spi-mem.c
> > @@ -178,6 +178,11 @@ bool spi_mem_default_supports_op(struct spi_mem *mem,
> >  			return false;
> >  	}
> >  
> > +	if (op->ecc_en) {
> > +		if (!spi_mem_controller_is_capable(ctlr, ecc))
> > +			return false;
> > +	}
> > +
> >  	return spi_mem_check_buswidth(mem, op);
> >  }
> >  EXPORT_SYMBOL_GPL(spi_mem_default_supports_op);
> > diff --git a/include/linux/spi/spi-mem.h b/include/linux/spi/spi-mem.h
> > index d7787c8f3746..e9238a858109 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;  
> 
> ECC should only concern the data phase right? Would it make more sense 
> to move this field under data?
> 
> Anyway, I don't know much about NAND or ECC so either way,

I've moved the field to the data struct because it makes indeed more
sense. I've also changed the type and name to "u8 ecc : 1;" to match
the style used for the dtr parameter.

> 
> Acked-by: Pratyush Yadav <p.yadav@ti.com>
> 
> >  };
> >  
> >  #define SPI_MEM_OP(__cmd, __addr, __dummy, __data)		\
> > @@ -223,9 +226,11 @@ static inline void *spi_mem_get_drvdata(struct spi_mem *mem)
> >  /**
> >   * struct spi_controller_mem_caps - SPI memory controller capabilities
> >   * @dtr: Supports DTR operations
> > + * @ecc: Supports operations with error correction
> >   */
> >  struct spi_controller_mem_caps {
> >  	bool dtr;
> > +	bool ecc;
> >  };
> >  
> >  #define spi_mem_controller_is_capable(ctlr, cap)		\
> > -- 
> > 2.27.0
> >   
> 


Thanks,
Miquèl
diff mbox series

Patch

diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
index cfe1c99db5f3..94758e7e747d 100644
--- a/drivers/spi/spi-mem.c
+++ b/drivers/spi/spi-mem.c
@@ -178,6 +178,11 @@  bool spi_mem_default_supports_op(struct spi_mem *mem,
 			return false;
 	}
 
+	if (op->ecc_en) {
+		if (!spi_mem_controller_is_capable(ctlr, ecc))
+			return false;
+	}
+
 	return spi_mem_check_buswidth(mem, op);
 }
 EXPORT_SYMBOL_GPL(spi_mem_default_supports_op);
diff --git a/include/linux/spi/spi-mem.h b/include/linux/spi/spi-mem.h
index d7787c8f3746..e9238a858109 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)		\
@@ -223,9 +226,11 @@  static inline void *spi_mem_get_drvdata(struct spi_mem *mem)
 /**
  * struct spi_controller_mem_caps - SPI memory controller capabilities
  * @dtr: Supports DTR operations
+ * @ecc: Supports operations with error correction
  */
 struct spi_controller_mem_caps {
 	bool dtr;
+	bool ecc;
 };
 
 #define spi_mem_controller_is_capable(ctlr, cap)		\