Re: [PATCH 2/6] spi: Support multi-bus controllers

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

On Tue, 2025-10-14 at 17:02 -0500, David Lechner wrote:
> Add support for SPI controllers with multiple physical SPI data buses.
> (A data bus in this context means lines connected to a serializer, so a
> controller with two data buses would have two serializers in a single
> controller).
> 
> This is common in the type of controller that can be used with parallel
> flash memories, but can be used for general purpose SPI as well.
> 
> To indicate support, a controller just needs to set ctlr->num_data_bus
> to something greater than 1. Peripherals indicate which bus they are
> connected to via device tree (ACPI support can be added if needed).
> 
> Signed-off-by: David Lechner <dlechner@baylibre.com>
> ---

LGTM,

Acked-by: Nuno Sá <nuno.sa@analog.com>

> 
> This patch has been seen in a different series [1] by Sean before:
> 
> [1]:
> https://lore.kernel.org/linux-spi/20250616220054.3968946-4-sean.anderson@linux.dev/
> 
> Changes:
> * Use u8 array instead of bitfield so that the order of the mapping is
>   preserved. (Now looks very much like chip select mapping.)
> * Added doc strings for added fields.
> * Tweaked the comments.
> ---
>  drivers/spi/spi.c       | 28 +++++++++++++++++++++++++++-
>  include/linux/spi/spi.h | 17 +++++++++++++++++
>  2 files changed, 44 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index
> 2e0647a06890290e1c9dc4a347a0864329795b08..84e5d5057eb41f1a522c4870265d78feb411
> 09c8 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -2354,7 +2354,7 @@ static void of_spi_parse_dt_cs_delay(struct device_node
> *nc,
>  static int of_spi_parse_dt(struct spi_controller *ctlr, struct spi_device
> *spi,
>  			   struct device_node *nc)
>  {
> -	u32 value, cs[SPI_DEVICE_CS_CNT_MAX];
> +	u32 value, buses[SPI_DEVICE_DATA_BUS_CNT_MAX],
> cs[SPI_DEVICE_CS_CNT_MAX];
>  	int rc, idx;
>  
>  	/* Mode (clock phase/polarity/etc.) */
> @@ -2446,6 +2446,31 @@ static int of_spi_parse_dt(struct spi_controller *ctlr,
> struct spi_device *spi,
>  	for (idx = 0; idx < rc; idx++)
>  		spi_set_chipselect(spi, idx, cs[idx]);
>  
> +	rc = of_property_read_variable_u32_array(nc, "spi-buses", buses, 1,
> +						 ARRAY_SIZE(buses));
> +	if (rc < 0 && rc != -EINVAL) {
> +		dev_err(&ctlr->dev, "%pOF has invalid 'spi-buses' property
> (%d)\n",
> +			nc, rc);
> +		return rc;
> +	}
> +
> +	if (rc == -EINVAL) {
> +		/* Default when property is omitted. */
> +		spi->num_data_bus = 1;
> +	} else {
> +		for (idx = 0; idx < rc; idx++) {
> +			if (buses[idx] >= ctlr->num_data_bus) {
> +				dev_err(&ctlr->dev,
> +					"%pOF has out of range 'spi-buses'
> property (%d/%d)\n",
> +					nc, buses[idx], ctlr->num_data_bus);
> +				return -EINVAL;
> +			}
> +			spi->data_bus[idx] = buses[idx];
> +		}
> +
> +		spi->num_data_bus = rc;
> +	}
> +
>  	/*
>  	 * By default spi->chip_select[0] will hold the physical CS number,
>  	 * so set bit 0 in spi->cs_index_mask.
> @@ -3054,6 +3079,7 @@ struct spi_controller *__spi_alloc_controller(struct
> device *dev,
>  	mutex_init(&ctlr->add_lock);
>  	ctlr->bus_num = -1;
>  	ctlr->num_chipselect = 1;
> +	ctlr->num_data_bus = 1;
>  	ctlr->target = target;
>  	if (IS_ENABLED(CONFIG_SPI_SLAVE) && target)
>  		ctlr->dev.class = &spi_target_class;
> diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
> index
> cb2c2df3108999a73b67ef4a7b0d2cb07adfc669..c314194d4e7e2b396795ece10e142118ca05
> f4eb 100644
> --- a/include/linux/spi/spi.h
> +++ b/include/linux/spi/spi.h
> @@ -23,6 +23,9 @@
>  /* Max no. of CS supported per spi device */
>  #define SPI_DEVICE_CS_CNT_MAX 4
>  
> +/* Max no. of data buses supported per spi device */
> +#define SPI_DEVICE_DATA_BUS_CNT_MAX 8
> +
>  struct dma_chan;
>  struct software_node;
>  struct ptp_system_timestamp;
> @@ -171,6 +174,9 @@ extern void spi_transfer_cs_change_delay_exec(struct
> spi_message *msg,
>   * @chip_select: Array of physical chipselect, spi->chipselect[i] gives
>   *	the corresponding physical CS for logical CS i.
>   * @num_chipselect: Number of physical chipselects used.
> + * @data_bus: Array of physical data buses. This is only used with
> specialized
> + * controllers that support multiple data buses.
> + * @num_data_bus: Number of physical data buses used.
>   * @cs_index_mask: Bit mask of the active chipselect(s) in the chipselect
> array
>   * @cs_gpiod: Array of GPIO descriptors of the corresponding chipselect lines
>   *	(optional, NULL when not using a GPIO line)
> @@ -231,6 +237,8 @@ struct spi_device {
>  
>  	u8			chip_select[SPI_DEVICE_CS_CNT_MAX];
>  	u8			num_chipselect;
> +	u8			data_bus[SPI_DEVICE_DATA_BUS_CNT_MAX];
> +	u8			num_data_bus;
>  
>  	/*
>  	 * Bit mask of the chipselect(s) that the driver need to use from
> @@ -401,6 +409,7 @@ extern struct spi_device *spi_new_ancillary_device(struct
> spi_device *spi, u8 ch
>   *	SPI targets, and are numbered from zero to num_chipselects.
>   *	each target has a chipselect signal, but it's common that not
>   *	every chipselect is connected to a target.
> + * @num_data_bus: Number of data buses supported by this controller. Default
> is 1.
>   * @dma_alignment: SPI controller constraint on DMA buffers alignment.
>   * @mode_bits: flags understood by this controller driver
>   * @buswidth_override_bits: flags to override for this controller driver
> @@ -576,6 +585,14 @@ struct spi_controller {
>  	 */
>  	u16			num_chipselect;
>  
> +	/*
> +	 * Some specialized SPI controllers can have more than one physical
> +	 * bus interface per controller (each having it's own serializer).
> This
> +	 * specifies the number of buses in that case. Other controllers do
> not
> +	 * need to set this (defaults to 1).
> +	 */
> +	u16			num_data_bus;
> +
>  	/* Some SPI controllers pose alignment requirements on DMAable
>  	 * buffers; let protocol drivers know about these requirements.
>  	 */