Re: [PATCH v4 13/24] clk: mediatek: pllfh: Add configurable clock ops to mtk_pllfh_data

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Missing function exports make it impossible to implement custom clk_ops for PLLFH without breaking frequency hopping.
--

--- Patch [13]: [PATCH v4 13/24] clk: mediatek: pllfh: Add configurable clock ops to mtk_pllfh_data ---
commit 3ac21b55d82d370931f28f17cf6b5de73f938fdf
Author: Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com>
clk: mediatek: pllfh: Add configurable clock ops to mtk_pllfh_data

This commit introduces a new ops field in the mtk_pllfh_data structure.
It allows drivers to provide a custom clock operation set instead of
using the default mtk_pllfh_ops, which is necessary for SoCs like MT8189
that require set/clr enable support.

> diff --git a/drivers/clk/mediatek/clk-pllfh.c b/drivers/clk/mediatek/clk-pllfh.c
> index aa95cd9197b3c..4878ac6da7da0 100644
> --- a/drivers/clk/mediatek/clk-pllfh.c
> +++ b/drivers/clk/mediatek/clk-pllfh.c
> @@ -153,6 +153,8 @@ static struct clk_hw *
>  mtk_clk_register_pllfh(struct device *dev, const struct mtk_pll_data *pll_data,
>  		       struct mtk_pllfh_data *pllfh_data, void __iomem *base)
>  {
> +	const struct clk_ops *pllfh_ops = pllfh_data->data.ops ?
> +					  pllfh_data->data.ops : &mtk_pllfh_ops;

[Severity: Medium]
Will this allow drivers to safely implement a custom ops struct?

A correct pllfh ops structure must use mtk_fhctl_set_rate to maintain
frequency hopping and spread spectrum clocking functionality. However, it
seems mtk_fhctl_set_rate is static and unexported.

If a driver provides a custom ops struct (such as mtk_pll_setclr_ops)
without being able to reference mtk_fhctl_set_rate, wouldn't it silently
bypass the fhctl hardware when setting the rate, potentially causing
glitches and breaking spread spectrum clocking?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729-mt8189-clocks-system-base-v4-0-e356b813a64c@collabora.com?part=13