| Message ID | 20211119173718.52938-1-andriy.shevchenko@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v1,1/2] spi: deduplicate spi_match_id() in __spi_register_driver() | expand |
On Fri, 19 Nov 2021 19:37:17 +0200, Andy Shevchenko wrote: > The same logic is used in spi_match_id() and in the __spi_register_driver(). > By switching the former from taking struct spi_device * to const char * as > the second parameter we may deduplicate the code. > > Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next Thanks! [1/2] spi: deduplicate spi_match_id() in __spi_register_driver() commit: 3f07657506df363709a37f99db04e9e0d0b1bce7 [2/2] spi: Fix multi-line comment style (no commit info) All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark
Hi Andy, On 19/11/2021 17:37, Andy Shevchenko wrote: > The same logic is used in spi_match_id() and in the __spi_register_driver(). > By switching the former from taking struct spi_device * to const char * as > the second parameter we may deduplicate the code. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > drivers/spi/spi.c | 17 ++++++----------- > 1 file changed, 6 insertions(+), 11 deletions(-) > > diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c > index b23e675953e1..dfa06103150e 100644 > --- a/drivers/spi/spi.c > +++ b/drivers/spi/spi.c > @@ -315,11 +315,10 @@ static void spi_statistics_add_transfer_stats(struct spi_statistics *stats, > * and the sysfs version makes coldplug work too. > */ > > -static const struct spi_device_id *spi_match_id(const struct spi_device_id *id, > - const struct spi_device *sdev) > +static const struct spi_device_id *spi_match_id(const struct spi_device_id *id, const char *name) > { > while (id->name[0]) { > - if (!strcmp(sdev->modalias, id->name)) > + if (!strcmp(name, id->name)) > return id; > id++; > } > @@ -330,7 +329,7 @@ const struct spi_device_id *spi_get_device_id(const struct spi_device *sdev) > { > const struct spi_driver *sdrv = to_spi_driver(sdev->dev.driver); > > - return spi_match_id(sdrv->id_table, sdev); > + return spi_match_id(sdrv->id_table, sdev->modalias); > } > EXPORT_SYMBOL_GPL(spi_get_device_id); > > @@ -352,7 +351,7 @@ static int spi_match_device(struct device *dev, struct device_driver *drv) > return 1; > > if (sdrv->id_table) > - return !!spi_match_id(sdrv->id_table, spi); > + return !!spi_match_id(sdrv->id_table, spi->modalias); > > return strcmp(spi->modalias, drv->name) == 0; > } > @@ -474,12 +473,8 @@ int __spi_register_driver(struct module *owner, struct spi_driver *sdrv) > if (sdrv->id_table) { > const struct spi_device_id *spi_id; > > - for (spi_id = sdrv->id_table; spi_id->name[0]; > - spi_id++) > - if (strcmp(spi_id->name, of_name) == 0) > - break; > - > - if (spi_id->name[0]) > + spi_id = spi_match_id(sdrv->id_table, of_name); > + if (!spi_id) > continue; > } else { > if (strcmp(sdrv->driver.name, of_name) == 0) > Following this change I am seeing the following warnings again although most of these have now been fixed ... WARNING KERN SPI driver mtd_dataflash has no spi_device_id for atmel,at45 WARNING KERN SPI driver mtd_dataflash has no spi_device_id for atmel,dataflash WARNING KERN SPI driver spi-nor has no spi_device_id for jedec,spi-nor WARNING KERN SPI driver mmc_spi has no spi_device_id for mmc-spi-slot WARNING KERN SPI driver cros-ec-spi has no spi_device_id for google,cros-ec-spi I have not looked any further yet, but this appears to cause the SPI ID match to fail. Cheers Jon
On Tue, Nov 23, 2021 at 03:22:38PM +0000, Jon Hunter wrote: > On 19/11/2021 17:37, Andy Shevchenko wrote: > > @@ -474,12 +473,8 @@ int __spi_register_driver(struct module *owner, struct spi_driver *sdrv) > > if (sdrv->id_table) { > > const struct spi_device_id *spi_id; > > - for (spi_id = sdrv->id_table; spi_id->name[0]; > > - spi_id++) > > - if (strcmp(spi_id->name, of_name) == 0) > > - break; > > - if (spi_id->name[0]) > > + spi_id = spi_match_id(sdrv->id_table, of_name); > > + if (!spi_id) Seems I have inverted condition here. Shouldn't it be if (spi_id) instead? > > continue; > > } else { > > if (strcmp(sdrv->driver.name, of_name) == 0) > > > > > Following this change I am seeing the following warnings again although most > of these have now been fixed ... > > WARNING KERN SPI driver mtd_dataflash has no spi_device_id for atmel,at45 > WARNING KERN SPI driver mtd_dataflash has no spi_device_id for > atmel,dataflash > WARNING KERN SPI driver spi-nor has no spi_device_id for jedec,spi-nor > WARNING KERN SPI driver mmc_spi has no spi_device_id for mmc-spi-slot > WARNING KERN SPI driver cros-ec-spi has no spi_device_id for > google,cros-ec-spi > > I have not looked any further yet, but this appears to cause the SPI ID > match to fail. Ah, thanks for testing! Can you try above?
On Tue, Nov 23, 2021 at 03:22:38PM +0000, Jon Hunter wrote: > On 19/11/2021 17:37, Andy Shevchenko wrote: > Following this change I am seeing the following warnings again although most > of these have now been fixed ... > > WARNING KERN SPI driver mtd_dataflash has no spi_device_id for atmel,at45 > WARNING KERN SPI driver mtd_dataflash has no spi_device_id for > atmel,dataflash > WARNING KERN SPI driver spi-nor has no spi_device_id for jedec,spi-nor > WARNING KERN SPI driver mmc_spi has no spi_device_id for mmc-spi-slot > WARNING KERN SPI driver cros-ec-spi has no spi_device_id for > google,cros-ec-spi > I have not looked any further yet, but this appears to cause the SPI ID > match to fail. Looking into the code it should be harmless warning. I.o.w. it shouldn't prevent driver registration. In any case I'm about to send a fix, thanks for the report!
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index b23e675953e1..dfa06103150e 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -315,11 +315,10 @@ static void spi_statistics_add_transfer_stats(struct spi_statistics *stats, * and the sysfs version makes coldplug work too. */ -static const struct spi_device_id *spi_match_id(const struct spi_device_id *id, - const struct spi_device *sdev) +static const struct spi_device_id *spi_match_id(const struct spi_device_id *id, const char *name) { while (id->name[0]) { - if (!strcmp(sdev->modalias, id->name)) + if (!strcmp(name, id->name)) return id; id++; } @@ -330,7 +329,7 @@ const struct spi_device_id *spi_get_device_id(const struct spi_device *sdev) { const struct spi_driver *sdrv = to_spi_driver(sdev->dev.driver); - return spi_match_id(sdrv->id_table, sdev); + return spi_match_id(sdrv->id_table, sdev->modalias); } EXPORT_SYMBOL_GPL(spi_get_device_id); @@ -352,7 +351,7 @@ static int spi_match_device(struct device *dev, struct device_driver *drv) return 1; if (sdrv->id_table) - return !!spi_match_id(sdrv->id_table, spi); + return !!spi_match_id(sdrv->id_table, spi->modalias); return strcmp(spi->modalias, drv->name) == 0; } @@ -474,12 +473,8 @@ int __spi_register_driver(struct module *owner, struct spi_driver *sdrv) if (sdrv->id_table) { const struct spi_device_id *spi_id; - for (spi_id = sdrv->id_table; spi_id->name[0]; - spi_id++) - if (strcmp(spi_id->name, of_name) == 0) - break; - - if (spi_id->name[0]) + spi_id = spi_match_id(sdrv->id_table, of_name); + if (!spi_id) continue; } else { if (strcmp(sdrv->driver.name, of_name) == 0)
The same logic is used in spi_match_id() and in the __spi_register_driver(). By switching the former from taking struct spi_device * to const char * as the second parameter we may deduplicate the code. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- drivers/spi/spi.c | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-)