diff mbox series

[v4,3/3] mtd: spi-nor: Add support for IS25LX256 operating in 1S-8S-8S octal read mode

Message ID 20221202135539.271936-4-nathan.morrison@timesys.com
State New
Headers
Series mtd: spi-nor: Extend SFDP to support additional octal modes as per latest JEDEC standard |

Commit Message

Nathan Barrett-Morrison Dec. 2, 2022, 1:55 p.m. UTC 
  This adds the IS25LX256 chip into the ISSI flash_info parts table

Signed-off-by: Nathan Barrett-Morrison <nathan.morrison@timesys.com>
---
 drivers/mtd/spi-nor/issi.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

Comments

Michael Walle Dec. 2, 2022, 3:20 p.m. UTC | #1 
Am 2022-12-02 14:55, schrieb Nathan Barrett-Morrison:
> This adds the IS25LX256 chip into the ISSI flash_info parts table
> 
> Signed-off-by: Nathan Barrett-Morrison <nathan.morrison@timesys.com>
> ---
>  drivers/mtd/spi-nor/issi.c | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/issi.c b/drivers/mtd/spi-nor/issi.c
> index 89a66a19d754..362bc3603d8f 100644
> --- a/drivers/mtd/spi-nor/issi.c
> +++ b/drivers/mtd/spi-nor/issi.c
> @@ -29,6 +29,35 @@ static const struct spi_nor_fixups is25lp256_fixups 
> = {
>  	.post_bfpt = is25lp256_post_bfpt_fixups,
>  };
> 
> +static int
> +is25lx256_post_bfpt_fixups(struct spi_nor *nor,
> +			   const struct sfdp_parameter_header *bfpt_header,
> +			   const struct sfdp_bfpt *bfpt)
> +{
> +	/*
> +	 * IS25LX256 supports both 1S-1S-8S and 1S-8S-8S.
> +	 * However, the BFPT does not contain any information denoting this
> +	 * functionality, so the proper fast read opcodes are never setup.
> +	 * We're correcting this issue via the fixup below.  Page program
> +	 * commands are detected and setup properly via the 4BAIT lookup.
> +	 */
> +	params->hwcaps.mask |= SNOR_HWCAPS_READ_1_1_8;
> +	spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_1_1_8],
> +				  0, 8, SPINOR_OP_READ_1_1_8,
> +				  SNOR_PROTO_1_1_8);
> +
> +	params->hwcaps.mask |= SNOR_HWCAPS_READ_1_8_8;
> +	spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_1_8_8],
> +				  0, 16, SPINOR_OP_READ_1_8_8,
> +				  SNOR_PROTO_1_8_8);
> +
> +	return 0;
> +}
> +
> +static const struct spi_nor_fixups is25lx256_fixups = {
> +	.post_bfpt = is25lx256_post_bfpt_fixups,
> +};
> +
>  static void pm25lv_nor_late_init(struct spi_nor *nor)
>  {
>  	struct spi_nor_erase_map *map = &nor->params->erase_map;
> @@ -74,6 +103,9 @@ static const struct flash_info issi_nor_parts[] = {
>  		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
>  		FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
>  		.fixups = &is25lp256_fixups },
> +	{ "is25lx256", INFO(0x9d5a19, 0, 0, 0)
> +		PARSE_SFDP
> +		.fixups = &is25lx256_fixups },

Very nice!

Subject is slightly wrong because you fix up the BFPT to get
correct 1-1-8 and 1-8-8 read modes.

With that fixed:
Reviewed-by: Michael Walle <michael@walle.cc>
kernel test robot Dec. 3, 2022, 4:32 a.m. UTC | #2 
Hi Nathan,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on v6.1-rc7]
[also build test ERROR on linus/master next-20221202]
[cannot apply to mtd/spi-nor/next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Nathan-Barrett-Morrison/mtd-spi-nor-Extend-SFDP-to-support-additional-octal-modes-as-per-latest-JEDEC-standard/20221202-215808
patch link:    https://lore.kernel.org/r/20221202135539.271936-4-nathan.morrison%40timesys.com
patch subject: [PATCH v4 3/3] mtd: spi-nor: Add support for IS25LX256 operating in 1S-8S-8S octal read mode
config: i386-randconfig-a001
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/26787a4933a6a8591b6a66fc036e3b3c674cb57c
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Nathan-Barrett-Morrison/mtd-spi-nor-Extend-SFDP-to-support-additional-octal-modes-as-per-latest-JEDEC-standard/20221202-215808
        git checkout 26787a4933a6a8591b6a66fc036e3b3c674cb57c
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/mtd/spi-nor/issi.c: In function 'is25lx256_post_bfpt_fixups':
>> drivers/mtd/spi-nor/issi.c:44:9: error: 'params' undeclared (first use in this function); did you mean 'parameq'?
      44 |         params->hwcaps.mask |= SNOR_HWCAPS_READ_1_1_8;
         |         ^~~~~~
         |         parameq
   drivers/mtd/spi-nor/issi.c:44:9: note: each undeclared identifier is reported only once for each function it appears in


vim +44 drivers/mtd/spi-nor/issi.c

    31	
    32	static int
    33	is25lx256_post_bfpt_fixups(struct spi_nor *nor,
    34				   const struct sfdp_parameter_header *bfpt_header,
    35				   const struct sfdp_bfpt *bfpt)
    36	{
    37		/*
    38		 * IS25LX256 supports both 1S-1S-8S and 1S-8S-8S.
    39		 * However, the BFPT does not contain any information denoting this
    40		 * functionality, so the proper fast read opcodes are never setup.
    41		 * We're correcting this issue via the fixup below.  Page program
    42		 * commands are detected and setup properly via the 4BAIT lookup.
    43		 */
  > 44		params->hwcaps.mask |= SNOR_HWCAPS_READ_1_1_8;
    45		spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_1_1_8],
    46					  0, 8, SPINOR_OP_READ_1_1_8,
    47					  SNOR_PROTO_1_1_8);
    48	
    49		params->hwcaps.mask |= SNOR_HWCAPS_READ_1_8_8;
    50		spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_1_8_8],
    51					  0, 16, SPINOR_OP_READ_1_8_8,
    52					  SNOR_PROTO_1_8_8);
    53	
    54		return 0;
    55	}
    56
