Re: [PATCH 14/15] clk: mediatek: Add MT8189 scp clock support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Incorrect status register offset (`sta_ofs`) in `scp_cg_regs` causes the clock framework to read from the SET register instead of the STA register.
--

commit 8d1a0c988f3fa1bfe741cdb400dead04bed09d17
Author: Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com>

clk: mediatek: Add MT8189 scp clock support

This commit adds support for the MT8189 scp clock controller. It provides
clock gate control for the System Control Processor.

> diff --git a/drivers/clk/mediatek/clk-mt8189-scp.c b/drivers/clk/mediatek/clk-mt8189-scp.c
> new file mode 100644
> index 0000000000000..75197cd98b521
> --- /dev/null
> +++ b/drivers/clk/mediatek/clk-mt8189-scp.c
[ ... ]
> +
> +static const struct mtk_gate_regs scp_cg_regs = {
> +	.set_ofs = 0x4,
> +	.clr_ofs = 0x8,
> +	.sta_ofs = 0x4,

[Severity: High]
Should sta_ofs be 0x0 here instead of 0x4?

Looking at the register offsets, sta_ofs shares the exact same offset as
set_ofs. When the clock framework evaluates the clock state during driver
probe and initialization, it will read from the SET register instead of the
STA register.

Since SET is typically write-only on MediaTek clock gates, could this return
garbage data? If so, it might cause the clock framework to incorrectly track
the state and fail to disable unused clocks left on by the bootloader.

> +};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260701-mt8189-clocks-system-base-v1-0-2b048feea50a@collabora.com?part=14