Re: [PATCH v3 2/6] clk: socfpga: agilex5: add usb3.1 reference clock

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Returning an error from probe after registering non-devm managed clocks leaves dangling pointers to devm-managed I/O memory in the global clock tree.
--

--- Patch [2]: [PATCH v3 2/6] clk: socfpga: agilex5: add usb3.1 reference clock ---
commit 10f11218bd38bd2427f2a47b44a955f750a91676
Author: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>

clk: socfpga: agilex5: add usb3.1 reference clock

This commit adds a fixed-factor reference clock for the Agilex5 DWC3
controller so the driver can request each input by name.

> diff --git a/drivers/clk/socfpga/clk-agilex5.c b/drivers/clk/socfpga/clk-agilex5.c
> index f7f0ad884f645..af3009ddc6ab3 100644
> --- a/drivers/clk/socfpga/clk-agilex5.c
> +++ b/drivers/clk/socfpga/clk-agilex5.c
> @@ -526,6 +526,21 @@ static int agilex5_clkmgr_init(struct platform_device *pdev)
>  	agilex5_clk_register_gate(agilex5_gate_clks,
>  				  ARRAY_SIZE(agilex5_gate_clks), clk_data);
>  
> +	/*
> +	 * usb31_ref_clk is a 1:1 alias of usb31_suspend_clk. The DWC3
> +	 * controller uses the same source for both its suspend and reference
> +	 * clock inputs on this SoC. Register it as a fixed-factor (passthrough)
> +	 * clock so that enabling it propagates to the parent gate without
> +	 * touching any gate register of its own.
> +	 */
> +	clk_data->clk_data.hws[AGILEX5_USB31_REF_CLK] =
> +		devm_clk_hw_register_fixed_factor(dev, "usb31_ref_clk",
> +						  "usb31_suspend_clk", 0, 1, 1);
> +	if (IS_ERR(clk_data->clk_data.hws[AGILEX5_USB31_REF_CLK]))
> +		return dev_err_probe(dev,
> +			PTR_ERR(clk_data->clk_data.hws[AGILEX5_USB31_REF_CLK]),
> +			"failed to register clock usb31_ref_clk\n");

[Severity: High]
If devm_clk_hw_register_fixed_factor() fails and we return an error here,
will this leave dangling pointers in the Common Clock Framework?

The previous clocks registered earlier in agilex5_clkmgr_init() (like PLLs
and gates) are registered globally without devm management. Since clk_data
and the IO memory base are devm-managed, returning an error from probe
causes them to be freed and unmapped.

Could this cause the system to crash due to unmapped memory access when the
clock framework later accesses these dangling clocks (for example, during
clk_disable_unused())?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1786518803.git.adrian.ho.yin.ng@altera.com?part=2