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(-)