Re: [PATCH v8 3/7] clk: nuvoton: ma35d1: Retrieve HXT/LXT from DT when possible

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] clk: nuvoton: ABI breakage by removing HXT from provider's hws array
--

commit 0466ee8e1824bb9d740e5cd18be8fe01b5d4fbe1
Author: Miquel Raynal <miquel.raynal@bootlin.com>

clk: nuvoton: ma35d1: Retrieve HXT/LXT from DT when possible

This commit modifies the clock driver to retrieve the external HXT and LXT
clocks from the device tree instead of registering them directly. It falls
back to registering LXT for backward compatibility with older device trees.

> diff --git a/drivers/clk/nuvoton/clk-ma35d1.c b/drivers/clk/nuvoton/clk-ma35d1.c
> index ceebcbd8c18b3..ed61a990870e4 100644
> --- a/drivers/clk/nuvoton/clk-ma35d1.c
> +++ b/drivers/clk/nuvoton/clk-ma35d1.c
[ ... ]
> @@ -240,12 +257,18 @@ static int ma35d1_clocks_probe(struct platform_device *pdev)
>  		return -EINVAL;
>  	}
>  
> -	hws[HXT] = ma35d1_clk_fixed("hxt", 24000000);

[Severity: Critical]
Does this unconditional removal break ABI compatibility with older device
trees?

Because HXT (index 0) is exported as a public ABI in the DT bindings,
removing its explicit registration leaves hws[HXT] as NULL in the provider's
clock array. If an existing downstream or older device tree consumer explicitly
references this clock via clocks = <&clk HXT>, won't the provider's
of_clk_hw_onecell_get() fail to resolve the clock and return an error?

> -	hws[HXT_GATE] = ma35d1_clk_gate(dev, "hxt_gate", "hxt",
> -					clk_base + REG_CLK_PWRCTL, 0);
> -	hws[LXT] = ma35d1_clk_fixed("lxt", 32768);
> -	hws[LXT_GATE] = ma35d1_clk_gate(dev, "lxt_gate", "lxt",
> -					clk_base + REG_CLK_PWRCTL, 1);
> +	num_clks = of_clk_get_parent_count(clk_node);
> +	if (num_clks < 1 || num_clks > 2)
> +		return -EINVAL;
> +
> +	/* Old DTs do not describe the low-speed crystal */
> +	if (num_clks == 1)
> +		hws[LXT] = ma35d1_clk_fixed("lxt", 32768);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261004-perso-ma35d1-upstream-clk-v8-0-e1a106a128c0@bootlin.com?part=3