Re: [PATCH v2 5/8] clk: clocking-wizard: Skip s_axi_aclk for static-config

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Double registration of the same `struct notifier_block` instance into two different SRCU notifier chains.
--

--- Patch [5]: [PATCH v2 5/8] clk: clocking-wizard: Skip s_axi_aclk for static-config ---
commit 02fdca3c9a0733ff035a7b08c1d76f778176b4c1
Author: Shubhrajyoti Datta <shubhrajyoti.datta@amd.com>
Subject: clk: clocking-wizard: Skip s_axi_aclk for static-config

This patch moves the axi_clk setup into the non-static-config branch,
skipping the s_axi_aclk initialization when the AXI bus interface is not
used in static-config mode.

> diff --git a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
> index 5470a717fccc8..fe73ee02b54ee 100644
> --- a/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
> +++ b/drivers/clk/xilinx/clk-xlnx-clock-wizard.c
[ ... ]
> @@ -1258,22 +1257,24 @@ static int clk_wzrd_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  	platform_set_drvdata(pdev, clk_wzrd);
>  
> -	clk_wzrd->axi_clk = devm_clk_get_enabled(&pdev->dev, "s_axi_aclk");
> -	if (IS_ERR(clk_wzrd->axi_clk))
> -		return dev_err_probe(&pdev->dev, PTR_ERR(clk_wzrd->axi_clk),
> -				     "s_axi_aclk not found\n");
> -	rate = clk_get_rate(clk_wzrd->axi_clk);
> -	if (rate > WZRD_ACLK_MAX_FREQ) {
> -		dev_err(&pdev->dev, "s_axi_aclk frequency (%lu) too high\n", rate);
> -		return -EINVAL;
> -	}
> -
>  	clk_wzrd->clk_in1 = devm_clk_get(&pdev->dev, "clk_in1");
>  	if (IS_ERR(clk_wzrd->clk_in1))
>  		return dev_err_probe(&pdev->dev, PTR_ERR(clk_wzrd->clk_in1),
>  				     "failed to get clk_in1\n");
>  
>  	if (!of_property_present(np, "xlnx,static-config")) {
> +		unsigned long rate;
> +
> +		clk_wzrd->axi_clk = devm_clk_get_enabled(&pdev->dev, "s_axi_aclk");

[Severity: High]
This is a pre-existing issue, but since we are modifying the setup of
axi_clk here, does the subsequent notifier registration corrupt the core
clock notifier lists?

Looking further down in drivers/clk/xilinx/clk-xlnx-clock-wizard.c in
clk_wzrd_probe(), the exact same notifier block instance is registered to
two different clocks unconditionally when xlnx,speed-grade is set:

    if (clk_wzrd->speed_grade) {
        clk_wzrd->nb.notifier_call = clk_wzrd_clk_notifier;

        ret = devm_clk_notifier_register(&pdev->dev, clk_wzrd->clk_in1,
                                         &clk_wzrd->nb);
        ...
        ret = devm_clk_notifier_register(&pdev->dev, clk_wzrd->axi_clk,
                                         &clk_wzrd->nb);
    }

Since srcu_notifier_chain_register() modifies the next pointer of the
passed notifier_block, doesn't the second registration overwrite the next
pointer established by the first?

Could this break the clk_in1 notifier list, causing its traversal to follow
into the axi_clk notifier list, potentially leading to an infinite loop,
skipped notifiers, or a kernel panic upon subsequent rate changes?

> +		if (IS_ERR(clk_wzrd->axi_clk))
> +			return dev_err_probe(&pdev->dev, PTR_ERR(clk_wzrd->axi_clk),
> +					     "s_axi_aclk not found\n");

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709111403.1579159-1-shubhrajyoti.datta@amd.com?part=5