mtd: spinand: Add Octal DTR SPI (8D-8D-8D) mode support

Setting dtr field of spi_mem_op is useful when creating templates for DTR ops in spinand.h. Also, 2 bytes cmd phases are required when operating in Octal DTR SPI mode.

Setting dtr field of spi_mem_op is useful when creating templates for DTR ops in spinand.h. Also, 2 bytes cmd phases are required when operating in Octal DTR SPI mode.

Create new templates for dtr mode cmd, address, dummy and data phase in spi_mem_op, which set the dtr field to 1 and also allow passing the nbytes for the cmd phase.

Signed-off-by Apurva Nandan <[email protected]>

---
 include/linux/spi/spi-mem.h | 41 +++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

include/linux/spi/spi-mem.h +34 -0

index 85e2ff7b840d..682378a9c600 100644
@@ -20,6 +20,14 @@
 		.nbytes = 1,					\
 	}
 
+#define SPI_MEM_OP_CMD_DTR(__nbytes, __opcode, __buswidth)	\
+	{							\
+		.nbytes = __nbytes,				\
+		.opcode = __opcode,				\
+		.buswidth = __buswidth,				\
+		.dtr = 1,					\
+	}
+
 #define SPI_MEM_OP_ADDR(__nbytes, __val, __buswidth)		\
 	{							\
 		.nbytes = __nbytes,				\

@@ -27,6 +35,14 @@
 		.buswidth = __buswidth,				\
 	}
 
+#define SPI_MEM_OP_ADDR_DTR(__nbytes, __val, __buswidth)	\
+	{							\
+		.nbytes = __nbytes,				\
+		.val = __val,					\
+		.buswidth = __buswidth,				\
+		.dtr = 1,					\
+	}
+
 #define SPI_MEM_OP_NO_ADDR	{ }
 
 #define SPI_MEM_OP_DUMMY(__nbytes, __buswidth)			\

@@ -35,6 +51,13 @@
 		.buswidth = __buswidth,				\
 	}
 
+#define SPI_MEM_OP_DUMMY_DTR(__nbytes, __buswidth)		\
+	{							\
+		.nbytes = __nbytes,				\
+		.buswidth = __buswidth,				\
+		.dtr = 1,					\
+	}
+
 #define SPI_MEM_OP_NO_DUMMY	{ }
 
 #define SPI_MEM_OP_DATA_IN(__nbytes, __buf, __buswidth)		\

@@ -45,6 +68,15 @@
 		.buswidth = __buswidth,				\
 	}
 
+#define SPI_MEM_OP_DATA_IN_DTR(__nbytes, __buf, __buswidth)	\
+	{							\
+		.dir = SPI_MEM_DATA_IN,				\
+		.nbytes = __nbytes,				\
+		.buf.in = __buf,				\
+		.buswidth = __buswidth,				\
+		.dtr = 1,					\
+	}
+
 #define SPI_MEM_OP_DATA_OUT(__nbytes, __buf, __buswidth)	\
 	{							\
 		.dir = SPI_MEM_DATA_OUT,			\

@@ -53,6 +85,15 @@
 		.buswidth = __buswidth,				\
 	}
 
