Re: [PATCH v2 08/13] firmware: arm_scmi: Harden clock protocol initialization

[Marek Szyprowski]

On 10.03.2026 19:40, Cristian Marussi wrote:
> Add proper error handling on failure to enumerate clocks features or
> rates.
>
> Signed-off-by: Cristian Marussi <cristian.marussi@xxxxxxx>

This patch landed yesterday in linux-next as commit 0d8b0c8068a8 
("firmware: arm_scmi: Harden clock protocol initialization"). In my 
tests I found that it causes a regression on RK3568 Odroid-M1 board 
(arch/arm64/boot/dts/rockchip/rk3568-odroid-m1.dts), cpufreq and GPU 
device are not probed properly:

# dmesg | grep scmi
scmi_core: SCMI protocol bus registered
arm-scmi arm-scmi.0.auto: Using scmi_smc_transport
arm-scmi arm-scmi.0.auto: SCMI max-rx-timeout: 30ms / max-msg-size: 
104bytes / max-msg: 20
scmi_protocol scmi_dev.1: Enabled polling mode TX channel - prot_id:16
arm-scmi arm-scmi.0.auto: SCMI Notifications - Core Enabled.
arm-scmi arm-scmi.0.auto: Malformed reply - real_sz:8 calc_sz:4  
(loop_num_ret:1)
arm-scmi arm-scmi.0.auto: SCMI Protocol v2.0 'rockchip:' Firmware 
version 0x0
arm-scmi arm-scmi.0.auto: Enabling SCMI Quirk 
[quirk_clock_rates_triplet_out_of_spec]
scmi-clocks scmi_dev.3: probe with driver scmi-clocks failed with error -22

# cat /sys/kernel/debug/devices_deferred
fde60000.gpu
cpufreq-dt

# dmesg | grep fde60000.gpu
rockchip-pm-domain fdd90000.power-management:power-controller: 
sync_state() pending due to fde60000.gpu
panfrost fde60000.gpu: get clock failed -517
panfrost fde60000.gpu: clk init failed -517
panfrost fde60000.gpu: get clock failed -517
panfrost fde60000.gpu: clk init failed -517

...

> ---
>   drivers/firmware/arm_scmi/clock.c | 22 ++++++++++++++++------
>   1 file changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/firmware/arm_scmi/clock.c b/drivers/firmware/arm_scmi/clock.c
> index c9b62edce4fd..bf956305a8fe 100644
> --- a/drivers/firmware/arm_scmi/clock.c
> +++ b/drivers/firmware/arm_scmi/clock.c
> @@ -402,10 +402,16 @@ static int scmi_clock_attributes_get(const struct scmi_protocol_handle *ph,
>   		    SUPPORTS_RATE_CHANGE_REQUESTED_NOTIF(attributes))
>   			clk->rate_change_requested_notifications = true;
>   		if (PROTOCOL_REV_MAJOR(ph->version) >= 0x3) {
> -			if (SUPPORTS_PARENT_CLOCK(attributes))
> -				scmi_clock_possible_parents(ph, clk_id, cinfo);
> -			if (SUPPORTS_GET_PERMISSIONS(attributes))
> -				scmi_clock_get_permissions(ph, clk_id, clk);
> +			if (SUPPORTS_PARENT_CLOCK(attributes)) {
> +				ret = scmi_clock_possible_parents(ph, clk_id, cinfo);
> +				if (ret)
> +					return ret;
> +			}
> +			if (SUPPORTS_GET_PERMISSIONS(attributes)) {
> +				ret = scmi_clock_get_permissions(ph, clk_id, clk);
> +				if (ret)
> +					return ret;
> +			}
>   			if (SUPPORTS_EXTENDED_CONFIG(attributes))
>   				clk->extended_config = true;
>   		}
> @@ -1143,8 +1149,12 @@ static int scmi_clock_protocol_init(const struct scmi_protocol_handle *ph)
>   	for (clkid = 0; clkid < cinfo->num_clocks; clkid++) {
>   		cinfo->clkds[clkid].id = clkid;
>   		ret = scmi_clock_attributes_get(ph, clkid, cinfo);
> -		if (!ret)
> -			scmi_clock_describe_rates_get(ph, clkid, cinfo);
> +		if (ret)
> +			return ret;
> +
> +		ret = scmi_clock_describe_rates_get(ph, clkid, cinfo);
> +		if (ret)
> +			return ret;
>   	}
>   
>   	if (PROTOCOL_REV_MAJOR(ph->version) >= 0x3) {

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland