[U-Boot] Regressions in MTD / SPI FLASH

[Eugeniy Paltsev]

Hi!
Comments are inlined:

>On 04/09/19 11:37 PM, Eugeniy Paltsev wrote:
>> We faced with regressions caused by
>> commit c4e8862308d4 (mtd: spi: Switch to new SPI NOR framework)
>> This switch was performed by removing entire u-boot spi-flash
>> core implementation and copying it from another project.
>> However the switch is performed without proper testing and
>> investigations about fixes/improvements were made in u-boot
>> spi-flash core. This results in regressions.
>>
>
>Apologies for the trouble...
>As stated in cover letter, this change was necessary as U-Boot SPI flash
>stack at that time did not features such as 4 byte addressing, SFDP
>parsing, SPI controllers with MMIO interfaces etc. Also there was need
>to move to SPI MEM framework to support both SPI NAND and SPI NOR
>flashes using a single SPI controller drivers.
>I have to disagree on the part that there was no proper testing... As
>evident from mailing list archives, patch series was reviewed by
>multiple reviewers and tested on their platforms as well...
>Unfortunately its impossible to get all boards owners to test it.

I'm not talking about getting all customers board and testing changes on them.
It could be done another way - i.e. like it is done for u-boot driver-model migration:
by introducing the option for choosing which stack will be used and allow customers
to switch the flash IC they use to new stack until some deadline.

>> One of regression we faced with is related to SST26 flash series which
>> is used on HSDK board. The cause is that SST26 protection ops
>> implementation was dropped. The fix of this regression is send
>> as a patch in this series.
>>
>
>I retained most U-Boot specific code as is (like support for BANK
>address registers, restriction in transfer sizes) but I somehow
>overlooked this part. Sorry for that
>
>> However there are another regressions. I.E: we also faced with broken
>> SPI flash on another SNPS boards - AXS101 and AXS103. They use different
>> flash IC (n25q512ax3) and I didn't investigate the cause yet.
>>
>
>Could you provide more details here:
>What exactly fails? Is the detected correctly? Does reads work fine? Is
>Erase or Write broken?

It seems to me that something is wrong with protection ops.
The erase and write finish without errors however nothing actually happens.

>Could you enable debug prints in your driver as well as debug prints in
>spi_mem_exec_op() (in drivers/spi/spi-mem.c) and post the logs?


Here is it.
As you can see all commands finish successfully however erase and write don't
change flash content.
------------------------------------->8-----------------------------------------
AXS# sf probe && echo OK
spi_flash_std_probe: slave=9fdabfc0, cs=0
9f | [6B in] 20 ba 20 10 00 00 [ret 0]
SF: Detected n25q512ax3 with page size 256 Bytes, erase size 4 KiB, total 64 MiB
OK
AXS# sf read 0x81000000 0x180000 0x10 && echo OK
device 0 offset 0x180000, size 0x10
0c 00 18 00 00 | [16B in] 53 00 f8 d9 04 00 52 fc ff ff 80 de ad be af af [ret 0]
SF: 16 bytes @ 0x180000 Read: OK
OK
AXS# md.b 0x81000000 0x10
81000000: 53 00 f8 d9 04 00 52 fc ff ff 80 de ad be af af    S.....R.........
AXS# sf protect unlock 0x0 0x4000000 && echo OK
05 | [1B in] 00 [ret 0]
OK
AXS# sf erase 0x180000 0x1000 && echo OK
06 | [0B -] [ret 0]
21 00 18 00 00 | [0B -] [ret 0]
05 | [1B in] 02 [ret 0]
70 | [1B in] 80 [ret 0]
04 | [0B -] [ret 0]
SF: 4096 bytes @ 0x180000 Erased: OK
OK
AXS# sf read 0x81000000 0x180000 0x10 && echo OK
device 0 offset 0x180000, size 0x10
0c 00 18 00 00 | [16B in] 53 00 f8 d9 04 00 52 fc ff ff 80 de ad be af af [ret 0]
SF: 16 bytes @ 0x180000 Read: OK
OK
AXS# md.b 0x81000000 0x10
81000000: 53 00 f8 d9 04 00 52 fc ff ff 80 de ad be af af    S.....R.........
AXS# mw 0x81000000 0 5
AXS# sf write 0x81000000 0x180000 0x10 && echo OK
device 0 offset 0x180000, size 0x10
06 | [0B -] [ret 0]
12 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] 00 [ret 0]
70 | [1B in] 80 [ret 0]
SF: 16 bytes @ 0x180000 Written: OK
OK
AXS# sf read 0x81000000 0x180000 0x10 && echo OK
device 0 offset 0x180000, size 0x10
0c 00 18 00 00 | [16B in] 53 00 f8 d9 04 00 52 fc ff ff 80 de ad be af af [ret 0]
SF: 16 bytes @ 0x180000 Read: OK
OK
AXS# md.b 0x81000000 0x10
81000000: 53 00 f8 d9 04 00 52 fc ff ff 80 de ad be af af    S.....R.........
------------------------------------->8-----------------------------------------


Here is how it should work (tested on v2018.09):
------------------------------------->8-----------------------------------------
AXS# sf probe && echo OK
SF: Detected n25q512 with page size 256 Bytes, erase size 4 KiB, total 64 MiB
OK
AXS# sf read 0x81000000 0x180000 0x10 && echo OK
device 0 offset 0x180000, size 0x10
SF: 16 bytes @ 0x180000 Read: OK
OK
AXS# md.b 0x81000000 0x10
81000000: 53 00 f8 d9 04 00 52 fc ff ff 80 de ad be af af    S.....R.........
AXS# sf protect unlock 0x0 0x4000000 && echo OK
AXS#
AXS# sf erase 0x180000 0x1000 && echo OK
SF: 4096 bytes @ 0x180000 Erased: OK
OK
AXS# sf read 0x81000000 0x180000 0x10 && echo OK
device 0 offset 0x180000, size 0x10
SF: 16 bytes @ 0x180000 Read: OK
OK
AXS# md.b 0x81000000 0x10
81000000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff    ................
AXS# mw 0x81000000 0 5
AXS# sf write 0x81000000 0x180000 0x10 && echo OK
device 0 offset 0x180000, size 0x10
SF: 16 bytes @ 0x180000 Written: OK
OK
AXS# sf read 0x81000000 0x180000 0x10 && echo OK
device 0 offset 0x180000, size 0x10
SF: 16 bytes @ 0x180000 Read: OK
OK
AXS# md.b 0x81000000 0x10
81000000: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00    ................
------------------------------------->8-----------------------------------------

>
>Regards
>Vignesh
>
>> I can also expect regressions among other u-boot users
>> and I believe that subsystem changes mustn't be done such harmful way.
>>
>> Eugeniy Paltsev (1):
>>   MTD: SPI: revert removing SST26* flash IC protection ops
>>
>>  drivers/mtd/spi/sf_internal.h  |   1 +
>>  drivers/mtd/spi/spi-nor-core.c | 181 +++++++++++++++++++++++++++++++++
>>  drivers/mtd/spi/spi-nor-ids.c  |   8 +-
>>  include/linux/mtd/spi-nor.h    |   4 +
>>  4 files changed, 190 insertions(+), 4 deletions(-)
>>