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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 03.01.2018 18:25, Geert Uytterhoeven wrote:
Hi Dirk,

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:
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...

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.

I apologize already now in case testing will take some time ;)

Anyhow, many thanks again for looking into this.

Best regards

Dirk









[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux