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

["Junhui Liu" <junhui.liu@pigmoral.tech>]

On Sat Oct 10, 2026 at 11:54 PM CST, sashiko-bot wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] clk: sunxi-ng: ahb_store_clk is missing CLK_IS_CRITICAL flag
> --
>
> commit f3ca9c131f39f726826090f6af6752e7b39c004c
> Author: Junhui Liu <junhui.liu@pigmoral.tech>
>
> clk: sunxi-ng: a733: Add bus clock gates
>
> This commit adds bus clock gates that control access to the device register
> interfaces on the Allwinner A733 SoC. It also intends to mark the ahb-store,
> mbus-store, and ahb-cpus clocks as critical to prevent access breakage to
> boot and storage devices.
>
>> diff --git a/drivers/clk/sunxi-ng/ccu-sun60i-a733.c b/drivers/clk/sunxi-ng/ccu-sun60i-a733.c
>> --- a/drivers/clk/sunxi-ng/ccu-sun60i-a733.c
>> +++ b/drivers/clk/sunxi-ng/ccu-sun60i-a733.c
>
> [ ... ]
>
>> +static SUNXI_CCU_GATE_HWS(ahb_msi_lite0_clk, "ahb-msi-lite0", ahb_hws,
>> +			  0x5c0, BIT(16), 0);
>> +
>> +/* This gate controls access to the storage controller MMIO registers. */
>> +static SUNXI_CCU_GATE_HWS(ahb_store_clk, "ahb-store", ahb_hws,
>> +			  0x5c0, BIT(24), 0);
>
> [Severity: High]
> The commit message notes that the ahb-store clock should be marked as
> critical, but the definition of ahb_store_clk here uses 0 for its flags
> parameter instead of CLK_IS_CRITICAL.

Oops, I forgot to update the commit message accordingly. I will send a
new version to update it.

>
> During late boot, the Common Clock Framework automatically disables any
> clocks that have no active consumers holding a reference to them.
>
> Could this omission allow the framework to inadvertently gate the storage
> controller MMIO registers, leading to a system freeze or storage probe
> failures?

ahb_store_clk has already been set as the parent of the NAND, MMC, SPI,
SPIF and UFS bus gates. Therefore, when any storage-related peripheral is
in use, ahb_store_clk will be enabled through the clock dependency, so it
no longer needs the CLK_IS_CRITICAL flag.

>
>> +static const struct clk_hw *ahb_store_hws[] = { &ahb_store_clk.common.hw };
>> +
>> +/* Keep enabled for access to the CPUS domain registers. */
>> +static SUNXI_CCU_GATE_HWS(ahb_cpus_clk, "ahb-cpus", ahb_hws,
>> +			  0x5c0, BIT(28), CLK_IS_CRITICAL);

-- 
Best regards,
Junhui Liu