Re: [PATCH 2/8] spi: sh-msiof: Fix DMA transfer size check

[Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>]

Hi Dirk,

On Thu, Jan 4, 2018 at 8:19 AM, Dirk Behme <dirk.behme@xxxxxxxxxxxx> wrote:
> On 03.01.2018 18:25, Geert Uytterhoeven wrote:
>> On Thu, Sep 7, 2017 at 11:12 AM, Geert Uytterhoeven
>> <geert@xxxxxxxxxxxxxx> wrote:
>>> On Thu, Sep 7, 2017 at 11:05 AM, Dirk Behme <dirk.behme@xxxxxxxxxxxx>
>>> wrote:
>>>> On 07.09.2017 10:59, Geert Uytterhoeven wrote:
>>>>> On Thu, Sep 7, 2017 at 10:42 AM, Dirk Behme <dirk.behme@xxxxxxxxxxxx>
>>>>> wrote:
>>>>>> On 07.09.2017 10:39, Geert Uytterhoeven wrote:
>>>>>>> Sounds OK. Have you tested this?
>>>>>>
>>>>>> I tested all 8 patches together if they fix the issues I observed with
>>>>>> plain
>>>>>> mainline.
>>>>>
>>>>> Could you please elaborate about the issues you observed with plain
>>>>> mainline?
>>>>> It may help to identify which patches are responsible for fixing them.
>>>>
>>>> I've been told that an easy test case is to just cat random data (any
>>>> mid
>>>> sized file) into SPI. E.g.:
>>>>
>>>> # cat /proc/cpuinfo > /dev/spidev32764.2
>>>> [  504.544948] spi_sh_msiof e6e90000.spi: failed to shut down hardware
>>>> [  504.551265] spidev spi32764.2: SPI transfer failed: -110
>>>> [  504.556625] spi_master spi32764: failed to transfer one message from
>>>> queue
>>>> [  504.564857] spi_sh_msiof e6e90000.spi: failed to shut down hardware
>>>> [  504.571177] spidev spi32764.2: SPI transfer failed: -110
>>>> [  504.576528] spi_master spi32764: failed to transfer one message from
>>>> queue
>>>> cat: write error: Connection timed out
>>>>
>>>> done on plain 4.13-rc6.
>>>
>>> Thank you, that's very valuable information!
>>> We'll give it a try...
>>
>> After some investigation, this is caused by DMA completion triggering
>> too early for TX-only transfers.  This should indeed be fixed by "[PATCH
>> 5/8]
>> spi: sh-msiof: Wait for Tx FIFO empty after DMA" you extracted from the
>> BSP.
>>
>> As that patch needs changes to apply (a) on current spi/for-next and (b)
>> without other patches from your series, I reworked it into "[PATCH] spi:
>> sh-msiof: Fix timeout failures for TX-only DMA transfers".
>>
>> As for the other patches from your series:
>>    - Upstream already has commit 36735783fdb599c9 ("spi: sh-msiof: Fix DMA
>>      transfer size check"),
>>    - spi/for-next has commit b8761434bdec32fa ("spi: sh-msiof: Implement
>>      cs-gpios configuration").
>> The remaining were either rejected, retracted, or it is unclear which
>> problem
>> (if any) they are really fixing.
>
> First, many thanks for looking into this!
>
>> Do you agree?
>
> If there are no more issues, then I agree ;) So lets rephrase that a little:
> We'll test what we have, now. And in case we find any remaining issues,
> we'll come back with more details, then. What will help us to look at the
> remaining patches.

Sure, if more issues can be identified, we should look into fixing them as
well.

Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html