kernel test robot Dec. 3, 2022, 7:44 a.m. UTC | #3 
Hi Nathan,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on v6.1-rc7]
[also build test ERROR on linus/master next-20221202]
[cannot apply to mtd/spi-nor/next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Nathan-Barrett-Morrison/mtd-spi-nor-Extend-SFDP-to-support-additional-octal-modes-as-per-latest-JEDEC-standard/20221202-215808
patch link:    https://lore.kernel.org/r/20221202135539.271936-4-nathan.morrison%40timesys.com
patch subject: [PATCH v4 3/3] mtd: spi-nor: Add support for IS25LX256 operating in 1S-8S-8S octal read mode
config: x86_64-randconfig-a001
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/26787a4933a6a8591b6a66fc036e3b3c674cb57c
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Nathan-Barrett-Morrison/mtd-spi-nor-Extend-SFDP-to-support-additional-octal-modes-as-per-latest-JEDEC-standard/20221202-215808
        git checkout 26787a4933a6a8591b6a66fc036e3b3c674cb57c
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/mtd/spi-nor/issi.c:44:2: error: use of undeclared identifier 'params'; did you mean 'parameq'?
           params->hwcaps.mask |= SNOR_HWCAPS_READ_1_1_8;
           ^~~~~~
           parameq
   include/linux/moduleparam.h:372:13: note: 'parameq' declared here
   extern bool parameq(const char *name1, const char *name2);
               ^
>> drivers/mtd/spi-nor/issi.c:44:8: error: member reference base type 'bool (const char *, const char *)' (aka '_Bool (const char *, const char *)') is not a structure or union
           params->hwcaps.mask |= SNOR_HWCAPS_READ_1_1_8;
           ~~~~~~^ ~~~~~~
   drivers/mtd/spi-nor/issi.c:45:29: error: use of undeclared identifier 'params'
           spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_1_1_8],
                                      ^
   drivers/mtd/spi-nor/issi.c:49:2: error: use of undeclared identifier 'params'; did you mean 'parameq'?
           params->hwcaps.mask |= SNOR_HWCAPS_READ_1_8_8;
           ^~~~~~
           parameq
   include/linux/moduleparam.h:372:13: note: 'parameq' declared here
   extern bool parameq(const char *name1, const char *name2);
               ^
   drivers/mtd/spi-nor/issi.c:49:8: error: member reference base type 'bool (const char *, const char *)' (aka '_Bool (const char *, const char *)') is not a structure or union
           params->hwcaps.mask |= SNOR_HWCAPS_READ_1_8_8;
           ~~~~~~^ ~~~~~~
   drivers/mtd/spi-nor/issi.c:50:29: error: use of undeclared identifier 'params'
           spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_1_8_8],
                                      ^
   6 errors generated.


