[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <3180475bd51e1e057d6aa7e1b62f564cb57a117e.camel@gmail.com>
Date: Wed, 15 Oct 2025 11:30:39 +0100
From: Nuno Sá <noname.nuno@...il.com>
To: David Lechner <dlechner@...libre.com>, 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>
Cc: 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 Tue, 2025-10-14 at 17:02 -0500, 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..707e5108efec41f7eff608a09fcebd9d28fa
> 2d70 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)
> +#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
>
> @@ -75,6 +78,8 @@
> #define SPI_ENGINE_CMD_REG_CLK_DIV 0x0
> #define SPI_ENGINE_CMD_REG_CONFIG 0x1
> #define SPI_ENGINE_CMD_REG_XFER_BITS 0x2
> +#define SPI_ENGINE_CMD_REG_SDI_MASK 0x3
> +#define SPI_ENGINE_CMD_REG_SDO_MASK 0x4
>
> #define SPI_ENGINE_MISC_SYNC 0x0
> #define SPI_ENGINE_MISC_SLEEP 0x1
> @@ -105,6 +110,10 @@
> #define SPI_ENGINE_OFFLOAD_CMD_FIFO_SIZE 16
> #define SPI_ENGINE_OFFLOAD_SDO_FIFO_SIZE 16
>
> +/* Extending SPI_MULTI_BUS_MODE values for optimizing messages. */
> +#define SPI_ENGINE_MULTI_BUS_MODE_UNKNOWN -1
> +#define SPI_ENGINE_MULTI_BUS_MODE_CONFLICTING -2
> +
> struct spi_engine_program {
> unsigned int length;
> uint16_t instructions[] __counted_by(length);
> @@ -142,6 +151,9 @@ struct spi_engine_offload {
> unsigned long flags;
> unsigned int offload_num;
> unsigned int spi_mode_config;
> + unsigned int multi_bus_mode;
> + u8 primary_bus_mask;
> + u8 all_bus_mask;
> u8 bits_per_word;
> };
>
> @@ -165,6 +177,22 @@ struct spi_engine {
> bool offload_requires_sync;
> };
>
> +static u8 spi_engine_primary_bus_flag(struct spi_device *spi)
> +{
> + return BIT(spi->data_bus[0]);
> +}
> +
> +static u8 spi_engine_all_bus_flags(struct spi_device *spi)
> +{
> + u8 flags = 0;
> + int i;
> +
> + for (i = 0; i < spi->num_data_bus; i++)
> + flags |= BIT(spi->data_bus[i]);
> +
> + return flags;
> +}
> +
> static void spi_engine_program_add_cmd(struct spi_engine_program *p,
> bool dry, uint16_t cmd)
> {
> @@ -193,7 +221,7 @@ static unsigned int spi_engine_get_config(struct
> spi_device *spi)
> }
>
> static void spi_engine_gen_xfer(struct spi_engine_program *p, bool dry,
> - struct spi_transfer *xfer)
> + struct spi_transfer *xfer, u32 num_lanes)
> {
> unsigned int len;
>
> @@ -204,6 +232,9 @@ static void spi_engine_gen_xfer(struct spi_engine_program
> *p, bool dry,
> else
> len = xfer->len / 4;
>
> + if (xfer->multi_bus_mode == SPI_MULTI_BUS_MODE_STRIPE)
> + len /= num_lanes;
> +
> while (len) {
> unsigned int n = min(len, 256U);
> unsigned int flags = 0;
> @@ -269,6 +300,7 @@ static int spi_engine_precompile_message(struct
> spi_message *msg)
> {
> unsigned int clk_div, max_hz = msg->spi->controller->max_speed_hz;
> struct spi_transfer *xfer;
> + int multi_bus_mode = SPI_ENGINE_MULTI_BUS_MODE_UNKNOWN;
> u8 min_bits_per_word = U8_MAX;
> u8 max_bits_per_word = 0;
>
> @@ -284,6 +316,24 @@ static int spi_engine_precompile_message(struct
> spi_message *msg)
> min_bits_per_word = min(min_bits_per_word, xfer-
> >bits_per_word);
> max_bits_per_word = max(max_bits_per_word, xfer-
> >bits_per_word);
> }
> +
> + if (xfer->rx_buf || xfer->offload_flags &
> SPI_OFFLOAD_XFER_RX_STREAM ||
> + xfer->tx_buf || xfer->offload_flags &
> SPI_OFFLOAD_XFER_TX_STREAM) {
I'm a bit confused by this condition. It looks like setting priv->multi_bus_mode
(and the other fields) only matters for msg->offload but the above will be true
for regular rx/tx messages, right? Or am i missing something?
If so, I wonder why doing this for all transfers if we only care about
multi_bus_mode for offload messages. I guess you want to validate
xfer->multi_bus_mode? I would then just take the switch() out of the condition
(I mean trying to setup a no data xfer with an invalid bus_mode should also be
seen as invalid IMO) and then use the offload conditions (or maybe simply msg-
>offload?) for the multi_bus_mode handling. To me, it makes the intent more
clear.
- Nuno Sá
Powered by blists - more mailing lists