Re: [PATCH v2 13/35] clk: mediatek: Add PLL support to common probe/remove helpers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] clk: mtk: redundant iomap for plls causes memory leak
--

commit 5289a3c612ab2d43573cbb1c18f5582e2661818a
Author: Akari Tsuyukusa <akkun11.open@gmail.com>

clk: mediatek: Add PLL support to common probe/remove helpers

This commit extends __mtk_clk_simple_probe() and __mtk_clk_simple_remove()
to support PLLs. This reduces boilerplate code in apmixedsys drivers.

> diff --git a/drivers/clk/mediatek/clk-mtk.c b/drivers/clk/mediatek/clk-mtk.c
> index 004098a839b2c..e06cd40d995a9 100644
> --- a/drivers/clk/mediatek/clk-mtk.c
> +++ b/drivers/clk/mediatek/clk-mtk.c

[ ... ]

> @@ -486,7 +487,7 @@ static int __mtk_clk_simple_probe(struct platform_device *pdev,
>  	}
>  
>  	/* Composite and divider clocks needs us to pass iomem pointer */
> -	if (mcd->composite_clks || mcd->divider_clks) {
> +	if (mcd->composite_clks || mcd->divider_clks || mcd->plls) {
>  		if (!mcd->shared_io)
>  			base = devm_platform_ioremap_resource(pdev, 0);
>  		else
> @@ -514,7 +515,7 @@ static int __mtk_clk_simple_probe(struct platform_device *pdev,
>  	num_clks = mcd->num_clks + mcd->num_composite_clks;
>  	num_clks += mcd->num_fixed_clks + mcd->num_factor_clks;
>  	num_clks += mcd->num_mux_clks + mcd->num_divider_clks;
> -	num_clks += mcd->num_cpumuxes;
> +	num_clks += mcd->num_cpumuxes + mcd->num_plls;
>  
>  	clk_data = mtk_alloc_clk_data(num_clks);
>  	if (!clk_data) {
> @@ -522,11 +523,18 @@ static int __mtk_clk_simple_probe(struct platform_device *pdev,
>  		goto free_base;
>  	}
>  
> +	if (mcd->plls) {
> +		r = mtk_clk_register_plls(&pdev->dev, mcd->plls,
> +					  mcd->num_plls, clk_data);

[Severity: Medium]
Does adding mcd->plls to the earlier base mapping condition cause a resource
leak? 

Since mtk_clk_register_plls() maps its own I/O memory using of_iomap()
internally and isn't passed the base pointer mapped above, the newly
mapped memory remains unused. 

If mcd->shared_io is true, of_iomap() maps memory into base but it is never
unmapped on the success path, leading to a virtual I/O memory leak each
time a device successfully probes. If mcd->shared_io is false, it
redundantly calls devm_platform_ioremap_resource() to create an unused
duplicate mapping.

Could mcd->plls be removed from the earlier mapping condition to prevent
this?

> +		if (r)
> +			goto free_data;
> +	}
> +

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013151734.3177690-1-akkun11.open@gmail.com?part=13