* [PATCH v3 0/3] firmware: arm_scmi: fix module auto-loading
@ 2026-08-20 19:48 Hans de Goede
2026-08-20 19:48 ` [PATCH v3 1/3] module: add SCMI device table alias support Hans de Goede
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Hans de Goede @ 2026-08-20 19:48 UTC (permalink / raw)
To: Bjorn Andersson, Cristian Marussi, Sudeep Holla
Cc: Hans de Goede, Daniel Lezcano, arm-scmi, linux-arm-kernel,
linux-arm-msm, imx, linux-kernel
Hi All,
Here is a 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. RFC 2/2 from Cristian which pre-populates the scmi { protocol, name }
tupple list with standard protocol info to break the circular dep:
https://patch.msgid.link/20250203100154.140877-2-cristian.marussi@arm.com
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 } tupples
have been registered which is done from scmi_driver_register(), creating
a circular dependency.
2. breaks the circular dependency by pre-populating the { protocol, name }
list with the standard protocols. This allows the devices to be created
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.
I've added a 3th patch extending the pre-populating to also include all
the { protocol, name } tupples for the in tree SCMI drivers for IMX
vendor protocols, so that module auto-loading will work for all in tree
drivers.
Out of tree drivers can still register new tupples as before but their
modules will need to be manually loaded (also as before).
Regards,
Hans
Bjorn Andersson (1):
module: add SCMI device table alias support
Cristian Marussi (1):
firmware: arm_scmi: Pre-register protocol, name tupples for standard
protocols
Hans de Goede (1):
firmware: arm_scmi: Pre-register protocol, name tupples for IMX
protocols
MAINTAINERS | 1 +
drivers/firmware/arm_scmi/bus.c | 75 +++++++++++++++++++++++--------
include/linux/device-id/scmi.h | 19 ++++++++
include/linux/mod_devicetable.h | 1 +
include/linux/scmi_protocol.h | 6 +--
scripts/mod/devicetable-offsets.c | 4 ++
scripts/mod/file2alias.c | 11 +++++
7 files changed, 94 insertions(+), 23 deletions(-)
create mode 100644 include/linux/device-id/scmi.h
--
2.55.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v3 1/3] module: add SCMI device table alias support
2026-08-20 19:48 [PATCH v3 0/3] firmware: arm_scmi: fix module auto-loading Hans de Goede
@ 2026-08-20 19:48 ` Hans de Goede
2026-08-24 12:22 ` Uwe Kleine-König
2026-08-20 19:48 ` [PATCH v3 2/3] firmware: arm_scmi: Pre-register protocol, name tupples for standard protocols Hans de Goede
2026-08-20 19:48 ` [PATCH v3 3/3] firmware: arm_scmi: Pre-register protocol, name tupples for IMX protocols Hans de Goede
2 siblings, 1 reply; 6+ messages in thread
From: Hans de Goede @ 2026-08-20 19:48 UTC (permalink / raw)
To: Bjorn Andersson, Cristian Marussi, Sudeep Holla
Cc: Hans de Goede, Daniel Lezcano, arm-scmi, linux-arm-kernel,
linux-arm-msm, imx, linux-kernel, Bjorn Andersson
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 v3:
- Adjust for ad428f5811bd ("mod_devicetable.h: Split into per subsystem
headers")
- Add '\n' to modalias_show() output, matching other subsystems' modalias
- Link to v2: https://lore.kernel.org/all/20260618-scmi-modalias-v2-1-8c7547c1be21@oss.qualcomm.com/
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 | 19 +++++++++++++++++++
include/linux/mod_devicetable.h | 1 +
include/linux/scmi_protocol.h | 6 +-----
scripts/mod/devicetable-offsets.c | 4 ++++
scripts/mod/file2alias.c | 11 +++++++++++
7 files changed, 47 insertions(+), 16 deletions(-)
create mode 100644 include/linux/device-id/scmi.h
diff --git a/MAINTAINERS b/MAINTAINERS
index fc6ca082106f..ee5beee606fc 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -26334,6 +26334,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 793be9eabaed..d12d5de15a1a 100644
--- a/drivers/firmware/arm_scmi/bus.c
+++ b/drivers/firmware/arm_scmi/bus.c
@@ -13,11 +13,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);
@@ -141,7 +142,7 @@ static int scmi_protocol_table_register(const struct scmi_device_id *id_table)
int ret = 0;
const struct scmi_device_id *entry;
- for (entry = id_table; entry->name && ret == 0; entry++)
+ for (entry = id_table; entry->name[0] && ret == 0; entry++)
ret = scmi_protocol_device_request(entry);
return ret;
@@ -197,18 +198,18 @@ 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);
}
static int scmi_dev_match_by_id_table(struct scmi_device *scmi_dev,
const struct scmi_device_id *id_table)
{
- if (!id_table || !id_table->name)
+ if (!id_table || !id_table->name[0])
return 0;
/* Always skip transport devices from matching */
- 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 &&
strncmp(scmi_dev->name, "__scmi_transport_device", 23) &&
!strcmp(id_table->name, scmi_dev->name))
@@ -245,7 +246,7 @@ static struct scmi_device *scmi_child_dev_find(struct device *parent,
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)
@@ -282,8 +283,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,
@@ -291,9 +291,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..4150481524d4
--- /dev/null
+++ b/include/linux/device-id/scmi.h
@@ -0,0 +1,19 @@
+/* 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>
+typedef unsigned long kernel_ulong_t;
+#endif
+
+#define SCMI_NAME_SIZE 32
+#define SCMI_MODULE_PREFIX "scmi:"
+
+struct scmi_device_id {
+ __u8 protocol_id;
+ char name[SCMI_NAME_SIZE];
+ kernel_ulong_t driver_data;
+};
+
+#endif /* ifndef LINUX_DEVICE_ID_SCMI_H */
diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
index a397213bedac..d241421c36aa 100644
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -49,6 +49,7 @@
#include "device-id/pnp.h"
#include "device-id/rio.h"
#include "device-id/rpmsg.h"
+#include "device-id/scmi.h"
#include "device-id/sdio.h"
#include "device-id/sdw.h"
#include "device-id/serio.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..da5bd712c8da 100644
--- a/scripts/mod/devicetable-offsets.c
+++ b/scripts/mod/devicetable-offsets.c
@@ -144,6 +144,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..a5283f4c8e6f 100644
--- a/scripts/mod/file2alias.c
+++ b/scripts/mod/file2alias.c
@@ -852,6 +852,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 +1501,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] 6+ messages in thread
* [PATCH v3 2/3] firmware: arm_scmi: Pre-register protocol, name tupples for standard protocols
2026-08-20 19:48 [PATCH v3 0/3] firmware: arm_scmi: fix module auto-loading Hans de Goede
2026-08-20 19:48 ` [PATCH v3 1/3] module: add SCMI device table alias support Hans de Goede
@ 2026-08-20 19:48 ` Hans de Goede
2026-08-20 19:48 ` [PATCH v3 3/3] firmware: arm_scmi: Pre-register protocol, name tupples for IMX protocols Hans de Goede
2 siblings, 0 replies; 6+ messages in thread
From: Hans de Goede @ 2026-08-20 19:48 UTC (permalink / raw)
To: Bjorn Andersson, Cristian Marussi, Sudeep Holla
Cc: Hans de Goede, Daniel Lezcano, arm-scmi, linux-arm-kernel,
linux-arm-msm, imx, linux-kernel
From: Cristian Marussi <cristian.marussi@arm.com>
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 } tupples
have been registered 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.
Pre-register the tupples for standard protocols to break this circular
dependency.
Tested-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
---
Changes in v3:
- Drop adding of a bus uevent function this is already done
- Update comments and commit message with a better explanation of why
- Link to v1/RFC: https://patch.msgid.link/20250203100154.140877-2-cristian.marussi@arm.com
---
drivers/firmware/arm_scmi/bus.c | 47 ++++++++++++++++++++++++++++-----
1 file changed, 40 insertions(+), 7 deletions(-)
diff --git a/drivers/firmware/arm_scmi/bus.c b/drivers/firmware/arm_scmi/bus.c
index d12d5de15a1a..111727904a89 100644
--- a/drivers/firmware/arm_scmi/bus.c
+++ b/drivers/firmware/arm_scmi/bus.c
@@ -77,12 +77,13 @@ static int scmi_protocol_device_request(const struct scmi_device_id *id_table)
if (phead) {
head = phead;
list_for_each_entry(rdev, head, node) {
+ /* pr_debug() because dups are expected for std protocols */
if (!strcmp(rdev->id_table->name, id_table->name)) {
- pr_err("Ignoring duplicate request [%d] %s\n",
- rdev->id_table->protocol_id,
- rdev->id_table->name);
- ret = -EINVAL;
- goto out;
+ pr_debug("Device already requested [%d] %s\n",
+ rdev->id_table->protocol_id,
+ rdev->id_table->name);
+ mutex_unlock(&scmi_requested_devices_mtx);
+ return 0;
}
}
}
@@ -579,17 +580,49 @@ static void scmi_devices_unregister(void)
bus_for_each_dev(&scmi_bus_type, NULL, NULL, __scmi_devices_unregister);
}
+/* 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 int __init scmi_bus_init(void)
{
int retval;
retval = bus_register(&scmi_bus_type);
- if (retval)
+ if (retval) {
pr_err("SCMI protocol bus register failed (%d)\n", retval);
+ return retval;
+ }
+
+ /*
+ * Driver module auto-loading requires the devices to already be created
+ * for udev to get the necessary uevents. But the devices are only
+ * created after their { protocol, name } tupples have been registered
+ * which is done from scmi_driver_register(). Pre-register the tupples
+ * for known (in tree) drivers to break this circular dependency.
+ */
+ retval = scmi_protocol_table_register(scmi_std_id_table);
+ if (retval) {
+ bus_unregister(&scmi_bus_type);
+ return retval;
+ }
pr_info("SCMI protocol bus registered\n");
- return retval;
+ return 0;
}
subsys_initcall(scmi_bus_init);
--
2.55.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v3 3/3] firmware: arm_scmi: Pre-register protocol, name tupples for IMX protocols
2026-08-20 19:48 [PATCH v3 0/3] firmware: arm_scmi: fix module auto-loading Hans de Goede
2026-08-20 19:48 ` [PATCH v3 1/3] module: add SCMI device table alias support Hans de Goede
2026-08-20 19:48 ` [PATCH v3 2/3] firmware: arm_scmi: Pre-register protocol, name tupples for standard protocols Hans de Goede
@ 2026-08-20 19:48 ` Hans de Goede
2 siblings, 0 replies; 6+ messages in thread
From: Hans de Goede @ 2026-08-20 19:48 UTC (permalink / raw)
To: Bjorn Andersson, Cristian Marussi, Sudeep Holla
Cc: Hans de Goede, Daniel Lezcano, arm-scmi, linux-arm-kernel,
linux-arm-msm, imx, linux-kernel
There are in tree drivers for various IMX vendor protocols, also
pre-register the { protocol, name } tupples for these to make module
auto-loading work for these in tree drivers.
Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
---
Changes in v3:
- New patch in v3 of this patch-set
---
drivers/firmware/arm_scmi/bus.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/firmware/arm_scmi/bus.c b/drivers/firmware/arm_scmi/bus.c
index 111727904a89..9f5740dc606f 100644
--- a/drivers/firmware/arm_scmi/bus.c
+++ b/drivers/firmware/arm_scmi/bus.c
@@ -15,6 +15,7 @@
#include <linux/slab.h>
#include <linux/string.h>
#include <linux/device.h>
+#include <linux/scmi_imx_protocol.h>
#include "common.h"
@@ -580,8 +581,8 @@ static void scmi_devices_unregister(void)
bus_for_each_dev(&scmi_bus_type, NULL, NULL, __scmi_devices_unregister);
}
-/* Standard protocols table */
static const struct scmi_device_id scmi_std_id_table[] = {
+ /* Standard protocols */
{ SCMI_PROTOCOL_POWER, "genpd" },
{ SCMI_PROTOCOL_SYSTEM, "syspower" },
{ SCMI_PROTOCOL_PERF, "perf" },
@@ -594,6 +595,12 @@ static const struct scmi_device_id scmi_std_id_table[] = {
{ SCMI_PROTOCOL_POWERCAP, "powercap" },
{ SCMI_PROTOCOL_PINCTRL, "pinctrl" },
{ SCMI_PROTOCOL_PINCTRL, "pinctrl-imx" },
+ /* Vendor protocols with in tree drivers */
+ { SCMI_PROTOCOL_IMX_BBM, "imx-bbm-key" },
+ { SCMI_PROTOCOL_IMX_BBM, "imx-bbm-rtc" },
+ { SCMI_PROTOCOL_IMX_CPU, "imx-cpu" },
+ { SCMI_PROTOCOL_IMX_LMM, "imx-lmm" },
+ { SCMI_PROTOCOL_IMX_MISC, "imx-misc-ctrl" },
{ },
};
--
2.55.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/3] module: add SCMI device table alias support
2026-08-20 19:48 ` [PATCH v3 1/3] module: add SCMI device table alias support Hans de Goede
@ 2026-08-24 12:22 ` Uwe Kleine-König
2026-08-24 14:58 ` Hans de Goede
0 siblings, 1 reply; 6+ messages in thread
From: Uwe Kleine-König @ 2026-08-24 12:22 UTC (permalink / raw)
To: Hans de Goede
Cc: Bjorn Andersson, Cristian Marussi, Sudeep Holla, Daniel Lezcano,
arm-scmi, linux-arm-kernel, linux-arm-msm, imx, linux-kernel,
Bjorn Andersson
[-- Attachment #1: Type: text/plain, Size: 1890 bytes --]
Hello,
I just replied to v2 and only then found out there is a v3 already.
There is something wrong with my mail filtering, sorry.
I'm forwarding my review comments to here (the part that still applies).
On Thu, Aug 20, 2026 at 09:48:48PM +0200, Hans de Goede wrote:
> diff --git a/include/linux/device-id/scmi.h b/include/linux/device-id/scmi.h
> new file mode 100644
> index 000000000000..4150481524d4
> --- /dev/null
> +++ b/include/linux/device-id/scmi.h
> @@ -0,0 +1,19 @@
> +/* 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>
> +typedef unsigned long kernel_ulong_t;
> +#endif
> +
> +#define SCMI_NAME_SIZE 32
> +#define SCMI_MODULE_PREFIX "scmi:"
> +
> +struct scmi_device_id {
> + __u8 protocol_id;
> + char name[SCMI_NAME_SIZE];
> + kernel_ulong_t driver_data;
Please use
union {
kernel_ulong_t driver_data;
const void *driver_data_ptr;
};
here instead of the plain driver_data member (or don't add driver_data
at all if not needed). This yields better type-safety, see e.g.
https://lore.kernel.org/all/cover.1780048925.git.u.kleine-koenig@baylibre.com
for an extended explanation.
> +};
> +
> +#endif /* ifndef LINUX_DEVICE_ID_SCMI_H */
> diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
> index a397213bedac..d241421c36aa 100644
> --- a/include/linux/mod_devicetable.h
> +++ b/include/linux/mod_devicetable.h
> @@ -49,6 +49,7 @@
> #include "device-id/pnp.h"
> #include "device-id/rio.h"
> #include "device-id/rpmsg.h"
> +#include "device-id/scmi.h"
Please don't add new includes to mod_devicetable.h. That header is ugly
and each include makes it worse. Just add <linux/device-id/scmi.h> to
the modpost source.
> #include "device-id/sdio.h"
> #include "device-id/sdw.h"
> #include "device-id/serio.h"
Thanks
Uwe
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/3] module: add SCMI device table alias support
2026-08-24 12:22 ` Uwe Kleine-König
@ 2026-08-24 14:58 ` Hans de Goede
0 siblings, 0 replies; 6+ messages in thread
From: Hans de Goede @ 2026-08-24 14:58 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Bjorn Andersson, Cristian Marussi, Sudeep Holla, Daniel Lezcano,
arm-scmi, linux-arm-kernel, linux-arm-msm, imx, linux-kernel,
Bjorn Andersson
Hi Uwe,
On 24-Aug-26 14:22, Uwe Kleine-König wrote:
> Hello,
>
> I just replied to v2 and only then found out there is a v3 already.
> There is something wrong with my mail filtering, sorry.
Not sure if it is your filtering, v2 is somewhat old
and I only just revived this series.
I'll add you to the Cc list for v4.
> I'm forwarding my review comments to here (the part that still applies).
>
> On Thu, Aug 20, 2026 at 09:48:48PM +0200, Hans de Goede wrote:
>> diff --git a/include/linux/device-id/scmi.h b/include/linux/device-id/scmi.h
>> new file mode 100644
>> index 000000000000..4150481524d4
>> --- /dev/null
>> +++ b/include/linux/device-id/scmi.h
>> @@ -0,0 +1,19 @@
>> +/* 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>
>> +typedef unsigned long kernel_ulong_t;
>> +#endif
>> +
>> +#define SCMI_NAME_SIZE 32
>> +#define SCMI_MODULE_PREFIX "scmi:"
>> +
>> +struct scmi_device_id {
>> + __u8 protocol_id;
>> + char name[SCMI_NAME_SIZE];
>> + kernel_ulong_t driver_data;
>
> Please use
>
> union {
> kernel_ulong_t driver_data;
> const void *driver_data_ptr;
> };
>
> here instead of the plain driver_data member (or don't add driver_data
> at all if not needed). This yields better type-safety, see e.g.
> https://lore.kernel.org/all/cover.1780048925.git.u.kleine-koenig@baylibre.com
> for an extended explanation.
Ack, I'll check if driver_data is used at all.
>
>> +};
>> +
>> +#endif /* ifndef LINUX_DEVICE_ID_SCMI_H */
>> diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
>> index a397213bedac..d241421c36aa 100644
>> --- a/include/linux/mod_devicetable.h
>> +++ b/include/linux/mod_devicetable.h
>> @@ -49,6 +49,7 @@
>> #include "device-id/pnp.h"
>> #include "device-id/rio.h"
>> #include "device-id/rpmsg.h"
>> +#include "device-id/scmi.h"
>
> Please don't add new includes to mod_devicetable.h. That header is ugly
> and each include makes it worse. Just add <linux/device-id/scmi.h> to
> the modpost source.
Hmm, I did that at first, but that is quite ugly also because
file2alias.c, does:
#include "../../include/linux/mod_devicetable.h"
So I ended up needing to change that to:
#include "../../include/linux/device-id/scmi.h"
#include "../../include/linux/mod_devicetable.h"
my thinking was that the purpose would be to move other
consumers of mod_devicetable.h to directly include
linux/device-id/*.h"
And keep mod_devicetable.h as a file which includes all
of them for use in devicetable-offsets.c + file2alias.c
But I can see how actually getting there will be a long time
in the making and in the mean time this will cause longer
compile times.
So if you're saying the somewhat ugly thing of adding
linux/device-id/scmi.h directly to devicetable-offsets.c +
file2alias.c is the right thing to do, then I'm happy to
do that for v4.
If I'm reading your comment correctly, that is what you
want me to do, right ?
Regards,
Hans
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-08-24 14:58 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-08-20 19:48 [PATCH v3 0/3] firmware: arm_scmi: fix module auto-loading Hans de Goede
2026-08-20 19:48 ` [PATCH v3 1/3] module: add SCMI device table alias support Hans de Goede
2026-08-24 12:22 ` Uwe Kleine-König
2026-08-24 14:58 ` Hans de Goede
2026-08-20 19:48 ` [PATCH v3 2/3] firmware: arm_scmi: Pre-register protocol, name tupples for standard protocols Hans de Goede
2026-08-20 19:48 ` [PATCH v3 3/3] firmware: arm_scmi: Pre-register protocol, name tupples for IMX 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®