[PATCH] mtd: spi-nor: winbond: add support for W25Q512NW-IQ/IN

[PATCH] mtd: spi-nor: winbond: add support for W25Q512NW-IQ/IN

[Jae Hyun Yoo]

Add support for Winbond W25Q512NW-IQ/IN

datasheet:
https://www.winbond.com/resource-files/W25Q512NW%20RevB%2007192021.pdf

Test result on AST2600 SoC's SPI controller:
$ cat /sys/bus/platform/devices/1e620000.spi/spi_master/spi0/spi0.1/spi-nor/jedec_id
ef6020

$ cat /sys/bus/platform/devices/1e620000.spi/spi_master/spi0/spi0.1/spi-nor/manufacturer
winbond

$ cat /sys/bus/platform/devices/1e620000.spi/spi_master/spi0/spi0.1/spi-nor/partname
w25q512nwq

$ hexdump /sys/bus/platform/devices/1e620000.spi/spi_master/spi0/spi0.1/spi-nor/sfdp
0000000 4653 5044 0106 ff01 0600 1001 0080 ff00
0000010 0084 0201 00d0 ff00 ffff ffff ffff ffff
0000020 ffff ffff ffff ffff ffff ffff ffff ffff
*
0000080 20e5 fffb ffff 1fff eb44 6b08 3b08 bb42
0000090 fffe ffff ffff 0000 ffff eb40 200c 520f
00000a0 d810 0000 0233 00a6 e781 d914 63e9 3376
00000b0 757a 757a bdf7 5cd5 f719 ff5d 70e9 a5f9
00000c0 ffff ffff ffff ffff ffff ffff ffff ffff
00000d0 0aff fff0 ff21 ffdc
00000d8

Signed-off-by: Jae Hyun Yoo <quic_jaehyoo@quicinc.com>
---
 drivers/mtd/spi-nor/winbond.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/mtd/spi-nor/winbond.c b/drivers/mtd/spi-nor/winbond.c
index ffaa24055259..d6f1a3b7267e 100644
--- a/drivers/mtd/spi-nor/winbond.c
+++ b/drivers/mtd/spi-nor/winbond.c
@@ -133,6 +133,9 @@ static const struct flash_info winbond_nor_parts[] = {
 	{ "w25m512jv", INFO(0xef7119, 0, 64 * 1024, 1024)
 		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_QUAD_READ |
 			      SPI_NOR_DUAL_READ) },
+	{ "w25q512nwq", INFO(0xef6020, 0, 64 * 1024, 1024)
+		PARSE_SFDP
+		OTP_INFO(256, 3, 0x1000, 0x1000) },
 	{ "w25q512nwm", INFO(0xef8020, 0, 64 * 1024, 1024)
 		PARSE_SFDP
 		OTP_INFO(256, 3, 0x1000, 0x1000) },
-- 
2.25.1


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

Re: [PATCH] mtd: spi-nor: winbond: add support for W25Q512NW-IQ/IN

[Cédric Le Goater]

On 7/10/22 16:57, Jae Hyun Yoo wrote:
> Add support for Winbond W25Q512NW-IQ/IN
> 
> datasheet:
> https://www.winbond.com/resource-files/W25Q512NW%20RevB%2007192021.pdf
> 
> Test result on AST2600 SoC's SPI controller:
> $ cat /sys/bus/platform/devices/1e620000.spi/spi_master/spi0/spi0.1/spi-nor/jedec_id
> ef6020
> 
> $ cat /sys/bus/platform/devices/1e620000.spi/spi_master/spi0/spi0.1/spi-nor/manufacturer
> winbond
> 
> $ cat /sys/bus/platform/devices/1e620000.spi/spi_master/spi0/spi0.1/spi-nor/partname
> w25q512nwq
> 
> $ hexdump /sys/bus/platform/devices/1e620000.spi/spi_master/spi0/spi0.1/spi-nor/sfdp
> 0000000 4653 5044 0106 ff01 0600 1001 0080 ff00
> 0000010 0084 0201 00d0 ff00 ffff ffff ffff ffff
> 0000020 ffff ffff ffff ffff ffff ffff ffff ffff
> *
> 0000080 20e5 fffb ffff 1fff eb44 6b08 3b08 bb42
> 0000090 fffe ffff ffff 0000 ffff eb40 200c 520f
> 00000a0 d810 0000 0233 00a6 e781 d914 63e9 3376
> 00000b0 757a 757a bdf7 5cd5 f719 ff5d 70e9 a5f9
> 00000c0 ffff ffff ffff ffff ffff ffff ffff ffff
> 00000d0 0aff fff0 ff21 ffdc
> 00000d8
> 
> Signed-off-by: Jae Hyun Yoo <quic_jaehyoo@quicinc.com>

Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.

> ---
>   drivers/mtd/spi-nor/winbond.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/winbond.c b/drivers/mtd/spi-nor/winbond.c
> index ffaa24055259..d6f1a3b7267e 100644
> --- a/drivers/mtd/spi-nor/winbond.c
> +++ b/drivers/mtd/spi-nor/winbond.c
> @@ -133,6 +133,9 @@ static const struct flash_info winbond_nor_parts[] = {
>   	{ "w25m512jv", INFO(0xef7119, 0, 64 * 1024, 1024)
>   		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_QUAD_READ |
>   			      SPI_NOR_DUAL_READ) },
> +	{ "w25q512nwq", INFO(0xef6020, 0, 64 * 1024, 1024)
> +		PARSE_SFDP
> +		OTP_INFO(256, 3, 0x1000, 0x1000) },
>   	{ "w25q512nwm", INFO(0xef8020, 0, 64 * 1024, 1024)
>   		PARSE_SFDP
>   		OTP_INFO(256, 3, 0x1000, 0x1000) },


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

Re: [PATCH] mtd: spi-nor: winbond: add support for W25Q512NW-IQ/IN

[Michael Walle]

Hi,

> Add support for Winbond W25Q512NW-IQ/IN
> 
> datasheet:
> https://www.winbond.com/resource-files/W25Q512NW%20RevB%2007192021.pdf

Please add that as a Link: tag before your SoB tag.

> Test result on AST2600 SoC's SPI controller:
> $ cat /sys/bus/platform/devices/1e620000.spi/spi_master/spi0/spi0.1/spi-nor/jedec_id
> ef6020
> 
> $ cat /sys/bus/platform/devices/1e620000.spi/spi_master/spi0/spi0.1/spi-nor/manufacturer
> winbond
> 
> $ cat /sys/bus/platform/devices/1e620000.spi/spi_master/spi0/spi0.1/spi-nor/partname
> w25q512nwq
> 
> $ hexdump /sys/bus/platform/devices/1e620000.spi/spi_master/spi0/spi0.1/spi-nor/sfdp
> 0000000 4653 5044 0106 ff01 0600 1001 0080 ff00
> 0000010 0084 0201 00d0 ff00 ffff ffff ffff ffff
> 0000020 ffff ffff ffff ffff ffff ffff ffff ffff
> *
> 0000080 20e5 fffb ffff 1fff eb44 6b08 3b08 bb42
> 0000090 fffe ffff ffff 0000 ffff eb40 200c 520f
> 00000a0 d810 0000 0233 00a6 e781 d914 63e9 3376
> 00000b0 757a 757a bdf7 5cd5 f719 ff5d 70e9 a5f9
> 00000c0 ffff ffff ffff ffff ffff ffff ffff ffff
> 00000d0 0aff fff0 ff21 ffdc
> 00000d8

This information goes below the --- line

> Signed-off-by: Jae Hyun Yoo <quic_jaehyoo@quicinc.com>
> ---
>  drivers/mtd/spi-nor/winbond.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/winbond.c b/drivers/mtd/spi-nor/winbond.c
> index ffaa24055259..d6f1a3b7267e 100644
> --- a/drivers/mtd/spi-nor/winbond.c
> +++ b/drivers/mtd/spi-nor/winbond.c
> @@ -133,6 +133,9 @@ static const struct flash_info winbond_nor_parts[] = {
>  	{ "w25m512jv", INFO(0xef7119, 0, 64 * 1024, 1024)
>  		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_QUAD_READ |
>  			      SPI_NOR_DUAL_READ) },
> +	{ "w25q512nwq", INFO(0xef6020, 0, 64 * 1024, 1024)

Please use INFO(0xef6020, 0, 0, 0) and test wether it will still
work correctly. We will then be able to convert it to SNOR_ID3()
later.

> +		PARSE_SFDP
> +		OTP_INFO(256, 3, 0x1000, 0x1000) },

