[PATCH 00/27] Export I2C and OF module aliases in missing drivers

[PATCH 00/27] Export I2C and OF module aliases in missing drivers

[Javier Martinez Canillas]

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(-)

-- 
2.4.3

[PATCH 04/27] [media] Export I2C module alias information in missing drivers

[Javier Martinez Canillas]

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>

---

 drivers/media/i2c/ir-kbd-i2c.c | 1 +
 drivers/media/i2c/s5k6a3.c     | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/media/i2c/ir-kbd-i2c.c b/drivers/media/i2c/ir-kbd-i2c.c
index 175a76114953..728d2cc8a3e7 100644
--- a/drivers/media/i2c/ir-kbd-i2c.c
+++ b/drivers/media/i2c/ir-kbd-i2c.c
@@ -478,6 +478,7 @@ static const struct i2c_device_id ir_kbd_id[] = {
 	{ "ir_rx_z8f0811_hdpvr", 0 },
 	{ }
 };
+MODULE_DEVICE_TABLE(i2c, ir_kbd_id);
 
 static struct i2c_driver ir_kbd_driver = {
 	.driver = {
diff --git a/drivers/media/i2c/s5k6a3.c b/drivers/media/i2c/s5k6a3.c
index bc389d5e42ae..b1b1574dfb95 100644
--- a/drivers/media/i2c/s5k6a3.c
+++ b/drivers/media/i2c/s5k6a3.c
@@ -363,6 +363,7 @@ static int s5k6a3_remove(struct i2c_client *client)
 static const struct i2c_device_id s5k6a3_ids[] = {
 	{ }
 };
+MODULE_DEVICE_TABLE(i2c, s5k6a3_ids);
 
 #ifdef CONFIG_OF
 static const struct of_device_id s5k6a3_of_match[] = {
-- 
2.4.3

[PATCH 11/27] [media] staging: media: lirc: Export I2C module alias information

[Javier Martinez Canillas]

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>

---

 drivers/staging/media/lirc/lirc_zilog.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/media/lirc/lirc_zilog.c b/drivers/staging/media/lirc/lirc_zilog.c
index 261e27d6b054..bebfc3b093bc 100644
--- a/drivers/staging/media/lirc/lirc_zilog.c
+++ b/drivers/staging/media/lirc/lirc_zilog.c
@@ -1364,6 +1364,7 @@ static const struct i2c_device_id ir_transceiver_id[] = {
 	{ "ir_rx_z8f0811_hdpvr", ID_FLAG_HDPVR              },
 	{ }
 };
+MODULE_DEVICE_TABLE(i2c, ir_transceiver_id);
 
 static struct i2c_driver driver = {
 	.driver = {
-- 
2.4.3

[PATCH 24/27] [media] smiapp: Export OF module alias information

[Javier Martinez Canillas]

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>

---

 drivers/media/i2c/smiapp/smiapp-core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c
index 636ebd6fe5dc..fb39dfd55e75 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -3131,6 +3131,7 @@ static const struct of_device_id smiapp_of_table[] = {
 	{ .compatible = "nokia,smia" },
 	{ },
 };
+MODULE_DEVICE_TABLE(of, smiapp_of_table);
 
 static const struct i2c_device_id smiapp_id_table[] = {
 	{ SMIAPP_NAME, 0 },
-- 
2.4.3

[PATCH 27/27] i2c: (RFC, don't apply) report OF style modalias when probing using DT

[Javier Martinez Canillas]

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;
-- 
2.4.3

Re: [PATCH 00/27] Export I2C and OF module aliases in missing drivers

[Dmitry Torokhov]

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:
> 
> 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>.

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.

-- 
Dmitry

Re: [PATCH 00/27] Export I2C and OF module aliases in missing drivers

[Dmitry Torokhov]

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:
> > 
> > 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>.
> 
> 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.

-- 
Dmitry

Re: [PATCH 00/27] Export I2C and OF module aliases in missing drivers

[Javier Martinez Canillas]

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:
>>>
>>> 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>.
>>
>> 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,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America