| Message ID | 1598889805-30399-1-git-send-email-yibin.gong@nxp.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | add ecspi ERR009165 for i.mx6/7 soc family | expand |
On 20-09-01 00:03, Robin Gong wrote: > There is ecspi ERR009165 on i.mx6/7 soc family, which cause FIFO > transfer to be send twice in DMA mode. Please get more information from: > https://www.nxp.com/docs/en/errata/IMX6DQCE.pdf. The workaround is adding > new sdma ram script which works in XCH mode as PIO inside sdma instead > of SMC mode, meanwhile, 'TX_THRESHOLD' should be 0. The issue should be > exist on all legacy i.mx6/7 soc family before i.mx6ul. > NXP fix this design issue from i.mx6ul, so newer chips including i.mx6ul/ > 6ull/6sll do not need this workaroud anymore. All other i.mx6/7/8 chips > still need this workaroud. This patch set add new 'fsl,imx6ul-ecspi' > for ecspi driver and 'ecspi_fixed' in sdma driver to choose if need errata > or not. > The first two reverted patches should be the same issue, though, it > seems 'fixed' by changing to other shp script. Hope Sean or Sascha could > have the chance to test this patch set if could fix their issues. > Besides, enable sdma support for i.mx8mm/8mq and fix ecspi1 not work > on i.mx8mm because the event id is zero. > > PS: > Please get sdma firmware from below linux-firmware and copy it to your > local rootfs /lib/firmware/imx/sdma. > https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/tree/imx/sdma Hi Robin, I took your patches and did a few test on the mainline available fsl,imx6q-sabrelite. I used a vanilla linux version v5.9-rc1 for all my tests except the needed SPI-NOR patches [1]. Following are my results: Testcase 1: "Using ROM-FW" === [OK] Playing Audio (SSI) [OK] TX/RX bytes on a different UART (not the serial used for interaction) [OK] Writing to the SPI-NOR [OK] Doing all at the same time (once for TX and once for RX on UART) Notes: - Your Patches adding a maybe noise message "sdma firmware not ready". Maybe we should consider about that if it should be a warning or a info. - For spi-nor I did run this test: dd if=/dev/urandom of=/var/tmp/test1M bs=1M count=1 && \ flashcp -v /var/tmp/test1M /dev/mtd2 and checked /proc/interrupts: 25: 2107169 0 0 0 GPC 31 Level 2008000.spi Testcase 2: "Using new FW from linux-firmware" === [OK] Playing Audio (SSI) [OK] TX/RX bytes on a different UART (not the serial used for interaction) [OK] Writing to the SPI-NOR [OK] Doing all at the same time (once for TX and once for RX on UART) Notes: - For spi-nor I did run this test: dd if=/dev/urandom of=/var/tmp/test1M bs=1M count=1 && \ flashcp -v /var/tmp/test1M /dev/mtd2 and checked /proc/interrupts: 25: 2107993 0 0 0 GPC 31 Level 2008000.spi I saw no SDMA interrupts during this testcase instead I saw only spi controller interrupts. - According linux-firmware you did a version bump from 3.5 to 4.5 but my dmesg shows: imx-sdma 20ec000.sdma: loaded firmware 3.5 SPI Benchmark: === flash_erase /dev/mtd2 0 0 && \ dd if=/dev/urandom of=/dev/mtd2 bs=1M count=1 - without firmware (ROM-FW) 1048576 bytes (1.0 MB, 1.0 MiB) copied, 51.9713 s, 20.2 kB/s - with firmware 1048576 bytes (1.0 MB, 1.0 MiB) copied, 59.4174 s, 17.6 kB/s Conclusion: === It seems that we don't have any performance boost with your patchset instead we are increasing the complexity and the interrupts... Pls let me know if I did something wrong during testing or if my test setup was wrong. Note: the /dev/mtd2 isn't mainlined yet but if you use barebox you only have to add: 8<--------------------------------------------------------------------- diff --git a/arch/arm/dts/imx6qdl-sabrelite.dtsi b/arch/arm/dts/imx6qdl-sabrelite.dtsi index ec3d364bde..256dd90a0f 100644 --- a/arch/arm/dts/imx6qdl-sabrelite.dtsi +++ b/arch/arm/dts/imx6qdl-sabrelite.dtsi @@ -38,6 +38,11 @@ label = "barebox-environment"; reg = <0xe0000 0x20000>; }; + + parition@100000 { + label = "user-partition"; + reg = <0x100000 0x100000>; + }; }; &ocotp { 8<--------------------------------------------------------------------- to the barebox device tree. [1] http://lists.infradead.org/pipermail/linux-mtd/2020-September/082099.html Regards, Marco
On 2020/09/12 0:40 Marco Felsch <m.felsch@pengutronix.de> wrote: > Hi Robin, > > I took your patches and did a few test on the mainline available > fsl,imx6q-sabrelite. I used a vanilla linux version v5.9-rc1 for all my tests except > the needed SPI-NOR patches [1]. Following are my results: Marco, thanks for your test :) > > Testcase 1: "Using ROM-FW" > === > [OK] Playing Audio (SSI) > [OK] TX/RX bytes on a different UART (not the serial used for > interaction) > [OK] Writing to the SPI-NOR > [OK] Doing all at the same time (once for TX and once for RX on UART) > > Notes: > - Your Patches adding a maybe noise message "sdma firmware not ready". > Maybe we should consider about that if it should be a warning or a info. That means the script you're using is ram script which may not be loaded as your case. That should be a warning I think, to avoid too much noise I have refine it to dev_warn_once. > > - For spi-nor I did run this test: > dd if=/dev/urandom of=/var/tmp/test1M bs=1M count=1 && \ > flashcp -v /var/tmp/test1M /dev/mtd2 > > and checked /proc/interrupts: > 25: 2107169 0 0 0 GPC 31 > Level 2008000.spi > > Testcase 2: "Using new FW from linux-firmware" > === > [OK] Playing Audio (SSI) > [OK] TX/RX bytes on a different UART (not the serial used for > interaction) > [OK] Writing to the SPI-NOR > [OK] Doing all at the same time (once for TX and once for RX on UART) > > Notes: > - For spi-nor I did run this test: > dd if=/dev/urandom of=/var/tmp/test1M bs=1M count=1 && \ > flashcp -v /var/tmp/test1M /dev/mtd2 > > and checked /proc/interrupts: > 25: 2107993 0 0 0 GPC 31 > Level 2008000.spi > > I saw no SDMA interrupts during this testcase instead I saw only spi > controller interrupts. That's not expected. But I have tried just now and show that SDMA interrupt caught by spi as belows. Are you sure sdma firmware loaded indeed? ./spidev_test -D /dev/spidev0.0 -s 1200000 -b 8 -S 512 -I 1 -l 8 -S 512 -I spi mode: 0x24 bits per word: 8 max speed: 1200000 Hz (1200 kHz) total: tx 0.5KB, rx 0.5KB root@imx6qpdlsolox:~# cat /proc/interrupts | grep dma 58: 2 0 GPC 2 Level sdma > > - According linux-firmware you did a version bump from 3.5 to 4.5 but my > dmesg shows: > imx-sdma 20ec000.sdma: loaded firmware 3.5 3.x is used for i.mx6 family while 4.x is used for i.mx7/8 since there are some change on ROM which depended by RAM script. Not means version bump between 3.5/4.5. 3.5 is correct on i.mx6q. > > SPI Benchmark: > === > flash_erase /dev/mtd2 0 0 && \ > dd if=/dev/urandom of=/dev/mtd2 bs=1M count=1 > > - without firmware (ROM-FW) > 1048576 bytes (1.0 MB, 1.0 MiB) copied, 51.9713 s, 20.2 kB/s > > - with firmware > 1048576 bytes (1.0 MB, 1.0 MiB) copied, 59.4174 s, 17.6 kB/s > > Conclusion: > === > It seems that we don't have any performance boost with your patchset instead > we are increasing the complexity and the interrupts... Yes, that's expected. What ERR009165 fix is data correct on spi bus though that bring performance drop in dma mode, because the workaround just let sdma do similar thing as cpu (PIO), while cpu running faster than sdma. If you care much spi performance, PIO is better way. If care cpu loading, dma way is better. > > Pls let me know if I did something wrong during testing or if my test setup was > wrong. Note: the /dev/mtd2 isn't mainlined yet but if you use barebox you only > have to add: > 8<--------------------------------------------------------------------- > diff --git a/arch/arm/dts/imx6qdl-sabrelite.dtsi > b/arch/arm/dts/imx6qdl-sabrelite.dtsi > index ec3d364bde..256dd90a0f 100644 > --- a/arch/arm/dts/imx6qdl-sabrelite.dtsi > +++ b/arch/arm/dts/imx6qdl-sabrelite.dtsi > @@ -38,6 +38,11 @@ > label = "barebox-environment"; > reg = <0xe0000 0x20000>; > }; > + > + parition@100000 { > + label = "user-partition"; > + reg = <0x100000 0x100000>; > + }; > }; > > &ocotp { > 8<--------------------------------------------------------------------- > to the barebox device tree. > > [1] > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Flists.infra > dead.org%2Fpipermail%2Flinux-mtd%2F2020-September%2F082099.html&am > p;data=02%7C01%7Cyibin.gong%40nxp.com%7C324d4b5c2f2344883a3108d85 > 67166f9%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C1%7C63735439234 > 6471210&sdata=ru8fKz6wpDhzYeaHIT28T0OybHlCFHJ41N1lJYuqKgE%3D& > amp;reserved=0 > > Regards, > Marco
There is ecspi ERR009165 on i.mx6/7 soc family, which cause FIFO transfer to be send twice in DMA mode. Please get more information from: https://www.nxp.com/docs/en/errata/IMX6DQCE.pdf. The workaround is adding new sdma ram script which works in XCH mode as PIO inside sdma instead of SMC mode, meanwhile, 'TX_THRESHOLD' should be 0. The issue should be exist on all legacy i.mx6/7 soc family before i.mx6ul. NXP fix this design issue from i.mx6ul, so newer chips including i.mx6ul/ 6ull/6sll do not need this workaroud anymore. All other i.mx6/7/8 chips still need this workaroud. This patch set add new 'fsl,imx6ul-ecspi' for ecspi driver and 'ecspi_fixed' in sdma driver to choose if need errata or not. The first two reverted patches should be the same issue, though, it seems 'fixed' by changing to other shp script. Hope Sean or Sascha could have the chance to test this patch set if could fix their issues. Besides, enable sdma support for i.mx8mm/8mq and fix ecspi1 not work on i.mx8mm because the event id is zero. PS: Please get sdma firmware from below linux-firmware and copy it to your local rootfs /lib/firmware/imx/sdma. https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/tree/imx/sdma v2: 1.Add commit log for reverted patches. 2.Add comment for 'ecspi_fixed' in sdma driver. 3.Add 'fsl,imx6sll-ecspi' compatible instead of 'fsl,imx6ul-ecspi' rather than remove. v3: 1.Confirm with design team make sure ERR009165 fixed on i.mx6ul/i.mx6ull /i.mx6sll, not fixed on i.mx8m/8mm and other i.mx6/7 legacy chips. Correct dts related dts patch in v2. 2.Clean eratta information in binding doc and new 'tx_glitch_fixed' flag in spi-imx driver to state ERR009165 fixed or not. 3.Enlarge burst size to fifo size for tx since tx_wml set to 0 in the errata workaroud, thus improve performance as possible. v4: 1.Add Ack tag from Mark and Vinod 2.Remove checking 'event_id1' zero as 'event_id0'. v5: 1.Add the last patch for compatible with the current uart driver which using rom script, so both uart ram script and rom script supported in latest firmware, by default uart rom script used. UART driver will be broken without this patch. v6: 1.Resend after rebase the latest next branch. 2.Remove below No.13~No.15 patches of v5 because they were mergered. ARM: dts: imx6ul: add dma support on ecspi ARM: dts: imx6sll: correct sdma compatible arm64: defconfig: Enable SDMA on i.mx8mq/8mm 3.Revert "dmaengine: imx-sdma: fix context cache" since 'context_loaded' removed. v7: 1.Put the last patch 13/13 'Revert "dmaengine: imx-sdma: fix context cache"' to the ahead of 03/13 'Revert "dmaengine: imx-sdma: refine to load context only once" so that no building waring during comes out during bisect. 2.Address Sascha's comments, including eliminating any i.mx6sx in this series, adding new 'is_imx6ul_ecspi()' instead imx in imx51 and taking care SMC bit for PIO. 3.Add back missing 'Reviewed-by' tag on 08/15(v5):09/13(v7) 'spi: imx: add new i.mx6ul compatible name in binding doc' v8: 1.remove 0003-Revert-dmaengine-imx-sdma-fix-context-cache.patch and merge it into 04/13 of v7 2.add 0005-spi-imx-fallback-to-PIO-if-dma-setup-failure.patch for no any ecspi function broken even if sdma firmware not updated. 3.merge 'tx.dst_maxburst' changes in the two continous patches into one patch to avoid confusion. 4.fix typo 'duplicated'. v9: 1. add "spi: imx: add dma_sync_sg_for_device after fallback from dma" to fix the potential issue brought by commit bcd8e7761ec9("spi: imx: fallback to PIO if dma setup failure") which is the only one patch of v8 merged. Thanks Matthias for reporting: https://lore.kernel.org/linux-arm-kernel/5d246dd81607bb6e5cb9af86ad4e53f7a7a99c50.camel@ew.tq-group.com/ 2. remove 05/13 of v8 "spi: imx:fallback to PIO if dma setup failure" since it's been merged. v10: 1. remove 01/13 "spi: imx: add dma_sync_sg_for_device after fallback from dma" since there is another independent patch merged: -- commit 809b1b04df898 ("spi: introduce fallback to pio") 2. add "dmaengine: dma: imx-sdma: add fw_loaded and is_ram_script" which is used to fix the potential dma_alloc_coherent() failure while this patchset applied but sdma firmware may not be ready for long time. 3. burst size change back from fifo size to normal wml to align with nxp internal tree which has been test for years. Overnight with loopback test with spidev failed with fifo size, but pass with wml(half of fifo size).Seems the whole fifo size fed may cause rxfifo overflow during tx shift out while rx shift in. "spi: imx: remove ERR009165 workaround on i.mx6ul" 4. remove 12/13 'dmaengine: imx-sdma: fix ecspi1 rx dma not work on i.mx8mm' since below two similar patches merged: -- commit 25962e1a7f1d ("dmaengine: imx-sdma: Fix the event id check to include RX event for UART6") -- commit 2f57b8d57673 ("dmaengine: imx-sdma: Fix: Remove 'always true' comparison") v11: 1. change dev_err() to dev_warn_once() in case sdma firmware not loaded to eliminate meaningless duplicate log print. v12: 1. take care uart_2_mcu_addr/uartsh_2_mcu_addr since such rom scripts are now located in the bottom part of sdma_script_start_addrs which are beyond the SDMA_SCRIPT_ADDRS_ARRAY_SIZE_V1. Reported by Frieder as below: https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg2263544.html v13: 1. rebase with latest linux-next. 2. remove 09/12 'spi: imx: add new i.mx6ul compatible name in binding doc' since it's been converted to yaml already. 3. add 'Fixes', 'Cc: stable@vger.kernel.org' and 'Test-by' tags for 03,04 since they are confirmed fix by Richard Leitner: https://lkml.org/lkml/2020/8/17/39 https://www.spinics.net/lists/dmaengine/msg23489.html 4. fix potential descriptor free unexpected on the next transfer before the last channel terminated: https://www.spinics.net/lists/dmaengine/msg23400.html Robin Gong (12): Revert "ARM: dts: imx6q: Use correct SDMA script for SPI5 core" Revert "ARM: dts: imx6: Use correct SDMA script for SPI cores" Revert "dmaengine: imx-sdma: refine to load context only once" dmaengine: imx-sdma: remove duplicated sdma_load_context dmaengine: dma: imx-sdma: add fw_loaded and is_ram_script dmaengine: imx-sdma: add mcu_2_ecspi script spi: imx: fix ERR009165 spi: imx: remove ERR009165 workaround on i.mx6ul dmaengine: imx-sdma: remove ERR009165 on i.mx6ul dma: imx-sdma: add i.mx6ul compatible name dmaengine: imx-sdma: add uart rom script dmaengine: imx-sdma: add terminated list for freed descriptor in worker .../devicetree/bindings/dma/fsl-imx-sdma.txt | 1 + arch/arm/boot/dts/imx6q.dtsi | 2 +- arch/arm/boot/dts/imx6qdl.dtsi | 8 +- drivers/dma/imx-sdma.c | 93 +++++++++++++++++----- drivers/spi/spi-imx.c | 52 ++++++++++-- include/linux/platform_data/dma-imx-sdma.h | 8 +- 6 files changed, 128 insertions(+), 36 deletions(-)