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

[Jerome Brunet <jbrunet@baylibre.com>]

On lun. 29 juin 2026 at 13:15, sashiko-bot@kernel.org wrote:

> 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.

Forgot to remove that unused define

>
> 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?

I don't quite get what the AI is trying to report here. If there are no
user, the clock will be gated ... so ? Am I missing something ?

>
>> +	.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),
>> +	},
>> +};

-- 
Jerome