* [PATCH v4] spi: imx: fix the burst length at DMA mode and CPU mode
@ 2024-02-01 10:54 carlos.song
2024-02-01 20:14 ` Benjamin Bigler
0 siblings, 1 reply; 3+ messages in thread
From: carlos.song @ 2024-02-01 10:54 UTC (permalink / raw)
To: broonie, shawnguo, s.hauer, kernel, linux-imx, benjamin, stefanmoring
Cc: linux-spi, linux-arm-kernel, linux-kernel
From: Carlos Song <carlos.song@nxp.com>
For DMA mode, the bus width of the DMA is equal to the size of data
word, so burst length should be configured as bits per word.
For CPU mode, because of the spi transfer len is in byte, so burst
length should be configured as bits per byte * spi_imx->count.
Signed-off-by: Carlos Song <carlos.song@nxp.com>
Reviewed-by: Clark Wang <xiaoning.wang@nxp.com>
Fixes: e9b220aeacf1 ("spi: spi-imx: correctly configure burst length when using dma")
Fixes: 5f66db08cbd3 ("spi: imx: Take in account bits per word instead of assuming 8-bits")
---
Changes for V3:
- include <linux/bits.h>
Changes for V4:
- keep the includes sorted alphabetically.
---
drivers/spi/spi-imx.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
index 546cdce525fc..f7990ac2c654 100644
--- a/drivers/spi/spi-imx.c
+++ b/drivers/spi/spi-imx.c
@@ -2,6 +2,7 @@
// Copyright 2004-2007 Freescale Semiconductor, Inc. All Rights Reserved.
// Copyright (C) 2008 Juergen Beisert
+#include <linux/bits.h>
#include <linux/clk.h>
#include <linux/completion.h>
#include <linux/delay.h>
@@ -660,15 +661,14 @@ static int mx51_ecspi_prepare_transfer(struct spi_imx_data *spi_imx,
<< MX51_ECSPI_CTRL_BL_OFFSET;
else {
if (spi_imx->usedma) {
- ctrl |= (spi_imx->bits_per_word *
- spi_imx_bytes_per_word(spi_imx->bits_per_word) - 1)
+ ctrl |= (spi_imx->bits_per_word - 1)
<< MX51_ECSPI_CTRL_BL_OFFSET;
} else {
if (spi_imx->count >= MX51_ECSPI_CTRL_MAX_BURST)
- ctrl |= (MX51_ECSPI_CTRL_MAX_BURST - 1)
+ ctrl |= (MX51_ECSPI_CTRL_MAX_BURST * BITS_PER_BYTE - 1)
<< MX51_ECSPI_CTRL_BL_OFFSET;
else
- ctrl |= (spi_imx->count * spi_imx->bits_per_word - 1)
+ ctrl |= (spi_imx->count * BITS_PER_BYTE - 1)
<< MX51_ECSPI_CTRL_BL_OFFSET;
}
}
--
2.34.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v4] spi: imx: fix the burst length at DMA mode and CPU mode
2024-02-01 10:54 [PATCH v4] spi: imx: fix the burst length at DMA mode and CPU mode carlos.song
@ 2024-02-01 20:14 ` Benjamin Bigler
2024-02-04 9:12 ` [EXT] " Carlos Song
0 siblings, 1 reply; 3+ messages in thread
From: Benjamin Bigler @ 2024-02-01 20:14 UTC (permalink / raw)
To: carlos.song, broonie, shawnguo, s.hauer, kernel, linux-imx, stefanmoring
Cc: linux-spi, linux-arm-kernel, linux-kernel
On Thu, 2024-02-01 at 18:54 +0800, carlos.song@nxp.com wrote:
> From: Carlos Song <carlos.song@nxp.com>
>
> For DMA mode, the bus width of the DMA is equal to the size of data
> word, so burst length should be configured as bits per word.
>
> For CPU mode, because of the spi transfer len is in byte, so burst
> length should be configured as bits per byte * spi_imx->count.
>
> Signed-off-by: Carlos Song <carlos.song@nxp.com>
> Reviewed-by: Clark Wang <xiaoning.wang@nxp.com>
> Fixes: e9b220aeacf1 ("spi: spi-imx: correctly configure burst length when using dma")
> Fixes: 5f66db08cbd3 ("spi: imx: Take in account bits per word instead of assuming 8-bits")
> ---
> Changes for V3:
> - include <linux/bits.h>
> Changes for V4:
> - keep the includes sorted alphabetically.
> ---
> drivers/spi/spi-imx.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
> index 546cdce525fc..f7990ac2c654 100644
> --- a/drivers/spi/spi-imx.c
> +++ b/drivers/spi/spi-imx.c
> @@ -2,6 +2,7 @@
> // Copyright 2004-2007 Freescale Semiconductor, Inc. All Rights Reserved.
> // Copyright (C) 2008 Juergen Beisert
>
> +#include <linux/bits.h>
> #include <linux/clk.h>
> #include <linux/completion.h>
> #include <linux/delay.h>
> @@ -660,15 +661,14 @@ static int mx51_ecspi_prepare_transfer(struct spi_imx_data *spi_imx,
> << MX51_ECSPI_CTRL_BL_OFFSET;
> else {
> if (spi_imx->usedma) {
> - ctrl |= (spi_imx->bits_per_word *
> - spi_imx_bytes_per_word(spi_imx->bits_per_word) - 1)
> + ctrl |= (spi_imx->bits_per_word - 1)
> << MX51_ECSPI_CTRL_BL_OFFSET;
> } else {
> if (spi_imx->count >= MX51_ECSPI_CTRL_MAX_BURST)
> - ctrl |= (MX51_ECSPI_CTRL_MAX_BURST - 1)
> + ctrl |= (MX51_ECSPI_CTRL_MAX_BURST * BITS_PER_BYTE - 1)
> << MX51_ECSPI_CTRL_BL_OFFSET;
> else
> - ctrl |= (spi_imx->count * spi_imx->bits_per_word - 1)
> + ctrl |= (spi_imx->count * BITS_PER_BYTE - 1)
I think that will not work for drivers which dont use bits_per_word=8.
https://lore.kernel.org/all/20230917164037.29284-1-stefanmoring@gmail.com/
> << MX51_ECSPI_CTRL_BL_OFFSET;
> }
> }
Best regards,
Benjamin Bigler
^ permalink raw reply [flat|nested] 3+ messages in thread
* RE: [EXT] Re: [PATCH v4] spi: imx: fix the burst length at DMA mode and CPU mode
2024-02-01 20:14 ` Benjamin Bigler
@ 2024-02-04 9:12 ` Carlos Song
0 siblings, 0 replies; 3+ messages in thread
From: Carlos Song @ 2024-02-04 9:12 UTC (permalink / raw)
To: Benjamin Bigler, broonie, shawnguo, s.hauer, kernel,
dl-linux-imx, stefanmoring
Cc: linux-spi, linux-arm-kernel, linux-kernel
> -----Original Message-----
> From: Benjamin Bigler <benjamin@bigler.one>
> Sent: Friday, February 2, 2024 4:15 AM
> To: Carlos Song <carlos.song@nxp.com>; broonie@kernel.org;
> shawnguo@kernel.org; s.hauer@pengutronix.de; kernel@pengutronix.de;
> dl-linux-imx <linux-imx@nxp.com>; stefanmoring@gmail.com
> Cc: linux-spi@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> linux-kernel@vger.kernel.org
> Subject: [EXT] Re: [PATCH v4] spi: imx: fix the burst length at DMA mode and
> CPU mode
>
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report this
> email' button
>
>
> On Thu, 2024-02-01 at 18:54 +0800, carlos.song@nxp.com wrote:
> > From: Carlos Song <carlos.song@nxp.com>
> >
> > For DMA mode, the bus width of the DMA is equal to the size of data
> > word, so burst length should be configured as bits per word.
> >
> > For CPU mode, because of the spi transfer len is in byte, so burst
> > length should be configured as bits per byte * spi_imx->count.
> >
> > Signed-off-by: Carlos Song <carlos.song@nxp.com>
> > Reviewed-by: Clark Wang <xiaoning.wang@nxp.com>
> > Fixes: e9b220aeacf1 ("spi: spi-imx: correctly configure burst length
> > when using dma")
> > Fixes: 5f66db08cbd3 ("spi: imx: Take in account bits per word instead
> > of assuming 8-bits")
> > ---
> > Changes for V3:
> > - include <linux/bits.h>
> > Changes for V4:
> > - keep the includes sorted alphabetically.
> > ---
> > drivers/spi/spi-imx.c | 8 ++++----
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c index
> > 546cdce525fc..f7990ac2c654 100644
> > --- a/drivers/spi/spi-imx.c
> > +++ b/drivers/spi/spi-imx.c
> > @@ -2,6 +2,7 @@
> > // Copyright 2004-2007 Freescale Semiconductor, Inc. All Rights Reserved.
> > // Copyright (C) 2008 Juergen Beisert
> >
> > +#include <linux/bits.h>
> > #include <linux/clk.h>
> > #include <linux/completion.h>
> > #include <linux/delay.h>
> > @@ -660,15 +661,14 @@ static int mx51_ecspi_prepare_transfer(struct
> spi_imx_data *spi_imx,
> > << MX51_ECSPI_CTRL_BL_OFFSET;
> > else {
> > if (spi_imx->usedma) {
> > - ctrl |= (spi_imx->bits_per_word *
> > -
> spi_imx_bytes_per_word(spi_imx->bits_per_word) - 1)
> > + ctrl |= (spi_imx->bits_per_word - 1)
> > << MX51_ECSPI_CTRL_BL_OFFSET;
> > } else {
> > if (spi_imx->count >=
> MX51_ECSPI_CTRL_MAX_BURST)
> > - ctrl |= (MX51_ECSPI_CTRL_MAX_BURST -
> 1)
> > + ctrl |= (MX51_ECSPI_CTRL_MAX_BURST *
> > + BITS_PER_BYTE - 1)
> > <<
> MX51_ECSPI_CTRL_BL_OFFSET;
> > else
> > - ctrl |= (spi_imx->count *
> spi_imx->bits_per_word - 1)
> > + ctrl |= (spi_imx->count * BITS_PER_BYTE
> > + - 1)
Hi,
Thank you! You re right. I have get the patch history that spi->bits_per_word = 9 will
break the driver, this is a special case.
So burst length should be calculated by the number of words, instead of the number of bytes, otherwise the transmission of these non-byte-aligned word will break the driver.
the burst length can be set by:
spi_imx->count/DIV_ROUND_UP(spi_imx->bits_per_word, BITS_PER_BYTE) * spi_imx->bits_per_word.
I will set V5 later.
> I think that will not work for drivers which dont use bits_per_word=8.
> https://lore.ker/
> nel.org%2Fall%2F20230917164037.29284-1-stefanmoring%40gmail.com%2F&
> data=05%7C02%7Ccarlos.song%40nxp.com%7C6f52557285a34b97c6f508dc2
> 3626f51%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638424152
> 883113184%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoi
> V2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=2RerZ6
> 2kHdgiwqi5yxnDPLwWViplttHA2E05WFZmTuw%3D&reserved=0
> > <<
> MX51_ECSPI_CTRL_BL_OFFSET;
> > }
> > }
>
> Best regards,
> Benjamin Bigler
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-02-04 9:12 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-01 10:54 [PATCH v4] spi: imx: fix the burst length at DMA mode and CPU mode carlos.song
2024-02-01 20:14 ` Benjamin Bigler
2024-02-04 9:12 ` [EXT] " Carlos Song
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).