Did you test OTP?

-michael

>  	{ "w25q512nwm", INFO(0xef8020, 0, 64 * 1024, 1024)
>  		PARSE_SFDP
>  		OTP_INFO(256, 3, 0x1000, 0x1000) },
> -- 
> 2.25.1


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

Re: [PATCH] mtd: spi-nor: winbond: add support for W25Q512NW-IQ/IN

[Jae Hyun Yoo]

Hi Michael,

On 7/11/2022 2:50 AM, Michael Walle wrote:
> Hi,
> 
>> Add support for Winbond W25Q512NW-IQ/IN
>>
>> datasheet:
>> https://www.winbond.com/resource-files/W25Q512NW%20RevB%2007192021.pdf
> 
> Please add that as a Link: tag before your SoB tag.

Sure, I'll move it using the Link: tag in v2.

>> Test result on AST2600 SoC's SPI controller:
>> $ cat /sys/bus/platform/devices/1e620000.spi/spi_master/spi0/spi0.1/spi-nor/jedec_id
>> ef6020
>>
>> $ cat /sys/bus/platform/devices/1e620000.spi/spi_master/spi0/spi0.1/spi-nor/manufacturer
>> winbond
>>
>> $ cat /sys/bus/platform/devices/1e620000.spi/spi_master/spi0/spi0.1/spi-nor/partname
>> w25q512nwq
>>
>> $ hexdump /sys/bus/platform/devices/1e620000.spi/spi_master/spi0/spi0.1/spi-nor/sfdp
>> 0000000 4653 5044 0106 ff01 0600 1001 0080 ff00
>> 0000010 0084 0201 00d0 ff00 ffff ffff ffff ffff
>> 0000020 ffff ffff ffff ffff ffff ffff ffff ffff
>> *
>> 0000080 20e5 fffb ffff 1fff eb44 6b08 3b08 bb42
>> 0000090 fffe ffff ffff 0000 ffff eb40 200c 520f
>> 00000a0 d810 0000 0233 00a6 e781 d914 63e9 3376
>> 00000b0 757a 757a bdf7 5cd5 f719 ff5d 70e9 a5f9
>> 00000c0 ffff ffff ffff ffff ffff ffff ffff ffff
>> 00000d0 0aff fff0 ff21 ffdc
>> 00000d8
> 
> This information goes below the --- line

I followed the commit 89051ff5dd3bfbdc95c315dc3377fc46dadddc7c but yes,
I'll move this information into the comment section.

>> Signed-off-by: Jae Hyun Yoo <quic_jaehyoo@quicinc.com>
>> ---
>>   drivers/mtd/spi-nor/winbond.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/mtd/spi-nor/winbond.c b/drivers/mtd/spi-nor/winbond.c
>> index ffaa24055259..d6f1a3b7267e 100644
>> --- a/drivers/mtd/spi-nor/winbond.c
>> +++ b/drivers/mtd/spi-nor/winbond.c
>> @@ -133,6 +133,9 @@ static const struct flash_info winbond_nor_parts[] = {
>>   	{ "w25m512jv", INFO(0xef7119, 0, 64 * 1024, 1024)
>>   		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_QUAD_READ |
>>   			      SPI_NOR_DUAL_READ) },
>> +	{ "w25q512nwq", INFO(0xef6020, 0, 64 * 1024, 1024)
> 
> Please use INFO(0xef6020, 0, 0, 0) and test wether it will still
> work correctly. We will then be able to convert it to SNOR_ID3()
> later.

Tested it but it doesn't work with INFO(0xef6020, 0, 0, 0). I'll keep
it as is.

>> +		PARSE_SFDP
>> +		OTP_INFO(256, 3, 0x1000, 0x1000) },
> 
> Did you test OTP?

Yes.

$ flash_otp_info -u /dev/mtd0
Number of OTP user blocks on /dev/mtd0: 3
block  0:  offset = 0x0000  size = 256 bytes  [unlocked]
block  1:  offset = 0x0100  size = 256 bytes  [unlocked]
block  2:  offset = 0x0200  size = 256 bytes  [unlocked]
$ flash_otp_dump -u /dev/mtd0 0x2d0
OTP user data for /dev/mtd0
0x02d0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
0x02e0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
0x02f0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
$ echo -n otp test | flash_otp_write -u /dev/mtd0 0x2d0
Writing OTP user data on /dev/mtd0 at offset 0x2d0
Wrote 8 bytes of OTP user data
$ flash_otp_dump -u /dev/mtd0 0x2d0
OTP user data for /dev/mtd0
0x02d0: 6f 74 70 20 74 65 73 74 ff ff ff ff ff ff ff ff
0x02e0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
0x02f0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
$ flash_otp_erase -u /dev/mtd0 0x200 0x100
$ flash_otp_dump -u /dev/mtd0 0x2d0
OTP user data for /dev/mtd0
0x02d0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
0x02e0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
0x02f0: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff

I'll add it to commit comment section too.

Thanks,

Jae

> -michael
> 
>>   	{ "w25q512nwm", INFO(0xef8020, 0, 64 * 1024, 1024)
>>   		PARSE_SFDP
>>   		OTP_INFO(256, 3, 0x1000, 0x1000) },
>> -- 
>> 2.25.1
> 

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

Re: [PATCH] mtd: spi-nor: winbond: add support for W25Q512NW-IQ/IN

[Michael Walle]

Hi,

