| Message ID | 20240201105451.507005-1-carlos.song@nxp.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v4] spi: imx: fix the burst length at DMA mode and CPU mode | expand |
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
> -----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
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; } }