Hello,
Short version:
This patch series is the SPI equivalent of the I2C one posted before [0].
This series add the missing MODULE_DEVICE_TABLE() for OF and SPI tables
to export that information so modules have the correct aliases built-in
and autoloading works correctly.
Longer version:
The SPI core always reports the MODALIAS uevent as "spi:<modalias>"
regardless of the mechanism that was used to register the device (i.e:
OF or board code) and the table that is used later to match the driver
with the device (i.e: SPI id table or OF match table).
But this means that OF-only drivers needs to have both OF and SPI id
tables that have to be kept in sync and also the device node's compatible
manufacturer prefix is stripped when reporting the MODALIAS. Which can
lead to issues if two vendors use the same SPI device name for example.
Also, there are many SPI drivers whose module auto-loading is not working
because of this fact that the SPI core always reports the MODALIAS as
spi:<modalias> and many developers didn't expect this since is not how
other subsystems behave.
I've identified SPI drivers with 3 types of different issues:
a) Those that have an spi_table but are not exported. The match works
if the driver is built-in but since the ID table is not exported,
module auto-load won't work.
b) Those that have a of_table but are not exported. This is currently
not an issue since even when the of_table is used to match the dev
with the driver, an OF modalias is not reported by the SPI core.
But if the SPI core is changed to report the MODALIAS of the form
of:N*T*C as it's made by other subsystems, then module auto-load
will break for these drivers.
c) Those that don't have an of_table but should since are OF drivers
with DT bindings doc for them. Since the SPI core does not report
a OF modalias and since spi_match_device() fallbacks to match the
device part of the compatible string with the SPI device ID table,
many OF drivers don't have an of_table to match. After all having
a SPI device ID table is mandatory so it works without a of_table.
So, in order to not make mandatory to have a SPI device ID table, all
these three kind of issues have to be addressed. This series does that.
I split the changes so the patches in this series are independent and
can be picked individually by subsystem maintainers.
Patches #1 and #2 solves a), patches #3 to #8 solves b) and patches
Patch #18 changes the logic of spi_uevent() to report an OF modalias if
the device was registered using OF. But this patch is included in the
series only as an RFC for illustration purposes since changing that
without first applying all the other patches in this series, will break
module autoloading for the drivers of devices registered using OF but
that lacks an of_match_table. I'll repost patch #18 once all the patches
in this series have landed.
[0]: https://lkml.org/lkml/2015/7/30/519
Best regards,
Javier
Javier Martinez Canillas (18):
iio: Export SPI module alias information in missing drivers
staging: iio: hmc5843: Export missing SPI module alias information
mtd: dataflash: Export OF module alias information
OMAPDSS: panel-sony-acx565akm: Export OF module alias information
mmc: mmc_spi: Export OF module alias information
staging: mt29f_spinand: Export OF module alias information
net: ks8851: Export OF module alias information
[media] s5c73m3: Export OF module alias information
mfd: cros_ec: spi: Add OF match table
iio: dac: ad7303: Add OF match table
iio: adc: max1027: Set struct spi_driver .of_match_table
mfd: stmpe: Add OF match table
iio: adc: mcp320x: Set struct spi_driver .of_match_table
iio: as3935: Add OF match table
iio: adc128s052: Add OF match table
iio: frequency: adf4350: Add OF match table
NFC: trf7970a: Add OF match table
spi: (RFC, don't apply) report OF style modalias when probing using DT
drivers/iio/adc/max1027.c | 1 +
drivers/iio/adc/mcp320x.c | 1 +
drivers/iio/adc/ti-adc128s052.c | 8 ++++++++
drivers/iio/amplifiers/ad8366.c | 1 +
drivers/iio/dac/ad7303.c | 7 +++++++
drivers/iio/frequency/adf4350.c | 9 +++++++++
drivers/iio/proximity/as3935.c | 7 +++++++
drivers/media/i2c/s5c73m3/s5c73m3-spi.c | 1 +
drivers/mfd/cros_ec_spi.c | 7 +++++++
drivers/mfd/stmpe-spi.c | 13 +++++++++++++
drivers/mmc/host/mmc_spi.c | 1 +
drivers/mtd/devices/mtd_dataflash.c | 1 +
drivers/net/ethernet/micrel/ks8851.c | 1 +
drivers/nfc/trf7970a.c | 7 +++++++
drivers/spi/spi.c | 8 ++++++++
drivers/staging/iio/magnetometer/hmc5843_spi.c | 1 +
drivers/staging/mt29f_spinand/mt29f_spinand.c | 1 +
.../video/fbdev/omap2/displays-new/panel-sony-acx565akm.c | 1 +
18 files changed, 76 insertions(+)
--
2.4.3
The SPI core always reports the MODALIAS uevent as "spi:<modalias>"
regardless of the mechanism that was used to register the device
(i.e: OF or board code) and the table that is used later to match
the driver with the device (i.e: SPI id table or OF match table).
So drivers needs to export the SPI id table and this be built into
the module or udev won't have the necessary information to autoload
the needed driver module when the device is added.
Signed-off-by: Javier Martinez Canillas <[email protected]>
---
drivers/iio/amplifiers/ad8366.c | 1 +
drivers/iio/frequency/adf4350.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/iio/amplifiers/ad8366.c b/drivers/iio/amplifiers/ad8366.c
index c0d364ebaea8..32b82a2dc894 100644
--- a/drivers/iio/amplifiers/ad8366.c
+++ b/drivers/iio/amplifiers/ad8366.c
@@ -195,6 +195,7 @@ static const struct spi_device_id ad8366_id[] = {
{"ad8366", 0},
{}
};
+MODULE_DEVICE_TABLE(spi, ad8366_id);
static struct spi_driver ad8366_driver = {
.driver = {
diff --git a/drivers/iio/frequency/adf4350.c b/drivers/iio/frequency/adf4350.c
index 9890c81c027d..8bd873fc3787 100644
--- a/drivers/iio/frequency/adf4350.c
+++ b/drivers/iio/frequency/adf4350.c
@@ -621,6 +621,7 @@ static const struct spi_device_id adf4350_id[] = {
{"adf4351", 4351},
{}
};
+MODULE_DEVICE_TABLE(spi, adf4350_id);
static struct spi_driver adf4350_driver = {
.driver = {
--
2.4.3
The SPI core always reports the MODALIAS uevent as "spi:<modalias>"
regardless of the mechanism that was used to register the device
(i.e: OF or board code) and the table that is used later to match
the driver with the device (i.e: SPI id table or OF match table).
So drivers needs to export the SPI id table and this be built into
the module or udev won't have the necessary information to autoload
the needed driver module when the device is added.
Signed-off-by: Javier Martinez Canillas <[email protected]>
---
drivers/staging/iio/magnetometer/hmc5843_spi.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/staging/iio/magnetometer/hmc5843_spi.c b/drivers/staging/iio/magnetometer/hmc5843_spi.c
index 8e658f736e1f..4dfb372d2fec 100644
--- a/drivers/staging/iio/magnetometer/hmc5843_spi.c
+++ b/drivers/staging/iio/magnetometer/hmc5843_spi.c
@@ -81,6 +81,7 @@ static const struct spi_device_id hmc5843_id[] = {
{ "hmc5983", HMC5983_ID },
{ }
};
+MODULE_DEVICE_TABLE(spi, hmc5843_id);
static struct spi_driver hmc5843_driver = {
.driver = {
--
2.4.3
The SPI core always reports the MODALIAS uevent as "spi:<modalias>"
regardless of the mechanism that was used to register the device
(i.e: OF or board code) and the table that is used later to match
the driver with the device (i.e: SPI id table or OF match table).
So drivers needs to export the SPI id table and this be built into
the module or udev won't have the necessary information to autoload
the needed driver module when the device is added.
But this means that OF-only drivers needs to have both OF and SPI id
tables that have to be kept in sync and also the dev node compatible
manufacturer prefix is stripped when reporting the MODALIAS. Which can
lead to issues if two vendors use the same SPI device name for example.
To avoid the above, the SPI core behavior may be changed in the future
to not require an SPI device table for OF-only drivers and report the
OF module alias. So, it's better to also export the OF table even when
is unused now to prevent breaking module loading when the core changes.
Signed-off-by: Javier Martinez Canillas <[email protected]>
---
drivers/mtd/devices/mtd_dataflash.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/mtd/devices/mtd_dataflash.c b/drivers/mtd/devices/mtd_dataflash.c
index 0099aba72a8b..df6f61137376 100644
--- a/drivers/mtd/devices/mtd_dataflash.c
+++ b/drivers/mtd/devices/mtd_dataflash.c
@@ -102,6 +102,7 @@ static const struct of_device_id dataflash_dt_ids[] = {
{ .compatible = "atmel,dataflash", },
{ /* sentinel */ }
};
+MODULE_DEVICE_TABLE(of, dataflash_dt_ids);
#endif
/* ......................................................................... */
--
2.4.3
The SPI core always reports the MODALIAS uevent as "spi:<modalias>"
regardless of the mechanism that was used to register the device
(i.e: OF or board code) and the table that is used later to match
the driver with the device (i.e: SPI id table or OF match table).
So drivers needs to export the SPI id table and this be built into
the module or udev won't have the necessary information to autoload
the needed driver module when the device is added.
But this means that OF-only drivers needs to have both OF and SPI id
tables that have to be kept in sync and also the dev node compatible
manufacturer prefix is stripped when reporting the MODALIAS. Which can
lead to issues if two vendors use the same SPI device name for example.
To avoid the above, the SPI core behavior may be changed in the future
to not require an SPI device table for OF-only drivers and report the
OF module alias. So, it's better to also export the OF table even when
is unused now to prevent breaking module loading when the core changes.
Signed-off-by: Javier Martinez Canillas <[email protected]>
---
drivers/video/fbdev/omap2/displays-new/panel-sony-acx565akm.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/video/fbdev/omap2/displays-new/panel-sony-acx565akm.c b/drivers/video/fbdev/omap2/displays-new/panel-sony-acx565akm.c
index 90cbc4c3406c..c581231c74a5 100644
--- a/drivers/video/fbdev/omap2/displays-new/panel-sony-acx565akm.c
+++ b/drivers/video/fbdev/omap2/displays-new/panel-sony-acx565akm.c
@@ -898,6 +898,7 @@ static const struct of_device_id acx565akm_of_match[] = {
{ .compatible = "omapdss,sony,acx565akm", },
{},
};
+MODULE_DEVICE_TABLE(of, acx565akm_of_match);
static struct spi_driver acx565akm_driver = {
.driver = {
--
2.4.3
The SPI core always reports the MODALIAS uevent as "spi:<modalias>"
regardless of the mechanism that was used to register the device
(i.e: OF or board code) and the table that is used later to match
the driver with the device (i.e: SPI id table or OF match table).
So drivers needs to export the SPI id table and this be built into
the module or udev won't have the necessary information to autoload
the needed driver module when the device is added.
But this means that OF-only drivers needs to have both OF and SPI id
tables that have to be kept in sync and also the dev node compatible
manufacturer prefix is stripped when reporting the MODALIAS. Which can
lead to issues if two vendors use the same SPI device name for example.
To avoid the above, the SPI core behavior may be changed in the future
to not require an SPI device table for OF-only drivers and report the
OF module alias. So, it's better to also export the OF table even when
is unused now to prevent breaking module loading when the core changes.
Signed-off-by: Javier Martinez Canillas <[email protected]>
---
drivers/mmc/host/mmc_spi.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/mmc/host/mmc_spi.c b/drivers/mmc/host/mmc_spi.c
index ae19d83bb9de..8ee11f4120fc 100644
--- a/drivers/mmc/host/mmc_spi.c
+++ b/drivers/mmc/host/mmc_spi.c
@@ -1511,6 +1511,7 @@ static const struct of_device_id mmc_spi_of_match_table[] = {
{ .compatible = "mmc-spi-slot", },
{},
};
+MODULE_DEVICE_TABLE(of, mmc_spi_of_match_table);
static struct spi_driver mmc_spi_driver = {
.driver = {
--
2.4.3
The SPI core always reports the MODALIAS uevent as "spi:<modalias>"
regardless of the mechanism that was used to register the device
(i.e: OF or board code) and the table that is used later to match
the driver with the device (i.e: SPI id table or OF match table).
So drivers needs to export the SPI id table and this be built into
the module or udev won't have the necessary information to autoload
the needed driver module when the device is added.
But this means that OF-only drivers needs to have both OF and SPI id
tables that have to be kept in sync and also the dev node compatible
manufacturer prefix is stripped when reporting the MODALIAS. Which can
lead to issues if two vendors use the same SPI device name for example.
To avoid the above, the SPI core behavior may be changed in the future
to not require an SPI device table for OF-only drivers and report the
OF module alias. So, it's better to also export the OF table even when
is unused now to prevent breaking module loading when the core changes.
Signed-off-by: Javier Martinez Canillas <[email protected]>
---
drivers/staging/mt29f_spinand/mt29f_spinand.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/staging/mt29f_spinand/mt29f_spinand.c b/drivers/staging/mt29f_spinand/mt29f_spinand.c
index ad30ce4206ef..1aa449e5fecf 100644
--- a/drivers/staging/mt29f_spinand/mt29f_spinand.c
+++ b/drivers/staging/mt29f_spinand/mt29f_spinand.c
@@ -941,6 +941,7 @@ static const struct of_device_id spinand_dt[] = {
{ .compatible = "spinand,mt29f", },
{}
};
+MODULE_DEVICE_TABLE(of, spinand_dt);
/*
* Device name structure description
--
2.4.3
The SPI core always reports the MODALIAS uevent as "spi:<modalias>"
regardless of the mechanism that was used to register the device
(i.e: OF or board code) and the table that is used later to match
the driver with the device (i.e: SPI id table or OF match table).
So drivers needs to export the SPI id table and this be built into
the module or udev won't have the necessary information to autoload
the needed driver module when the device is added.
But this means that OF-only drivers needs to have both OF and SPI id
tables that have to be kept in sync and also the dev node compatible
manufacturer prefix is stripped when reporting the MODALIAS. Which can
lead to issues if two vendors use the same SPI device name for example.
To avoid the above, the SPI core behavior may be changed in the future
to not require an SPI device table for OF-only drivers and report the
OF module alias. So, it's better to also export the OF table even when
is unused now to prevent breaking module loading when the core changes.
Signed-off-by: Javier Martinez Canillas <[email protected]>
---
drivers/net/ethernet/micrel/ks8851.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/ethernet/micrel/ks8851.c b/drivers/net/ethernet/micrel/ks8851.c
index 66d4ab703f45..60f43ec22175 100644
--- a/drivers/net/ethernet/micrel/ks8851.c
+++ b/drivers/net/ethernet/micrel/ks8851.c
@@ -1601,6 +1601,7 @@ static const struct of_device_id ks8851_match_table[] = {
{ .compatible = "micrel,ks8851" },
{ }
};
+MODULE_DEVICE_TABLE(of, ks8851_match_table);
static struct spi_driver ks8851_driver = {
.driver = {
--
2.4.3
The SPI core always reports the MODALIAS uevent as "spi:<modalias>"
regardless of the mechanism that was used to register the device
(i.e: OF or board code) and the table that is used later to match
the driver with the device (i.e: SPI id table or OF match table).
So drivers needs to export the SPI id table and this be built into
the module or udev won't have the necessary information to autoload
the needed driver module when the device is added.
But this means that OF-only drivers needs to have both OF and SPI id
tables that have to be kept in sync and also the dev node compatible
manufacturer prefix is stripped when reporting the MODALIAS. Which can
lead to issues if two vendors use the same SPI device name for example.
To avoid the above, the SPI core behavior may be changed in the future
to not require an SPI device table for OF-only drivers and report the
OF module alias. So, it's better to also export the OF table even when
is unused now to prevent breaking module loading when the core changes.
Signed-off-by: Javier Martinez Canillas <[email protected]>
---
drivers/media/i2c/s5c73m3/s5c73m3-spi.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/media/i2c/s5c73m3/s5c73m3-spi.c b/drivers/media/i2c/s5c73m3/s5c73m3-spi.c
index fa4a5ebda6b2..9983635ec253 100644
--- a/drivers/media/i2c/s5c73m3/s5c73m3-spi.c
+++ b/drivers/media/i2c/s5c73m3/s5c73m3-spi.c
@@ -31,6 +31,7 @@ static const struct of_device_id s5c73m3_spi_ids[] = {
{ .compatible = "samsung,s5c73m3" },
{ }
};
+MODULE_DEVICE_TABLE(of, s5c73m3_spi_ids;);
enum spi_direction {
SPI_DIR_RX,
--
2.4.3
The Documentation/devicetree/bindings/mfd/cros-ec.txt DT binding doc lists
"google,cros-ec-spi" as a compatible string but the corresponding driver
does not have an OF match table. Add the table to the driver so the SPI
core can do an OF style match.
Signed-off-by: Javier Martinez Canillas <[email protected]>
---
drivers/mfd/cros_ec_spi.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/mfd/cros_ec_spi.c b/drivers/mfd/cros_ec_spi.c
index 16f228dc243f..30a296b4e748 100644
--- a/drivers/mfd/cros_ec_spi.c
+++ b/drivers/mfd/cros_ec_spi.c
@@ -701,6 +701,12 @@ static int cros_ec_spi_resume(struct device *dev)
static SIMPLE_DEV_PM_OPS(cros_ec_spi_pm_ops, cros_ec_spi_suspend,
cros_ec_spi_resume);
+static const struct of_device_id cros_ec_spi_of_match[] = {
+ { .compatible = "google,cros-ec-spi", },
+ { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, cros_ec_spi_of_match);
+
static const struct spi_device_id cros_ec_spi_id[] = {
{ "cros-ec-spi", 0 },
{ }
@@ -710,6 +716,7 @@ MODULE_DEVICE_TABLE(spi, cros_ec_spi_id);
static struct spi_driver cros_ec_driver_spi = {
.driver = {
.name = "cros-ec-spi",
+ .of_match_table = of_match_ptr(cros_ec_spi_of_match),
.owner = THIS_MODULE,
.pm = &cros_ec_spi_pm_ops,
},
--
2.4.3
The Documentation/devicetree/bindings/iio/dac/ad7303.txt DT binding doc
lists "adi,ad7303" as a compatible string but the corresponding driver
does not have an OF match table. Add the table to the driver so the SPI
core can do an OF style match.
Signed-off-by: Javier Martinez Canillas <[email protected]>
---
drivers/iio/dac/ad7303.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/iio/dac/ad7303.c b/drivers/iio/dac/ad7303.c
index fa2810032968..18a4ad5ff8c5 100644
--- a/drivers/iio/dac/ad7303.c
+++ b/drivers/iio/dac/ad7303.c
@@ -281,6 +281,12 @@ static int ad7303_remove(struct spi_device *spi)
return 0;
}
+static const struct of_device_id ad7303_spi_of_match[] = {
+ { .compatible = "adi,ad7303", },
+ { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, ad7303_spi_of_match);
+
static const struct spi_device_id ad7303_spi_ids[] = {
{ "ad7303", 0 },
{}
@@ -290,6 +296,7 @@ MODULE_DEVICE_TABLE(spi, ad7303_spi_ids);
static struct spi_driver ad7303_driver = {
.driver = {
.name = "ad7303",
+ .of_match_table = of_match_ptr(ad7303_spi_of_match),
.owner = THIS_MODULE,
},
.probe = ad7303_probe,
--
2.4.3
The driver has an OF id table but the .of_match_table is not set so
the SPI core can't do an OF style match and the table was unused.
Signed-off-by: Javier Martinez Canillas <[email protected]>
---
drivers/iio/adc/max1027.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/iio/adc/max1027.c b/drivers/iio/adc/max1027.c
index 44bf815adb6c..54a8302aaace 100644
--- a/drivers/iio/adc/max1027.c
+++ b/drivers/iio/adc/max1027.c
@@ -508,6 +508,7 @@ static int max1027_remove(struct spi_device *spi)
static struct spi_driver max1027_driver = {
.driver = {
.name = "max1027",
+ .of_match_table = of_match_ptr(max1027_adc_dt_ids),
.owner = THIS_MODULE,
},
.probe = max1027_probe,
--
2.4.3
The Documentation/devicetree/bindings/mfd/stmpe.txt DT binding doc lists
"st,stmpe[610|801|811|1601|2401|2403]" as valid compatible strings but
the corresponding driver does not have an OF match table. Add the table
to the driver so the SPI core can do an OF style match.
Signed-off-by: Javier Martinez Canillas <[email protected]>
---
drivers/mfd/stmpe-spi.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/mfd/stmpe-spi.c b/drivers/mfd/stmpe-spi.c
index 6fdb30e84a2b..618ba244d98a 100644
--- a/drivers/mfd/stmpe-spi.c
+++ b/drivers/mfd/stmpe-spi.c
@@ -11,6 +11,7 @@
#include <linux/interrupt.h>
#include <linux/kernel.h>
#include <linux/module.h>
+#include <linux/of.h>
#include <linux/types.h>
#include "stmpe.h"
@@ -108,6 +109,17 @@ static int stmpe_spi_remove(struct spi_device *spi)
return stmpe_remove(stmpe);
}
+static const struct of_device_id stmpe_spi_of_match[] = {
+ { .compatible = "st,stmpe610", },
+ { .compatible = "st,stmpe801", },
+ { .compatible = "st,stmpe811", },
+ { .compatible = "st,stmpe1601", },
+ { .compatible = "st,stmpe2401", },
+ { .compatible = "st,stmpe2403", },
+ { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, stmpe_spi_of_match);
+
static const struct spi_device_id stmpe_spi_id[] = {
{ "stmpe610", STMPE610 },
{ "stmpe801", STMPE801 },
@@ -122,6 +134,7 @@ MODULE_DEVICE_TABLE(spi, stmpe_id);
static struct spi_driver stmpe_spi_driver = {
.driver = {
.name = "stmpe-spi",
+ .of_match_table = of_match_ptr(stmpe_spi_of_match),
.owner = THIS_MODULE,
#ifdef CONFIG_PM
.pm = &stmpe_dev_pm_ops,
--
2.4.3
The driver has an OF id table but the .of_match_table is not set so
the SPI core can't do an OF style match and the table was unused.
Signed-off-by: Javier Martinez Canillas <[email protected]>
---
drivers/iio/adc/mcp320x.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/iio/adc/mcp320x.c b/drivers/iio/adc/mcp320x.c
index b19e4f9d16e0..41a21e986c1a 100644
--- a/drivers/iio/adc/mcp320x.c
+++ b/drivers/iio/adc/mcp320x.c
@@ -404,6 +404,7 @@ MODULE_DEVICE_TABLE(spi, mcp320x_id);
static struct spi_driver mcp320x_driver = {
.driver = {
.name = "mcp320x",
+ .of_match_table = of_match_ptr(mcp320x_dt_ids),
.owner = THIS_MODULE,
},
.probe = mcp320x_probe,
--
2.4.3
The Documentation/devicetree/bindings/iio/proximity/as3935.txt DT binding
doc lists "ams,as3935" as a compatible string but the corresponding driver
does not have an OF match table. Add the table to the driver so the SPI
core can do an OF style match.
Signed-off-by: Javier Martinez Canillas <[email protected]>
---
drivers/iio/proximity/as3935.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/iio/proximity/as3935.c b/drivers/iio/proximity/as3935.c
index bc0d68efd455..e95035136889 100644
--- a/drivers/iio/proximity/as3935.c
+++ b/drivers/iio/proximity/as3935.c
@@ -434,6 +434,12 @@ static int as3935_remove(struct spi_device *spi)
return 0;
}
+static const struct of_device_id as3935_of_match[] = {
+ { .compatible = "ams,as3935", },
+ { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, as3935_of_match);
+
static const struct spi_device_id as3935_id[] = {
{"as3935", 0},
{},
@@ -443,6 +449,7 @@ MODULE_DEVICE_TABLE(spi, as3935_id);
static struct spi_driver as3935_driver = {
.driver = {
.name = "as3935",
+ .of_match_table = of_match_ptr(as3935_of_match),
.owner = THIS_MODULE,
.pm = AS3935_PM_OPS,
},
--
2.4.3
The Documentation/devicetree/bindings/iio/adc/ti-adc128s052.txt DT binding
doc lists "ti,adc128s052" or "ti,adc122s021" as compatible strings but the
corresponding driver does not have an OF match table. Add the table to the
driver so the SPI core can do an OF style match.
Signed-off-by: Javier Martinez Canillas <[email protected]>
---
drivers/iio/adc/ti-adc128s052.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/iio/adc/ti-adc128s052.c b/drivers/iio/adc/ti-adc128s052.c
index 915be6b60097..98c0d2b444bf 100644
--- a/drivers/iio/adc/ti-adc128s052.c
+++ b/drivers/iio/adc/ti-adc128s052.c
@@ -174,6 +174,13 @@ static int adc128_remove(struct spi_device *spi)
return 0;
}
+static const struct of_device_id adc128_of_match[] = {
+ { .compatible = "ti,adc128s052", },
+ { .compatible = "ti,adc122s021", },
+ { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, adc128_of_match);
+
static const struct spi_device_id adc128_id[] = {
{ "adc128s052", 0}, /* index into adc128_config */
{ "adc122s021", 1},
@@ -184,6 +191,7 @@ MODULE_DEVICE_TABLE(spi, adc128_id);
static struct spi_driver adc128_driver = {
.driver = {
.name = "adc128s052",
+ .of_match_table = of_match_ptr(adc128_of_match),
.owner = THIS_MODULE,
},
.probe = adc128_probe,
--
2.4.3
The Documentation/devicetree/bindings/iio/frequency/adf4350.txt DT binding
doc lists "adi,adf4350" or "adi,adf4351" as compatible strings but the
corresponding driver does not have an OF match table. Add the table to the
driver so the SPI core can do an OF style match.
Signed-off-by: Javier Martinez Canillas <[email protected]>
---
drivers/iio/frequency/adf4350.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/iio/frequency/adf4350.c b/drivers/iio/frequency/adf4350.c
index 8bd873fc3787..73f27e0a08dd 100644
--- a/drivers/iio/frequency/adf4350.c
+++ b/drivers/iio/frequency/adf4350.c
@@ -616,6 +616,13 @@ static int adf4350_remove(struct spi_device *spi)
return 0;
}
+static const struct of_device_id adf4350_of_match[] = {
+ { .compatible = "adi,adf4350", },
+ { .compatible = "adi,adf4351", },
+ { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, adf4350_of_match);
+
static const struct spi_device_id adf4350_id[] = {
{"adf4350", 4350},
{"adf4351", 4351},
@@ -626,6 +633,7 @@ MODULE_DEVICE_TABLE(spi, adf4350_id);
static struct spi_driver adf4350_driver = {
.driver = {
.name = "adf4350",
+ .of_match_table = of_match_ptr(adf4350_of_match),
.owner = THIS_MODULE,
},
.probe = adf4350_probe,
--
2.4.3
The Documentation/devicetree/bindings/net/nfc/trf7970a.txt DT binding doc
lists "ti,trf7970a" as a compatible string but the corresponding driver
does not have an OF match table. Add the table to the driver so the SPI
core can do an OF style match.
Signed-off-by: Javier Martinez Canillas <[email protected]>
---
drivers/nfc/trf7970a.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/nfc/trf7970a.c b/drivers/nfc/trf7970a.c
index 85b4d86772d8..2021f2135445 100644
--- a/drivers/nfc/trf7970a.c
+++ b/drivers/nfc/trf7970a.c
@@ -2209,6 +2209,12 @@ static const struct dev_pm_ops trf7970a_pm_ops = {
trf7970a_pm_runtime_resume, NULL)
};
+static const struct of_device_id trf7970a_of_match[] = {
+ { .compatible = "ti,trf7970a", },
+ { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, trf7970a_of_match);
+
static const struct spi_device_id trf7970a_id_table[] = {
{ "trf7970a", 0 },
{ }
@@ -2221,6 +2227,7 @@ static struct spi_driver trf7970a_spi_driver = {
.id_table = trf7970a_id_table,
.driver = {
.name = "trf7970a",
+ .of_match_table = of_match_ptr(trf7970a_of_match),
.owner = THIS_MODULE,
.pm = &trf7970a_pm_ops,
},
--
2.4.3
From: Javier Martinez Canillas <[email protected]>
An SPI driver that supports both OF and legacy platforms, will have
both an OF and SPI ID table. This means that when built as a module,
the aliases will be filled from both tables, e.g:
$ modinfo cros_ec_spi | grep alias
alias: spi:cros-ec-spi
alias: of:N*T*Cgoogle,cros-ec-spi*
But currently the SPI core always report a module alias of the form
spi:<modalias>, e.g:
$ cat /sys/devices/12d40000.spi/spi_master/spi2/spi2.0/modalias
spi:cros-ec-spi
And also this spi:<modalias> is always reported to user-space as
MODALIAS regardless of the mechanism that was used to register the
device (i.e: OF or board code).
This means that OF-only drivers needs to have both OF and SPI id
tables that have to be kept in sync and also the dev node compatible
manufacturer prefix is stripped when reporting the MODALIAS. Which can
lead to issues if two vendors use the same SPI device name for example.
This patch changes the SPI core to report an OF related MODALIAS uevent
(of:N*T*C) env var instead so the module can be auto-loaded and also
the correct modalias is reported on sysfs, e.g:
$ cat /sys/devices/12d40000.spi/spi_master/spi2/spi2.0/modalias
of:Ncros-ecT<NULL>Cgoogle,cros-ec-spi
Signed-off-by: Javier Martinez Canillas <[email protected]>
Signed-off-by: Javier Martinez Canillas <[email protected]>
---
drivers/spi/spi.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 08861a0233ca..beb7fb2b15c5 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -59,6 +59,10 @@ modalias_show(struct device *dev, struct device_attribute *a, char *buf)
const struct spi_device *spi = to_spi_device(dev);
int len;
+ len = of_device_get_modalias(dev, buf, PAGE_SIZE - 1);
+ if (len != -ENODEV)
+ return len;
+
len = acpi_device_modalias(dev, buf, PAGE_SIZE - 1);
if (len != -ENODEV)
return len;
@@ -250,6 +254,10 @@ static int spi_uevent(struct device *dev, struct kobj_uevent_env *env)
const struct spi_device *spi = to_spi_device(dev);
int rc;
+ rc = of_device_uevent_modalias(dev, env);
+ if (rc != -ENODEV)
+ return rc;
+
rc = acpi_device_uevent_modalias(dev, env);
if (rc != -ENODEV)
return rc;
--
2.4.3
On 08/20/2015 09:07 AM, Javier Martinez Canillas wrote:
> The Documentation/devicetree/bindings/iio/frequency/adf4350.txt DT binding
> doc lists "adi,adf4350" or "adi,adf4351" as compatible strings but the
> corresponding driver does not have an OF match table. Add the table to the
> driver so the SPI core can do an OF style match.
>
> Signed-off-by: Javier Martinez Canillas <[email protected]>
Acked-by: Lars-Peter Clausen <[email protected]>
Thanks.
On 08/20/2015 09:07 AM, Javier Martinez Canillas wrote:
> The Documentation/devicetree/bindings/iio/dac/ad7303.txt DT binding doc
> lists "adi,ad7303" as a compatible string but the corresponding driver
> does not have an OF match table. Add the table to the driver so the SPI
> core can do an OF style match.
>
> Signed-off-by: Javier Martinez Canillas <[email protected]>
Acked-by: Lars-Peter Clausen <[email protected]>
Thanks.
On 08/20/2015 09:07 AM, Javier Martinez Canillas wrote:
> The SPI core always reports the MODALIAS uevent as "spi:<modalias>"
> regardless of the mechanism that was used to register the device
> (i.e: OF or board code) and the table that is used later to match
> the driver with the device (i.e: SPI id table or OF match table).
>
> So drivers needs to export the SPI id table and this be built into
> the module or udev won't have the necessary information to autoload
> the needed driver module when the device is added.
>
> Signed-off-by: Javier Martinez Canillas <[email protected]>
Acked-by: Lars-Peter Clausen <[email protected]>
Thanks.
On Thu, Aug 20, 2015 at 09:07:31AM +0200, Javier Martinez Canillas wrote:
> From: Javier Martinez Canillas <[email protected]>
>
> An SPI driver that supports both OF and legacy platforms, will have
> both an OF and SPI ID table. This means that when built as a module,
> the aliases will be filled from both tables, e.g:
This is tagged as something that can't be applied but you've not
explained why it can't be applied or what comments might be useful :(
On Thu, Aug 20, 2015 at 09:07:26AM +0200, Javier Martinez Canillas wrote:
> The driver has an OF id table but the .of_match_table is not set so
> the SPI core can't do an OF style match and the table was unused.
>
Is an OF style match necessary?
I have been using devicetree and it matches based on the .id_table.
Couldn't we just remove the mcp320x_dt_ids table instead?
> Signed-off-by: Javier Martinez Canillas <[email protected]>
> ---
>
> drivers/iio/adc/mcp320x.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/iio/adc/mcp320x.c b/drivers/iio/adc/mcp320x.c
> index b19e4f9d16e0..41a21e986c1a 100644
> --- a/drivers/iio/adc/mcp320x.c
> +++ b/drivers/iio/adc/mcp320x.c
> @@ -404,6 +404,7 @@ MODULE_DEVICE_TABLE(spi, mcp320x_id);
> static struct spi_driver mcp320x_driver = {
> .driver = {
> .name = "mcp320x",
> + .of_match_table = of_match_ptr(mcp320x_dt_ids),
> .owner = THIS_MODULE,
> },
> .probe = mcp320x_probe,
> --
> 2.4.3
>
On Thu, Aug 20, 2015 at 11:36:34AM -0700, Mark Brown wrote:
> On Thu, Aug 20, 2015 at 09:07:31AM +0200, Javier Martinez Canillas wrote:
> > From: Javier Martinez Canillas <[email protected]>
> >
> > An SPI driver that supports both OF and legacy platforms, will have
> > both an OF and SPI ID table. This means that when built as a module,
> > the aliases will be filled from both tables, e.g:
>
> This is tagged as something that can't be applied but you've not
> explained why it can't be applied or what comments might be useful :(
I believe that's mostly addressed in the cover letter [1].
Patch #18 changes the logic of spi_uevent() to report an OF modalias
if the device was registered using OF. But this patch is included in
the series only as an RFC for illustration purposes since changing
that without first applying all the other patches in this series, will
break module autoloading for the drivers of devices registered using
OF but that lacks an of_match_table. I'll repost patch #18 once all
the patches in this series have landed.
IOW, it's labeled as such mostly for safety, since it has quite a few
distributed dependencies.
Brian
[1] https://lkml.org/lkml/2015/8/20/109
On Thu, Aug 20, 2015 at 09:07:13AM +0200, Javier Martinez Canillas wrote:
> Patches #1 and #2 solves a), patches #3 to #8 solves b) and patches
^^^ I'm dying to know how this sentence ends :)
> Patch #18 changes the logic of spi_uevent() to report an OF modalias if
> the device was registered using OF. But this patch is included in the
> series only as an RFC for illustration purposes since changing that
> without first applying all the other patches in this series, will break
> module autoloading for the drivers of devices registered using OF but
> that lacks an of_match_table. I'll repost patch #18 once all the patches
> in this series have landed.
On a more productive note, the patches I've looked at look good to me.
The missing aliases are a problem enough that should be fixed (i.e.,
part (b)). I'll leave the SPI framework changes to others to comment on.
Brian
On 08/20/2015 11:08 PM, Brian Norris wrote:
> On Thu, Aug 20, 2015 at 11:36:34AM -0700, Mark Brown wrote:
>> On Thu, Aug 20, 2015 at 09:07:31AM +0200, Javier Martinez Canillas wrote:
>>> From: Javier Martinez Canillas <[email protected]>
>>>
>>> An SPI driver that supports both OF and legacy platforms, will have
>>> both an OF and SPI ID table. This means that when built as a module,
>>> the aliases will be filled from both tables, e.g:
>>
>> This is tagged as something that can't be applied but you've not
>> explained why it can't be applied or what comments might be useful :(
>
As Brian pointed out it was in the cover letter so I thought it would
just be duplicated information. But you are right, I should had added
a brief note as well just to make the patch self contained.
> I believe that's mostly addressed in the cover letter [1].
>
> Patch #18 changes the logic of spi_uevent() to report an OF modalias
> if the device was registered using OF. But this patch is included in
> the series only as an RFC for illustration purposes since changing
> that without first applying all the other patches in this series, will
> break module autoloading for the drivers of devices registered using
> OF but that lacks an of_match_table. I'll repost patch #18 once all
> the patches in this series have landed.
>
> IOW, it's labeled as such mostly for safety, since it has quite a few
> distributed dependencies.
>
That's correct. If is merged before the other patches, module autoloading
will break for the drivers that are relying on the current behavior.
I just added for illustration purposes to show what was the plan.
> Brian
>
> [1] https://lkml.org/lkml/2015/8/20/109
>
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
Hello Brian,
On 08/20/2015 11:11 PM, Brian Norris wrote:
> On Thu, Aug 20, 2015 at 09:07:13AM +0200, Javier Martinez Canillas wrote:
>> Patches #1 and #2 solves a), patches #3 to #8 solves b) and patches
>
> ^^^ I'm dying to know how this sentence ends :)
>
Sigh, I did some last minute restructuring of the cover letter and
seems I missed a sentence. I meant to said:
"and patches #9 to #17 solves c)."
>> Patch #18 changes the logic of spi_uevent() to report an OF modalias if
>> the device was registered using OF. But this patch is included in the
>> series only as an RFC for illustration purposes since changing that
>> without first applying all the other patches in this series, will break
>> module autoloading for the drivers of devices registered using OF but
>> that lacks an of_match_table. I'll repost patch #18 once all the patches
>> in this series have landed.
>
> On a more productive note, the patches I've looked at look good to me.
> The missing aliases are a problem enough that should be fixed (i.e.,
> part (b)). I'll leave the SPI framework changes to others to comment on.
>
Great, thanks a lot for your feedback.
> Brian
>
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
On Thu, Aug 20, 2015 at 09:07:16AM +0200, Javier Martinez Canillas wrote:
> The SPI core always reports the MODALIAS uevent as "spi:<modalias>"
> regardless of the mechanism that was used to register the device
> (i.e: OF or board code) and the table that is used later to match
> the driver with the device (i.e: SPI id table or OF match table).
>
> So drivers needs to export the SPI id table and this be built into
> the module or udev won't have the necessary information to autoload
> the needed driver module when the device is added.
>
> But this means that OF-only drivers needs to have both OF and SPI id
> tables that have to be kept in sync and also the dev node compatible
> manufacturer prefix is stripped when reporting the MODALIAS. Which can
> lead to issues if two vendors use the same SPI device name for example.
>
> To avoid the above, the SPI core behavior may be changed in the future
> to not require an SPI device table for OF-only drivers and report the
> OF module alias. So, it's better to also export the OF table even when
> is unused now to prevent breaking module loading when the core changes.
>
> Signed-off-by: Javier Martinez Canillas <[email protected]>
I appreciate the context for the whole problem here, but this commit
subject mostly doesn't apply to this driver, and it deson't seem to
match the problem this is solving. As I see it, the patch description
for this patch should be more like:
"The OF ID table is used for auto-probing the device, but it is not
exported to the module device table. That means this driver won't
autoload when built as a module.
Export the OF ID table to fix that.
(insert some reference to the larger issue here)"
Do you want to rewrite the message, shall I just prepend my
modifications, or am I off-base?
> ---
>
> drivers/mtd/devices/mtd_dataflash.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/mtd/devices/mtd_dataflash.c b/drivers/mtd/devices/mtd_dataflash.c
> index 0099aba72a8b..df6f61137376 100644
> --- a/drivers/mtd/devices/mtd_dataflash.c
> +++ b/drivers/mtd/devices/mtd_dataflash.c
> @@ -102,6 +102,7 @@ static const struct of_device_id dataflash_dt_ids[] = {
> { .compatible = "atmel,dataflash", },
> { /* sentinel */ }
> };
> +MODULE_DEVICE_TABLE(of, dataflash_dt_ids);
> #endif
>
> /* ......................................................................... */
For the patch:
Reviewed-by: Brian Norris <[email protected]>
Hello Michael,
On 08/20/2015 10:09 PM, Michael Welling wrote:
> On Thu, Aug 20, 2015 at 09:07:26AM +0200, Javier Martinez Canillas wrote:
>> The driver has an OF id table but the .of_match_table is not set so
>> the SPI core can't do an OF style match and the table was unused.
>>
>
> Is an OF style match necessary?
>
Did you read the cover letter [0] on which I explain why is needed to
avoid breaking module autoloading in the future? Once the SPI core is
changed by RFC patch 18/18? (you were cc'ed in the cover letter BTW).
> I have been using devicetree and it matches based on the .id_table.
>
Yes it fallbacks to the .id_table or the driver name but the correct
thing to do for devices registered by OF, is to match using the
compatible string.
> Couldn't we just remove the mcp320x_dt_ids table instead?
>
No, that is the wrong thing to do IMHO since the compatible string
contains both vendor and device name whle the .id_table only contains
a device name.
So it makes sense to match using the compatible string and also report
the OF modalias information to user-space.
Otherwise what's the point of the vendor in the compatible string for
SPI devices? You can just use "bar" instead of "foo,bar" as a string.
[0]: https://lkml.org/lkml/2015/8/20/109
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
Hello Brian,
On 08/20/2015 11:54 PM, Brian Norris wrote:
> On Thu, Aug 20, 2015 at 09:07:16AM +0200, Javier Martinez Canillas wrote:
>> The SPI core always reports the MODALIAS uevent as "spi:<modalias>"
>> regardless of the mechanism that was used to register the device
>> (i.e: OF or board code) and the table that is used later to match
>> the driver with the device (i.e: SPI id table or OF match table).
>>
>> So drivers needs to export the SPI id table and this be built into
>> the module or udev won't have the necessary information to autoload
>> the needed driver module when the device is added.
>>
>> But this means that OF-only drivers needs to have both OF and SPI id
>> tables that have to be kept in sync and also the dev node compatible
>> manufacturer prefix is stripped when reporting the MODALIAS. Which can
>> lead to issues if two vendors use the same SPI device name for example.
>>
>> To avoid the above, the SPI core behavior may be changed in the future
>> to not require an SPI device table for OF-only drivers and report the
>> OF module alias. So, it's better to also export the OF table even when
>> is unused now to prevent breaking module loading when the core changes.
>>
>> Signed-off-by: Javier Martinez Canillas <[email protected]>
>
> I appreciate the context for the whole problem here, but this commit
> subject mostly doesn't apply to this driver, and it deson't seem to
> match the problem this is solving. As I see it, the patch description
> for this patch should be more like:
>
> "The OF ID table is used for auto-probing the device, but it is not
> exported to the module device table. That means this driver won't
> autoload when built as a module.
>
> Export the OF ID table to fix that.
>
> (insert some reference to the larger issue here)"
>
> Do you want to rewrite the message, shall I just prepend my
> modifications, or am I off-base?
>
Well, I believe my commit message is more accurate than yours :-)
I mean, right now the OF modalias information is not really used neither
by the SPI core since always MODALIAS=spi:foo is reported nor by user
space since the MODALIAS uevent will never match the OF exported modalias.
So this patch really is a no-op right now and is not fixing anything.
As long as the driver has a MODULE_DEVICE_TABLE(spi,...), things will
work without a MODULE_DEVICE_TABLE(of,...).
However, this patch is needed to avoid breaking module autoloading in
the future once RFC patch 18/18 is applied and the SPI core is changed
to report a MODALIAS=of:N*T*Cfoo,bar for SPI devices registered by OF.
Having said that, I will of course re-spin the patch and write a
commit message that you agree on. So I'll be happy to trim it down if
is providing too much content or is not correct.
I just don't think your suggestion better matches reality than mine.
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
On Fri, Aug 21, 2015 at 12:02:40AM +0200, Javier Martinez Canillas wrote:
> Hello Michael,
>
> On 08/20/2015 10:09 PM, Michael Welling wrote:
> > On Thu, Aug 20, 2015 at 09:07:26AM +0200, Javier Martinez Canillas wrote:
> >> The driver has an OF id table but the .of_match_table is not set so
> >> the SPI core can't do an OF style match and the table was unused.
> >>
> >
> > Is an OF style match necessary?
> >
>
> Did you read the cover letter [0] on which I explain why is needed to
> avoid breaking module autoloading in the future? Once the SPI core is
> changed by RFC patch 18/18? (you were cc'ed in the cover letter BTW).
Well I have read it now. :)
>
> > I have been using devicetree and it matches based on the .id_table.
> >
>
> Yes it fallbacks to the .id_table or the driver name but the correct
> thing to do for devices registered by OF, is to match using the
> compatible string.
>
> > Couldn't we just remove the mcp320x_dt_ids table instead?
> >
>
> No, that is the wrong thing to do IMHO since the compatible string
> contains both vendor and device name whle the .id_table only contains
> a device name.
>
> So it makes sense to match using the compatible string and also report
> the OF modalias information to user-space.
>
> Otherwise what's the point of the vendor in the compatible string for
> SPI devices? You can just use "bar" instead of "foo,bar" as a string.
>
Well then shouldn't the patch include adding the vendor to the compatible
string?
> [0]: https://lkml.org/lkml/2015/8/20/109
>
> Best regards,
> --
> Javier Martinez Canillas
> Open Source Group
> Samsung Research America
On Fri, Aug 21, 2015 at 12:13:34AM +0200, Javier Martinez Canillas wrote:
> So this patch really is a no-op right now and is not fixing anything.
> As long as the driver has a MODULE_DEVICE_TABLE(spi,...), things will
This driver does not have a MODULE_DEVICE_TABLE() at all, nor does it
use spi_device_id. So, it is currently broken for the module use case.
> work without a MODULE_DEVICE_TABLE(of,...).
Brian
Hello Michael,
On 08/21/2015 12:29 AM, Michael Welling wrote:
> On Fri, Aug 21, 2015 at 12:02:40AM +0200, Javier Martinez Canillas wrote:
>> Hello Michael,
>>
>> On 08/20/2015 10:09 PM, Michael Welling wrote:
>>> On Thu, Aug 20, 2015 at 09:07:26AM +0200, Javier Martinez Canillas wrote:
>>>> The driver has an OF id table but the .of_match_table is not set so
>>>> the SPI core can't do an OF style match and the table was unused.
>>>>
>>>
>>> Is an OF style match necessary?
>>>
>>
>> Did you read the cover letter [0] on which I explain why is needed to
>> avoid breaking module autoloading in the future? Once the SPI core is
>> changed by RFC patch 18/18? (you were cc'ed in the cover letter BTW).
>
> Well I have read it now. :)
>
Great :)
>>
>>> I have been using devicetree and it matches based on the .id_table.
>>>
>>
>> Yes it fallbacks to the .id_table or the driver name but the correct
>> thing to do for devices registered by OF, is to match using the
>> compatible string.
>>
>>> Couldn't we just remove the mcp320x_dt_ids table instead?
>>>
>>
>> No, that is the wrong thing to do IMHO since the compatible string
>> contains both vendor and device name whle the .id_table only contains
>> a device name.
>>
>> So it makes sense to match using the compatible string and also report
>> the OF modalias information to user-space.
>>
>> Otherwise what's the point of the vendor in the compatible string for
>> SPI devices? You can just use "bar" instead of "foo,bar" as a string.
>>
>
> Well then shouldn't the patch include adding the vendor to the compatible
> string?
>
Well, I was talking in general. You are right that this specific driver does
not have a vendor prefix for the compatible strings. This is incorrect
according to the ePAPR document [0].
However, these compatible strings are already documented as a DT binding doc
in Documentation/devicetree/bindings/iio/adc/mcp320x.txt so I don't know
what is the correct thing to do in this situation.
Changing the compatible string will break old DTB with newer kernels and that
is a no go. But that doesn't invalidate what I said since once the SPI core
is changed and report OF modalias, you will need a MODULE_DEVICE_TABLE(of,...)
regardless if the compatible string has a vendor prefix or not.
[0]: https://www.power.org/documentation/epapr-version-1-1/
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
Hello Brian,
On 08/21/2015 12:34 AM, Brian Norris wrote:
> On Fri, Aug 21, 2015 at 12:13:34AM +0200, Javier Martinez Canillas wrote:
>> So this patch really is a no-op right now and is not fixing anything.
>> As long as the driver has a MODULE_DEVICE_TABLE(spi,...), things will
>
> This driver does not have a MODULE_DEVICE_TABLE() at all, nor does it
> use spi_device_id. So, it is currently broken for the module use case.
>
That is correct but also that does not mean that this patch will fix
module autoloading right now. It will though once the SPI core is
changed to report a OF type module alias.
So what about something along this lines?
mtd: dataflash: Export OF module alias information
The SPI core currently reports the MODALIAS uevent as "spi:<modalias>"
even for SPI devices that were registered by OF.
That means the OF module alias exported by MODULE_OF_TABLE(of,...) is
currently not used and user-space has no way to autoload this module.
But is still a good practice to add the OF module alias information
into the kernel module even when currently is unused so once the SPI
core is changed to report a correct OF modalias uevent, module
autoloading will be working for this driver.
>> work without a MODULE_DEVICE_TABLE(of,...).
>
> Brian
>
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
On Thu, Aug 20, 2015 at 11:45:09PM +0200, Javier Martinez Canillas wrote:
> On 08/20/2015 11:08 PM, Brian Norris wrote:
> >> This is tagged as something that can't be applied but you've not
> >> explained why it can't be applied or what comments might be useful :(
> As Brian pointed out it was in the cover letter so I thought it would
> just be duplicated information. But you are right, I should had added
> a brief note as well just to make the patch self contained.
Right, a big part of what I was looking for was something about why this
is an incompatible change in the changelog so that once it gets applied
someone with out of tree code which gets broken can see what happens.
Plus...
> > I believe that's mostly addressed in the cover letter [1].
> > Patch #18 changes the logic of spi_uevent() to report an OF modalias
> > if the device was registered using OF. But this patch is included in
> > the series only as an RFC for illustration purposes since changing
> > that without first applying all the other patches in this series, will
> > break module autoloading for the drivers of devices registered using
> > OF but that lacks an of_match_table. I'll repost patch #18 once all
> > the patches in this series have landed.
> > IOW, it's labeled as such mostly for safety, since it has quite a few
> > distributed dependencies.
Are there really only 17 drivers that are missing an explict of_table?
That seems like a low number.
Hello Mark,
On 08/21/2015 01:25 AM, Mark Brown wrote:
> On Thu, Aug 20, 2015 at 11:45:09PM +0200, Javier Martinez Canillas wrote:
>> On 08/20/2015 11:08 PM, Brian Norris wrote:
>
>>>> This is tagged as something that can't be applied but you've not
>>>> explained why it can't be applied or what comments might be useful :(
>
>> As Brian pointed out it was in the cover letter so I thought it would
>> just be duplicated information. But you are right, I should had added
>> a brief note as well just to make the patch self contained.
>
> Right, a big part of what I was looking for was something about why this
> is an incompatible change in the changelog so that once it gets applied
> someone with out of tree code which gets broken can see what happens.
Yes, you are absolutely right. When I finally post this as a proper
patch I'll make sure to have a big NOTE so people can track module
autoload issues for OF drivers down to this commit.
> Plus...
>
>>> I believe that's mostly addressed in the cover letter [1].
>
>>> Patch #18 changes the logic of spi_uevent() to report an OF modalias
>>> if the device was registered using OF. But this patch is included in
>>> the series only as an RFC for illustration purposes since changing
>>> that without first applying all the other patches in this series, will
>>> break module autoloading for the drivers of devices registered using
>>> OF but that lacks an of_match_table. I'll repost patch #18 once all
>>> the patches in this series have landed.
>
>>> IOW, it's labeled as such mostly for safety, since it has quite a few
>>> distributed dependencies.
>
> Are there really only 17 drivers that are missing an explict of_table?
> That seems like a low number.
>
In fact the 17 patches are the combination of the SPI drivers that:
a) Have a .id_table but not a MODULE_DEVICE_TABLE(spi,...)
b) Have a .of_match_table but no a MODULE_DEVICE_TABLE(of,...)
c) Don't have a .of_match_table but have a DT binding document
Maybe there are more SPI drivers out there that only have a .id_table
and don't have a .of_match_table nor a DT binding doc. But in that case
there isn't too much I can do since I've no information that these are
drivers are actually used in systems booted with OF.
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
On Fri, Aug 21, 2015 at 12:48:23AM +0200, Javier Martinez Canillas wrote:
> Hello Michael,
>
> On 08/21/2015 12:29 AM, Michael Welling wrote:
> > On Fri, Aug 21, 2015 at 12:02:40AM +0200, Javier Martinez Canillas wrote:
> >> Hello Michael,
> >>
> >> On 08/20/2015 10:09 PM, Michael Welling wrote:
> >>> On Thu, Aug 20, 2015 at 09:07:26AM +0200, Javier Martinez Canillas wrote:
> >>>> The driver has an OF id table but the .of_match_table is not set so
> >>>> the SPI core can't do an OF style match and the table was unused.
> >>>>
> >>>
> >>> Is an OF style match necessary?
> >>>
> >>
> >> Did you read the cover letter [0] on which I explain why is needed to
> >> avoid breaking module autoloading in the future? Once the SPI core is
> >> changed by RFC patch 18/18? (you were cc'ed in the cover letter BTW).
> >
> > Well I have read it now. :)
> >
>
> Great :)
>
> >>
> >>> I have been using devicetree and it matches based on the .id_table.
> >>>
> >>
> >> Yes it fallbacks to the .id_table or the driver name but the correct
> >> thing to do for devices registered by OF, is to match using the
> >> compatible string.
> >>
> >>> Couldn't we just remove the mcp320x_dt_ids table instead?
> >>>
> >>
> >> No, that is the wrong thing to do IMHO since the compatible string
> >> contains both vendor and device name whle the .id_table only contains
> >> a device name.
> >>
> >> So it makes sense to match using the compatible string and also report
> >> the OF modalias information to user-space.
> >>
> >> Otherwise what's the point of the vendor in the compatible string for
> >> SPI devices? You can just use "bar" instead of "foo,bar" as a string.
> >>
> >
> > Well then shouldn't the patch include adding the vendor to the compatible
> > string?
> >
>
> Well, I was talking in general. You are right that this specific driver does
> not have a vendor prefix for the compatible strings. This is incorrect
> according to the ePAPR document [0].
>
> However, these compatible strings are already documented as a DT binding doc
> in Documentation/devicetree/bindings/iio/adc/mcp320x.txt so I don't know
> what is the correct thing to do in this situation.
>
> Changing the compatible string will break old DTB with newer kernels and that
> is a no go. But that doesn't invalidate what I said since once the SPI core
> is changed and report OF modalias, you will need a MODULE_DEVICE_TABLE(of,...)
> regardless if the compatible string has a vendor prefix or not.
>
Okay.
Acked-by: Michael Welling <[email protected]>
> [0]: https://www.power.org/documentation/epapr-version-1-1/
>
> Best regards,
> --
> Javier Martinez Canillas
> Open Source Group
> Samsung Research America
On Fri, Aug 21, 2015 at 01:47:00AM +0200, Javier Martinez Canillas wrote:
> On 08/21/2015 01:25 AM, Mark Brown wrote:
> > On Thu, Aug 20, 2015 at 11:45:09PM +0200, Javier Martinez Canillas wrote:
> >>> IOW, it's labeled as such mostly for safety, since it has quite a few
> >>> distributed dependencies.
> > Are there really only 17 drivers that are missing an explict of_table?
> > That seems like a low number.
> In fact the 17 patches are the combination of the SPI drivers that:
> a) Have a .id_table but not a MODULE_DEVICE_TABLE(spi,...)
> b) Have a .of_match_table but no a MODULE_DEVICE_TABLE(of,...)
> c) Don't have a .of_match_table but have a DT binding document
> Maybe there are more SPI drivers out there that only have a .id_table
> and don't have a .of_match_table nor a DT binding doc. But in that case
> there isn't too much I can do since I've no information that these are
> drivers are actually used in systems booted with OF.
We could at the very least scan through the in tree DTs. There does
seem to be a substantial overlap between systems that often don't use
modular kernels but could and systems where people are using the I2C
and SPI ID mapping shims.
Hi Javier,
On Fri, Aug 21, 2015 at 12:57:54AM +0200, Javier Martinez Canillas wrote:
> On 08/21/2015 12:34 AM, Brian Norris wrote:
> > On Fri, Aug 21, 2015 at 12:13:34AM +0200, Javier Martinez Canillas wrote:
> >> So this patch really is a no-op right now and is not fixing anything.
> >> As long as the driver has a MODULE_DEVICE_TABLE(spi,...), things will
> >
> > This driver does not have a MODULE_DEVICE_TABLE() at all, nor does it
> > use spi_device_id. So, it is currently broken for the module use case.
> >
>
> That is correct but also that does not mean that this patch will fix
> module autoloading right now. It will though once the SPI core is
> changed to report a OF type module alias.
Ah, thanks for the patience. I missed the point that it will still be
broken.
> So what about something along this lines?
>
> mtd: dataflash: Export OF module alias information
>
> The SPI core currently reports the MODALIAS uevent as "spi:<modalias>"
> even for SPI devices that were registered by OF.
>
> That means the OF module alias exported by MODULE_OF_TABLE(of,...) is
> currently not used and user-space has no way to autoload this module.
>
> But is still a good practice to add the OF module alias information
nit: s/is/it is/
> into the kernel module even when currently is unused so once the SPI
ditto
> core is changed to report a correct OF modalias uevent, module
> autoloading will be working for this driver.
Otherwise, looks good. I'll either patch in this commit message, or
await v2. Your call.
Regards,
Brian
Hello Brian,
On 08/22/2015 12:47 AM, Brian Norris wrote:
> Hi Javier,
>
> On Fri, Aug 21, 2015 at 12:57:54AM +0200, Javier Martinez Canillas wrote:
>> On 08/21/2015 12:34 AM, Brian Norris wrote:
>>> On Fri, Aug 21, 2015 at 12:13:34AM +0200, Javier Martinez Canillas wrote:
>>>> So this patch really is a no-op right now and is not fixing anything.
>>>> As long as the driver has a MODULE_DEVICE_TABLE(spi,...), things will
>>>
>>> This driver does not have a MODULE_DEVICE_TABLE() at all, nor does it
>>> use spi_device_id. So, it is currently broken for the module use case.
>>>
>>
>> That is correct but also that does not mean that this patch will fix
>> module autoloading right now. It will though once the SPI core is
>> changed to report a OF type module alias.
>
> Ah, thanks for the patience. I missed the point that it will still be
> broken.
>
No worries, I'm glad that we are on the same page now.
>> So what about something along this lines?
>>
>> mtd: dataflash: Export OF module alias information
>>
>> The SPI core currently reports the MODALIAS uevent as "spi:<modalias>"
>> even for SPI devices that were registered by OF.
>>
>> That means the OF module alias exported by MODULE_OF_TABLE(of,...) is
>> currently not used and user-space has no way to autoload this module.
>>
>> But is still a good practice to add the OF module alias information
>
> nit: s/is/it is/
>
>> into the kernel module even when currently is unused so once the SPI
>
> ditto
>
>> core is changed to report a correct OF modalias uevent, module
>> autoloading will be working for this driver.
>
> Otherwise, looks good. I'll either patch in this commit message, or
> await v2. Your call.
>
If you don't mind fixing it up yourself when applying, then that
would be really helpful. Otherwise I can post a v2 on Monday.
> Regards,
> Brian
>
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
On Sat, Aug 22, 2015 at 02:26:14AM +0200, Javier Martinez Canillas wrote:
> On 08/22/2015 12:47 AM, Brian Norris wrote:
> > Otherwise, looks good. I'll either patch in this commit message, or
> > await v2. Your call.
>
> If you don't mind fixing it up yourself when applying, then that
> would be really helpful. Otherwise I can post a v2 on Monday.
Queued to l2-mtd.git/next (for 4.4), with the edited commit message.
Hello Brian,
On 08/22/2015 03:05 AM, Brian Norris wrote:
> On Sat, Aug 22, 2015 at 02:26:14AM +0200, Javier Martinez Canillas wrote:
>> On 08/22/2015 12:47 AM, Brian Norris wrote:
>>> Otherwise, looks good. I'll either patch in this commit message, or
>>> await v2. Your call.
>>
>> If you don't mind fixing it up yourself when applying, then that
>> would be really helpful. Otherwise I can post a v2 on Monday.
>
> Queued to l2-mtd.git/next (for 4.4), with the edited commit message.
>
Great, thanks a lot for your help!
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
On 20/08/15 23:48, Javier Martinez Canillas wrote:
> Hello Michael,
>
> On 08/21/2015 12:29 AM, Michael Welling wrote:
>> On Fri, Aug 21, 2015 at 12:02:40AM +0200, Javier Martinez Canillas wrote:
>>> Hello Michael,
>>>
>>> On 08/20/2015 10:09 PM, Michael Welling wrote:
>>>> On Thu, Aug 20, 2015 at 09:07:26AM +0200, Javier Martinez Canillas wrote:
>>>>> The driver has an OF id table but the .of_match_table is not set so
>>>>> the SPI core can't do an OF style match and the table was unused.
>>>>>
>>>>
>>>> Is an OF style match necessary?
>>>>
>>>
>>> Did you read the cover letter [0] on which I explain why is needed to
>>> avoid breaking module autoloading in the future? Once the SPI core is
>>> changed by RFC patch 18/18? (you were cc'ed in the cover letter BTW).
>>
>> Well I have read it now. :)
>>
>
> Great :)
>
>>>
>>>> I have been using devicetree and it matches based on the .id_table.
>>>>
>>>
>>> Yes it fallbacks to the .id_table or the driver name but the correct
>>> thing to do for devices registered by OF, is to match using the
>>> compatible string.
>>>
>>>> Couldn't we just remove the mcp320x_dt_ids table instead?
>>>>
>>>
>>> No, that is the wrong thing to do IMHO since the compatible string
>>> contains both vendor and device name whle the .id_table only contains
>>> a device name.
>>>
>>> So it makes sense to match using the compatible string and also report
>>> the OF modalias information to user-space.
>>>
>>> Otherwise what's the point of the vendor in the compatible string for
>>> SPI devices? You can just use "bar" instead of "foo,bar" as a string.
>>>
>>
>> Well then shouldn't the patch include adding the vendor to the compatible
>> string?
>>
>
> Well, I was talking in general. You are right that this specific driver does
> not have a vendor prefix for the compatible strings. This is incorrect
> according to the ePAPR document [0].
>
> However, these compatible strings are already documented as a DT binding doc
> in Documentation/devicetree/bindings/iio/adc/mcp320x.txt so I don't know
> what is the correct thing to do in this situation.
Take the view the old version is wrong but needs to be supported and add also
the corrected strings + document them.
cc'd The device tree list for any more comments on this.
>
> Changing the compatible string will break old DTB with newer kernels and that
> is a no go. But that doesn't invalidate what I said since once the SPI core
> is changed and report OF modalias, you will need a MODULE_DEVICE_TABLE(of,...)
> regardless if the compatible string has a vendor prefix or not.
>
> [0]: https://www.power.org/documentation/epapr-version-1-1/
>
> Best regards,
>
On 20/08/15 08:07, Javier Martinez Canillas wrote:
> The SPI core always reports the MODALIAS uevent as "spi:<modalias>"
> regardless of the mechanism that was used to register the device
> (i.e: OF or board code) and the table that is used later to match
> the driver with the device (i.e: SPI id table or OF match table).
>
> So drivers needs to export the SPI id table and this be built into
> the module or udev won't have the necessary information to autoload
> the needed driver module when the device is added.
>
> Signed-off-by: Javier Martinez Canillas <[email protected]>
Applied to the togreg branch of iio.git.
This is too late for the upcoming merge window so it will be queued up for
the next one.
Thanks,
Jonathan
> ---
>
> drivers/staging/iio/magnetometer/hmc5843_spi.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/staging/iio/magnetometer/hmc5843_spi.c b/drivers/staging/iio/magnetometer/hmc5843_spi.c
> index 8e658f736e1f..4dfb372d2fec 100644
> --- a/drivers/staging/iio/magnetometer/hmc5843_spi.c
> +++ b/drivers/staging/iio/magnetometer/hmc5843_spi.c
> @@ -81,6 +81,7 @@ static const struct spi_device_id hmc5843_id[] = {
> { "hmc5983", HMC5983_ID },
> { }
> };
> +MODULE_DEVICE_TABLE(spi, hmc5843_id);
>
> static struct spi_driver hmc5843_driver = {
> .driver = {
>
On 20/08/15 08:07, Javier Martinez Canillas wrote:
> The driver has an OF id table but the .of_match_table is not set so
> the SPI core can't do an OF style match and the table was unused.
>
> Signed-off-by: Javier Martinez Canillas <[email protected]>
Applied to the togreg branch of iio.git
Thanks,
Joanthan
> ---
>
> drivers/iio/adc/max1027.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/iio/adc/max1027.c b/drivers/iio/adc/max1027.c
> index 44bf815adb6c..54a8302aaace 100644
> --- a/drivers/iio/adc/max1027.c
> +++ b/drivers/iio/adc/max1027.c
> @@ -508,6 +508,7 @@ static int max1027_remove(struct spi_device *spi)
> static struct spi_driver max1027_driver = {
> .driver = {
> .name = "max1027",
> + .of_match_table = of_match_ptr(max1027_adc_dt_ids),
> .owner = THIS_MODULE,
> },
> .probe = max1027_probe,
>
On 20/08/15 08:07, Javier Martinez Canillas wrote:
> The Documentation/devicetree/bindings/iio/proximity/as3935.txt DT binding
> doc lists "ams,as3935" as a compatible string but the corresponding driver
> does not have an OF match table. Add the table to the driver so the SPI
> core can do an OF style match.
>
> Signed-off-by: Javier Martinez Canillas <[email protected]>
Applied to the togreg branch of iio.git - initially pushed out as testing
for the autobuilders to play with it.
> ---
>
> drivers/iio/proximity/as3935.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/iio/proximity/as3935.c b/drivers/iio/proximity/as3935.c
> index bc0d68efd455..e95035136889 100644
> --- a/drivers/iio/proximity/as3935.c
> +++ b/drivers/iio/proximity/as3935.c
> @@ -434,6 +434,12 @@ static int as3935_remove(struct spi_device *spi)
> return 0;
> }
>
> +static const struct of_device_id as3935_of_match[] = {
> + { .compatible = "ams,as3935", },
> + { /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, as3935_of_match);
> +
> static const struct spi_device_id as3935_id[] = {
> {"as3935", 0},
> {},
> @@ -443,6 +449,7 @@ MODULE_DEVICE_TABLE(spi, as3935_id);
> static struct spi_driver as3935_driver = {
> .driver = {
> .name = "as3935",
> + .of_match_table = of_match_ptr(as3935_of_match),
> .owner = THIS_MODULE,
> .pm = AS3935_PM_OPS,
> },
>
On 20/08/15 08:07, Javier Martinez Canillas wrote:
> The Documentation/devicetree/bindings/iio/adc/ti-adc128s052.txt DT binding
> doc lists "ti,adc128s052" or "ti,adc122s021" as compatible strings but the
> corresponding driver does not have an OF match table. Add the table to the
> driver so the SPI core can do an OF style match.
>
> Signed-off-by: Javier Martinez Canillas <[email protected]>
Applied to the togreg branch of iio.git - pushed out as testing for the
autobuilders to play with it.
Thanks,
Jonathan
> ---
>
> drivers/iio/adc/ti-adc128s052.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/iio/adc/ti-adc128s052.c b/drivers/iio/adc/ti-adc128s052.c
> index 915be6b60097..98c0d2b444bf 100644
> --- a/drivers/iio/adc/ti-adc128s052.c
> +++ b/drivers/iio/adc/ti-adc128s052.c
> @@ -174,6 +174,13 @@ static int adc128_remove(struct spi_device *spi)
> return 0;
> }
>
> +static const struct of_device_id adc128_of_match[] = {
> + { .compatible = "ti,adc128s052", },
> + { .compatible = "ti,adc122s021", },
> + { /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, adc128_of_match);
> +
> static const struct spi_device_id adc128_id[] = {
> { "adc128s052", 0}, /* index into adc128_config */
> { "adc122s021", 1},
> @@ -184,6 +191,7 @@ MODULE_DEVICE_TABLE(spi, adc128_id);
> static struct spi_driver adc128_driver = {
> .driver = {
> .name = "adc128s052",
> + .of_match_table = of_match_ptr(adc128_of_match),
> .owner = THIS_MODULE,
> },
> .probe = adc128_probe,
>
On 20/08/15 16:49, Lars-Peter Clausen wrote:
> On 08/20/2015 09:07 AM, Javier Martinez Canillas wrote:
>> The Documentation/devicetree/bindings/iio/frequency/adf4350.txt DT binding
>> doc lists "adi,adf4350" or "adi,adf4351" as compatible strings but the
>> corresponding driver does not have an OF match table. Add the table to the
>> driver so the SPI core can do an OF style match.
>>
>> Signed-off-by: Javier Martinez Canillas <[email protected]>
>
> Acked-by: Lars-Peter Clausen <[email protected]>
Applied to the togreg branch of iio.git
Thanks,
>
> Thanks.
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
On 20/08/15 16:49, Lars-Peter Clausen wrote:
> On 08/20/2015 09:07 AM, Javier Martinez Canillas wrote:
>> The Documentation/devicetree/bindings/iio/dac/ad7303.txt DT binding doc
>> lists "adi,ad7303" as a compatible string but the corresponding driver
>> does not have an OF match table. Add the table to the driver so the SPI
>> core can do an OF style match.
>>
>> Signed-off-by: Javier Martinez Canillas <[email protected]>
>
>
> Acked-by: Lars-Peter Clausen <[email protected]>
Applied to the togreg branch of iio.git - pushed out as testing.
Thanks,
Jonathan
>
> Thanks.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
On 20/08/15 16:49, Lars-Peter Clausen wrote:
> On 08/20/2015 09:07 AM, Javier Martinez Canillas wrote:
>> The SPI core always reports the MODALIAS uevent as "spi:<modalias>"
>> regardless of the mechanism that was used to register the device
>> (i.e: OF or board code) and the table that is used later to match
>> the driver with the device (i.e: SPI id table or OF match table).
>>
>> So drivers needs to export the SPI id table and this be built into
>> the module or udev won't have the necessary information to autoload
>> the needed driver module when the device is added.
>>
>> Signed-off-by: Javier Martinez Canillas <[email protected]>
>
> Acked-by: Lars-Peter Clausen <[email protected]>
Applied to the togreg branch of iio.git - initially pushed out as testing.
Thanks,
Jonathan
>
> Thanks.
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
On 21/08/15 00:53, Michael Welling wrote:
> On Fri, Aug 21, 2015 at 12:48:23AM +0200, Javier Martinez Canillas wrote:
>> Hello Michael,
>>
>> On 08/21/2015 12:29 AM, Michael Welling wrote:
>>> On Fri, Aug 21, 2015 at 12:02:40AM +0200, Javier Martinez Canillas wrote:
>>>> Hello Michael,
>>>>
>>>> On 08/20/2015 10:09 PM, Michael Welling wrote:
>>>>> On Thu, Aug 20, 2015 at 09:07:26AM +0200, Javier Martinez Canillas wrote:
>>>>>> The driver has an OF id table but the .of_match_table is not set so
>>>>>> the SPI core can't do an OF style match and the table was unused.
>>>>>>
>>>>>
>>>>> Is an OF style match necessary?
>>>>>
>>>>
>>>> Did you read the cover letter [0] on which I explain why is needed to
>>>> avoid breaking module autoloading in the future? Once the SPI core is
>>>> changed by RFC patch 18/18? (you were cc'ed in the cover letter BTW).
>>>
>>> Well I have read it now. :)
>>>
>>
>> Great :)
>>
>>>>
>>>>> I have been using devicetree and it matches based on the .id_table.
>>>>>
>>>>
>>>> Yes it fallbacks to the .id_table or the driver name but the correct
>>>> thing to do for devices registered by OF, is to match using the
>>>> compatible string.
>>>>
>>>>> Couldn't we just remove the mcp320x_dt_ids table instead?
>>>>>
>>>>
>>>> No, that is the wrong thing to do IMHO since the compatible string
>>>> contains both vendor and device name whle the .id_table only contains
>>>> a device name.
>>>>
>>>> So it makes sense to match using the compatible string and also report
>>>> the OF modalias information to user-space.
>>>>
>>>> Otherwise what's the point of the vendor in the compatible string for
>>>> SPI devices? You can just use "bar" instead of "foo,bar" as a string.
>>>>
>>>
>>> Well then shouldn't the patch include adding the vendor to the compatible
>>> string?
>>>
>>
>> Well, I was talking in general. You are right that this specific driver does
>> not have a vendor prefix for the compatible strings. This is incorrect
>> according to the ePAPR document [0].
>>
>> However, these compatible strings are already documented as a DT binding doc
>> in Documentation/devicetree/bindings/iio/adc/mcp320x.txt so I don't know
>> what is the correct thing to do in this situation.
>>
>> Changing the compatible string will break old DTB with newer kernels and that
>> is a no go. But that doesn't invalidate what I said since once the SPI core
>> is changed and report OF modalias, you will need a MODULE_DEVICE_TABLE(of,...)
>> regardless if the compatible string has a vendor prefix or not.
>>
>
> Okay.
>
> Acked-by: Michael Welling <[email protected]>
Applied to the togreg branch of iio.git
Thanks,
Jonathan
>
>> [0]: https://www.power.org/documentation/epapr-version-1-1/
>>
>> Best regards,
>> --
>> Javier Martinez Canillas
>> Open Source Group
>> Samsung Research America
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
On Sat, Aug 22, 2015 at 12:48 PM, Jonathan Cameron <[email protected]> wrote:
> On 20/08/15 23:48, Javier Martinez Canillas wrote:
>> Hello Michael,
>>
>> On 08/21/2015 12:29 AM, Michael Welling wrote:
>>> On Fri, Aug 21, 2015 at 12:02:40AM +0200, Javier Martinez Canillas wrote:
>>>> Hello Michael,
>>>>
>>>> On 08/20/2015 10:09 PM, Michael Welling wrote:
>>>>> On Thu, Aug 20, 2015 at 09:07:26AM +0200, Javier Martinez Canillas wrote:
>>>>>> The driver has an OF id table but the .of_match_table is not set so
>>>>>> the SPI core can't do an OF style match and the table was unused.
>>>>>>
>>>>>
>>>>> Is an OF style match necessary?
>>>>>
>>>>
>>>> Did you read the cover letter [0] on which I explain why is needed to
>>>> avoid breaking module autoloading in the future? Once the SPI core is
>>>> changed by RFC patch 18/18? (you were cc'ed in the cover letter BTW).
>>>
>>> Well I have read it now. :)
>>>
>>
>> Great :)
>>
>>>>
>>>>> I have been using devicetree and it matches based on the .id_table.
>>>>>
>>>>
>>>> Yes it fallbacks to the .id_table or the driver name but the correct
>>>> thing to do for devices registered by OF, is to match using the
>>>> compatible string.
>>>>
>>>>> Couldn't we just remove the mcp320x_dt_ids table instead?
>>>>>
>>>>
>>>> No, that is the wrong thing to do IMHO since the compatible string
>>>> contains both vendor and device name whle the .id_table only contains
>>>> a device name.
>>>>
>>>> So it makes sense to match using the compatible string and also report
>>>> the OF modalias information to user-space.
>>>>
>>>> Otherwise what's the point of the vendor in the compatible string for
>>>> SPI devices? You can just use "bar" instead of "foo,bar" as a string.
>>>>
>>>
>>> Well then shouldn't the patch include adding the vendor to the compatible
>>> string?
>>>
>>
>> Well, I was talking in general. You are right that this specific driver does
>> not have a vendor prefix for the compatible strings. This is incorrect
>> according to the ePAPR document [0].
>>
>> However, these compatible strings are already documented as a DT binding doc
>> in Documentation/devicetree/bindings/iio/adc/mcp320x.txt so I don't know
>> what is the correct thing to do in this situation.
> Take the view the old version is wrong but needs to be supported and add also
> the corrected strings + document them.
>
> cc'd The device tree list for any more comments on this.
Agreed. Document both and mark the old one deprecated.
Rob
Hello Rob and Jonathan,
On 08/24/2015 12:10 AM, Rob Herring wrote:
> On Sat, Aug 22, 2015 at 12:48 PM, Jonathan Cameron <[email protected]> wrote:
>> On 20/08/15 23:48, Javier Martinez Canillas wrote:
>>> Hello Michael,
>>>
>>> On 08/21/2015 12:29 AM, Michael Welling wrote:
>>>> On Fri, Aug 21, 2015 at 12:02:40AM +0200, Javier Martinez Canillas wrote:
>>>>> Hello Michael,
>>>>>
>>>>> On 08/20/2015 10:09 PM, Michael Welling wrote:
>>>>>> On Thu, Aug 20, 2015 at 09:07:26AM +0200, Javier Martinez Canillas wrote:
>>>>>>> The driver has an OF id table but the .of_match_table is not set so
>>>>>>> the SPI core can't do an OF style match and the table was unused.
>>>>>>>
>>>>>>
>>>>>> Is an OF style match necessary?
>>>>>>
>>>>>
>>>>> Did you read the cover letter [0] on which I explain why is needed to
>>>>> avoid breaking module autoloading in the future? Once the SPI core is
>>>>> changed by RFC patch 18/18? (you were cc'ed in the cover letter BTW).
>>>>
>>>> Well I have read it now. :)
>>>>
>>>
>>> Great :)
>>>
>>>>>
>>>>>> I have been using devicetree and it matches based on the .id_table.
>>>>>>
>>>>>
>>>>> Yes it fallbacks to the .id_table or the driver name but the correct
>>>>> thing to do for devices registered by OF, is to match using the
>>>>> compatible string.
>>>>>
>>>>>> Couldn't we just remove the mcp320x_dt_ids table instead?
>>>>>>
>>>>>
>>>>> No, that is the wrong thing to do IMHO since the compatible string
>>>>> contains both vendor and device name whle the .id_table only contains
>>>>> a device name.
>>>>>
>>>>> So it makes sense to match using the compatible string and also report
>>>>> the OF modalias information to user-space.
>>>>>
>>>>> Otherwise what's the point of the vendor in the compatible string for
>>>>> SPI devices? You can just use "bar" instead of "foo,bar" as a string.
>>>>>
>>>>
>>>> Well then shouldn't the patch include adding the vendor to the compatible
>>>> string?
>>>>
>>>
>>> Well, I was talking in general. You are right that this specific driver does
>>> not have a vendor prefix for the compatible strings. This is incorrect
>>> according to the ePAPR document [0].
>>>
>>> However, these compatible strings are already documented as a DT binding doc
>>> in Documentation/devicetree/bindings/iio/adc/mcp320x.txt so I don't know
>>> what is the correct thing to do in this situation.
>> Take the view the old version is wrong but needs to be supported and add also
>> the corrected strings + document them.
>>
>> cc'd The device tree list for any more comments on this.
>
> Agreed. Document both and mark the old one deprecated.
>
Thanks a lot for your comments. I'll do that as a follow up then.
> Rob
>
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
Hello Mark,
On 08/21/2015 07:29 PM, Mark Brown wrote:
> On Fri, Aug 21, 2015 at 01:47:00AM +0200, Javier Martinez Canillas wrote:
>> On 08/21/2015 01:25 AM, Mark Brown wrote:
>>> On Thu, Aug 20, 2015 at 11:45:09PM +0200, Javier Martinez Canillas wrote:
>
>>>>> IOW, it's labeled as such mostly for safety, since it has quite a few
>>>>> distributed dependencies.
>
>>> Are there really only 17 drivers that are missing an explict of_table?
>>> That seems like a low number.
>
>> In fact the 17 patches are the combination of the SPI drivers that:
>
>> a) Have a .id_table but not a MODULE_DEVICE_TABLE(spi,...)
>> b) Have a .of_match_table but no a MODULE_DEVICE_TABLE(of,...)
>> c) Don't have a .of_match_table but have a DT binding document
>
>> Maybe there are more SPI drivers out there that only have a .id_table
>> and don't have a .of_match_table nor a DT binding doc. But in that case
>> there isn't too much I can do since I've no information that these are
>> drivers are actually used in systems booted with OF.
>
> We could at the very least scan through the in tree DTs. There does
> seem to be a substantial overlap between systems that often don't use
> modular kernels but could and systems where people are using the I2C
> and SPI ID mapping shims.
>
My script does this but didn't find any matches.
Basically what I do is for drivers with no OF table, to take the name field
from the spi_device_id in the SPI id table and see if one of these are used
as a compatible string either in a DTS or mentioned in a DT binding.
I found some matches in DT bindings which are the type c) issue but none
used in an in tree DTS.
But that doesn't mean that there could be drivers with no OF table, no DT
binding and with out-of-tree DTS that are using compatible = "bar" instead
compatible = "foo,bar" and relying on SPI matching using the SPI device id
table as a fallback. But I guess these should be fixed for module autoload.
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
On Thu, 20 Aug 2015, Javier Martinez Canillas wrote:
> The Documentation/devicetree/bindings/mfd/stmpe.txt DT binding doc lists
> "st,stmpe[610|801|811|1601|2401|2403]" as valid compatible strings but
> the corresponding driver does not have an OF match table. Add the table
> to the driver so the SPI core can do an OF style match.
>
> Signed-off-by: Javier Martinez Canillas <[email protected]>
> ---
>
> drivers/mfd/stmpe-spi.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
Applied, thanks.
> diff --git a/drivers/mfd/stmpe-spi.c b/drivers/mfd/stmpe-spi.c
> index 6fdb30e84a2b..618ba244d98a 100644
> --- a/drivers/mfd/stmpe-spi.c
> +++ b/drivers/mfd/stmpe-spi.c
> @@ -11,6 +11,7 @@
> #include <linux/interrupt.h>
> #include <linux/kernel.h>
> #include <linux/module.h>
> +#include <linux/of.h>
> #include <linux/types.h>
> #include "stmpe.h"
>
> @@ -108,6 +109,17 @@ static int stmpe_spi_remove(struct spi_device *spi)
> return stmpe_remove(stmpe);
> }
>
> +static const struct of_device_id stmpe_spi_of_match[] = {
> + { .compatible = "st,stmpe610", },
> + { .compatible = "st,stmpe801", },
> + { .compatible = "st,stmpe811", },
> + { .compatible = "st,stmpe1601", },
> + { .compatible = "st,stmpe2401", },
> + { .compatible = "st,stmpe2403", },
> + { /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, stmpe_spi_of_match);
> +
> static const struct spi_device_id stmpe_spi_id[] = {
> { "stmpe610", STMPE610 },
> { "stmpe801", STMPE801 },
> @@ -122,6 +134,7 @@ MODULE_DEVICE_TABLE(spi, stmpe_id);
> static struct spi_driver stmpe_spi_driver = {
> .driver = {
> .name = "stmpe-spi",
> + .of_match_table = of_match_ptr(stmpe_spi_of_match),
> .owner = THIS_MODULE,
> #ifdef CONFIG_PM
> .pm = &stmpe_dev_pm_ops,
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
On Thu, 20 Aug 2015, Javier Martinez Canillas wrote:
> The Documentation/devicetree/bindings/mfd/cros-ec.txt DT binding doc lists
> "google,cros-ec-spi" as a compatible string but the corresponding driver
> does not have an OF match table. Add the table to the driver so the SPI
> core can do an OF style match.
>
> Signed-off-by: Javier Martinez Canillas <[email protected]>
> ---
>
> drivers/mfd/cros_ec_spi.c | 7 +++++++
> 1 file changed, 7 insertions(+)
Applied, thanks.
> diff --git a/drivers/mfd/cros_ec_spi.c b/drivers/mfd/cros_ec_spi.c
> index 16f228dc243f..30a296b4e748 100644
> --- a/drivers/mfd/cros_ec_spi.c
> +++ b/drivers/mfd/cros_ec_spi.c
> @@ -701,6 +701,12 @@ static int cros_ec_spi_resume(struct device *dev)
> static SIMPLE_DEV_PM_OPS(cros_ec_spi_pm_ops, cros_ec_spi_suspend,
> cros_ec_spi_resume);
>
> +static const struct of_device_id cros_ec_spi_of_match[] = {
> + { .compatible = "google,cros-ec-spi", },
> + { /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, cros_ec_spi_of_match);
> +
> static const struct spi_device_id cros_ec_spi_id[] = {
> { "cros-ec-spi", 0 },
> { }
> @@ -710,6 +716,7 @@ MODULE_DEVICE_TABLE(spi, cros_ec_spi_id);
> static struct spi_driver cros_ec_driver_spi = {
> .driver = {
> .name = "cros-ec-spi",
> + .of_match_table = of_match_ptr(cros_ec_spi_of_match),
> .owner = THIS_MODULE,
> .pm = &cros_ec_spi_pm_ops,
> },
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
Hello Jonathan,
On 08/22/2015 07:59 PM, Jonathan Cameron wrote:
> On 20/08/15 08:07, Javier Martinez Canillas wrote:
>> The SPI core always reports the MODALIAS uevent as "spi:<modalias>"
>> regardless of the mechanism that was used to register the device
>> (i.e: OF or board code) and the table that is used later to match
>> the driver with the device (i.e: SPI id table or OF match table).
>>
>> So drivers needs to export the SPI id table and this be built into
>> the module or udev won't have the necessary information to autoload
>> the needed driver module when the device is added.
>>
>> Signed-off-by: Javier Martinez Canillas <[email protected]>
> Applied to the togreg branch of iio.git.
>
> This is too late for the upcoming merge window so it will be queued up for
> the next one.
>
IMHO this patch and "[PATCH 01/18] iio: Export SPI module alias
information in missing drivers" [0] are fixing broken module
autoloading which are bugs so are material for the 4.3 -rc cycle.
> Thanks,
>
> Jonathan
[0]: https://lkml.org/lkml/2015/8/20/111
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America