[U-Boot] [PATCH RFT 0/3] spi-nor: spi-nor-ids: Fix 4 Byte addressing for n25q256 and n25q512*

Wed Oct 9 11:48:13 UTC 2019

Hi Eugeniy,

On 07/10/19 8:16 PM, Eugeniy Paltsev wrote:
> Hi Vignesh,
> 
> I've tested your "[U-Boot,RFT,v2,0/3] spi-nor: spi-nor-ids: Fix 4 Byte addressing " series
> applies on the latest master (879396a2405).
> 'axs103_defconfig' was used without changes.
> > Probe/read/write/erase work for n25q512ax3.
>

Thanks for the testing!

> Lock/unlock don't work, so here is debug log:
> (I've tried to run lock/unlock after unlock/lock and write/erase after lock, so I guess it will cover all combinations useful for debug)
> 
> --------------------->8--------------------------
> AXS# sf probe && echo OK
> 9f | [6B in] 20 ba 20 10 00 00 [ret 0]
> 06 | [0B -] [ret 0]
> b7 | [0B -] [ret 0]
> 04 | [0B -] [ret 0]
> SF: Detected n25q512ax3 with page size 256 Bytes, erase size 4 KiB, total 64 MiB
> OK
> AXS# sf protect lock 0x0 0x4000000 && echo OK
> 05 | [1B in] 9c [ret 0]
> OK

You are trying to lock entire 64MB space. Per Data sheet of flash, this
requires setting BP3-BP0 bits in Status register. But only BP2-BP0 are set

Neither the old framework nor current framework (or even Linux kernel)
supports handling of BP3 bit. So this failure is _not a regression_.

In this particular case, U-Boot code sets only BP2-BP0 when asked to
lock entire 64MB range. But this effectively locks sectors 960 to 1023.

Regards
Vignesh

> AXS# mw 0x81000000 0 5 && sf write 0x81000000 0x180000 0x10 && echo OK
> device 0 offset 0x180000, size 0x10
> 06 | [0B -] [ret 0]
> 02 00 18 00 00 | [16B out] 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [ret 0]
> 05 | [1B in] 9c [ret 0]
> 70 | [1B in] 81 [ret 0]
> SF: 16 bytes @ 0x180000 Written: OK
> OK
> AXS# sf protect unlock 0x0 0x4000000 && echo OK
> 05 | [1B in] 9c [ret 0]
> 06 | [0B -] [ret 0]
> 01 | [1B out] 00 [ret 0]
> 05 | [1B in] 00 [ret 0]
> 70 | [1B in] 81 [ret 0]
> 05 | [1B in] 00 [ret 0]
> OK
> AXS# sf protect lock 0x0 0x4000000 && echo OK
> 05 | [1B in] 00 [ret 0]
> 06 | [0B -] [ret 0]
> 01 | [1B out] 9c [ret 0]
> 05 | [1B in] 9c [ret 0]
> 70 | [1B in] 81 [ret 0]
> 05 | [1B in] 9c [ret 0]
> OK
> AXS# sf protect lock 0x0 0x4000000 && echo OK
> 05 | [1B in] 9c [ret 0]
> OK
> AXS#  
> AXS# 
> AXS# sf protect unlock 0x0 0x4000000 && echo OK
> 05 | [1B in] 9c [ret 0]
> 06 | [0B -] [ret 0]
> 01 | [1B out] 00 [ret 0]
> 05 | [1B in] 00 [ret 0]
> 70 | [1B in] 81 [ret 0]
> 05 | [1B in] 00 [ret 0]
> OK
> AXS# sf protect unlock 0x0 0x4000000 && echo OK
> 05 | [1B in] 00 [ret 0]
> OK
> AXS#  
> AXS# 
> AXS# sf protect lock 0x0 0x4000000 && echo OK  
> 05 | [1B in] 00 [ret 0]
> 06 | [0B -] [ret 0]
> 01 | [1B out] 9c [ret 0]
> 05 | [1B in] 9c [ret 0]
> 70 | [1B in] 81 [ret 0]
> 05 | [1B in] 9c [ret 0]
> OK
> AXS# sf erase 0x180000 0x1000 && echo OK
> 06 | [0B -] [ret 0]
> 20 00 18 00 00 | [0B -] [ret 0]
> 05 | [1B in] 9f [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 9f [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 9f [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 9f [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 9f [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 9f [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 9f [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 9f [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 9f [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 9f [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 9f [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 9f [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 9f [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 9f [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 9f [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 9f [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 9f [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 9f [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 9f [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 9f [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 9f [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 9f [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 9f [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 9f [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 9f [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 9f [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 9f [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 9f [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 9f [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 9f [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 9f [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 9f [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 9f [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 9f [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 9f [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 9f [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 9f [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 9f [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 9f [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 9f [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 9f [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 9f [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 9f [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 9f [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 9f [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 9f [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 9f [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 9f [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 9f [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 9f [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 9f [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 9f [ret 0]
> 70 | [1B in] 01 [ret 0]
> 05 | [1B in] 9c [ret 0]
> 70 | [1B in] 81 [ret 0]
> 04 | [0B -] [ret 0]
> SF: 4096 bytes @ 0x180000 Erased: OK
> OK
> AXS# sf read 0x81000000 0x180000 0x10 && md.b 0x81000000 0x10 && echo OK
> device 0 offset 0x180000, size 0x10
> 0b 00 18 00 00 | [16B in] ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff [ret 0]
> SF: 16 bytes @ 0x180000 Read: OK
> 81000000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
> OK
> AXS# mw 0x81000000 0 5 && sf write 0x81000000 0x180000 0x10 && echo OK
> device 0 offset 0x180000, size 0x10
> 06 | [0B -] [ret 0]
> 02 00 18 00 00 | [16B out] 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [ret 0]
> 05 | [1B in] 9c [ret 0]
> 70 | [1B in] 81 [ret 0]
> SF: 16 bytes @ 0x180000 Written: OK
> OK
> AXS# sf read 0x81000000 0x180000 0x10 && echo OK
> device 0 offset 0x180000, size 0x10
> 0b 00 18 00 00 | [16B in] 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [ret 0]
> SF: 16 bytes @ 0x180000 Read: OK
> OK
> --------------------->8--------------------------
> 
> ---
>  Eugeniy Paltsev
> 
> ________________________________________
> From: Vignesh Raghavendra <vigneshr at ti.com>
> Sent: Wednesday, September 25, 2019 11:11
> To: Eugeniy Paltsev; Jagan Teki; Ashish Kumar; Simon Goldschmidt
> Cc: u-boot at lists.denx.de; Tom Rini; Alexey Brodkin
> Subject: Re: [PATCH RFT 0/3] spi-nor: spi-nor-ids: Fix 4 Byte addressing for n25q256 and n25q512*
> 
> 
> 
> On 24/09/19 10:53 PM, Eugeniy Paltsev wrote:
>> Hi Vignesh,
>>
>> I've check this patches on top of 31e086e460f.
>> The read/write/erase seems to work.
>>
>> However, as I can see 'sf protect lock' doesn't work - it finish successfully but the area remains unlocked.
> 
> Did you verify that area is indeed unlocked by writing data and then reading it back?
> I was able to find a board with mt25qu512a which is same as n25q512a in terms of locking
> I see it works fine:
> 
> => sf probe
> SF: Detected n25q512a with page size 256 Bytes, erase size 4 KiB, total 64 MiB
> => sf protect lock 0 0x4000000;  echo $?
> 0
> => sf write 0x82000000 0x3FF0000 0x100
> device 0 offset 0x3ff0000, size 0x100
> SF: 256 bytes @ 0x3ff0000 Written: ERROR -5
> 
> If you still see failures wrt locking, could you provide debug logs from
> spi_mem_exec_op() (in drivers/spi/spi-mem.c) just like last time?
> 
> 
> Regards
> Vignesh
> 
>> As I remember It worked with old u-boot spi-nor code, but I need to check it.
>>
>> ---
>>  Eugeniy Paltsev
>>
>>
>> ________________________________________
>> From: Vignesh Raghavendra <vigneshr at ti.com>
>> Sent: Tuesday, September 24, 2019 08:56
>> To: Jagan Teki; Eugeniy Paltsev; Ashish Kumar; Simon Goldschmidt
>> Cc: Vignesh Raghavendra; u-boot at lists.denx.de; Tom Rini; Alexey Brodkin
>> Subject: [PATCH RFT 0/3] spi-nor: spi-nor-ids: Fix 4 Byte addressing for n25q256 and n25q512*
>>
>> This series removes SPI_NOR_4B_OPCODES flags from legacy variants of
>> n25q256* and n25q512* and adds entries for newer variants of those
>> flashes that support 4 Byte opcodes.
>>
>> I don't have the flash devices. So its only compile tested.
>>
>> Ashish, Simon,
>>
>> I would greatly appreciate if you could test these patches and make sure
>> 4 Byte opcodes are being used. (Probably by enabling/adding prints to
>> cmd->opcode in spi_mem_exec_op() in drivers/spi/spi-mem.c
>>
>> Euginey,
>>
>> Could you test this series on top of latest u-boot master and confirm
>> that your test cases still work?
>>
>> Regards
>> Vignesh
>>
>> Vignesh Raghavendra (3):
>>   spi-nor: spi-nor-ids: Disable SPI_NOR_4B_OPCODES for n25q512* and
>>     n25q256*
>>   spi-nor: spi-nor-ids: Rename mt25qu512a entry
>>   spi-nor: spi-nor-ids: Add entries for newer variants of n25q256* and
>>     n25q512*
>>
>>  drivers/mtd/spi/spi-nor-ids.c | 13 ++++++++-----
>>  1 file changed, 8 insertions(+), 5 deletions(-)
>>
>> --
>> 2.23.0
>>
> 
> --
> Regards
> Vignesh
> 

-- 
Regards
Vignesh