sun6i SPI driver issue

sun6i SPI driver issue

[Sergey Suloev]

Hi guys,

I have recently found a critical issue with sun6i SPI driver. It 
actually found it long time ago while using spidev but I didn't 
considered it as something important. Now it seems like a blocking issue 
that prevents using the driver in any serious application.
I have created a DRM driver for a ili9488-based display, its source is 
available here [1] and device tree overlay [2]. The driver has been 
successfully verified on RPi3, but? it is failing on sunxi: the driver 
gets loaded, display seems working but SPI transfers are periodically 
failing with error -110 (timeout). Due to the issue the picture is 
frozen and completely refreshed again from time to time.

The problem is reproduced on OrangePi PC and BananaPi M2. I am providing 
dmesg from my OrangePi PC [3].

Let know if you need any additional info.

Thanks

-------------------------------------------------------------------

[1] https://github.com/sergey-suloev/tinydrm/blob/master/ili9488.c

[2] 
https://github.com/sergey-suloev/tinydrm/blob/master/overlays/sunxi/mainline/sun8i-h3-ertft0356-drm.dts 


[3] https://pastebin.com/xYqWKcJ2

sun6i SPI driver issue

[Maxime Ripard]

On Mon, Mar 19, 2018 at 06:43:43PM +0300, Sergey Suloev wrote:
> I have recently found a critical issue with sun6i SPI driver. It actually
> found it long time ago while using spidev but I didn't considered it as
> something important. Now it seems like a blocking issue that prevents using
> the driver in any serious application.

So is it "critical" or "not important"?

> I have created a DRM driver for a ili9488-based display, its source is
> available here [1] and device tree overlay [2]. The driver has been
> successfully verified on RPi3, but? it is failing on sunxi: the driver gets
> loaded, display seems working but SPI transfers are periodically failing
> with error -110 (timeout). Due to the issue the picture is frozen and
> completely refreshed again from time to time.

On which kernel version was this tested?

Do you have any additional details on what exactly is happening?

Is it timeouting because you never received any interrupts? Is the
first part of your transfer (the one that would fit in the FIFO
actually done, and it's the second part that fails? Something else?

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180326/d6ef926f/attachment.sig>

sun6i SPI driver issue

[Maxime Ripard]

Hi,

Please don't reply in HTML.

On Mon, Mar 26, 2018 at 02:57:27PM +0300, Sergey Suloev wrote:
> On 03/26/2018 02:45 PM, Maxime Ripard wrote:
> 
>     On Mon, Mar 19, 2018 at 06:43:43PM +0300, Sergey Suloev wrote:
> 
>         I have recently found a critical issue with sun6i SPI driver. It actually
>         found it long time ago while using spidev but I didn't considered it as
>         something important. Now it seems like a blocking issue that prevents using
>         the driver in any serious application.
> 
>     So is it "critical" or "not important"?
> 
> 
>         I have created a DRM driver for a ili9488-based display, its source is
>         available here [1] and device tree overlay [2]. The driver has been
>         successfully verified on RPi3, but? it is failing on sunxi: the driver gets
>         loaded, display seems working but SPI transfers are periodically failing
>         with error -110 (timeout). Due to the issue the picture is frozen and
>         completely refreshed again from time to time.
> 
>     On which kernel version was this tested?
> 
>     Do you have any additional details on what exactly is happening?
> 
>     Is it timeouting because you never received any interrupts? Is the
>     first part of your transfer (the one that would fit in the FIFO
>     actually done, and it's the second part that fails? Something else?
>
> the issue is related to wrong PIO algorithm in sun6i SPI driver: the transfer
> length must be <= FIFO length in PIO mode, i.e. the current implementation is
> 100% broken.

Right, but you still don't give any detail on what "the issue" is
exactly, so it's kind of difficult to try to come up with a fix for
it.

> I put the restriction? above and tested with BPi M2 with ili9488
> slave - it works perfectly.

The current code has:
https://elixir.bootlin.com/linux/latest/source/drivers/spi/spi-sun6i.c#L195

Which seems to match what you're saying. So what did you change
exactly?

> I also applied DMA stuff offered? by Michal ([3] hramrach at gmail.com)
> and it works great too.

That patch has never been sumbitted upstream as far as I know, so
I don't really care about it.

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180327/3ff3ed71/attachment.sig>

sun6i spi driver issue

[Sergey Suloev]

Hi,

the following patch [1] isn't working on my BPi M2, OPi PC, OPi PC+. I 
have no more boards to test currently but I am sure it will fail on 
others too. As soon as the max transfer size is set to a value MORE THAN 
FIFO then most of transfers would end up with -110 (timeout).

[1] https://patchwork.kernel.org/patch/9634315/

I have reverted the patch back in my local kernel and it works well on 
all the boards mentioned. It works fine too without subtracting 1 byte 
from FIFO, i.e. one could use the code below:

static size_t sun6i_spi_max_transfer_size(struct spi_device *spi)
{
 ??????? struct sun6i_spi *sspi = spi_master_get_devdata(spi->master);

 ??????? return sspi->fifo_depth;
}

Thanks

sun6i spi driver issue

[Maxime Ripard]

On Sat, Mar 24, 2018 at 09:42:24AM +0300, Sergey Suloev wrote:
> the following patch [1] isn't working on my BPi M2, OPi PC, OPi PC+. I have
> no more boards to test currently but I am sure it will fail on others too.
> As soon as the max transfer size is set to a value MORE THAN FIFO then most
> of transfers would end up with -110 (timeout).
> 
> [1] https://patchwork.kernel.org/patch/9634315/

Is that related to your other discussion? Why did you drop most of the
recipients from that thread, including the SPI maintainer and the SPI
ML?

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180327/15a6ee24/attachment.sig>