Re: [PATCH v2] opp: Use clk_get_optional() to avoid leaving opp_table->clk as an error pointer

[Brian Masney]

On Sat, Jul 25, 2026 at 11:58:22PM +0530, Praveen Talari wrote:
> _update_opp_table_clk() uses clk_get(dev, NULL) to acquire the
> device's clock. On platforms where the perf domain device has no
> Linux clock and is instead managed entirely by firmware via
> devm_pm_opp_of_add_table() (through
> of_genpd_add_provider_simple()/onecell()), clk_get() returns
> -ENOENT. That case is treated as valid (the OPP table can still
> have entries sourced from firmware), but opp_table->clk is left
> holding ERR_PTR(-ENOENT) rather than being reset to NULL:
> 
> 	opp_table->clk = clk_get(dev, NULL);
> 	ret = PTR_ERR_OR_ZERO(opp_table->clk);
> 	...
> 	if (ret == -ENOENT) {
> 		opp_table->clk_count = 1;
> 		return opp_table;   /* opp_table->clk is still ERR_PTR(-ENOENT) */
> 	}
> 
> Consumers that only check IS_ERR(opp_table->clk) treat this as a
> valid clk and pass it straight into the clk consumer API. In
> particular, dev_pm_opp_set_rate() calls
> clk_round_rate(opp_table->clk, target_freq), and clk_round_rate()
> only guards against a NULL clk, so it dereferences the error pointer
> to read clk->exclusive_count and crashes:
> 
>   Unable to handle kernel NULL pointer dereference at virtual
>   address 000000000000002e
>   ...
>   pc : clk_round_rate+0x3c/0x188
>   ...
>   Call trace:
>    clk_round_rate+0x3c/0x188 (P)
>    dev_pm_opp_set_rate+0x114/0x33c
> 
> Rather than teaching every clk consumer API to special-case
> ERR_PTR(-ENOENT), fix it at the source: use clk_get_optional()
> instead of clk_get() in _update_opp_table_clk(), which already
> translates -ENOENT into a NULL clk. This documents that the clock is
> genuinely optional for such devices, and keeps opp_table->clk holding
> either a valid clk or NULL, never a lingering -ENOENT error pointer.
> _opp_config_clk_single() is only wired up via opp_table->config_clks
> when a clk was actually found, and every other opp_table->clk
> consumer already tolerates NULL through the standard clk API (which
> treats a NULL clk as a no-op), so no other call site needs to change.
> 
> Suggested-by: Sebastian Reichel <sre@xxxxxxxxxx>
> Signed-off-by: Praveen Talari <praveen.talari@xxxxxxxxxxxxxxxx>

Reviewed-by: Brian Masney <bmasney@xxxxxxxxxx>