linux-kernel - Re: [PATCH 4/6] spi: axi-spi-engine: support SPI_MULTI_BUS_MODE

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: <39794baf-0a9f-465e-916d-6d5340e508de@baylibre.com>
Date: Wed, 15 Oct 2025 17:01:22 -0500
From: David Lechner <dlechner@...libre.com>
To: Marcelo Schmitt <marcelo.schmitt1@...il.com>
Cc: Mark Brown <broonie@...nel.org>, 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 4/6] spi: axi-spi-engine: support
 SPI_MULTI_BUS_MODE_STRIPE

On 10/15/25 3:53 PM, Marcelo Schmitt wrote:
> On 10/14, David Lechner wrote:
>> Add support for SPI_MULTI_BUS_MODE_STRIPE to the AXI SPI engine driver.
>>
>> The v2.0.0 version of the AXI SPI Engine IP core supports multiple
>> buses. This can be used with SPI_MULTI_BUS_MODE_STRIPE to support
>> reading from simultaneous sampling ADCs that have a separate SDO line
>> for each analog channel. This allows reading all channels at the same
>> time to increase throughput.
>>
>> Signed-off-by: David Lechner <dlechner@...libre.com>
>> ---
>>  drivers/spi/spi-axi-spi-engine.c | 128 +++++++++++++++++++++++++++++++++++++--
>>  1 file changed, 124 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/spi/spi-axi-spi-engine.c b/drivers/spi/spi-axi-spi-engine.c
>> index e06f412190fd243161a0b3df992f26157531f6a1..707e5108efec41f7eff608a09fcebd9d28fa2d70 100644
>> --- a/drivers/spi/spi-axi-spi-engine.c
>> +++ b/drivers/spi/spi-axi-spi-engine.c
>> @@ -23,6 +23,9 @@
>>  #include <linux/spi/spi.h>
>>  #include <trace/events/spi.h>
>>  
>> +#define SPI_ENGINE_REG_DATA_WIDTH		0x0C
>> +#define   SPI_ENGINE_REG_DATA_WIDTH_NUM_OF_SDIO_MASK	GENMASK(24, 16)
> would it be 8-bit mask?
> #define   SPI_ENGINE_REG_DATA_WIDTH_NUM_OF_SDIO_MASK   GENMASK(23, 16)

Ah, good catch.

> 
>> +#define   SPI_ENGINE_REG_DATA_WIDTH_MASK		GENMASK(15, 0)
>>  #define SPI_ENGINE_REG_OFFLOAD_MEM_ADDR_WIDTH	0x10
>>  #define SPI_ENGINE_REG_RESET			0x40
>>  
> ...
>>  
>> +	data_width_reg_val = readl(spi_engine->base + SPI_ENGINE_REG_DATA_WIDTH);
>> +
>>  	if (adi_axi_pcore_ver_gteq(version, 1, 1)) {
>>  		unsigned int sizes = readl(spi_engine->base +
>>  				SPI_ENGINE_REG_OFFLOAD_MEM_ADDR_WIDTH);
>> @@ -1097,6 +1214,9 @@ static int spi_engine_probe(struct platform_device *pdev)
>>  	}
>>  	if (adi_axi_pcore_ver_gteq(version, 1, 3))
>>  		host->mode_bits |= SPI_MOSI_IDLE_LOW | SPI_MOSI_IDLE_HIGH;
>> +	if (adi_axi_pcore_ver_gteq(version, 2, 0))
>> +		host->num_data_bus = FIELD_GET(SPI_ENGINE_REG_DATA_WIDTH_NUM_OF_SDIO_MASK,
>> +					       data_width_reg_val);
>>  
> Not sure I'm following the use of DATA_WIDTH and NUM_OF_SDIO.
> HDL doc [1] states NUM_OF_SDIO 'is equal with the maximum supported SDI lines in
> bits'. And the code sets that to be the number of buses. That should work for
> AD7380 because each AD7380 SDO bus has only one line. But, it won't support
> AD4630 (or even AD4030) because each AD4630 rx bus has 4 data lines. I can't
> find it in HDL, but I'd expect to also have something like NUM_OF_SDIO_PER_BUS.
> Or DATA_WIDTH is the number of lines per bus and HDL doc is unclear to me?

Right now, the HDL doesn't distinguish between the two, so we only have the
case where each "SDIO" is a separate bus. The AD4630 project has extra IP
blocks to unscramble things to simulate having 4 lines on each bus rather than
8 buses.

DATA_WIDTH has to do with how wide the bus between the SPI Engine and DMA
is, so it has nothing to do with the wiring to the peripheral.

> Well, it would be nice if we can have host->num_data_bus set in a way that
> minimizes diff when multiple lines per bus gets implemented (if that's not
> currently supported).

I agree it would be nice. However, the register name and meaning already exists
even in older versions of the IP block (as NUM_OF_SDI), so I think it would be
best to stick with the existing name. Ideally, when support for multiple wires
per bus is added, then we would compile like this: NUM_OF_SDIO=2 SDIO_BUS_WIDTH=4
rather than NUM_OF_SDIO=8 NUM_OF_SDIO_PER_BUS=4.

> 
> [1]: https://github.com/analogdevicesinc/hdl/pull/1808/files#diff-d1274cfe2e206aa66a0ecd3da04b3e62fc5fad9e12029b34b226c6f91454d34dR77

Well, the linked PR isn't merged yet, so I guess we could ask there to rename
NUM_OF_SDIO to NUM_OF_SDIO_BUS there if you think that is a better name since
it is being renamed anyway.