Re: [PATCH 4/6] spi: axi-spi-engine: support SPI_MULTI_BUS_MODE_STRIPE

From: Nuno Sá

Date: Wed Oct 15 2025 - 06:30:08 EST


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@xxxxxxxxxxxx>
> ---
>  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á