* [PATCH v7 0/2] firmware: arm_scmi: fix module auto-loading
@ 2026-09-18 9:29 Hans de Goede
2026-09-18 9:29 ` [PATCH v7 1/2] module: add SCMI device table alias support Hans de Goede
2026-09-18 9:29 ` [PATCH v7 2/2] firmware: arm_scmi: Always create devices for standard protocols Hans de Goede
0 siblings, 2 replies; 13+ messages in thread
From: Hans de Goede @ 2026-09-18 9:29 UTC (permalink / raw)
To: Bjorn Andersson, Cristian Marussi, Sudeep Holla
Cc: Hans de Goede, Daniel Lezcano, Bjorn Andersson, Frank.Li,
arm-scmi, linux-arm-kernel, linux-arm-msm, imx, linux-kernel
Hi All,
Here is v7 of the patch series fixing arm_scmi module autoloading this
combines:
1. Patch 1/2 from Bjorn to add support for scmi bus modaliases to modpost:
https://lore.kernel.org/all/20260618-scmi-modalias-v2-1-8c7547c1be21@oss.qualcomm.com/
2. A new patch 2/2 to always create devices for standard protocols,
replacing Cristian's patch to pre-register the standard protocol device-ids
from v4.
1. is not enough by itself because driver module auto-loading requires
the devices to already be created for udev to get the necessary uevents
based on which udev auto-loads modules.
But SCMI devices are only created after their { protocol, name } device-id
has been registered which is done from scmi_driver_register(), creating
a circular dependency.
2. breaks the circular dependency by always creating devices for standard
protocols without relying on the requested-devices list. This creates
devices before the module with the driver is loaded, after which module
auto loading works the same as it does on any other bus.
I've tested this on a T14s Snapdragon laptop with Fedora's kernel config
where scmi_cpufreq is a module. With this series scmi_cpufreq correctly
autoloads even if it is not included in the initramfs.
Changes in v7:
- Rebase on top of v7.3-rc3
- Drop:
"firmware: arm_scmi: Fix scmi_protocol_table_register() error handling"
"firmware: arm_scmi: Fix scmi_protocol_device_unrequest() race"
these issues are already fixed through other patches in v7.3-rc3
Changes in v6:
- New: "firmware: arm_scmi: Fix scmi_protocol_device_unrequest() race"
- Add Fixes tags to the bug-fix patches
- Replace the if (!phead) return NULL; early exit with return scmi_dev;
Since a standard protocol device may have been created at this point
(from local Shashiko run)
Changes in v5:
- New patch: "firmware: arm_scmi: Fix scmi_protocol_table_register()
error handling"
- Replace "Pre-register protocol, name tupples for standard protocol" with
"Always create devices for standard protocols" pre-registering requires
scmi_protocol_device_request() to allow duplicate device-ids, but then if
the first driver of two with duplicate devce-ids gets unloaded device
creation for the second driver will fail. Allowing duplicates causes all
kinds of problems, so this new approach avoids this.
- Drop "Pre-register protocol, name tupples for IMX protocols" patch.
Shashiko rightfully turns out this will cause problems if other vendors
also use the same vendor protocol-id space values, e.g. 0x80, 0x81 are
likely to also be used by other vendors.
Changes in v4:
- Drop unused driver_data member from struct scmi_device_id (Uwe)
- Drop device-id/scmi.h include from mod_devicetable.h (Uwe)
- Add device-id/scmi.h to devicetable-offsets.c and file2alias.c (Uwe)
Changes in v3:
- v3 is the first series combining Bjorn and Christian's work see above.
Regards,
Hans
Bjorn Andersson (1):
module: add SCMI device table alias support
Hans de Goede (1):
firmware: arm_scmi: Always create devices for standard protocols
MAINTAINERS | 1 +
drivers/firmware/arm_scmi/bus.c | 71 +++++++++++++++++++++++++------
include/linux/device-id/scmi.h | 17 ++++++++
include/linux/scmi_protocol.h | 6 +--
scripts/mod/devicetable-offsets.c | 5 +++
scripts/mod/file2alias.c | 12 ++++++
6 files changed, 93 insertions(+), 19 deletions(-)
create mode 100644 include/linux/device-id/scmi.h
--
2.55.0
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH v7 1/2] module: add SCMI device table alias support 2026-09-18 9:29 [PATCH v7 0/2] firmware: arm_scmi: fix module auto-loading Hans de Goede @ 2026-09-18 9:29 ` Hans de Goede 2026-09-18 9:53 ` Daniel Lezcano 2026-09-18 13:32 ` Sudeep Holla 2026-09-18 9:29 ` [PATCH v7 2/2] firmware: arm_scmi: Always create devices for standard protocols Hans de Goede 1 sibling, 2 replies; 13+ messages in thread From: Hans de Goede @ 2026-09-18 9:29 UTC (permalink / raw) To: Bjorn Andersson, Cristian Marussi, Sudeep Holla Cc: Hans de Goede, Daniel Lezcano, Bjorn Andersson, Frank.Li, arm-scmi, linux-arm-kernel, linux-arm-msm, imx, linux-kernel 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> --- Changes in v7: - Rebase on top of v7.3-rc3 Changes in v4: - Drop unused driver_data member from struct scmi_device_id (Uwe) - Drop device-id/scmi.h include from mod_devicetable.h (Uwe) - Add device-id/scmi.h to devicetable-offsets.c and file2alias.c (Uwe) Changes in v3: - Adjust for ad428f5811bd ("mod_devicetable.h: Split into per subsystem headers") - Add '\n' to modalias_show() output, matching other subsystems' modalias Changes in v2: - Drop #include <linux/mod_devicetable.h> from scmi_protocol.h - Link to v1: https://patch.msgid.link/20260616-scmi-modalias-v1-0-662b8dd52ab2@oss.qualcomm.com --- MAINTAINERS | 1 + drivers/firmware/arm_scmi/bus.c | 21 ++++++++++----------- include/linux/device-id/scmi.h | 17 +++++++++++++++++ include/linux/scmi_protocol.h | 6 +----- scripts/mod/devicetable-offsets.c | 5 +++++ scripts/mod/file2alias.c | 12 ++++++++++++ 6 files changed, 46 insertions(+), 16 deletions(-) create mode 100644 include/linux/device-id/scmi.h diff --git a/MAINTAINERS b/MAINTAINERS index c2414447892c..f351ec56a2ec 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -26604,6 +26604,7 @@ F: drivers/pmdomain/arm/ F: drivers/powercap/arm_scmi_powercap.c F: drivers/regulator/scmi-regulator.c F: drivers/reset/reset-scmi.c +F: include/linux/device-id/scmi.h F: include/linux/sc[mp]i_protocol.h F: include/trace/events/scmi.h F: include/uapi/linux/virtio_scmi.h diff --git a/drivers/firmware/arm_scmi/bus.c b/drivers/firmware/arm_scmi/bus.c index e060edbe7e83..51de50fbc382 100644 --- a/drivers/firmware/arm_scmi/bus.c +++ b/drivers/firmware/arm_scmi/bus.c @@ -12,11 +12,12 @@ #include <linux/of.h> #include <linux/kernel.h> #include <linux/slab.h> +#include <linux/string.h> #include <linux/device.h> #include "common.h" -#define SCMI_UEVENT_MODALIAS_FMT "%s:%02x:%s" +#define SCMI_UEVENT_MODALIAS_FMT SCMI_MODULE_PREFIX "%02x:%s" BLOCKING_NOTIFIER_HEAD(scmi_requested_devices_nh); EXPORT_SYMBOL_GPL(scmi_requested_devices_nh); @@ -185,7 +186,7 @@ static int scmi_protocol_table_register(const struct scmi_device_id *id_table) const struct scmi_device_id *entry; int ret; - for (entry = id_table; entry->name; entry++) { + for (entry = id_table; entry->name[0]; entry++) { ret = scmi_protocol_device_request(entry); if (ret) goto err_unrequest; @@ -205,7 +206,7 @@ scmi_protocol_table_unregister(const struct scmi_device_id *id_table) { const struct scmi_device_id *entry; - for (entry = id_table; entry->name; entry++) + for (entry = id_table; entry->name[0]; entry++) scmi_protocol_device_unrequest(entry); } @@ -219,10 +220,10 @@ static int __scmi_dev_match_by_id_table(struct scmi_device *scmi_dev, const struct scmi_device_id *id_table, bool skip_transport) { - if (!id_table || !id_table->name) + if (!id_table || !id_table->name[0]) return 0; - for (; id_table->protocol_id && id_table->name; id_table++) + for (; id_table->protocol_id && id_table->name[0]; id_table++) if (id_table->protocol_id == scmi_dev->protocol_id && !(skip_transport && scmi_device_is_transport(scmi_dev)) && !strcmp(id_table->name, scmi_dev->name)) @@ -266,7 +267,7 @@ scmi_child_dev_find_get(struct device *parent, int prot_id, const char *name) struct device *dev; id_table[0].protocol_id = prot_id; - id_table[0].name = name; + strscpy(id_table[0].name, name, sizeof(id_table[0].name)); dev = device_find_child(parent, &id_table, scmi_match_by_id_table); if (!dev) @@ -300,8 +301,7 @@ static int scmi_device_uevent(const struct device *dev, struct kobj_uevent_env * const struct scmi_device *scmi_dev = to_scmi_dev(dev); return add_uevent_var(env, "MODALIAS=" SCMI_UEVENT_MODALIAS_FMT, - dev_name(&scmi_dev->dev), scmi_dev->protocol_id, - scmi_dev->name); + scmi_dev->protocol_id, scmi_dev->name); } static ssize_t modalias_show(struct device *dev, @@ -309,9 +309,8 @@ static ssize_t modalias_show(struct device *dev, { struct scmi_device *scmi_dev = to_scmi_dev(dev); - return sysfs_emit(buf, SCMI_UEVENT_MODALIAS_FMT, - dev_name(&scmi_dev->dev), scmi_dev->protocol_id, - scmi_dev->name); + return sysfs_emit(buf, SCMI_UEVENT_MODALIAS_FMT "\n", + scmi_dev->protocol_id, scmi_dev->name); } static DEVICE_ATTR_RO(modalias); 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]; +}; + +#endif /* ifndef LINUX_DEVICE_ID_SCMI_H */ diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h index 5ab73b1ab9aa..ba53302d95f5 100644 --- a/include/linux/scmi_protocol.h +++ b/include/linux/scmi_protocol.h @@ -9,6 +9,7 @@ #define _LINUX_SCMI_PROTOCOL_H #include <linux/bitfield.h> +#include <linux/device-id/scmi.h> #include <linux/device.h> #include <linux/notifier.h> #include <linux/types.h> @@ -951,11 +952,6 @@ struct scmi_device { #define to_scmi_dev(d) container_of_const(d, struct scmi_device, dev) -struct scmi_device_id { - u8 protocol_id; - const char *name; -}; - struct scmi_driver { const char *name; int (*probe)(struct scmi_device *sdev); diff --git a/scripts/mod/devicetable-offsets.c b/scripts/mod/devicetable-offsets.c index b4178c42d08f..91ec3704ee2b 100644 --- a/scripts/mod/devicetable-offsets.c +++ b/scripts/mod/devicetable-offsets.c @@ -1,5 +1,6 @@ // SPDX-License-Identifier: GPL-2.0 #define COMPILE_OFFSETS +#include <linux/device-id/scmi.h> #include <linux/kbuild.h> #include <linux/mod_devicetable.h> @@ -144,6 +145,10 @@ int main(void) DEVID(rpmsg_device_id); DEVID_FIELD(rpmsg_device_id, name); + DEVID(scmi_device_id); + DEVID_FIELD(scmi_device_id, protocol_id); + DEVID_FIELD(scmi_device_id, name); + DEVID(i2c_device_id); DEVID_FIELD(i2c_device_id, name); diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c index 8d36c74dec2d..5379b1def07b 100644 --- a/scripts/mod/file2alias.c +++ b/scripts/mod/file2alias.c @@ -121,6 +121,7 @@ typedef struct { /* Big exception to the "don't include kernel headers into userspace, which * even potentially has different endianness and word sizes, since * we handle those differences explicitly below */ +#include "../../include/linux/device-id/scmi.h" #include "../../include/linux/mod_devicetable.h" struct devtable { @@ -852,6 +853,16 @@ static void do_rpmsg_entry(struct module *mod, void *symval) module_alias_printf(mod, false, RPMSG_DEVICE_MODALIAS_FMT, *name); } +/* Looks like: scmi:NN:S */ +static void do_scmi_entry(struct module *mod, void *symval) +{ + DEF_FIELD(symval, scmi_device_id, protocol_id); + DEF_FIELD_ADDR(symval, scmi_device_id, name); + + module_alias_printf(mod, false, SCMI_MODULE_PREFIX "%02x:%s", + protocol_id, *name); +} + /* Looks like: i2c:S */ static void do_i2c_entry(struct module *mod, void *symval) { @@ -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}, -- 2.55.0 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v7 1/2] module: add SCMI device table alias support 2026-09-18 9:29 ` [PATCH v7 1/2] module: add SCMI device table alias support Hans de Goede @ 2026-09-18 9:53 ` Daniel Lezcano 2026-09-18 10:02 ` Hans de Goede 2026-09-18 13:32 ` Sudeep Holla 1 sibling, 1 reply; 13+ messages in thread From: Daniel Lezcano @ 2026-09-18 9:53 UTC (permalink / raw) To: Hans de Goede, Bjorn Andersson, Cristian Marussi, Sudeep Holla Cc: Bjorn Andersson, Frank.Li, arm-scmi, linux-arm-kernel, linux-arm-msm, imx, linux-kernel Hi Hans, thanks for taking care of that On 9/18/26 11:29, 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> > --- [ ... ] > > -#define SCMI_UEVENT_MODALIAS_FMT "%s:%02x:%s" > +#define SCMI_UEVENT_MODALIAS_FMT SCMI_MODULE_PREFIX "%02x:%s" > > BLOCKING_NOTIFIER_HEAD(scmi_requested_devices_nh); > EXPORT_SYMBOL_GPL(scmi_requested_devices_nh); > @@ -185,7 +186,7 @@ static int scmi_protocol_table_register(const struct scmi_device_id *id_table) > const struct scmi_device_id *entry; > int ret; > > - for (entry = id_table; entry->name; entry++) { > + for (entry = id_table; entry->name[0]; entry++) { Is it possible to rely on a NULL sentinel? Here if the id_table is NULL, entry->name | entry->name[0] dereference the NULL pointer for (entry = id_table; entry != NULL; entry++) > ret = scmi_protocol_device_request(entry); [ ... ] > #include <linux/bitfield.h> > +#include <linux/device-id/scmi.h> > #include <linux/device.h> > #include <linux/notifier.h> > #include <linux/types.h> > @@ -951,11 +952,6 @@ struct scmi_device { > > #define to_scmi_dev(d) container_of_const(d, struct scmi_device, dev) > > -struct scmi_device_id { > - u8 protocol_id; > - const char *name; > -}; > - What is the reason of converting the char * to a fixed array? That limits the name and may result in truncation and potentially name collision, no ? [ ... ] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v7 1/2] module: add SCMI device table alias support 2026-09-18 9:53 ` Daniel Lezcano @ 2026-09-18 10:02 ` Hans de Goede 2026-09-18 10:13 ` Daniel Lezcano 0 siblings, 1 reply; 13+ messages in thread From: Hans de Goede @ 2026-09-18 10:02 UTC (permalink / raw) To: Daniel Lezcano, Bjorn Andersson, Cristian Marussi, Sudeep Holla Cc: Bjorn Andersson, Frank.Li, arm-scmi, linux-arm-kernel, linux-arm-msm, imx, linux-kernel Hi Daniel, On 18-Sep-26 11:53, Daniel Lezcano wrote: > > Hi Hans, > > thanks for taking care of that > > > On 9/18/26 11:29, 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> >> --- > > [ ... ] > >> -#define SCMI_UEVENT_MODALIAS_FMT "%s:%02x:%s" >> +#define SCMI_UEVENT_MODALIAS_FMT SCMI_MODULE_PREFIX "%02x:%s" >> BLOCKING_NOTIFIER_HEAD(scmi_requested_devices_nh); >> EXPORT_SYMBOL_GPL(scmi_requested_devices_nh); >> @@ -185,7 +186,7 @@ static int scmi_protocol_table_register(const struct scmi_device_id *id_table) >> const struct scmi_device_id *entry; >> int ret; >> - for (entry = id_table; entry->name; entry++) { >> + for (entry = id_table; entry->name[0]; entry++) { > > Is it possible to rely on a NULL sentinel? > > Here if the id_table is NULL, entry->name | entry->name[0] dereference the NULL pointer The NULL deref on id_table is NULL already happened with the old code, which would deref entry to check the name pointer, This just adjusts the check to check for name being an empty string since it now is a fixed-size string / char array. > for (entry = id_table; entry != NULL; entry++) > >> ret = scmi_protocol_device_request(entry); > > [ ... ] > >> #include <linux/bitfield.h> >> +#include <linux/device-id/scmi.h> >> #include <linux/device.h> >> #include <linux/notifier.h> >> #include <linux/types.h> >> @@ -951,11 +952,6 @@ struct scmi_device { >> #define to_scmi_dev(d) container_of_const(d, struct scmi_device, dev) >> -struct scmi_device_id { >> - u8 protocol_id; >> - const char *name; >> -}; >> - > > What is the reason of converting the char * to a fixed array? That limits the name and may result in truncation and potentially name collision, no ? Because of how modpost works to generate modaliases inside the .ko any string buffers in device_id structs need to have a fixed length. So the truncation / name collision issue pretty much applies to all foo_device_id structs in the kernel. People should now to make sure that any strings used will fit inside the fixed string. And I would expect the compiler to warn for overly long strings. Regards, Hans ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v7 1/2] module: add SCMI device table alias support 2026-09-18 10:02 ` Hans de Goede @ 2026-09-18 10:13 ` Daniel Lezcano 2026-09-18 13:38 ` Sudeep Holla 0 siblings, 1 reply; 13+ messages in thread From: Daniel Lezcano @ 2026-09-18 10:13 UTC (permalink / raw) To: Hans de Goede, Bjorn Andersson, Cristian Marussi, Sudeep Holla Cc: Bjorn Andersson, Frank.Li, arm-scmi, linux-arm-kernel, linux-arm-msm, imx, linux-kernel On 9/18/26 12:02, Hans de Goede wrote: > Hi Daniel, > > On 18-Sep-26 11:53, Daniel Lezcano wrote: >> >> Hi Hans, >> >> thanks for taking care of that >> >> >> On 9/18/26 11:29, 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> >>> --- >> [ ... ] >>> #include <linux/bitfield.h> >>> +#include <linux/device-id/scmi.h> >>> #include <linux/device.h> >>> #include <linux/notifier.h> >>> #include <linux/types.h> >>> @@ -951,11 +952,6 @@ struct scmi_device { >>> #define to_scmi_dev(d) container_of_const(d, struct scmi_device, dev) >>> -struct scmi_device_id { >>> - u8 protocol_id; >>> - const char *name; >>> -}; >>> - >> >> What is the reason of converting the char * to a fixed array? That limits the name and may result in truncation and potentially name collision, no ? > > Because of how modpost works to generate modaliases inside the .ko > any string buffers in device_id structs need to have a fixed length. > > So the truncation / name collision issue pretty much applies to all > foo_device_id structs in the kernel. People should now to make sure > that any strings used will fit inside the fixed string. And I would > expect the compiler to warn for overly long strings. Ok, thanks for the clarification ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v7 1/2] module: add SCMI device table alias support 2026-09-18 10:13 ` Daniel Lezcano @ 2026-09-18 13:38 ` Sudeep Holla 2026-09-18 14:43 ` Rob Clark ` (2 more replies) 0 siblings, 3 replies; 13+ messages in thread From: Sudeep Holla @ 2026-09-18 13:38 UTC (permalink / raw) To: Daniel Lezcano Cc: Hans de Goede, Bjorn Andersson, Cristian Marussi, Bjorn Andersson, Sudeep Holla, Frank.Li, arm-scmi, linux-arm-kernel, linux-arm-msm, imx, linux-kernel On Fri, Sep 18, 2026 at 12:13:10PM +0200, Daniel Lezcano wrote: > On 9/18/26 12:02, Hans de Goede wrote: > > Hi Daniel, > > > > On 18-Sep-26 11:53, Daniel Lezcano wrote: > > > > > > Hi Hans, > > > > > > thanks for taking care of that > > > > > > > > > On 9/18/26 11:29, 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> > > > > --- > > > > > [ ... ] > > > > > #include <linux/bitfield.h> > > > > +#include <linux/device-id/scmi.h> > > > > #include <linux/device.h> > > > > #include <linux/notifier.h> > > > > #include <linux/types.h> > > > > @@ -951,11 +952,6 @@ struct scmi_device { > > > > #define to_scmi_dev(d) container_of_const(d, struct scmi_device, dev) > > > > -struct scmi_device_id { > > > > - u8 protocol_id; > > > > - const char *name; > > > > -}; > > > > - > > > > > > What is the reason of converting the char * to a fixed array? That limits the name and may result in truncation and potentially name collision, no ? > > > > Because of how modpost works to generate modaliases inside the .ko > > any string buffers in device_id structs need to have a fixed length. > > > > So the truncation / name collision issue pretty much applies to all > > foo_device_id structs in the kernel. People should now to make sure > > that any strings used will fit inside the fixed string. And I would > > expect the compiler to warn for overly long strings. > Ok, thanks for the clarification Not related to above topic, but just wanted to check with Daniel if this solves module auto-loading for you. Was there someone else who was testing this series and was having issue with earlier versions or its you only Hans de Goede and it was with v1/v2 when Bjorn posted them. I remember something vaguely and couldn't trace back all the versions easily. -- Regards, Sudeep ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v7 1/2] module: add SCMI device table alias support 2026-09-18 13:38 ` Sudeep Holla @ 2026-09-18 14:43 ` Rob Clark 2026-09-18 14:48 ` Daniel Lezcano 2026-09-18 20:15 ` Daniel Lezcano 2 siblings, 0 replies; 13+ messages in thread From: Rob Clark @ 2026-09-18 14:43 UTC (permalink / raw) To: Sudeep Holla Cc: Daniel Lezcano, Hans de Goede, Bjorn Andersson, Cristian Marussi, Bjorn Andersson, Frank.Li, arm-scmi, linux-arm-kernel, linux-arm-msm, imx, linux-kernel On Fri, Sep 18, 2026 at 7:10 AM Sudeep Holla <sudeep.holla@kernel.org> wrote: > > On Fri, Sep 18, 2026 at 12:13:10PM +0200, Daniel Lezcano wrote: > > On 9/18/26 12:02, Hans de Goede wrote: > > > Hi Daniel, > > > > > > On 18-Sep-26 11:53, Daniel Lezcano wrote: > > > > > > > > Hi Hans, > > > > > > > > thanks for taking care of that > > > > > > > > > > > > On 9/18/26 11:29, 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> > > > > > --- > > > > > > > > [ ... ] > > > > > > > #include <linux/bitfield.h> > > > > > +#include <linux/device-id/scmi.h> > > > > > #include <linux/device.h> > > > > > #include <linux/notifier.h> > > > > > #include <linux/types.h> > > > > > @@ -951,11 +952,6 @@ struct scmi_device { > > > > > #define to_scmi_dev(d) container_of_const(d, struct scmi_device, dev) > > > > > -struct scmi_device_id { > > > > > - u8 protocol_id; > > > > > - const char *name; > > > > > -}; > > > > > - > > > > > > > > What is the reason of converting the char * to a fixed array? That limits the name and may result in truncation and potentially name collision, no ? > > > > > > Because of how modpost works to generate modaliases inside the .ko > > > any string buffers in device_id structs need to have a fixed length. > > > > > > So the truncation / name collision issue pretty much applies to all > > > foo_device_id structs in the kernel. People should now to make sure > > > that any strings used will fit inside the fixed string. And I would > > > expect the compiler to warn for overly long strings. > > Ok, thanks for the clarification > > Not related to above topic, but just wanted to check with Daniel if this > solves module auto-loading for you. Was there someone else who was testing > this series and was having issue with earlier versions or its you only > Hans de Goede and it was with v1/v2 when Bjorn posted them. I remember > something vaguely and couldn't trace back all the versions easily. I'm using v7 on glymur slim7x, and it solves the scmi-cpufreq autoloading Tested-by: Rob Clark <rob.clark@oss.qualcomm.com> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v7 1/2] module: add SCMI device table alias support 2026-09-18 13:38 ` Sudeep Holla 2026-09-18 14:43 ` Rob Clark @ 2026-09-18 14:48 ` Daniel Lezcano 2026-09-18 20:15 ` Daniel Lezcano 2 siblings, 0 replies; 13+ messages in thread From: Daniel Lezcano @ 2026-09-18 14:48 UTC (permalink / raw) To: Sudeep Holla Cc: Hans de Goede, Bjorn Andersson, Cristian Marussi, Bjorn Andersson, Frank.Li, arm-scmi, linux-arm-kernel, linux-arm-msm, imx, linux-kernel On 9/18/26 15:38, Sudeep Holla wrote: > On Fri, Sep 18, 2026 at 12:13:10PM +0200, Daniel Lezcano wrote: >> On 9/18/26 12:02, Hans de Goede wrote: >>> Hi Daniel, >>> >>> On 18-Sep-26 11:53, Daniel Lezcano wrote: >>>> >>>> Hi Hans, >>>> >>>> thanks for taking care of that >>>> >>>> >>>> On 9/18/26 11:29, 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> >>>>> --- >>>> >> >> [ ... ] >> >>>>> #include <linux/bitfield.h> >>>>> +#include <linux/device-id/scmi.h> >>>>> #include <linux/device.h> >>>>> #include <linux/notifier.h> >>>>> #include <linux/types.h> >>>>> @@ -951,11 +952,6 @@ struct scmi_device { >>>>> #define to_scmi_dev(d) container_of_const(d, struct scmi_device, dev) >>>>> -struct scmi_device_id { >>>>> - u8 protocol_id; >>>>> - const char *name; >>>>> -}; >>>>> - >>>> >>>> What is the reason of converting the char * to a fixed array? That limits the name and may result in truncation and potentially name collision, no ? >>> >>> Because of how modpost works to generate modaliases inside the .ko >>> any string buffers in device_id structs need to have a fixed length. >>> >>> So the truncation / name collision issue pretty much applies to all >>> foo_device_id structs in the kernel. People should now to make sure >>> that any strings used will fit inside the fixed string. And I would >>> expect the compiler to warn for overly long strings. >> Ok, thanks for the clarification > > Not related to above topic, but just wanted to check with Daniel if this > solves module auto-loading for you. Was there someone else who was testing > this series and was having issue with earlier versions or its you only > Hans de Goede and it was with v1/v2 when Bjorn posted them. I remember > something vaguely and couldn't trace back all the versions easily. Sure, I'll try it with my t14s laptop (Hamoa) ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v7 1/2] module: add SCMI device table alias support 2026-09-18 13:38 ` Sudeep Holla 2026-09-18 14:43 ` Rob Clark 2026-09-18 14:48 ` Daniel Lezcano @ 2026-09-18 20:15 ` Daniel Lezcano 2 siblings, 0 replies; 13+ messages in thread From: Daniel Lezcano @ 2026-09-18 20:15 UTC (permalink / raw) To: Sudeep Holla Cc: Hans de Goede, Bjorn Andersson, Cristian Marussi, Bjorn Andersson, Frank.Li, arm-scmi, linux-arm-kernel, linux-arm-msm, imx, linux-kernel On 9/18/26 15:38, Sudeep Holla wrote: [ ... ] > Not related to above topic, but just wanted to check with Daniel if this > solves module auto-loading for you. Was there someone else who was testing > this series and was having issue with earlier versions or its you only > Hans de Goede and it was with v1/v2 when Bjorn posted them. I remember > something vaguely and couldn't trace back all the versions easily. The scmi-cpufreq driver loads automatically now. Tested-by: Daniel Lezcano <daniel.lezcano@oss.qualcomm.com> # Lenovo T14s Hamoa ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v7 1/2] module: add SCMI device table alias support 2026-09-18 9:29 ` [PATCH v7 1/2] module: add SCMI device table alias support Hans de Goede 2026-09-18 9:53 ` Daniel Lezcano @ 2026-09-18 13:32 ` Sudeep Holla 2026-09-18 14:09 ` Hans de Goede 1 sibling, 1 reply; 13+ messages in thread From: Sudeep Holla @ 2026-09-18 13:32 UTC (permalink / raw) To: Hans de Goede Cc: Bjorn Andersson, Cristian Marussi, Daniel Lezcano, Bjorn Andersson, Frank.Li, arm-scmi, linux-arm-kernel, linux-arm-msm, imx, linux-kernel 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> > --- > Changes in v7: > - Rebase on top of v7.3-rc3 > > Changes in v4: > - Drop unused driver_data member from struct scmi_device_id (Uwe) > - Drop device-id/scmi.h include from mod_devicetable.h (Uwe) > - Add device-id/scmi.h to devicetable-offsets.c and file2alias.c (Uwe) > > Changes in v3: > - Adjust for ad428f5811bd ("mod_devicetable.h: Split into per subsystem > headers") > - Add '\n' to modalias_show() output, matching other subsystems' modalias > > Changes in v2: > - Drop #include <linux/mod_devicetable.h> from scmi_protocol.h > - Link to v1: https://patch.msgid.link/20260616-scmi-modalias-v1-0-662b8dd52ab2@oss.qualcomm.com > --- > MAINTAINERS | 1 + > drivers/firmware/arm_scmi/bus.c | 21 ++++++++++----------- > include/linux/device-id/scmi.h | 17 +++++++++++++++++ > include/linux/scmi_protocol.h | 6 +----- > scripts/mod/devicetable-offsets.c | 5 +++++ > scripts/mod/file2alias.c | 12 ++++++++++++ > 6 files changed, 46 insertions(+), 16 deletions(-) > create mode 100644 include/linux/device-id/scmi.h > [...] > 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]; > +}; > + > +#endif /* ifndef LINUX_DEVICE_ID_SCMI_H */ You need to include this above file in include/linux/mod_devicetable.h to avoid duplication of its inclusion elsewhere. That's the pattern followed I see. > diff --git a/include/linux/scmi_protocol.h b/include/linux/scmi_protocol.h > index 5ab73b1ab9aa..ba53302d95f5 100644 > --- a/include/linux/scmi_protocol.h > +++ b/include/linux/scmi_protocol.h > @@ -9,6 +9,7 @@ > #define _LINUX_SCMI_PROTOCOL_H > > #include <linux/bitfield.h> > +#include <linux/device-id/scmi.h> > #include <linux/device.h> > #include <linux/notifier.h> > #include <linux/types.h> > @@ -951,11 +952,6 @@ struct scmi_device { > > #define to_scmi_dev(d) container_of_const(d, struct scmi_device, dev) > > -struct scmi_device_id { > - u8 protocol_id; > - const char *name; > -}; > - > struct scmi_driver { > const char *name; > int (*probe)(struct scmi_device *sdev); > diff --git a/scripts/mod/devicetable-offsets.c b/scripts/mod/devicetable-offsets.c > index b4178c42d08f..91ec3704ee2b 100644 > --- a/scripts/mod/devicetable-offsets.c > +++ b/scripts/mod/devicetable-offsets.c > @@ -1,5 +1,6 @@ > // SPDX-License-Identifier: GPL-2.0 > #define COMPILE_OFFSETS > +#include <linux/device-id/scmi.h> Drop this for above mentioned reason. > #include <linux/kbuild.h> > #include <linux/mod_devicetable.h> > > @@ -144,6 +145,10 @@ int main(void) > DEVID(rpmsg_device_id); > DEVID_FIELD(rpmsg_device_id, name); > > + DEVID(scmi_device_id); > + DEVID_FIELD(scmi_device_id, protocol_id); > + DEVID_FIELD(scmi_device_id, name); > + > DEVID(i2c_device_id); > DEVID_FIELD(i2c_device_id, name); > > diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c > index 8d36c74dec2d..5379b1def07b 100644 > --- a/scripts/mod/file2alias.c > +++ b/scripts/mod/file2alias.c > @@ -121,6 +121,7 @@ typedef struct { > /* Big exception to the "don't include kernel headers into userspace, which > * even potentially has different endianness and word sizes, since > * we handle those differences explicitly below */ > +#include "../../include/linux/device-id/scmi.h" Drop this for above mentioned reason. -- Regards, Sudeep ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v7 1/2] module: add SCMI device table alias support 2026-09-18 13:32 ` Sudeep Holla @ 2026-09-18 14:09 ` Hans de Goede 2026-09-18 20:39 ` Uwe Kleine-König 0 siblings, 1 reply; 13+ messages in thread From: Hans de Goede @ 2026-09-18 14:09 UTC (permalink / raw) To: Sudeep Holla, Uwe Kleine-König Cc: Bjorn Andersson, Cristian Marussi, Daniel Lezcano, Bjorn Andersson, Frank.Li, arm-scmi, linux-arm-kernel, linux-arm-msm, imx, linux-kernel Hi Sudeep, On 18-Sep-26 15:32, Sudeep Holla wrote: [...] >> 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]; >> +}; >> + >> +#endif /* ifndef LINUX_DEVICE_ID_SCMI_H */ > > You need to include this above file in include/linux/mod_devicetable.h > to avoid duplication of its inclusion elsewhere. That's the pattern followed > I see. That is what I did for v3, but Uwe, who is leading the splitting of mod_devicetable.h from one large monster include to smaller per subsystem/bus includes explicitly requested me to follow this pattern, see: https://lore.kernel.org/arm-scmi/aow2GFAfy84k-N2B@monoceros/ [...] >> diff --git a/scripts/mod/devicetable-offsets.c b/scripts/mod/devicetable-offsets.c >> index b4178c42d08f..91ec3704ee2b 100644 >> --- a/scripts/mod/devicetable-offsets.c >> +++ b/scripts/mod/devicetable-offsets.c >> @@ -1,5 +1,6 @@ >> // SPDX-License-Identifier: GPL-2.0 >> #define COMPILE_OFFSETS >> +#include <linux/device-id/scmi.h> > > Drop this for above mentioned reason. This was explictly added at request of Uwe because the goal is to get rid of mod_devicetable.h including all the device-id headers in a big single catch-all include. This is done because currently many files include mod_devicetable.h and then if any of the device-id headers changes everything needs to be rebuild, making incremental kernel builds much slower. (and the same for the second include you point out) Also replying to your other question here: > Not related to above topic, but just wanted to check with Daniel if this > solves module auto-loading for you. Was there someone else who was testing > this series and was having issue with earlier versions or its you only > Hans de Goede and it was with v1/v2 when Bjorn posted them. I remember > something vaguely and couldn't trace back all the versions easily. I tested Bjorn's original patch-series but since that makes the kernel call modprobe it does not work when the scmi bus is enumerated during the initramfs phase and the modules are only part of the rootfs. So I've replaced this with a rewrite of Christian's approach to always instantiate standard protocol devices. The rewrite was done because of various lifetime management issues with Cristian's original approach found by Shashiko. Note that Bjorn's modprobe approach also only used the protocol id as modalias, which means that if a vendor protocol id of say 0x80 were used and we have multiple vendor protocol drivers in kernel which share 0x80 as id, modules from both vendors would get loaded. This is something which I did not realize at the time, but this is another good reason to go with the new approach from this version of the series. As for testing, I was the main reporter of this issue since I'm trying to get Qualcomm Snapdragon laptops to work OOTB with standard unmodified Fedora aarch64 live/installer media and scmi_cpufreq needs a workaround to be manually loaded there, see: https://fedoraproject.org/wiki/Snapdragon_WoA_Laptop_Install_(Fedora_Beta) As you can see only a few workarounds are still necessary and manual SCMI auto-loading is one of them. So I really hope that we can get this series merged in time for 7.4 . Regards, Hans ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v7 1/2] module: add SCMI device table alias support 2026-09-18 14:09 ` Hans de Goede @ 2026-09-18 20:39 ` Uwe Kleine-König 0 siblings, 0 replies; 13+ messages in thread From: Uwe Kleine-König @ 2026-09-18 20:39 UTC (permalink / raw) To: Hans de Goede Cc: Sudeep Holla, Bjorn Andersson, Cristian Marussi, Daniel Lezcano, Bjorn Andersson, Frank.Li, arm-scmi, linux-arm-kernel, linux-arm-msm, imx, linux-kernel [-- Attachment #1: Type: text/plain, Size: 2295 bytes --] On Fri, Sep 18, 2026 at 04:09:36PM +0200, Hans de Goede wrote: > Hi Sudeep, > > On 18-Sep-26 15:32, Sudeep Holla wrote: > > [...] > > >> 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]; > >> +}; > >> + > >> +#endif /* ifndef LINUX_DEVICE_ID_SCMI_H */ > > > > You need to include this above file in include/linux/mod_devicetable.h > > to avoid duplication of its inclusion elsewhere. That's the pattern followed > > I see. > > That is what I did for v3, but Uwe, who is leading the splitting > of mod_devicetable.h from one large monster include to smaller > per subsystem/bus includes explicitly requested me to follow this > pattern, see: > > https://lore.kernel.org/arm-scmi/aow2GFAfy84k-N2B@monoceros/ > > [...] > > >> diff --git a/scripts/mod/devicetable-offsets.c b/scripts/mod/devicetable-offsets.c > >> index b4178c42d08f..91ec3704ee2b 100644 > >> --- a/scripts/mod/devicetable-offsets.c > >> +++ b/scripts/mod/devicetable-offsets.c > >> @@ -1,5 +1,6 @@ > >> // SPDX-License-Identifier: GPL-2.0 > >> #define COMPILE_OFFSETS > >> +#include <linux/device-id/scmi.h> > > > > Drop this for above mentioned reason. > > This was explictly added at request of Uwe because the goal is to > get rid of mod_devicetable.h including all the device-id headers > in a big single catch-all include. This is done because currently > many files include mod_devicetable.h and then if any of the device-id > headers changes everything needs to be rebuild, making incremental > kernel builds much slower. > > (and the same for the second include you point out) FTR: That all sounds right. Let's not add the the dependency mess that is mod_devicetable.h. Best regards Uwe [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v7 2/2] firmware: arm_scmi: Always create devices for standard protocols 2026-09-18 9:29 [PATCH v7 0/2] firmware: arm_scmi: fix module auto-loading Hans de Goede 2026-09-18 9:29 ` [PATCH v7 1/2] module: add SCMI device table alias support Hans de Goede @ 2026-09-18 9:29 ` Hans de Goede 1 sibling, 0 replies; 13+ messages in thread From: Hans de Goede @ 2026-09-18 9:29 UTC (permalink / raw) To: Bjorn Andersson, Cristian Marussi, Sudeep Holla Cc: Hans de Goede, Daniel Lezcano, Bjorn Andersson, Frank.Li, arm-scmi, linux-arm-kernel, linux-arm-msm, imx, linux-kernel Protocol driver module auto-loading requires the devices to already be created for udev to get the necessary uevents based on which udev loads modules. But SCMI devices are only created after their { protocol, name } device-id has been added to the requested-devices list which is done from scmi_driver_register(). This creates a circular dependency where device creation is waiting for the driver to register and loading the module with the driver is waiting for the device to be created. Add a list of standard protocol device-ids and always create devices for these without relying on these being added to the requested-devices list. This removes the circular dependency, fixing module auto-loading. Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com> --- Changes in v7: - Rebase on top of v7.3-rc3 Changes in v6: - Replace the if (!phead) return NULL; early exit with return scmi_dev; Since a standard protocol device may have been created at this point (Shashiko). Changes in v5: - This is a new patch in v5 replacing "Pre-register protocol, name tupples for standard protocol". Pre-registering requires scmi_protocol_device_request() to allow duplicate device-ids, but then if the first driver of two with duplicate devce-ids gets unloaded device creation for the second driver will fail. Allowing duplicates causes all kinds of problems, so this new approach avoids this. --- drivers/firmware/arm_scmi/bus.c | 50 +++++++++++++++++++++++++++++++-- 1 file changed, 47 insertions(+), 3 deletions(-) diff --git a/drivers/firmware/arm_scmi/bus.c b/drivers/firmware/arm_scmi/bus.c index 51de50fbc382..31365d59b98a 100644 --- a/drivers/firmware/arm_scmi/bus.c +++ b/drivers/firmware/arm_scmi/bus.c @@ -533,6 +533,34 @@ _scmi_device_create(struct device_node *np, struct device *parent, return sdev; } +/* Standard protocols table */ +static const struct scmi_device_id scmi_std_id_table[] = { + { SCMI_PROTOCOL_POWER, "genpd" }, + { SCMI_PROTOCOL_SYSTEM, "syspower" }, + { SCMI_PROTOCOL_PERF, "perf" }, + { SCMI_PROTOCOL_PERF, "cpufreq" }, + { SCMI_PROTOCOL_CLOCK, "clocks" }, + { SCMI_PROTOCOL_SENSOR, "hwmon" }, + { SCMI_PROTOCOL_SENSOR, "iiodev" }, + { SCMI_PROTOCOL_RESET, "reset" }, + { SCMI_PROTOCOL_VOLTAGE, "regulator" }, + { SCMI_PROTOCOL_POWERCAP, "powercap" }, + { SCMI_PROTOCOL_PINCTRL, "pinctrl" }, + { SCMI_PROTOCOL_PINCTRL, "pinctrl-imx" }, + { }, +}; + +static bool scmi_device_id_in_std_id_table(const struct scmi_device_id *id) +{ + for (int i = 0; scmi_std_id_table[i].name[0]; i++) { + if (scmi_std_id_table[i].protocol_id == id->protocol_id && + !strcmp(scmi_std_id_table[i].name, id->name)) + return true; + } + + return false; +} + /** * scmi_device_create - A method to create one or more SCMI devices * @@ -562,22 +590,38 @@ struct scmi_device *scmi_device_create(struct device_node *np, { struct list_head *phead; struct scmi_requested_dev *rdev; - struct scmi_device *scmi_dev = NULL; + struct scmi_device *sdev, *scmi_dev = NULL; if (name) return _scmi_device_create(np, parent, protocol, name); + /* + * Always create devices for standard protocols, even if the device-ids + * have not been registered into scmi_requested_devices yet. This allows + * auto-loading of SCMI protocol driver modules for standard protocols. + */ + for (int i = 0; scmi_std_id_table[i].name[0]; i++) { + if (scmi_std_id_table[i].protocol_id != protocol) + continue; + + sdev = _scmi_device_create(np, parent, protocol, scmi_std_id_table[i].name); + if (sdev) + scmi_dev = sdev; + } + mutex_lock(&scmi_requested_devices_mtx); phead = idr_find(&scmi_requested_devices, protocol); /* Nothing to do. */ if (!phead) { mutex_unlock(&scmi_requested_devices_mtx); - return NULL; + return scmi_dev; } /* Walk the list of requested devices for protocol and create them */ list_for_each_entry(rdev, phead, node) { - struct scmi_device *sdev; + /* Standard proto matches already have their dev created above */ + if (scmi_device_id_in_std_id_table(rdev->id_table)) + continue; sdev = _scmi_device_create(np, parent, rdev->id_table->protocol_id, -- 2.55.0 ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2026-09-18 20:39 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-09-18 9:29 [PATCH v7 0/2] firmware: arm_scmi: fix module auto-loading Hans de Goede 2026-09-18 9:29 ` [PATCH v7 1/2] module: add SCMI device table alias support Hans de Goede 2026-09-18 9:53 ` Daniel Lezcano 2026-09-18 10:02 ` Hans de Goede 2026-09-18 10:13 ` Daniel Lezcano 2026-09-18 13:38 ` Sudeep Holla 2026-09-18 14:43 ` Rob Clark 2026-09-18 14:48 ` Daniel Lezcano 2026-09-18 20:15 ` Daniel Lezcano 2026-09-18 13:32 ` Sudeep Holla 2026-09-18 14:09 ` Hans de Goede 2026-09-18 20:39 ` Uwe Kleine-König 2026-09-18 9:29 ` [PATCH v7 2/2] firmware: arm_scmi: Always create devices for standard protocols Hans de Goede
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®