linux-kernel - Re: [PATCH 13/31] clk: mediatek: pll: Implement unregister API

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <CAGXv+5GKmOrT=yOkadsKVz38vvAj1-1sBN8i86zMLv3yusZeQw@mail.gmail.com>
Date:   Wed, 26 Jan 2022 14:18:06 +0800
From:   Chen-Yu Tsai <wenst@...omium.org>
To:     Miles Chen <miles.chen@...iatek.com>
Cc:     chun-jie.chen@...iatek.com, linux-arm-kernel@...ts.infradead.org,
        linux-clk@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-mediatek@...ts.infradead.org, matthias.bgg@...il.com,
        mturquette@...libre.com, sboyd@...nel.org
Subject: Re: [PATCH 13/31] clk: mediatek: pll: Implement unregister API

On Wed, Jan 26, 2022 at 2:04 PM Miles Chen <miles.chen@...iatek.com> wrote:
>
> > +static void mtk_clk_unregister_pll(struct clk *clk)
> > +{
> > +     struct clk_hw *hw = __clk_get_hw(clk);
> > +     struct mtk_clk_pll *pll = to_mtk_clk_pll(hw);
> > +
> > +     clk_unregister(clk);
> > +     kfree(pll);
> > +}
> > +
>
> mtk_clk_unregister_pll() looks different.
> Do we need to check hw before passing it to to_mtk_clk_pll(hw), like
> mtk_clk_unregister_mux()?

Good catch.

In theory we should, since __get_clk_hw() would return NULL if clk is NULL.
However the code already does that check before it calls
mtk_clk_unregister_pll(), so the code is safe. We should make everything
consistent though.

I might have written the code on separate days and thus introduced some
discrepancies. I'll fix it in v2.


ChenYu

> >  void mtk_clk_register_plls(struct device_node *node,
> >               const struct mtk_pll_data *plls, int num_plls, struct clk_onecell_data *clk_data)
> >  {
> > @@ -388,4 +397,44 @@ void mtk_clk_register_plls(struct device_node *node,
> >  }
> >  EXPORT_SYMBOL_GPL(mtk_clk_register_plls);
> >
> > +static __iomem void *mtk_clk_pll_get_base(struct clk *clk,
> > +                                       const struct mtk_pll_data *data)
> > +{
> > +     struct clk_hw *hw = __clk_get_hw(clk);
> > +     struct mtk_clk_pll *pll = to_mtk_clk_pll(hw);
> > +
> > +     return pll->base_addr - data->reg;
> > +}
> > +
> > +void mtk_clk_unregister_plls(const struct mtk_pll_data *plls, int num_plls,
> > +                          struct clk_onecell_data *clk_data)
> > +{
> > +     __iomem void *base = NULL;
> > +     int i;
> > +
> > +     if (!clk_data)
> > +             return;
> > +
> > +     for (i = num_plls; i > 0; i--) {
> > +             const struct mtk_pll_data *pll = &plls[i - 1];
> > +
> > +             if (IS_ERR_OR_NULL(clk_data->clks[pll->id]))
> > +                     continue;
> > +
> > +             /*
> > +              * This is quite ugly but unfortunately the clks don't have
> > +              * any device tied to them, so there's no place to store the
> > +              * pointer to the I/O region base address. We have to fetch
> > +              * it from one of the registered clks.
> > +              */
> > +             base = mtk_clk_pll_get_base(clk_data->clks[pll->id], pll);
> > +
> > +             mtk_clk_unregister_pll(clk_data->clks[pll->id]);
> > +             clk_data->clks[pll->id] = ERR_PTR(-ENOENT);
> > +     }
> > +
> > +     iounmap(base);
> > +}
> > +EXPORT_SYMBOL_GPL(mtk_clk_unregister_plls);
> > +
> >  MODULE_LICENSE("GPL");
> > diff --git a/drivers/clk/mediatek/clk-pll.h b/drivers/clk/mediatek/clk-pll.h
> > index d01b0c38311d..a889b1e472e7 100644
> > --- a/drivers/clk/mediatek/clk-pll.h
> > +++ b/drivers/clk/mediatek/clk-pll.h
> > @@ -51,5 +51,7 @@ struct mtk_pll_data {
> >  void mtk_clk_register_plls(struct device_node *node,
> >                          const struct mtk_pll_data *plls, int num_plls,
> >                          struct clk_onecell_data *clk_data);
> > +void mtk_clk_unregister_plls(const struct mtk_pll_data *plls, int num_plls,
> > +                          struct clk_onecell_data *clk_data);
> >
> >  #endif /* __DRV_CLK_MTK_PLL_H */
> > --
> > 2.35.0.rc0.227.g00780c9af4-goog
>
>