RE: [PACTH v2] spi: imx: add 16/32 bits per word support for target mode

Carlos Song
2025-10-28T09:29:17Z

> -----Original Message-----
> From: Frank Li <frank.li@nxp.com>
> Sent: Tuesday, October 28, 2025 2:50 AM
> To: Carlos Song <carlos.song@nxp.com>
> Cc: mkl@pengutronix.de; broonie@kernel.org; shawnguo@kernel.org;
> s.hauer@pengutronix.de; kernel@pengutronix.de; festevam@gmail.com;
> linux-spi@vger.kernel.org; imx@lists.linux.dev;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: Re: [PACTH v2] spi: imx: add 16/32 bits per word support for target
> mode
>
> On Mon, Oct 27, 2025 at 03:39:36AM +0000, Carlos Song wrote:
> >
> >
> > > -----Original Message-----
> > > From: Frank Li <frank.li@nxp.com>
> > > Sent: Friday, October 24, 2025 9:25 PM
> > > To: Carlos Song <carlos.song@nxp.com>
> > > Cc: mkl@pengutronix.de; broonie@kernel.org; shawnguo@kernel.org;
> > > s.hauer@pengutronix.de; kernel@pengutronix.de; festevam@gmail.com;
> > > linux-spi@vger.kernel.org; imx@lists.linux.dev;
> > > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> > > Subject: Re: [PACTH v2] spi: imx: add 16/32 bits per word support
> > > for target mode
> > >
> > > On Fri, Oct 24, 2025 at 07:31:07PM +0800, carlos.song@nxp.com wrote:
> > > > From: Carlos Song <carlos.song@nxp.com>
> > > >
> > > > Now for ECSPI only support 8 bits per word in target mode.
> > > > Enable 16/32 bits per word support for spi-imx target mode.
> > > >
> > > > Signed-off-by: Carlos Song <carlos.song@nxp.com>
> > > > Signed-off-by: Clark Wang <xiaoning.wang@nxp.com>
> > > > ---
> > > > drivers/spi/spi-imx.c | 12 ++++++++++--
> > > > 1 file changed, 10 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c index
> > > > 155ddeb8fcd4..017f83f5dfdf 100644
> > > > --- a/drivers/spi/spi-imx.c
> > > > +++ b/drivers/spi/spi-imx.c
> > > > @@ -424,8 +424,12 @@ static void spi_imx_buf_tx_swap(struct
> > > > spi_imx_data *spi_imx)
> > > >
> > > > static void mx53_ecspi_rx_target(struct spi_imx_data *spi_imx) {
> > > > - u32 val = ioread32be(spi_imx->base + MXC_CSPIRXDATA);
> > > > + u32 val = readl(spi_imx->base + MXC_CSPIRXDATA);
> > > >
> > > > + if (spi_imx->bits_per_word <= 8)
> > > > + swab32s(&val);
> > > > + else if (spi_imx->bits_per_word <= 16)
> > > > + swahw32s(&val);
> > >
> > > Needn't swap when bits_per_word > 24, like 32? or our hardware max
> > > support to 16?
> > >
> > > Frank
> >
> > I think we don't need. We are reading/writing FIFO by 32-bits word. In
> > this case, we just need keep the default byte order.
> >
> > Is it more reasonable only support bits per word=8/16/32 for target mode?
> > -b 8/16/32 can cover most use case.
>
> yes, I only can't understand why needn't swap at 32bits workds, but other
> needs.
>
> FIFO 31..24 23..16 15..8 7..0
> B0 B1 B2 B3
>
> next you
>
> in memory val is
>
> 0x0: B3
> 0x1: B2
> 0x2: B1
> 0x3: B0
>
> swab32s() to bits_per_work 8
>
> 0x0: B0
> 0x1: B1
> 0x2: B2
> 0x3: B3
>
> if bits per_word 16
>
> val
>
> 0x0: X
> 0x1: X
> 0x2: B1
> 0x3: B0
>
> after swahw32s change to
>
> 0x0: B1
> 0x1: B0
> 0x2: x
> 0x3: x
>
> B0 and B1 still be swapped.
>
> Does SPI defined term word fixed big-endian?
>
> Frank
>

No, SPI doesn't define this. This swap is caused by the SPI-IMX FIFO design.
SPI-IMX FIFO is fixed little-endian.

One example to easy understand:

If buf is:

0XB0B1B2B3

So data in TXFIFO is:
FIFO 31..24 23..16 15..8 7..0
B0 B1 B2 B3

So we can always see data on SPI bus:
0xB3B2B1B0

For this patch:
bits per word= 8: it means one byte is one word, so it should not effect by big endian or little endian, so every bytes should keep the same order between memory and SPI bus.
We write FIFO by 32 bits word, So we should swap every bytes in this word from 0XB0B1B2B3 to 0XB3B2B1B0

Data in TXFIFO is
FIFO 31..24 23..16 15..8 7..0
B3 B2 B1 B0
Then we can see data on SPI bus is
0XB0B1B2B3

For bits per word = 16, it means two bytes in one word, so every 2 bytes is one word, the word should be a little endian word,

so we should swap half word from
0XB0B1B2B3 to 0XB2B3B0B1

Then data in TXFIFO is:
FIFO 31..24 23..16 15..8 7..0
B2 B3 B0 B1
Then we can see data on SPI bus is
0XB1B0B3B2

For bis per word =32, it means for bytes in one word, so every 4 bytes is one word. The whole word is a little endian.
So we keep the default order.
0XB0B1B2B3

FIFO 31..24 23..16 15..8 7..0
B0 B1 B2 B3

So we can see the data on bus is:
0xB3B2B1B0

The RX handle is the same with TX. From bus to FIFO is a little word, so swap in memory.

Carlos

> > Carlos
> > > > if (spi_imx->rx_buf) {
> > > > int n_bytes = spi_imx->target_burst % sizeof(val);
> > > >
> > > > @@ -453,12 +457,16 @@ static void mx53_ecspi_tx_target(struct
> > > spi_imx_data *spi_imx)
> > > > if (spi_imx->tx_buf) {
> > > > memcpy(((u8 *)&val) + sizeof(val) - n_bytes,
> > > > spi_imx->tx_buf, n_bytes);
> > > > + if (spi_imx->bits_per_word <= 8)
> > > > + swab32s(&val);
> > > > + else if (spi_imx->bits_per_word <= 16)
> > > > + swahw32s(&val);
> > > > spi_imx->tx_buf += n_bytes;
> > > > }
> > > >
> > > > spi_imx->count -= n_bytes;
> > > >
> > > > - iowrite32be(val, spi_imx->base + MXC_CSPITXDATA);
> > > > + writel(val, spi_imx->base + MXC_CSPITXDATA);
> > > > }
> > > >
> > > > /* MX51 eCSPI */
> > > > --
> > > > 2.34.1
> > > >