linux-kernel - Re: [PATCH 3/6] spi: add multi_bus_mode field to struct spi

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c4b5a42f5f1d3f577cb986946b642b4edc1300e9.camel@gmail.com>
Date: Thu, 16 Oct 2025 10:08:53 +0100
From: Nuno Sá <noname.nuno@...il.com>
To: David Lechner <dlechner@...libre.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 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:
> > > > 
> > > > > > > >         controller    < data bits <     peripheral
> > > > > > > >         ----------   ----------------   ----------
> > > > > > > >             SDI 0    0-0-0-1-0-0-0-1    SDO 0
> > > > > > > >             SDI 1    1-0-0-0-1-0-0-0    SDO 1
> > > > 
> > > > > > > Out of curiosity, how does this work for devices like AD4030 where
> > > > > > > the same
> > > > > > > word
> > > 
> > > The AD4030 is just one channel, so doesn't do interleaving. But you
> > > probably
> > > meant AD4630 when it is wired up with only 1 SDO line. That line has to be
> > > shared
> > > by both of the simultaneous converters so it alternates between sending
> > > one bit
> > > from each word. This patch series doesn't address that case. But this
> > > series will
> > > work for the AD4630 when it has 2 SDO lines wired up.
> > > 
> > 
> > Hmm I didn't even remembered that one. But what I meant with interleaved was
> > having
> > the same data word spread through multiple SDO lines (one bit per line)
> > which is what
> > (also) happens with the devices I mentioned. And since you mentioned "...two
> > different data words at the same time, one on each bus...", I raised the
> > question.
> 
> Ah, yes, I know what you are talking about now. I didn't mention that use case
> in
> the cover letter because I didn't want to confuse things. But actually the
> AD4630
> can have 8 SDO lines, 4 per each data bus/ADC channel. The groups of 4 act
> like a
> quad SPI where 4 bits of one data word are sent at the same time. Those 4
> lines are
> considered one "bus" since they are all connected to the same serialzer that
> combines
> the bits into a single word. We already have support for this sort of thing in
> Linux.
> And sure, we could mix the two together. So a SPI transfer might look like:
> 
> struct spi_transfer example = {
> 	rx_buf = rx_buf;
> 	len = 4; /* 2 x 16-bit words */
> 	rx_nbits = 4; /* each bus is quad SPI */
> 	multi_bus_mode = SPI_MULTI_BUS_MODE_STRIPE; /* 2 data buses */
> 	bits_per_word = 16;
> };
> 
> This would result in a transfer that reads two 16-bit words in 4 SCLK cycles.
> 
> And the .dts would look like:
> 
> spi {
> 	adc@0 {
> 		compatible = "adi,ad4630-16";
> 		reg = <0>;
> 		...
> 		spi-rx-bus-width = <4>;
> 		spi-buses = <2>;
> 		...
> 	};
> };

Yes, it makes sense! I guess the above is what Mark meant in the first place.

> 
> 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 */
	/* 4 lanes which is actually quadspi */
	multi_bus_mode = SPI_MULTI_BUS_MODE_STRIPE; 
};

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á

> > > > >