| Message ID | 20210104153436.20083-1-gustav.wiklander@axis.com |
|---|---|
| State | New |
| Headers | show |
| Series | spi: spidev: Fix so the module is autoloaded when built as external | expand |
On Mon, Jan 04, 2021 at 04:34:35PM +0100, Gustav Wiklander wrote: > From: Gustav Wiklander <gustavwi@axis.com> > > The spi framework sets the modalias for the spi device to belong in > either the acpi device table or the SPI device table. It can never > be in the OF table. Therefore the spidev driver should populate the > spi device table rather than the OF table. > > NOTE: platform drivers and i2c drivers support aliases in the > OF device table. Why is this a good solution rather than ensuring the the OF IDs can be used directly?
On 1/4/21 10:34 PM, Mark Brown wrote: > On Mon, Jan 04, 2021 at 04:34:35PM +0100, Gustav Wiklander wrote: >> From: Gustav Wiklander <gustavwi@axis.com> >> >> The spi framework sets the modalias for the spi device to belong in >> either the acpi device table or the SPI device table. It can never >> be in the OF table. Therefore the spidev driver should populate the >> spi device table rather than the OF table. >> >> NOTE: platform drivers and i2c drivers support aliases in the >> OF device table. > > Why is this a good solution rather than ensuring the the OF IDs can be > used directly? > Hi Mark, You suggestion is of course a solid alternative forward. However, the downside with supporting the OF device table for automatic module loading is that a lot of spi device drivers must be updated. Also it is unclear what is the preferred way to do this in the kernel see this patch: https://lore.kernel.org/lkml/20190618052644.32446-1-bjorn.andersson@linaro.org/ If adding support of OF device table the spi device drivers must now include a MODULE_DEVICE_TABLE(of,...) as the spi device alias will no longer match the alias in the module. This command gives 186 spi device drivers. git grep "MODULE_DEVICE_TABLE(spi" | wc -l 186 Best regards Gustav Wiklander
On Tue, Jan 05, 2021 at 10:44:21AM +0100, Gustav Wiklander wrote: > On 1/4/21 10:34 PM, Mark Brown wrote: > > On Mon, Jan 04, 2021 at 04:34:35PM +0100, Gustav Wiklander wrote: > > > The spi framework sets the modalias for the spi device to belong in > > > either the acpi device table or the SPI device table. It can never > > > be in the OF table. Therefore the spidev driver should populate the > > > spi device table rather than the OF table. > > Why is this a good solution rather than ensuring the the OF IDs can be > > used directly? > You suggestion is of course a solid alternative forward. However, the > downside with supporting the OF device table for automatic module loading is > that a lot of spi device drivers must be updated. Also Is the module code too limited to cope with more than one table? > If adding support of OF device table the spi device drivers must now include > a MODULE_DEVICE_TABLE(of,...) as the spi device alias will no longer match > the alias in the module. > This command gives 186 spi device drivers. How about SPI drivers that already have an OF table and expect it to work, I rather suspect we have a lot of cases where people are adding SPI IDs to DT that don't appear in the module tables and frankly I think that's a reasonable expectation. If there's an issue here beyond missing MODULE_DEVICE_TABLEs in drivers I'd expect it to be fixed in the core, otherwise we're just leaving sharp edges for everyone.
diff --git a/drivers/spi/spidev.c b/drivers/spi/spidev.c index 859910ec8d9f..5cf419a046da 100644 --- a/drivers/spi/spidev.c +++ b/drivers/spi/spidev.c @@ -687,6 +687,19 @@ static const struct of_device_id spidev_dt_ids[] = { MODULE_DEVICE_TABLE(of, spidev_dt_ids); #endif +static const struct spi_device_id spidev_ids[] = { + { .name = "dh2228fv" }, + { .name = "ltc2488" }, + { .name = "achc" }, + { .name = "sx1301" }, + { .name = "bk4" }, + { .name = "dhcom-board" }, + { .name = "m53cpld" }, + { .name = "xc7a35t" }, + {}, +}; +MODULE_DEVICE_TABLE(spi, spidev_ids); + #ifdef CONFIG_ACPI /* Dummy SPI devices not to be used in production systems */ @@ -817,7 +830,7 @@ static struct spi_driver spidev_spi_driver = { }, .probe = spidev_probe, .remove = spidev_remove, - + .id_table = spidev_ids, /* NOTE: suspend/resume methods are not necessary here. * We don't do anything except pass the requests to/from * the underlying controller. The refrigerator handles