| Message ID | 20230711171756.86736-5-andriy.shevchenko@linux.intel.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | spi: Header and core clean up and refactoring | expand |
Hi, On Tue, Jul 11, 2023 at 08:17:46PM +0300, Andy Shevchenko wrote: > The commit 0c79378c0199 ("spi: add ancillary device support") > added a dozen of duplicating lines of code. We may move them > to the __spi_add_device(). Note, that the code may be called > under the mutex. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com> -- Sebastian > --- > drivers/spi/spi.c | 32 ++++++++++---------------------- > 1 file changed, 10 insertions(+), 22 deletions(-) > > diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c > index 6d74218cf38e..876d40d2c708 100644 > --- a/drivers/spi/spi.c > +++ b/drivers/spi/spi.c > @@ -631,6 +631,16 @@ static int __spi_add_device(struct spi_device *spi) > struct device *dev = ctlr->dev.parent; > int status; > > + /* Chipselects are numbered 0..max; validate. */ > + if (spi_get_chipselect(spi, 0) >= ctlr->num_chipselect) { > + dev_err(dev, "cs%d >= max %d\n", spi_get_chipselect(spi, 0), > + ctlr->num_chipselect); > + return -EINVAL; > + } > + > + /* Set the bus ID string */ > + spi_dev_set_name(spi); > + > /* > * We need to make sure there's no other device with this > * chipselect **BEFORE** we call setup(), else we'll trash > @@ -689,19 +699,8 @@ static int __spi_add_device(struct spi_device *spi) > int spi_add_device(struct spi_device *spi) > { > struct spi_controller *ctlr = spi->controller; > - struct device *dev = ctlr->dev.parent; > int status; > > - /* Chipselects are numbered 0..max; validate. */ > - if (spi_get_chipselect(spi, 0) >= ctlr->num_chipselect) { > - dev_err(dev, "cs%d >= max %d\n", spi_get_chipselect(spi, 0), > - ctlr->num_chipselect); > - return -EINVAL; > - } > - > - /* Set the bus ID string */ > - spi_dev_set_name(spi); > - > mutex_lock(&ctlr->add_lock); > status = __spi_add_device(spi); > mutex_unlock(&ctlr->add_lock); > @@ -712,17 +711,6 @@ EXPORT_SYMBOL_GPL(spi_add_device); > 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_get_chipselect(spi, 0) >= ctlr->num_chipselect) { > - dev_err(dev, "cs%d >= max %d\n", spi_get_chipselect(spi, 0), > - ctlr->num_chipselect); > - return -EINVAL; > - } > - > - /* Set the bus ID string */ > - spi_dev_set_name(spi); > > WARN_ON(!mutex_is_locked(&ctlr->add_lock)); > return __spi_add_device(spi); > -- > 2.40.0.1.gaa8946217a0b >
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 6d74218cf38e..876d40d2c708 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -631,6 +631,16 @@ static int __spi_add_device(struct spi_device *spi) struct device *dev = ctlr->dev.parent; int status; + /* Chipselects are numbered 0..max; validate. */ + if (spi_get_chipselect(spi, 0) >= ctlr->num_chipselect) { + dev_err(dev, "cs%d >= max %d\n", spi_get_chipselect(spi, 0), + ctlr->num_chipselect); + return -EINVAL; + } + + /* Set the bus ID string */ + spi_dev_set_name(spi); + /* * We need to make sure there's no other device with this * chipselect **BEFORE** we call setup(), else we'll trash @@ -689,19 +699,8 @@ static int __spi_add_device(struct spi_device *spi) int spi_add_device(struct spi_device *spi) { struct spi_controller *ctlr = spi->controller; - struct device *dev = ctlr->dev.parent; int status; - /* Chipselects are numbered 0..max; validate. */ - if (spi_get_chipselect(spi, 0) >= ctlr->num_chipselect) { - dev_err(dev, "cs%d >= max %d\n", spi_get_chipselect(spi, 0), - ctlr->num_chipselect); - return -EINVAL; - } - - /* Set the bus ID string */ - spi_dev_set_name(spi); - mutex_lock(&ctlr->add_lock); status = __spi_add_device(spi); mutex_unlock(&ctlr->add_lock); @@ -712,17 +711,6 @@ EXPORT_SYMBOL_GPL(spi_add_device); 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_get_chipselect(spi, 0) >= ctlr->num_chipselect) { - dev_err(dev, "cs%d >= max %d\n", spi_get_chipselect(spi, 0), - ctlr->num_chipselect); - return -EINVAL; - } - - /* Set the bus ID string */ - spi_dev_set_name(spi); WARN_ON(!mutex_is_locked(&ctlr->add_lock)); return __spi_add_device(spi);
The commit 0c79378c0199 ("spi: add ancillary device support") added a dozen of duplicating lines of code. We may move them to the __spi_add_device(). Note, that the code may be called under the mutex. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- drivers/spi/spi.c | 32 ++++++++++---------------------- 1 file changed, 10 insertions(+), 22 deletions(-)