Re: [PATCH 1/2] clk: mediatek: mt8173: fix the FHCTL DDS mask width

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing platform_set_drvdata() in probe causes a NULL pointer dereference in the remove function.
--

commit 89002485f2d73ae2cb8fc035297e800427c96c19
Author: Ryan Brue <ryanbrue.dev@gmail.com>
Subject: clk: mediatek: mt8173: fix the FHCTL DDS mask width

This commit fixes the FHCTL DDS field mask width for the MT8173's hoppable
PLLs by changing the mask from 22-bit to 21-bit. This resolves a 1000 us
polling timeout issue during rate changes that resulted in a warning and
register dump on every hop.

> diff --git a/drivers/clk/mediatek/clk-mt8173-apmixedsys.c b/drivers/clk/mediatek/clk-mt8173-apmixedsys.c
> index d7d416172ab35..327240cd34fbe 100644
> --- a/drivers/clk/mediatek/clk-mt8173-apmixedsys.c
> +++ b/drivers/clk/mediatek/clk-mt8173-apmixedsys.c
> @@ -100,7 +100,7 @@ enum fh_pll_id {
>  			.fh_id = _fhid,					\
>  			.fh_ver = FHCTL_PLLFH_V1,			\
>  			.fhx_offset = _offset,				\
> -			.dds_mask = GENMASK(21, 0),			\
> +			.dds_mask = GENMASK(20, 0),			\
>  			.slope0_value = 0x6003c97,			\
>  			.slope1_value = 0x6003c97,			\
>  			.sfstrx_en = BIT(2),				\

[Severity: High]
This is a pre-existing issue, but while reviewing this file, I noticed a
problem in the probe and remove functions.

In clk_mt8173_apmixed_probe(), the driver successfully returns without saving 
the allocated driver data:

	r = of_clk_add_hw_provider(dev->of_node, of_clk_hw_onecell_get, clk_data);
	if (r)
		goto unregister_ref2usb;

	return 0;

Later, if the driver is unbound, clk_mt8173_apmixed_remove() attempts to
retrieve this data:

	struct clk_hw_onecell_data *clk_data = platform_get_drvdata(pdev);

	of_clk_del_provider(node);
	mtk_clk_unregister_ref2usb_tx(clk_data->hws[CLK_APMIXED_REF2USB_TX]);

Could this cause a NULL pointer dereference when clk_data->hws is accessed,
crashing the system if a privileged user unbinds the driver via sysfs? Should
we add a call to platform_set_drvdata(pdev, clk_data) before the probe
function returns to prevent this?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260912-mt8173-fix-dds-mask-width-v1-0-3d0c0292c85c@gmail.com?part=1