Re: [PATCH v4 02/11] clk: sunxi-ng: add Allwinner B288 CCU driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] clk: sunxi-ng: missing parent index table for tcon0_clk
--

commit 3f9c552550550b015864167770907fa314ac3239
Author: Pablo Mazzini <pmazzini@gmail.com>

clk: sunxi-ng: add Allwinner B288 CCU driver

This commit introduces the Allwinner B288 CCU driver, covering the PLLs,
the CPU/AHB/APB tree, module clocks, bus gates, and resets. The driver
is noted to be closely related to the A64 CCU.

> diff --git a/drivers/clk/sunxi-ng/ccu-sun8i-b288.c b/drivers/clk/sunxi-ng/ccu-sun8i-b288.c
> new file mode 100644
> index 0000000000000..c53afd6f9af98
> --- /dev/null
> +++ b/drivers/clk/sunxi-ng/ccu-sun8i-b288.c

[ ... ]

> +/* DE_CFG @ 0x104, EE_CFG @ 0x108, EDMA_CFG @ 0x10c -- M[3:0] mux[26:24] gate 31 */
> +static SUNXI_CCU_M_DATA_WITH_MUX_GATE(de_clk, "de", de_parents, 0x104,
> +				      0, 4, 24, 3, BIT(31), CLK_SET_RATE_PARENT);
> +static SUNXI_CCU_M_DATA_WITH_MUX_GATE(ee_clk, "ee", de_parents, 0x108,
> +				      0, 4, 24, 3, BIT(31), CLK_SET_RATE_PARENT);
> +static SUNXI_CCU_M_DATA_WITH_MUX_GATE(edma_clk, "edma", de_parents, 0x10c,
> +				      0, 4, 24, 3, BIT(31), CLK_SET_RATE_PARENT);
> +
> +/* TCON_CFG @ 0x118 -- mux[26:24] gate 31, no divider */
> +static SUNXI_CCU_MUX_DATA_WITH_GATE(tcon0_clk, "tcon0", tcon0_parents, 0x118,
> +				    24, 3,		/* mux */
> +				    BIT(31),		/* gate */
> +				    CLK_SET_RATE_PARENT);

[Severity: Medium]
Does the B288 hardware actually use a 1:1 index mapping (0 and 1) for this
TCON mux?

The closely related A64 SoC uses a {0, 2} index mapping for its TCON PLLs.
Because SUNXI_CCU_MUX_DATA_WITH_GATE defaults to a 1:1 mapping, if a display
driver requests to use pll_video1 for the TCON, the CCU driver will write
index 1 to the mux register. If the hardware maps the second parent to index
2 instead of 1, this will result in incorrect clock parent selection and could
break video output.

If a custom mapping is needed, it might be slightly tricky to define since the
ccu_mux.h framework currently lacks a SUNXI_CCU_MUX_DATA_TABLE_WITH_GATE
macro to combine a table with clk_parent_data.

> +
> +/* MBUS_CFG @ 0x15c -- M[2:0] mux[25:24] gate 31 */
> +static SUNXI_CCU_M_DATA_WITH_MUX_GATE(mbus_clk, "mbus", mbus_parents, 0x15c,
> +				      0, 3, 24, 2, BIT(31), CLK_IS_CRITICAL);

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