Messages in this thread | | | From | Chen-Yu Tsai <> | Date | Wed, 26 Jan 2022 14:18:06 +0800 | Subject | Re: [PATCH 13/31] clk: mediatek: pll: Implement unregister API |
| |
On Wed, Jan 26, 2022 at 2:04 PM Miles Chen <miles.chen@mediatek.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 > >
| |