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

[Sudeep Holla <sudeep.holla@kernel.org>]

On Wed, Sep 23, 2026 at 01:26:00PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 23-Sep-26 12:49, Sudeep Holla wrote:
> > On Wed, Sep 23, 2026 at 12:23:50PM +0200, Hans de Goede wrote:
> >> 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 .
> >>
> > 
> > I looked at hda_device_id and its uses. It looks like it does use
> > char * and there were loads of drivers initialising the string. I must
> > be missing something then ?
> 
> If you look for hda_device_id in:
> 
> scripts/mod/devicetable-offsets.c
> scripts/mod/file2alias.c
> 
> Neither references the name member of struct hda_device_id.
> 
> So the actual modalias(es) added to the .ko by modpost
> do not include the name, they are of the following format:
> 
>         ADD(alias, "v", vendor_id != 0, vendor_id);
>         ADD(alias, "r", rev_id != 0, rev_id);
>         ADD(alias, "a", api_version != 0, api_version);
> 
>         module_alias_printf(mod, true, "hdaudio:%s", alias);
>

Indeed, thanks for the pointer.

> >> 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.
> >>
> > 
> > Your response made me dig further and I found snd_hdac_codec_modalias()
> > which seems to do the magic there.
> 
> Note that function:
> 
> int snd_hdac_codec_modalias(const struct hdac_device *codec, char *buf, size_t size)
> {
>         return scnprintf(buf, size, "hdaudio:v%08Xr%08Xa%02X\n",
>                         codec->vendor_id, codec->revision_id, codec->type);
> }
> 
> Also does not include any name field into the modalias. Note this
> side is the modalias which shows up under
> 
> 
> /sys/bus/xxx/devices/yyy/modalias
> 
> not the one which gets included into the .ko (and can be shown
> by "modinfo") that one comes from scripts/mod/file2alias, but
> the 2 must match of course otherwise udev will not load the .ko.
> 
> So it seems the name field in struct hda_device_id is only
> there for the kernel to include it in some log messages,
> just like e.g. the dmi_device_id "ident" string.
> 
> But this is not used by the modpost code which adds
> the modalias to the .ko, that tool is the one which has
> problems with non const size strings.
> 

Agreed.

Uwe, are you OK to keep the name with limited size ? Let us know.

If Uwe is fine, yes I can take v7 as is.

-- 
Regards,
Sudeep