Re: [PATCH v3 2/3] clk: qcom: camcc-sm8250: use icc-clk for camera NoC AXI clock

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] clk: camcc-sm8250: premature icc_sync_state on old DT causes system hang
--

commit efec91c2d7f6b65bf67f582b282617ccb26d996e
Author: Atanas Filipov <atanas.filipov@oss.qualcomm.com>

clk: qcom: camcc-sm8250: use icc-clk for camera NoC AXI clock

CAM_CC_CAMNOC_AXI_CLK is shared by several camera blocks. Setting its
rate directly from each consumer means the last writer wins.

Register the clock as an icc-clk path through the common icc_hws
support, as gcc-ipq9574 does. The interconnect core aggregates all
requests and the clock runs at the highest requested rate.

Use the RCG rather than the branch: icc-clk enables the clock at
registration, when TITAN_TOP_GDSC is still off. Consumers enable the
branch themselves.

Signed-off-by: Atanas Filipov <atanas.filipov@oss.qualcomm.com>

> diff --git a/drivers/clk/qcom/camcc-sm8250.c b/drivers/clk/qcom/camcc-sm8250.c
> index 21e9423676216..84a91103e5bef 100644
> --- a/drivers/clk/qcom/camcc-sm8250.c
> +++ b/drivers/clk/qcom/camcc-sm8250.c

[ ... ]

> @@ -2443,6 +2454,7 @@ static struct platform_driver cam_cc_sm8250_driver = {
>  	.driver = {
>  		.name = "cam_cc-sm8250",
>  		.of_match_table = cam_cc_sm8250_match_table,
> +		.sync_state = icc_sync_state,

[Severity: High]
Could this cause premature state synchronization when booting with an older
Device Tree?

On older DTs where the camcc node lacks the #interconnect-cells property,
the interconnect core skips counting it during initialization:

drivers/interconnect/core.c:of_count_icc_providers() {
	...
	if (of_property_present(child, "#interconnect-cells") &&
	    likely(!of_match_node(ignore_list, child)))
		count++;
	...
}

Because of this, providers_count only reflects the other interconnect
providers. However, since the driver unconditionally registers
icc_sync_state, the driver core will still execute it once the device
probes.

When this executes, the extra call increments the static count in
icc_sync_state without a corresponding +1 in providers_count:

drivers/interconnect/core.c:icc_sync_state() {
	...
	count++;

	if (count < providers_count)
		return;
	...
}

Will this cause the interconnect core to reach providers_count prematurely,
forcing synced_state to true and dropping bandwidth requests on all other
system interconnects before their consumers have probed?

>  	},
>  };
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016170701.1504338-1-atanas.filipov@oss.qualcomm.com?part=2