Re: [PATCH v2 3/8] clk: sunxi-ng: a733: Add PRCM CCU

On Sun, 12 Jul 2026 08:13:39 +0000
[email protected] wrote:

> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Medium] Missing reset controls for hardware modules R-TWD, R-PPU, R-TZMA, and R-CPU-BIST.

Those devices indeed have no reset controls, so the code is right.

> - [Medium] The mux shift for `r_cpu_timer0..3` is incorrectly set to 4, which is anomalous for PRCM CCU clocks and likely a copy-paste error.

Those timer control register differs from the other ones, so the code
is correct: CLK_SRC_SEL is bits [6:4], S_TIMER<n>_CLK_GATING is bit 0.

Cheers,
Andre

> --
> 
> --- Patch [3]: [PATCH v2 3/8] clk: sunxi-ng: a733: Add PRCM CCU ---
> commit 52650c9e3c767283edee5eb6338cdb409a63f50e
> Author: Junhui Liu <[email protected]>
> 
> clk: sunxi-ng: a733: Add PRCM CCU
> 
> Add support for the Power Reset Clock Management (PRCM) module found in
> the Allwinner A733 SoC. This clock controller manages the clock control
> and reset functions for device modules within the CPUS domain.
> 
> > diff --git a/drivers/clk/sunxi-ng/ccu-sun60i-a733-r.c b/drivers/clk/sunxi-ng/ccu-sun60i-a733-r.c
> > new file mode 100644
> > index 0000000000000..a56cd3edff5fe
> > --- /dev/null
> > +++ b/drivers/clk/sunxi-ng/ccu-sun60i-a733-r.c  
> [ ... ]
> > +static SUNXI_CCU_P_DATA_WITH_MUX_GATE(r_cpu_timer0, "r-timer0", r_apb_parents, 0x100,
> > +				      1, 3,	/* P */
> > +				      4, 3,	/* mux */
> > +				      BIT(0),	/* gate */
> > +				      0);
> > +static SUNXI_CCU_P_DATA_WITH_MUX_GATE(r_cpu_timer1, "r-timer1", r_apb_parents, 0x104,
> > +				      1, 3,	/* P */
> > +				      4, 3,	/* mux */
> > +				      BIT(0),	/* gate */
> > +				      0);
> > +static SUNXI_CCU_P_DATA_WITH_MUX_GATE(r_cpu_timer2, "r-timer2", r_apb_parents, 0x108,
> > +				      1, 3,	/* P */
> > +				      4, 3,	/* mux */
> > +				      BIT(0),	/* gate */
> > +				      0);
> > +static SUNXI_CCU_P_DATA_WITH_MUX_GATE(r_cpu_timer3, "r-timer3", r_apb_parents, 0x10c,
> > +				      1, 3,	/* P */
> > +				      4, 3,	/* mux */
> > +				      BIT(0),	/* gate */
> > +				      0);  
> 
> [Severity: Medium]
> Are the mux shift values for r_cpu_timer0 through r_cpu_timer3 correct?
> 
> Most of the other PRCM clocks in this driver use bit 24 for the mux shift.
> If the hardware actually expects the mux configuration at bit 24, writing
> to bits 4-6 might fail to select the correct parent clock and could
> accidentally overwrite other configuration bits, resulting in incorrect
> timer frequencies.
> 
> [ ... ]
> > +static struct ccu_reset_map sun60i_a733_r_ccu_resets[] = {
> > +	[RST_BUS_R_TIMER]	= { 0x11c, BIT(16) },
> > +	[RST_BUS_R_PWM]		= { 0x13c, BIT(16) },  
> 
> [Severity: Medium]
> Should there be reset controls for the R-TWD, R-PPU, R-TZMA, and
> R-CPU-BIST modules mapped here?
> 
> These modules have clock gate offsets defined (0x12c, 0x1ac, 0x1b0, 0x1bc)
> elsewhere in the driver, but are omitted from this reset list. If they do
> have corresponding reset lines, missing them could prevent their drivers
> from taking the hardware out of reset during initialization, potentially
> causing uninitialized states or bus hangs upon access.
> 
> > +	[RST_BUS_R_SPI]		= { 0x15c, BIT(16) },
> > +	[RST_BUS_R_MSGBOX]	= { 0x17c, BIT(16) },  
> [ ... ]
>