[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

[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


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

[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

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

[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
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 7486 bytes --]

Hi Daniel,

[FYI, it's a private test report for your RFC patch.]
[auto build test ERROR on linus/master]
[also build test ERROR on v5.7-rc4 next-20200507]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Daniel-Walker/mtd-spi-nor-add-conditional-4B-opcodes/20200508-055015
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 6e7f2eacf09811d092c1b41263108ac7fe0d089d
config: arm-socfpga_defconfig (attached as .config)
compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project 6d2a66b10d458e34c852be46028092d2b46edc14)
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm cross compiling tool for clang build
        # apt-get install binutils-arm-linux-gnueabi
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/mtd/spi-nor/core.c:3140:15: error: cannot assign to variable 'info' with const-qualified type 'const struct flash_info *'
                   info->flags |= SPI_NOR_4B_OPCODES;
                   ~~~~~~~~~~~ ^
   drivers/mtd/spi-nor/core.c:3086:27: note: variable 'info' declared const here
           const struct flash_info *info;
           ~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
   1 error generated.

vim +3140 drivers/mtd/spi-nor/core.c

  3082	
  3083	int spi_nor_scan(struct spi_nor *nor, const char *name,
  3084			 const struct spi_nor_hwcaps *hwcaps)
  3085	{
  3086		const struct flash_info *info;
  3087		struct device *dev = nor->dev;
  3088		struct mtd_info *mtd = &nor->mtd;
  3089		struct device_node *np = spi_nor_get_flash_node(nor);
  3090		int ret;
  3091		int i;
  3092	
  3093		ret = spi_nor_check(nor);
  3094		if (ret)
  3095			return ret;
  3096	
  3097		/* Reset SPI protocol for all commands. */
  3098		nor->reg_proto = SNOR_PROTO_1_1_1;
  3099		nor->read_proto = SNOR_PROTO_1_1_1;
  3100		nor->write_proto = SNOR_PROTO_1_1_1;
  3101	
  3102		/*
  3103		 * We need the bounce buffer early to read/write registers when going
  3104		 * through the spi-mem layer (buffers have to be DMA-able).
  3105		 * For spi-mem drivers, we'll reallocate a new buffer if
  3106		 * nor->page_size turns out to be greater than PAGE_SIZE (which
  3107		 * shouldn't happen before long since NOR pages are usually less
  3108		 * than 1KB) after spi_nor_scan() returns.
  3109		 */
  3110		nor->bouncebuf_size = PAGE_SIZE;
  3111		nor->bouncebuf = devm_kmalloc(dev, nor->bouncebuf_size,
  3112					      GFP_KERNEL);
  3113		if (!nor->bouncebuf)
  3114			return -ENOMEM;
  3115	
  3116		info = spi_nor_get_flash_info(nor, name);
  3117		if (IS_ERR(info))
  3118			return PTR_ERR(info);
  3119	
  3120		nor->info = info;
  3121	
  3122		spi_nor_debugfs_init(nor, info);
  3123	
  3124		mutex_init(&nor->lock);
  3125	
  3126		/*
  3127		 * Make sure the XSR_RDY flag is set before calling
  3128		 * spi_nor_wait_till_ready(). Xilinx S3AN share MFR
  3129		 * with Atmel spi-nor
  3130		 */
  3131		if (info->flags & SPI_NOR_XSR_RDY)
  3132			nor->flags |=  SNOR_F_READY_XSR_RDY;
  3133	
  3134		if (info->flags & SPI_NOR_HAS_LOCK)
  3135			nor->flags |= SNOR_F_HAS_LOCK;
  3136	
  3137		/* Add SPI_NOR_4B_OPCODES if force in the device tree */
  3138		if (info->flags & SPI_NOR_COND_4B_OPCODES &&
  3139			of_property_read_bool(np, "force-4b-opcodes"))
> 3140			info->flags |= SPI_NOR_4B_OPCODES;
  3141	
  3142		mtd->_write = spi_nor_write;
  3143	
  3144		/* Init flash parameters based on flash_info struct and SFDP */
  3145		ret = spi_nor_init_params(nor);
  3146		if (ret)
  3147			return ret;
  3148	
  3149		if (!mtd->name)
  3150			mtd->name = dev_name(dev);
  3151		mtd->priv = nor;
  3152		mtd->type = MTD_NORFLASH;
  3153		mtd->writesize = 1;
  3154		mtd->flags = MTD_CAP_NORFLASH;
  3155		mtd->size = nor->params->size;
  3156		mtd->_erase = spi_nor_erase;
  3157		mtd->_read = spi_nor_read;
  3158		mtd->_resume = spi_nor_resume;
  3159	
  3160		if (nor->params->locking_ops) {
  3161			mtd->_lock = spi_nor_lock;
  3162			mtd->_unlock = spi_nor_unlock;
  3163			mtd->_is_locked = spi_nor_is_locked;
  3164		}
  3165	
  3166		if (info->flags & USE_FSR)
  3167			nor->flags |= SNOR_F_USE_FSR;
  3168		if (info->flags & SPI_NOR_HAS_TB) {
  3169			nor->flags |= SNOR_F_HAS_SR_TB;
  3170			if (info->flags & SPI_NOR_TB_SR_BIT6)
  3171				nor->flags |= SNOR_F_HAS_SR_TB_BIT6;
  3172		}
  3173	
  3174		if (info->flags & NO_CHIP_ERASE)
  3175			nor->flags |= SNOR_F_NO_OP_CHIP_ERASE;
  3176		if (info->flags & USE_CLSR)
  3177			nor->flags |= SNOR_F_USE_CLSR;
  3178	
  3179		if (info->flags & SPI_NOR_4BIT_BP) {
  3180			nor->flags |= SNOR_F_HAS_4BIT_BP;
  3181			if (info->flags & SPI_NOR_BP3_SR_BIT6)
  3182				nor->flags |= SNOR_F_HAS_SR_BP3_BIT6;
  3183		}
  3184	
  3185		if (info->flags & SPI_NOR_NO_ERASE)
  3186			mtd->flags |= MTD_NO_ERASE;
  3187	
  3188		mtd->dev.parent = dev;
  3189		nor->page_size = nor->params->page_size;
  3190		mtd->writebufsize = nor->page_size;
  3191	
  3192		if (of_property_read_bool(np, "broken-flash-reset"))
  3193			nor->flags |= SNOR_F_BROKEN_RESET;
  3194	
  3195		/*
  3196		 * Configure the SPI memory:
  3197		 * - select op codes for (Fast) Read, Page Program and Sector Erase.
  3198		 * - set the number of dummy cycles (mode cycles + wait states).
  3199		 * - set the SPI protocols for register and memory accesses.
  3200		 */
  3201		ret = spi_nor_setup(nor, hwcaps);
  3202		if (ret)
  3203			return ret;
  3204	
  3205		if (info->flags & SPI_NOR_4B_OPCODES)
  3206			nor->flags |= SNOR_F_4B_OPCODES;
  3207	
  3208		ret = spi_nor_set_addr_width(nor);
  3209		if (ret)
  3210			return ret;
  3211	
  3212		/* Send all the required SPI flash commands to initialize device */
  3213		ret = spi_nor_init(nor);
  3214		if (ret)
  3215			return ret;
  3216	
  3217		dev_info(dev, "%s (%lld Kbytes)\n", info->name,
  3218				(long long)mtd->size >> 10);
  3219	
  3220		dev_dbg(dev,
  3221			"mtd .name = %s, .size = 0x%llx (%lldMiB), "
  3222			".erasesize = 0x%.8x (%uKiB) .numeraseregions = %d\n",
  3223			mtd->name, (long long)mtd->size, (long long)(mtd->size >> 20),
  3224			mtd->erasesize, mtd->erasesize / 1024, mtd->numeraseregions);
  3225	
  3226		if (mtd->numeraseregions)
  3227			for (i = 0; i < mtd->numeraseregions; i++)
  3228				dev_dbg(dev,
  3229					"mtd.eraseregions[%d] = { .offset = 0x%llx, "
  3230					".erasesize = 0x%.8x (%uKiB), "
  3231					".numblocks = %d }\n",
  3232					i, (long long)mtd->eraseregions[i].offset,
  3233					mtd->eraseregions[i].erasesize,
  3234					mtd->eraseregions[i].erasesize / 1024,
  3235					mtd->eraseregions[i].numblocks);
  3236		return 0;
  3237	}
  3238	EXPORT_SYMBOL_GPL(spi_nor_scan);
  3239	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 26333 bytes --]

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

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 7129 bytes --]

Hi Daniel,

[FYI, it's a private test report for your RFC patch.]
[auto build test ERROR on linus/master]
[also build test ERROR on v5.7-rc4 next-20200507]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Daniel-Walker/mtd-spi-nor-add-conditional-4B-opcodes/20200508-055015
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 6e7f2eacf09811d092c1b41263108ac7fe0d089d
config: arm-defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 9.3.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day GCC_VERSION=9.3.0 make.cross ARCH=arm 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/mtd/spi-nor/core.c: In function 'spi_nor_scan':
>> drivers/mtd/spi-nor/core.c:3140:15: error: assignment of member 'flags' in read-only object
    3140 |   info->flags |= SPI_NOR_4B_OPCODES;
         |               ^~

vim +/flags +3140 drivers/mtd/spi-nor/core.c

  3082	
  3083	int spi_nor_scan(struct spi_nor *nor, const char *name,
  3084			 const struct spi_nor_hwcaps *hwcaps)
  3085	{
  3086		const struct flash_info *info;
  3087		struct device *dev = nor->dev;
  3088		struct mtd_info *mtd = &nor->mtd;
  3089		struct device_node *np = spi_nor_get_flash_node(nor);
  3090		int ret;
  3091		int i;
  3092	
  3093		ret = spi_nor_check(nor);
  3094		if (ret)
  3095			return ret;
  3096	
  3097		/* Reset SPI protocol for all commands. */
  3098		nor->reg_proto = SNOR_PROTO_1_1_1;
  3099		nor->read_proto = SNOR_PROTO_1_1_1;
  3100		nor->write_proto = SNOR_PROTO_1_1_1;
  3101	
  3102		/*
  3103		 * We need the bounce buffer early to read/write registers when going
  3104		 * through the spi-mem layer (buffers have to be DMA-able).
  3105		 * For spi-mem drivers, we'll reallocate a new buffer if
  3106		 * nor->page_size turns out to be greater than PAGE_SIZE (which
  3107		 * shouldn't happen before long since NOR pages are usually less
  3108		 * than 1KB) after spi_nor_scan() returns.
  3109		 */
  3110		nor->bouncebuf_size = PAGE_SIZE;
  3111		nor->bouncebuf = devm_kmalloc(dev, nor->bouncebuf_size,
  3112					      GFP_KERNEL);
  3113		if (!nor->bouncebuf)
  3114			return -ENOMEM;
  3115	
  3116		info = spi_nor_get_flash_info(nor, name);
  3117		if (IS_ERR(info))
  3118			return PTR_ERR(info);
  3119	
  3120		nor->info = info;
  3121	
  3122		spi_nor_debugfs_init(nor, info);
  3123	
  3124		mutex_init(&nor->lock);
  3125	
  3126		/*
  3127		 * Make sure the XSR_RDY flag is set before calling
  3128		 * spi_nor_wait_till_ready(). Xilinx S3AN share MFR
  3129		 * with Atmel spi-nor
  3130		 */
  3131		if (info->flags & SPI_NOR_XSR_RDY)
  3132			nor->flags |=  SNOR_F_READY_XSR_RDY;
  3133	
  3134		if (info->flags & SPI_NOR_HAS_LOCK)
  3135			nor->flags |= SNOR_F_HAS_LOCK;
  3136	
  3137		/* Add SPI_NOR_4B_OPCODES if force in the device tree */
  3138		if (info->flags & SPI_NOR_COND_4B_OPCODES &&
  3139			of_property_read_bool(np, "force-4b-opcodes"))
> 3140			info->flags |= SPI_NOR_4B_OPCODES;
  3141	
  3142		mtd->_write = spi_nor_write;
  3143	
  3144		/* Init flash parameters based on flash_info struct and SFDP */
  3145		ret = spi_nor_init_params(nor);
  3146		if (ret)
  3147			return ret;
  3148	
  3149		if (!mtd->name)
  3150			mtd->name = dev_name(dev);
  3151		mtd->priv = nor;
  3152		mtd->type = MTD_NORFLASH;
  3153		mtd->writesize = 1;
  3154		mtd->flags = MTD_CAP_NORFLASH;
  3155		mtd->size = nor->params->size;
  3156		mtd->_erase = spi_nor_erase;
  3157		mtd->_read = spi_nor_read;
  3158		mtd->_resume = spi_nor_resume;
  3159	
  3160		if (nor->params->locking_ops) {
  3161			mtd->_lock = spi_nor_lock;
  3162			mtd->_unlock = spi_nor_unlock;
  3163			mtd->_is_locked = spi_nor_is_locked;
  3164		}
  3165	
  3166		if (info->flags & USE_FSR)
  3167			nor->flags |= SNOR_F_USE_FSR;
  3168		if (info->flags & SPI_NOR_HAS_TB) {
  3169			nor->flags |= SNOR_F_HAS_SR_TB;
  3170			if (info->flags & SPI_NOR_TB_SR_BIT6)
  3171				nor->flags |= SNOR_F_HAS_SR_TB_BIT6;
  3172		}
  3173	
  3174		if (info->flags & NO_CHIP_ERASE)
  3175			nor->flags |= SNOR_F_NO_OP_CHIP_ERASE;
  3176		if (info->flags & USE_CLSR)
  3177			nor->flags |= SNOR_F_USE_CLSR;
  3178	
  3179		if (info->flags & SPI_NOR_4BIT_BP) {
  3180			nor->flags |= SNOR_F_HAS_4BIT_BP;
  3181			if (info->flags & SPI_NOR_BP3_SR_BIT6)
  3182				nor->flags |= SNOR_F_HAS_SR_BP3_BIT6;
  3183		}
  3184	
  3185		if (info->flags & SPI_NOR_NO_ERASE)
  3186			mtd->flags |= MTD_NO_ERASE;
  3187	
  3188		mtd->dev.parent = dev;
  3189		nor->page_size = nor->params->page_size;
  3190		mtd->writebufsize = nor->page_size;
  3191	
  3192		if (of_property_read_bool(np, "broken-flash-reset"))
  3193			nor->flags |= SNOR_F_BROKEN_RESET;
  3194	
  3195		/*
  3196		 * Configure the SPI memory:
  3197		 * - select op codes for (Fast) Read, Page Program and Sector Erase.
  3198		 * - set the number of dummy cycles (mode cycles + wait states).
  3199		 * - set the SPI protocols for register and memory accesses.
  3200		 */
  3201		ret = spi_nor_setup(nor, hwcaps);
  3202		if (ret)
  3203			return ret;
  3204	
  3205		if (info->flags & SPI_NOR_4B_OPCODES)
  3206			nor->flags |= SNOR_F_4B_OPCODES;
  3207	
  3208		ret = spi_nor_set_addr_width(nor);
  3209		if (ret)
  3210			return ret;
  3211	
  3212		/* Send all the required SPI flash commands to initialize device */
  3213		ret = spi_nor_init(nor);
  3214		if (ret)
  3215			return ret;
  3216	
  3217		dev_info(dev, "%s (%lld Kbytes)\n", info->name,
  3218				(long long)mtd->size >> 10);
  3219	
  3220		dev_dbg(dev,
  3221			"mtd .name = %s, .size = 0x%llx (%lldMiB), "
  3222			".erasesize = 0x%.8x (%uKiB) .numeraseregions = %d\n",
  3223			mtd->name, (long long)mtd->size, (long long)(mtd->size >> 20),
  3224			mtd->erasesize, mtd->erasesize / 1024, mtd->numeraseregions);
  3225	
  3226		if (mtd->numeraseregions)
  3227			for (i = 0; i < mtd->numeraseregions; i++)
  3228				dev_dbg(dev,
  3229					"mtd.eraseregions[%d] = { .offset = 0x%llx, "
  3230					".erasesize = 0x%.8x (%uKiB), "
  3231					".numblocks = %d }\n",
  3232					i, (long long)mtd->eraseregions[i].offset,
  3233					mtd->eraseregions[i].erasesize,
  3234					mtd->eraseregions[i].erasesize / 1024,
  3235					mtd->eraseregions[i].numblocks);
  3236		return 0;
  3237	}
  3238	EXPORT_SYMBOL_GPL(spi_nor_scan);
  3239	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 51821 bytes --]

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

[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

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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

[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
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

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.

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.



______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/