Message-ID: <873479ong5.fsf@ocarina.mail-host-address-is-not-set>
Date: Thu, 23 Oct 2025 13:40:58 +0200
From: Javier Martinez Canillas <javierm@...hat.com>
To: Alexander Sverdlin <alexander.sverdlin@...il.com>, Mark Brown
 <broonie@...nel.org>
Cc: Wolfram Sang <wsa@...-dreams.de>, Herve Codina
 <herve.codina@...tlin.com>, David Rhodes <david.rhodes@...rus.com>,
 Richard Fitzgerald <rf@...nsource.cirrus.com>, Liam Girdwood
 <lgirdwood@...il.com>, Rob Herring <robh@...nel.org>, Krzysztof Kozlowski
 <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, Jaroslav Kysela
 <perex@...ex.cz>, Takashi Iwai <tiwai@...e.com>, Nikita Shubin
 <nikita.shubin@...uefel.me>, Axel Lin <axel.lin@...ics.com>, Brian Austin
 <brian.austin@...rus.com>, linux-sound@...r.kernel.org,
 patches@...nsource.cirrus.com, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, Thomas Petazzoni
 <thomas.petazzoni@...tlin.com>
Subject: Re: [PATCH 1/3] ASoC: cs4271: Fix cs4271 I2C and SPI drivers
 automatic module loading

Alexander Sverdlin <alexander.sverdlin@...il.com> writes:

Hello Alexander,

> Hi Mark,
>
> On Wed, 2025-10-22 at 15:56 +0100, Mark Brown wrote:
>> > > I'm very reluctant to touch this stuff for SPI without some very careful
>> > > analysis that it's not going to cause things to explode on people, right
>> > > now things seem to be working well enough so I'm not clear we'd be
>> > > solving an actual problem.
>> 
>> > The actual problem is that i2c-core is producing "of:" prefixed uevents
>> > instead of "i2c:" prefixed uevents starting from v4.18.
>> 
>> > Most of the dual-bus ASoC CODECs are affected.
>> 
>> That's a description of what change but not of a concrete problem that
>> users are experiencing.
>
> the concrete problem Herve has experienced is that cs4271-i2c will not be
> loaded automatically starting with Linux v4.18 (commit af503716ac14
> "i2c: core: report OF style module alias for devices registered via OF").
>
>> > Now declaring "of:" to be the new I2C bus prefix for uevents starting from
>> > Linux v4.18 sounds strange.
>>

I don't find that strange at all. My opinion is that is the correct
thing to do for the following reasons:

* The struct of_device_id table (and not the struct i2c_device_id table)
  is used to match registered devices through DT / OF with I2C drivers.

* All other bus types but SPI report an MODALIAS=of: for devices that
  are registered through OF.

* I2C (and even SPI) devices registered by ACPI report a MODALIAS=acpi:
  and not a MODALIAS=i2c: or MODALIAS=spi:.

So I would claim that I2C reporting MODALIAS=of: when devices are 
registered through OF are consistent with other buses, using the same
data to both load modules and match drivers and also more consistent
on how the I2C subsystem handles registration through ACPI, OF and pdata.

Unfortunately the DT support in SPI was not complete at the time, and I
don't think it can't be changed at this time without breaking something
as Mark correctly said.

I fixed a lot of I2C drivers and DTS when doing the I2C converstion and
even with that some regressions were introduced like the one you report.

>> I think a robust solution would involve having the OF aliases namespaced
>> by bus, or just not using the OF aliases but potentially having
>> collisions if two vendors pick the same device name.
>
> But this sounds like the situation before the above mentioned commit
> af503716ac14, when both i2c and spi were symmetrically namespaced with
> i2c: and spi: respectively and contained the "compatible" stripped of the
> vendor prefix.
>

Is not the same for the reasons I mentioned above. What Mark suggests is
to encode the bus type information in the OF compatible string, while still
being consistent about the table used to report modaliases and match devices.

Maybe we could have something like the following (not much tested) patch ?

>From b00f5914606fb72a5f7bdb38e63d109264261dee Mon Sep 17 00:00:00 2001
From: Javier Martinez Canillas <javierm@...hat.com>
Date: Thu, 23 Oct 2025 13:32:04 +0200
Subject: [PATCH RFC] of: Report the bus type in module alias type sub-field

The modaliases for devices registered through Device Trees don't have any
information about the bus of the device. For example, an I2C device has:

