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@xxxxxxxxxx>님이 작성:
>
> 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@xxxxxxxxx>
> > Signed-off-by: Ijae Kim <ae878000@xxxxxxxxx>
> > Signed-off-by: Myeonghun Pak <mhun512@xxxxxxxxx>
> >
> > ---
> >  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
>