[PATCH] clk: mediatek: mt6735: Unregister PLLs on probe failure

[PATCH] clk: mediatek: mt6735: Unregister PLLs on probe failure

[Myeonghun Pak]

mtk_clk_register_plls() registers the apmixedsys PLL clocks manually, while
clk_mt6735_apmixed_remove() unregisters them on driver removal.

If devm_of_clk_add_hw_provider() fails after the PLL registration succeeds,
probe returns the error directly and the remove callback is not run. This
leaves the registered PLL clocks behind on the probe failure path.

Add an unregister_plls error path so provider registration failures unwind the
PLLs before returning the error.

Fixes: 43c04ed79189 ("clk: mediatek: Add drivers for MediaTek MT6735 main clock and reset drivers")
Co-developed-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Myeonghun Pak <mhun512@gmail.com>

---
 drivers/clk/mediatek/clk-mt6735-apmixedsys.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/mediatek/clk-mt6735-apmixedsys.c b/drivers/clk/mediatek/clk-mt6735-apmixedsys.c
index 9e30c089a2..04cf9665ec 100644
--- a/drivers/clk/mediatek/clk-mt6735-apmixedsys.c
+++ b/drivers/clk/mediatek/clk-mt6735-apmixedsys.c
@@ -102,10 +102,17 @@ static int clk_mt6735_apmixed_probe(struct platform_device *pdev)
 
 	ret = devm_of_clk_add_hw_provider(&pdev->dev, of_clk_hw_onecell_get,
 					  clk_data);
-	if (ret)
+	if (ret) {
 		dev_err(&pdev->dev,
 			"Failed to register clock provider: %d\n", ret);
+		goto unregister_plls;
+	}
+
+	return 0;
 
+unregister_plls:
+	mtk_clk_unregister_plls(apmixedsys_plls, ARRAY_SIZE(apmixedsys_plls),
+				clk_data);
 	return ret;
 }
 
-- 
2.47.1

Re: [PATCH] clk: mediatek: mt6735: Unregister PLLs on probe failure

[Brian Masney]

Hi Myeonghun,

On Tue, Jun 23, 2026 at 06:41:11PM +0900, Myeonghun Pak wrote:
> mtk_clk_register_plls() registers the apmixedsys PLL clocks manually, while
> clk_mt6735_apmixed_remove() unregisters them on driver removal.
> 
> If devm_of_clk_add_hw_provider() fails after the PLL registration succeeds,
> probe returns the error directly and the remove callback is not run. This
> leaves the registered PLL clocks behind on the probe failure path.
> 
> Add an unregister_plls error path so provider registration failures unwind the
> PLLs before returning the error.

Please run your patches through checkpatch.pl before sending.

> 
> Fixes: 43c04ed79189 ("clk: mediatek: Add drivers for MediaTek MT6735 main clock and reset drivers")
> Co-developed-by: Ijae Kim <ae878000@gmail.com>
> Signed-off-by: Ijae Kim <ae878000@gmail.com>
> Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
> 
> ---
>  drivers/clk/mediatek/clk-mt6735-apmixedsys.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/mediatek/clk-mt6735-apmixedsys.c b/drivers/clk/mediatek/clk-mt6735-apmixedsys.c
> index 9e30c089a2..04cf9665ec 100644
> --- a/drivers/clk/mediatek/clk-mt6735-apmixedsys.c
> +++ b/drivers/clk/mediatek/clk-mt6735-apmixedsys.c
> @@ -102,10 +102,17 @@ static int clk_mt6735_apmixed_probe(struct platform_device *pdev)
>  
>  	ret = devm_of_clk_add_hw_provider(&pdev->dev, of_clk_hw_onecell_get,
>  					  clk_data);
> -	if (ret)
> +	if (ret) {
>  		dev_err(&pdev->dev,
>  			"Failed to register clock provider: %d\n", ret);
> +		goto unregister_plls;
> +	}
> +
> +	return 0;
>  
> +unregister_plls:
> +	mtk_clk_unregister_plls(apmixedsys_plls, ARRAY_SIZE(apmixedsys_plls),
> +				clk_data);
>  	return ret;
>  }

This fix is correct. Since only one path will encounter this, personally
I would just put the call to mtk_clk_unregister_plls() inside the if
statement above to simplify this further.

Brian

Re: [PATCH] clk: mediatek: mt6735: Unregister PLLs on probe failure

[Myeonghun Pak]

Hi,

Thanks for the review.

I will send a v2 with mtk_clk_unregister_plls() moved directly into the
devm_of_clk_add_hw_provider() failure branch, and I will make sure it passes
checkpatch before sending.

Best regards,
Myeonghun

2026년 6월 23일 (화) 오후 11:35, Brian Masney <bmasney@redhat.com>님이 작성:
>
> Hi Myeonghun,
>
> On Tue, Jun 23, 2026 at 06:41:11PM +0900, Myeonghun Pak wrote:
> > mtk_clk_register_plls() registers the apmixedsys PLL clocks manually, while
> > clk_mt6735_apmixed_remove() unregisters them on driver removal.
> >
> > If devm_of_clk_add_hw_provider() fails after the PLL registration succeeds,
> > probe returns the error directly and the remove callback is not run. This
> > leaves the registered PLL clocks behind on the probe failure path.
> >
> > Add an unregister_plls error path so provider registration failures unwind the
> > PLLs before returning the error.
>
> Please run your patches through checkpatch.pl before sending.
>
> >
> > Fixes: 43c04ed79189 ("clk: mediatek: Add drivers for MediaTek MT6735 main clock and reset drivers")
> > Co-developed-by: Ijae Kim <ae878000@gmail.com>
> > Signed-off-by: Ijae Kim <ae878000@gmail.com>
> > Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
> >
> > ---
> >  drivers/clk/mediatek/clk-mt6735-apmixedsys.c | 9 ++++++++-
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/clk/mediatek/clk-mt6735-apmixedsys.c b/drivers/clk/mediatek/clk-mt6735-apmixedsys.c
> > index 9e30c089a2..04cf9665ec 100644
> > --- a/drivers/clk/mediatek/clk-mt6735-apmixedsys.c
> > +++ b/drivers/clk/mediatek/clk-mt6735-apmixedsys.c
> > @@ -102,10 +102,17 @@ static int clk_mt6735_apmixed_probe(struct platform_device *pdev)
> >
> >       ret = devm_of_clk_add_hw_provider(&pdev->dev, of_clk_hw_onecell_get,
> >                                         clk_data);
> > -     if (ret)
> > +     if (ret) {
> >               dev_err(&pdev->dev,
> >                       "Failed to register clock provider: %d\n", ret);
> > +             goto unregister_plls;
> > +     }
> > +
> > +     return 0;
> >
> > +unregister_plls:
> > +     mtk_clk_unregister_plls(apmixedsys_plls, ARRAY_SIZE(apmixedsys_plls),
> > +                             clk_data);
> >       return ret;
> >  }
>
> This fix is correct. Since only one path will encounter this, personally
> I would just put the call to mtk_clk_unregister_plls() inside the if
> statement above to simplify this further.
>
> Brian
>