+#define SPI_MEM_OP_DATA_OUT_DTR(__nbytes, __buf, __buswidth)	\
+	{							\

and you get to define a DTR op like that:

	struct spi_mem_op op =
		SPI_MEM_OP(SPI_MEM_OP_EXT_CMD(2, 0x1234, 8, SPI_MEM_OP_DTR),
			   SPI_MEM_OP_ADDR(4, 0xdeadbeef, 8, SPI_MEM_OP_DTR),
			   SPI_MEM_OP_DATA_OUT(128, buf, 8, SPI_MEM_OP_DTR));

This also means we can extend the struct without having to define new macros.

Quoted 7 lines · Boris Brezillon

and you get to define a DTR op like that:

	struct spi_mem_op op =
		SPI_MEM_OP(SPI_MEM_OP_EXT_CMD(2, 0x1234, 8, SPI_MEM_OP_DTR),
			   SPI_MEM_OP_ADDR(4, 0xdeadbeef, 8, SPI_MEM_OP_DTR),
			   SPI_MEM_OP_DATA_OUT(128, buf, 8, SPI_MEM_OP_DTR));

Quoted 1 line

This also means we can extend the struct without having to define new macros.

I like this. It would also let us easily mix-and-match the ecc parameter that Miquel is adding.

include/linux/spi/spi-mem.h +2 -1

-#define SPI_MEM_OP_DATA_IN(__nbytes, __buf, __buswidth)                \
+#define SPI_MEM_OP_DATA_IN(__nbytes, __buf, __buswidth, ...)   \
        {                                                       \
                .dir = SPI_MEM_DATA_IN,                         \
                .nbytes = __nbytes,                             \
                .buf.in = __buf,                                \
                .buswidth = __buswidth,                         \
+               __VA_ARGS__                                     \

include/linux/mtd/spinand.h +0 -0

index 69e06e741717..77927afcea0f 100644
@@ -194,6 +194,18 @@
 		   SPI_MEM_OP_NO_DUMMY,					\
 		   SPI_MEM_OP_DATA_OUT_DTR(len, buf, 8))
 

Manufacturers have been deviating from the standard SPI operations for NAND flashes. There have been variations in non-page read/write instructions too. Additionally, operations, including non-page r/w ops, vary when flash is in different SPI mode, eg. Octal DTR.

To avoid live-patching in hot-paths or vendor-specific adjustment, it is better to have a set of operation templates and variants for non-page read/write operations as well. These would get initialized at the probe time or when flash changes modes. These would be called 'ctrl_ops'.

To make code better understandable, create two types of op templates which are: data_ops and ctrl_ops. Reason for having two different type of templates is the difference in their use cases i.e. it is possible to have ops of different protocol for read/write/update simulatneously in the data_ops, but all the ops in the ctrl_ops follow same protocol.

Rename op_templates to data_ops, and the ctrl_ops would be introduced in later commits.

'ctrl_ops' are op templates for non-page read/write operations, which are: reset, get_feature, set_feature, write_enable, block_erase, page_read and program_execute ops. The 'ctrl_ops' struct contains in it op templates for each of this op, as well as enum spinand_protocol denoting protocol of all these ops.

We require these new op templates because of deviation in standard SPINAND ops by manufacturers and also due to changes when there is a change in SPI protocol/mode. This prevents the core from live-patching and vendor-specific adjustments in ops.

Define 'ctrl_ops', add macros to initialize it and add it in spinand_device.

Signed-off-by Apurva Nandan <[email protected]>

---
 include/linux/mtd/spinand.h | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

include/linux/mtd/spinand.h +1 -0

index 439d8ce40e1d..e5df6220ec1e 100644
@@ -356,6 +356,35 @@ struct spinand_op_variants {
 			sizeof(struct spi_mem_op),			\
 	}
 
+struct spinand_ctrl_ops {

include/linux/mtd/spinand.h +20 -0

+};
+
+#define SPINAND_CTRL_OPS(__protocol, __reset, __get_feature, __set_feature,	\
+			 __write_enable, __block_erase, __page_read,		\
+			 __program_execute)					\
+	{									\
+		.ops = {							\
+			.reset = __reset,					\
+			.get_feature = __get_feature,				\
+			.set_feature = __set_feature,				\
+			.write_enable = __write_enable,				\
+			.block_erase = __block_erase,				\
+			.page_read = __page_read,				\
+			.program_execute = __program_execute,			\
+		},								\
+		.protocol = __protocol,						\
+	}
+
 /**
  * spinand_ecc_info - description of the on-die ECC implemented by a SPI NAND
  *		      chip

@@ -468,6 +497,8 @@ struct spinand_dirmap {
  * @data_ops.read_cache: read cache op template
  * @data_ops.write_cache: write cache op template
  * @data_ops.update_cache: update cache op template
+ * @ctrl_ops: various SPI mem op templates for handling the flash device, i.e.
+ *	      non page-read/write ops.
  * @select_target: select a specific target/die. Usually called before sending
  *		   a command addressing a page or an eraseblock embedded in
  *		   this die. Only required if your chip exposes several dies

struct spinand_device {
	...
	enum spinand_protocol protocol;
	const struct spinand_op_templates *op_templates[SPINAND_NUM_PROTOCOLS];

	...
};

Quoted 28 lines · On Sat, 1 Jan 2022 13:12:38 +0530 Apurva Nandan <[email protected]> wrote:

'ctrl_ops' are op templates for non-page read/write operations, which are: reset, get_feature, set_feature, write_enable, block_erase, page_read and program_execute ops. The 'ctrl_ops' struct contains in it op templates for each of this op, as well as enum spinand_protocol denoting protocol of all these ops.

We require these new op templates because of deviation in standard SPINAND ops by manufacturers and also due to changes when there is a change in SPI protocol/mode. This prevents the core from live-patching and vendor-specific adjustments in ops.

Define 'ctrl_ops', add macros to initialize it and add it in spinand_device.

Signed-off-by Apurva Nandan <[email protected]>

---
  include/linux/mtd/spinand.h | 33 +++++++++++++++++++++++++++++++++
  1 file changed, 33 insertions(+)

include/linux/mtd/spinand.h +1 -0

index 439d8ce40e1d..e5df6220ec1e 100644
@@ -356,6 +356,35 @@ struct spinand_op_variants {
  			sizeof(struct spi_mem_op),			\
  	}
 
+struct spinand_ctrl_ops {

include/linux/mtd/spinand.h struct spinand_op_variants { · line 369

Quoted 10 lines

include/linux/mtd/spinand.h +10 -0

+	const struct {
+		struct spi_mem_op reset;
+		struct spi_mem_op get_feature;
+		struct spi_mem_op set_feature;
+		struct spi_mem_op write_enable;
+		struct spi_mem_op block_erase;
+		struct spi_mem_op page_read;
+		struct spi_mem_op program_execute;
+	} ops;
+	const enum spinand_protocol protocol;

Do you really need that protocol field?

Quoted 31 lines

include/linux/mtd/spinand.h +20 -0

+};
+
+#define SPINAND_CTRL_OPS(__protocol, __reset, __get_feature, __set_feature,	\
+			 __write_enable, __block_erase, __page_read,		\
+			 __program_execute)					\
+	{									\
+		.ops = {							\
+			.reset = __reset,					\
+			.get_feature = __get_feature,				\
+			.set_feature = __set_feature,				\
+			.write_enable = __write_enable,				\
+			.block_erase = __block_erase,				\
+			.page_read = __page_read,				\
+			.program_execute = __program_execute,			\
+		},								\
+		.protocol = __protocol,						\
+	}
+
  /**
   * spinand_ecc_info - description of the on-die ECC implemented by a SPI NAND
   *		      chip

@@ -468,6 +497,8 @@ struct spinand_dirmap {
   * @data_ops.read_cache: read cache op template
   * @data_ops.write_cache: write cache op template
   * @data_ops.update_cache: update cache op template
+ * @ctrl_ops: various SPI mem op templates for handling the flash device, i.e.
+ *	      non page-read/write ops.
   * @select_target: select a specific target/die. Usually called before sending
   *		   a command addressing a page or an eraseblock embedded in
   *		   this die. Only required if your chip exposes several dies

include/linux/mtd/spinand.h struct spinand_device { · line 533

Quoted 5 lines

include/linux/mtd/spinand.h +2 -0

@@ -498,6 +529,8 @@ struct spinand_device {
  		const struct spi_mem_op *update_cache;
  	} data_ops;
 
+	const struct spinand_ctrl_ops *ctrl_ops;
+

Okay, I had something slightly different in mind. First, I'd put all templates in a struct:

struct spinand_op_templates {
	const struct spi_mem_op *read_cache;
	const struct spi_mem_op *write_cache;
	const struct spi_mem_op *update_cache;
	const struct spi_mem_op *reset;
	const struct spi_mem_op *get_feature;
	const struct spi_mem_op *set_feature;
	const struct spi_mem_op *write_enable;
	const struct spi_mem_op *block_erase;
	const struct spi_mem_op *page_load;
	const struct spi_mem_op *program_execute;
};

Then, at the spinand level, I'd define an array of templates:

enum spinand_protocol {
	SPINAND_1S_1S_1S,
	SPINAND_2S_2S_2S,
	SPINAND_4S_4S_4S,
	SPINAND_8S_8S_8S,
	SPINAND_8D_8D_8D,
	SPINAND_NUM_PROTOCOLS,
};

struct spinand_device {
	...
	enum spinand_protocol protocol;
	const struct spinand_op_templates *op_templates[SPINAND_NUM_PROTOCOLS];
	...
};

This way, you can easily pick the right set of operations based on the protocol/mode you're in:

#define spinand_get_op_template(spinand, opname) \
	((spinand)->op_templates[(spinand)->protocol]->opname)

static int spinand_read_reg_op(struct spinand_device *spinand, u8 reg, u8 *val)

{
	struct spi_mem_op op = *spinand_get_op_template(spinand, get_feature);
	int ret;

	...
}

Hi Boris,

Quoted 110 lines · Boris Brezillon

Quoted 28 lines · On Sat, 1 Jan 2022 13:12:38 +0530 Apurva Nandan <[email protected]> wrote:

'ctrl_ops' are op templates for non-page read/write operations, which are: reset, get_feature, set_feature, write_enable, block_erase, page_read and program_execute ops. The 'ctrl_ops' struct contains in it op templates for each of this op, as well as enum spinand_protocol denoting protocol of all these ops.

We require these new op templates because of deviation in standard SPINAND ops by manufacturers and also due to changes when there is a change in SPI protocol/mode. This prevents the core from live-patching and vendor-specific adjustments in ops.

Define 'ctrl_ops', add macros to initialize it and add it in spinand_device.

Signed-off-by Apurva Nandan <[email protected]>

---
  include/linux/mtd/spinand.h | 33 +++++++++++++++++++++++++++++++++
  1 file changed, 33 insertions(+)

include/linux/mtd/spinand.h +1 -0

index 439d8ce40e1d..e5df6220ec1e 100644
@@ -356,6 +356,35 @@ struct spinand_op_variants {
  			sizeof(struct spi_mem_op),			\
  	}
 
+struct spinand_ctrl_ops {

include/linux/mtd/spinand.h struct spinand_op_variants { · line 369

Quoted 10 lines

include/linux/mtd/spinand.h +10 -0

+	const struct {
+		struct spi_mem_op reset;
+		struct spi_mem_op get_feature;
+		struct spi_mem_op set_feature;
+		struct spi_mem_op write_enable;
+		struct spi_mem_op block_erase;
+		struct spi_mem_op page_read;
+		struct spi_mem_op program_execute;
+	} ops;
+	const enum spinand_protocol protocol;  

Do you really need that protocol field?

Quoted 31 lines

include/linux/mtd/spinand.h +20 -0

+};
+
+#define SPINAND_CTRL_OPS(__protocol, __reset, __get_feature, __set_feature,	\
+			 __write_enable, __block_erase, __page_read,		\
+			 __program_execute)					\
+	{									\
+		.ops = {							\
+			.reset = __reset,					\
+			.get_feature = __get_feature,				\
+			.set_feature = __set_feature,				\
+			.write_enable = __write_enable,				\
+			.block_erase = __block_erase,				\
+			.page_read = __page_read,				\
+			.program_execute = __program_execute,			\
+		},								\
+		.protocol = __protocol,						\
+	}
+
  /**
   * spinand_ecc_info - description of the on-die ECC implemented by a SPI NAND
   *		      chip

@@ -468,6 +497,8 @@ struct spinand_dirmap {
   * @data_ops.read_cache: read cache op template
   * @data_ops.write_cache: write cache op template
   * @data_ops.update_cache: update cache op template
+ * @ctrl_ops: various SPI mem op templates for handling the flash device, i.e.
+ *	      non page-read/write ops.
   * @select_target: select a specific target/die. Usually called before sending
   *		   a command addressing a page or an eraseblock embedded in
   *		   this die. Only required if your chip exposes several dies

include/linux/mtd/spinand.h struct spinand_device { · line 533

Quoted 5 lines

include/linux/mtd/spinand.h +2 -0

@@ -498,6 +529,8 @@ struct spinand_device {
  		const struct spi_mem_op *update_cache;
  	} data_ops;
 
+	const struct spinand_ctrl_ops *ctrl_ops;
+  

Okay, I had something slightly different in mind. First, I'd put all templates in a struct:

struct spinand_op_templates {
	const struct spi_mem_op *read_cache;
	const struct spi_mem_op *write_cache;
	const struct spi_mem_op *update_cache;
	const struct spi_mem_op *reset;
	const struct spi_mem_op *get_feature;
	const struct spi_mem_op *set_feature;
	const struct spi_mem_op *write_enable;
	const struct spi_mem_op *block_erase;
	const struct spi_mem_op *page_load;
	const struct spi_mem_op *program_execute;
};

Then, at the spinand level, I'd define an array of templates:

enum spinand_protocol {
	SPINAND_1S_1S_1S,
	SPINAND_2S_2S_2S,
	SPINAND_4S_4S_4S,
	SPINAND_8S_8S_8S,
	SPINAND_8D_8D_8D,
	SPINAND_NUM_PROTOCOLS,
};

struct spinand_device {
	...
	enum spinand_protocol protocol;
	const struct spinand_op_templates *op_templates[SPINAND_NUM_PROTOCOLS];

Quoted 7 lines

This way, you can easily pick the right set of operations based on the protocol/mode you're in:

#define spinand_get_op_template(spinand, opname) \
	((spinand)->op_templates[(spinand)->protocol]->opname)

Quoted 7 lines

static int spinand_read_reg_op(struct spinand_device *spinand, u8 reg, u8 *val)

{
	struct spi_mem_op op = *spinand_get_op_template(spinand, get_feature);
	int ret;

	...
}  

I find a couple of issues with this  method,

1. read_cache, write_cache, update_cache op templates don't fit well with the other non-data ops, as
these data ops are used to create a dirmap, and that can be done only once at probe time. Hence, there
is a different mechanism of selecting of data ops and non-data ops.

Hence, this division in the op templates
struct as data_ops and ctrl_ops is required. Currently, the core only supports using a single protocol for
data ops, chosen at the time of probing.

2. If we use this single op_templates struct, I can't think of any good way to initialize these in the
manufacturers driver (winbond.c), refer to 17th patch in this series. Could you please suggest a macro
implementation also for winbond.c with the suggested op_templates struct.

Hi Apurva,

Quoted 113 lines · On Tue, 15 Feb 2022 21:03:52 +0530 Apurva Nandan<[email protected]> wrote:

Hi Boris,

Quoted 110 lines · Boris Brezillon

Quoted 28 lines · On Sat, 1 Jan 2022 13:12:38 +0530 Apurva Nandan<[email protected]> wrote:

'ctrl_ops' are op templates for non-page read/write operations, which are: reset, get_feature, set_feature, write_enable, block_erase, page_read and program_execute ops. The 'ctrl_ops' struct contains in it op templates for each of this op, as well as enum spinand_protocol denoting protocol of all these ops.

We require these new op templates because of deviation in standard SPINAND ops by manufacturers and also due to changes when there is a change in SPI protocol/mode. This prevents the core from live-patching and vendor-specific adjustments in ops.

Define 'ctrl_ops', add macros to initialize it and add it in spinand_device.

Signed-off-by Apurva Nandan<[email protected]>

---
   include/linux/mtd/spinand.h | 33 +++++++++++++++++++++++++++++++++
   1 file changed, 33 insertions(+)

include/linux/mtd/spinand.h +1 -0

index 439d8ce40e1d..e5df6220ec1e 100644
@@ -356,6 +356,35 @@ struct spinand_op_variants {
   			sizeof(struct spi_mem_op),			\
   	}
 
+struct spinand_ctrl_ops {

include/linux/mtd/spinand.h struct spinand_op_variants { · line 369

Quoted 10 lines

include/linux/mtd/spinand.h +10 -0

+	const struct {
+		struct spi_mem_op reset;
+		struct spi_mem_op get_feature;
+		struct spi_mem_op set_feature;
+		struct spi_mem_op write_enable;
+		struct spi_mem_op block_erase;
+		struct spi_mem_op page_read;
+		struct spi_mem_op program_execute;
+	} ops;
+	const enum spinand_protocol protocol;

Do you really need that protocol field?

Quoted 31 lines

include/linux/mtd/spinand.h +20 -0

+};
+
+#define SPINAND_CTRL_OPS(__protocol, __reset, __get_feature, __set_feature,	\
+			 __write_enable, __block_erase, __page_read,		\
+			 __program_execute)					\
+	{									\
+		.ops = {							\
+			.reset = __reset,					\
+			.get_feature = __get_feature,				\
+			.set_feature = __set_feature,				\
+			.write_enable = __write_enable,				\
+			.block_erase = __block_erase,				\
+			.page_read = __page_read,				\
+			.program_execute = __program_execute,			\
+		},								\
+		.protocol = __protocol,						\
+	}
+
   /**
    * spinand_ecc_info - description of the on-die ECC implemented by a SPI NAND
    *		      chip

@@ -468,6 +497,8 @@ struct spinand_dirmap {
    * @data_ops.read_cache: read cache op template
    * @data_ops.write_cache: write cache op template
    * @data_ops.update_cache: update cache op template
+ * @ctrl_ops: various SPI mem op templates for handling the flash device, i.e.
+ *	      non page-read/write ops.
    * @select_target: select a specific target/die. Usually called before sending
    *		   a command addressing a page or an eraseblock embedded in
    *		   this die. Only required if your chip exposes several dies

include/linux/mtd/spinand.h struct spinand_device { · line 533

Quoted 5 lines

include/linux/mtd/spinand.h +2 -0

@@ -498,6 +529,8 @@ struct spinand_device {
   		const struct spi_mem_op *update_cache;
   	} data_ops;
 
+	const struct spinand_ctrl_ops *ctrl_ops;
+

Okay, I had something slightly different in mind. First, I'd put all templates in a struct:

struct spinand_op_templates {
	const struct spi_mem_op *read_cache;
	const struct spi_mem_op *write_cache;
	const struct spi_mem_op *update_cache;
	const struct spi_mem_op *reset;
	const struct spi_mem_op *get_feature;
	const struct spi_mem_op *set_feature;
	const struct spi_mem_op *write_enable;
	const struct spi_mem_op *block_erase;
	const struct spi_mem_op *page_load;
	const struct spi_mem_op *program_execute;
};

Then, at the spinand level, I'd define an array of templates:

enum spinand_protocol {
	SPINAND_1S_1S_1S,
	SPINAND_2S_2S_2S,
	SPINAND_4S_4S_4S,
	SPINAND_8S_8S_8S,
	SPINAND_8D_8D_8D,
	SPINAND_NUM_PROTOCOLS,
};

struct spinand_device {
	...
	enum spinand_protocol protocol;
	const struct spinand_op_templates *op_templates[SPINAND_NUM_PROTOCOLS];

It should probably be

	struct spinand_op_templates op_templates[SPINAND_NUM_PROTOCOLS];

with the spinand_op_templates struct defined as:

struct spinand_op_templates {
struct spi_mem_op read_cache;
struct spi_mem_op write_cache;
struct spi_mem_op update_cache;
struct spi_mem_op reset;
struct spi_mem_op get_feature;
struct spi_mem_op set_feature;
struct spi_mem_op write_enable;
struct spi_mem_op block_erase;
struct spi_mem_op page_load;
struct spi_mem_op program_execute;
};

so the NAND framework can populate these ops.

Or maybe even better, define an enum that contains all the ops:

enum spinand_op_id {
SPI_NAND_OP_READ_CACHE,
SPI_NAND_OP_WRITE_CACHE,
SPI_NAND_OP_UPDATE_CACHE,
SPI_NAND_OP_RESET,
...
SPI_NAND_NUM_OPS,
};

struct spinand_device {
	...
  	enum spinand_protocol protocol;
  	struct spi_mem_op op_templates[SPINAND_NUM_PROTOCOLS][SPI_NAND_NUM_OPS];
	...
};

Quoted 13 lines

Quoted 6 lines

This way, you can easily pick the right set of operations based on the protocol/mode you're in:

#define spinand_get_op_template(spinand, opname) \
	((spinand)->op_templates[(spinand)->protocol]->opname)

Quoted 7 lines

static int spinand_read_reg_op(struct spinand_device *spinand, u8 reg, u8 *val)

{
	struct spi_mem_op op = *spinand_get_op_template(spinand, get_feature);
	int ret;

	...
}

Quoted 7 lines

I find a couple of issues with this  method,

1. read_cache, write_cache, update_cache op templates don't fit well with the other non-data ops, as
these data ops are used to create a dirmap, and that can be done only once at probe time. Hence, there
is a different mechanism of selecting of data ops and non-data ops.

Not sure I see why this is a problem. You can populate data-ops for all modes, and pick the one that provides the best perfs when you create the dirmap (which should really be at the end of the probe, if it's not already).

Quoted 4 lines

Hence, this division in the op templates
struct as data_ops and ctrl_ops is required. Currently, the core only supports using a single protocol for
data ops, chosen at the time of probing.

Again, I don't see why you need to differentiate the control and data ops when populating this table. Those are just operations the NAND supports, and the data operations is just a subset.

Quoted 5 lines

2. If we use this single op_templates struct, I can't think of any good way to initialize these in the
manufacturers driver (winbond.c), refer to 17th patch in this series. Could you please suggest a macro
implementation also for winbond.c with the suggested op_templates struct.

First replace the op_variants field by something more generic:

struct spinand_info {
...

	const struct spinand_op_variants **ops_variants;

...
};

#define SPINAND_OP_VARIANTS(_id, ...) \
	[SPI_NAND_OP_ ## _id] = { __VA_ARGS__ }

#define SPINAND_OPS_VARIANTS(name, ...)
	const struct spinand_op_variants name[]{
		__VA_ARGS__,
	};

#define SPINAND_INFO_OPS_VARIANTS(defs)
.ops_variants = defs

...

static SPINAND_OPS_VARIANTS(w35n01jw_ops_variants,

		SPINAND_OP_VARIANTS(READ_CACHE,
			SPINAND_PAGE_READ_FROM_CACHE_OCTALIO_DTR_OP(0, 24, NULL, 0),
			SPINAND_PAGE_READ_FROM_CACHE_OP(true, 0, 1, NULL, 0),
			...)),
		SPINAND_OP_VARIANTS(WRITE_CACHE,
			SPINAND_PROG_LOAD_OCTALIO_DTR(true, 0, NULL, 0),
			SPINAND_PROG_LOAD(true, 0, NULL, 0)),
		...
		SPINAND_OP_VARIANTS(RESET,
			SPINAND_RESET_OP_OCTAL_DTR,
			SPINAND_RESET_OP,
		...
		);

...

	SPINAND_INFO("W35N01JW",
		     SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0xdc),
		     NAND_MEMORG(1, 4096, 128, 64, 512, 20, 1, 1, 1),
		     NAND_ECCREQ(1, 512),
		     SPINAND_HAS_OCTAL_DTR_BIT | SPINAND_HAS_CR_FEAT_BIT,
		     SPINAND_ECCINFO(&w35n01jw_ooblayout, NULL),
		     SPINAND_INFO_OPS_VARIANTS(&w35n01jw_ops_variants)),

You also need to adjust spinand_match_and_init() to account for this new layout and put each template op in the right subset based on op.cmd.width and op.cmd.dtr.

Regards,

Boris

Quoted 58 lines

Quoted 5 lines

1. read_cache, write_cache, update_cache op templates don't fit well with the other non-data ops, as
these data ops are used to create a dirmap, and that can be done only once at probe time. Hence, there
is a different mechanism of selecting of data ops and non-data ops.

Not sure I see why this is a problem. You can populate data-ops for all modes, and pick the one that provides the best perfs when you create the dirmap (which should really be at the end of the probe, if it's not already).

Quoted 4 lines

Hence, this division in the op templates
struct as data_ops and ctrl_ops is required. Currently, the core only supports using a single protocol for
data ops, chosen at the time of probing.

Again, I don't see why you need to differentiate the control and data ops when populating this table. Those are just operations the NAND supports, and the data operations is just a subset.

Quoted 5 lines

2. If we use this single op_templates struct, I can't think of any good way to initialize these in the
manufacturers driver (winbond.c), refer to 17th patch in this series. Could you please suggest a macro
implementation also for winbond.c with the suggested op_templates struct.

First replace the op_variants field by something more generic:

struct spinand_info {
...

	const struct spinand_op_variants **ops_variants;

...
};

#define SPINAND_OP_VARIANTS(_id, ...) \
	[SPI_NAND_OP_ ## _id] = { __VA_ARGS__ }

#define SPINAND_OPS_VARIANTS(name, ...)
	const struct spinand_op_variants name[]{
		__VA_ARGS__,
	};

#define SPINAND_INFO_OPS_VARIANTS(defs)
.ops_variants = defs

...

static SPINAND_OPS_VARIANTS(w35n01jw_ops_variants,

		SPINAND_OP_VARIANTS(READ_CACHE,
			SPINAND_PAGE_READ_FROM_CACHE_OCTALIO_DTR_OP(0, 24, NULL, 0),
			SPINAND_PAGE_READ_FROM_CACHE_OP(true, 0, 1, NULL, 0),
			...)),
		SPINAND_OP_VARIANTS(WRITE_CACHE,
			SPINAND_PROG_LOAD_OCTALIO_DTR(true, 0, NULL, 0),
			SPINAND_PROG_LOAD(true, 0, NULL, 0)),
		...
		SPINAND_OP_VARIANTS(RESET,
			SPINAND_RESET_OP_OCTAL_DTR,
			SPINAND_RESET_OP,
		...
		);

Quoted 1 line

...

I find a issue with this implementation, please give corrective suggestions:

In type of op variant listing, there is no way to specify the protocol of the op in the variants struct itself.
    - This will lead to filtering/sorting/searching of ops for finding
the protocols in the spinand core
    while in spinand_match_and_init(), which I don't feel is a good way
for protocol based op categorization.

    - This would also lead to complexities in cases of mixed mode
operations.

    - In addition, we can't simply choose the first supported protocol
in each op id, as some ops have
    intendependency of protocol with other ops. This is because
non-data ops (like reset, block erase..)
    cannot be in different protocols at same time, so it would make
sense to have some form of protocol
    based arrangement while listing them.

Hi Apurva,

Quoted 113 lines · On Tue, 15 Feb 2022 21:03:52 +0530 Apurva Nandan <[email protected]> wrote:

Hi Boris,

Quoted 110 lines · Boris Brezillon

Quoted 28 lines · On Sat, 1 Jan 2022 13:12:38 +0530 Apurva Nandan <[email protected]> wrote:

'ctrl_ops' are op templates for non-page read/write operations, which are: reset, get_feature, set_feature, write_enable, block_erase, page_read and program_execute ops. The 'ctrl_ops' struct contains in it op templates for each of this op, as well as enum spinand_protocol denoting protocol of all these ops.

We require these new op templates because of deviation in standard SPINAND ops by manufacturers and also due to changes when there is a change in SPI protocol/mode. This prevents the core from live-patching and vendor-specific adjustments in ops.

Define 'ctrl_ops', add macros to initialize it and add it in spinand_device.

Signed-off-by Apurva Nandan <[email protected]>

---
   include/linux/mtd/spinand.h | 33 +++++++++++++++++++++++++++++++++
   1 file changed, 33 insertions(+)

include/linux/mtd/spinand.h +1 -0

index 439d8ce40e1d..e5df6220ec1e 100644
@@ -356,6 +356,35 @@ struct spinand_op_variants {
   			sizeof(struct spi_mem_op),			\
   	}
 
+struct spinand_ctrl_ops {

include/linux/mtd/spinand.h struct spinand_op_variants { · line 369

Quoted 10 lines

include/linux/mtd/spinand.h +10 -0

+	const struct {
+		struct spi_mem_op reset;
+		struct spi_mem_op get_feature;
+		struct spi_mem_op set_feature;
+		struct spi_mem_op write_enable;
+		struct spi_mem_op block_erase;
+		struct spi_mem_op page_read;
+		struct spi_mem_op program_execute;
+	} ops;
+	const enum spinand_protocol protocol;

Do you really need that protocol field?

Quoted 31 lines

include/linux/mtd/spinand.h +20 -0

+};
+
+#define SPINAND_CTRL_OPS(__protocol, __reset, __get_feature, __set_feature,	\
+			 __write_enable, __block_erase, __page_read,		\
+			 __program_execute)					\
+	{									\
+		.ops = {							\
+			.reset = __reset,					\
+			.get_feature = __get_feature,				\
+			.set_feature = __set_feature,				\
+			.write_enable = __write_enable,				\
+			.block_erase = __block_erase,				\
+			.page_read = __page_read,				\
+			.program_execute = __program_execute,			\
+		},								\
+		.protocol = __protocol,						\
+	}
+
   /**
    * spinand_ecc_info - description of the on-die ECC implemented by a SPI NAND
    *		      chip

@@ -468,6 +497,8 @@ struct spinand_dirmap {
    * @data_ops.read_cache: read cache op template
    * @data_ops.write_cache: write cache op template
    * @data_ops.update_cache: update cache op template
+ * @ctrl_ops: various SPI mem op templates for handling the flash device, i.e.
+ *	      non page-read/write ops.
    * @select_target: select a specific target/die. Usually called before sending
    *		   a command addressing a page or an eraseblock embedded in
    *		   this die. Only required if your chip exposes several dies

include/linux/mtd/spinand.h struct spinand_device { · line 533

Quoted 5 lines

include/linux/mtd/spinand.h +2 -0

@@ -498,6 +529,8 @@ struct spinand_device {
   		const struct spi_mem_op *update_cache;
   	} data_ops;
 
+	const struct spinand_ctrl_ops *ctrl_ops;
+

Okay, I had something slightly different in mind. First, I'd put all templates in a struct:

struct spinand_op_templates {
	const struct spi_mem_op *read_cache;
	const struct spi_mem_op *write_cache;
	const struct spi_mem_op *update_cache;
	const struct spi_mem_op *reset;
	const struct spi_mem_op *get_feature;
	const struct spi_mem_op *set_feature;
	const struct spi_mem_op *write_enable;
	const struct spi_mem_op *block_erase;
	const struct spi_mem_op *page_load;
	const struct spi_mem_op *program_execute;
};

Then, at the spinand level, I'd define an array of templates:

enum spinand_protocol {
	SPINAND_1S_1S_1S,
	SPINAND_2S_2S_2S,
	SPINAND_4S_4S_4S,
	SPINAND_8S_8S_8S,
	SPINAND_8D_8D_8D,
	SPINAND_NUM_PROTOCOLS,
};

struct spinand_device {
	...
	enum spinand_protocol protocol;
	const struct spinand_op_templates *op_templates[SPINAND_NUM_PROTOCOLS];

It should probably be

	struct spinand_op_templates op_templates[SPINAND_NUM_PROTOCOLS];

with the spinand_op_templates struct defined as:

struct spinand_op_templates {
struct spi_mem_op read_cache;
struct spi_mem_op write_cache;
struct spi_mem_op update_cache;
struct spi_mem_op reset;
struct spi_mem_op get_feature;
struct spi_mem_op set_feature;
struct spi_mem_op write_enable;
struct spi_mem_op block_erase;
struct spi_mem_op page_load;
struct spi_mem_op program_execute;
};

so the NAND framework can populate these ops.

Or maybe even better, define an enum that contains all the ops:

enum spinand_op_id {
SPI_NAND_OP_READ_CACHE,
SPI_NAND_OP_WRITE_CACHE,
SPI_NAND_OP_UPDATE_CACHE,
SPI_NAND_OP_RESET,
...
SPI_NAND_NUM_OPS,
};

struct spinand_device {
	...
  	enum spinand_protocol protocol;
  	struct spi_mem_op op_templates[SPINAND_NUM_PROTOCOLS][SPI_NAND_NUM_OPS];
	...
};

Quoted 13 lines

Quoted 6 lines

This way, you can easily pick the right set of operations based on the protocol/mode you're in:

#define spinand_get_op_template(spinand, opname) \
	((spinand)->op_templates[(spinand)->protocol]->opname)

Quoted 7 lines

static int spinand_read_reg_op(struct spinand_device *spinand, u8 reg, u8 *val)

{
	struct spi_mem_op op = *spinand_get_op_template(spinand, get_feature);
	int ret;

	...
}

Quoted 7 lines

I find a couple of issues with this  method,

1. read_cache, write_cache, update_cache op templates don't fit well with the other non-data ops, as
these data ops are used to create a dirmap, and that can be done only once at probe time. Hence, there
is a different mechanism of selecting of data ops and non-data ops.

Not sure I see why this is a problem. You can populate data-ops for all modes, and pick the one that provides the best perfs when you create the dirmap (which should really be at the end of the probe, if it's not already).

Quoted 4 lines

Hence, this division in the op templates
struct as data_ops and ctrl_ops is required. Currently, the core only supports using a single protocol for
data ops, chosen at the time of probing.

Again, I don't see why you need to differentiate the control and data ops when populating this table. Those are just operations the NAND supports, and the data operations is just a subset.

Quoted 5 lines

2. If we use this single op_templates struct, I can't think of any good way to initialize these in the
manufacturers driver (winbond.c), refer to 17th patch in this series. Could you please suggest a macro
implementation also for winbond.c with the suggested op_templates struct.

First replace the op_variants field by something more generic:

struct spinand_info {
...

	const struct spinand_op_variants **ops_variants;

...
};

#define SPINAND_OP_VARIANTS(_id, ...) \
	[SPI_NAND_OP_ ## _id] = { __VA_ARGS__ }

#define SPINAND_OPS_VARIANTS(name, ...)
	const struct spinand_op_variants name[]{
		__VA_ARGS__,
	};

#define SPINAND_INFO_OPS_VARIANTS(defs)
.ops_variants = defs

...

static SPINAND_OPS_VARIANTS(w35n01jw_ops_variants,

		SPINAND_OP_VARIANTS(READ_CACHE,
			SPINAND_PAGE_READ_FROM_CACHE_OCTALIO_DTR_OP(0, 24, NULL, 0),
			SPINAND_PAGE_READ_FROM_CACHE_OP(true, 0, 1, NULL, 0),
			...)),
		SPINAND_OP_VARIANTS(WRITE_CACHE,
			SPINAND_PROG_LOAD_OCTALIO_DTR(true, 0, NULL, 0),
			SPINAND_PROG_LOAD(true, 0, NULL, 0)),
		...
		SPINAND_OP_VARIANTS(RESET,
			SPINAND_RESET_OP_OCTAL_DTR,
			SPINAND_RESET_OP,
		...
		);

...

	SPINAND_INFO("W35N01JW",
		     SPINAND_ID(SPINAND_READID_METHOD_OPCODE_DUMMY, 0xdc),
		     NAND_MEMORG(1, 4096, 128, 64, 512, 20, 1, 1, 1),
		     NAND_ECCREQ(1, 512),
		     SPINAND_HAS_OCTAL_DTR_BIT | SPINAND_HAS_CR_FEAT_BIT,
		     SPINAND_ECCINFO(&w35n01jw_ooblayout, NULL),
		     SPINAND_INFO_OPS_VARIANTS(&w35n01jw_ops_variants)),

You also need to adjust spinand_match_and_init() to account for this new layout and put each template op in the right subset based on op.cmd.width and op.cmd.dtr.

Regards,

Boris

Quoted 49 lines

Quoted 13 lines

Quoted 6 lines

This way, you can easily pick the right set of operations based on the protocol/mode you're in:

#define spinand_get_op_template(spinand, opname) \
	((spinand)->op_templates[(spinand)->protocol]->opname)

Quoted 7 lines

static int spinand_read_reg_op(struct spinand_device *spinand, u8 reg, u8 *val)

{
	struct spi_mem_op op = *spinand_get_op_template(spinand, get_feature);
	int ret;

	...
}  

Quoted 7 lines

I find a couple of issues with this  method,

1. read_cache, write_cache, update_cache op templates don't fit well with the other non-data ops, as
these data ops are used to create a dirmap, and that can be done only once at probe time. Hence, there
is a different mechanism of selecting of data ops and non-data ops.

Not sure I see why this is a problem. You can populate data-ops for all modes, and pick the one that provides the best perfs when you create the dirmap (which should really be at the end of the probe, if it's not already).

Quoted 4 lines

Hence, this division in the op templates
struct as data_ops and ctrl_ops is required. Currently, the core only supports using a single protocol for
data ops, chosen at the time of probing.

Again, I don't see why you need to differentiate the control and data ops when populating this table. Those are just operations the NAND supports, and the data operations is just a subset.

Quoted 5 lines

2. If we use this single op_templates struct, I can't think of any good way to initialize these in the
manufacturers driver (winbond.c), refer to 17th patch in this series. Could you please suggest a macro
implementation also for winbond.c with the suggested op_templates struct.

First replace the op_variants field by something more generic:

struct spinand_info {
...

	const struct spinand_op_variants **ops_variants;

...
};

#define SPINAND_OP_VARIANTS(_id, ...) \
	[SPI_NAND_OP_ ## _id] = { __VA_ARGS__ }

Quoted 7 lines

#define SPINAND_OPS_VARIANTS(name, ...)
	const struct spinand_op_variants name[]{
		__VA_ARGS__,
	};

#define SPINAND_INFO_OPS_VARIANTS(defs)
.ops_variants = defs

If we modify these macros, it would require other spinand vendor drivers to change (toshiba, micron, etc).
The older macros suit them well, should we go about changing them to this new macro (will require re-testing all of them),
or can we keep them unchanged and have new set of macros with different name (please give suggestion for it) for op variants.

Quoted 54 lines · On Thu, 10 Mar 2022 13:27:06 +0530 Apurva Nandan <[email protected]> wrote:

Quoted 54 lines

Quoted 13 lines

Quoted 6 lines

This way, you can easily pick the right set of operations based on the protocol/mode you're in:

#define spinand_get_op_template(spinand, opname) \
	((spinand)->op_templates[(spinand)->protocol]->opname)

Quoted 7 lines

static int spinand_read_reg_op(struct spinand_device *spinand, u8 reg, u8 *val)

{
	struct spi_mem_op op = *spinand_get_op_template(spinand, get_feature);
	int ret;

	...
}

Quoted 7 lines

I find a couple of issues with this  method,

1. read_cache, write_cache, update_cache op templates don't fit well with the other non-data ops, as
these data ops are used to create a dirmap, and that can be done only once at probe time. Hence, there
is a different mechanism of selecting of data ops and non-data ops.

Not sure I see why this is a problem. You can populate data-ops for all modes, and pick the one that provides the best perfs when you create the dirmap (which should really be at the end of the probe, if it's not already).

Quoted 4 lines

Hence, this division in the op templates
struct as data_ops and ctrl_ops is required. Currently, the core only supports using a single protocol for
data ops, chosen at the time of probing.

Again, I don't see why you need to differentiate the control and data ops when populating this table. Those are just operations the NAND supports, and the data operations is just a subset.

Quoted 5 lines

2. If we use this single op_templates struct, I can't think of any good way to initialize these in the
manufacturers driver (winbond.c), refer to 17th patch in this series. Could you please suggest a macro
implementation also for winbond.c with the suggested op_templates struct.

First replace the op_variants field by something more generic:

struct spinand_info {
...

	const struct spinand_op_variants **ops_variants;

...
};

#define SPINAND_OP_VARIANTS(_id, ...) \
	[SPI_NAND_OP_ ## _id] = { __VA_ARGS__ }

#define SPINAND_OPS_VARIANTS(name, ...)
	const struct spinand_op_variants name[]{
		__VA_ARGS__,
	};

Quoted 8 lines

Quoted 2 lines

#define SPINAND_INFO_OPS_VARIANTS(defs)
.ops_variants = defs

If we modify these macros, it would require other spinand vendor drivers to change (toshiba, micron, etc).
The older macros suit them well, should we go about changing them to this new macro (will require re-testing all of them),
or can we keep them unchanged and have new set of macros with different name (please give suggestion for it) for op variants.

I'd rather have everything converted to the new approach (we don't want 2 ways of describing the same thing), and I'm not sure you can make the old macros map to the new solution, so I fear you'll have to patch all vendors. This being said, I'm fine providing simple wrappers if that helps, but I don't see how they'd make the description simpler/more compact to be honest.

Introduce change_mode() manufacturer_op to let the vendor provide the implementation of switching of SPI IO modes.

The method to switch to different SPI IO mode may vary across manufacturers. For example, for Winbond, Octal DTR is enabled by writing values to the volatile configuration register. So, let the manufacturer's code have their own implementation for switching to any given SPI IO mode. Manufacturer's code need to take care, if the requested protocol change is allowed/needed and how to apply it.

Signed-off-by Apurva Nandan <[email protected]>

---
 include/linux/mtd/spinand.h | 3 +++
 1 file changed, 3 insertions(+)

include/linux/mtd/spinand.h +1 -0

index 5dae0649f2fb..ad924271a248 100644
@@ -298,6 +298,7 @@ struct spinand_devid {
 /**
  * struct manufacurer_ops - SPI NAND manufacturer specific operations
  * @init: initialize a SPI NAND device
+ * @change_mode: switch the SPI NAND flash to a specific SPI protocol
  * @cleanup: cleanup a SPI NAND device
  *
  * Each SPI NAND manufacturer driver should implement this interface so that

include/linux/mtd/spinand.h +0 -0

 	void (*cleanup)(struct spinand_device *spinand);
 };
 

Enable Octal DTR SPI mode, i.e. 8D-8D-8D mode, if the SPI NAND flash device supports it. Mixed OSPI (1S-1S-8S & 1S-8S-8S), mixed DTR modes (1S-1D-8D), etc. aren't supported yet.

The method to switch to Octal DTR SPI mode may vary across manufacturers. For example, for Winbond, it is enabled by writing values to the volatile configuration register. So, let the manufacturer's code have their own implementation for switching to Octal DTR SPI mode.

Check for the SPI NAND device's support for Octal DTR mode using spinand flags, and if the data_ops and ctrl_ops are 8D-8D-8D, call change_mode() manufacturer op. If the SPI controller doesn't supports these modes, the selected data_ops and ctrl_ops will prevent switching to the Octal DTR mode. And finally update the spinand protocol and ctrl_ops on success. Similarly, for disabling Octal DTR mode, call change_mode(), and update protocol and ctrl_ops.

Signed-off-by Apurva Nandan <[email protected]>

---
 drivers/mtd/nand/spi/core.c | 79 +++++++++++++++++++++++++++++++++++++
 include/linux/mtd/spinand.h |  1 +
 2 files changed, 80 insertions(+)

drivers/mtd/nand/spi/core.c +22 -0

index 1a602e4dd6bd..2fd08085db6f 100644
@@ -1067,6 +1067,81 @@ spinand_select_ctrl_ops_variant(struct spinand_device *spinand,
 	return NULL;
 }
 
+static bool spinand_op_is_octal_dtr(const struct spi_mem_op *op)
+{
+	return  op->cmd.buswidth == 8 && op->cmd.dtr &&
+		op->addr.buswidth == 8 && op->addr.dtr &&
+		op->data.buswidth == 8 && op->data.dtr;
+}
+
+static int spinand_init_octal_dtr_enable(struct spinand_device *spinand)
+{
+	struct device *dev = &spinand->spimem->spi->dev;
+	const struct spinand_ctrl_ops *octal_dtr_ctrl_ops;
+	int ret;
+
+	if (!(spinand->flags & SPINAND_HAS_OCTAL_DTR_BIT))
+		return 0;
+
+	if (!(spinand_op_is_octal_dtr(spinand->data_ops.read_cache) &&
+	      spinand_op_is_octal_dtr(spinand->data_ops.write_cache) &&
+	      spinand_op_is_octal_dtr(spinand->data_ops.update_cache)))
+		return 0;
+
+	octal_dtr_ctrl_ops = spinand_select_ctrl_ops_variant(spinand,

drivers/mtd/nand/spi/core.c +11 -0

+	}
+
+	ret = spinand->manufacturer->ops->change_mode(spinand,
+						      SPINAND_8D_8D_8D);
+	if (ret) {
+		dev_err(dev,
+			"Failed to enable Octal DTR SPI mode (err = %d)\n",
+			ret);
+		return ret;
+	}
+

drivers/mtd/nand/spi/core.c +26 -0

+{
+	struct device *dev = &spinand->spimem->spi->dev;
+	int ret;
+
+	if (!spinand->manufacturer->ops->change_mode)
+		return -EINVAL;
+
+	ret = spinand->manufacturer->ops->change_mode(spinand,
+						      SPINAND_1S_1S_1S);
+
+	if (ret) {
+		dev_err(dev,
+			"Failed to disable Octal DTR SPI mode (err = %d)\n",
+			ret);
+		return ret;
+	}
+
+	spinand->protocol = SPINAND_1S_1S_1S;
+	spinand->ctrl_ops = &spinand_default_ctrl_ops;
+	return 0;
+}
+
 /**
  * spinand_match_and_init() - Try to find a match between a device ID and an
  *			      entry in a spinand_info table

@@ -1203,6 +1278,10 @@ static int spinand_init_flash(struct spinand_device *spinand)
 			break;
 	}
 
+	ret = spinand_init_octal_dtr_enable(spinand);
+	if (ret)
+		return ret;
+
 	if (ret)
 		spinand_manufacturer_cleanup(spinand);
 

include/linux/mtd/spinand.h +0 -0

index a8c071983a27..f12aa4516fab 100644

include/linux/mtd/spinand.h +0 -0

 /**
  * struct spinand_ondie_ecc_conf - private SPI-NAND on-die ECC engine structure

The flash might or might not have gone a power-down during sleep. Hence, its SPI IO mode is unpredictable for the core at the time of resume. To ensure proper reinitialization during resume, disable the Octal DTR SPI IO mode and bring the flash to 1S-1S-1S mode when performing suspend using mtd_suspend().

Signed-off-by Apurva Nandan <[email protected]>

---
 drivers/mtd/nand/spi/core.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

drivers/mtd/nand/spi/core.c +0 -0

index 2fd08085db6f..9d1c72634e5a 100644

drivers/mtd/nand/spi/core.c +5 -0

+
+	return 0;
+}
+
 static int spinand_init(struct spinand_device *spinand)
 {
 	struct device *dev = &spinand->spimem->spi->dev;

@@ -1381,6 +1391,7 @@ static int spinand_init(struct spinand_device *spinand)
 	mtd->_erase = spinand_mtd_erase;
 	mtd->_max_bad_blocks = nanddev_mtd_max_bad_blocks;
 	mtd->_resume = spinand_mtd_resume;
+	mtd->_suspend = spinand_mtd_suspend;
 
 	if (nand->ecc.engine) {
 		ret = mtd_ooblayout_count_freebytes(mtd);

Add implementation of change_mode() for Winbond's manufacturer_ops, that executes octal_dtr_enable() and octal_dtr_disable() according to requested protocol.

Datasheet: https://www.winbond.com/export/sites/winbond/datasheet/W35N01JW_Datasheet_Brief.pdf

Signed-off-by Apurva Nandan <[email protected]>

---
 drivers/mtd/nand/spi/winbond.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

drivers/mtd/nand/spi/winbond.c +12 -0

index c7478faf6cee..d8eccb40c80f 100644
@@ -242,8 +242,33 @@ static int winbond_spinand_octal_dtr_disable(struct spinand_device *spinand)
 	return 0;
 }
 
+static int winbond_change_spi_mode(struct spinand_device *spinand,
+				   const enum spinand_protocol protocol)
+{
+	if (spinand->protocol == protocol)
+		return 0;
+
+	switch (spinand->protocol) {
+	case SPINAND_1S_1S_1S:
+		if (protocol == SPINAND_8D_8D_8D)
+			return winbond_spinand_octal_dtr_enable(spinand);
+		break;
+

drivers/mtd/nand/spi/winbond.c +3 -0

+}
+
 static const struct spinand_manufacturer_ops winbond_spinand_manuf_ops = {
 	.init = winbond_spinand_init,
+	.change_mode = winbond_change_spi_mode,
 };
 
 const struct spinand_manufacturer winbond_spinand_manufacturer = {