Re: [PATCH v7 1/2] module: add SCMI device table alias support

[Hans de Goede <johannes.goede@oss.qualcomm.com>]

Hi,

On 23-Sep-26 11:07, Sudeep Holla wrote:
> On Mon, Sep 21, 2026 at 05:19:45PM +0200, Uwe Kleine-König wrote:
>> On Fri, Sep 18, 2026 at 11:29:50AM +0200, Hans de Goede wrote:
>>> From: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
>>>
>>> SCMI client drivers already describe their bus match data with
>>> MODULE_DEVICE_TABLE(scmi, ...), but modpost does not know how to consume
>>> SCMI device tables. As a result, SCMI modules do not get generated module
>>> aliases from their id tables.
>>>
>>> Move struct scmi_device_id to mod_devicetable.h so it has a fixed layout
>>> visible to modpost, add the corresponding generated offsets and teach
>>> file2alias to emit scmi:<protocol>:<name> aliases.
>>>
>>> Use the same stable alias format for SCMI device uevents and sysfs
>>> modaliases. The previous string included the instance-specific device
>>> name, which is not useful for matching modules.
>>>
>>> Assisted-by: Codex:GPT-5.5
>>> Reviewed-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
>>> Tested-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
>>> Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
>>> Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
> 
> [...]
> 
>>> diff --git a/include/linux/device-id/scmi.h b/include/linux/device-id/scmi.h
>>> new file mode 100644
>>> index 000000000000..1b4ccfa9dcc5
>>> --- /dev/null
>>> +++ b/include/linux/device-id/scmi.h
>>> @@ -0,0 +1,17 @@
>>> +/* SPDX-License-Identifier: GPL-2.0-only */
>>> +#ifndef LINUX_DEVICE_ID_SCMI_H
>>> +#define LINUX_DEVICE_ID_SCMI_H
>>> +
>>> +#ifdef __KERNEL__
>>> +#include <linux/types.h>
>>> +#endif
>>> +
>>> +#define SCMI_NAME_SIZE		32
>>> +#define SCMI_MODULE_PREFIX	"scmi:"
>>> +
>>> +struct scmi_device_id {
>>> +	__u8 protocol_id;
>>> +	char name[SCMI_NAME_SIZE];
>>
>> I wonder if you tried to keep this a char *. ISTR someone did something
>> similar recently and they claimed it worked. That would get rid of the
>> artificial name size limit and simplify this patch.
>>
> 
> I agree with this. 

Ok, so I checked and no other include/linux/device-id/*.h file
defines a foo_device_id field with a type of "char *" and
then uses that field in scripts/mod/devicetable-offsets.c /
scripts/mod/file2alias.c .

2 device-id/foo.h headers (dmi, pcmcia) do define a "char *"
field, but then do NOT use that to generate a modalias.

So scmi_device_id would be the first to do this.

Despite that I still gave this a try. Things do compile,
but it results in a modalias for the scmi-cpufreq driver
of: scmi:13:(null) and auto-loading unsurprisingly does
not work.

TL;DR: using 'char *' instead of char [<max-len>] in
a device-id struct field that is used for modaliases
does NOT work.

So I believe that this v7 is ready to merge as is.

> [...]
> 
>>> @@ -1491,6 +1502,7 @@ static const struct devtable devtable[] = {
>>>  	{"virtio", SIZE_virtio_device_id, do_virtio_entry},
>>>  	{"vmbus", SIZE_hv_vmbus_device_id, do_vmbus_entry},
>>>  	{"rpmsg", SIZE_rpmsg_device_id, do_rpmsg_entry},
>>> +	{"scmi", SIZE_scmi_device_id, do_scmi_entry},
>>>  	{"i2c", SIZE_i2c_device_id, do_i2c_entry},
>>>  	{"i3c", SIZE_i3c_device_id, do_i3c_entry},
>>>  	{"slim", SIZE_slim_device_id, do_slim_entry},
>>
>> I wonder if this is supposed to be ordered alphabetically ...
>>
> 
> This may be hard, just the best effort unless it is cleaned up first.
> 
> Anyways, I plan to send PR by end of the week, so I really need this
> in by tomorrow worst case. I am bit worried if there will conflict all
> over the place in this file as I know couple of other patches that may
> come via other trees touching same entries(one is SMCCC)🤞.

Thank you for the ping, as mentioned above I give Uwe's suggestion
a try but it does not work.

So I believe that this v7 is ready for merging as is.

Regards,

Hans