Re: [PATCH 2/2] firmware: arm_scmi: request modules for discovered protocols

Hi,

On 16-Jun-26 20:09, Bjorn Andersson wrote:
> SCMI client devices are created from SCMI driver id tables. If such a
> driver is modular, the core does not know the driver's client name until
> the module has already loaded, so normal device uevent based autoloading
> cannot break the dependency cycle.
> 
> Emit a protocol-level alias for each SCMI device id table entry and
> request that alias when the SCMI core discovers an implemented protocol.
> This loads modules that have registered interest in the protocol; their
> normal SCMI driver registration then requests the concrete client device
> and the SCMI bus matches it by protocol and name.
> 
> This allows e.g. ARM_SCMI_CPUFREQ=m to autoload on systems that expose
> only the SCMI Performance protocol node, where the cpufreq client name
> is Linux-internal and not available from firmware before loading the
> module.
> 
> Assisted-by: Codex:GPT-5.5
> Signed-off-by: Bjorn Andersson <[email protected]>
> ---
>  drivers/firmware/arm_scmi/driver.c | 2 ++
>  include/linux/mod_devicetable.h    | 1 +
>  scripts/mod/file2alias.c           | 4 +++-
>  3 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
> index 3e0d975ec94c..8538eedc7c3a 100644
> --- a/drivers/firmware/arm_scmi/driver.c
> +++ b/drivers/firmware/arm_scmi/driver.c
> @@ -47,6 +47,7 @@
>  #include <trace/events/scmi.h>
>  
>  #define SCMI_VENDOR_MODULE_ALIAS_FMT	"scmi-protocol-0x%02x-%s"
> +#define SCMI_MODULE_ALIAS_FMT		SCMI_PROTOCOL_MODULE_PREFIX "0x%02x"
>  
>  static DEFINE_IDA(scmi_id);
>  
> @@ -3362,6 +3363,7 @@ static int scmi_probe(struct platform_device *pdev)
>  		}
>  
>  		of_node_get(child);
> +		request_module(SCMI_MODULE_ALIAS_FMT, prot_id);

I think it would be better to use request_module_nowait() here. AFAICT there
is no need to synchronously wait here for the module to actually get loaded.

Either way the patch looks good to me:

Reviewed-by: Hans de Goede <[email protected]>

Regards,

Hans






 


>  		scmi_create_protocol_devices(child, info, prot_id, NULL);
>  	}
>  
> diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
> index 769382f2eadd..2cc7e78e35a3 100644
> --- a/include/linux/mod_devicetable.h
> +++ b/include/linux/mod_devicetable.h
> @@ -477,6 +477,7 @@ struct rpmsg_device_id {
>  
>  #define SCMI_NAME_SIZE		32
>  #define SCMI_MODULE_PREFIX	"scmi:"
> +#define SCMI_PROTOCOL_MODULE_PREFIX	"scmi-protocol-"
>  
>  struct scmi_device_id {
>  	__u8 protocol_id;
> diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
> index a5283f4c8e6f..40a37b6bf1ad 100644
> --- a/scripts/mod/file2alias.c
> +++ b/scripts/mod/file2alias.c
> @@ -852,7 +852,7 @@ 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 */
> +/* Looks like: scmi:NN:S and scmi-protocol-0xNN */
>  static void do_scmi_entry(struct module *mod, void *symval)
>  {
>  	DEF_FIELD(symval, scmi_device_id, protocol_id);
> @@ -860,6 +860,8 @@ static void do_scmi_entry(struct module *mod, void *symval)
>  
>  	module_alias_printf(mod, false, SCMI_MODULE_PREFIX "%02x:%s",
>  			    protocol_id, *name);
> +	module_alias_printf(mod, false, SCMI_PROTOCOL_MODULE_PREFIX "0x%02x",
> +			    protocol_id);
>  }
>  
>  /* Looks like: i2c:S */
>