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

[sashiko-bot@kernel.org]

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

--- Patch [3]: [PATCH v2 3/8] clk: sunxi-ng: a733: Add PRCM CCU ---
commit 52650c9e3c767283edee5eb6338cdb409a63f50e
Author: Junhui Liu <junhui.liu@pigmoral.tech>

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) },
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260711-a733-clk-v2-0-974d188cbe0c@pigmoral.tech?part=3