Re: [PATCH v2 2/6] MIPS: ath79: move the bootstrap reference clock to setup

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] MIPS: ath79: missing OF clock provider on ref_np in
  ath79_setup_ref_clk()
--

commit ba6521274f3bdc958b3f63de9327877ef55638bc
Author: Rosen Penev <rosenp@gmail.com>

MIPS: ath79: move the bootstrap reference clock to setup

This commit moves the bootstrap reference clock detection to plat_time_init(),
registering a fixed-rate clock when the device tree omits clock-frequency and
warning on mismatches with the bootstrap pins.

> diff --git a/arch/mips/ath79/setup.c b/arch/mips/ath79/setup.c
> index d9e1cc77ad04d..dff0fbe07b047 100644
> --- a/arch/mips/ath79/setup.c
> +++ b/arch/mips/ath79/setup.c
[ ... ]
> +static void __init ath79_setup_ref_clk(void)
> +{
> +	struct device_node *np, *ref_np;
> +	unsigned long rate, strap_rate;
> +	struct clk *clk;
> +	bool strapped;
> +	u32 dt_rate;
> +
> +	strap_rate = ath79_bootstrap_ref_rate(&strapped);
> +
> +	np = of_find_matching_node(NULL, ath79_pll_ids);
> +	ref_np = np ? of_parse_phandle(np, "clocks", 0) : NULL;
> +	of_node_put(np);
> +
> +	if (ref_np && !of_property_read_u32(ref_np, "clock-frequency", &dt_rate)) {
> +		of_node_put(ref_np);
> +		rate = dt_rate;
> +		if (strapped && rate != strap_rate)
> +			pr_warn("ath79: reference clock is %lu Hz in the device tree, but the bootstrap pins select %lu Hz\n",
> +				rate, strap_rate);
> +		return;
> +	}
> +	of_node_put(ref_np);
> +
> +	clk = clk_register_fixed_rate(NULL, "ref", NULL, 0, strap_rate);
> +	if (IS_ERR(clk)) {
> +		pr_err("ath79: failed to register the reference clock\n");
> +		return;
> +	}
> +
> +	clk_register_clkdev(clk, "ref", NULL);
> +}

[Severity: Medium]
When ref_np exists in the device tree without a clock-frequency property, does
skipping of_clk_add_provider() for ref_np break DT consumers that reference
<&ref> by phandle?

In of_clk_init(), _of_fixed_clk_setup() fails when clock-frequency is absent,
leaving ref_np without an OF clock provider.  ath79_clocks_init_dt() works
around this by falling back to clk_get_sys(NULL, "ref"), but
arch/mips/boot/dts/qca/ar9331.dtsi also wires the UART to <&ref>:

arch/mips/boot/dts/qca/ar9331.dtsi:
	uart: serial@18020000 {
		compatible = "qca,ar9330-uart";
		...
		clocks = <&ref>;
		clock-names = "uart";

When ar933x_uart_probe() calls devm_clk_get(&pdev->dev, "uart"),
of_clk_get_hw() sees that ref_np is an available DT node with no registered OF
provider and returns -EPROBE_DEFER without falling back to clkdev, causing the
UART probe to fail permanently.

Registering clk as an OF clock provider on ref_np via of_clk_add_provider()
when ref_np is non-NULL would allow phandle lookups to succeed.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110013646.1582970-1-rosenp@gmail.com?part=2