diff mbox series

spi: spidev: Fix so the module is autoloaded when built as external

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

Commit Message

Gustav Wiklander Jan. 4, 2021, 3:34 p.m. UTC
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.

Signed-off-by: Gustav Wiklander <gustavwi@axis.com>
---
 drivers/spi/spidev.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

Comments

Mark Brown Jan. 4, 2021, 9:34 p.m. UTC | #1
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?
Gustav Wiklander Jan. 5, 2021, 9:44 a.m. UTC | #2
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
Mark Brown Jan. 7, 2021, 5:40 p.m. UTC | #3
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 mbox series

Patch

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