Re: [PATCHv4 1/6] spi: add ancillary device support

[Sebastian Reichel <sebastian.reichel@collabora.com>]

[-- Attachment #1: Type: text/plain, Size: 1623 bytes --]

Hi Greg,

On Fri, Jun 11, 2021 at 11:06:53AM +0200, Greg Kroah-Hartman wrote:
> On Wed, Jun 09, 2021 at 05:12:30PM +0200, Sebastian Reichel wrote:
> > Introduce support for ancillary devices, similar to existing
> > implementation for I2C. This is useful for devices having
> > multiple chip-selects, for example some microcontrollers
> > provide a normal SPI interface and a flashing SPI interface.
> > 
> > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> > ---
> > [...]
> > +static int spi_add_device_locked(struct spi_device *spi)
> > +{
> > +	struct spi_controller *ctlr = spi->controller;
> > +	struct device *dev = ctlr->dev.parent;
> > +
> > +	/* Chipselects are numbered 0..max; validate. */
> > +	if (spi->chip_select >= ctlr->num_chipselect) {
> > +		dev_err(dev, "cs%d >= max %d\n", spi->chip_select,
> > +			ctlr->num_chipselect);
> > +		return -EINVAL;
> > +	}
> > +
> > +	/* Set the bus ID string */
> > +	spi_dev_set_name(spi);
> > +
> > +	WARN_ON(!mutex_is_locked(&spi_add_lock));
> 
> So you just rebooted a machine that has panic-on-warn set.  Not
> nice.
> 
> If this really can happen, test for it and recover, do not reboot
> devices.
> 
> If this really can never happen, why are you testing for it?

This is reached when ancillary device is not registered in
the main SPI device's probe routine, which would be a driver
bug. The gehc-achc driver calls it in the right place, so
this is not reached with this patchset, but the function to
register ancillary devices is generic and is expected to be
also used by others.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]