RE: [PATCH v13 00/12] add ecspi ERR009165 for i.mx6/7 soc family

[Robin Gong <yibin.gong@xxxxxxx>]

On 2020/09/12 0:40 Marco Felsch <m.felsch@xxxxxxxxxxxxxx> 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&amp;sdata=ru8fKz6wpDhzYeaHIT28T0OybHlCFHJ41N1lJYuqKgE%3D&
> amp;reserved=0
> 
> Regards,
>   Marco