Am 2022-07-13 16:26, schrieb Jae Hyun Yoo:
>>> +	{ "w25q512nwq", INFO(0xef6020, 0, 64 * 1024, 1024)
>> 
>> Please use INFO(0xef6020, 0, 0, 0) and test wether it will still
>> work correctly. We will then be able to convert it to SNOR_ID3()
>> later.
> 
> Tested it but it doesn't work with INFO(0xef6020, 0, 0, 0). I'll keep
> it as is.

Can you please look into this? I'd expect this to work if the SFDP
tables are correct because all this should come from the tables.
You can look at /sys/kernel/debug/spi-nor/spi0.0/params and see
what is changing there.

Thanks,
-michael

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

Re: [PATCH] mtd: spi-nor: winbond: add support for W25Q512NW-IQ/IN

[Jae Hyun Yoo]

Hi Michael,

On 7/13/2022 7:32 AM, Michael Walle wrote:
> Hi,
> 
> Am 2022-07-13 16:26, schrieb Jae Hyun Yoo:
>>>> +    { "w25q512nwq", INFO(0xef6020, 0, 64 * 1024, 1024)
>>>
>>> Please use INFO(0xef6020, 0, 0, 0) and test wether it will still
>>> work correctly. We will then be able to convert it to SNOR_ID3()
>>> later.
>>
>> Tested it but it doesn't work with INFO(0xef6020, 0, 0, 0). I'll keep
>> it as is.
> 
> Can you please look into this? I'd expect this to work if the SFDP
> tables are correct because all this should come from the tables.
> You can look at /sys/kernel/debug/spi-nor/spi0.0/params and see
> what is changing there.

I tested it again but result is still the same. I can check the parsed
info like below if I use INFO(0xef6020, 0, 64 * 1024, 1024) but I can't
even check the debugfs info if I use INFO(0xef6020, 0, 0, 0) since it
doesn't boot at all. I think, this patch should go as is and the size
parsing issue could be fixed using a separate fix.

Thanks,
Jae

$ cat /sys/kernel/debug/spi-nor/spi0.0/capabilities
Supported read modes by the flash
  1S-1S-1S
   opcode        0x13
   mode cycles   0
   dummy cycles  0
  1S-1S-1S (fast read)
   opcode        0x0c
   mode cycles   0
   dummy cycles  8
  1S-1S-2S
   opcode        0x3c
   mode cycles   0
   dummy cycles  8
  1S-2S-2S
   opcode        0xbc
   mode cycles   2
   dummy cycles  2
  1S-1S-4S
   opcode        0x6c
   mode cycles   0
   dummy cycles  8
  1S-4S-4S
   opcode        0xec
   mode cycles   2
   dummy cycles  4
  4S-4S-4S
   opcode        0xec
   mode cycles   2
   dummy cycles  0

Supported page program modes by the flash
  1S-1S-1S
   opcode        0x12
  1S-1S-4S
   opcode        0x34
$ cat /sys/kernel/debug/spi-nor/spi0.0/params
name            w25q512nwq
id              ef 60 20
size            64.0 MiB
write size      1
page size       256
address width   4
flags           4B_OPCODES | HAS_4BAIT | HAS_16BIT_SR | SOFT_RESET

opcodes
  read           0x6c
   dummy cycles  8
  erase          0xdc
  program        0x12
  8D extension   none

protocols
  read           1S-1S-4S
  write          1S-1S-1S
  register       1S-1S-1S

erase commands
  21 (4.00 KiB) [1]
  dc (64.0 KiB) [3]
  c7 (64.0 MiB)

sector map
  region (in hex)   | erase mask | flags
  ------------------+------------+----------
  00000000-03ffffff |     [ 123] |

> Thanks,
> -michael

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

Re: [PATCH] mtd: spi-nor: winbond: add support for W25Q512NW-IQ/IN

[Michael Walle]

Hi,

Am 2022-07-13 23:01, schrieb Jae Hyun Yoo:
> On 7/13/2022 7:32 AM, Michael Walle wrote:
>> Am 2022-07-13 16:26, schrieb Jae Hyun Yoo:
>>>>> +    { "w25q512nwq", INFO(0xef6020, 0, 64 * 1024, 1024)
>>>> 
>>>> Please use INFO(0xef6020, 0, 0, 0) and test wether it will still
>>>> work correctly. We will then be able to convert it to SNOR_ID3()
>>>> later.
>>> 
>>> Tested it but it doesn't work with INFO(0xef6020, 0, 0, 0). I'll keep
>>> it as is.
>> 
>> Can you please look into this? I'd expect this to work if the SFDP
>> tables are correct because all this should come from the tables.
>> You can look at /sys/kernel/debug/spi-nor/spi0.0/params and see
>> what is changing there.
> 
> I tested it again but result is still the same. I can check the parsed
> info like below if I use INFO(0xef6020, 0, 64 * 1024, 1024) but I can't
> even check the debugfs info if I use INFO(0xef6020, 0, 0, 0) since it
> doesn't boot at all. I think, this patch should go as is and the size
> parsing issue could be fixed using a separate fix.

What does "doesn't boot at all" mean? Are there any kernel startup
messages?

Just to be sure, you have PARSE_SFDP set, right?

The entry should be (skipping OTP to make sure that isn't
the problem here):
{ "w25q512nwq", INFO(0xef6020, 0, 0, 0) PARSE_SFDP }

-michael

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

Re: [PATCH] mtd: spi-nor: winbond: add support for W25Q512NW-IQ/IN

[Jae Hyun Yoo]

Hi Michael,

On 7/14/2022 12:41 AM, Michael Walle wrote:
> Hi,
> 
> Am 2022-07-13 23:01, schrieb Jae Hyun Yoo:
>> On 7/13/2022 7:32 AM, Michael Walle wrote:
>>> Am 2022-07-13 16:26, schrieb Jae Hyun Yoo:
>>>>>> +    { "w25q512nwq", INFO(0xef6020, 0, 64 * 1024, 1024)
>>>>>
>>>>> Please use INFO(0xef6020, 0, 0, 0) and test wether it will still
>>>>> work correctly. We will then be able to convert it to SNOR_ID3()
>>>>> later.
>>>>
>>>> Tested it but it doesn't work with INFO(0xef6020, 0, 0, 0). I'll keep
>>>> it as is.
>>>
>>> Can you please look into this? I'd expect this to work if the SFDP
>>> tables are correct because all this should come from the tables.
>>> You can look at /sys/kernel/debug/spi-nor/spi0.0/params and see
>>> what is changing there.
>>
>> I tested it again but result is still the same. I can check the parsed
>> info like below if I use INFO(0xef6020, 0, 64 * 1024, 1024) but I can't
>> even check the debugfs info if I use INFO(0xef6020, 0, 0, 0) since it
>> doesn't boot at all. I think, this patch should go as is and the size
>> parsing issue could be fixed using a separate fix.
> 
> What does "doesn't boot at all" mean? Are there any kernel startup
> messages?

I'm sharing the error messages below.

[    0.748594] spi-nor spi0.0: w25q512nwq (65536 Kbytes)
[    0.865216] spi-aspeed-smc 1e620000.spi: CE0 read buswidth:4 [0x406c0741]
[    0.872833] ------------[ cut here ]------------
[    0.877984] WARNING: CPU: 1 PID: 1 at drivers/mtd/mtdcore.c:583 
add_mtd_device+0x28c/0x53c
[    0.887237] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 
5.15.43-AUTOINC-dirty-23801a6 #1
[    0.896076] Hardware name: Generic DT based system
[    0.901421] Backtrace:
[    0.904152] [<809722a8>] (dump_backtrace) from [<809724a0>] 
(show_stack+0x20/0x24)
[    0.912622]  r7:00000247 r6:00000009 r5:60000013 r4:80b2c980
[    0.918933] [<80972480>] (show_stack) from [<8097d318>] 
(dump_stack_lvl+0x48/0x54)
[    0.927384] [<8097d2d0>] (dump_stack_lvl) from [<8097d33c>] 
(dump_stack+0x18/0x1c)
[    0.935842]  r5:806464dc r4:80b7b538
[    0.939825] [<8097d324>] (dump_stack) from [<801216ac>] 
(__warn+0xf8/0x154)
[    0.947606] [<801215b4>] (__warn) from [<80972b90>] 
(warn_slowpath_fmt+0x84/0xe4)
[    0.955970]  r7:806464dc r6:00000247 r5:80b7b538 r4:81074000
[    0.962281] [<80972b10>] (warn_slowpath_fmt) from [<806464dc>] 
(add_mtd_device+0x28c/0x53c)
[    0.971618]  r10:bd7eece8 r9:813e1138 r8:00000000 r7:81074000 
r6:bd7eeccc r5:813e1040
[    0.980354]  r4:813e1040
[    0.983173] [<80646250>] (add_mtd_device) from [<80646810>] 
(mtd_device_parse_register+0x84/0x2e4)
[    0.993187]  r10:bd7eece8 r9:81074000 r8:00000000 r7:00000000 
r6:00000000 r5:00000000
[    1.001923]  r4:813e1040
[    1.004742] [<8064678c>] (mtd_device_parse_register) from 
[<80652240>] (spi_nor_probe+0x28c/0x2d8)
[    1.014758]  r9:81074000 r8:00000000 r7:00000000 r6:04000000 
r5:00000000 r4:813e1040
[    1.023396] [<80651fb4>] (spi_nor_probe) from [<80677e38>] 
(spi_mem_probe+0x78/0x9c)
[    1.032052]  r9:bd7eece8 r8:00000000 r7:81339800 r6:80d84b3c 
r5:813e3c00 r4:812a75c0
[    1.040690] [<80677dc0>] (spi_mem_probe) from [<806711ac>] 
(spi_probe+0x94/0xbc)
[    1.048957]  r7:813e3c00 r6:80d84b2c r5:813e3c00 r4:00000000
[    1.055267] [<80671118>] (spi_probe) from [<805ee98c>] 
(really_probe+0x1d8/0x46c)
[    1.063632]  r7:813e3c00 r6:80d84b3c r5:00000000 r4:813e3c00
[    1.069943] [<805ee7b4>] (really_probe) from [<805eed48>] 
(__driver_probe_device+0x128/0x204)
[    1.079468]  r6:81075bcc r5:80d84b3c r4:813e3c00
[    1.084615] [<805eec20>] (__driver_probe_device) from [<805eee68>] 
(driver_probe_device+0x44/0xd4)
[    1.094627]  r9:bd7eece8 r8:00000000 r7:813e3c00 r6:81075bcc 
r5:80deec7c r4:80deeb70
[    1.103265] [<805eee24>] (driver_probe_device) from [<805ef07c>] 
(__device_attach_driver+0xa4/0x13c)
[    1.113470]  r9:bd7eece8 r8:00000001 r7:813e3c00 r6:81075bcc 
r5:80d84b3c r4:00000001
[    1.122109] [<805eefd8>] (__device_attach_driver) from [<805eca0c>] 
(bus_for_each_drv+0xa0/0xe4)
[    1.131925]  r7:805eefd8 r6:81074000 r5:81075bcc r4:00000000
[    1.138235] [<805ec96c>] (bus_for_each_drv) from [<805ef3a8>] 
(__device_attach+0xd4/0x1c8)
[    1.147470]  r7:813e3c44 r6:80d84e9c r5:81074000 r4:813e3c00
[    1.153781] [<805ef2d4>] (__device_attach) from [<805ef864>] 
(device_initial_probe+0x1c/0x20)
[    1.163307]  r8:80deeb44 r7:81339800 r6:80d84e9c r5:813e3c00 r4:813e3c00
[    1.170782] [<805ef848>] (device_initial_probe) from [<805ecc1c>] 
(bus_probe_device+0x94/0x9c)
[    1.180401] [<805ecb88>] (bus_probe_device) from [<805eaa50>] 
(device_add+0x400/0x9c4)
[    1.189246]  r7:81339800 r6:81074000 r5:00000000 r4:813e3c00
[    1.195556] [<805ea650>] (device_add) from [<8067655c>] 
(__spi_add_device+0x74/0x148)
[    1.204306]  r10:bd7eeccc r9:80b6209c r8:80b83eb0 r7:81150c10 
r6:813e3c00 r5:81339800
[    1.213041]  r4:00000000
[    1.215861] [<806764e8>] (__spi_add_device) from [<80676698>] 
(spi_add_device+0x68/0x98)
[    1.224901]  r7:bd7eed30 r6:00000000 r5:813e3c00 r4:81339a0c
[    1.231212] [<80676630>] (spi_add_device) from [<80677230>] 
(spi_register_controller+0x8bc/0xc20)
[    1.241122]  r5:813e3c00 r4:81339800
[    1.245106] [<80676974>] (spi_register_controller) from [<806775b8>] 
(devm_spi_register_controller+0x24/0x60)
[    1.256184]  r10:812a3880 r9:80a56ae4 r8:81150c10 r7:81150c00 
r6:81150c10 r5:81339800
[    1.264919]  r4:81339800
[    1.267739] [<80677594>] (devm_spi_register_controller) from 
[<80679990>] (aspeed_spi_probe+0x184/0x230)
[    1.278332]  r7:81150c00 r6:00000000 r5:81339b80 r4:81339800
[    1.284642] [<8067980c>] (aspeed_spi_probe) from [<805f12e4>] 
(platform_probe+0x6c/0xc0)
[    1.293687]  r10:00000000 r9:00000000 r8:00000000 r7:81150c10 
r6:80d85e4c r5:81150c10
[    1.302423]  r4:00000000 r3:8067980c
[    1.306407] [<805f1278>] (platform_probe) from [<805ee98c>] 
(really_probe+0x1d8/0x46c)
[    1.315254]  r7:81150c10 r6:80d85e4c r5:00000000 r4:81150c10
[    1.321564] [<805ee7b4>] (really_probe) from [<805eed48>] 
(__driver_probe_device+0x128/0x204)
[    1.331089]  r6:80d85e4c r5:80d85e4c r4:81150c10
[    1.336236] [<805eec20>] (__driver_probe_device) from [<805eee68>] 
(driver_probe_device+0x44/0xd4)
[    1.346248]  r9:00000000 r8:00000000 r7:81150c10 r6:80d85e4c 
r5:80deec7c r4:80deeb70
[    1.354886] [<805eee24>] (driver_probe_device) from [<805ef1b8>] 
(__driver_attach+0xa4/0x1c0)
[    1.364414]  r9:00000000 r8:81075ee4 r7:00000000 r6:80d85e4c 
r5:81150c54 r4:81150c10
[    1.373052] [<805ef114>] (__driver_attach) from [<805ec4e4>] 
(bus_for_each_dev+0x94/0xd4)
[    1.382188]  r7:00000000 r6:81074000 r5:805ef114 r4:80d85e4c
[    1.388499] [<805ec450>] (bus_for_each_dev) from [<805ef93c>] 
(driver_attach+0x2c/0x30)
[    1.397442]  r7:80d7f088 r6:00000000 r5:813a5000 r4:80d85e4c
[    1.403753] [<805ef910>] (driver_attach) from [<805ece88>] 
(bus_add_driver+0x120/0x200)
[    1.412692] [<805ecd68>] (bus_add_driver) from [<805f03b4>] 
(driver_register+0x98/0x128)
[    1.421733]  r7:80dc9000 r6:81074000 r5:00000000 r4:80d85e4c
[    1.428044] [<805f031c>] (driver_register) from [<805f26bc>] 
(__platform_driver_register+0x2c/0x34)
[    1.438151]  r5:8124e940 r4:80c23fdc
[    1.442135] [<805f2690>] (__platform_driver_register) from 
[<80c24000>] (aspeed_spi_driver_init+0x24/0x28)
[    1.452922] [<80c23fdc>] (aspeed_spi_driver_init) from [<80c01650>] 
(do_one_initcall+0xa4/0x1a0)
[    1.462739] [<80c015ac>] (do_one_initcall) from [<80c01988>] 
(kernel_init_freeable+0x1d8/0x230)
[    1.472461]  r9:80c4485c r8:80c4483c r7:80dc9000 r6:00000006 
r5:8124e940 r4:80c6b118
[    1.481100] [<80c017b0>] (kernel_init_freeable) from [<8097db0c>] 
(kernel_init+0x20/0x138)
[    1.490340]  r10:00000000 r9:00000000 r8:00000000 r7:00000000 
r6:00000000 r5:8097daec
[    1.499075]  r4:00000000
[    1.501894] [<8097daec>] (kernel_init) from [<80100130>] 
(ret_from_fork+0x14/0x24)
[    1.510352] Exception stack(0x81075fb0 to 0x81075ff8)
[    1.515993] 5fa0:                                     00000000 
00000000 00000000 00000000
[    1.525122] 5fc0: 00000000 00000000 00000000 00000000 00000000 
00000000 00000000 00000000
[    1.534250] 5fe0: 00000000 00000000 00000000 00000000 00000013 00000000
[    1.541630]  r5:8097daec r4:00000000
[    1.545654] ---[ end trace 90aead4c9c23f630 ]---
[    1.550828] spi-nor: probe of spi0.0 failed with error -22
[    1.557724] spi-nor spi0.1: w25q512nwm (65536 Kbytes)
[    1.674226] spi-aspeed-smc 1e620000.spi: CE1 read buswidth:4 [0x406c0741]

> Just to be sure, you have PARSE_SFDP set, right?

Yea, right.

> The entry should be (skipping OTP to make sure that isn't
> the problem here):
> { "w25q512nwq", INFO(0xef6020, 0, 0, 0) PARSE_SFDP }

I tested it also but I'm seeing the same error message pattern.

Thanks,

Jae

> -michael

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

Re: [PATCH] mtd: spi-nor: winbond: add support for W25Q512NW-IQ/IN

[Michael Walle]

Hi,

Am 2022-07-14 15:47, schrieb Jae Hyun Yoo:
> On 7/14/2022 12:41 AM, Michael Walle wrote:
>> What does "doesn't boot at all" mean? Are there any kernel startup
>> messages?
> 
> I'm sharing the error messages below.

Thanks.

> [    0.748594] spi-nor spi0.0: w25q512nwq (65536 Kbytes)
> [    0.865216] spi-aspeed-smc 1e620000.spi: CE0 read buswidth:4 
> [0x406c0741]
> [    0.872833] ------------[ cut here ]------------
> [    0.877984] WARNING: CPU: 1 PID: 1 at drivers/mtd/mtdcore.c:583
> add_mtd_device+0x28c/0x53c
> [    0.887237] CPU: 1 PID: 1 Comm: swapper/0 Not tainted
> 5.15.43-AUTOINC-dirty-23801a6 #1

Could you please try it on the latest (vanilla) linux-next?

-michael

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

Re: [PATCH] mtd: spi-nor: winbond: add support for W25Q512NW-IQ/IN

[Michael Walle]

Am 2022-07-14 16:16, schrieb Michael Walle:
> Am 2022-07-14 15:47, schrieb Jae Hyun Yoo:
>> On 7/14/2022 12:41 AM, Michael Walle wrote:
>>> What does "doesn't boot at all" mean? Are there any kernel startup
>>> messages?
>> 
>> I'm sharing the error messages below.
> 
> Thanks.
> 
>> [    0.748594] spi-nor spi0.0: w25q512nwq (65536 Kbytes)
>> [    0.865216] spi-aspeed-smc 1e620000.spi: CE0 read buswidth:4 
>> [0x406c0741]
>> [    0.872833] ------------[ cut here ]------------
>> [    0.877984] WARNING: CPU: 1 PID: 1 at drivers/mtd/mtdcore.c:583
>> add_mtd_device+0x28c/0x53c
>> [    0.887237] CPU: 1 PID: 1 Comm: swapper/0 Not tainted
>> 5.15.43-AUTOINC-dirty-23801a6 #1
> 
> Could you please try it on the latest (vanilla) linux-next?

or spi-nor/next [1] as there are quite a lot of changes. The
patches shall be based on that.

-michael

[1] 
git://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git#spi-nor/next

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

Re: [PATCH] mtd: spi-nor: winbond: add support for W25Q512NW-IQ/IN

[Jae Hyun Yoo]

On 7/14/2022 7:21 AM, Michael Walle wrote:
> Am 2022-07-14 16:16, schrieb Michael Walle:
>> Am 2022-07-14 15:47, schrieb Jae Hyun Yoo:
>>> On 7/14/2022 12:41 AM, Michael Walle wrote:
>>>> What does "doesn't boot at all" mean? Are there any kernel startup
>>>> messages?
>>>
>>> I'm sharing the error messages below.
>>
>> Thanks.
>>
>>> [    0.748594] spi-nor spi0.0: w25q512nwq (65536 Kbytes)
>>> [    0.865216] spi-aspeed-smc 1e620000.spi: CE0 read buswidth:4 
>>> [0x406c0741]
>>> [    0.872833] ------------[ cut here ]------------
>>> [    0.877984] WARNING: CPU: 1 PID: 1 at drivers/mtd/mtdcore.c:583
>>> add_mtd_device+0x28c/0x53c
>>> [    0.887237] CPU: 1 PID: 1 Comm: swapper/0 Not tainted
>>> 5.15.43-AUTOINC-dirty-23801a6 #1
>>
>> Could you please try it on the latest (vanilla) linux-next?
> 
> or spi-nor/next [1] as there are quite a lot of changes. The
> patches shall be based on that.

Okay. Let me try that. I tested it using 5.15.43 with back-ported
spi-nor patches from the latest. I'll back-port more changes from
the spi-nor/next and will test the INFO(0xef6020, 0, 0, 0) setting
again.

-Jae

> -michael
> 
> [1] 
> git://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git#spi-nor/next

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

Re: [PATCH] mtd: spi-nor: winbond: add support for W25Q512NW-IQ/IN

[Jae Hyun Yoo]

Hi Michael,

On 7/14/2022 7:30 AM, Jae Hyun Yoo wrote:
> On 7/14/2022 7:21 AM, Michael Walle wrote:
>> Am 2022-07-14 16:16, schrieb Michael Walle:
>>> Am 2022-07-14 15:47, schrieb Jae Hyun Yoo:
>>>> On 7/14/2022 12:41 AM, Michael Walle wrote:
>>>>> What does "doesn't boot at all" mean? Are there any kernel startup
>>>>> messages?
>>>>
>>>> I'm sharing the error messages below.
>>>
>>> Thanks.
>>>
>>>> [    0.748594] spi-nor spi0.0: w25q512nwq (65536 Kbytes)
>>>> [    0.865216] spi-aspeed-smc 1e620000.spi: CE0 read buswidth:4 
>>>> [0x406c0741]
>>>> [    0.872833] ------------[ cut here ]------------
>>>> [    0.877984] WARNING: CPU: 1 PID: 1 at drivers/mtd/mtdcore.c:583
>>>> add_mtd_device+0x28c/0x53c
>>>> [    0.887237] CPU: 1 PID: 1 Comm: swapper/0 Not tainted
>>>> 5.15.43-AUTOINC-dirty-23801a6 #1
>>>
>>> Could you please try it on the latest (vanilla) linux-next?
>>
>> or spi-nor/next [1] as there are quite a lot of changes. The
>> patches shall be based on that.
> 
> Okay. Let me try that. I tested it using 5.15.43 with back-ported
> spi-nor patches from the latest. I'll back-port more changes from
> the spi-nor/next and will test the INFO(0xef6020, 0, 0, 0) setting
> again.

I tested the setting again after cherry picking all SPI relating changes
from the 'for-next' branch of
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi repository.

No luck! It's still making the same warning dump at 'add_mtd_device'
since 'mtd->erasesize' is checked as 0.

I traced it further to check if the erasesize is properly parsed from
the sfdp and checked that erase map seems parsed and initialized
correctly in 'spi_nor_parse_bfpt' but problem is, a target
mtd->erasesize is not properly selected in 'spi_nor_select_erase' since
the 'wanted_size' variable is initialized as sector size of info table
so a selected target mtd->erasesize is also 0 so looks like it's the
reason why it can't initialize mtd device if we use
INFO(0xef6020, 0, 0, 0).

Also, checked that the mtd->erasesize is set to 4096 if I enable
CONFIG_MTD_SPI_NOR_USE_4K_SECTORS so the SPI flash can be initialized 
with the INFO(0xef6020, 0, 0, 0) setting but, it should cover even when
the configuration is not enabled. I think, this patch should go as it
is. The erasesize selecting issue could be fixed using a separate
patch.

Are you still sure that the INFO(0xef6020, 0, 0, 0) works in the
latest spi-next?

Thanks,

Jae

> -Jae
> 
>> -michael
>>
>> [1] 
>> git://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git#spi-nor/next

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

Re: [PATCH] mtd: spi-nor: winbond: add support for W25Q512NW-IQ/IN

[Jae Hyun Yoo]

Hi Michael,

On 7/15/2022 1:15 PM, Jae Hyun Yoo wrote:
> Hi Michael,
> 
> On 7/14/2022 7:30 AM, Jae Hyun Yoo wrote:
>> On 7/14/2022 7:21 AM, Michael Walle wrote:
>>> Am 2022-07-14 16:16, schrieb Michael Walle:
>>>> Am 2022-07-14 15:47, schrieb Jae Hyun Yoo:
>>>>> On 7/14/2022 12:41 AM, Michael Walle wrote:
>>>>>> What does "doesn't boot at all" mean? Are there any kernel startup
>>>>>> messages?
>>>>>
>>>>> I'm sharing the error messages below.
>>>>
>>>> Thanks.
>>>>
>>>>> [    0.748594] spi-nor spi0.0: w25q512nwq (65536 Kbytes)
>>>>> [    0.865216] spi-aspeed-smc 1e620000.spi: CE0 read buswidth:4 
>>>>> [0x406c0741]
>>>>> [    0.872833] ------------[ cut here ]------------
>>>>> [    0.877984] WARNING: CPU: 1 PID: 1 at drivers/mtd/mtdcore.c:583
>>>>> add_mtd_device+0x28c/0x53c
>>>>> [    0.887237] CPU: 1 PID: 1 Comm: swapper/0 Not tainted
>>>>> 5.15.43-AUTOINC-dirty-23801a6 #1
>>>>
>>>> Could you please try it on the latest (vanilla) linux-next?
>>>
>>> or spi-nor/next [1] as there are quite a lot of changes. The
>>> patches shall be based on that.
>>
>> Okay. Let me try that. I tested it using 5.15.43 with back-ported
>> spi-nor patches from the latest. I'll back-port more changes from
>> the spi-nor/next and will test the INFO(0xef6020, 0, 0, 0) setting
>> again.
> 
> I tested the setting again after cherry picking all SPI relating changes
> from the 'for-next' branch of
> git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi repository.
> 
> No luck! It's still making the same warning dump at 'add_mtd_device'
> since 'mtd->erasesize' is checked as 0.
> 
> I traced it further to check if the erasesize is properly parsed from
> the sfdp and checked that erase map seems parsed and initialized
> correctly in 'spi_nor_parse_bfpt' but problem is, a target
> mtd->erasesize is not properly selected in 'spi_nor_select_erase' since
> the 'wanted_size' variable is initialized as sector size of info table
> so a selected target mtd->erasesize is also 0 so looks like it's the
> reason why it can't initialize mtd device if we use
> INFO(0xef6020, 0, 0, 0).
> 
> Also, checked that the mtd->erasesize is set to 4096 if I enable
> CONFIG_MTD_SPI_NOR_USE_4K_SECTORS so the SPI flash can be initialized 
> with the INFO(0xef6020, 0, 0, 0) setting but, it should cover even when
> the configuration is not enabled. I think, this patch should go as it
> is. The erasesize selecting issue could be fixed using a separate
> patch.
> 
> Are you still sure that the INFO(0xef6020, 0, 0, 0) works in the
> latest spi-next?

I also tried to fix the issue and made a fix like below.

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 502967c76c5f..f8a020f80a56 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -2117,7 +2117,7 @@ spi_nor_select_uniform_erase(struct 
spi_nor_erase_map *map,
                  * If the current erase size is the one, stop here:
                  * we have found the right uniform Sector Erase command.
                  */
-               if (tested_erase->size == wanted_size) {
+               if (wanted_size && tested_erase->size == wanted_size) {
                         erase = tested_erase;
                         break;
                 }

Tested that it makes the INFO(0xef6020, 0, 0, 0) setting work and a
selected mtd->erasesize is 65536 which is what I expected for this
device.

Not sure if it's a right fix or not. Please review and let me know if
it's good to submit or not.

Thanks,

Jae

> Thanks,
> 
> Jae
> 
>> -Jae
>>
>>> -michael
>>>
>>> [1] 
>>> git://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git#spi-nor/next

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

Re: [PATCH] mtd: spi-nor: winbond: add support for W25Q512NW-IQ/IN

[Michael Walle]

Hi,

Am 2022-07-15 22:15, schrieb Jae Hyun Yoo:
> On 7/14/2022 7:30 AM, Jae Hyun Yoo wrote:
>> On 7/14/2022 7:21 AM, Michael Walle wrote:
>>> Am 2022-07-14 16:16, schrieb Michael Walle:
>>>> Am 2022-07-14 15:47, schrieb Jae Hyun Yoo:
>>>>> On 7/14/2022 12:41 AM, Michael Walle wrote:
>>>>>> What does "doesn't boot at all" mean? Are there any kernel startup
>>>>>> messages?
>>>>> 
>>>>> I'm sharing the error messages below.
>>>> 
>>>> Thanks.
>>>> 
>>>>> [    0.748594] spi-nor spi0.0: w25q512nwq (65536 Kbytes)
>>>>> [    0.865216] spi-aspeed-smc 1e620000.spi: CE0 read buswidth:4 
>>>>> [0x406c0741]
>>>>> [    0.872833] ------------[ cut here ]------------
>>>>> [    0.877984] WARNING: CPU: 1 PID: 1 at drivers/mtd/mtdcore.c:583
>>>>> add_mtd_device+0x28c/0x53c
>>>>> [    0.887237] CPU: 1 PID: 1 Comm: swapper/0 Not tainted
>>>>> 5.15.43-AUTOINC-dirty-23801a6 #1
>>>> 
>>>> Could you please try it on the latest (vanilla) linux-next?
>>> 
>>> or spi-nor/next [1] as there are quite a lot of changes. The
>>> patches shall be based on that.
>> 
>> Okay. Let me try that. I tested it using 5.15.43 with back-ported
>> spi-nor patches from the latest. I'll back-port more changes from
>> the spi-nor/next and will test the INFO(0xef6020, 0, 0, 0) setting
>> again.
> 
> I tested the setting again after cherry picking all SPI relating 
> changes
> from the 'for-next' branch of
> git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi repository.

That is not the tree I mentioned in my previous mail. Why do you
cherry-pick the changes instead of just trying the spi-nor/next
tree?

> No luck! It's still making the same warning dump at 'add_mtd_device'
> since 'mtd->erasesize' is checked as 0.
> 
> I traced it further to check if the erasesize is properly parsed from
> the sfdp and checked that erase map seems parsed and initialized
> correctly in 'spi_nor_parse_bfpt' but problem is, a target
> mtd->erasesize is not properly selected in 'spi_nor_select_erase' since
> the 'wanted_size' variable is initialized as sector size of info table
> so a selected target mtd->erasesize is also 0 so looks like it's the
> reason why it can't initialize mtd device if we use
> INFO(0xef6020, 0, 0, 0).

Have a look at
https://lore.kernel.org/linux-mtd/20220510140232.3519184-2-michael@walle.cc/
wanted_size can be 0. In this case spi_nor_select_uniform_erase()
should return the biggest valid erase type. Could you please check that
(1) spi_nor_select_uniform_erase() return non-NULL
(2) check what value erase->size has (I guess it should be 64k in your 
case)

 From what you tell me erase->size should be zero. If that is the
case look at spi_nor_parse_bfpt() where the erase sizes are parsed.
Also take a look at spi_nor_parse_4bait() where the erase types might
be cleared again.

I've checked your SFDP data and it contains three valid erase sizes
and opcodes for 3byte addressing and two valid erase opcodes for 4
byte addressing. So that looks all good. After all the SFDP parsing
I expect that you have two valid erase types:
  - erase size 4096 with erase opcode 21h
  - erase size 65536 with erase opcode DCh

> Also, checked that the mtd->erasesize is set to 4096 if I enable
> CONFIG_MTD_SPI_NOR_USE_4K_SECTORS so the SPI flash can be initialized
> with the INFO(0xef6020, 0, 0, 0) setting but, it should cover even
> when
> the configuration is not enabled. I think, this patch should go as it
> is. The erasesize selecting issue could be fixed using a separate
> patch.
> Are you still sure that the INFO(0xef6020, 0, 0, 0) works in the
> latest spi-next?

I've got two tested-by's with two different flashes, so yes, I'm
pretty sure it works in principle. It might still be something
wrong with your flash though.

-michael

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

Re: [PATCH] mtd: spi-nor: winbond: add support for W25Q512NW-IQ/IN

[Michael Walle]

Hi,

Am 2022-07-16 00:35, schrieb Jae Hyun Yoo:
> On 7/15/2022 1:15 PM, Jae Hyun Yoo wrote:
>> On 7/14/2022 7:30 AM, Jae Hyun Yoo wrote:
>>> On 7/14/2022 7:21 AM, Michael Walle wrote:
>>>> Am 2022-07-14 16:16, schrieb Michael Walle:
>>>>> Am 2022-07-14 15:47, schrieb Jae Hyun Yoo:
>>>>>> On 7/14/2022 12:41 AM, Michael Walle wrote:
>>>>>>> What does "doesn't boot at all" mean? Are there any kernel 
>>>>>>> startup
>>>>>>> messages?
>>>>>> 
>>>>>> I'm sharing the error messages below.
>>>>> 
>>>>> Thanks.
>>>>> 
>>>>>> [    0.748594] spi-nor spi0.0: w25q512nwq (65536 Kbytes)
>>>>>> [    0.865216] spi-aspeed-smc 1e620000.spi: CE0 read buswidth:4 
>>>>>> [0x406c0741]
>>>>>> [    0.872833] ------------[ cut here ]------------
>>>>>> [    0.877984] WARNING: CPU: 1 PID: 1 at drivers/mtd/mtdcore.c:583
>>>>>> add_mtd_device+0x28c/0x53c
>>>>>> [    0.887237] CPU: 1 PID: 1 Comm: swapper/0 Not tainted
>>>>>> 5.15.43-AUTOINC-dirty-23801a6 #1
>>>>> 
>>>>> Could you please try it on the latest (vanilla) linux-next?
>>>> 
>>>> or spi-nor/next [1] as there are quite a lot of changes. The
>>>> patches shall be based on that.
>>> 
>>> Okay. Let me try that. I tested it using 5.15.43 with back-ported
>>> spi-nor patches from the latest. I'll back-port more changes from
>>> the spi-nor/next and will test the INFO(0xef6020, 0, 0, 0) setting
>>> again.
>> 
>> I tested the setting again after cherry picking all SPI relating 
>> changes
>> from the 'for-next' branch of
>> git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi repository.
>> 
>> No luck! It's still making the same warning dump at 'add_mtd_device'
>> since 'mtd->erasesize' is checked as 0.
>> 
>> I traced it further to check if the erasesize is properly parsed from
>> the sfdp and checked that erase map seems parsed and initialized
>> correctly in 'spi_nor_parse_bfpt' but problem is, a target
>> mtd->erasesize is not properly selected in 'spi_nor_select_erase' 
>> since
>> the 'wanted_size' variable is initialized as sector size of info table
>> so a selected target mtd->erasesize is also 0 so looks like it's the
>> reason why it can't initialize mtd device if we use
>> INFO(0xef6020, 0, 0, 0).
>> 
>> Also, checked that the mtd->erasesize is set to 4096 if I enable
>> CONFIG_MTD_SPI_NOR_USE_4K_SECTORS so the SPI flash can be initialized 
>> with the INFO(0xef6020, 0, 0, 0) setting but, it should cover even 
>> when
>> the configuration is not enabled. I think, this patch should go as it
>> is. The erasesize selecting issue could be fixed using a separate
>> patch.
>> 
>> Are you still sure that the INFO(0xef6020, 0, 0, 0) works in the
>> latest spi-next?
> 
> I also tried to fix the issue and made a fix like below.
> 
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 502967c76c5f..f8a020f80a56 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -2117,7 +2117,7 @@ spi_nor_select_uniform_erase(struct
> spi_nor_erase_map *map,
>                  * If the current erase size is the one, stop here:
>                  * we have found the right uniform Sector Erase 
> command.
>                  */
> -               if (tested_erase->size == wanted_size) {
> +               if (wanted_size && tested_erase->size == wanted_size) {
>                         erase = tested_erase;
>                         break;
>                 }
> 
> Tested that it makes the INFO(0xef6020, 0, 0, 0) setting work and a
> selected mtd->erasesize is 65536 which is what I expected for this
> device.
> 
> Not sure if it's a right fix or not. Please review and let me know if
> it's good to submit or not.

Ahh, I think I know whats going wrong here. Thanks!

4bait will set the erase size to 0 if there is no corresponding
opcode for the 4byte erase. So you'll end up with
et[0]: 4096 - 21h
et[1]: 0 - FFh
et[2]: 65536 - DCh
et[3]: --

And spi_nor_select_uniform_erase() will select et[1].

Could you try the following:

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index ce5d69317d46..a2c8de250e01 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -2113,6 +2113,10 @@ spi_nor_select_uniform_erase(struct 
spi_nor_erase_map *map,

                 tested_erase = &map->erase_type[i];

+               /* Skip masked erase types. */
+               if (!tested_erase->size)
+                       continue;
+
                 /*
                  * If the current erase size is the one, stop here:
                  * we have found the right uniform Sector Erase command.


-michael

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

Re: [PATCH] mtd: spi-nor: winbond: add support for W25Q512NW-IQ/IN

[Jae Hyun Yoo]

Hi,

On 7/15/2022 3:52 PM, Michael Walle wrote:
> Hi,
> 
> Am 2022-07-15 22:15, schrieb Jae Hyun Yoo:
>> On 7/14/2022 7:30 AM, Jae Hyun Yoo wrote:
>>> On 7/14/2022 7:21 AM, Michael Walle wrote:
>>>> Am 2022-07-14 16:16, schrieb Michael Walle:
>>>>> Am 2022-07-14 15:47, schrieb Jae Hyun Yoo:
>>>>>> On 7/14/2022 12:41 AM, Michael Walle wrote:
>>>>>>> What does "doesn't boot at all" mean? Are there any kernel startup
>>>>>>> messages?
>>>>>>
>>>>>> I'm sharing the error messages below.
>>>>>
>>>>> Thanks.
>>>>>
>>>>>> [    0.748594] spi-nor spi0.0: w25q512nwq (65536 Kbytes)
>>>>>> [    0.865216] spi-aspeed-smc 1e620000.spi: CE0 read buswidth:4 
>>>>>> [0x406c0741]
>>>>>> [    0.872833] ------------[ cut here ]------------
>>>>>> [    0.877984] WARNING: CPU: 1 PID: 1 at drivers/mtd/mtdcore.c:583
>>>>>> add_mtd_device+0x28c/0x53c
>>>>>> [    0.887237] CPU: 1 PID: 1 Comm: swapper/0 Not tainted
>>>>>> 5.15.43-AUTOINC-dirty-23801a6 #1
>>>>>
>>>>> Could you please try it on the latest (vanilla) linux-next?
>>>>
>>>> or spi-nor/next [1] as there are quite a lot of changes. The
>>>> patches shall be based on that.
>>>
>>> Okay. Let me try that. I tested it using 5.15.43 with back-ported
>>> spi-nor patches from the latest. I'll back-port more changes from
>>> the spi-nor/next and will test the INFO(0xef6020, 0, 0, 0) setting
>>> again.
>>
>> I tested the setting again after cherry picking all SPI relating changes
>> from the 'for-next' branch of
>> git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi repository.
> 
> That is not the tree I mentioned in my previous mail. Why do you
> cherry-pick the changes instead of just trying the spi-nor/next
> tree?

I compared it with 
git://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git#spi-nor/next 
too. Result is still the same.

>> No luck! It's still making the same warning dump at 'add_mtd_device'
>> since 'mtd->erasesize' is checked as 0.
>>
>> I traced it further to check if the erasesize is properly parsed from
>> the sfdp and checked that erase map seems parsed and initialized
>> correctly in 'spi_nor_parse_bfpt' but problem is, a target
>> mtd->erasesize is not properly selected in 'spi_nor_select_erase' since
>> the 'wanted_size' variable is initialized as sector size of info table
>> so a selected target mtd->erasesize is also 0 so looks like it's the
>> reason why it can't initialize mtd device if we use
>> INFO(0xef6020, 0, 0, 0).
> 
> Have a look at
> https://lore.kernel.org/linux-mtd/20220510140232.3519184-2-michael@walle.cc/ 
> 
> wanted_size can be 0. In this case spi_nor_select_uniform_erase()
> should return the biggest valid erase type. Could you please check that
> (1) spi_nor_select_uniform_erase() return non-NULL
> (2) check what value erase->size has (I guess it should be 64k in your 
> case)
> 
>  From what you tell me erase->size should be zero. If that is the
> case look at spi_nor_parse_bfpt() where the erase sizes are parsed.
> Also take a look at spi_nor_parse_4bait() where the erase types might
> be cleared again.
> 
> I've checked your SFDP data and it contains three valid erase sizes
> and opcodes for 3byte addressing and two valid erase opcodes for 4
> byte addressing. So that looks all good. After all the SFDP parsing
> I expect that you have two valid erase types:
>   - erase size 4096 with erase opcode 21h
>   - erase size 65536 with erase opcode DCh

I checked SFDP is parsed and these three erase sizes are enumerated
in the for loop in spi_nor_select_uniform_erase().

65536
0
4096

>> Also, checked that the mtd->erasesize is set to 4096 if I enable
>> CONFIG_MTD_SPI_NOR_USE_4K_SECTORS so the SPI flash can be initialized
>> with the INFO(0xef6020, 0, 0, 0) setting but, it should cover even
>> when
>> the configuration is not enabled. I think, this patch should go as it
>> is. The erasesize selecting issue could be fixed using a separate
>> patch.
>> Are you still sure that the INFO(0xef6020, 0, 0, 0) works in the
>> latest spi-next?
> 
> I've got two tested-by's with two different flashes, so yes, I'm
> pretty sure it works in principle. It might still be something
> wrong with your flash though.

Yes, I read the thread you are upstreaming currently.
https://lore.kernel.org/all/20220510140232.3519184-1-michael@walle.cc/

Were those tested in CONFIG_MTD_SPI_NOR_USE_4K_SECTORS enabled build?

As I said above, mine also works if I enable the config but we should
make it work without the config, right?

-Jae

> -michael

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

Re: [PATCH] mtd: spi-nor: winbond: add support for W25Q512NW-IQ/IN

[Jae Hyun Yoo]

Hi,

On 7/15/2022 4:03 PM, Michael Walle wrote:
> Hi,
> 
> Am 2022-07-16 00:35, schrieb Jae Hyun Yoo:
>> On 7/15/2022 1:15 PM, Jae Hyun Yoo wrote:
>>> On 7/14/2022 7:30 AM, Jae Hyun Yoo wrote:
>>>> On 7/14/2022 7:21 AM, Michael Walle wrote:
>>>>> Am 2022-07-14 16:16, schrieb Michael Walle:
>>>>>> Am 2022-07-14 15:47, schrieb Jae Hyun Yoo:
>>>>>>> On 7/14/2022 12:41 AM, Michael Walle wrote:
>>>>>>>> What does "doesn't boot at all" mean? Are there any kernel startup
>>>>>>>> messages?
>>>>>>>
>>>>>>> I'm sharing the error messages below.
>>>>>>
>>>>>> Thanks.
>>>>>>
>>>>>>> [    0.748594] spi-nor spi0.0: w25q512nwq (65536 Kbytes)
>>>>>>> [    0.865216] spi-aspeed-smc 1e620000.spi: CE0 read buswidth:4 
>>>>>>> [0x406c0741]
>>>>>>> [    0.872833] ------------[ cut here ]------------
>>>>>>> [    0.877984] WARNING: CPU: 1 PID: 1 at drivers/mtd/mtdcore.c:583
>>>>>>> add_mtd_device+0x28c/0x53c
>>>>>>> [    0.887237] CPU: 1 PID: 1 Comm: swapper/0 Not tainted
>>>>>>> 5.15.43-AUTOINC-dirty-23801a6 #1
>>>>>>
>>>>>> Could you please try it on the latest (vanilla) linux-next?
>>>>>
>>>>> or spi-nor/next [1] as there are quite a lot of changes. The
>>>>> patches shall be based on that.
>>>>
>>>> Okay. Let me try that. I tested it using 5.15.43 with back-ported
>>>> spi-nor patches from the latest. I'll back-port more changes from
>>>> the spi-nor/next and will test the INFO(0xef6020, 0, 0, 0) setting
>>>> again.
>>>
>>> I tested the setting again after cherry picking all SPI relating changes
>>> from the 'for-next' branch of
>>> git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi repository.
>>>
>>> No luck! It's still making the same warning dump at 'add_mtd_device'
>>> since 'mtd->erasesize' is checked as 0.
>>>
>>> I traced it further to check if the erasesize is properly parsed from
>>> the sfdp and checked that erase map seems parsed and initialized
>>> correctly in 'spi_nor_parse_bfpt' but problem is, a target
>>> mtd->erasesize is not properly selected in 'spi_nor_select_erase' since
>>> the 'wanted_size' variable is initialized as sector size of info table
>>> so a selected target mtd->erasesize is also 0 so looks like it's the
>>> reason why it can't initialize mtd device if we use
>>> INFO(0xef6020, 0, 0, 0).
>>>
>>> Also, checked that the mtd->erasesize is set to 4096 if I enable
>>> CONFIG_MTD_SPI_NOR_USE_4K_SECTORS so the SPI flash can be initialized 
>>> with the INFO(0xef6020, 0, 0, 0) setting but, it should cover even when
>>> the configuration is not enabled. I think, this patch should go as it
>>> is. The erasesize selecting issue could be fixed using a separate
>>> patch.
>>>
>>> Are you still sure that the INFO(0xef6020, 0, 0, 0) works in the
>>> latest spi-next?
>>
>> I also tried to fix the issue and made a fix like below.
>>
>> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
>> index 502967c76c5f..f8a020f80a56 100644
>> --- a/drivers/mtd/spi-nor/core.c
>> +++ b/drivers/mtd/spi-nor/core.c
>> @@ -2117,7 +2117,7 @@ spi_nor_select_uniform_erase(struct
>> spi_nor_erase_map *map,
>>                  * If the current erase size is the one, stop here:
>>                  * we have found the right uniform Sector Erase command.
>>                  */
>> -               if (tested_erase->size == wanted_size) {
>> +               if (wanted_size && tested_erase->size == wanted_size) {
>>                         erase = tested_erase;
>>                         break;
>>                 }
>>
>> Tested that it makes the INFO(0xef6020, 0, 0, 0) setting work and a
>> selected mtd->erasesize is 65536 which is what I expected for this
>> device.
>>
>> Not sure if it's a right fix or not. Please review and let me know if
>> it's good to submit or not.
> 
> Ahh, I think I know whats going wrong here. Thanks!
> 
> 4bait will set the erase size to 0 if there is no corresponding
> opcode for the 4byte erase. So you'll end up with
> et[0]: 4096 - 21h
> et[1]: 0 - FFh
> et[2]: 65536 - DCh
> et[3]: --
> 
> And spi_nor_select_uniform_erase() will select et[1].
> 
> Could you try the following:
> 
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index ce5d69317d46..a2c8de250e01 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -2113,6 +2113,10 @@ spi_nor_select_uniform_erase(struct 
> spi_nor_erase_map *map,
> 
>                  tested_erase = &map->erase_type[i];
> 
> +               /* Skip masked erase types. */
> +               if (!tested_erase->size)
> +                       continue;

Yes, it also works. Do you want me to update this patch with adding this
fix? Or is it good to go as is so that the INFO table can be replaced
with SNOR_ID3 later after the fix is merged?

Thanks,
Jae

> +
>                  /*
>                   * If the current erase size is the one, stop here:
>                   * we have found the right uniform Sector Erase command.
> 
> 
> -michael

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

Re: [PATCH] mtd: spi-nor: winbond: add support for W25Q512NW-IQ/IN

[Michael Walle]

Hi,

>> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
>> index ce5d69317d46..a2c8de250e01 100644
>> --- a/drivers/mtd/spi-nor/core.c
>> +++ b/drivers/mtd/spi-nor/core.c
>> @@ -2113,6 +2113,10 @@ spi_nor_select_uniform_erase(struct 
>> spi_nor_erase_map *map,
>> 
>>                  tested_erase = &map->erase_type[i];
>> 
>> +               /* Skip masked erase types. */
>> +               if (!tested_erase->size)
>> +                       continue;
> 
> Yes, it also works. Do you want me to update this patch with adding 
> this
> fix? Or is it good to go as is so that the INFO table can be replaced
> with SNOR_ID3 later after the fix is merged?

Please add this as a separate preceding patch to your original one
where you add the flash. Keep the INFO(0xef6020, 0, 0, 0). Then we
will replace it later with the SNOR_ID3().

Thanks for debugging!
-michael

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

Re: [PATCH] mtd: spi-nor: winbond: add support for W25Q512NW-IQ/IN

[Jae Hyun Yoo]

Hi,

On 7/15/2022 4:20 PM, Michael Walle wrote:
> Hi,
> 
>>> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
>>> index ce5d69317d46..a2c8de250e01 100644
>>> --- a/drivers/mtd/spi-nor/core.c
>>> +++ b/drivers/mtd/spi-nor/core.c
>>> @@ -2113,6 +2113,10 @@ spi_nor_select_uniform_erase(struct 
>>> spi_nor_erase_map *map,
>>>
>>>                  tested_erase = &map->erase_type[i];
>>>
>>> +               /* Skip masked erase types. */
>>> +               if (!tested_erase->size)
>>> +                       continue;
>>
>> Yes, it also works. Do you want me to update this patch with adding this
>> fix? Or is it good to go as is so that the INFO table can be replaced
>> with SNOR_ID3 later after the fix is merged?
> 
> Please add this as a separate preceding patch to your original one
> where you add the flash. Keep the INFO(0xef6020, 0, 0, 0). Then we
> will replace it later with the SNOR_ID3().

Sure, I'll submit v3 series including the fix then.

Thanks,
Jae

> Thanks for debugging!
> -michael

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