[RFC-PATCH] mtd: spi-nor: add conditional 4B opcodes

[RFC-PATCH] mtd: spi-nor: add conditional 4B opcodes

[Daniel Walker]

Some chips have 4B opcodes, but there is no way to know if they have
them. This device tree option allows platform owners to force enable 4b
opcodes when they know their chips support it even when it can be
automatically identified.

Cc: xe-linux-external@cisco.com
Signed-off-by: Daniel Walker <danielwa@cisco.com>
---
 drivers/mtd/spi-nor/core.c      | 5 +++++
 drivers/mtd/spi-nor/core.h      | 5 +++++
 drivers/mtd/spi-nor/micron-st.c | 2 +-
 3 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index cc68ea84318e..2bd130687f4b 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -3134,6 +3134,11 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
 	if (info->flags & SPI_NOR_HAS_LOCK)
 		nor->flags |= SNOR_F_HAS_LOCK;
 
+	/* Add SPI_NOR_4B_OPCODES if force in the device tree */
+	if (info->flags & SPI_NOR_COND_4B_OPCODES &&
+		of_property_read_bool(np, "force-4b-opcodes"))
+		info->flags |= SPI_NOR_4B_OPCODES;
+
 	mtd->_write = spi_nor_write;
 
 	/* Init flash parameters based on flash_info struct and SFDP */
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 6f2f6b27173f..49e17415d834 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -312,6 +312,11 @@ struct flash_info {
 					 * Must be used with SPI_NOR_4BIT_BP.
 					 */
 
+#define SPI_NOR_COND_4B_OPCODES	BIT(19) /*
+					 * Same as SPI_NOR_4B_OPCODES, but
+					 * must also be force in the device
+					 * tree.
+					 */
 	/* Part specific fixup hooks. */
 	const struct spi_nor_fixups *fixups;
 };
