Re: [PATCH v3 00/14] spi: airoha: driver fixes & improvements

[Lorenzo Bianconi <lorenzo@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 6884 bytes --]

> 
> On 22.08.2025 09:46, Lorenzo Bianconi wrote:
> >> On 21.08.2025 14:34, Lorenzo Bianconi wrote:
> >>>> On 21.08.2025 09:58, Lorenzo Bianconi wrote:
> >>>>>> This patch series greatly improve airoha snfi driver and fix a
> >>>>>> number of serious bug.
> >>>>>>
> >>>>>> Fixed bugs:
> >>>>>>  * Fix reading/writing of flashes with more than one plane per lun
> >>>>>>  * Fix inability to read/write oob area
> >>>>>>  * Fill the buffer with 0xff before writing
> >>>>>>  * Fix reading of flashes supporting continuous reading mode
> >>>>>>  * Fix error paths
> >>>>>>
> >>>>>> Improvements:
> >>>>>>  * Add support of dual/quad wires spi modes in exec_op().
> >>>>>>  * Support of dualio/quadio flash reading commands
> >>>>>>  * Remove dirty hack that reads flash page settings from SNFI registers
> >>>>>>    during driver startup
> >>>>>>
> >>>>>> Unfortunately I am unable to test the driver with linux at the moment,
> >>>>>> so only the following testing was done:
> >>>>> It seems to me this is quite an important rework of the driver. I would prefer
> >>>>> to have some test results for this series. Are you able to run mtd_test kernel
> >>>>> module for testing?
> >>>> I'll try to build latest openwrt with this patches  and mtd_test kernel
> >>>> module and try it on one of our boards.
> >>> what board are you using for testing? If it is based on Airoha-7581 you could
> >>> use the following repo for testing.
> >>>
> >>> https://github.com/Ansuel/openwrt/tree/openwrt-24.10-airoha-an7581-stable
> >>>
> >>> Regards,
> >>> Lorenzo
> >> What tests do you suggest to run?
> > IIRC I run all of them. Can you please report even if there are some
> > improvements (or penalties) in read/write speed
> Do you mean to run it twice? with patches and without?

I mean I would suggest to check if there are any difference in read/write
speed (so yes, you need a baseline).

Regards,
Lorenzo

