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(-) >> _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot