From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pg0-f66.google.com ([74.125.83.66]:35404 "EHLO mail-pg0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754349AbdIGJMR (ORCPT ); Thu, 7 Sep 2017 05:12:17 -0400 MIME-Version: 1.0 In-Reply-To: References: <20170906070507.26223-1-dirk.behme@de.bosch.com> <20170906070507.26223-3-dirk.behme@de.bosch.com> <931484ec-8830-0e63-98cb-9dd58ce35230@de.bosch.com> <09ce4c7a-347c-7df8-d8ed-4eb61a00bd20@de.bosch.com> From: Geert Uytterhoeven Date: Thu, 7 Sep 2017 11:12:16 +0200 Message-ID: Subject: Re: [PATCH 2/8] spi: sh-msiof: Fix DMA transfer size check To: Dirk Behme Cc: Linux-Renesas , linux-spi , Geert Uytterhoeven , Hiromitsu Yamasaki Content-Type: text/plain; charset="UTF-8" Sender: linux-renesas-soc-owner@vger.kernel.org List-ID: Hi Dirk, On Thu, Sep 7, 2017 at 11:05 AM, Dirk Behme wrote: > On 07.09.2017 10:59, Geert Uytterhoeven wrote: >> On Thu, Sep 7, 2017 at 10:42 AM, Dirk Behme >> wrote: >>> On 07.09.2017 10:39, Geert Uytterhoeven wrote: >>>> On Thu, Sep 7, 2017 at 10:33 AM, Dirk Behme >>>> wrote: >>>>> On 07.09.2017 10:31, Geert Uytterhoeven wrote: >>>>>> On Wed, Sep 6, 2017 at 9:05 AM, Dirk Behme >>>>>> wrote: >>>>>>> From: Hiromitsu Yamasaki >>>>>>> DMA supports 32-bit words only, >>>>>>> even if BITLEN1 of SITMDR2 register is 16bit. >>>>>>> >>>>>>> Signed-off-by: Hiromitsu Yamasaki >>>>>>> Signed-off-by: Dirk Behme >>>>>>> --- >>>>>>> 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 >>>>> >>>>> 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@linux-m68k.org 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 From mboxrd@z Thu Jan 1 00:00:00 1970 From: Geert Uytterhoeven Subject: Re: [PATCH 2/8] spi: sh-msiof: Fix DMA transfer size check Date: Thu, 7 Sep 2017 11:12:16 +0200 Message-ID: References: <20170906070507.26223-1-dirk.behme@de.bosch.com> <20170906070507.26223-3-dirk.behme@de.bosch.com> <931484ec-8830-0e63-98cb-9dd58ce35230@de.bosch.com> <09ce4c7a-347c-7df8-d8ed-4eb61a00bd20@de.bosch.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Cc: Linux-Renesas , linux-spi , Geert Uytterhoeven , Hiromitsu Yamasaki To: Dirk Behme Return-path: In-Reply-To: Sender: linux-spi-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-ID: Hi Dirk, On Thu, Sep 7, 2017 at 11:05 AM, Dirk Behme wrote: > On 07.09.2017 10:59, Geert Uytterhoeven wrote: >> On Thu, Sep 7, 2017 at 10:42 AM, Dirk Behme >> wrote: >>> On 07.09.2017 10:39, Geert Uytterhoeven wrote: >>>> On Thu, Sep 7, 2017 at 10:33 AM, Dirk Behme >>>> wrote: >>>>> On 07.09.2017 10:31, Geert Uytterhoeven wrote: >>>>>> On Wed, Sep 6, 2017 at 9:05 AM, Dirk Behme >>>>>> wrote: >>>>>>> From: Hiromitsu Yamasaki >>>>>>> DMA supports 32-bit words only, >>>>>>> even if BITLEN1 of SITMDR2 register is 16bit. >>>>>>> >>>>>>> Signed-off-by: Hiromitsu Yamasaki >>>>>>> Signed-off-by: Dirk Behme >>>>>>> --- >>>>>>> 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 >>>>> >>>>> 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-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org 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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html