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

124 more lines

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 <redacted>



---

 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

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 <redacted>

---

 drivers/video/backlight/tosa_bl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/video/backlight/tosa_bl.c b/drivers/video/backlight/tosa_bl.c
index 3ad676558c80..83742d806391 100644
--- a/drivers/video/backlight/tosa_bl.c
+++ b/drivers/video/backlight/tosa_bl.c

@@ -158,6 +158,7 @@ static const struct i2c_device_id tosa_bl_id[] = {
 	{ "tosa-bl", 0 },
 	{ },
 };
+MODULE_DEVICE_TABLE(i2c, tosa_bl_id);
 
 static struct i2c_driver tosa_bl_driver = {
 	.driver = {

-- 
2.4.3

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

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:

quoted

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

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:

quoted

Hi Javier,

On Thu, Jul 30, 2015 at 06:18:25PM +0200, Javier Martinez Canillas wrote:

quoted

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