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

[Daniel Lezcano]

Hi Hans,

thanks for taking care of that


On 9/18/26 11:29, Hans de Goede wrote:
> From: Bjorn Andersson <bjorn.andersson@xxxxxxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxxxxxx>
> Tested-by: Hans de Goede <johannes.goede@xxxxxxxxxxxxxxxx>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@xxxxxxxxxxxxxxxx>
> Signed-off-by: Hans de Goede <johannes.goede@xxxxxxxxxxxxxxxx>
> ---

[ ... ]

>   
> -#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 ?

[ ... ]