diff --git a/drivers/mtd/spi-nor/micron-st.c b/drivers/mtd/spi-nor/micron-st.c
index 6c034b9718e2..f827454eaa5f 100644
--- a/drivers/mtd/spi-nor/micron-st.c
+++ b/drivers/mtd/spi-nor/micron-st.c
@@ -37,7 +37,7 @@ static const struct flash_info st_parts[] = {
 			       SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },
 	{ "n25q256a",    INFO(0x20ba19, 0, 64 * 1024,  512, SECT_4K |
 			      USE_FSR | SPI_NOR_DUAL_READ |
-			      SPI_NOR_QUAD_READ) },
+			      SPI_NOR_QUAD_READ | SPI_NOR_COND_4B_OPCODES) },
 	{ "mt25qu256a",  INFO6(0x20bb19, 0x104400, 64 * 1024,  512,
 			       SECT_4K | USE_FSR | SPI_NOR_DUAL_READ |
 			       SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },
-- 
2.17.1

Re: [RFC-PATCH] mtd: spi-nor: add conditional 4B opcodes

[Pratyush Yadav]

On 07/05/20 09:20AM, Daniel Walker wrote:
> Some chips have 4B opcodes, but there is no way to know if they have
> them. This device tree option allows platform owners to force enable 4b
> opcodes when they know their chips support it even when it can be
> automatically identified.

Do you mean that two chips might have the same ID but one of them can 
support 4B opcodes and the other can not? Is it possible to detect this 
in a fixup hook? I think it would be better to do something like this in 
a fixup hook instead of via device tree.
 
> Cc: xe-linux-external@cisco.com
> Signed-off-by: Daniel Walker <danielwa@cisco.com>
> ---
>  drivers/mtd/spi-nor/core.c      | 5 +++++
>  drivers/mtd/spi-nor/core.h      | 5 +++++
>  drivers/mtd/spi-nor/micron-st.c | 2 +-
>  3 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index cc68ea84318e..2bd130687f4b 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -3134,6 +3134,11 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
>  	if (info->flags & SPI_NOR_HAS_LOCK)
>  		nor->flags |= SNOR_F_HAS_LOCK;
>  
> +	/* Add SPI_NOR_4B_OPCODES if force in the device tree */
> +	if (info->flags & SPI_NOR_COND_4B_OPCODES &&
> +		of_property_read_bool(np, "force-4b-opcodes"))
> +		info->flags |= SPI_NOR_4B_OPCODES;
> +
>  	mtd->_write = spi_nor_write;
>  
>  	/* Init flash parameters based on flash_info struct and SFDP */
> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> index 6f2f6b27173f..49e17415d834 100644
> --- a/drivers/mtd/spi-nor/core.h
> +++ b/drivers/mtd/spi-nor/core.h
> @@ -312,6 +312,11 @@ struct flash_info {
>  					 * Must be used with SPI_NOR_4BIT_BP.
>  					 */
>  
> +#define SPI_NOR_COND_4B_OPCODES	BIT(19) /*
> +					 * Same as SPI_NOR_4B_OPCODES, but
> +					 * must also be force in the device
> +					 * tree.
> +					 */
>  	/* Part specific fixup hooks. */
>  	const struct spi_nor_fixups *fixups;
>  };
> diff --git a/drivers/mtd/spi-nor/micron-st.c b/drivers/mtd/spi-nor/micron-st.c
> index 6c034b9718e2..f827454eaa5f 100644
> --- a/drivers/mtd/spi-nor/micron-st.c
> +++ b/drivers/mtd/spi-nor/micron-st.c
> @@ -37,7 +37,7 @@ static const struct flash_info st_parts[] = {
>  			       SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },
>  	{ "n25q256a",    INFO(0x20ba19, 0, 64 * 1024,  512, SECT_4K |
>  			      USE_FSR | SPI_NOR_DUAL_READ |
> -			      SPI_NOR_QUAD_READ) },
> +			      SPI_NOR_QUAD_READ | SPI_NOR_COND_4B_OPCODES) },
>  	{ "mt25qu256a",  INFO6(0x20bb19, 0x104400, 64 * 1024,  512,
>  			       SECT_4K | USE_FSR | SPI_NOR_DUAL_READ |
>  			       SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },

-- 
Regards,
Pratyush Yadav

Re: [RFC-PATCH] mtd: spi-nor: add conditional 4B opcodes

[Daniel Walker (danielwa)]

On Thu, May 07, 2020 at 11:33:46PM +0530, Pratyush Yadav wrote:
> On 07/05/20 09:20AM, Daniel Walker wrote:
> > Some chips have 4B opcodes, but there is no way to know if they have
> > them. This device tree option allows platform owners to force enable 4b
> > opcodes when they know their chips support it even when it can be
> > automatically identified.
> 
> Do you mean that two chips might have the same ID but one of them can 
> support 4B opcodes and the other can not? Is it possible to detect this 
> in a fixup hook? I think it would be better to do something like this in 
> a fixup hook instead of via device tree.
  
Yes. The chip I added the option for is an example of this, it's n25q256a. I'm not familiar with the
fixup hook mechanism, but I would assume you need some way to tell between the 4B
opcode chips and the non-4B opcode chips. For n25q256a, we have not found a way
to do that.

Daniel

Re: [RFC-PATCH] mtd: spi-nor: add conditional 4B opcodes

[Pratyush Yadav]

Hi Daniel,

On 07/05/20 06:13PM, Daniel Walker (danielwa) wrote:
> On Thu, May 07, 2020 at 11:33:46PM +0530, Pratyush Yadav wrote:
> > On 07/05/20 09:20AM, Daniel Walker wrote:
> > > Some chips have 4B opcodes, but there is no way to know if they have
> > > them. This device tree option allows platform owners to force enable 4b
> > > opcodes when they know their chips support it even when it can be
> > > automatically identified.
> > 
> > Do you mean that two chips might have the same ID but one of them can 
> > support 4B opcodes and the other can not? Is it possible to detect this 
> > in a fixup hook? I think it would be better to do something like this in 
> > a fixup hook instead of via device tree.
>   
> Yes. The chip I added the option for is an example of this, it's n25q256a. I'm not familiar with the
> fixup hook mechanism, but I would assume you need some way to tell between the 4B
> opcode chips and the non-4B opcode chips. For n25q256a, we have not found a way
> to do that.

I'm assuming this patch is related to [0]. If all you want is to address 
memory above 16M, why not switch to 4-byte addressing mode instead? 
Taking a quick look at the datasheet tells me this can be done via the 
"Enter 4-byte address mode" command (0xB7). Then just use the regular 
read/program commands with 4-byte addresses. Does that work for you? Is 
there any reason you _have_ to use dedicated 4B opcodes?

[0] https://lore.kernel.org/linux-mtd/20200417174620.16420-1-danielwa@cisco.com/ 

-- 
Regards,
Pratyush Yadav

Re: [RFC-PATCH] mtd: spi-nor: add conditional 4B opcodes

[Daniel Walker (danielwa)]

On Sat, May 09, 2020 at 12:37:35AM +0530, Pratyush Yadav wrote:
> Hi Daniel,
> 
> On 07/05/20 06:13PM, Daniel Walker (danielwa) wrote:
> > On Thu, May 07, 2020 at 11:33:46PM +0530, Pratyush Yadav wrote:
> > > On 07/05/20 09:20AM, Daniel Walker wrote:
> > > > Some chips have 4B opcodes, but there is no way to know if they have
> > > > them. This device tree option allows platform owners to force enable 4b
> > > > opcodes when they know their chips support it even when it can be
> > > > automatically identified.
> > > 
> > > Do you mean that two chips might have the same ID but one of them can 
> > > support 4B opcodes and the other can not? Is it possible to detect this 
> > > in a fixup hook? I think it would be better to do something like this in 
> > > a fixup hook instead of via device tree.
> >   
> > Yes. The chip I added the option for is an example of this, it's n25q256a. I'm not familiar with the
> > fixup hook mechanism, but I would assume you need some way to tell between the 4B
> > opcode chips and the non-4B opcode chips. For n25q256a, we have not found a way
> > to do that.
> 
> I'm assuming this patch is related to [0]. If all you want is to address 
> memory above 16M, why not switch to 4-byte addressing mode instead? 
> Taking a quick look at the datasheet tells me this can be done via the 
> "Enter 4-byte address mode" command (0xB7). Then just use the regular 
> read/program commands with 4-byte addresses. Does that work for you? Is 
> there any reason you _have_ to use dedicated 4B opcodes?

It might, I don't think we need anything beyond access to move than 16M. Your
proposal would be to have a hook which enters the 0xB7 command?

I guess the question would be do all the chips have this ability.

Daniel

Re: [RFC-PATCH] mtd: spi-nor: add conditional 4B opcodes

[Tudor.Ambarus]

Hi, Daniel,

On Thursday, May 7, 2020 7:20:47 PM EEST Daniel Walker wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
> 
> Some chips have 4B opcodes, but there is no way to know if they have
> them. This device tree option allows platform owners to force enable 4b
> opcodes when they know their chips support it even when it can be
> automatically identified.

I would like to detect this at run-time if possible. Maybe we can distinguish 
between the flavors of your flash by inspecting BFPT[16], bit 29. I'm planning 
to parse the 16th dword of BFPT. What does your flash return after applying 
the following patch?

diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
index f6038d3a3684..99f0ce57c7d0 100644
--- a/drivers/mtd/spi-nor/sfdp.c
+++ b/drivers/mtd/spi-nor/sfdp.c
@@ -457,6 +457,10 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
        /* Fix endianness of the BFPT DWORDs. */
        le32_to_cpu_array(bfpt.dwords, BFPT_DWORD_MAX);
 
+       for(i = 0; i < BFPT_DWORD_MAX; i++)
+               dev_err(nor->dev, "bfpt.dwords[%d] = %08x\n",
+                       i + 1, bfpt.dwords[i]);
+
        /* Number of address bytes. */
        switch (bfpt.dwords[BFPT_DWORD(1)] & BFPT_DWORD1_ADDRESS_BYTES_MASK) {
        case BFPT_DWORD1_ADDRESS_BYTES_3_ONLY:
@@ -972,6 +976,9 @@ static int spi_nor_parse_4bait(struct spi_nor *nor,
        /* Fix endianness of the 4BAIT DWORDs. */
        le32_to_cpu_array(dwords, SFDP_4BAIT_DWORD_MAX);
 
+       for(i = 0; i < SFDP_4BAIT_DWORD_MAX; i++)
+               dev_err(nor->dev, "4bait[%d] = %08x\n", i, dwords[i]);
+
        /*
         * Compute the subset of (Fast) Read commands for which the 4-byte
         * version is supported.