vim +44 drivers/mtd/spi-nor/issi.c

    31	
    32	static int
    33	is25lx256_post_bfpt_fixups(struct spi_nor *nor,
    34				   const struct sfdp_parameter_header *bfpt_header,
    35				   const struct sfdp_bfpt *bfpt)
    36	{
    37		/*
    38		 * IS25LX256 supports both 1S-1S-8S and 1S-8S-8S.
    39		 * However, the BFPT does not contain any information denoting this
    40		 * functionality, so the proper fast read opcodes are never setup.
    41		 * We're correcting this issue via the fixup below.  Page program
    42		 * commands are detected and setup properly via the 4BAIT lookup.
    43		 */
  > 44		params->hwcaps.mask |= SNOR_HWCAPS_READ_1_1_8;
    45		spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_1_1_8],
    46					  0, 8, SPINOR_OP_READ_1_1_8,
    47					  SNOR_PROTO_1_1_8);
    48	
    49		params->hwcaps.mask |= SNOR_HWCAPS_READ_1_8_8;
    50		spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_1_8_8],
    51					  0, 16, SPINOR_OP_READ_1_8_8,
    52					  SNOR_PROTO_1_8_8);
    53	
    54		return 0;
    55	}
    56
Tudor Ambarus Dec. 26, 2022, 8:04 a.m. UTC | #4 
Hi, Nathan,

The series is starting to look good, but I'll need another version,
please.

On 02.12.2022 15:55, Nathan Barrett-Morrison wrote:
> This adds the IS25LX256 chip into the ISSI flash_info parts table

Describe your changes in imperative mood, e.g. "Add support for
S25LX256" instead of "This adds ..."

It may worth to re-read
https://www.kernel.org/doc/html/latest/process/submitting-patches.html
once in a while.

