[PATCH 1/3] mtd: spi-nor: Add support for Octal SPI mode.

[PATCH 1/3] mtd: spi-nor: Add support for Octal SPI mode.

[Artur Jedrysek]

This patch adds support for Octal SPI data reads in SPI NOR framework.
Opcodes for programming using octal interface are also present for the
sake of completeness, despite not being used.

Micron mt35xu512 flash is added as an example chip supporting that mode.

Signed-off-by: Artur Jedrysek <jartur@cadence.com>
---
 drivers/mtd/spi-nor/spi-nor.c | 20 ++++++++++++++++++--
 include/linux/mtd/spi-nor.h   |  9 +++++++++
 2 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 1ae872b..db188e2 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -85,6 +85,7 @@ struct flash_info {
 					 * Use dedicated 4byte address op codes
 					 * to support memory size above 128Mib.
 					 */
+#define SPI_NOR_OCTAL_READ	BIT(12)	/* Flash supports Octal Read */
 };
 
 #define JEDEC_MFR(info)	((info)->id[0])
@@ -159,6 +160,7 @@ static inline int spi_nor_read_dummy_cycles(struct spi_nor *nor)
 	case SPI_NOR_FAST:
 	case SPI_NOR_DUAL:
 	case SPI_NOR_QUAD:
+	case SPI_NOR_OCTAL:
 		return 8;
 	case SPI_NOR_NORMAL:
 		return 0;
@@ -220,6 +222,8 @@ static inline u8 spi_nor_convert_3to4_read(u8 opcode)
 		{ SPINOR_OP_READ_1_2_2,	SPINOR_OP_READ_1_2_2_4B },
 		{ SPINOR_OP_READ_1_1_4,	SPINOR_OP_READ_1_1_4_4B },
 		{ SPINOR_OP_READ_1_4_4,	SPINOR_OP_READ_1_4_4_4B },
