Re: [PATCH v2 7/8] clk: sunxi-ng: a733: Add bus clock gates

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Bus clock gates for multiple peripherals are incorrectly parented to the 24MHz external oscillator (hosc) instead of their actual AHB/APB buses.
- [Low] Typographical error in the string names for the IOMMU1 system clocks, using an underscore instead of a hyphen.
--

--- Patch [7]: [PATCH v2 7/8] clk: sunxi-ng: a733: Add bus clock gates ---
commit 92eebd6b3ec66a1f50b982f0effa98e2238c2a4d
Author: Junhui Liu <junhui.liu@pigmoral.tech>

clk: sunxi-ng: a733: Add bus clock gates

This commit adds the bus clock gates that control access to the devices'
register interface on the Allwinner A733 SoC, including UARTs, SPI, I2C,
and multimedia engines. It also marks ahb-store and mbus-store as critical
to ensure boot/storage devices remain accessible.

> diff --git a/drivers/clk/sunxi-ng/ccu-sun60i-a733.c b/drivers/clk/sunxi-ng/ccu-sun60i-a733.c
> index bf26e310f08a9..48d7c0395ae83 100644
> --- a/drivers/clk/sunxi-ng/ccu-sun60i-a733.c
> +++ b/drivers/clk/sunxi-ng/ccu-sun60i-a733.c
> @@ -448,6 +454,8 @@ static SUNXI_CCU_M_DATA_WITH_MUX_GATE(trace_clk, "trace", trace_parents, 0x540,
>  				 BIT(31),	/* gate */
>  				 0);
>  
> +static SUNXI_CCU_GATE_DATA(bus_its_pcie0_aclk_clk, "bus-its-pcie0-aclk", hosc, 0x574, BIT(1), 0);
> +

[Severity: High]
Are these bus clock gates correctly parented to hosc instead of their
respective AHB or APB buses?

If the Common Clock Framework is misinformed about the hardware topology, it
might dynamically disable the actual underlying bus clock if it believes it 
has no active children. Could this cause a synchronous external abort 
(bus hang) when drivers try to access the registers of these peripherals?

This same parenting to hosc also appears in several other bus gates added 
later in this patch, such as bus_msi_lite0_clk, bus_npu_clk, bus_ufs_clk, 
and the bus_i2spcm series of clocks.

[ ... ]

> +static SUNXI_CCU_GATE_HWS(mbus_iommu1_sys_clk, "mbus-iommu1-sys", mbus_hws, 0x5b4, BIT(0), 0);
> +static SUNXI_CCU_GATE_HWS(apb_iommu1_sys_clk, "apb_iommu1-sys", apb0_hws, 0x5b4, BIT(1), 0);
> +static SUNXI_CCU_GATE_HWS(ahb_iommu1_sys_clk, "ahb_iommu1-sys", ahb_hws, 0x5b4, BIT(2), 0);

[Severity: Low]
This isn't a bug, but there's a typo in the string names for the iommu1
system clocks.

They use an underscore instead of a hyphen ("apb_iommu1-sys" instead of
"apb-iommu1-sys"). Does this break the naming convention established by the
iommu0 clocks just above it?

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