> 
> Signed-off-by: Nathan Barrett-Morrison <nathan.morrison@timesys.com>
> ---
>   drivers/mtd/spi-nor/issi.c | 32 ++++++++++++++++++++++++++++++++
>   1 file changed, 32 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/issi.c b/drivers/mtd/spi-nor/issi.c
> index 89a66a19d754..362bc3603d8f 100644
> --- a/drivers/mtd/spi-nor/issi.c
> +++ b/drivers/mtd/spi-nor/issi.c
> @@ -29,6 +29,35 @@ static const struct spi_nor_fixups is25lp256_fixups = {
>   	.post_bfpt = is25lp256_post_bfpt_fixups,
>   };
>   
> +static int
> +is25lx256_post_bfpt_fixups(struct spi_nor *nor,
> +			   const struct sfdp_parameter_header *bfpt_header,
> +			   const struct sfdp_bfpt *bfpt)
> +{
> +	/*
> +	 * IS25LX256 supports both 1S-1S-8S and 1S-8S-8S.
> +	 * However, the BFPT does not contain any information denoting this
> +	 * functionality, so the proper fast read opcodes are never setup.
> +	 * We're correcting this issue via the fixup below.  Page program
> +	 * commands are detected and setup properly via the 4BAIT lookup.
> +	 */
> +	params->hwcaps.mask |= SNOR_HWCAPS_READ_1_1_8;

add a reference to params, the build fails here.

> +	spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_1_1_8],
> +				  0, 8, SPINOR_OP_READ_1_1_8,
> +				  SNOR_PROTO_1_1_8);
> +
> +	params->hwcaps.mask |= SNOR_HWCAPS_READ_1_8_8;
> +	spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_1_8_8],
> +				  0, 16, SPINOR_OP_READ_1_8_8,
> +				  SNOR_PROTO_1_8_8);
> +
> +	return 0;
> +}
> +
> +static const struct spi_nor_fixups is25lx256_fixups = {
> +	.post_bfpt = is25lx256_post_bfpt_fixups,
> +};
> +
>   static void pm25lv_nor_late_init(struct spi_nor *nor)
>   {
>   	struct spi_nor_erase_map *map = &nor->params->erase_map;
> @@ -74,6 +103,9 @@ static const struct flash_info issi_nor_parts[] = {
>   		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
>   		FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
>   		.fixups = &is25lp256_fixups },
> +	{ "is25lx256", INFO(0x9d5a19, 0, 0, 0)
> +		PARSE_SFDP
> +		.fixups = &is25lx256_fixups },

For every new flash addition or update, we ask contributors to do some
little tests. Would you please run the commands from below and send us
the output?

# dd if=/dev/urandom of=./qspi_test bs=1M count=6
6+0 records in
6+0 records out

# mtd_debug write /dev/mtd4 0 6291456 qspi_test
Copied 6291456 bytes from qspi_test to address 0x00000000 in flash

# mtd_debug erase /dev/mtd4 0 6291456
Erased 6291456 bytes from address 0x00000000 in flash

# mtd_debug read /dev/mtd4 0 6291456 qspi_read
Copied 6291456 bytes from address 0x00000000 in flash to qspi_read

# hexdump qspi_read
0000000 ffff ffff ffff ffff ffff ffff ffff ffff
*

0600000

# mtd_debug write /dev/mtd4 0 6291456 qspi_test
Copied 6291456 bytes from qspi_test to address 0x00000000 in flash

# mtd_debug read /dev/mtd4 0 6291456 qspi_read
Copied 6291456 bytes from address 0x00000000 in flash to qspi_read

# sha1sum qspi_test qspi_read
57f8d4fee65622104e24276e865f662844f12242  qspi_test
57f8d4fee65622104e24276e865f662844f12242  qspi_read

# cat /sys/bus/spi/devices/spi0.0/spi-nor/partname
is25wp256

# cat /sys/bus/spi/devices/spi0.0/spi-nor/jedec_id
9d7019

# cat /sys/bus/spi/devices/spi0.0/spi-nor/manufacturer
issi

# xxd -p /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
53464450060101ff00060110300000ff9d05010380000002ffffffffffff
ffffffffffffffffffffffffffffffffffffe520f9ffffffff0f44eb086b
083b80bbfeffffffffff00ffffff44eb0c200f5210d800ff234ac90082d8
11cecccd68467a757a75f7aed55c4a422cfff030faa9ffffffffffffffff
ffffffffffffffff501950169ff9c0648fefffff

# md5sum /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
ba14818b9ec42713f24d94d66bb90ba0  /sys/bus/spi/devices/spi0.0/spi-nor/sfdp
Tudor Ambarus Dec. 26, 2022, 8:17 a.m. UTC | #5 
On 26.12.2022 10:04, Tudor Ambarus wrote:
> Hi, Nathan,
> 
> The series is starting to look good, but I'll need another version,
> please.
> 
> On 02.12.2022 15:55, Nathan Barrett-Morrison wrote:
>> This adds the IS25LX256 chip into the ISSI flash_info parts table
> 
> Describe your changes in imperative mood, e.g. "Add support for
> S25LX256" instead of "This adds ..."
> 
> It may worth to re-read
> https://www.kernel.org/doc/html/latest/process/submitting-patches.html
> once in a while.
> 
>>
>> Signed-off-by: Nathan Barrett-Morrison <nathan.morrison@timesys.com>
>> ---
>>   drivers/mtd/spi-nor/issi.c | 32 ++++++++++++++++++++++++++++++++
>>   1 file changed, 32 insertions(+)
>>
>> diff --git a/drivers/mtd/spi-nor/issi.c b/drivers/mtd/spi-nor/issi.c
>> index 89a66a19d754..362bc3603d8f 100644
>> --- a/drivers/mtd/spi-nor/issi.c
>> +++ b/drivers/mtd/spi-nor/issi.c
>> @@ -29,6 +29,35 @@ static const struct spi_nor_fixups is25lp256_fixups 
>> = {
>>       .post_bfpt = is25lp256_post_bfpt_fixups,
>>   };
>> +static int
>> +is25lx256_post_bfpt_fixups(struct spi_nor *nor,
>> +               const struct sfdp_parameter_header *bfpt_header,
>> +               const struct sfdp_bfpt *bfpt)
>> +{
>> +    /*
>> +     * IS25LX256 supports both 1S-1S-8S and 1S-8S-8S.
>> +     * However, the BFPT does not contain any information denoting this
>> +     * functionality, so the proper fast read opcodes are never setup.
>> +     * We're correcting this issue via the fixup below.  Page program
>> +     * commands are detected and setup properly via the 4BAIT lookup.
>> +     */

Why don't you set the READ support when parsing the 4bait table? We need
to see the SFDP dump to determine how we treat this. I'm not sure a
post_bfpt hook is the right thing to do for this flash.

Thanks,
ta
Michael Walle Dec. 27, 2022, 12:37 p.m. UTC | #6 
Am 2022-12-26 09:17, schrieb Tudor Ambarus:
> On 26.12.2022 10:04, Tudor Ambarus wrote:
>> Hi, Nathan,
>> 
>> The series is starting to look good, but I'll need another version,
>> please.
>> 
>> On 02.12.2022 15:55, Nathan Barrett-Morrison wrote:
>>> This adds the IS25LX256 chip into the ISSI flash_info parts table
>> 
>> Describe your changes in imperative mood, e.g. "Add support for
>> S25LX256" instead of "This adds ..."
>> 
>> It may worth to re-read
>> https://www.kernel.org/doc/html/latest/process/submitting-patches.html
>> once in a while.
>> 
>>> 
>>> Signed-off-by: Nathan Barrett-Morrison <nathan.morrison@timesys.com>
>>> ---
>>>   drivers/mtd/spi-nor/issi.c | 32 ++++++++++++++++++++++++++++++++
>>>   1 file changed, 32 insertions(+)
>>> 
>>> diff --git a/drivers/mtd/spi-nor/issi.c b/drivers/mtd/spi-nor/issi.c
>>> index 89a66a19d754..362bc3603d8f 100644
>>> --- a/drivers/mtd/spi-nor/issi.c
>>> +++ b/drivers/mtd/spi-nor/issi.c
>>> @@ -29,6 +29,35 @@ static const struct spi_nor_fixups 
>>> is25lp256_fixups = {
>>>       .post_bfpt = is25lp256_post_bfpt_fixups,
>>>   };
>>> +static int
>>> +is25lx256_post_bfpt_fixups(struct spi_nor *nor,
>>> +               const struct sfdp_parameter_header *bfpt_header,
>>> +               const struct sfdp_bfpt *bfpt)
>>> +{
>>> +    /*
>>> +     * IS25LX256 supports both 1S-1S-8S and 1S-8S-8S.
>>> +     * However, the BFPT does not contain any information denoting 
>>> this
>>> +     * functionality, so the proper fast read opcodes are never 
>>> setup.
>>> +     * We're correcting this issue via the fixup below.  Page 
>>> program
>>> +     * commands are detected and setup properly via the 4BAIT 
>>> lookup.
>>> +     */
> 
> Why don't you set the READ support when parsing the 4bait table?

That would deviate from the read handling of all the other modes.

> We need to see the SFDP dump to determine how we treat this. I'm not
> sure a post_bfpt hook is the right thing to do for this flash.

See:
https://lore.kernel.org/linux-mtd/e07105d41ec62a6ee47ca0295ca347dc@walle.cc/

But yes, I missed that you should include the sfdp dump and some other
commands in the comment section of the patch which adds support for that
flash (see Tudors reply).

Tudor, what is the status of your documentation patch? I'd really like
to refer to the kernel docs instead of having write the same over and
over again ;)

-michael
Tudor Ambarus Dec. 27, 2022, 1:24 p.m. UTC | #7 
On 27.12.2022 14:37, Michael Walle wrote:
> Am 2022-12-26 09:17, schrieb Tudor Ambarus:
>> On 26.12.2022 10:04, Tudor Ambarus wrote:
>>> Hi, Nathan,
>>>
>>> The series is starting to look good, but I'll need another version,
>>> please.
>>>
>>> On 02.12.2022 15:55, Nathan Barrett-Morrison wrote:
>>>> This adds the IS25LX256 chip into the ISSI flash_info parts table
>>>
>>> Describe your changes in imperative mood, e.g. "Add support for
>>> S25LX256" instead of "This adds ..."
>>>
>>> It may worth to re-read
>>> https://www.kernel.org/doc/html/latest/process/submitting-patches.html
>>> once in a while.
>>>
>>>>
>>>> Signed-off-by: Nathan Barrett-Morrison <nathan.morrison@timesys.com>
>>>> ---
>>>>   drivers/mtd/spi-nor/issi.c | 32 ++++++++++++++++++++++++++++++++
>>>>   1 file changed, 32 insertions(+)
>>>>
>>>> diff --git a/drivers/mtd/spi-nor/issi.c b/drivers/mtd/spi-nor/issi.c
>>>> index 89a66a19d754..362bc3603d8f 100644
>>>> --- a/drivers/mtd/spi-nor/issi.c
>>>> +++ b/drivers/mtd/spi-nor/issi.c
>>>> @@ -29,6 +29,35 @@ static const struct spi_nor_fixups 
>>>> is25lp256_fixups = {
>>>>       .post_bfpt = is25lp256_post_bfpt_fixups,
>>>>   };
>>>> +static int
>>>> +is25lx256_post_bfpt_fixups(struct spi_nor *nor,
>>>> +               const struct sfdp_parameter_header *bfpt_header,
>>>> +               const struct sfdp_bfpt *bfpt)
>>>> +{
>>>> +    /*
>>>> +     * IS25LX256 supports both 1S-1S-8S and 1S-8S-8S.
>>>> +     * However, the BFPT does not contain any information denoting 
>>>> this
>>>> +     * functionality, so the proper fast read opcodes are never setup.
>>>> +     * We're correcting this issue via the fixup below.  Page program
>>>> +     * commands are detected and setup properly via the 4BAIT lookup.
>>>> +     */
>>
>> Why don't you set the READ support when parsing the 4bait table?
> 
> That would deviate from the read handling of all the other modes.

Oh, I see now, so you need to amend BFPT with the 3B opcodes so that the
4bait table convert the 3B read opcodes to 4B. That's fine, yes.

> 
>> We need to see the SFDP dump to determine how we treat this. I'm not
>> sure a post_bfpt hook is the right thing to do for this flash.
> 
> See:
> https://lore.kernel.org/linux-mtd/e07105d41ec62a6ee47ca0295ca347dc@walle.cc/

Okay, it looks fine. I still need the write, erase, verify erase, write,
read and compare test.

> 
> But yes, I missed that you should include the sfdp dump and some other
> commands in the comment section of the patch which adds support for that
> flash (see Tudors reply).
> 
Nathan, please resubmit addressing those small comments if you can.

> Tudor, what is the status of your documentation patch? I'd really like
> to refer to the kernel docs instead of having write the same over and
> over again ;)

I hope to have it ready for 6.3.

Cheers,
ta
diff mbox series

Patch

diff --git a/drivers/mtd/spi-nor/issi.c b/drivers/mtd/spi-nor/issi.c
index 89a66a19d754..362bc3603d8f 100644
--- a/drivers/mtd/spi-nor/issi.c
+++ b/drivers/mtd/spi-nor/issi.c
@@ -29,6 +29,35 @@  static const struct spi_nor_fixups is25lp256_fixups = {
 	.post_bfpt = is25lp256_post_bfpt_fixups,
 };
 
+static int
+is25lx256_post_bfpt_fixups(struct spi_nor *nor,
+			   const struct sfdp_parameter_header *bfpt_header,
+			   const struct sfdp_bfpt *bfpt)
+{
+	/*
+	 * IS25LX256 supports both 1S-1S-8S and 1S-8S-8S.
+	 * However, the BFPT does not contain any information denoting this
+	 * functionality, so the proper fast read opcodes are never setup.
+	 * We're correcting this issue via the fixup below.  Page program
+	 * commands are detected and setup properly via the 4BAIT lookup.
+	 */
+	params->hwcaps.mask |= SNOR_HWCAPS_READ_1_1_8;
+	spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_1_1_8],
+				  0, 8, SPINOR_OP_READ_1_1_8,
+				  SNOR_PROTO_1_1_8);
+
+	params->hwcaps.mask |= SNOR_HWCAPS_READ_1_8_8;
+	spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_1_8_8],
+				  0, 16, SPINOR_OP_READ_1_8_8,
+				  SNOR_PROTO_1_8_8);
+
+	return 0;
+}
+
+static const struct spi_nor_fixups is25lx256_fixups = {
+	.post_bfpt = is25lx256_post_bfpt_fixups,
+};
+
 static void pm25lv_nor_late_init(struct spi_nor *nor)
 {
 	struct spi_nor_erase_map *map = &nor->params->erase_map;
@@ -74,6 +103,9 @@  static const struct flash_info issi_nor_parts[] = {
 		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
 		FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
 		.fixups = &is25lp256_fixups },
+	{ "is25lx256", INFO(0x9d5a19, 0, 0, 0)
+		PARSE_SFDP
+		.fixups = &is25lx256_fixups },
 
 	/* PMC */
 	{ "pm25lv512",   INFO(0,        0, 32 * 1024,    2)