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

[Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>]

Hi Dirk,

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:
>>>> On Thu, Sep 7, 2017 at 10:33 AM, Dirk Behme <dirk.behme@xxxxxxxxxxxx>
>>>> wrote:
>>>>> On 07.09.2017 10:31, Geert Uytterhoeven wrote:
>>>>>> On Wed, Sep 6, 2017 at 9:05 AM, Dirk Behme <dirk.behme@xxxxxxxxxxxx>
>>>>>> wrote:
>>>>>>> From: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@xxxxxxxxxxx>
>>>>>>> DMA supports 32-bit words only,
>>>>>>> even if BITLEN1 of SITMDR2 register is 16bit.
>>>>>>>
>>>>>>> Signed-off-by: Hiromitsu Yamasaki <hiromitsu.yamasaki.ym@xxxxxxxxxxx>
>>>>>>> Signed-off-by: Dirk Behme <dirk.behme@xxxxxxxxxxxx>
>>>>>>> ---
>>>>>>>     drivers/spi/spi-sh-msiof.c | 2 +-
>>>>>>>     1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
>>>>>>> index 2b4d3a520176..f9300fdf41e5 100644
>>>>>>> --- a/drivers/spi/spi-sh-msiof.c
>>>>>>> +++ b/drivers/spi/spi-sh-msiof.c
>>>>>>> @@ -904,7 +904,7 @@ static int sh_msiof_transfer_one(struct
>>>>>>> spi_master
>>>>>>> *master,
>>>>>>>                                    break;
>>>>>>>                            copy32 = copy_bswap32;
>>>>>>>                    } else if (bits <= 16) {
>>>>>>> -                       if (l & 1)
>>>>>>> +                       if (l & 3)
>>>>>>>                                    break;
>>>>>>>                            copy32 = copy_wswap32;
>>>>>>>                    } else {
>>>>>>
>>>>>> Looks OK, as l is in bytes, not 16-bit words.
>>>>>>
>>>>>> Reviewed-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
>>>>>
>>>>> What do you think about CCing this to -stable?
>>>>
>>>> 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...

Note that this test does not exercise the code path affected by this particular
patch ;-)

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