$ cat /sys/devices/platform/soc/fe804000.i2c/i2c-1/1-003c/uevent
DRIVER=ssd130x-i2c
OF_NAME=oled
OF_FULLNAME=/soc/i2c at 7e804000/oled at 3c
OF_COMPATIBLE_0=solomon,ssd1306fb-i2c
OF_COMPATIBLE_N=1
MODALIAS=of:NoledT(null)Csolomon,ssd1306fb-i2c

$ modinfo ssd130x-i2c | grep alias
alias:          of:N*T*Csolomon,ssd1309fb-i2cC*
alias:          of:N*T*Csolomon,ssd1309fb-i2c
alias:          of:N*T*Csolomon,ssd1307fb-i2cC*
alias:          of:N*T*Csolomon,ssd1307fb-i2c
alias:          of:N*T*Csolomon,ssd1306fb-i2cC*
alias:          of:N*T*Csolomon,ssd1306fb-i2c
alias:          of:N*T*Csolomon,ssd1305fb-i2cC*
alias:          of:N*T*Csolomon,ssd1305fb-i2c
alias:          of:N*T*Csinowealth,sh1106-i2cC*
alias:          of:N*T*Csinowealth,sh1106-i2c

The module aliases and compatible string have the bus (-i2c) as suffix to
denote that is a driver for a device that can be accessed through I2C.

This is done to prevent disambiguate in the case that the same device can
be accessed through another bus (i.e: SPI) and have a different driver.

To prevent this and allow to use the same compatible string for the same
device regardless of the bus type used, let's add information about the
bus type in the devide type module aliases sub-field that are reported to
user-space. The same device then will report something like following:

$ cat /sys/devices/platform/soc/fe804000.i2c/i2c-1/1-003c/uevent
DRIVER=ssd130x-i2c
OF_NAME=oled
OF_FULLNAME=/soc/i2c at 7e804000/oled at 3c
OF_COMPATIBLE_0=solomon,ssd1306fb-i2c
OF_COMPATIBLE_N=1
OF_TYPE=i2c
MODALIAS=of:NoledTi2cCsolomon,ssd1306fb-i2c

Signed-off-by: Javier Martinez Canillas <javierm@...hat.com>
---
 drivers/of/device.c | 6 ++++--
 drivers/of/module.c | 8 ++++++--
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/of/device.c b/drivers/of/device.c
index f7e75e527667..4187decc2873 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -225,8 +225,10 @@ void of_device_uevent(const struct device *dev, struct kobj_uevent_env *env)
 	add_uevent_var(env, "OF_NAME=%pOFn", dev->of_node);
 	add_uevent_var(env, "OF_FULLNAME=%pOF", dev->of_node);
 	type = of_node_get_device_type(dev->of_node);
-	if (type)
-		add_uevent_var(env, "OF_TYPE=%s", type);
+	if (!type)
+		type = dev_bus_name(dev);
+
+	add_uevent_var(env, "OF_TYPE=%s", type);
 
 	/* Since the compatible field can contain pretty much anything
 	 * it's not really legal to split it out with commas. We split it
diff --git a/drivers/of/module.c b/drivers/of/module.c
index 1e735fc130ad..f22ddc83ef40 100644
--- a/drivers/of/module.c
+++ b/drivers/of/module.c
@@ -11,6 +11,7 @@
 ssize_t of_modalias(const struct device_node *np, char *str, ssize_t len)
 {
 	const char *compat;
+	const char *type;
 	char *c;
 	struct property *p;
 	ssize_t csize;
@@ -24,10 +25,13 @@ ssize_t of_modalias(const struct device_node *np, char *str, ssize_t len)
 	if ((len > 0 && !str) || len < 0)
 		return -EINVAL;
 
+	type = of_node_get_device_type(dev->of_node);
+	if (!type)
+		type = dev_bus_name(dev);
+
 	/* Name & Type */
 	/* %p eats all alphanum characters, so %c must be used here */
-	csize = snprintf(str, len, "of:N%pOFn%c%s", np, 'T',
-			 of_node_get_device_type(np));
+	csize = snprintf(str, len, "of:N%pOFn%c%s", np, 'T', type);
 	tsize = csize;
 	if (csize >= len)
 		csize = len > 0 ? len - 1 : 0;

base-commit: 3a8660878839faadb4f1a6dd72c3179c1df56787
-- 
2.51.0

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat

Powered by blists - more mailing lists