> >
> >> I have a single flash I boot from. It have only 2 mtd partitions:
> >>
> >> [    2.980849] spi-nand spi0.0: Micron SPI NAND was found.
> >> [    2.986102] spi-nand spi0.0: 256 MiB, block size: 128 KiB, page size:
> >> 2048, OOB size: 128
> >> [    2.994493] 2 fixed-partitions partitions found on MTD device spi0.0
> >> [    3.000856] Creating 2 MTD partitions on "spi0.0":
> >> [    3.005651] 0x000000000000-0x000000020000 : "bl2"
> >> [    3.011247] 0x000000020000-0x000010000000 : "ubi"
> >>
> >> Most of tests are destructive. So If I use "bl2" or "ubi" partition for
> >> test, next time I will be unable to boot :-(
> > yes, I flashed the device after carrying out the test.
> >
> > Regards,
> > Lorenzo
> >
> >> Do you suggest to patch u-boot & linux to have more mtd partitions?
> >>
> >> This is the results of the only read-only test I found.
> >>
> >> root@OpenWrt:/lib/modules/6.6.79# insmod mtd_test.ko
> >> root@OpenWrt:/lib/modules/6.6.79# insmod mtd_readtest.ko dev=1
> >> [  159.121706]
> >> [  159.123220] =================================================
> >> [  159.129053] mtd_readtest: MTD device: 1
> >> [  159.132898] mtd_readtest: MTD device size 268304384, eraseblock size
> >> 131072, page size 2048, count of eraseblocks 2047, pages per eraseblock
> >> 64, OOB size 128
> >> [  159.147008] mtd_test: scanning for bad eraseblocks
> >> [  159.152141] mtd_test: scanned 2047 eraseblocks, 0 are bad
> >> [  159.157549] mtd_readtest: testing page read
> >>
> >> Mikhail
> >>
> >>>> Actually patches can be divided on to parts:
> >>>> * fixes of current driver (patches 1-10)
> >>>> * change of behavior to avoid reading flash page settings from SNFI
> >>>> registers during driver startup (patches 11-14)
> >>>>
> >>>> The changes are based on the code we are using for more than 3 years. I
> >>>> adapt it to latest linux/u-boot code.
> >>>>
> >>>> Up to now the only known issue appears on en7523 chips only. Here a
> >>>> corresponding patch description (not added to this series)
> >>>> ====================================================== spi: airoha:
> >>>> en7523: workaround flash damaging if UART_TXD was short to GND We found
> >>>> that some serial console may pull TX line to GROUND during board boot
> >>>> time. Airoha uses TX line as one of it's BOOT pins. This will lead to
> >>>> booting in RESERVED boot mode. It was found that some flashes operates
> >>>> incorrectly in RESERVED mode. Micron and Skyhigh flashes are definitely
> >>>> affected by the issue, Winbond flashes are NOT affected. Details:
> >>>> -------- DMA reading of odd pages on affected flashes operates
> >>>> incorrectly. Page reading offset (start of the page) on hardware level
> >>>> is replaced by 0x10. Thus results in incorrect data reading. Usage of
> >>>> UBI make things even worse. Any attempt to access UBI leads to ubi
> >>>> damaging. As result OS loading becomes impossible. Non-DMA reading is
> >>>> OK. =======================================================
> >>>>
> >>>> Regards,
> >>>> Mikhail
> >>>>
> >>>>
> >>>>> Regards,
> >>>>> Lorenzo
> >>>>>
> >>>>>>  * Driver compiles without error.
> >>>>>>  * All changes were tested with corresponding u-boot driver. U-Boot
> >>>>>>    SpiNAND driver was modified as well to match linux-6.17-rc2 with
> >>>>>>    additional fixes for continuous mode.
> >>>>>>
> >>>>>> Changes v2:
> >>>>>>  * minor fix
> >>>>>>  * add comments to code
> >>>>>>
> >>>>>> Changes v3:
> >>>>>>  * add patch to prevent continuous reading
> >>>>>>
> >>>>>> Mikhail Kshevetskiy (14):
> >>>>>>   spi: airoha: return an error for continuous mode dirmap creation cases
> >>>>>>   spi: airoha: remove unnecessary restriction length
> >>>>>>   spi: airoha: add support of dual/quad wires spi modes
> >>>>>>   spi: airoha: remove unnecessary switch to non-dma mode
> >>>>>>   spi: airoha: unify dirmap read/write code
> >>>>>>   spi: airoha: switch back to non-dma mode in the case of error
> >>>>>>   spi: airoha: fix reading/writing of flashes with more than one plane
> >>>>>>     per lun
> >>>>>>   spi: airoha: support of dualio/quadio flash reading commands
> >>>>>>   spi: airoha: allow reading/writing of oob area
> >>>>>>   spi: airoha: buffer must be 0xff-ed before writing
> >>>>>>   spi: airoha: avoid setting of page/oob sizes in REG_SPI_NFI_PAGEFMT
> >>>>>>   spi: airoha: reduce the number of modification of REG_SPI_NFI_CNFG and
> >>>>>>     REG_SPI_NFI_SECCUS_SIZE registers
> >>>>>>   spi: airoha: set custom sector size equal to flash page size
> >>>>>>   spi: airoha: avoid reading flash page settings from SNFI registers
> >>>>>>     during driver startup
> >>>>>>
> >>>>>>  drivers/spi/spi-airoha-snfi.c | 508 +++++++++++++++++-----------------
> >>>>>>  1 file changed, 260 insertions(+), 248 deletions(-)
> >>>>>>
> >>>>>> -- 
> >>>>>> 2.50.1
> >>>>>>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]