Re: [PATCH 06/15] clk: mediatek: Add MT8189 apmixedsys clock support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] I/O memory mapped for the `fhctl` node via `fhctl_parse_dt()` is permanently leaked if `mtk_clk_register_pllfhs()` fails during probe.
--

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

clk: mediatek: Add MT8189 apmixedsys clock support
    
Add support for the MT8189 apmixedsys clock controller, which provides
PLLs generated from SoC 26m.

> diff --git a/drivers/clk/mediatek/clk-mt8189-apmixedsys.c b/drivers/clk/mediatek/clk-mt8189-apmixedsys.c
> new file mode 100644
> index 0000000000000..0657a50c30d9f
> --- /dev/null
> +++ b/drivers/clk/mediatek/clk-mt8189-apmixedsys.c
[ ... ]
> @@ -140,25 +140,25 @@ static int clk_mt8189_apmixed_probe(struct platform_device *pdev)
> +	int r;
> +	struct clk_hw_onecell_data *clk_data;
> +	struct device_node *node = pdev->dev.of_node;
> +	const u8 *fhctl_node = "mediatek,mt8189-fhctl";
> +
> +	clk_data = mtk_alloc_clk_data(ARRAY_SIZE(apmixed_plls));
> +	if (!clk_data)
> +		return -ENOMEM;
> +
> +	fhctl_parse_dt(fhctl_node, pllfhs, ARRAY_SIZE(pllfhs));
> +
> +	r = mtk_clk_register_pllfhs(&pdev->dev, apmixed_plls, ARRAY_SIZE(apmixed_plls),
> +				    pllfhs, ARRAY_SIZE(pllfhs), clk_data);
> +	if (r)
> +		goto free_apmixed_data;

[Severity: Medium]
Does this code leak the I/O memory mapped for the fhctl node?

Looking at fhctl_parse_dt(), it calls of_iomap() and stores the mapped memory
base in the pllfhs structs.

If mtk_clk_register_pllfhs() fails, it returns an error and cleans up after
itself, but its internal error path does not unmap the fhctl base mapped by
fhctl_parse_dt().

Here, jumping to the free_apmixed_data label bypasses the call to
mtk_clk_unregister_pllfhs() (which would normally unmap the fhctl base).
Should the fhctl base be unmapped on this error path to prevent a memory
leak if the probe defers or fails?

> +
> +	r = of_clk_add_hw_provider(node, of_clk_hw_onecell_get, clk_data);
> +	if (r)
> +		goto unregister_plls;
> +
> +	platform_set_drvdata(pdev, clk_data);
> +
> +	return 0;
> +
> +unregister_plls:
> +	mtk_clk_unregister_pllfhs(apmixed_plls, ARRAY_SIZE(apmixed_plls), pllfhs,
> +				  ARRAY_SIZE(pllfhs), clk_data);
> +free_apmixed_data:
> +	mtk_free_clk_data(clk_data);
> +	return r;
> +}

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