linux-kernel - Re: [PATCH] PM / devfreq: mtk-cci: refactor error handling of probe and remove

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <414e7dbd903920183d731844666667f8f1a3c6da.camel@mediatek.com>
Date:   Thu, 4 May 2023 03:00:16 +0000
From:   Jia-wei Chang (張佳偉) 
        <Jia-wei.Chang@...iatek.com>
To:     "matthias.bgg@...il.com" <matthias.bgg@...il.com>,
        Johnson Wang (王聖鑫) 
        <Johnson.Wang@...iatek.com>,
        "angelogioacchino.delregno@...labora.com" 
        <angelogioacchino.delregno@...labora.com>,
        "kyungmin.park@...sung.com" <kyungmin.park@...sung.com>,
        "myungjoo.ham@...sung.com" <myungjoo.ham@...sung.com>,
        "cw00.choi@...sung.com" <cw00.choi@...sung.com>
CC:     "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-mediatek@...ts.infradead.org" 
        <linux-mediatek@...ts.infradead.org>,
        "hsinyi@...gle.com" <hsinyi@...gle.com>,
        Project_Global_Chrome_Upstream_Group 
        <Project_Global_Chrome_Upstream_Group@...iatek.com>
Subject: Re: [PATCH] PM / devfreq: mtk-cci: refactor error handling of probe
 and remove

On Wed, 2023-05-03 at 13:40 +0200, AngeloGioacchino Del Regno wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> Il 03/05/23 11:27, jia-wei.chang ha scritto:
> > From: Jia-Wei Chang <jia-wei.chang@...iatek.com>
> > 
> > To refactor the regulator/clk handlers so it can follow the way of
> > "Free
> > the Last Thing Style".
> > 
> > Signed-off-by: Jia-Wei Chang <jia-wei.chang@...iatek.com>
> > Fixes: 86d231b1db1b ("PM / devfreq: mediatek: Introduce MediaTek
> > CCI devfreq driver")
> > ---
> >   drivers/devfreq/mtk-cci-devfreq.c | 47 ++++++++++++++++++------
> > -------
> >   1 file changed, 28 insertions(+), 19 deletions(-)
> > 
> > diff --git a/drivers/devfreq/mtk-cci-devfreq.c
> > b/drivers/devfreq/mtk-cci-devfreq.c
> > index e5458ada5197..d2f743774147 100644
> > --- a/drivers/devfreq/mtk-cci-devfreq.c
> > +++ b/drivers/devfreq/mtk-cci-devfreq.c
> > @@ -294,14 +294,14 @@ static int mtk_ccifreq_probe(struct
> > platform_device *pdev)
> >       if (IS_ERR(drv->sram_reg)) {
> >               ret = PTR_ERR(drv->sram_reg);
> >               if (ret == -EPROBE_DEFER)
> > -                     goto out_free_resources;
> > +                     goto out_disable_proc_reg;
> > 
> >               drv->sram_reg = NULL;
> >       } else {
> >               ret = regulator_enable(drv->sram_reg);
> >               if (ret) {
> >                       dev_err(dev, "failed to enable sram
> > regulator\n");
> > -                     goto out_free_resources;
> > +                     goto out_disable_proc_reg;
> >               }
> >       }
> > 
> > @@ -316,12 +316,16 @@ static int mtk_ccifreq_probe(struct
> > platform_device *pdev)
> > 
> >       ret = clk_prepare_enable(drv->cci_clk);
> >       if (ret)
> > -             goto out_free_resources;
> > +             goto out_disable_sram_reg;
> > +
> > +     ret = clk_prepare_enable(drv->inter_clk);
> 
> Adding a clk_prepare_enable() call for a clock must be done in a
> separate commit.
> Besides, there shouldn't be any need to do that, as when you call
> clk_set_parent()
> (done in mtk_ccifreq_target()) on a clock that has flag
> CLK_OPS_PARENT_ENABLE, the
> clk core will automatically call clk_core_prepare_enable() on the new
> parent.
> 
> If you're facing a bug for which the parent is not getting enabled,
> the solution
> is to add CLK_OPS_PARENT_ENABLE to the interested clock.
> 
> Regards,
> Angelo
> 

Hi Angelo Sir,

Thanks for your suggestion.
I will send a new patch to remove clk_prepare_enable() call on the
parent clock, since it is not directly related to this commit purpose.

As you suggested, clock parent enable will be handled by flag if
necessary.