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

["Nuno Sá" <noname.nuno@gmail.com>]

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@baylibre.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á