Re: [PATCH v3 05/15] clk: qcom: clk-rpmh: Add support for Kuno RPMh clocks
flat view
From: sashiko-bot@kernel.org
Date: 2026-08-21 16:21:26
Also in:
linux-clk
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: Pre-existing issues: - [High] Probe function mutates globally shared static state, causing accumulative corruption on probe retry or unbind/bind. --
--- Patch [5]: [PATCH v3 05/15] clk: qcom: clk-rpmh: Add support for Kuno RPMh clocks ---commit 6a55939d348aa54a2946c4b7160f00630d3c9710 Author: Hardeep Sharma [off-list ref] clk: qcom: clk-rpmh: Add support for Kuno RPMh clocks Add the RPMh clock description for the Qualcomm Kuno SoC so the qcom,kuno-rpmh-clk compatible can provide the RPMh-managed clocks to consumers: the CXO div-2 (and always-on variant), RF clock 1 (and always-on variant), the QPIC BCM clock and the IPA clock.
quoted hunk ↗ jump to hunk
diff --git a/drivers/clk/qcom/clk-rpmh.c b/drivers/clk/qcom/clk-rpmh.c index 6367b2a5a4e08..d8c3a6b12d4a4 100644 --- a/drivers/clk/qcom/clk-rpmh.c +++ b/drivers/clk/qcom/clk-rpmh.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -943,6 +943,20 @@ static const struct clk_rpmh_desc clk_rpmh_kaanapali = { .num_clks = ARRAY_SIZE(kaanapali_rpmh_clocks), }; +static struct clk_hw *kuno_rpmh_clocks[] = { + [RPMH_CXO_CLK] = &clk_rpmh_bi_tcxo_div2.hw, + [RPMH_CXO_CLK_A] = &clk_rpmh_bi_tcxo_div2_ao.hw, + [RPMH_RF_CLK1] = &clk_rpmh_rf_clk1_a.hw, + [RPMH_RF_CLK1_A] = &clk_rpmh_rf_clk1_a_ao.hw, + [RPMH_QPIC_CLK] = &clk_rpmh_qpic_clk.hw, + [RPMH_IPA_CLK] = &clk_rpmh_ipa.hw, +};
[Severity: High]
This is a pre-existing issue, but does adding this new clock array expose it
to state corruption during probe retries or unbind/bind sequences?
Looking at clk_rpmh_probe() in drivers/clk/qcom/clk-rpmh.c, if a clock
resource is missing or if probe fails transitorily and is retried, the
function mutates the statically allocated array:
drivers/clk/qcom/clk-rpmh.c:clk_rpmh_probe() {
...
hw_clks[i] = NULL;
...
rpmh_clk->res_addr += res_addr;
rpmh_clk->dev = &pdev->dev;
...
}
Since kuno_rpmh_clocks is a globally shared static structure, writing NULL here
permanently erases the clock definition for any future bindings.
Additionally, modifying res_addr accumulates the hardware address on subsequent
probe attempts, leading to corrupted register offsets.
Could we avoid mutating these static clock structures by allocating a per-device
copy during probe, or isolating the runtime state from the static clock
descriptions?
quoted hunk ↗ jump to hunk
+ +static const struct clk_rpmh_desc clk_rpmh_kuno = { + .clks = kuno_rpmh_clocks, + .num_clks = ARRAY_SIZE(kuno_rpmh_clocks), +}; +
[ ... ] -- Sashiko AI review · https://sashiko.dev/#/patchset/20260821-kuno-soc-support-v3-0-196e2b306368@oss.qualcomm.com?part=5