Message-ID: <66f94eb6-15a9-457f-a7b8-47710652a34b@baylibre.com>
Date: Thu, 16 Oct 2025 10:25:41 -0500
From: David Lechner <dlechner@...libre.com>
To: Nuno Sá <noname.nuno@...il.com>,
 Mark Brown <broonie@...nel.org>
Cc: Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>,
 Marcelo Schmitt <marcelo.schmitt@...log.com>,
 Michael Hennerich <michael.hennerich@...log.com>,
 Nuno Sá <nuno.sa@...log.com>,
 Jonathan Cameron <jic23@...nel.org>, Andy Shevchenko <andy@...nel.org>,
 Sean Anderson <sean.anderson@...ux.dev>, linux-spi@...r.kernel.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-iio@...r.kernel.org
Subject: Re: [PATCH 3/6] spi: add multi_bus_mode field to struct spi_transfer

On 10/16/25 4:08 AM, Nuno Sá wrote:
> On Wed, 2025-10-15 at 13:38 -0500, David Lechner wrote:
>> On 10/15/25 11:43 AM, Nuno Sá wrote:
>>> On Wed, 2025-10-15 at 11:15 -0500, David Lechner wrote:
>>>> On 10/15/25 10:18 AM, Mark Brown wrote:
>>>>> On Wed, Oct 15, 2025 at 03:43:09PM +0100, Nuno Sá wrote:
>>>>>> On Wed, 2025-10-15 at 13:01 +0100, Mark Brown wrote:
>>>>>>> On Wed, Oct 15, 2025 at 11:16:01AM +0100, Nuno Sá wrote:
>>>>>>>> On Tue, 2025-10-14 at 17:02 -0500, David Lechner wrote:

...

>>
>> The AXI SPI Engine doesn't know how to do the quad SPI part yet though, so
>> it isn't something we could implement right now.
>>
>> If we tried to do it with spi-buses = <8>; then we would end up with the
>> "interleaved" bits (or nibbles depending on the wiring) that requires the
>> extra IP block to sort out when using SPI offloading. Technically, we could
> 
> I think that extra block already exists today. I was thinking the idea was just:
> 
> // the case where we just have one channel with eg: 32 bits words (eg: test
> patterns) 
> struct spi_transfer example = {
> 	rx_buf = rx_buf;
> 	len = 1; /* 1 32bit words */

This would still need to be len = 4; since there are 4 bytes in a
32-bit word. (If this was tx with SPI_MULTI_BUS_MODE_MIRROR, then
len = 1 would be correct, but for striping, it is still the length
of all data combined).

> 	/* 4 lanes which is actually quadspi */
> 	multi_bus_mode = SPI_MULTI_BUS_MODE_STRIPE; 
> };

This will work with the caveat that for non-offload case, the software 
will need to rearrange the bits in rx_buf into the correct order after
the spi_sync().

For example, u8 *rx_buf will contain bits of the 32-bit word in the
following order:

rx_buf[0] = b28 b24 b20 b16 b12 b8 b4 b0
rx_buf[1] = b29 b25 b21 b17 b13 b9 b5 b1
rx_buf[2] = b30 b26 b22 b18 b14 b10 b6 b2
rx_buf[3] = b31 b27 b23 b19 b15 b11 b7 b3

The correct order of course would be (assuming little endian):


rx_buf[0] = b7 b6 b5 b4 b3 b2 b1 b0
...

And for the offload case, it would require an extra IP block between
SPI and DMA to rearrange the bits in hardware. So to "do it right", the
driver should be checking for a hardware description that such an IP
block exists in the pipline before creating such a SPI xfer.

> 
> I still did not looked at how the stripe mode is implemented in the hdl IP but
> maybe the above would work as we get 8 bits per lane and we do have the data
> reorder IP (or at least used to have) after the offload engine. 
> 
> That said, I do see now the above is not the intended usecase for this series
> and even if it works we kind of have to hack the xfer len to 1 which does not
> reflect reality.
> 
>> make it work, but it would require a bunch of extra hardware description that
>> the driver would have to interpret in order to correctly format the struct
>> spi_transfer. I was hoping we could avoid that and just teach the SPI Engine
>> how to do dual/quad SPI like other SPI controllers.
> 
> Agreed!
> 
> - Nuno Sá
> 

Powered by blists - more mailing lists