Re: [PATCH v2 6/6] clk: sunxi-ng: sun6i-rtc: add a733 support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] The master DCXO enable bit is incorrectly mapped as the exclusive enable bit for the 32kHz divider. This will cause the clock framework to inadvertently turn off the master oscillator, freezing the system.
--

commit 83ebb861eeea7934e5df335b0d1620cad0b733bf
Author: Jerome Brunet <jbrunet@baylibre.com>

clk: sunxi-ng: sun6i-rtc: add a733 support

This commit adds support for the sun60i a733 CCU RTC. It introduces a
new auto-detecting oscillator divider and several phy reference clocks
with dedicated clock gates.

> diff --git a/drivers/clk/sunxi-ng/ccu-sun6i-rtc.c b/drivers/clk/sunxi-ng/ccu-sun6i-rtc.c
> index 25dd87e78eb7a..2ec09e3464724 100644
> --- a/drivers/clk/sunxi-ng/ccu-sun6i-rtc.c
> +++ b/drivers/clk/sunxi-ng/ccu-sun6i-rtc.c

[ ... ]

> @@ -227,8 +237,28 @@ static struct clk_fixed_factor osc24M_32k_div_clk = {
>  					    0),
>  };
>  
> -static SUNXI_CCU_GATE_HW(osc24M_32k_clk, "osc24M-32k", &osc24M_32k_div_clk.hw,
> -			 LOSC_OUT_GATING_REG, BIT(16), 0);
> +static struct clk_div_table osc24M_32k_div_a733_table[] = {
> +	{ .val = 0, .div = 732 },
> +	{ .val = 1, .div = 586 },
> +	{ .val = 2, .div = 793 },
> +	{ .val = 3, .div = 732 },
> +	{ /* Sentinel */ },
> +};
> +
> +static struct ccu_div osc24M_32k_div_a733_clk = {
> +	.enable = BIT(1),

[Severity: Critical]
This maps the master DCXO enable bit, BIT(1) defined earlier as
DCXO_CTRL_DCXO_EN, directly to the 32kHz divider's lifecycle.

If a board uses an external 32kHz crystal, the rtc-32k clock mux is
reparented to osc32k_clk, leaving this internal osc24M-32k-div clock with
zero active consumers.

During late boot, the clock framework's clk_disable_unused() routine walks
the clock tree and will disable this clock, clearing BIT(1) in DCXO_CTRL_REG.

Could this cause a hard system freeze? Since this bit physically turns off
the shared master 24MHz crystal (DCXO), does it immediately stop the CPU,
PLLs, and all peripherals?

> +	.div	= _SUNXI_CCU_DIV_TABLE(14, 2, osc24M_32k_div_a733_table),
> +	.common	= {
> +		.reg		= DCXO_CTRL_REG,
> +		.hw.init	= CLK_HW_INIT_PARENTS_DATA("osc24M-32k-div",
> +							   osc24M,
> +							   &ccu_rodiv_ops,
> +							   0),
> +	},
> +};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260629-a733-rtc-v2-0-7b72112784f8@baylibre.com?part=6