+		{ SPINOR_OP_READ_1_1_8,	SPINOR_OP_READ_1_1_8_4B },
+		{ SPINOR_OP_READ_1_8_8,	SPINOR_OP_READ_1_8_8_4B },
 	};
 
 	return spi_nor_convert_opcode(opcode, spi_nor_3to4_read,
@@ -232,6 +236,8 @@ static inline u8 spi_nor_convert_3to4_program(u8 opcode)
 		{ SPINOR_OP_PP,		SPINOR_OP_PP_4B },
 		{ SPINOR_OP_PP_1_1_4,	SPINOR_OP_PP_1_1_4_4B },
 		{ SPINOR_OP_PP_1_4_4,	SPINOR_OP_PP_1_4_4_4B },
+		{ SPINOR_OP_PP_1_1_8,	SPINOR_OP_PP_1_1_8_4B },
+		{ SPINOR_OP_PP_1_8_8,	SPINOR_OP_PP_1_8_8_4B },
 	};
 
 	return spi_nor_convert_opcode(opcode, spi_nor_3to4_program,
@@ -1035,6 +1041,11 @@ static const struct flash_info spi_nor_ids[] = {
 	{ "n25q512ax3",  INFO(0x20ba20, 0, 64 * 1024, 1024, SECT_4K | USE_FSR | SPI_NOR_QUAD_READ) },
 	{ "n25q00",      INFO(0x20ba21, 0, 64 * 1024, 2048, SECT_4K | USE_FSR | SPI_NOR_QUAD_READ) },
 	{ "n25q00a",     INFO(0x20bb21, 0, 64 * 1024, 2048, SECT_4K | USE_FSR | SPI_NOR_QUAD_READ) },
+	{
+		"mt35xu512", INFO(0x2c5b1a, 0, 128 * 1024, 512,
+			SECT_4K | USE_FSR | SPI_NOR_OCTAL_READ |
+			SPI_NOR_4B_OPCODES)
+	},
 
 	/* PMC */
 	{ "pm25lv512",   INFO(0,        0, 32 * 1024,    2, SECT_4K_PMC) },
@@ -1667,8 +1678,10 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
 	if (info->flags & SPI_NOR_NO_FR)
 		nor->flash_read = SPI_NOR_NORMAL;
 
-	/* Quad/Dual-read mode takes precedence over fast/normal */
-	if (mode == SPI_NOR_QUAD && info->flags & SPI_NOR_QUAD_READ) {
+	/* Octal/Quad/Dual-read mode takes precedence over fast/normal */
+	if (mode == SPI_NOR_OCTAL && info->flags & SPI_NOR_OCTAL_READ) {
+		nor->flash_read = SPI_NOR_OCTAL;
+	} else if (mode == SPI_NOR_QUAD && info->flags & SPI_NOR_QUAD_READ) {
 		ret = set_quad_mode(nor, info);
 		if (ret) {
 			dev_err(dev, "quad mode not supported\n");
@@ -1681,6 +1694,9 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
 
 	/* Default commands */
 	switch (nor->flash_read) {
+	case SPI_NOR_OCTAL:
+		nor->read_opcode = SPINOR_OP_READ_1_1_8;
+		break;
 	case SPI_NOR_QUAD:
 		nor->read_opcode = SPINOR_OP_READ_1_1_4;
 		break;
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index f2a7180..aad6318 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -47,9 +47,13 @@
 #define SPINOR_OP_READ_1_2_2	0xbb	/* Read data bytes (Dual I/O SPI) */
 #define SPINOR_OP_READ_1_1_4	0x6b	/* Read data bytes (Quad Output SPI) */
 #define SPINOR_OP_READ_1_4_4	0xeb	/* Read data bytes (Quad I/O SPI) */
+#define SPINOR_OP_READ_1_1_8	0x8b	/* Read data bytes (Octal Output SPI) */
+#define SPINOR_OP_READ_1_8_8	0xcb	/* Read data bytes (Octal I/O SPI) */
 #define SPINOR_OP_PP		0x02	/* Page program (up to 256 bytes) */
 #define SPINOR_OP_PP_1_1_4	0x32	/* Quad page program */
 #define SPINOR_OP_PP_1_4_4	0x38	/* Quad page program */
+#define SPINOR_OP_PP_1_1_8	0x82	/* Octal page program */
+#define SPINOR_OP_PP_1_8_8	0xc2	/* Octal page program */
 #define SPINOR_OP_BE_4K		0x20	/* Erase 4KiB block */
 #define SPINOR_OP_BE_4K_PMC	0xd7	/* Erase 4KiB block on PMC chips */
 #define SPINOR_OP_BE_32K	0x52	/* Erase 32KiB block */
@@ -66,9 +70,13 @@
 #define SPINOR_OP_READ_1_2_2_4B	0xbc	/* Read data bytes (Dual I/O SPI) */
 #define SPINOR_OP_READ_1_1_4_4B	0x6c	/* Read data bytes (Quad Output SPI) */
 #define SPINOR_OP_READ_1_4_4_4B	0xec	/* Read data bytes (Quad I/O SPI) */
+#define SPINOR_OP_READ_1_1_8_4B	0x7c	/* Read data bytes (Octal Output SPI) */
+#define SPINOR_OP_READ_1_8_8_4B	0xcc	/* Read data bytes (Octal I/O SPI) */
 #define SPINOR_OP_PP_4B		0x12	/* Page program (up to 256 bytes) */
 #define SPINOR_OP_PP_1_1_4_4B	0x34	/* Quad page program */
 #define SPINOR_OP_PP_1_4_4_4B	0x3e	/* Quad page program */
+#define SPINOR_OP_PP_1_1_8_4B	0x84	/* Octal page program */
+#define SPINOR_OP_PP_1_8_8_4B	0x8e	/* Octal page program */
 #define SPINOR_OP_BE_4K_4B	0x21	/* Erase 4KiB block */
 #define SPINOR_OP_BE_32K_4B	0x5c	/* Erase 32KiB block */
 #define SPINOR_OP_SE_4B		0xdc	/* Sector erase (usually 64KiB) */
@@ -124,6 +132,7 @@ enum read_mode {
 	SPI_NOR_FAST,
 	SPI_NOR_DUAL,
 	SPI_NOR_QUAD,
+	SPI_NOR_OCTAL,
 };
 
 #define SPI_NOR_MAX_CMD_SIZE	8
-- 
2.2.2

Re: [PATCH 1/3] mtd: spi-nor: Add support for Octal SPI mode.

[Boris Brezillon]

Hi Artur,

Can you please make sure all patches of a patch series are part of the
same thread? git send-email should take care of that for you.

On Mon, 6 Mar 2017 12:30:23 +0000
Artur Jedrysek <jartur@cadence.com> wrote:

> This patch adds support for Octal SPI data reads in SPI NOR framework.
> Opcodes for programming using octal interface are also present for the
> sake of completeness, despite not being used.
> 
> Micron mt35xu512 flash is added as an example chip supporting that mode.
> 
> Signed-off-by: Artur Jedrysek <jartur@cadence.com>
> ---
>  drivers/mtd/spi-nor/spi-nor.c | 20 ++++++++++++++++++--
>  include/linux/mtd/spi-nor.h   |  9 +++++++++
>  2 files changed, 27 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 1ae872b..db188e2 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -85,6 +85,7 @@ struct flash_info {
>  					 * Use dedicated 4byte address op codes
>  					 * to support memory size above 128Mib.
>  					 */
> +#define SPI_NOR_OCTAL_READ	BIT(12)	/* Flash supports Octal Read */
>  };
>  
>  #define JEDEC_MFR(info)	((info)->id[0])
> @@ -159,6 +160,7 @@ static inline int spi_nor_read_dummy_cycles(struct spi_nor *nor)
>  	case SPI_NOR_FAST:
>  	case SPI_NOR_DUAL:
>  	case SPI_NOR_QUAD:
> +	case SPI_NOR_OCTAL:
>  		return 8;
>  	case SPI_NOR_NORMAL:
>  		return 0;
> @@ -220,6 +222,8 @@ static inline u8 spi_nor_convert_3to4_read(u8 opcode)
>  		{ SPINOR_OP_READ_1_2_2,	SPINOR_OP_READ_1_2_2_4B },
>  		{ SPINOR_OP_READ_1_1_4,	SPINOR_OP_READ_1_1_4_4B },
>  		{ SPINOR_OP_READ_1_4_4,	SPINOR_OP_READ_1_4_4_4B },
> +		{ SPINOR_OP_READ_1_1_8,	SPINOR_OP_READ_1_1_8_4B },
> +		{ SPINOR_OP_READ_1_8_8,	SPINOR_OP_READ_1_8_8_4B },
>  	};
>  
>  	return spi_nor_convert_opcode(opcode, spi_nor_3to4_read,
> @@ -232,6 +236,8 @@ static inline u8 spi_nor_convert_3to4_program(u8 opcode)
>  		{ SPINOR_OP_PP,		SPINOR_OP_PP_4B },
>  		{ SPINOR_OP_PP_1_1_4,	SPINOR_OP_PP_1_1_4_4B },
>  		{ SPINOR_OP_PP_1_4_4,	SPINOR_OP_PP_1_4_4_4B },
> +		{ SPINOR_OP_PP_1_1_8,	SPINOR_OP_PP_1_1_8_4B },
> +		{ SPINOR_OP_PP_1_8_8,	SPINOR_OP_PP_1_8_8_4B },
>  	};
>  
>  	return spi_nor_convert_opcode(opcode, spi_nor_3to4_program,
> @@ -1035,6 +1041,11 @@ static const struct flash_info spi_nor_ids[] = {
>  	{ "n25q512ax3",  INFO(0x20ba20, 0, 64 * 1024, 1024, SECT_4K | USE_FSR | SPI_NOR_QUAD_READ) },
>  	{ "n25q00",      INFO(0x20ba21, 0, 64 * 1024, 2048, SECT_4K | USE_FSR | SPI_NOR_QUAD_READ) },
>  	{ "n25q00a",     INFO(0x20bb21, 0, 64 * 1024, 2048, SECT_4K | USE_FSR | SPI_NOR_QUAD_READ) },
> +	{
> +		"mt35xu512", INFO(0x2c5b1a, 0, 128 * 1024, 512,
> +			SECT_4K | USE_FSR | SPI_NOR_OCTAL_READ |
> +			SPI_NOR_4B_OPCODES)
> +	},
>  
>  	/* PMC */
>  	{ "pm25lv512",   INFO(0,        0, 32 * 1024,    2, SECT_4K_PMC) },
> @@ -1667,8 +1678,10 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
>  	if (info->flags & SPI_NOR_NO_FR)
>  		nor->flash_read = SPI_NOR_NORMAL;
>  
> -	/* Quad/Dual-read mode takes precedence over fast/normal */
> -	if (mode == SPI_NOR_QUAD && info->flags & SPI_NOR_QUAD_READ) {
> +	/* Octal/Quad/Dual-read mode takes precedence over fast/normal */
> +	if (mode == SPI_NOR_OCTAL && info->flags & SPI_NOR_OCTAL_READ) {
> +		nor->flash_read = SPI_NOR_OCTAL;
> +	} else if (mode == SPI_NOR_QUAD && info->flags & SPI_NOR_QUAD_READ) {
>  		ret = set_quad_mode(nor, info);
>  		if (ret) {
>  			dev_err(dev, "quad mode not supported\n");
> @@ -1681,6 +1694,9 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
>  
>  	/* Default commands */
>  	switch (nor->flash_read) {
> +	case SPI_NOR_OCTAL:
> +		nor->read_opcode = SPINOR_OP_READ_1_1_8;
> +		break;
>  	case SPI_NOR_QUAD:
>  		nor->read_opcode = SPINOR_OP_READ_1_1_4;
>  		break;
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index f2a7180..aad6318 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -47,9 +47,13 @@
>  #define SPINOR_OP_READ_1_2_2	0xbb	/* Read data bytes (Dual I/O SPI) */
>  #define SPINOR_OP_READ_1_1_4	0x6b	/* Read data bytes (Quad Output SPI) */
>  #define SPINOR_OP_READ_1_4_4	0xeb	/* Read data bytes (Quad I/O SPI) */
> +#define SPINOR_OP_READ_1_1_8	0x8b	/* Read data bytes (Octal Output SPI) */
> +#define SPINOR_OP_READ_1_8_8	0xcb	/* Read data bytes (Octal I/O SPI) */
>  #define SPINOR_OP_PP		0x02	/* Page program (up to 256 bytes) */
>  #define SPINOR_OP_PP_1_1_4	0x32	/* Quad page program */
>  #define SPINOR_OP_PP_1_4_4	0x38	/* Quad page program */
> +#define SPINOR_OP_PP_1_1_8	0x82	/* Octal page program */
> +#define SPINOR_OP_PP_1_8_8	0xc2	/* Octal page program */
>  #define SPINOR_OP_BE_4K		0x20	/* Erase 4KiB block */
>  #define SPINOR_OP_BE_4K_PMC	0xd7	/* Erase 4KiB block on PMC chips */
>  #define SPINOR_OP_BE_32K	0x52	/* Erase 32KiB block */
> @@ -66,9 +70,13 @@
>  #define SPINOR_OP_READ_1_2_2_4B	0xbc	/* Read data bytes (Dual I/O SPI) */
>  #define SPINOR_OP_READ_1_1_4_4B	0x6c	/* Read data bytes (Quad Output SPI) */
>  #define SPINOR_OP_READ_1_4_4_4B	0xec	/* Read data bytes (Quad I/O SPI) */
> +#define SPINOR_OP_READ_1_1_8_4B	0x7c	/* Read data bytes (Octal Output SPI) */
> +#define SPINOR_OP_READ_1_8_8_4B	0xcc	/* Read data bytes (Octal I/O SPI) */
>  #define SPINOR_OP_PP_4B		0x12	/* Page program (up to 256 bytes) */
>  #define SPINOR_OP_PP_1_1_4_4B	0x34	/* Quad page program */
>  #define SPINOR_OP_PP_1_4_4_4B	0x3e	/* Quad page program */
> +#define SPINOR_OP_PP_1_1_8_4B	0x84	/* Octal page program */
> +#define SPINOR_OP_PP_1_8_8_4B	0x8e	/* Octal page program */
>  #define SPINOR_OP_BE_4K_4B	0x21	/* Erase 4KiB block */
>  #define SPINOR_OP_BE_32K_4B	0x5c	/* Erase 32KiB block */
>  #define SPINOR_OP_SE_4B		0xdc	/* Sector erase (usually 64KiB) */
> @@ -124,6 +132,7 @@ enum read_mode {
>  	SPI_NOR_FAST,
>  	SPI_NOR_DUAL,
>  	SPI_NOR_QUAD,
> +	SPI_NOR_OCTAL,
>  };
>  
>  #define SPI_NOR_MAX_CMD_SIZE	8

[v2, 1/4] mtd: spi-nor: Add support for Octal SPI mode.

[Artur Jedrysek]

This patch adds support for Octal SPI data reads in SPI NOR framework.
Opcodes for programming using octal interface are also present for the
sake of completeness, despite not being used.

Micron mt35xu512 flash is added as an example chip supporting that mode.

Signed-off-by: Artur Jedrysek <jartur@cadence.com>
---
Changelog:
v2: None.
---
 drivers/mtd/spi-nor/spi-nor.c | 20 ++++++++++++++++++--
 include/linux/mtd/spi-nor.h   |  9 +++++++++
 2 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 1ae872b..db188e2 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -85,6 +85,7 @@ struct flash_info {
 					 * Use dedicated 4byte address op codes
 					 * to support memory size above 128Mib.
 					 */
+#define SPI_NOR_OCTAL_READ	BIT(12)	/* Flash supports Octal Read */
 };
 
 #define JEDEC_MFR(info)	((info)->id[0])
@@ -159,6 +160,7 @@ static inline int spi_nor_read_dummy_cycles(struct spi_nor *nor)
 	case SPI_NOR_FAST:
 	case SPI_NOR_DUAL:
 	case SPI_NOR_QUAD:
+	case SPI_NOR_OCTAL:
 		return 8;
 	case SPI_NOR_NORMAL:
 		return 0;
@@ -220,6 +222,8 @@ static inline u8 spi_nor_convert_3to4_read(u8 opcode)
 		{ SPINOR_OP_READ_1_2_2,	SPINOR_OP_READ_1_2_2_4B },
 		{ SPINOR_OP_READ_1_1_4,	SPINOR_OP_READ_1_1_4_4B },
 		{ SPINOR_OP_READ_1_4_4,	SPINOR_OP_READ_1_4_4_4B },
+		{ SPINOR_OP_READ_1_1_8,	SPINOR_OP_READ_1_1_8_4B },
+		{ SPINOR_OP_READ_1_8_8,	SPINOR_OP_READ_1_8_8_4B },
 	};
 
 	return spi_nor_convert_opcode(opcode, spi_nor_3to4_read,
@@ -232,6 +236,8 @@ static inline u8 spi_nor_convert_3to4_program(u8 opcode)
 		{ SPINOR_OP_PP,		SPINOR_OP_PP_4B },
 		{ SPINOR_OP_PP_1_1_4,	SPINOR_OP_PP_1_1_4_4B },
 		{ SPINOR_OP_PP_1_4_4,	SPINOR_OP_PP_1_4_4_4B },
+		{ SPINOR_OP_PP_1_1_8,	SPINOR_OP_PP_1_1_8_4B },
+		{ SPINOR_OP_PP_1_8_8,	SPINOR_OP_PP_1_8_8_4B },
 	};
 
 	return spi_nor_convert_opcode(opcode, spi_nor_3to4_program,
@@ -1035,6 +1041,11 @@ static const struct flash_info spi_nor_ids[] = {
 	{ "n25q512ax3",  INFO(0x20ba20, 0, 64 * 1024, 1024, SECT_4K | USE_FSR | SPI_NOR_QUAD_READ) },
 	{ "n25q00",      INFO(0x20ba21, 0, 64 * 1024, 2048, SECT_4K | USE_FSR | SPI_NOR_QUAD_READ) },
 	{ "n25q00a",     INFO(0x20bb21, 0, 64 * 1024, 2048, SECT_4K | USE_FSR | SPI_NOR_QUAD_READ) },
+	{
+		"mt35xu512", INFO(0x2c5b1a, 0, 128 * 1024, 512,
+			SECT_4K | USE_FSR | SPI_NOR_OCTAL_READ |
+			SPI_NOR_4B_OPCODES)
+	},
 
 	/* PMC */
 	{ "pm25lv512",   INFO(0,        0, 32 * 1024,    2, SECT_4K_PMC) },
@@ -1667,8 +1678,10 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
 	if (info->flags & SPI_NOR_NO_FR)
 		nor->flash_read = SPI_NOR_NORMAL;
 
-	/* Quad/Dual-read mode takes precedence over fast/normal */
-	if (mode == SPI_NOR_QUAD && info->flags & SPI_NOR_QUAD_READ) {
+	/* Octal/Quad/Dual-read mode takes precedence over fast/normal */
+	if (mode == SPI_NOR_OCTAL && info->flags & SPI_NOR_OCTAL_READ) {
+		nor->flash_read = SPI_NOR_OCTAL;
+	} else if (mode == SPI_NOR_QUAD && info->flags & SPI_NOR_QUAD_READ) {
 		ret = set_quad_mode(nor, info);
 		if (ret) {
 			dev_err(dev, "quad mode not supported\n");
@@ -1681,6 +1694,9 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
 
 	/* Default commands */
 	switch (nor->flash_read) {
+	case SPI_NOR_OCTAL:
+		nor->read_opcode = SPINOR_OP_READ_1_1_8;
+		break;
 	case SPI_NOR_QUAD:
 		nor->read_opcode = SPINOR_OP_READ_1_1_4;
 		break;
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index f2a7180..aad6318 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -47,9 +47,13 @@
 #define SPINOR_OP_READ_1_2_2	0xbb	/* Read data bytes (Dual I/O SPI) */
 #define SPINOR_OP_READ_1_1_4	0x6b	/* Read data bytes (Quad Output SPI) */
 #define SPINOR_OP_READ_1_4_4	0xeb	/* Read data bytes (Quad I/O SPI) */
+#define SPINOR_OP_READ_1_1_8	0x8b	/* Read data bytes (Octal Output SPI) */
+#define SPINOR_OP_READ_1_8_8	0xcb	/* Read data bytes (Octal I/O SPI) */
 #define SPINOR_OP_PP		0x02	/* Page program (up to 256 bytes) */
 #define SPINOR_OP_PP_1_1_4	0x32	/* Quad page program */
 #define SPINOR_OP_PP_1_4_4	0x38	/* Quad page program */
+#define SPINOR_OP_PP_1_1_8	0x82	/* Octal page program */
+#define SPINOR_OP_PP_1_8_8	0xc2	/* Octal page program */
 #define SPINOR_OP_BE_4K		0x20	/* Erase 4KiB block */
 #define SPINOR_OP_BE_4K_PMC	0xd7	/* Erase 4KiB block on PMC chips */
 #define SPINOR_OP_BE_32K	0x52	/* Erase 32KiB block */
@@ -66,9 +70,13 @@
 #define SPINOR_OP_READ_1_2_2_4B	0xbc	/* Read data bytes (Dual I/O SPI) */
 #define SPINOR_OP_READ_1_1_4_4B	0x6c	/* Read data bytes (Quad Output SPI) */
 #define SPINOR_OP_READ_1_4_4_4B	0xec	/* Read data bytes (Quad I/O SPI) */
+#define SPINOR_OP_READ_1_1_8_4B	0x7c	/* Read data bytes (Octal Output SPI) */
+#define SPINOR_OP_READ_1_8_8_4B	0xcc	/* Read data bytes (Octal I/O SPI) */
 #define SPINOR_OP_PP_4B		0x12	/* Page program (up to 256 bytes) */
 #define SPINOR_OP_PP_1_1_4_4B	0x34	/* Quad page program */
 #define SPINOR_OP_PP_1_4_4_4B	0x3e	/* Quad page program */
+#define SPINOR_OP_PP_1_1_8_4B	0x84	/* Octal page program */
+#define SPINOR_OP_PP_1_8_8_4B	0x8e	/* Octal page program */
 #define SPINOR_OP_BE_4K_4B	0x21	/* Erase 4KiB block */
 #define SPINOR_OP_BE_32K_4B	0x5c	/* Erase 32KiB block */
 #define SPINOR_OP_SE_4B		0xdc	/* Sector erase (usually 64KiB) */
@@ -124,6 +132,7 @@ enum read_mode {
 	SPI_NOR_FAST,
 	SPI_NOR_DUAL,
 	SPI_NOR_QUAD,
+	SPI_NOR_OCTAL,
 };
 
 #define SPI_NOR_MAX_CMD_SIZE	8
-- 
2.2.2

[v2, 2/4] mtd: spi-nor: Add Octal SPI support to Cadence QSPI driver.

[Artur Jedrysek]

Recent versions of Cadence QSPI controller support Octal SPI transfers
as well. This patch updates existing driver to support such feature.

It is not possible to determine whether or not octal mode is supported
just by looking at revision register alone. To solve that, an additional
compatible in Device Tree is added to indicate such capability.
Both (revision and compatible) are used to determine, which mode to
pass to spi_nor_scan() call.

Signed-off-by: Artur Jedrysek <jartur@cadence.com>
---
Changelog:
v2: Use new compatible in DT, instead of boolean property, to indicate
Octal SPI support.
Extracted Kconfig update to seperate patch.
---
 drivers/mtd/spi-nor/cadence-quadspi.c | 69 +++++++++++++++++++++++++++++++----
 1 file changed, 61 insertions(+), 8 deletions(-)

diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
index 9f8102d..a96471d 100644
--- a/drivers/mtd/spi-nor/cadence-quadspi.c
+++ b/drivers/mtd/spi-nor/cadence-quadspi.c
@@ -76,6 +76,7 @@ struct cqspi_st {
 	u32			fifo_depth;
 	u32			fifo_width;
 	u32			trigger_address;
+	u32			max_mode;
 	struct cqspi_flash_pdata f_pdata[CQSPI_MAX_CHIPSELECT];
 };
 
@@ -87,6 +88,10 @@ struct cqspi_st {
 #define CQSPI_INST_TYPE_SINGLE			0
 #define CQSPI_INST_TYPE_DUAL			1
 #define CQSPI_INST_TYPE_QUAD			2
+#define CQSPI_INST_TYPE_OCTAL			3
+
+#define CQSPI_MAX_MODE_QUAD			0
+#define CQSPI_MAX_MODE_OCTAL			1
 
 #define CQSPI_DUMMY_CLKS_PER_BYTE		8
 #define CQSPI_DUMMY_BYTES_MAX			4
@@ -204,6 +209,14 @@ struct cqspi_st {
 #define CQSPI_REG_CMDWRITEDATALOWER		0xA8
 #define CQSPI_REG_CMDWRITEDATAUPPER		0xAC
 
+#define CQSPI_REG_MODULEID			0xFC
+#define CQSPI_REG_MODULEID_CONF_ID_MASK		0x3
+#define CQSPI_REG_MODULEID_CONF_ID_LSB		0
+#define CQSPI_REG_MODULEID_CONF_ID_OCTAL_PHY	0x0
+#define CQSPI_REG_MODULEID_CONF_ID_OCTAL	0x1
+#define CQSPI_REG_MODULEID_CONF_ID_QUAD_PHY	0x2
+#define CQSPI_REG_MODULEID_CONF_ID_QUAD		0x3
+
 /* Interrupt status bits */
 #define CQSPI_REG_IRQ_MODE_ERR			BIT(0)
 #define CQSPI_REG_IRQ_UNDERFLOW			BIT(1)
@@ -866,6 +879,9 @@ static int cqspi_set_protocol(struct spi_nor *nor, const int read)
 		case SPI_NOR_QUAD:
 			f_pdata->data_width = CQSPI_INST_TYPE_QUAD;
 			break;
+		case SPI_NOR_OCTAL:
+			f_pdata->data_width = CQSPI_INST_TYPE_OCTAL;
+			break;
 		default:
 			return -EINVAL;
 		}
@@ -977,6 +993,17 @@ static int cqspi_write_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len)
 	return ret;
 }
 
+static const u32 cqspi_max_mode_quad = CQSPI_MAX_MODE_QUAD;
+static const u32 cqspi_max_mode_octal = CQSPI_MAX_MODE_OCTAL;
+
+static struct of_device_id const cqspi_dt_ids[] = {
+	{ .compatible = "cdns,qspi-nor", .data = &cqspi_max_mode_quad },
+	{ .compatible = "cdns,ospi-nor", .data = &cqspi_max_mode_octal },
+	{ /* end of table */ }
+};
+
+MODULE_DEVICE_TABLE(of, cqspi_dt_ids);
+
 static int cqspi_of_get_flash_pdata(struct platform_device *pdev,
 				    struct cqspi_flash_pdata *f_pdata,
 				    struct device_node *np)
@@ -1018,6 +1045,13 @@ static int cqspi_of_get_pdata(struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;
 	struct cqspi_st *cqspi = platform_get_drvdata(pdev);
+	const struct of_device_id *match;
+
+	cqspi->max_mode = CQSPI_MAX_MODE_QUAD;
+
+	match = of_match_node(cqspi_dt_ids, np);
+	if (match && match->data)
+		cqspi->max_mode = *((u32 *)match->data);
 
 	cqspi->is_decoded_cs = of_property_read_bool(np, "cdns,is-decoded-cs");
 
@@ -1074,9 +1108,35 @@ static int cqspi_setup_flash(struct cqspi_st *cqspi, struct device_node *np)
 	struct cqspi_flash_pdata *f_pdata;
 	struct spi_nor *nor;
 	struct mtd_info *mtd;
+	enum read_mode mode;
+	enum read_mode dt_mode = SPI_NOR_QUAD;
 	unsigned int cs;
+	unsigned int rev_reg;
 	int i, ret;
 
+	/* Determine, whether or not octal transfer MAY be supported */
+	rev_reg = readl(cqspi->iobase + CQSPI_REG_MODULEID);
+	dev_info(dev, "CQSPI Module id %x\n", rev_reg);
+
+	switch (rev_reg & CQSPI_REG_MODULEID_CONF_ID_MASK) {
+	case CQSPI_REG_MODULEID_CONF_ID_OCTAL_PHY:
+	case CQSPI_REG_MODULEID_CONF_ID_OCTAL:
+		mode = SPI_NOR_OCTAL;
+		break;
+	case CQSPI_REG_MODULEID_CONF_ID_QUAD:
+	case CQSPI_REG_MODULEID_CONF_ID_QUAD_PHY:
+		mode = SPI_NOR_QUAD;
+		break;
+	}
+
+	if (cqspi->max_mode == CQSPI_MAX_MODE_OCTAL)
+		dt_mode = SPI_NOR_OCTAL;
+
+	if (mode == SPI_NOR_QUAD && dt_mode == SPI_NOR_OCTAL)
+		dev_warn(dev, "Requested octal mode is not supported by the device.");
+	else if (mode == SPI_NOR_OCTAL && dt_mode == SPI_NOR_QUAD)
+		mode = SPI_NOR_QUAD;
+
 	/* Get flash device data */
 	for_each_available_child_of_node(dev->of_node, np) {
 		ret = of_property_read_u32(np, "reg", &cs);
@@ -1123,7 +1183,7 @@ static int cqspi_setup_flash(struct cqspi_st *cqspi, struct device_node *np)
 			goto err;
 		}
 
-		ret = spi_nor_scan(nor, NULL, SPI_NOR_QUAD);
+		ret = spi_nor_scan(nor, NULL, mode);
 		if (ret)
 			goto err;
 
@@ -1277,13 +1337,6 @@ static const struct dev_pm_ops cqspi__dev_pm_ops = {
 #define CQSPI_DEV_PM_OPS	NULL
 #endif
 
-static struct of_device_id const cqspi_dt_ids[] = {
-	{.compatible = "cdns,qspi-nor",},
-	{ /* end of table */ }
-};
-
-MODULE_DEVICE_TABLE(of, cqspi_dt_ids);
-
 static struct platform_driver cqspi_platform_driver = {
 	.probe = cqspi_probe,
 	.remove = cqspi_remove,
-- 
2.2.2

[v2, 3/4] mtd: spi-nor: Add Xtensa CPU support for cadence-quadspi.

[Artur Jedrysek]

Cadence Quad SPI driver successfully runs on Xtensa CPU, and Kconfig
file is updated to indicate that.

Signed-off-by: Artur Jedrysek <jartur@cadence.com>
---
Changelog:
v2: This change is extracted from previous patch (updating CQSPI driver).
---
 drivers/mtd/spi-nor/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
index 7252087..f195749 100644
--- a/drivers/mtd/spi-nor/Kconfig
+++ b/drivers/mtd/spi-nor/Kconfig
@@ -50,7 +50,7 @@ config SPI_ATMEL_QUADSPI
 
 config SPI_CADENCE_QUADSPI
 	tristate "Cadence Quad SPI controller"
-	depends on OF && (ARM || COMPILE_TEST)
+	depends on OF && (ARM || XTENSA || COMPILE_TEST)
 	help
 	  Enable support for the Cadence Quad SPI Flash controller.
 
-- 
2.2.2

[v2, 4/4] dt-bindings: mtd: Add Octal SPI support to Cadence QSPI.

[Artur Jedrysek]

This patch updates Cadence QSPI Device Tree documentation to include
information about new compatible used to indicate, whether or not
Octal SPI transfers are supported by the device.

Signed-off-by: Artur Jedrysek <jartur@cadence.com>
---
Changelog:
v2: Use new compatible, instead of boolean property, to indicate
Octal SPI support.
---
 Documentation/devicetree/bindings/mtd/cadence-quadspi.txt | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt b/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
index f248056..41d1e98 100644
--- a/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
+++ b/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
@@ -1,7 +1,9 @@
 * Cadence Quad SPI controller
 
 Required properties:
-- compatible : Should be "cdns,qspi-nor".
+- compatible : Should be "cdns,{qspi|ospi}-nor".
+  Use "cdns,qspi-nor" for Quad SPI controller.
+  Use "cdns,ospi-nor" for Octal SPI controller.
 - reg : Contains two entries, each of which is a tuple consisting of a
 	physical address and length. The first entry is the address and
 	length of the controller register set. The second entry is the
-- 
2.2.2

Re: [v2, 2/4] mtd: spi-nor: Add Octal SPI support to Cadence QSPI driver.

[Marek Vasut]

On 03/08/2017 09:02 AM, Artur Jedrysek wrote:
> Recent versions of Cadence QSPI controller support Octal SPI transfers
> as well. This patch updates existing driver to support such feature.
> 
> It is not possible to determine whether or not octal mode is supported
> just by looking at revision register alone. To solve that, an additional
> compatible in Device Tree is added to indicate such capability.
> Both (revision and compatible) are used to determine, which mode to
> pass to spi_nor_scan() call.
> 
> Signed-off-by: Artur Jedrysek <jartur@cadence.com>
> ---
> Changelog:
> v2: Use new compatible in DT, instead of boolean property, to indicate
> Octal SPI support.
> Extracted Kconfig update to seperate patch.
> ---
>  drivers/mtd/spi-nor/cadence-quadspi.c | 69 +++++++++++++++++++++++++++++++----
>  1 file changed, 61 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
> index 9f8102d..a96471d 100644
> --- a/drivers/mtd/spi-nor/cadence-quadspi.c
> +++ b/drivers/mtd/spi-nor/cadence-quadspi.c
> @@ -76,6 +76,7 @@ struct cqspi_st {
>  	u32			fifo_depth;
>  	u32			fifo_width;
>  	u32			trigger_address;
> +	u32			max_mode;

I think it's time to introduce flags instead,
ie. #define CQSPI_FLAG_SUPPORTS_OCTAL BIT(0)

>  	struct cqspi_flash_pdata f_pdata[CQSPI_MAX_CHIPSELECT];
>  };
>  
> @@ -87,6 +88,10 @@ struct cqspi_st {
>  #define CQSPI_INST_TYPE_SINGLE			0
>  #define CQSPI_INST_TYPE_DUAL			1
>  #define CQSPI_INST_TYPE_QUAD			2
> +#define CQSPI_INST_TYPE_OCTAL			3
> +
> +#define CQSPI_MAX_MODE_QUAD			0
> +#define CQSPI_MAX_MODE_OCTAL			1
>  
>  #define CQSPI_DUMMY_CLKS_PER_BYTE		8
>  #define CQSPI_DUMMY_BYTES_MAX			4
> @@ -204,6 +209,14 @@ struct cqspi_st {
>  #define CQSPI_REG_CMDWRITEDATALOWER		0xA8
>  #define CQSPI_REG_CMDWRITEDATAUPPER		0xAC
>  
> +#define CQSPI_REG_MODULEID			0xFC
> +#define CQSPI_REG_MODULEID_CONF_ID_MASK		0x3
> +#define CQSPI_REG_MODULEID_CONF_ID_LSB		0
> +#define CQSPI_REG_MODULEID_CONF_ID_OCTAL_PHY	0x0
> +#define CQSPI_REG_MODULEID_CONF_ID_OCTAL	0x1
> +#define CQSPI_REG_MODULEID_CONF_ID_QUAD_PHY	0x2
> +#define CQSPI_REG_MODULEID_CONF_ID_QUAD		0x3
> +
>  /* Interrupt status bits */
>  #define CQSPI_REG_IRQ_MODE_ERR			BIT(0)
>  #define CQSPI_REG_IRQ_UNDERFLOW			BIT(1)
> @@ -866,6 +879,9 @@ static int cqspi_set_protocol(struct spi_nor *nor, const int read)
>  		case SPI_NOR_QUAD:
>  			f_pdata->data_width = CQSPI_INST_TYPE_QUAD;
>  			break;
> +		case SPI_NOR_OCTAL:
> +			f_pdata->data_width = CQSPI_INST_TYPE_OCTAL;
> +			break;
>  		default:
>  			return -EINVAL;
>  		}
> @@ -977,6 +993,17 @@ static int cqspi_write_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len)
>  	return ret;
>  }
>  
> +static const u32 cqspi_max_mode_quad = CQSPI_MAX_MODE_QUAD;
> +static const u32 cqspi_max_mode_octal = CQSPI_MAX_MODE_OCTAL;
> +
> +static struct of_device_id const cqspi_dt_ids[] = {
> +	{ .compatible = "cdns,qspi-nor", .data = &cqspi_max_mode_quad },
> +	{ .compatible = "cdns,ospi-nor", .data = &cqspi_max_mode_octal },

.data = (void *)CQSPI_FLAG_SUPPORTS_OCTAL , then you don't need that
static const stuff ...

> +	{ /* end of table */ }
> +};
> +
> +MODULE_DEVICE_TABLE(of, cqspi_dt_ids);
> +
>  static int cqspi_of_get_flash_pdata(struct platform_device *pdev,
>  				    struct cqspi_flash_pdata *f_pdata,
>  				    struct device_node *np)
> @@ -1018,6 +1045,13 @@ static int cqspi_of_get_pdata(struct platform_device *pdev)
>  {
>  	struct device_node *np = pdev->dev.of_node;
>  	struct cqspi_st *cqspi = platform_get_drvdata(pdev);
> +	const struct of_device_id *match;
> +
> +	cqspi->max_mode = CQSPI_MAX_MODE_QUAD;
> +
> +	match = of_match_node(cqspi_dt_ids, np);
> +	if (match && match->data)
> +		cqspi->max_mode = *((u32 *)match->data);

Use flags instead, see above.

>  	cqspi->is_decoded_cs = of_property_read_bool(np, "cdns,is-decoded-cs");
>  
> @@ -1074,9 +1108,35 @@ static int cqspi_setup_flash(struct cqspi_st *cqspi, struct device_node *np)
>  	struct cqspi_flash_pdata *f_pdata;
>  	struct spi_nor *nor;
>  	struct mtd_info *mtd;
> +	enum read_mode mode;
> +	enum read_mode dt_mode = SPI_NOR_QUAD;
>  	unsigned int cs;
> +	unsigned int rev_reg;
>  	int i, ret;
>  
> +	/* Determine, whether or not octal transfer MAY be supported */

But you already know that from DT, no ?

> +	rev_reg = readl(cqspi->iobase + CQSPI_REG_MODULEID);
> +	dev_info(dev, "CQSPI Module id %x\n", rev_reg);
> +
> +	switch (rev_reg & CQSPI_REG_MODULEID_CONF_ID_MASK) {
> +	case CQSPI_REG_MODULEID_CONF_ID_OCTAL_PHY:
> +	case CQSPI_REG_MODULEID_CONF_ID_OCTAL:
> +		mode = SPI_NOR_OCTAL;
> +		break;
> +	case CQSPI_REG_MODULEID_CONF_ID_QUAD:
> +	case CQSPI_REG_MODULEID_CONF_ID_QUAD_PHY:

Does this work on all revisions of CQSPI ?

> +		mode = SPI_NOR_QUAD;
> +		break;
> +	}
> +
> +	if (cqspi->max_mode == CQSPI_MAX_MODE_OCTAL)
> +		dt_mode = SPI_NOR_OCTAL;
> +
> +	if (mode == SPI_NOR_QUAD && dt_mode == SPI_NOR_OCTAL)
> +		dev_warn(dev, "Requested octal mode is not supported by the device.");
> +	else if (mode == SPI_NOR_OCTAL && dt_mode == SPI_NOR_QUAD)
> +		mode = SPI_NOR_QUAD;
> +
>  	/* Get flash device data */
>  	for_each_available_child_of_node(dev->of_node, np) {
>  		ret = of_property_read_u32(np, "reg", &cs);
> @@ -1123,7 +1183,7 @@ static int cqspi_setup_flash(struct cqspi_st *cqspi, struct device_node *np)
>  			goto err;
>  		}
>  
> -		ret = spi_nor_scan(nor, NULL, SPI_NOR_QUAD);
> +		ret = spi_nor_scan(nor, NULL, mode);
>  		if (ret)
>  			goto err;
>  
> @@ -1277,13 +1337,6 @@ static const struct dev_pm_ops cqspi__dev_pm_ops = {
>  #define CQSPI_DEV_PM_OPS	NULL
>  #endif
>  
> -static struct of_device_id const cqspi_dt_ids[] = {
> -	{.compatible = "cdns,qspi-nor",},
> -	{ /* end of table */ }
> -};
> -
> -MODULE_DEVICE_TABLE(of, cqspi_dt_ids);
> -
>  static struct platform_driver cqspi_platform_driver = {
>  	.probe = cqspi_probe,
>  	.remove = cqspi_remove,
> 


-- 
Best regards,
Marek Vasut

Re: [v2, 4/4] dt-bindings: mtd: Add Octal SPI support to Cadence QSPI.

[Marek Vasut]

On 03/08/2017 09:05 AM, Artur Jedrysek wrote:
> This patch updates Cadence QSPI Device Tree documentation to include
> information about new compatible used to indicate, whether or not
> Octal SPI transfers are supported by the device.
> 
> Signed-off-by: Artur Jedrysek <jartur@cadence.com>
> ---
> Changelog:
> v2: Use new compatible, instead of boolean property, to indicate
> Octal SPI support.
> ---
>  Documentation/devicetree/bindings/mtd/cadence-quadspi.txt | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt b/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
> index f248056..41d1e98 100644
> --- a/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
> +++ b/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
> @@ -1,7 +1,9 @@
>  * Cadence Quad SPI controller
>  
>  Required properties:
> -- compatible : Should be "cdns,qspi-nor".
> +- compatible : Should be "cdns,{qspi|ospi}-nor".

Please explicitly list all compatibles , ie.
Should be "cdns,qspi-nor" or "cdns,ospi-nor".

But I think the ospi is backward compatible with qspi, right ? So the
binding for ospi should list both, ie.
compatible = "cdns,ospi-nor", "cdns,qspi-nor";

> +  Use "cdns,qspi-nor" for Quad SPI controller.
> +  Use "cdns,ospi-nor" for Octal SPI controller.
>  - reg : Contains two entries, each of which is a tuple consisting of a
>  	physical address and length. The first entry is the address and
>  	length of the controller register set. The second entry is the
> 


-- 
Best regards,
Marek Vasut

RE: [v2, 2/4] mtd: spi-nor: Add Octal SPI support to Cadence QSPI driver.

[Artur Jedrysek]

From: Marek Vasut [mailto:marek.vasut@gmail.com]
Sent: 10 March 2017 04:37
> On 03/08/2017 09:02 AM, Artur Jedrysek wrote:
> > Recent versions of Cadence QSPI controller support Octal SPI transfers
> > as well. This patch updates existing driver to support such feature.
> >
> > It is not possible to determine whether or not octal mode is supported
> > just by looking at revision register alone. To solve that, an additional
> > compatible in Device Tree is added to indicate such capability.
> > Both (revision and compatible) are used to determine, which mode to
> > pass to spi_nor_scan() call.
> >
> > Signed-off-by: Artur Jedrysek <jartur@cadence.com>
> > ---
> > Changelog:
> > v2: Use new compatible in DT, instead of boolean property, to indicate
> > Octal SPI support.
> > Extracted Kconfig update to seperate patch.
> > ---
> >  drivers/mtd/spi-nor/cadence-quadspi.c | 69 +++++++++++++++++++++++++++++++----
> >  1 file changed, 61 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
> > index 9f8102d..a96471d 100644
> > --- a/drivers/mtd/spi-nor/cadence-quadspi.c
> > +++ b/drivers/mtd/spi-nor/cadence-quadspi.c
> > @@ -76,6 +76,7 @@ struct cqspi_st {
> >  	u32			fifo_depth;
> >  	u32			fifo_width;
> >  	u32			trigger_address;
> > +	u32			max_mode;
> 
> I think it's time to introduce flags instead,
> ie. #define CQSPI_FLAG_SUPPORTS_OCTAL BIT(0)
> 
> >  	struct cqspi_flash_pdata f_pdata[CQSPI_MAX_CHIPSELECT];
> >  };
> >
> > @@ -87,6 +88,10 @@ struct cqspi_st {
> >  #define CQSPI_INST_TYPE_SINGLE			0
> >  #define CQSPI_INST_TYPE_DUAL			1
> >  #define CQSPI_INST_TYPE_QUAD			2
> > +#define CQSPI_INST_TYPE_OCTAL			3
> > +
> > +#define CQSPI_MAX_MODE_QUAD			0
> > +#define CQSPI_MAX_MODE_OCTAL			1
> >
> >  #define CQSPI_DUMMY_CLKS_PER_BYTE		8
> >  #define CQSPI_DUMMY_BYTES_MAX			4
> > @@ -204,6 +209,14 @@ struct cqspi_st {
> >  #define CQSPI_REG_CMDWRITEDATALOWER		0xA8
> >  #define CQSPI_REG_CMDWRITEDATAUPPER		0xAC
> >
> > +#define CQSPI_REG_MODULEID			0xFC
> > +#define CQSPI_REG_MODULEID_CONF_ID_MASK		0x3
> > +#define CQSPI_REG_MODULEID_CONF_ID_LSB		0
> > +#define CQSPI_REG_MODULEID_CONF_ID_OCTAL_PHY	0x0
> > +#define CQSPI_REG_MODULEID_CONF_ID_OCTAL	0x1
> > +#define CQSPI_REG_MODULEID_CONF_ID_QUAD_PHY	0x2
> > +#define CQSPI_REG_MODULEID_CONF_ID_QUAD		0x3
> > +
> >  /* Interrupt status bits */
> >  #define CQSPI_REG_IRQ_MODE_ERR			BIT(0)
> >  #define CQSPI_REG_IRQ_UNDERFLOW			BIT(1)
> > @@ -866,6 +879,9 @@ static int cqspi_set_protocol(struct spi_nor *nor, const int read)
> >  		case SPI_NOR_QUAD:
> >  			f_pdata->data_width = CQSPI_INST_TYPE_QUAD;
> >  			break;
> > +		case SPI_NOR_OCTAL:
> > +			f_pdata->data_width = CQSPI_INST_TYPE_OCTAL;
> > +			break;
> >  		default:
> >  			return -EINVAL;
> >  		}
> > @@ -977,6 +993,17 @@ static int cqspi_write_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len)
> >  	return ret;
> >  }
> >
> > +static const u32 cqspi_max_mode_quad = CQSPI_MAX_MODE_QUAD;
> > +static const u32 cqspi_max_mode_octal = CQSPI_MAX_MODE_OCTAL;
> > +
> > +static struct of_device_id const cqspi_dt_ids[] = {
> > +	{ .compatible = "cdns,qspi-nor", .data = &cqspi_max_mode_quad },
> > +	{ .compatible = "cdns,ospi-nor", .data = &cqspi_max_mode_octal },
> 
> .data = (void *)CQSPI_FLAG_SUPPORTS_OCTAL , then you don't need that
> static const stuff ...
> 

I had some doubts regarding that approach, as it may be dependent on the
CPU architecture (32-bit, 64-bit) and endianness. .data needs to be first
casted to unsigned long for reading, to ensure correct access and to 
allow bitwise operations on it. That solution works, and if such approach
is preferred, then it will be used in next version of the patch.

> > +	{ /* end of table */ }
> > +};
> > +
> > +MODULE_DEVICE_TABLE(of, cqspi_dt_ids);
> > +
> >  static int cqspi_of_get_flash_pdata(struct platform_device *pdev,
> >  				    struct cqspi_flash_pdata *f_pdata,
> >  				    struct device_node *np)
> > @@ -1018,6 +1045,13 @@ static int cqspi_of_get_pdata(struct platform_device *pdev)
> >  {
> >  	struct device_node *np = pdev->dev.of_node;
> >  	struct cqspi_st *cqspi = platform_get_drvdata(pdev);
> > +	const struct of_device_id *match;
> > +
> > +	cqspi->max_mode = CQSPI_MAX_MODE_QUAD;
> > +
> > +	match = of_match_node(cqspi_dt_ids, np);
> > +	if (match && match->data)
> > +		cqspi->max_mode = *((u32 *)match->data);
> 
> Use flags instead, see above.
> 
> >  	cqspi->is_decoded_cs = of_property_read_bool(np, "cdns,is-decoded-cs");
> >
> > @@ -1074,9 +1108,35 @@ static int cqspi_setup_flash(struct cqspi_st *cqspi, struct device_node *np)
> >  	struct cqspi_flash_pdata *f_pdata;
> >  	struct spi_nor *nor;
> >  	struct mtd_info *mtd;
> > +	enum read_mode mode;
> > +	enum read_mode dt_mode = SPI_NOR_QUAD;
> >  	unsigned int cs;
> > +	unsigned int rev_reg;
> >  	int i, ret;
> >
> > +	/* Determine, whether or not octal transfer MAY be supported */
> 
> But you already know that from DT, no ?
> 

Cadence Octal (formerly Quad) SPI Controller is sold as an IP, and is
configurable. This includes max SPI mode. It is possible to detect, that
Octal SPI controller is configured (during hardware compilation) to support
up to Quad mode, using revision register.

> > +	rev_reg = readl(cqspi->iobase + CQSPI_REG_MODULEID);
> > +	dev_info(dev, "CQSPI Module id %x\n", rev_reg);
> > +
> > +	switch (rev_reg & CQSPI_REG_MODULEID_CONF_ID_MASK) {
> > +	case CQSPI_REG_MODULEID_CONF_ID_OCTAL_PHY:
> > +	case CQSPI_REG_MODULEID_CONF_ID_OCTAL:
> > +		mode = SPI_NOR_OCTAL;
> > +		break;
> > +	case CQSPI_REG_MODULEID_CONF_ID_QUAD:
> > +	case CQSPI_REG_MODULEID_CONF_ID_QUAD_PHY:
> 
> Does this work on all revisions of CQSPI ?
> 

After having a more thorough look at specification of older IP version
(quad only) it seems, that revision register format has indeed changed.
This will be fixed in the next version of the patch.

> > +		mode = SPI_NOR_QUAD;
> > +		break;
> > +	}
> > +
> > +	if (cqspi->max_mode == CQSPI_MAX_MODE_OCTAL)
> > +		dt_mode = SPI_NOR_OCTAL;
> > +
> > +	if (mode == SPI_NOR_QUAD && dt_mode == SPI_NOR_OCTAL)
> > +		dev_warn(dev, "Requested octal mode is not supported by the device.");
> > +	else if (mode == SPI_NOR_OCTAL && dt_mode == SPI_NOR_QUAD)
> > +		mode = SPI_NOR_QUAD;
> > +
> >  	/* Get flash device data */
> >  	for_each_available_child_of_node(dev->of_node, np) {
> >  		ret = of_property_read_u32(np, "reg", &cs);
> > @@ -1123,7 +1183,7 @@ static int cqspi_setup_flash(struct cqspi_st *cqspi, struct device_node *np)
> >  			goto err;
> >  		}
> >
> > -		ret = spi_nor_scan(nor, NULL, SPI_NOR_QUAD);
> > +		ret = spi_nor_scan(nor, NULL, mode);
> >  		if (ret)
> >  			goto err;
> >
> > @@ -1277,13 +1337,6 @@ static const struct dev_pm_ops cqspi__dev_pm_ops = {
> >  #define CQSPI_DEV_PM_OPS	NULL
> >  #endif
> >
> > -static struct of_device_id const cqspi_dt_ids[] = {
> > -	{.compatible = "cdns,qspi-nor",},
> > -	{ /* end of table */ }
> > -};
> > -
> > -MODULE_DEVICE_TABLE(of, cqspi_dt_ids);
> > -
> >  static struct platform_driver cqspi_platform_driver = {
> >  	.probe = cqspi_probe,
> >  	.remove = cqspi_remove,
> >
> 
> 
> --
> Best regards,
> Marek Vasut

RE: [v2, 4/4] dt-bindings: mtd: Add Octal SPI support to Cadence QSPI.

[Artur Jedrysek]

> From: Marek Vasut [mailto:marek.vasut@gmail.com]
> Sent: 10 March 2017 04:39
> On 03/08/2017 09:05 AM, Artur Jedrysek wrote:
> > This patch updates Cadence QSPI Device Tree documentation to include
> > information about new compatible used to indicate, whether or not
> > Octal SPI transfers are supported by the device.
> >
> > Signed-off-by: Artur Jedrysek <jartur@cadence.com>
> > ---
> > Changelog:
> > v2: Use new compatible, instead of boolean property, to indicate
> > Octal SPI support.
> > ---
> >  Documentation/devicetree/bindings/mtd/cadence-quadspi.txt | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
> b/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
> > index f248056..41d1e98 100644
> > --- a/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
> > +++ b/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
> > @@ -1,7 +1,9 @@
> >  * Cadence Quad SPI controller
> >
> >  Required properties:
> > -- compatible : Should be "cdns,qspi-nor".
> > +- compatible : Should be "cdns,{qspi|ospi}-nor".
> 
> Please explicitly list all compatibles , ie.
> Should be "cdns,qspi-nor" or "cdns,ospi-nor".
> 

Two possible options are also explicitly listed right below. I did that
in the way it was done in other driver for Cadence IP:
/Documentation/devicetree/bindings/net/macb.txt
Should I remove all of that and replace it with both options, separated
by an "or" word?

> But I think the ospi is backward compatible with qspi, right ? So the
> binding for ospi should list both, ie.
> compatible = "cdns,ospi-nor", "cdns,qspi-nor";
> 

Yes, the ospi is backwards compatible with qspi. However, the sole point
of introducing new compatible was to differentiate between them (which
was previously done using a boolean flag, but that was criticized).
Listing both bindings for ospi would only help for kernels not
containing driver update being subject of this patch series, as both
ospi and qspi are handled by the same driver - just with a slight
difference. I apologize if I got something wrong here.

> > +  Use "cdns,qspi-nor" for Quad SPI controller.
> > +  Use "cdns,ospi-nor" for Octal SPI controller.
> >  - reg : Contains two entries, each of which is a tuple consisting of a
> >  	physical address and length. The first entry is the address and
> >  	length of the controller register set. The second entry is the
> >
> 
> 
> --
> Best regards,
> Marek Vasut

Re: [v2, 2/4] mtd: spi-nor: Add Octal SPI support to Cadence QSPI driver.

[Marek Vasut]

On 03/10/2017 01:00 PM, Artur Jedrysek wrote:
> 
> From: Marek Vasut [mailto:marek.vasut@gmail.com]
> Sent: 10 March 2017 04:37
>> On 03/08/2017 09:02 AM, Artur Jedrysek wrote:
>>> Recent versions of Cadence QSPI controller support Octal SPI transfers
>>> as well. This patch updates existing driver to support such feature.
>>>
>>> It is not possible to determine whether or not octal mode is supported
>>> just by looking at revision register alone. To solve that, an additional
>>> compatible in Device Tree is added to indicate such capability.
>>> Both (revision and compatible) are used to determine, which mode to
>>> pass to spi_nor_scan() call.
>>>
>>> Signed-off-by: Artur Jedrysek <jartur@cadence.com>
>>> ---
>>> Changelog:
>>> v2: Use new compatible in DT, instead of boolean property, to indicate
>>> Octal SPI support.
>>> Extracted Kconfig update to seperate patch.
>>> ---
>>>  drivers/mtd/spi-nor/cadence-quadspi.c | 69 +++++++++++++++++++++++++++++++----
>>>  1 file changed, 61 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
>>> index 9f8102d..a96471d 100644
>>> --- a/drivers/mtd/spi-nor/cadence-quadspi.c
>>> +++ b/drivers/mtd/spi-nor/cadence-quadspi.c
>>> @@ -76,6 +76,7 @@ struct cqspi_st {
>>>  	u32			fifo_depth;
>>>  	u32			fifo_width;
>>>  	u32			trigger_address;
>>> +	u32			max_mode;
>>
>> I think it's time to introduce flags instead,
>> ie. #define CQSPI_FLAG_SUPPORTS_OCTAL BIT(0)
>>
>>>  	struct cqspi_flash_pdata f_pdata[CQSPI_MAX_CHIPSELECT];
>>>  };
>>>
>>> @@ -87,6 +88,10 @@ struct cqspi_st {
>>>  #define CQSPI_INST_TYPE_SINGLE			0
>>>  #define CQSPI_INST_TYPE_DUAL			1
>>>  #define CQSPI_INST_TYPE_QUAD			2
>>> +#define CQSPI_INST_TYPE_OCTAL			3
>>> +
>>> +#define CQSPI_MAX_MODE_QUAD			0
>>> +#define CQSPI_MAX_MODE_OCTAL			1
>>>
>>>  #define CQSPI_DUMMY_CLKS_PER_BYTE		8
>>>  #define CQSPI_DUMMY_BYTES_MAX			4
>>> @@ -204,6 +209,14 @@ struct cqspi_st {
>>>  #define CQSPI_REG_CMDWRITEDATALOWER		0xA8
>>>  #define CQSPI_REG_CMDWRITEDATAUPPER		0xAC
>>>
>>> +#define CQSPI_REG_MODULEID			0xFC
>>> +#define CQSPI_REG_MODULEID_CONF_ID_MASK		0x3
>>> +#define CQSPI_REG_MODULEID_CONF_ID_LSB		0
>>> +#define CQSPI_REG_MODULEID_CONF_ID_OCTAL_PHY	0x0
>>> +#define CQSPI_REG_MODULEID_CONF_ID_OCTAL	0x1
>>> +#define CQSPI_REG_MODULEID_CONF_ID_QUAD_PHY	0x2
>>> +#define CQSPI_REG_MODULEID_CONF_ID_QUAD		0x3
>>> +
>>>  /* Interrupt status bits */
>>>  #define CQSPI_REG_IRQ_MODE_ERR			BIT(0)
>>>  #define CQSPI_REG_IRQ_UNDERFLOW			BIT(1)
>>> @@ -866,6 +879,9 @@ static int cqspi_set_protocol(struct spi_nor *nor, const int read)
>>>  		case SPI_NOR_QUAD:
>>>  			f_pdata->data_width = CQSPI_INST_TYPE_QUAD;
>>>  			break;
>>> +		case SPI_NOR_OCTAL:
>>> +			f_pdata->data_width = CQSPI_INST_TYPE_OCTAL;
>>> +			break;
>>>  		default:
>>>  			return -EINVAL;
>>>  		}
>>> @@ -977,6 +993,17 @@ static int cqspi_write_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len)
>>>  	return ret;
>>>  }
>>>
>>> +static const u32 cqspi_max_mode_quad = CQSPI_MAX_MODE_QUAD;
>>> +static const u32 cqspi_max_mode_octal = CQSPI_MAX_MODE_OCTAL;
>>> +
>>> +static struct of_device_id const cqspi_dt_ids[] = {
>>> +	{ .compatible = "cdns,qspi-nor", .data = &cqspi_max_mode_quad },
>>> +	{ .compatible = "cdns,ospi-nor", .data = &cqspi_max_mode_octal },
>>
>> .data = (void *)CQSPI_FLAG_SUPPORTS_OCTAL , then you don't need that
>> static const stuff ...
>>
> 
> I had some doubts regarding that approach, as it may be dependent on the
> CPU architecture (32-bit, 64-bit) and endianness. .data needs to be first
> casted to unsigned long for reading, to ensure correct access and to 
> allow bitwise operations on it. That solution works, and if such approach
> is preferred, then it will be used in next version of the patch.

Good

>>> +	{ /* end of table */ }
>>> +};
>>> +
>>> +MODULE_DEVICE_TABLE(of, cqspi_dt_ids);
>>> +
>>>  static int cqspi_of_get_flash_pdata(struct platform_device *pdev,
>>>  				    struct cqspi_flash_pdata *f_pdata,
>>>  				    struct device_node *np)
>>> @@ -1018,6 +1045,13 @@ static int cqspi_of_get_pdata(struct platform_device *pdev)
>>>  {
>>>  	struct device_node *np = pdev->dev.of_node;
>>>  	struct cqspi_st *cqspi = platform_get_drvdata(pdev);
>>> +	const struct of_device_id *match;
>>> +
>>> +	cqspi->max_mode = CQSPI_MAX_MODE_QUAD;
>>> +
>>> +	match = of_match_node(cqspi_dt_ids, np);
>>> +	if (match && match->data)
>>> +		cqspi->max_mode = *((u32 *)match->data);
>>
>> Use flags instead, see above.
>>
>>>  	cqspi->is_decoded_cs = of_property_read_bool(np, "cdns,is-decoded-cs");
>>>
>>> @@ -1074,9 +1108,35 @@ static int cqspi_setup_flash(struct cqspi_st *cqspi, struct device_node *np)
>>>  	struct cqspi_flash_pdata *f_pdata;
>>>  	struct spi_nor *nor;
>>>  	struct mtd_info *mtd;
>>> +	enum read_mode mode;
>>> +	enum read_mode dt_mode = SPI_NOR_QUAD;
>>>  	unsigned int cs;
>>> +	unsigned int rev_reg;
>>>  	int i, ret;
>>>
>>> +	/* Determine, whether or not octal transfer MAY be supported */
>>
>> But you already know that from DT, no ?
>>
> 
> Cadence Octal (formerly Quad) SPI Controller is sold as an IP, and is
> configurable. This includes max SPI mode. It is possible to detect, that
> Octal SPI controller is configured (during hardware compilation) to support
> up to Quad mode, using revision register.

So the octal-spi controller always has this config register, but the
quad-spi controller may or may not have this register ?

>>> +	rev_reg = readl(cqspi->iobase + CQSPI_REG_MODULEID);
>>> +	dev_info(dev, "CQSPI Module id %x\n", rev_reg);
>>> +
>>> +	switch (rev_reg & CQSPI_REG_MODULEID_CONF_ID_MASK) {
>>> +	case CQSPI_REG_MODULEID_CONF_ID_OCTAL_PHY:
>>> +	case CQSPI_REG_MODULEID_CONF_ID_OCTAL:
>>> +		mode = SPI_NOR_OCTAL;
>>> +		break;
>>> +	case CQSPI_REG_MODULEID_CONF_ID_QUAD:
>>> +	case CQSPI_REG_MODULEID_CONF_ID_QUAD_PHY:
>>
>> Does this work on all revisions of CQSPI ?
>>
> 
> After having a more thorough look at specification of older IP version
> (quad only) it seems, that revision register format has indeed changed.
> This will be fixed in the next version of the patch.

Can the quad-spi controller be configured only as dual or single ?
What about the octal one ? These cases should probably be handled
somehow too, right ?

>>> +		mode = SPI_NOR_QUAD;
>>> +		break;
>>> +	}
>>> +
>>> +	if (cqspi->max_mode == CQSPI_MAX_MODE_OCTAL)
>>> +		dt_mode = SPI_NOR_OCTAL;
>>> +
>>> +	if (mode == SPI_NOR_QUAD && dt_mode == SPI_NOR_OCTAL)
>>> +		dev_warn(dev, "Requested octal mode is not supported by the device.");
>>> +	else if (mode == SPI_NOR_OCTAL && dt_mode == SPI_NOR_QUAD)
>>> +		mode = SPI_NOR_QUAD;
>>> +
>>>  	/* Get flash device data */
>>>  	for_each_available_child_of_node(dev->of_node, np) {
>>>  		ret = of_property_read_u32(np, "reg", &cs);
>>> @@ -1123,7 +1183,7 @@ static int cqspi_setup_flash(struct cqspi_st *cqspi, struct device_node *np)
>>>  			goto err;
>>>  		}
>>>
>>> -		ret = spi_nor_scan(nor, NULL, SPI_NOR_QUAD);
>>> +		ret = spi_nor_scan(nor, NULL, mode);
>>>  		if (ret)
>>>  			goto err;
>>>
>>> @@ -1277,13 +1337,6 @@ static const struct dev_pm_ops cqspi__dev_pm_ops = {
>>>  #define CQSPI_DEV_PM_OPS	NULL
>>>  #endif
>>>
>>> -static struct of_device_id const cqspi_dt_ids[] = {
>>> -	{.compatible = "cdns,qspi-nor",},
>>> -	{ /* end of table */ }
>>> -};
>>> -
>>> -MODULE_DEVICE_TABLE(of, cqspi_dt_ids);
>>> -
>>>  static struct platform_driver cqspi_platform_driver = {
>>>  	.probe = cqspi_probe,
>>>  	.remove = cqspi_remove,
>>>
>>
>>
>> --
>> Best regards,
>> Marek Vasut


-- 
Best regards,
Marek Vasut

Re: [v2, 4/4] dt-bindings: mtd: Add Octal SPI support to Cadence QSPI.

[Marek Vasut]

On 03/10/2017 01:03 PM, Artur Jedrysek wrote:
> 
>> From: Marek Vasut [mailto:marek.vasut@gmail.com]
>> Sent: 10 March 2017 04:39
>> On 03/08/2017 09:05 AM, Artur Jedrysek wrote:
>>> This patch updates Cadence QSPI Device Tree documentation to include
>>> information about new compatible used to indicate, whether or not
>>> Octal SPI transfers are supported by the device.
>>>
>>> Signed-off-by: Artur Jedrysek <jartur@cadence.com>
>>> ---
>>> Changelog:
>>> v2: Use new compatible, instead of boolean property, to indicate
>>> Octal SPI support.
>>> ---
>>>  Documentation/devicetree/bindings/mtd/cadence-quadspi.txt | 4 +++-
>>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
>> b/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
>>> index f248056..41d1e98 100644
>>> --- a/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
>>> +++ b/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
>>> @@ -1,7 +1,9 @@
>>>  * Cadence Quad SPI controller
>>>
>>>  Required properties:
>>> -- compatible : Should be "cdns,qspi-nor".
>>> +- compatible : Should be "cdns,{qspi|ospi}-nor".
>>
>> Please explicitly list all compatibles , ie.
>> Should be "cdns,qspi-nor" or "cdns,ospi-nor".
>>
> 
> Two possible options are also explicitly listed right below. I did that
> in the way it was done in other driver for Cadence IP:
> /Documentation/devicetree/bindings/net/macb.txt
> Should I remove all of that and replace it with both options, separated
> by an "or" word?

Wait for Rob to confirm.

>> But I think the ospi is backward compatible with qspi, right ? So the
>> binding for ospi should list both, ie.
>> compatible = "cdns,ospi-nor", "cdns,qspi-nor";
>>
> 
> Yes, the ospi is backwards compatible with qspi. However, the sole point
> of introducing new compatible was to differentiate between them (which
> was previously done using a boolean flag, but that was criticized).
> Listing both bindings for ospi would only help for kernels not
> containing driver update being subject of this patch series, as both
> ospi and qspi are handled by the same driver - just with a slight
> difference. I apologize if I got something wrong here.

You got it right, I was just curious about the compatibility.

>>> +  Use "cdns,qspi-nor" for Quad SPI controller.
>>> +  Use "cdns,ospi-nor" for Octal SPI controller.
>>>  - reg : Contains two entries, each of which is a tuple consisting of a
>>>  	physical address and length. The first entry is the address and
>>>  	length of the controller register set. The second entry is the
>>>
>>
>>
>> --
>> Best regards,
>> Marek Vasut


-- 
Best regards,
Marek Vasut

RE: [v2, 2/4] mtd: spi-nor: Add Octal SPI support to Cadence QSPI driver.

[Artur Jedrysek]

From: Marek Vasut [mailto:marek.vasut@gmail.com]
Sent: 10 March 2017 13:50
> On 03/10/2017 01:00 PM, Artur Jedrysek wrote:
> >
> > From: Marek Vasut [mailto:marek.vasut@gmail.com]
> > Sent: 10 March 2017 04:37
> >> On 03/08/2017 09:02 AM, Artur Jedrysek wrote:
> >>> Recent versions of Cadence QSPI controller support Octal SPI transfers
> >>> as well. This patch updates existing driver to support such feature.
> >>>
> >>> It is not possible to determine whether or not octal mode is supported
> >>> just by looking at revision register alone. To solve that, an additional
> >>> compatible in Device Tree is added to indicate such capability.
> >>> Both (revision and compatible) are used to determine, which mode to
> >>> pass to spi_nor_scan() call.
> >>>
> >>> Signed-off-by: Artur Jedrysek <jartur@cadence.com>
> >>> ---
> >>> Changelog:
> >>> v2: Use new compatible in DT, instead of boolean property, to indicate
> >>> Octal SPI support.
> >>> Extracted Kconfig update to seperate patch.
> >>> ---
> >>>  drivers/mtd/spi-nor/cadence-quadspi.c | 69 +++++++++++++++++++++++++++++++----
> >>>  1 file changed, 61 insertions(+), 8 deletions(-)
> >>>
> >>> diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
> >>> index 9f8102d..a96471d 100644
> >>> --- a/drivers/mtd/spi-nor/cadence-quadspi.c
> >>> +++ b/drivers/mtd/spi-nor/cadence-quadspi.c
> >>> @@ -76,6 +76,7 @@ struct cqspi_st {
> >>>  	u32			fifo_depth;
> >>>  	u32			fifo_width;
> >>>  	u32			trigger_address;
> >>> +	u32			max_mode;
> >>
> >> I think it's time to introduce flags instead,
> >> ie. #define CQSPI_FLAG_SUPPORTS_OCTAL BIT(0)
> >>
> >>>  	struct cqspi_flash_pdata f_pdata[CQSPI_MAX_CHIPSELECT];
> >>>  };
> >>>
> >>> @@ -87,6 +88,10 @@ struct cqspi_st {
> >>>  #define CQSPI_INST_TYPE_SINGLE			0
> >>>  #define CQSPI_INST_TYPE_DUAL			1
> >>>  #define CQSPI_INST_TYPE_QUAD			2
> >>> +#define CQSPI_INST_TYPE_OCTAL			3
> >>> +
> >>> +#define CQSPI_MAX_MODE_QUAD			0
> >>> +#define CQSPI_MAX_MODE_OCTAL			1
> >>>
> >>>  #define CQSPI_DUMMY_CLKS_PER_BYTE		8
> >>>  #define CQSPI_DUMMY_BYTES_MAX			4
> >>> @@ -204,6 +209,14 @@ struct cqspi_st {
> >>>  #define CQSPI_REG_CMDWRITEDATALOWER		0xA8
> >>>  #define CQSPI_REG_CMDWRITEDATAUPPER		0xAC
> >>>
> >>> +#define CQSPI_REG_MODULEID			0xFC
> >>> +#define CQSPI_REG_MODULEID_CONF_ID_MASK		0x3
> >>> +#define CQSPI_REG_MODULEID_CONF_ID_LSB		0
> >>> +#define CQSPI_REG_MODULEID_CONF_ID_OCTAL_PHY	0x0
> >>> +#define CQSPI_REG_MODULEID_CONF_ID_OCTAL	0x1
> >>> +#define CQSPI_REG_MODULEID_CONF_ID_QUAD_PHY	0x2
> >>> +#define CQSPI_REG_MODULEID_CONF_ID_QUAD		0x3
> >>> +
> >>>  /* Interrupt status bits */
> >>>  #define CQSPI_REG_IRQ_MODE_ERR			BIT(0)
> >>>  #define CQSPI_REG_IRQ_UNDERFLOW			BIT(1)
> >>> @@ -866,6 +879,9 @@ static int cqspi_set_protocol(struct spi_nor *nor, const int read)
> >>>  		case SPI_NOR_QUAD:
> >>>  			f_pdata->data_width = CQSPI_INST_TYPE_QUAD;
> >>>  			break;
> >>> +		case SPI_NOR_OCTAL:
> >>> +			f_pdata->data_width = CQSPI_INST_TYPE_OCTAL;
> >>> +			break;
> >>>  		default:
> >>>  			return -EINVAL;
> >>>  		}
> >>> @@ -977,6 +993,17 @@ static int cqspi_write_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len)
> >>>  	return ret;
> >>>  }
> >>>
> >>> +static const u32 cqspi_max_mode_quad = CQSPI_MAX_MODE_QUAD;
> >>> +static const u32 cqspi_max_mode_octal = CQSPI_MAX_MODE_OCTAL;
> >>> +
> >>> +static struct of_device_id const cqspi_dt_ids[] = {
> >>> +	{ .compatible = "cdns,qspi-nor", .data = &cqspi_max_mode_quad },
> >>> +	{ .compatible = "cdns,ospi-nor", .data = &cqspi_max_mode_octal },
> >>
> >> .data = (void *)CQSPI_FLAG_SUPPORTS_OCTAL , then you don't need that
> >> static const stuff ...
> >>
> >
> > I had some doubts regarding that approach, as it may be dependent on the
> > CPU architecture (32-bit, 64-bit) and endianness. .data needs to be first
> > casted to unsigned long for reading, to ensure correct access and to
> > allow bitwise operations on it. That solution works, and if such approach
> > is preferred, then it will be used in next version of the patch.
> 
> Good
> 
> >>> +	{ /* end of table */ }
> >>> +};
> >>> +
> >>> +MODULE_DEVICE_TABLE(of, cqspi_dt_ids);
> >>> +
> >>>  static int cqspi_of_get_flash_pdata(struct platform_device *pdev,
> >>>  				    struct cqspi_flash_pdata *f_pdata,
> >>>  				    struct device_node *np)
> >>> @@ -1018,6 +1045,13 @@ static int cqspi_of_get_pdata(struct platform_device *pdev)
> >>>  {
> >>>  	struct device_node *np = pdev->dev.of_node;
> >>>  	struct cqspi_st *cqspi = platform_get_drvdata(pdev);
> >>> +	const struct of_device_id *match;
> >>> +
> >>> +	cqspi->max_mode = CQSPI_MAX_MODE_QUAD;
> >>> +
> >>> +	match = of_match_node(cqspi_dt_ids, np);
> >>> +	if (match && match->data)
> >>> +		cqspi->max_mode = *((u32 *)match->data);
> >>
> >> Use flags instead, see above.
> >>
> >>>  	cqspi->is_decoded_cs = of_property_read_bool(np, "cdns,is-decoded-cs");
> >>>
> >>> @@ -1074,9 +1108,35 @@ static int cqspi_setup_flash(struct cqspi_st *cqspi, struct device_node
> *np)
> >>>  	struct cqspi_flash_pdata *f_pdata;
> >>>  	struct spi_nor *nor;
> >>>  	struct mtd_info *mtd;
> >>> +	enum read_mode mode;
> >>> +	enum read_mode dt_mode = SPI_NOR_QUAD;
> >>>  	unsigned int cs;
> >>> +	unsigned int rev_reg;
> >>>  	int i, ret;
> >>>
> >>> +	/* Determine, whether or not octal transfer MAY be supported */
> >>
> >> But you already know that from DT, no ?
> >>
> >
> > Cadence Octal (formerly Quad) SPI Controller is sold as an IP, and is
> > configurable. This includes max SPI mode. It is possible to detect, that
> > Octal SPI controller is configured (during hardware compilation) to support
> > up to Quad mode, using revision register.
> 
> So the octal-spi controller always has this config register, but the
> quad-spi controller may or may not have this register ?
> 

This register was always present. In Quad-SPI, however, it didn't contain
information about maximum possible mode, as only quad was possible, and
meaning of bits checked here was different.

> >>> +	rev_reg = readl(cqspi->iobase + CQSPI_REG_MODULEID);
> >>> +	dev_info(dev, "CQSPI Module id %x\n", rev_reg);
> >>> +
> >>> +	switch (rev_reg & CQSPI_REG_MODULEID_CONF_ID_MASK) {
> >>> +	case CQSPI_REG_MODULEID_CONF_ID_OCTAL_PHY:
> >>> +	case CQSPI_REG_MODULEID_CONF_ID_OCTAL:
> >>> +		mode = SPI_NOR_OCTAL;
> >>> +		break;
> >>> +	case CQSPI_REG_MODULEID_CONF_ID_QUAD:
> >>> +	case CQSPI_REG_MODULEID_CONF_ID_QUAD_PHY:
> >>
> >> Does this work on all revisions of CQSPI ?
> >>
> >
> > After having a more thorough look at specification of older IP version
> > (quad only) it seems, that revision register format has indeed changed.
> > This will be fixed in the next version of the patch.
> 
> Can the quad-spi controller be configured only as dual or single ?
> What about the octal one ? These cases should probably be handled
> somehow too, right ?
> 

Quad-SPI controller can always support single, dual and quad. There was
no option to configure max mode. Octal-SPI controller can be configured
to support either octal or quad mode. No controller could be configured 
(during hardware compilation/synthesis) to support only single/dual 
SPI mode. To put it shortly: single, dual and quad is always supported.

> >>> +		mode = SPI_NOR_QUAD;
> >>> +		break;
> >>> +	}
> >>> +
> >>> +	if (cqspi->max_mode == CQSPI_MAX_MODE_OCTAL)
> >>> +		dt_mode = SPI_NOR_OCTAL;
> >>> +
> >>> +	if (mode == SPI_NOR_QUAD && dt_mode == SPI_NOR_OCTAL)
> >>> +		dev_warn(dev, "Requested octal mode is not supported by the device.");
> >>> +	else if (mode == SPI_NOR_OCTAL && dt_mode == SPI_NOR_QUAD)
> >>> +		mode = SPI_NOR_QUAD;
> >>> +
> >>>  	/* Get flash device data */
> >>>  	for_each_available_child_of_node(dev->of_node, np) {
> >>>  		ret = of_property_read_u32(np, "reg", &cs);
> >>> @@ -1123,7 +1183,7 @@ static int cqspi_setup_flash(struct cqspi_st *cqspi, struct device_node *np)
> >>>  			goto err;
> >>>  		}
> >>>
> >>> -		ret = spi_nor_scan(nor, NULL, SPI_NOR_QUAD);
> >>> +		ret = spi_nor_scan(nor, NULL, mode);
> >>>  		if (ret)
> >>>  			goto err;
> >>>
> >>> @@ -1277,13 +1337,6 @@ static const struct dev_pm_ops cqspi__dev_pm_ops = {
> >>>  #define CQSPI_DEV_PM_OPS	NULL
> >>>  #endif
> >>>
> >>> -static struct of_device_id const cqspi_dt_ids[] = {
> >>> -	{.compatible = "cdns,qspi-nor",},
> >>> -	{ /* end of table */ }
> >>> -};
> >>> -
> >>> -MODULE_DEVICE_TABLE(of, cqspi_dt_ids);
> >>> -
> >>>  static struct platform_driver cqspi_platform_driver = {
> >>>  	.probe = cqspi_probe,
> >>>  	.remove = cqspi_remove,
> >>>
> >>
> >>
> >> --
> >> Best regards,
> >> Marek Vasut
> 
> 
> --
> Best regards,
> Marek Vasut

Re: [v2, 2/4] mtd: spi-nor: Add Octal SPI support to Cadence QSPI driver.

[Marek Vasut]

On 03/10/2017 03:09 PM, Artur Jedrysek wrote:

CCing Dinh.

[...]

>>>>> +	/* Determine, whether or not octal transfer MAY be supported */
>>>>
>>>> But you already know that from DT, no ?
>>>>
>>>
>>> Cadence Octal (formerly Quad) SPI Controller is sold as an IP, and is
>>> configurable. This includes max SPI mode. It is possible to detect, that
>>> Octal SPI controller is configured (during hardware compilation) to support
>>> up to Quad mode, using revision register.
>>
>> So the octal-spi controller always has this config register, but the
>> quad-spi controller may or may not have this register ?
>>
> 
> This register was always present. In Quad-SPI, however, it didn't contain
> information about maximum possible mode, as only quad was possible, and
> meaning of bits checked here was different.

OK

>>>>> +	rev_reg = readl(cqspi->iobase + CQSPI_REG_MODULEID);
>>>>> +	dev_info(dev, "CQSPI Module id %x\n", rev_reg);
>>>>> +
>>>>> +	switch (rev_reg & CQSPI_REG_MODULEID_CONF_ID_MASK) {
>>>>> +	case CQSPI_REG_MODULEID_CONF_ID_OCTAL_PHY:
>>>>> +	case CQSPI_REG_MODULEID_CONF_ID_OCTAL:
>>>>> +		mode = SPI_NOR_OCTAL;
>>>>> +		break;
>>>>> +	case CQSPI_REG_MODULEID_CONF_ID_QUAD:
>>>>> +	case CQSPI_REG_MODULEID_CONF_ID_QUAD_PHY:
>>>>
>>>> Does this work on all revisions of CQSPI ?
>>>>
>>>
>>> After having a more thorough look at specification of older IP version
>>> (quad only) it seems, that revision register format has indeed changed.
>>> This will be fixed in the next version of the patch.
>>
>> Can the quad-spi controller be configured only as dual or single ?
>> What about the octal one ? These cases should probably be handled
>> somehow too, right ?
>>
> 
> Quad-SPI controller can always support single, dual and quad. There was
> no option to configure max mode. Octal-SPI controller can be configured
> to support either octal or quad mode. No controller could be configured 
> (during hardware compilation/synthesis) to support only single/dual 
> SPI mode. To put it shortly: single, dual and quad is always supported.

So basically the whole check you need to perform here is

mode = quad;
if (controller->flags & CAN_DO_OCTAL) {
 if (readl(ID_REGISTER) & IS_CONFIGURED_AS_OCTAL)
   mode = octal;
}

-- 
Best regards,
Marek Vasut

RE: [v2, 2/4] mtd: spi-nor: Add Octal SPI support to Cadence QSPI driver.

[Artur Jedrysek]

From: Marek Vasut [mailto:marek.vasut@gmail.com]
Sent: 10 marca 2017 15:15

> On 03/10/2017 03:09 PM, Artur Jedrysek wrote:
> 
> CCing Dinh.
> 
> [...]
> 
> >>>>> +	/* Determine, whether or not octal transfer MAY be supported */
> >>>>
> >>>> But you already know that from DT, no ?
> >>>>
> >>>
> >>> Cadence Octal (formerly Quad) SPI Controller is sold as an IP, and is
> >>> configurable. This includes max SPI mode. It is possible to detect, that
> >>> Octal SPI controller is configured (during hardware compilation) to support
> >>> up to Quad mode, using revision register.
> >>
> >> So the octal-spi controller always has this config register, but the
> >> quad-spi controller may or may not have this register ?
> >>
> >
> > This register was always present. In Quad-SPI, however, it didn't contain
> > information about maximum possible mode, as only quad was possible, and
> > meaning of bits checked here was different.
> 
> OK
> 
> >>>>> +	rev_reg = readl(cqspi->iobase + CQSPI_REG_MODULEID);
> >>>>> +	dev_info(dev, "CQSPI Module id %x\n", rev_reg);
> >>>>> +
> >>>>> +	switch (rev_reg & CQSPI_REG_MODULEID_CONF_ID_MASK) {
> >>>>> +	case CQSPI_REG_MODULEID_CONF_ID_OCTAL_PHY:
> >>>>> +	case CQSPI_REG_MODULEID_CONF_ID_OCTAL:
> >>>>> +		mode = SPI_NOR_OCTAL;
> >>>>> +		break;
> >>>>> +	case CQSPI_REG_MODULEID_CONF_ID_QUAD:
> >>>>> +	case CQSPI_REG_MODULEID_CONF_ID_QUAD_PHY:
> >>>>
> >>>> Does this work on all revisions of CQSPI ?
> >>>>
> >>>
> >>> After having a more thorough look at specification of older IP version
> >>> (quad only) it seems, that revision register format has indeed changed.
> >>> This will be fixed in the next version of the patch.
> >>
> >> Can the quad-spi controller be configured only as dual or single ?
> >> What about the octal one ? These cases should probably be handled
> >> somehow too, right ?
> >>
> >
> > Quad-SPI controller can always support single, dual and quad. There was
> > no option to configure max mode. Octal-SPI controller can be configured
> > to support either octal or quad mode. No controller could be configured
> > (during hardware compilation/synthesis) to support only single/dual
> > SPI mode. To put it shortly: single, dual and quad is always supported.
> 
> So basically the whole check you need to perform here is
> 
> mode = quad;
> if (controller->flags & CAN_DO_OCTAL) {
>  if (readl(ID_REGISTER) & IS_CONFIGURED_AS_OCTAL)
>    mode = octal;
> }
> 

Yes. Something similar was already written and tested, and will be sent
in the next version of the patch instead.

> --
> Best regards,
> Marek Vasut

Re: [v2, 4/4] dt-bindings: mtd: Add Octal SPI support to Cadence QSPI.

[Rob Herring]

On Fri, Mar 10, 2017 at 01:52:31PM +0100, Marek Vasut wrote:
> On 03/10/2017 01:03 PM, Artur Jedrysek wrote:
> > 
> >> From: Marek Vasut [mailto:marek.vasut@gmail.com]
> >> Sent: 10 March 2017 04:39
> >> On 03/08/2017 09:05 AM, Artur Jedrysek wrote:
> >>> This patch updates Cadence QSPI Device Tree documentation to include
> >>> information about new compatible used to indicate, whether or not
> >>> Octal SPI transfers are supported by the device.
> >>>
> >>> Signed-off-by: Artur Jedrysek <jartur@cadence.com>
> >>> ---
> >>> Changelog:
> >>> v2: Use new compatible, instead of boolean property, to indicate
> >>> Octal SPI support.
> >>> ---
> >>>  Documentation/devicetree/bindings/mtd/cadence-quadspi.txt | 4 +++-
> >>>  1 file changed, 3 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
> >> b/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
> >>> index f248056..41d1e98 100644
> >>> --- a/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
> >>> +++ b/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
> >>> @@ -1,7 +1,9 @@
> >>>  * Cadence Quad SPI controller
> >>>
> >>>  Required properties:
> >>> -- compatible : Should be "cdns,qspi-nor".
> >>> +- compatible : Should be "cdns,{qspi|ospi}-nor".
> >>
> >> Please explicitly list all compatibles , ie.
> >> Should be "cdns,qspi-nor" or "cdns,ospi-nor".
> >>
> > 
> > Two possible options are also explicitly listed right below. I did that
> > in the way it was done in other driver for Cadence IP:
> > /Documentation/devicetree/bindings/net/macb.txt
> > Should I remove all of that and replace it with both options, separated
> > by an "or" word?
> 
> Wait for Rob to confirm.

As Marek suggested, but with one valid combination per line. I don't 
think checkpatch.pl will work with it as is.

> 
> >> But I think the ospi is backward compatible with qspi, right ? So the
> >> binding for ospi should list both, ie.
> >> compatible = "cdns,ospi-nor", "cdns,qspi-nor";
> >>
> > 
> > Yes, the ospi is backwards compatible with qspi. However, the sole point
> > of introducing new compatible was to differentiate between them (which
> > was previously done using a boolean flag, but that was criticized).
> > Listing both bindings for ospi would only help for kernels not
> > containing driver update being subject of this patch series, as both
> > ospi and qspi are handled by the same driver - just with a slight
> > difference. I apologize if I got something wrong here.
> 
> You got it right, I was just curious about the compatibility.

You are assuming every OS/user everywhere is updated. It doesn't hurt to 
have the backwards compatibility.

Rob

[PATCH v3, 1/4] mtd: spi-nor: Add support for Octal SPI mode.

[Artur Jedrysek]

This patch adds support for Octal SPI data reads in SPI NOR framework.
Opcodes for programming using octal interface are also present for the
sake of completeness, despite not being used.

Micron mt35xu512 flash is added as an example chip supporting that mode.

Signed-off-by: Artur Jedrysek <jartur@cadence.com>
---
Changelog:
v2: None.
---
v3: None
---
 drivers/mtd/spi-nor/spi-nor.c | 20 ++++++++++++++++++--
 include/linux/mtd/spi-nor.h   |  9 +++++++++
 2 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 1ae872b..db188e2 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -85,6 +85,7 @@ struct flash_info {
 					 * Use dedicated 4byte address op codes
 					 * to support memory size above 128Mib.
 					 */
+#define SPI_NOR_OCTAL_READ	BIT(12)	/* Flash supports Octal Read */
 };
 
 #define JEDEC_MFR(info)	((info)->id[0])
@@ -159,6 +160,7 @@ static inline int spi_nor_read_dummy_cycles(struct spi_nor *nor)
 	case SPI_NOR_FAST:
 	case SPI_NOR_DUAL:
 	case SPI_NOR_QUAD:
+	case SPI_NOR_OCTAL:
 		return 8;
 	case SPI_NOR_NORMAL:
 		return 0;
@@ -220,6 +222,8 @@ static inline u8 spi_nor_convert_3to4_read(u8 opcode)
 		{ SPINOR_OP_READ_1_2_2,	SPINOR_OP_READ_1_2_2_4B },
 		{ SPINOR_OP_READ_1_1_4,	SPINOR_OP_READ_1_1_4_4B },
 		{ SPINOR_OP_READ_1_4_4,	SPINOR_OP_READ_1_4_4_4B },
+		{ SPINOR_OP_READ_1_1_8,	SPINOR_OP_READ_1_1_8_4B },
+		{ SPINOR_OP_READ_1_8_8,	SPINOR_OP_READ_1_8_8_4B },
 	};
 
 	return spi_nor_convert_opcode(opcode, spi_nor_3to4_read,
@@ -232,6 +236,8 @@ static inline u8 spi_nor_convert_3to4_program(u8 opcode)
 		{ SPINOR_OP_PP,		SPINOR_OP_PP_4B },
 		{ SPINOR_OP_PP_1_1_4,	SPINOR_OP_PP_1_1_4_4B },
 		{ SPINOR_OP_PP_1_4_4,	SPINOR_OP_PP_1_4_4_4B },
+		{ SPINOR_OP_PP_1_1_8,	SPINOR_OP_PP_1_1_8_4B },
+		{ SPINOR_OP_PP_1_8_8,	SPINOR_OP_PP_1_8_8_4B },
 	};
 
 	return spi_nor_convert_opcode(opcode, spi_nor_3to4_program,
@@ -1035,6 +1041,11 @@ static const struct flash_info spi_nor_ids[] = {
 	{ "n25q512ax3",  INFO(0x20ba20, 0, 64 * 1024, 1024, SECT_4K | USE_FSR | SPI_NOR_QUAD_READ) },
 	{ "n25q00",      INFO(0x20ba21, 0, 64 * 1024, 2048, SECT_4K | USE_FSR | SPI_NOR_QUAD_READ) },
 	{ "n25q00a",     INFO(0x20bb21, 0, 64 * 1024, 2048, SECT_4K | USE_FSR | SPI_NOR_QUAD_READ) },
+	{
+		"mt35xu512", INFO(0x2c5b1a, 0, 128 * 1024, 512,
+			SECT_4K | USE_FSR | SPI_NOR_OCTAL_READ |
+			SPI_NOR_4B_OPCODES)
+	},
 
 	/* PMC */
 	{ "pm25lv512",   INFO(0,        0, 32 * 1024,    2, SECT_4K_PMC) },
@@ -1667,8 +1678,10 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
 	if (info->flags & SPI_NOR_NO_FR)
 		nor->flash_read = SPI_NOR_NORMAL;
 
-	/* Quad/Dual-read mode takes precedence over fast/normal */
-	if (mode == SPI_NOR_QUAD && info->flags & SPI_NOR_QUAD_READ) {
+	/* Octal/Quad/Dual-read mode takes precedence over fast/normal */
+	if (mode == SPI_NOR_OCTAL && info->flags & SPI_NOR_OCTAL_READ) {
+		nor->flash_read = SPI_NOR_OCTAL;
+	} else if (mode == SPI_NOR_QUAD && info->flags & SPI_NOR_QUAD_READ) {
 		ret = set_quad_mode(nor, info);
 		if (ret) {
 			dev_err(dev, "quad mode not supported\n");
@@ -1681,6 +1694,9 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
 
 	/* Default commands */
 	switch (nor->flash_read) {
+	case SPI_NOR_OCTAL:
+		nor->read_opcode = SPINOR_OP_READ_1_1_8;
+		break;
 	case SPI_NOR_QUAD:
 		nor->read_opcode = SPINOR_OP_READ_1_1_4;
 		break;
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index f2a7180..aad6318 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -47,9 +47,13 @@
 #define SPINOR_OP_READ_1_2_2	0xbb	/* Read data bytes (Dual I/O SPI) */
 #define SPINOR_OP_READ_1_1_4	0x6b	/* Read data bytes (Quad Output SPI) */
 #define SPINOR_OP_READ_1_4_4	0xeb	/* Read data bytes (Quad I/O SPI) */
+#define SPINOR_OP_READ_1_1_8	0x8b	/* Read data bytes (Octal Output SPI) */
+#define SPINOR_OP_READ_1_8_8	0xcb	/* Read data bytes (Octal I/O SPI) */
 #define SPINOR_OP_PP		0x02	/* Page program (up to 256 bytes) */
 #define SPINOR_OP_PP_1_1_4	0x32	/* Quad page program */
 #define SPINOR_OP_PP_1_4_4	0x38	/* Quad page program */
+#define SPINOR_OP_PP_1_1_8	0x82	/* Octal page program */
+#define SPINOR_OP_PP_1_8_8	0xc2	/* Octal page program */
 #define SPINOR_OP_BE_4K		0x20	/* Erase 4KiB block */
 #define SPINOR_OP_BE_4K_PMC	0xd7	/* Erase 4KiB block on PMC chips */
 #define SPINOR_OP_BE_32K	0x52	/* Erase 32KiB block */
@@ -66,9 +70,13 @@
 #define SPINOR_OP_READ_1_2_2_4B	0xbc	/* Read data bytes (Dual I/O SPI) */
 #define SPINOR_OP_READ_1_1_4_4B	0x6c	/* Read data bytes (Quad Output SPI) */
 #define SPINOR_OP_READ_1_4_4_4B	0xec	/* Read data bytes (Quad I/O SPI) */
+#define SPINOR_OP_READ_1_1_8_4B	0x7c	/* Read data bytes (Octal Output SPI) */
+#define SPINOR_OP_READ_1_8_8_4B	0xcc	/* Read data bytes (Octal I/O SPI) */
 #define SPINOR_OP_PP_4B		0x12	/* Page program (up to 256 bytes) */
 #define SPINOR_OP_PP_1_1_4_4B	0x34	/* Quad page program */
 #define SPINOR_OP_PP_1_4_4_4B	0x3e	/* Quad page program */
+#define SPINOR_OP_PP_1_1_8_4B	0x84	/* Octal page program */
+#define SPINOR_OP_PP_1_8_8_4B	0x8e	/* Octal page program */
 #define SPINOR_OP_BE_4K_4B	0x21	/* Erase 4KiB block */
 #define SPINOR_OP_BE_32K_4B	0x5c	/* Erase 32KiB block */
 #define SPINOR_OP_SE_4B		0xdc	/* Sector erase (usually 64KiB) */
@@ -124,6 +132,7 @@ enum read_mode {
 	SPI_NOR_FAST,
 	SPI_NOR_DUAL,
 	SPI_NOR_QUAD,
+	SPI_NOR_OCTAL,
 };
 
 #define SPI_NOR_MAX_CMD_SIZE	8
-- 
2.2.2

[PATCH v3, 2/4] mtd: spi-nor: Add Octal SPI support to Cadence QSPI driver.

[Artur Jedrysek]

Recent versions of Cadence QSPI controller support Octal SPI transfers
as well. This patch updates existing driver to support such feature.

It is not possible to determine whether or not octal mode is supported
just by looking at revision register alone. To solve that, an additional
compatible in Device Tree is added to indicate such capability.
Both (revision and compatible) are used to determine, which mode to
pass to spi_nor_scan() call.

Signed-off-by: Artur Jedrysek <jartur@cadence.com>
---
Changelog:
v2: Use new compatible in DT, instead of boolean property, to indicate
Octal SPI support.
Extracted Kconfig update to seperate patch.
---
v3: Fixed handling revision register.
---
 drivers/mtd/spi-nor/cadence-quadspi.c | 56 ++++++++++++++++++++++++++++++-----
 1 file changed, 48 insertions(+), 8 deletions(-)

diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
index 9f8102d..a1561d0 100644
--- a/drivers/mtd/spi-nor/cadence-quadspi.c
+++ b/drivers/mtd/spi-nor/cadence-quadspi.c
@@ -76,6 +76,7 @@ struct cqspi_st {
 	u32			fifo_depth;
 	u32			fifo_width;
 	u32			trigger_address;
+	u32			caps;
 	struct cqspi_flash_pdata f_pdata[CQSPI_MAX_CHIPSELECT];
 };
 
@@ -87,6 +88,9 @@ struct cqspi_st {
 #define CQSPI_INST_TYPE_SINGLE			0
 #define CQSPI_INST_TYPE_DUAL			1
 #define CQSPI_INST_TYPE_QUAD			2
+#define CQSPI_INST_TYPE_OCTAL			3
+
+#define CQSPI_FLAG_SUPPORTS_OCTAL		BIT(0)
 
 #define CQSPI_DUMMY_CLKS_PER_BYTE		8
 #define CQSPI_DUMMY_BYTES_MAX			4
@@ -204,6 +208,14 @@ struct cqspi_st {
 #define CQSPI_REG_CMDWRITEDATALOWER		0xA8
 #define CQSPI_REG_CMDWRITEDATAUPPER		0xAC
 
+#define CQSPI_REG_MODULEID			0xFC
+#define CQSPI_REG_MODULEID_CONF_ID_MASK		0x3
+#define CQSPI_REG_MODULEID_CONF_ID_LSB		0
+#define CQSPI_REG_MODULEID_CONF_ID_OCTAL_PHY	0x0
+#define CQSPI_REG_MODULEID_CONF_ID_OCTAL	0x1
+#define CQSPI_REG_MODULEID_CONF_ID_QUAD_PHY	0x2
+#define CQSPI_REG_MODULEID_CONF_ID_QUAD		0x3
+
 /* Interrupt status bits */
 #define CQSPI_REG_IRQ_MODE_ERR			BIT(0)
 #define CQSPI_REG_IRQ_UNDERFLOW			BIT(1)
@@ -866,6 +878,9 @@ static int cqspi_set_protocol(struct spi_nor *nor, const int read)
 		case SPI_NOR_QUAD:
 			f_pdata->data_width = CQSPI_INST_TYPE_QUAD;
 			break;
+		case SPI_NOR_OCTAL:
+			f_pdata->data_width = CQSPI_INST_TYPE_OCTAL;
+			break;
 		default:
 			return -EINVAL;
 		}
@@ -977,6 +992,17 @@ static int cqspi_write_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len)
 	return ret;
 }
 
+static struct of_device_id const cqspi_dt_ids[] = {
+	{ .compatible = "cdns,qspi-nor", .data = (void *)0 },
+	{
+		.compatible = "cdns,ospi-nor",
+		.data = (void *)CQSPI_FLAG_SUPPORTS_OCTAL
+	},
+	{ /* end of table */ }
+};
+
+MODULE_DEVICE_TABLE(of, cqspi_dt_ids);
+
 static int cqspi_of_get_flash_pdata(struct platform_device *pdev,
 				    struct cqspi_flash_pdata *f_pdata,
 				    struct device_node *np)
@@ -1018,6 +1044,12 @@ static int cqspi_of_get_pdata(struct platform_device *pdev)
 {
 	struct device_node *np = pdev->dev.of_node;
 	struct cqspi_st *cqspi = platform_get_drvdata(pdev);
+	const struct of_device_id *match;
+
+	match = of_match_node(cqspi_dt_ids, np);
+	if (match && match->data)
+		cqspi->caps |= (unsigned long)match->data &
+				CQSPI_FLAG_SUPPORTS_OCTAL;
 
 	cqspi->is_decoded_cs = of_property_read_bool(np, "cdns,is-decoded-cs");
 
@@ -1074,9 +1106,23 @@ static int cqspi_setup_flash(struct cqspi_st *cqspi, struct device_node *np)
 	struct cqspi_flash_pdata *f_pdata;
 	struct spi_nor *nor;
 	struct mtd_info *mtd;
+	enum read_mode mode = SPI_NOR_QUAD;
 	unsigned int cs;
+	unsigned int rev_reg;
 	int i, ret;
 
+	if (cqspi->caps & CQSPI_FLAG_SUPPORTS_OCTAL) {
+		/* Determine, whether or not octal transfer MAY be supported */
+		rev_reg = readl(cqspi->iobase + CQSPI_REG_MODULEID);
+		dev_info(dev, "CQSPI Module id %x\n", rev_reg);
+
+		switch (rev_reg & CQSPI_REG_MODULEID_CONF_ID_MASK) {
+		case CQSPI_REG_MODULEID_CONF_ID_OCTAL_PHY:
+		case CQSPI_REG_MODULEID_CONF_ID_OCTAL:
+			mode = SPI_NOR_OCTAL;
+		}
+	}
+
 	/* Get flash device data */
 	for_each_available_child_of_node(dev->of_node, np) {
 		ret = of_property_read_u32(np, "reg", &cs);
@@ -1123,7 +1169,7 @@ static int cqspi_setup_flash(struct cqspi_st *cqspi, struct device_node *np)
 			goto err;
 		}
 
-		ret = spi_nor_scan(nor, NULL, SPI_NOR_QUAD);
+		ret = spi_nor_scan(nor, NULL, mode);
 		if (ret)
 			goto err;
 
@@ -1160,6 +1206,7 @@ static int cqspi_probe(struct platform_device *pdev)
 	mutex_init(&cqspi->bus_mutex);
 	cqspi->pdev = pdev;
 	platform_set_drvdata(pdev, cqspi);
+	cqspi->caps = 0;
 
 	/* Obtain configuration from OF. */
 	ret = cqspi_of_get_pdata(pdev);
@@ -1277,13 +1324,6 @@ static const struct dev_pm_ops cqspi__dev_pm_ops = {
 #define CQSPI_DEV_PM_OPS	NULL
 #endif
 
-static struct of_device_id const cqspi_dt_ids[] = {
-	{.compatible = "cdns,qspi-nor",},
-	{ /* end of table */ }
-};
-
-MODULE_DEVICE_TABLE(of, cqspi_dt_ids);
-
 static struct platform_driver cqspi_platform_driver = {
 	.probe = cqspi_probe,
 	.remove = cqspi_remove,
-- 
2.2.2

[PATCH v3, 3/4] mtd: spi-nor: Add Xtensa CPU support for cadence-quadspi.

[Artur Jedrysek]

Cadence Quad SPI driver successfully runs on Xtensa CPU, and Kconfig
file is updated to indicate that.

Signed-off-by: Artur Jedrysek <jartur@cadence.com>
---
Changelog:
v2: This change is extracted from previous patch (updating CQSPI driver).
---
v3: None
---
 drivers/mtd/spi-nor/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
index 7252087..f195749 100644
--- a/drivers/mtd/spi-nor/Kconfig
+++ b/drivers/mtd/spi-nor/Kconfig
@@ -50,7 +50,7 @@ config SPI_ATMEL_QUADSPI
 
 config SPI_CADENCE_QUADSPI
 	tristate "Cadence Quad SPI controller"
-	depends on OF && (ARM || COMPILE_TEST)
+	depends on OF && (ARM || XTENSA || COMPILE_TEST)
 	help
 	  Enable support for the Cadence Quad SPI Flash controller.
 
-- 
2.2.2

[PATCH v3, 4/4] dt-bindings: mtd: Add Octal SPI support to Cadence QSPI.

[Artur Jedrysek]

This patch updates Cadence QSPI Device Tree documentation to include
information about new compatible used to indicate, whether or not
Octal SPI transfers are supported by the device.

Signed-off-by: Artur Jedrysek <jartur@cadence.com>
---
Changelog:
v2: Use new compatible, instead of boolean property, to indicate
Octal SPI support.
---
v3: Changes according to comments.
---
 Documentation/devicetree/bindings/mtd/cadence-quadspi.txt | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt b/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
index f248056..298b0a3 100644
--- a/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
+++ b/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
@@ -1,7 +1,9 @@
 * Cadence Quad SPI controller
 
 Required properties:
-- compatible : Should be "cdns,qspi-nor".
+- compatible : Should be one of:
+  - "cdns,qspi-nor" for Quad SPI controller.
+  - "cdns,ospi-nor", "cdns,qspi-nor" for Octal SPI controller.
 - reg : Contains two entries, each of which is a tuple consisting of a
 	physical address and length. The first entry is the address and
 	length of the controller register set. The second entry is the
-- 
2.2.2

Re: [PATCH v3, 3/4] mtd: spi-nor: Add Xtensa CPU support for cadence-quadspi.

[Marek Vasut]

On 03/20/2017 12:26 PM, Artur Jedrysek wrote:
> Cadence Quad SPI driver successfully runs on Xtensa CPU, and Kconfig
> file is updated to indicate that.
> 
> Signed-off-by: Artur Jedrysek <jartur@cadence.com>

Acked-by: Marek Vasut <marek.vasut@gmail.com>

> ---
> Changelog:
> v2: This change is extracted from previous patch (updating CQSPI driver).
> ---
> v3: None
> ---
>  drivers/mtd/spi-nor/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
> index 7252087..f195749 100644
> --- a/drivers/mtd/spi-nor/Kconfig
> +++ b/drivers/mtd/spi-nor/Kconfig
> @@ -50,7 +50,7 @@ config SPI_ATMEL_QUADSPI
>  
>  config SPI_CADENCE_QUADSPI
>  	tristate "Cadence Quad SPI controller"
> -	depends on OF && (ARM || COMPILE_TEST)
> +	depends on OF && (ARM || XTENSA || COMPILE_TEST)
>  	help
>  	  Enable support for the Cadence Quad SPI Flash controller.
>  
> 


-- 
Best regards,
Marek Vasut

Re: [PATCH v3, 2/4] mtd: spi-nor: Add Octal SPI support to Cadence QSPI driver.

[Marek Vasut]

On 03/20/2017 12:25 PM, Artur Jedrysek wrote:
> Recent versions of Cadence QSPI controller support Octal SPI transfers
> as well. This patch updates existing driver to support such feature.
> 
> It is not possible to determine whether or not octal mode is supported
> just by looking at revision register alone. To solve that, an additional
> compatible in Device Tree is added to indicate such capability.
> Both (revision and compatible) are used to determine, which mode to
> pass to spi_nor_scan() call.
> 
> Signed-off-by: Artur Jedrysek <jartur@cadence.com>
> ---
> Changelog:
> v2: Use new compatible in DT, instead of boolean property, to indicate
> Octal SPI support.
> Extracted Kconfig update to seperate patch.
> ---
> v3: Fixed handling revision register.
> ---
>  drivers/mtd/spi-nor/cadence-quadspi.c | 56 ++++++++++++++++++++++++++++++-----
>  1 file changed, 48 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c b/drivers/mtd/spi-nor/cadence-quadspi.c
> index 9f8102d..a1561d0 100644
> --- a/drivers/mtd/spi-nor/cadence-quadspi.c
> +++ b/drivers/mtd/spi-nor/cadence-quadspi.c
> @@ -76,6 +76,7 @@ struct cqspi_st {
>  	u32			fifo_depth;
>  	u32			fifo_width;
>  	u32			trigger_address;
> +	u32			caps;

Please rename it to flags to be consistent with CQSPI_FLAG_ ...

>  	struct cqspi_flash_pdata f_pdata[CQSPI_MAX_CHIPSELECT];
>  };
>  
> @@ -87,6 +88,9 @@ struct cqspi_st {
>  #define CQSPI_INST_TYPE_SINGLE			0
>  #define CQSPI_INST_TYPE_DUAL			1
>  #define CQSPI_INST_TYPE_QUAD			2
> +#define CQSPI_INST_TYPE_OCTAL			3
> +
> +#define CQSPI_FLAG_SUPPORTS_OCTAL		BIT(0)
>  
>  #define CQSPI_DUMMY_CLKS_PER_BYTE		8
>  #define CQSPI_DUMMY_BYTES_MAX			4
> @@ -204,6 +208,14 @@ struct cqspi_st {
>  #define CQSPI_REG_CMDWRITEDATALOWER		0xA8
>  #define CQSPI_REG_CMDWRITEDATAUPPER		0xAC
>  
> +#define CQSPI_REG_MODULEID			0xFC
> +#define CQSPI_REG_MODULEID_CONF_ID_MASK		0x3
> +#define CQSPI_REG_MODULEID_CONF_ID_LSB		0
> +#define CQSPI_REG_MODULEID_CONF_ID_OCTAL_PHY	0x0
> +#define CQSPI_REG_MODULEID_CONF_ID_OCTAL	0x1
> +#define CQSPI_REG_MODULEID_CONF_ID_QUAD_PHY	0x2
> +#define CQSPI_REG_MODULEID_CONF_ID_QUAD		0x3
> +
>  /* Interrupt status bits */
>  #define CQSPI_REG_IRQ_MODE_ERR			BIT(0)
>  #define CQSPI_REG_IRQ_UNDERFLOW			BIT(1)
> @@ -866,6 +878,9 @@ static int cqspi_set_protocol(struct spi_nor *nor, const int read)
>  		case SPI_NOR_QUAD:
>  			f_pdata->data_width = CQSPI_INST_TYPE_QUAD;
>  			break;
> +		case SPI_NOR_OCTAL:
> +			f_pdata->data_width = CQSPI_INST_TYPE_OCTAL;
> +			break;
>  		default:
>  			return -EINVAL;
>  		}
> @@ -977,6 +992,17 @@ static int cqspi_write_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len)
>  	return ret;
>  }
>  
> +static struct of_device_id const cqspi_dt_ids[] = {
> +	{ .compatible = "cdns,qspi-nor", .data = (void *)0 },

Can you make the formatting for qspi and ospi the same ? Minor nit here
of course ...

> +	{
> +		.compatible = "cdns,ospi-nor",
> +		.data = (void *)CQSPI_FLAG_SUPPORTS_OCTAL
> +	},
> +	{ /* end of table */ }
> +};
> +
> +MODULE_DEVICE_TABLE(of, cqspi_dt_ids);
> +
>  static int cqspi_of_get_flash_pdata(struct platform_device *pdev,
>  				    struct cqspi_flash_pdata *f_pdata,
>  				    struct device_node *np)
> @@ -1018,6 +1044,12 @@ static int cqspi_of_get_pdata(struct platform_device *pdev)
>  {
>  	struct device_node *np = pdev->dev.of_node;
>  	struct cqspi_st *cqspi = platform_get_drvdata(pdev);
> +	const struct of_device_id *match;
> +
> +	match = of_match_node(cqspi_dt_ids, np);
> +	if (match && match->data)
> +		cqspi->caps |= (unsigned long)match->data &
> +				CQSPI_FLAG_SUPPORTS_OCTAL;

Just use match->data, without clearing any bits .

>  	cqspi->is_decoded_cs = of_property_read_bool(np, "cdns,is-decoded-cs");
>  
> @@ -1074,9 +1106,23 @@ static int cqspi_setup_flash(struct cqspi_st *cqspi, struct device_node *np)
>  	struct cqspi_flash_pdata *f_pdata;
>  	struct spi_nor *nor;
>  	struct mtd_info *mtd;
> +	enum read_mode mode = SPI_NOR_QUAD;
>  	unsigned int cs;
> +	unsigned int rev_reg;
>  	int i, ret;
>  
> +	if (cqspi->caps & CQSPI_FLAG_SUPPORTS_OCTAL) {
> +		/* Determine, whether or not octal transfer MAY be supported */
> +		rev_reg = readl(cqspi->iobase + CQSPI_REG_MODULEID);
> +		dev_info(dev, "CQSPI Module id %x\n", rev_reg);

dev_dbg() please.

> +		switch (rev_reg & CQSPI_REG_MODULEID_CONF_ID_MASK) {
> +		case CQSPI_REG_MODULEID_CONF_ID_OCTAL_PHY:
> +		case CQSPI_REG_MODULEID_CONF_ID_OCTAL:
> +			mode = SPI_NOR_OCTAL;

break;

> +		}
> +	}
> +
>  	/* Get flash device data */
>  	for_each_available_child_of_node(dev->of_node, np) {
>  		ret = of_property_read_u32(np, "reg", &cs);
> @@ -1123,7 +1169,7 @@ static int cqspi_setup_flash(struct cqspi_st *cqspi, struct device_node *np)
>  			goto err;
>  		}
>  
> -		ret = spi_nor_scan(nor, NULL, SPI_NOR_QUAD);
> +		ret = spi_nor_scan(nor, NULL, mode);
>  		if (ret)
>  			goto err;
>  
> @@ -1160,6 +1206,7 @@ static int cqspi_probe(struct platform_device *pdev)
>  	mutex_init(&cqspi->bus_mutex);
>  	cqspi->pdev = pdev;
>  	platform_set_drvdata(pdev, cqspi);
> +	cqspi->caps = 0;

The structure is zeroed-out on allocation, so this shouldn't be needed.

>  	/* Obtain configuration from OF. */
>  	ret = cqspi_of_get_pdata(pdev);
> @@ -1277,13 +1324,6 @@ static const struct dev_pm_ops cqspi__dev_pm_ops = {
>  #define CQSPI_DEV_PM_OPS	NULL
>  #endif
>  
> -static struct of_device_id const cqspi_dt_ids[] = {
> -	{.compatible = "cdns,qspi-nor",},
> -	{ /* end of table */ }
> -};
> -
> -MODULE_DEVICE_TABLE(of, cqspi_dt_ids);
> -
>  static struct platform_driver cqspi_platform_driver = {
>  	.probe = cqspi_probe,
>  	.remove = cqspi_remove,
> 


-- 
Best regards,
Marek Vasut

Re: [PATCH v3, 4/4] dt-bindings: mtd: Add Octal SPI support to Cadence QSPI.

[Rob Herring]

On Mon, Mar 20, 2017 at 11:27:34AM +0000, Artur Jedrysek wrote:
> This patch updates Cadence QSPI Device Tree documentation to include
> information about new compatible used to indicate, whether or not
> Octal SPI transfers are supported by the device.
> 
> Signed-off-by: Artur Jedrysek <jartur@cadence.com>
> ---
> Changelog:
> v2: Use new compatible, instead of boolean property, to indicate
> Octal SPI support.
> ---
> v3: Changes according to comments.
> ---
>  Documentation/devicetree/bindings/mtd/cadence-quadspi.txt | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Acked-by: Rob Herring <robh@kernel.org>