Re: [PATCH v3 1/5] spi: allow ancillary devices to share parent's chip selects

[Jonathan Cameron]

On Fri, 13 Feb 2026 16:47:33 +0200
Antoniu Miclaus <antoniu.miclaus@xxxxxxxxxx> wrote:

> When registering an ancillary SPI device, the current code flags a chip
> select conflict with the parent device. This happens because the
> ancillary device intentionally uses one of the parent's chip selects,
> but __spi_add_device() checks against all existing devices including
> the parent.
> 
> Allow this by passing the parent device pointer to __spi_add_device()
> and skipping the conflict check when the existing device is the parent.

Smells like a fix. If so why don't we have a fixes tag?

This will need review from Mark. I haven't really understood how this
is intended to work.

Jonathan


> 
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@xxxxxxxxxx>
> Reviewed-by: David Lechner <dlechner@xxxxxxxxxxxx>
> ---
> Changes in v3:
>   - Drop redundant NULL check for info->parent in spi_dev_check()
> 
>  drivers/spi/spi.c | 29 +++++++++++++++++++++++------
>  1 file changed, 23 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index e25df9990f82..693bdcc5a12a 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -641,12 +641,26 @@ static inline int spi_dev_check_cs(struct device *dev,
>  	return 0;
>  }
>  
> +struct spi_dev_check_info {
> +	struct spi_device *new_spi;
> +	struct spi_device *parent;	/* set for ancillary devices */
> +};
> +
>  static int spi_dev_check(struct device *dev, void *data)
>  {
>  	struct spi_device *spi = to_spi_device(dev);
> -	struct spi_device *new_spi = data;
> +	struct spi_dev_check_info *info = data;
> +	struct spi_device *new_spi = info->new_spi;
>  	int status, idx;
>  
> +	/*
> +	 * When registering an ancillary device, skip checking against the
> +	 * parent device since the ancillary is intentionally using one of
> +	 * the parent's chip selects.
> +	 */
> +	if (spi == info->parent)
> +		return 0;
> +
>  	if (spi->controller == new_spi->controller) {
>  		for (idx = 0; idx < spi->num_chipselect; idx++) {
>  			status = spi_dev_check_cs(dev, spi, idx, new_spi, 0);
> @@ -663,10 +677,11 @@ static void spi_cleanup(struct spi_device *spi)
>  		spi->controller->cleanup(spi);
>  }
>  
> -static int __spi_add_device(struct spi_device *spi)
> +static int __spi_add_device(struct spi_device *spi, struct spi_device *parent)
>  {
>  	struct spi_controller *ctlr = spi->controller;
>  	struct device *dev = ctlr->dev.parent;
> +	struct spi_dev_check_info check_info;
>  	int status, idx;
>  	u8 cs;
>  
> @@ -710,7 +725,9 @@ static int __spi_add_device(struct spi_device *spi)
>  	 * chipselect **BEFORE** we call setup(), else we'll trash
>  	 * its configuration.
>  	 */
> -	status = bus_for_each_dev(&spi_bus_type, NULL, spi, spi_dev_check);
> +	check_info.new_spi = spi;
> +	check_info.parent = parent;
> +	status = bus_for_each_dev(&spi_bus_type, NULL, &check_info, spi_dev_check);
>  	if (status)
>  		return status;
>  
> @@ -772,7 +789,7 @@ int spi_add_device(struct spi_device *spi)
>  	spi_dev_set_name(spi);
>  
>  	mutex_lock(&ctlr->add_lock);
> -	status = __spi_add_device(spi);
> +	status = __spi_add_device(spi, NULL);
>  	mutex_unlock(&ctlr->add_lock);
>  	return status;
>  }
> @@ -2580,8 +2597,8 @@ struct spi_device *spi_new_ancillary_device(struct spi_device *spi,
>  
>  	WARN_ON(!mutex_is_locked(&ctlr->add_lock));
>  
> -	/* Register the new device */
> -	rc = __spi_add_device(ancillary);
> +	/* Register the new device, passing the parent to skip CS conflict check */
> +	rc = __spi_add_device(ancillary, spi);
>  	if (rc) {
>  		dev_err(&spi->dev, "failed to register ancillary device\n");
>  		goto err_out;