[alsa-devel] [PATCH 00/27] Export I2C and OF module aliases in missing drivers
Hello,
Short version:
This series add the missing MODULE_DEVICE_TABLE() for OF and I2C tables to export that information so modules have the correct aliases built-in and autoloading works correctly.
Longer version:
Currently it's mandatory for I2C drivers to have an I2C device ID table regardless if the device was registered using platform data or OF. This is because the I2C core needs an I2C device ID table for two reasons:
1) Match the I2C client with a I2C device ID so a struct i2c_device_id is passed to the I2C driver probe() function.
2) Export the module aliases from the I2C device ID table so userspace can auto-load the correct module. This is because i2c_device_uevent always reports a MODALIAS of the form i2c:<client->name>.
Lee Jones posted a patch series [0] to solve 1) by allowing the I2C drivers to have a probe() function that does not get a i2c_device_id.
The problem is that his series didn't take into account 2) so if that was merged and the I2C ID table is removed from all the drivers that don't needed it, module auto-loading will break for those.
But even now there are many I2C drivers were module auto-loading is not working because of the fact that the I2C core always reports the MODALIAS as i2c:<client->name> and many developers didn't expect this.
I've identified I2C drivers with 3 types of different issues:
a) Those that have an i2c_table but are not exported. The match works and the correct i2c_device_id is passed on probe 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 I2C core. But if the I2C 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 a of_table but should since are OF drivers with DT bindings doc for them. Since the I2C core does not report a OF modalias and since i2c_device_match() fallbacks to match the device part of the compatible string with the I2C device ID table, many OF drivers don't have an of_table to match. After all having a I2C device ID table is mandatory so it works without a of_table.
So, in order to not make mandatory to have a I2C device ID table, at least a) and b) needs to be addressed, this series does that.
c) should be fixed too since it seems wrong that a driver with a DT binding document, does not have a OF table and export it to modules.
Also stripping the vendor part from the compatible string to match with the I2C devices ID table and reporting only the device name to user-space doesn't seem to be correct. I've identified at least two drivers that have the same name on their I2C device ID table so the manufacturer prefix is important. But I've not tried to fix c) yet since that is not so easy to automate due drivers not having all the information (i.e: the device name can match a documented compatible string device part but without the vendor prefix is hard to tell).
I split the changes so the patches in this series are independent and can be picked individually by subsystem maintainers. Patch #27 changes the logic of i2c_device_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 fixing c) will break module auto-loading for the drivers of devices registered with OF but that don't have a of_match_table.
Although arguably, those drivers were relying on the assumption that a MODALIAS=i2c:<foo> would always be reported even for the OF case which is not the true on other subsystems.
[0]: https://lkml.org/lkml/2014/8/28/283
Best regards, Javier
Javier Martinez Canillas (27): mfd: stw481x: Export I2C module alias information spi: xcomm: Export I2C module alias information iio: Export I2C module alias information in missing drivers [media] Export I2C module alias information in missing drivers macintosh: therm_windtunnel: Export I2C module alias information misc: eeprom: Export I2C module alias information in missing drivers Input: Export I2C module alias information in missing drivers power: Export I2C module alias information in missing drivers i2c: core: Export I2C module alias information in dummy driver backlight: tosa: Export I2C module alias information [media] staging: media: lirc: Export I2C module alias information usb: phy: isp1301: Export I2C module alias information ALSA: ppc: keywest: Export I2C module alias information hwmon: (nct7904) Export I2C module alias information regulator: fan53555: Export I2C module alias information mfd: Export OF module alias information in missing drivers iio: Export OF module alias information in missing drivers hwmon: (g762) Export OF module alias information extcon: Export OF module alias information in missing drivers ASoC: Export OF module alias information in missing codec drivers rtc: Export OF module alias information in missing drivers macintosh: therm_windtunnel: Export OF module alias information leds: Export OF module alias information in missing drivers [media] smiapp: Export OF module alias information Input: touchscreen - Export OF module alias information regulator: isl9305: Export OF module alias information i2c: (RFC, don't apply) report OF style modalias when probing using DT
drivers/extcon/extcon-rt8973a.c | 1 + drivers/extcon/extcon-sm5502.c | 1 + drivers/hwmon/g762.c | 1 + drivers/hwmon/nct7904.c | 1 + drivers/i2c/i2c-core.c | 9 +++++++++ drivers/iio/accel/mma8452.c | 1 + drivers/iio/accel/stk8312.c | 1 + drivers/iio/accel/stk8ba50.c | 1 + drivers/iio/light/cm32181.c | 1 + drivers/iio/light/cm3232.c | 1 + drivers/iio/light/cm36651.c | 1 + drivers/iio/light/gp2ap020a00f.c | 1 + drivers/iio/light/stk3310.c | 1 + drivers/input/misc/gp2ap002a00f.c | 1 + drivers/input/touchscreen/egalax_ts.c | 1 + drivers/input/touchscreen/goodix.c | 1 + drivers/input/touchscreen/mms114.c | 1 + drivers/leds/leds-pca963x.c | 1 + drivers/leds/leds-tca6507.c | 1 + drivers/macintosh/therm_windtunnel.c | 2 ++ drivers/media/i2c/ir-kbd-i2c.c | 1 + drivers/media/i2c/s5k6a3.c | 1 + drivers/media/i2c/smiapp/smiapp-core.c | 1 + drivers/mfd/rt5033.c | 1 + drivers/mfd/stw481x.c | 1 + drivers/mfd/tps65217.c | 1 + drivers/mfd/tps65218.c | 1 + drivers/misc/eeprom/eeprom.c | 1 + drivers/misc/eeprom/max6875.c | 1 + drivers/power/bq24190_charger.c | 1 + drivers/power/rt5033_battery.c | 2 +- drivers/regulator/fan53555.c | 1 + drivers/regulator/isl9305.c | 1 + drivers/rtc/rtc-ab-b5ze-s3.c | 1 + drivers/rtc/rtc-isl12022.c | 1 + drivers/rtc/rtc-isl12057.c | 1 + drivers/spi/spi-xcomm.c | 1 + drivers/staging/media/lirc/lirc_zilog.c | 1 + drivers/usb/phy/phy-isp1301.c | 1 + drivers/video/backlight/tosa_bl.c | 1 + sound/ppc/keywest.c | 1 + sound/soc/codecs/da9055.c | 1 + sound/soc/codecs/wm8510.c | 1 + sound/soc/codecs/wm8523.c | 1 + sound/soc/codecs/wm8580.c | 1 + 45 files changed, 54 insertions(+), 1 deletion(-)
The I2C core always reports the MODALIAS uevent as "i2c:<client name" regardless if the driver was matched using the I2C id_table or the of_match_table. So the driver needs to export the I2C table and this be built into the module or udev won't have the necessary information to auto load the correct module when the device is added.
Signed-off-by: Javier Martinez Canillas javier@osg.samsung.com
---
sound/ppc/keywest.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/sound/ppc/keywest.c b/sound/ppc/keywest.c index 6120a067494a..f644a8c57e0a 100644 --- a/sound/ppc/keywest.c +++ b/sound/ppc/keywest.c @@ -101,6 +101,7 @@ static const struct i2c_device_id keywest_i2c_id[] = { { "keywest", 0 }, /* instantiated by us if needed */ { } }; +MODULE_DEVICE_TABLE(i2c, keywest_i2c_id);
static struct i2c_driver keywest_driver = { .driver = {
The I2C core always reports the MODALIAS uevent as "i2c:<client name" regardless if the driver was matched using the I2C id_table or the of_match_table. So technically there's no need for a driver to export the OF table since currently it's not used.
In fact, the I2C device ID table is mandatory for I2C drivers since a i2c_device_id is passed to the driver's probe function even if the I2C core used the OF table to match the driver.
And since the I2C core uses different tables, OF-only drivers needs to have duplicated data that has to be kept in sync and also the dev node compatible manufacturer prefix is stripped when reporting the MODALIAS.
To avoid the above, the I2C core behavior may be changed in the future to not require an I2C device table for OF-only drivers and report the OF module alias. So, it's better to also export the OF table to prevent breaking module autoloading if that happens.
Signed-off-by: Javier Martinez Canillas javier@osg.samsung.com
---
sound/soc/codecs/da9055.c | 1 + sound/soc/codecs/wm8510.c | 1 + sound/soc/codecs/wm8523.c | 1 + sound/soc/codecs/wm8580.c | 1 + 4 files changed, 4 insertions(+)
diff --git a/sound/soc/codecs/da9055.c b/sound/soc/codecs/da9055.c index 04c76f37e1fd..19635d830b47 100644 --- a/sound/soc/codecs/da9055.c +++ b/sound/soc/codecs/da9055.c @@ -1533,6 +1533,7 @@ static const struct of_device_id da9055_of_match[] = { { .compatible = "dlg,da9055-codec", }, { } }; +MODULE_DEVICE_TABLE(of, da9055_of_match);
/* I2C codec control layer */ static struct i2c_driver da9055_i2c_driver = { diff --git a/sound/soc/codecs/wm8510.c b/sound/soc/codecs/wm8510.c index 3cff5a699e57..b098a83a44d8 100644 --- a/sound/soc/codecs/wm8510.c +++ b/sound/soc/codecs/wm8510.c @@ -598,6 +598,7 @@ static const struct of_device_id wm8510_of_match[] = { { .compatible = "wlf,wm8510" }, { }, }; +MODULE_DEVICE_TABLE(of, wm8510_of_match);
static const struct regmap_config wm8510_regmap = { .reg_bits = 7, diff --git a/sound/soc/codecs/wm8523.c b/sound/soc/codecs/wm8523.c index 5f8fde56e68b..aa287a3965e7 100644 --- a/sound/soc/codecs/wm8523.c +++ b/sound/soc/codecs/wm8523.c @@ -430,6 +430,7 @@ static const struct of_device_id wm8523_of_match[] = { { .compatible = "wlf,wm8523" }, { }, }; +MODULE_DEVICE_TABLE(of, wm8523_of_match);
static const struct regmap_config wm8523_regmap = { .reg_bits = 8, diff --git a/sound/soc/codecs/wm8580.c b/sound/soc/codecs/wm8580.c index abf60355f7f7..66602bf02f6e 100644 --- a/sound/soc/codecs/wm8580.c +++ b/sound/soc/codecs/wm8580.c @@ -916,6 +916,7 @@ static const struct of_device_id wm8580_of_match[] = { { .compatible = "wlf,wm8580" }, { }, }; +MODULE_DEVICE_TABLE(of, wm8580_of_match);
static const struct regmap_config wm8580_regmap = { .reg_bits = 7,
The patch
ASoC: Export OF module alias information in missing codec drivers
has been applied to the asoc tree at
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From bf08f39e6088c52c6fc7cce2ef7fbbd7bf4692b9 Mon Sep 17 00:00:00 2001
From: Javier Martinez Canillas javier@osg.samsung.com Date: Thu, 30 Jul 2015 18:18:45 +0200 Subject: [PATCH] ASoC: Export OF module alias information in missing codec drivers
The I2C core always reports the MODALIAS uevent as "i2c:<client name" regardless if the driver was matched using the I2C id_table or the of_match_table. So technically there's no need for a driver to export the OF table since currently it's not used.
In fact, the I2C device ID table is mandatory for I2C drivers since a i2c_device_id is passed to the driver's probe function even if the I2C core used the OF table to match the driver.
And since the I2C core uses different tables, OF-only drivers needs to have duplicated data that has to be kept in sync and also the dev node compatible manufacturer prefix is stripped when reporting the MODALIAS.
To avoid the above, the I2C core behavior may be changed in the future to not require an I2C device table for OF-only drivers and report the OF module alias. So, it's better to also export the OF table to prevent breaking module autoloading if that happens.
Signed-off-by: Javier Martinez Canillas javier@osg.samsung.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/codecs/da9055.c | 1 + sound/soc/codecs/wm8510.c | 1 + sound/soc/codecs/wm8523.c | 1 + sound/soc/codecs/wm8580.c | 1 + 4 files changed, 4 insertions(+)
diff --git a/sound/soc/codecs/da9055.c b/sound/soc/codecs/da9055.c index 66bb446..ab39de6 100644 --- a/sound/soc/codecs/da9055.c +++ b/sound/soc/codecs/da9055.c @@ -1533,6 +1533,7 @@ static const struct of_device_id da9055_of_match[] = { { .compatible = "dlg,da9055-codec", }, { } }; +MODULE_DEVICE_TABLE(of, da9055_of_match);
/* I2C codec control layer */ static struct i2c_driver da9055_i2c_driver = { diff --git a/sound/soc/codecs/wm8510.c b/sound/soc/codecs/wm8510.c index dac5beb..0617415 100644 --- a/sound/soc/codecs/wm8510.c +++ b/sound/soc/codecs/wm8510.c @@ -598,6 +598,7 @@ static const struct of_device_id wm8510_of_match[] = { { .compatible = "wlf,wm8510" }, { }, }; +MODULE_DEVICE_TABLE(of, wm8510_of_match);
static const struct regmap_config wm8510_regmap = { .reg_bits = 7, diff --git a/sound/soc/codecs/wm8523.c b/sound/soc/codecs/wm8523.c index 43ea8ae..483ec72 100644 --- a/sound/soc/codecs/wm8523.c +++ b/sound/soc/codecs/wm8523.c @@ -430,6 +430,7 @@ static const struct of_device_id wm8523_of_match[] = { { .compatible = "wlf,wm8523" }, { }, }; +MODULE_DEVICE_TABLE(of, wm8523_of_match);
static const struct regmap_config wm8523_regmap = { .reg_bits = 8, diff --git a/sound/soc/codecs/wm8580.c b/sound/soc/codecs/wm8580.c index 759a792..a9f3900 100644 --- a/sound/soc/codecs/wm8580.c +++ b/sound/soc/codecs/wm8580.c @@ -916,6 +916,7 @@ static const struct of_device_id wm8580_of_match[] = { { .compatible = "wlf,wm8580" }, { }, }; +MODULE_DEVICE_TABLE(of, wm8580_of_match);
static const struct regmap_config wm8580_regmap = { .reg_bits = 7,
An I2C driver that supports both OF and legacy platforms, will have both a OF and I2C ID table. This means that when built as a module, the aliases will be filled from both tables but currently always an alias of the form i2c:<deviceId> is reported, e.g:
$ cat /sys/class/i2c-adapter/i2c-8/8-004b/modalias i2c:maxtouch
So if a device is probed by matching its compatible string, udev can get a MODALIAS uevent env var that doesn't match with one of the valid aliases so the module won't be auto-loaded.
This patch changes the I2C core to report a OF related MODALIAS uevent (of:N*T*C) env var instead so the module can be auto-loaded and also report the correct alias using sysfs:
$ cat /sys/class/i2c-adapter/i2c-8/8-004b/modalias of:NtrackpadT<NULL>Catmel,maxtouch
Signed-off-by: Javier Martinez Canillas javier@osg.samsung.com
---
drivers/i2c/i2c-core.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c index 92dddfeb3f39..c0668c2ed9da 100644 --- a/drivers/i2c/i2c-core.c +++ b/drivers/i2c/i2c-core.c @@ -489,6 +489,10 @@ static int i2c_device_uevent(struct device *dev, struct kobj_uevent_env *env) struct i2c_client *client = to_i2c_client(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; @@ -726,6 +730,10 @@ show_modalias(struct device *dev, struct device_attribute *attr, char *buf) struct i2c_client *client = to_i2c_client(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;
Hi Javier,
On Thu, Jul 30, 2015 at 06:18:25PM +0200, Javier Martinez Canillas wrote:
Hello,
Short version:
This series add the missing MODULE_DEVICE_TABLE() for OF and I2C tables to export that information so modules have the correct aliases built-in and autoloading works correctly.
Longer version:
Currently it's mandatory for I2C drivers to have an I2C device ID table regardless if the device was registered using platform data or OF. This is because the I2C core needs an I2C device ID table for two reasons:
Match the I2C client with a I2C device ID so a struct i2c_device_id is passed to the I2C driver probe() function.
Export the module aliases from the I2C device ID table so userspace can auto-load the correct module. This is because i2c_device_uevent always reports a MODALIAS of the form i2c:<client->name>.
Why are we not fixing this? We emit specially carved uevent for ACPI-based devices, why not the same for OF? Platform bus does this...
Thanks.
On Thu, Jul 30, 2015 at 09:35:17AM -0700, Dmitry Torokhov wrote:
Hi Javier,
On Thu, Jul 30, 2015 at 06:18:25PM +0200, Javier Martinez Canillas wrote:
Hello,
Short version:
This series add the missing MODULE_DEVICE_TABLE() for OF and I2C tables to export that information so modules have the correct aliases built-in and autoloading works correctly.
Longer version:
Currently it's mandatory for I2C drivers to have an I2C device ID table regardless if the device was registered using platform data or OF. This is because the I2C core needs an I2C device ID table for two reasons:
Match the I2C client with a I2C device ID so a struct i2c_device_id is passed to the I2C driver probe() function.
Export the module aliases from the I2C device ID table so userspace can auto-load the correct module. This is because i2c_device_uevent always reports a MODALIAS of the form i2c:<client->name>.
Why are we not fixing this? We emit specially carved uevent for ACPI-based devices, why not the same for OF? Platform bus does this...
Ah, now I see the 27/27 patch. I think it is exactly what we need. And probably for SPI bus as well.
Thanks.
Hello Dmitry,
Thanks a lot for your feedback.
On 07/30/2015 06:37 PM, Dmitry Torokhov wrote:
On Thu, Jul 30, 2015 at 09:35:17AM -0700, Dmitry Torokhov wrote:
Hi Javier,
On Thu, Jul 30, 2015 at 06:18:25PM +0200, Javier Martinez Canillas wrote:
Hello,
Short version:
This series add the missing MODULE_DEVICE_TABLE() for OF and I2C tables to export that information so modules have the correct aliases built-in and autoloading works correctly.
Longer version:
Currently it's mandatory for I2C drivers to have an I2C device ID table regardless if the device was registered using platform data or OF. This is because the I2C core needs an I2C device ID table for two reasons:
Match the I2C client with a I2C device ID so a struct i2c_device_id is passed to the I2C driver probe() function.
Export the module aliases from the I2C device ID table so userspace can auto-load the correct module. This is because i2c_device_uevent always reports a MODALIAS of the form i2c:<client->name>.
Why are we not fixing this? We emit specially carved uevent for ACPI-based devices, why not the same for OF? Platform bus does this...
Ah, now I see the 27/27 patch. I think it is exactly what we need. And
Yes, patch 27/27 is needed but the problem is as I explained before that there are drivers relying on the current behavior. The item c) in the list of issues that I mentioned. So those drivers need to be fixed before that patch is merged...
probably for SPI bus as well.
Yes, I didn't mention SPI because the cover letter became too long already but it does indeed have the same issue and I discussed this with Mark already some time ago [0].
Once I2C uevent report is fixed, I plan to do the same for SPI.
Thanks.
[0]: https://lkml.org/lkml/2014/9/11/458
Best regards,
participants (3)
-
Dmitry Torokhov -
Javier Martinez Canillas -
Mark Brown