| Message ID | 20240225065234.413687-2-mmyangfl@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | clk: hisilicon: Migrate devm APIs | expand |
On 2/25/2024 2:52 PM, David Yang wrote: > Helper functions extract common operations on platform drivers. > > During migration to devm APIs, (virtual) fixed clocks were found hard on > devm APIs, since they often depended by crucial peripherals, thus require > early initialization before device probing, and cannot use devm APIs. We have core_initcall() in drivers so CRGs are probed very early. This shouldn't be a problem. > > One solution to this problem is to add a "fixed-clock" node to device tree, > independent to clock device, and make those peripherals depend on that. > However, there is also some devices that do use fixed clocks provided by > drivers, for example clk-hi3660.c . > > To simplify codes, we migrate clocks of other types to devm APIs, while > keep fixed clocks self-managed, alongside with struct hisi_clock_data, and > remove devm-managed hisi_clock_data. Do we really want? How about leave old SoCs alone and just introduce a new set of APIs for new SoCs? Just like CCF, devm_ functions are simply wrappers of old APIs with the help of devres, the old APIs are still available. So for HiSilicon, I think we can take a similar approach, i.e., add a new set of wrapper functions with the help of devres rather than modifying old code. The implementation of officially provided devm_ APIs can be a good example. > > `hisi_clk_alloc` will be removed in the following patch. > > Signed-off-by: David Yang <mmyangfl@gmail.com> > --- > drivers/clk/hisilicon/clk.c | 157 ++++++++++++++++++++++++++++++++++ > drivers/clk/hisilicon/clk.h | 46 +++++++++- > drivers/clk/hisilicon/crg.h | 5 ++ > drivers/clk/hisilicon/reset.c | 42 +++++++++ > 4 files changed, 248 insertions(+), 2 deletions(-) > > diff --git a/drivers/clk/hisilicon/clk.c b/drivers/clk/hisilicon/clk.c > index 09368fd32bef..e50115f8e236 100644 > --- a/drivers/clk/hisilicon/clk.c > +++ b/drivers/clk/hisilicon/clk.c > @@ -88,6 +88,25 @@ struct hisi_clock_data *hisi_clk_init(struct device_node *np, > } > EXPORT_SYMBOL_GPL(hisi_clk_init); > > +void hisi_clk_free(struct device_node *np, struct hisi_clock_data *data) > +{ > + if (data->clks) { > + if (data->clks->fixed_rate_clks_num) > + hisi_clk_unregister_fixed_rate(data->clks->fixed_rate_clks, > + data->clks->fixed_rate_clks_num, > + data); > + if (data->clks->fixed_factor_clks_num) > + hisi_clk_unregister_fixed_factor(data->clks->fixed_factor_clks, > + data->clks->fixed_factor_clks_num, > + data); > + } > + > + of_clk_del_provider(np); > + kfree(data->clk_data.clks); > + kfree(data); > +} > +EXPORT_SYMBOL_GPL(hisi_clk_free); > + > int hisi_clk_register_fixed_rate(const struct hisi_fixed_rate_clock *clks, > int nums, struct hisi_clock_data *data) > { > @@ -341,3 +360,141 @@ void __init hi6220_clk_register_divider(const struct hi6220_divider_clock *clks, > data->clk_data.clks[clks[i].id] = clk; > } > } > + > +static size_t hisi_clocks_get_nr(const struct hisi_clocks *clks) > +{ > + if (clks->nr) > + return clks->nr; > + > + return clks->fixed_rate_clks_num + clks->fixed_factor_clks_num + > + clks->mux_clks_num + clks->phase_clks_num + > + clks->divider_clks_num + clks->gate_clks_num + > + clks->gate_sep_clks_num + clks->customized_clks_num; > +} > + > +int hisi_clk_early_init(struct device_node *np, const struct hisi_clocks *clks) > +{ > + struct hisi_clock_data *data; > + int ret; > + > + data = hisi_clk_init(np, hisi_clocks_get_nr(clks)); > + if (!data) > + return -ENOMEM; > + data->clks = clks; > + > + ret = hisi_clk_register_fixed_rate(clks->fixed_rate_clks, > + clks->fixed_rate_clks_num, data); > + if (ret) > + goto err; > + > + ret = hisi_clk_register_fixed_factor(clks->fixed_factor_clks, > + clks->fixed_factor_clks_num, data); > + if (ret) > + goto err; > + > + np->data = data; > + return 0; > + > +err: > + hisi_clk_free(np, data); > + return ret; > +} > +EXPORT_SYMBOL_GPL(hisi_clk_early_init); > + > +static int hisi_clk_register(struct device *dev, const struct hisi_clocks *clks, > + struct hisi_clock_data *data) > +{ > + int ret; > + > + if (clks->mux_clks_num) { > + ret = hisi_clk_register_mux(clks->mux_clks, > + clks->mux_clks_num, data); > + if (ret) > + return ret; > + } > + > + if (clks->phase_clks_num) { > + ret = hisi_clk_register_phase(dev, clks->phase_clks, > + clks->phase_clks_num, data); > + if (ret) > + return ret; > + } > + > + if (clks->divider_clks_num) { > + ret = hisi_clk_register_divider(clks->divider_clks, > + clks->divider_clks_num, data); > + if (ret) > + return ret; > + } > + > + if (clks->gate_clks_num) { > + ret = hisi_clk_register_gate(clks->gate_clks, > + clks->gate_clks_num, data); > + if (ret) > + return ret; > + } > + > + if (clks->gate_sep_clks_num) { > + hisi_clk_register_gate_sep(clks->gate_sep_clks, > + clks->gate_sep_clks_num, data); > + } > + > + if (clks->clk_register_customized && clks->customized_clks_num) { > + ret = clks->clk_register_customized(dev, clks->customized_clks, > + clks->customized_clks_num, data); > + if (ret) > + return ret; > + } > + > + return 0; > +} > + > +int hisi_clk_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct device_node *np = dev->of_node; > + const struct hisi_clocks *clks; > + struct hisi_clock_data *data; > + int ret; > + > + clks = of_device_get_match_data(dev); > + if (!clks) > + return -ENOENT; > + > + if (!np->data) { > + ret = hisi_clk_early_init(np, clks); > + if (ret) > + return ret; > + } > + > + data = np->data; > + np->data = NULL; > + > + if (clks->prologue) { > + ret = clks->prologue(dev, data); > + if (ret) > + goto err; > + } > + > + ret = hisi_clk_register(dev, clks, data); > + if (ret) > + goto err; > + > + platform_set_drvdata(pdev, data); > + return 0; > + > +err: > + hisi_clk_free(np, data); > + return ret; > +} > +EXPORT_SYMBOL_GPL(hisi_clk_probe); > + > +void hisi_clk_remove(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct device_node *np = dev->of_node; > + struct hisi_clock_data *data = platform_get_drvdata(pdev); > + > + hisi_clk_free(np, data); > +} > +EXPORT_SYMBOL_GPL(hisi_clk_remove); > diff --git a/drivers/clk/hisilicon/clk.h b/drivers/clk/hisilicon/clk.h > index 7a9b42e1b027..87b17e9b79a3 100644 > --- a/drivers/clk/hisilicon/clk.h > +++ b/drivers/clk/hisilicon/clk.h > @@ -17,10 +17,12 @@ > #include <linux/spinlock.h> > > struct platform_device; > +struct hisi_clocks; > > struct hisi_clock_data { > - struct clk_onecell_data clk_data; > - void __iomem *base; > + struct clk_onecell_data clk_data; > + void __iomem *base; > + const struct hisi_clocks *clks; > }; > > struct hisi_fixed_rate_clock { > @@ -103,6 +105,39 @@ struct hisi_gate_clock { > const char *alias; > }; > > +struct hisi_clocks { > + /* if 0, sum all *_num */ > + size_t nr; > + > + int (*prologue)(struct device *dev, struct hisi_clock_data *data); > + > + const struct hisi_fixed_rate_clock *fixed_rate_clks; > + size_t fixed_rate_clks_num; > + > + const struct hisi_fixed_factor_clock *fixed_factor_clks; > + size_t fixed_factor_clks_num; > + > + const struct hisi_mux_clock *mux_clks; > + size_t mux_clks_num; > + > + const struct hisi_phase_clock *phase_clks; > + size_t phase_clks_num; > + > + const struct hisi_divider_clock *divider_clks; > + size_t divider_clks_num; > + > + const struct hisi_gate_clock *gate_clks; > + size_t gate_clks_num; > + > + const struct hisi_gate_clock *gate_sep_clks; > + size_t gate_sep_clks_num; > + > + const void *customized_clks; > + size_t customized_clks_num; > + int (*clk_register_customized)(struct device *dev, const void *clks, > + size_t num, struct hisi_clock_data *data); > +}; > + > struct clk *hisi_register_clkgate_sep(struct device *, const char *, > const char *, unsigned long, > void __iomem *, u8, > @@ -113,6 +148,7 @@ struct clk *hi6220_register_clkdiv(struct device *dev, const char *name, > > struct hisi_clock_data *hisi_clk_alloc(struct platform_device *, int); > struct hisi_clock_data *hisi_clk_init(struct device_node *, int); > +void hisi_clk_free(struct device_node *np, struct hisi_clock_data *data); > int hisi_clk_register_fixed_rate(const struct hisi_fixed_rate_clock *, > int, struct hisi_clock_data *); > int hisi_clk_register_fixed_factor(const struct hisi_fixed_factor_clock *, > @@ -154,4 +190,10 @@ hisi_clk_unregister(mux) > hisi_clk_unregister(divider) > hisi_clk_unregister(gate) > > +/* helper functions for platform driver */ > + > +int hisi_clk_early_init(struct device_node *np, const struct hisi_clocks *clks); > +int hisi_clk_probe(struct platform_device *pdev); > +void hisi_clk_remove(struct platform_device *pdev); > + > #endif /* __HISI_CLK_H */ > diff --git a/drivers/clk/hisilicon/crg.h b/drivers/clk/hisilicon/crg.h > index 803f6ba6d7a2..bd8e76b1f6d7 100644 > --- a/drivers/clk/hisilicon/crg.h > +++ b/drivers/clk/hisilicon/crg.h > @@ -22,4 +22,9 @@ struct hisi_crg_dev { > const struct hisi_crg_funcs *funcs; > }; > > +/* helper functions for platform driver */ > + > +int hisi_crg_probe(struct platform_device *pdev); > +void hisi_crg_remove(struct platform_device *pdev); > + > #endif /* __HISI_CRG_H */ > diff --git a/drivers/clk/hisilicon/reset.c b/drivers/clk/hisilicon/reset.c > index 93cee17db8b1..c7d4c9ea7183 100644 > --- a/drivers/clk/hisilicon/reset.c > +++ b/drivers/clk/hisilicon/reset.c > @@ -6,11 +6,15 @@ > */ > > #include <linux/io.h> > +#include <linux/kernel.h> > #include <linux/of_address.h> > #include <linux/platform_device.h> > #include <linux/reset-controller.h> > #include <linux/slab.h> > #include <linux/spinlock.h> > + > +#include "clk.h" > +#include "crg.h" > #include "reset.h" > > #define HISI_RESET_BIT_MASK 0x1f > @@ -116,3 +120,41 @@ void hisi_reset_exit(struct hisi_reset_controller *rstc) > reset_controller_unregister(&rstc->rcdev); > } > EXPORT_SYMBOL_GPL(hisi_reset_exit); > + > +int hisi_crg_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct hisi_crg_dev *crg; > + int ret; > + > + crg = devm_kmalloc(dev, sizeof(*crg), GFP_KERNEL); > + if (!crg) > + return -ENOMEM; > + > + ret = hisi_clk_probe(pdev); > + if (ret) > + return ret; > + > + crg->rstc = hisi_reset_init(pdev); > + if (!crg->rstc) { > + ret = -ENOMEM; > + goto err; > + } > + > + platform_set_drvdata(pdev, crg); > + return 0; > + > +err: > + hisi_clk_remove(pdev); > + return ret; > +} > +EXPORT_SYMBOL_GPL(hisi_crg_probe); > + > +void hisi_crg_remove(struct platform_device *pdev) > +{ > + struct hisi_crg_dev *crg = platform_get_drvdata(pdev); > + > + hisi_reset_exit(crg->rstc); > + hisi_clk_remove(pdev); > +} > +EXPORT_SYMBOL_GPL(hisi_crg_remove);
Yang Xiwen <forbidden405@outlook.com> 于2024年2月25日周日 19:56写道： > > On 2/25/2024 2:52 PM, David Yang wrote: > > Helper functions extract common operations on platform drivers. > > > > During migration to devm APIs, (virtual) fixed clocks were found hard on > > devm APIs, since they often depended by crucial peripherals, thus require > > early initialization before device probing, and cannot use devm APIs. > > > We have core_initcall() in drivers so CRGs are probed very early. This > shouldn't be a problem. > This is also handled within hisi_clk_early_init(), and tested against real devices. > > > > > One solution to this problem is to add a "fixed-clock" node to device tree, > > independent to clock device, and make those peripherals depend on that. > > However, there is also some devices that do use fixed clocks provided by > > drivers, for example clk-hi3660.c . > > > > To simplify codes, we migrate clocks of other types to devm APIs, while > > keep fixed clocks self-managed, alongside with struct hisi_clock_data, and > > remove devm-managed hisi_clock_data. > > > Do we really want? How about leave old SoCs alone and just introduce a > new set of APIs for new SoCs? > > > Just like CCF, devm_ functions are simply wrappers of old APIs with the > help of devres, the old APIs are still available. > > > So for HiSilicon, I think we can take a similar approach, i.e., add a > new set of wrapper functions with the help of devres rather than > modifying old code. > > > The implementation of officially provided devm_ APIs can be a good example. > This is somewhat requested by the maintainer, see replies in v5 of the previous series https://lore.kernel.org/all/d3b057408117a71bcd153f4a91bcdfe1.sboyd@kernel.org/ While this series migrates into devm_, it mainly serves as a cleanup. Existing drivers share most of registration flows, while lack of proper maintenance over times. This series improves overall code quality, and reduce redundancy code by introducing newly-designed common functions. We might as well keep old API, but this series has no intention on how new device will be introduced. > > > > > `hisi_clk_alloc` will be removed in the following patch. > > > > Signed-off-by: David Yang <mmyangfl@gmail.com> > > --- > > drivers/clk/hisilicon/clk.c | 157 ++++++++++++++++++++++++++++++++++ > > drivers/clk/hisilicon/clk.h | 46 +++++++++- > > drivers/clk/hisilicon/crg.h | 5 ++ > > drivers/clk/hisilicon/reset.c | 42 +++++++++ > > 4 files changed, 248 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/clk/hisilicon/clk.c b/drivers/clk/hisilicon/clk.c > > index 09368fd32bef..e50115f8e236 100644 > > --- a/drivers/clk/hisilicon/clk.c > > +++ b/drivers/clk/hisilicon/clk.c > > @@ -88,6 +88,25 @@ struct hisi_clock_data *hisi_clk_init(struct device_node *np, > > } > > EXPORT_SYMBOL_GPL(hisi_clk_init); > > > > +void hisi_clk_free(struct device_node *np, struct hisi_clock_data *data) > > +{ > > + if (data->clks) { > > + if (data->clks->fixed_rate_clks_num) > > + hisi_clk_unregister_fixed_rate(data->clks->fixed_rate_clks, > > + data->clks->fixed_rate_clks_num, > > + data); > > + if (data->clks->fixed_factor_clks_num) > > + hisi_clk_unregister_fixed_factor(data->clks->fixed_factor_clks, > > + data->clks->fixed_factor_clks_num, > > + data); > > + } > > + > > + of_clk_del_provider(np); > > + kfree(data->clk_data.clks); > > + kfree(data); > > +} > > +EXPORT_SYMBOL_GPL(hisi_clk_free); > > + > > int hisi_clk_register_fixed_rate(const struct hisi_fixed_rate_clock *clks, > > int nums, struct hisi_clock_data *data) > > { > > @@ -341,3 +360,141 @@ void __init hi6220_clk_register_divider(const struct hi6220_divider_clock *clks, > > data->clk_data.clks[clks[i].id] = clk; > > } > > } > > + > > +static size_t hisi_clocks_get_nr(const struct hisi_clocks *clks) > > +{ > > + if (clks->nr) > > + return clks->nr; > > + > > + return clks->fixed_rate_clks_num + clks->fixed_factor_clks_num + > > + clks->mux_clks_num + clks->phase_clks_num + > > + clks->divider_clks_num + clks->gate_clks_num + > > + clks->gate_sep_clks_num + clks->customized_clks_num; > > +} > > + > > +int hisi_clk_early_init(struct device_node *np, const struct hisi_clocks *clks) > > +{ > > + struct hisi_clock_data *data; > > + int ret; > > + > > + data = hisi_clk_init(np, hisi_clocks_get_nr(clks)); > > + if (!data) > > + return -ENOMEM; > > + data->clks = clks; > > + > > + ret = hisi_clk_register_fixed_rate(clks->fixed_rate_clks, > > + clks->fixed_rate_clks_num, data); > > + if (ret) > > + goto err; > > + > > + ret = hisi_clk_register_fixed_factor(clks->fixed_factor_clks, > > + clks->fixed_factor_clks_num, data); > > + if (ret) > > + goto err; > > + > > + np->data = data; > > + return 0; > > + > > +err: > > + hisi_clk_free(np, data); > > + return ret; > > +} > > +EXPORT_SYMBOL_GPL(hisi_clk_early_init); > > + > > +static int hisi_clk_register(struct device *dev, const struct hisi_clocks *clks, > > + struct hisi_clock_data *data) > > +{ > > + int ret; > > + > > + if (clks->mux_clks_num) { > > + ret = hisi_clk_register_mux(clks->mux_clks, > > + clks->mux_clks_num, data); > > + if (ret) > > + return ret; > > + } > > + > > + if (clks->phase_clks_num) { > > + ret = hisi_clk_register_phase(dev, clks->phase_clks, > > + clks->phase_clks_num, data); > > + if (ret) > > + return ret; > > + } > > + > > + if (clks->divider_clks_num) { > > + ret = hisi_clk_register_divider(clks->divider_clks, > > + clks->divider_clks_num, data); > > + if (ret) > > + return ret; > > + } > > + > > + if (clks->gate_clks_num) { > > + ret = hisi_clk_register_gate(clks->gate_clks, > > + clks->gate_clks_num, data); > > + if (ret) > > + return ret; > > + } > > + > > + if (clks->gate_sep_clks_num) { > > + hisi_clk_register_gate_sep(clks->gate_sep_clks, > > + clks->gate_sep_clks_num, data); > > + } > > + > > + if (clks->clk_register_customized && clks->customized_clks_num) { > > + ret = clks->clk_register_customized(dev, clks->customized_clks, > > + clks->customized_clks_num, data); > > + if (ret) > > + return ret; > > + } > > + > > + return 0; > > +} > > + > > +int hisi_clk_probe(struct platform_device *pdev) > > +{ > > + struct device *dev = &pdev->dev; > > + struct device_node *np = dev->of_node; > > + const struct hisi_clocks *clks; > > + struct hisi_clock_data *data; > > + int ret; > > + > > + clks = of_device_get_match_data(dev); > > + if (!clks) > > + return -ENOENT; > > + > > + if (!np->data) { > > + ret = hisi_clk_early_init(np, clks); > > + if (ret) > > + return ret; > > + } > > + > > + data = np->data; > > + np->data = NULL; > > + > > + if (clks->prologue) { > > + ret = clks->prologue(dev, data); > > + if (ret) > > + goto err; > > + } > > + > > + ret = hisi_clk_register(dev, clks, data); > > + if (ret) > > + goto err; > > + > > + platform_set_drvdata(pdev, data); > > + return 0; > > + > > +err: > > + hisi_clk_free(np, data); > > + return ret; > > +} > > +EXPORT_SYMBOL_GPL(hisi_clk_probe); > > + > > +void hisi_clk_remove(struct platform_device *pdev) > > +{ > > + struct device *dev = &pdev->dev; > > + struct device_node *np = dev->of_node; > > + struct hisi_clock_data *data = platform_get_drvdata(pdev); > > + > > + hisi_clk_free(np, data); > > +} > > +EXPORT_SYMBOL_GPL(hisi_clk_remove); > > diff --git a/drivers/clk/hisilicon/clk.h b/drivers/clk/hisilicon/clk.h > > index 7a9b42e1b027..87b17e9b79a3 100644 > > --- a/drivers/clk/hisilicon/clk.h > > +++ b/drivers/clk/hisilicon/clk.h > > @@ -17,10 +17,12 @@ > > #include <linux/spinlock.h> > > > > struct platform_device; > > +struct hisi_clocks; > > > > struct hisi_clock_data { > > - struct clk_onecell_data clk_data; > > - void __iomem *base; > > + struct clk_onecell_data clk_data; > > + void __iomem *base; > > + const struct hisi_clocks *clks; > > }; > > > > struct hisi_fixed_rate_clock { > > @@ -103,6 +105,39 @@ struct hisi_gate_clock { > > const char *alias; > > }; > > > > +struct hisi_clocks { > > + /* if 0, sum all *_num */ > > + size_t nr; > > + > > + int (*prologue)(struct device *dev, struct hisi_clock_data *data); > > + > > + const struct hisi_fixed_rate_clock *fixed_rate_clks; > > + size_t fixed_rate_clks_num; > > + > > + const struct hisi_fixed_factor_clock *fixed_factor_clks; > > + size_t fixed_factor_clks_num; > > + > > + const struct hisi_mux_clock *mux_clks; > > + size_t mux_clks_num; > > + > > + const struct hisi_phase_clock *phase_clks; > > + size_t phase_clks_num; > > + > > + const struct hisi_divider_clock *divider_clks; > > + size_t divider_clks_num; > > + > > + const struct hisi_gate_clock *gate_clks; > > + size_t gate_clks_num; > > + > > + const struct hisi_gate_clock *gate_sep_clks; > > + size_t gate_sep_clks_num; > > + > > + const void *customized_clks; > > + size_t customized_clks_num; > > + int (*clk_register_customized)(struct device *dev, const void *clks, > > + size_t num, struct hisi_clock_data *data); > > +}; > > + > > struct clk *hisi_register_clkgate_sep(struct device *, const char *, > > const char *, unsigned long, > > void __iomem *, u8, > > @@ -113,6 +148,7 @@ struct clk *hi6220_register_clkdiv(struct device *dev, const char *name, > > > > struct hisi_clock_data *hisi_clk_alloc(struct platform_device *, int); > > struct hisi_clock_data *hisi_clk_init(struct device_node *, int); > > +void hisi_clk_free(struct device_node *np, struct hisi_clock_data *data); > > int hisi_clk_register_fixed_rate(const struct hisi_fixed_rate_clock *, > > int, struct hisi_clock_data *); > > int hisi_clk_register_fixed_factor(const struct hisi_fixed_factor_clock *, > > @@ -154,4 +190,10 @@ hisi_clk_unregister(mux) > > hisi_clk_unregister(divider) > > hisi_clk_unregister(gate) > > > > +/* helper functions for platform driver */ > > + > > +int hisi_clk_early_init(struct device_node *np, const struct hisi_clocks *clks); > > +int hisi_clk_probe(struct platform_device *pdev); > > +void hisi_clk_remove(struct platform_device *pdev); > > + > > #endif /* __HISI_CLK_H */ > > diff --git a/drivers/clk/hisilicon/crg.h b/drivers/clk/hisilicon/crg.h > > index 803f6ba6d7a2..bd8e76b1f6d7 100644 > > --- a/drivers/clk/hisilicon/crg.h > > +++ b/drivers/clk/hisilicon/crg.h > > @@ -22,4 +22,9 @@ struct hisi_crg_dev { > > const struct hisi_crg_funcs *funcs; > > }; > > > > +/* helper functions for platform driver */ > > + > > +int hisi_crg_probe(struct platform_device *pdev); > > +void hisi_crg_remove(struct platform_device *pdev); > > + > > #endif /* __HISI_CRG_H */ > > diff --git a/drivers/clk/hisilicon/reset.c b/drivers/clk/hisilicon/reset.c > > index 93cee17db8b1..c7d4c9ea7183 100644 > > --- a/drivers/clk/hisilicon/reset.c > > +++ b/drivers/clk/hisilicon/reset.c > > @@ -6,11 +6,15 @@ > > */ > > > > #include <linux/io.h> > > +#include <linux/kernel.h> > > #include <linux/of_address.h> > > #include <linux/platform_device.h> > > #include <linux/reset-controller.h> > > #include <linux/slab.h> > > #include <linux/spinlock.h> > > + > > +#include "clk.h" > > +#include "crg.h" > > #include "reset.h" > > > > #define HISI_RESET_BIT_MASK 0x1f > > @@ -116,3 +120,41 @@ void hisi_reset_exit(struct hisi_reset_controller *rstc) > > reset_controller_unregister(&rstc->rcdev); > > } > > EXPORT_SYMBOL_GPL(hisi_reset_exit); > > + > > +int hisi_crg_probe(struct platform_device *pdev) > > +{ > > + struct device *dev = &pdev->dev; > > + struct hisi_crg_dev *crg; > > + int ret; > > + > > + crg = devm_kmalloc(dev, sizeof(*crg), GFP_KERNEL); > > + if (!crg) > > + return -ENOMEM; > > + > > + ret = hisi_clk_probe(pdev); > > + if (ret) > > + return ret; > > + > > + crg->rstc = hisi_reset_init(pdev); > > + if (!crg->rstc) { > > + ret = -ENOMEM; > > + goto err; > > + } > > + > > + platform_set_drvdata(pdev, crg); > > + return 0; > > + > > +err: > > + hisi_clk_remove(pdev); > > + return ret; > > +} > > +EXPORT_SYMBOL_GPL(hisi_crg_probe); > > + > > +void hisi_crg_remove(struct platform_device *pdev) > > +{ > > + struct hisi_crg_dev *crg = platform_get_drvdata(pdev); > > + > > + hisi_reset_exit(crg->rstc); > > + hisi_clk_remove(pdev); > > +} > > +EXPORT_SYMBOL_GPL(hisi_crg_remove); > > > -- > Regards, > Yang Xiwen >
diff --git a/drivers/clk/hisilicon/clk.c b/drivers/clk/hisilicon/clk.c index 09368fd32bef..e50115f8e236 100644 --- a/drivers/clk/hisilicon/clk.c +++ b/drivers/clk/hisilicon/clk.c @@ -88,6 +88,25 @@ struct hisi_clock_data *hisi_clk_init(struct device_node *np, } EXPORT_SYMBOL_GPL(hisi_clk_init); +void hisi_clk_free(struct device_node *np, struct hisi_clock_data *data) +{ + if (data->clks) { + if (data->clks->fixed_rate_clks_num) + hisi_clk_unregister_fixed_rate(data->clks->fixed_rate_clks, + data->clks->fixed_rate_clks_num, + data); + if (data->clks->fixed_factor_clks_num) + hisi_clk_unregister_fixed_factor(data->clks->fixed_factor_clks, + data->clks->fixed_factor_clks_num, + data); + } + + of_clk_del_provider(np); + kfree(data->clk_data.clks); + kfree(data); +} +EXPORT_SYMBOL_GPL(hisi_clk_free); + int hisi_clk_register_fixed_rate(const struct hisi_fixed_rate_clock *clks, int nums, struct hisi_clock_data *data) { @@ -341,3 +360,141 @@ void __init hi6220_clk_register_divider(const struct hi6220_divider_clock *clks, data->clk_data.clks[clks[i].id] = clk; } } + +static size_t hisi_clocks_get_nr(const struct hisi_clocks *clks) +{ + if (clks->nr) + return clks->nr; + + return clks->fixed_rate_clks_num + clks->fixed_factor_clks_num + + clks->mux_clks_num + clks->phase_clks_num + + clks->divider_clks_num + clks->gate_clks_num + + clks->gate_sep_clks_num + clks->customized_clks_num; +} + +int hisi_clk_early_init(struct device_node *np, const struct hisi_clocks *clks) +{ + struct hisi_clock_data *data; + int ret; + + data = hisi_clk_init(np, hisi_clocks_get_nr(clks)); + if (!data) + return -ENOMEM; + data->clks = clks; + + ret = hisi_clk_register_fixed_rate(clks->fixed_rate_clks, + clks->fixed_rate_clks_num, data); + if (ret) + goto err; + + ret = hisi_clk_register_fixed_factor(clks->fixed_factor_clks, + clks->fixed_factor_clks_num, data); + if (ret) + goto err; + + np->data = data; + return 0; + +err: + hisi_clk_free(np, data); + return ret; +} +EXPORT_SYMBOL_GPL(hisi_clk_early_init); + +static int hisi_clk_register(struct device *dev, const struct hisi_clocks *clks, + struct hisi_clock_data *data) +{ + int ret; + + if (clks->mux_clks_num) { + ret = hisi_clk_register_mux(clks->mux_clks, + clks->mux_clks_num, data); + if (ret) + return ret; + } + + if (clks->phase_clks_num) { + ret = hisi_clk_register_phase(dev, clks->phase_clks, + clks->phase_clks_num, data); + if (ret) + return ret; + } + + if (clks->divider_clks_num) { + ret = hisi_clk_register_divider(clks->divider_clks, + clks->divider_clks_num, data); + if (ret) + return ret; + } + + if (clks->gate_clks_num) { + ret = hisi_clk_register_gate(clks->gate_clks, + clks->gate_clks_num, data); + if (ret) + return ret; + } + + if (clks->gate_sep_clks_num) { + hisi_clk_register_gate_sep(clks->gate_sep_clks, + clks->gate_sep_clks_num, data); + } + + if (clks->clk_register_customized && clks->customized_clks_num) { + ret = clks->clk_register_customized(dev, clks->customized_clks, + clks->customized_clks_num, data); + if (ret) + return ret; + } + + return 0; +} + +int hisi_clk_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct device_node *np = dev->of_node; + const struct hisi_clocks *clks; + struct hisi_clock_data *data; + int ret; + + clks = of_device_get_match_data(dev); + if (!clks) + return -ENOENT; + + if (!np->data) { + ret = hisi_clk_early_init(np, clks); + if (ret) + return ret; + } + + data = np->data; + np->data = NULL; + + if (clks->prologue) { + ret = clks->prologue(dev, data); + if (ret) + goto err; + } + + ret = hisi_clk_register(dev, clks, data); + if (ret) + goto err; + + platform_set_drvdata(pdev, data); + return 0; + +err: + hisi_clk_free(np, data); + return ret; +} +EXPORT_SYMBOL_GPL(hisi_clk_probe); + +void hisi_clk_remove(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct device_node *np = dev->of_node; + struct hisi_clock_data *data = platform_get_drvdata(pdev); + + hisi_clk_free(np, data); +} +EXPORT_SYMBOL_GPL(hisi_clk_remove); diff --git a/drivers/clk/hisilicon/clk.h b/drivers/clk/hisilicon/clk.h index 7a9b42e1b027..87b17e9b79a3 100644 --- a/drivers/clk/hisilicon/clk.h +++ b/drivers/clk/hisilicon/clk.h @@ -17,10 +17,12 @@ #include <linux/spinlock.h> struct platform_device; +struct hisi_clocks; struct hisi_clock_data { - struct clk_onecell_data clk_data; - void __iomem *base; + struct clk_onecell_data clk_data; + void __iomem *base; + const struct hisi_clocks *clks; }; struct hisi_fixed_rate_clock { @@ -103,6 +105,39 @@ struct hisi_gate_clock { const char *alias; }; +struct hisi_clocks { + /* if 0, sum all *_num */ + size_t nr; + + int (*prologue)(struct device *dev, struct hisi_clock_data *data); + + const struct hisi_fixed_rate_clock *fixed_rate_clks; + size_t fixed_rate_clks_num; + + const struct hisi_fixed_factor_clock *fixed_factor_clks; + size_t fixed_factor_clks_num; + + const struct hisi_mux_clock *mux_clks; + size_t mux_clks_num; + + const struct hisi_phase_clock *phase_clks; + size_t phase_clks_num; + + const struct hisi_divider_clock *divider_clks; + size_t divider_clks_num; + + const struct hisi_gate_clock *gate_clks; + size_t gate_clks_num; + + const struct hisi_gate_clock *gate_sep_clks; + size_t gate_sep_clks_num; + + const void *customized_clks; + size_t customized_clks_num; + int (*clk_register_customized)(struct device *dev, const void *clks, + size_t num, struct hisi_clock_data *data); +}; + struct clk *hisi_register_clkgate_sep(struct device *, const char *, const char *, unsigned long, void __iomem *, u8, @@ -113,6 +148,7 @@ struct clk *hi6220_register_clkdiv(struct device *dev, const char *name, struct hisi_clock_data *hisi_clk_alloc(struct platform_device *, int); struct hisi_clock_data *hisi_clk_init(struct device_node *, int); +void hisi_clk_free(struct device_node *np, struct hisi_clock_data *data); int hisi_clk_register_fixed_rate(const struct hisi_fixed_rate_clock *, int, struct hisi_clock_data *); int hisi_clk_register_fixed_factor(const struct hisi_fixed_factor_clock *, @@ -154,4 +190,10 @@ hisi_clk_unregister(mux) hisi_clk_unregister(divider) hisi_clk_unregister(gate) +/* helper functions for platform driver */ + +int hisi_clk_early_init(struct device_node *np, const struct hisi_clocks *clks); +int hisi_clk_probe(struct platform_device *pdev); +void hisi_clk_remove(struct platform_device *pdev); + #endif /* __HISI_CLK_H */ diff --git a/drivers/clk/hisilicon/crg.h b/drivers/clk/hisilicon/crg.h index 803f6ba6d7a2..bd8e76b1f6d7 100644 --- a/drivers/clk/hisilicon/crg.h +++ b/drivers/clk/hisilicon/crg.h @@ -22,4 +22,9 @@ struct hisi_crg_dev { const struct hisi_crg_funcs *funcs; }; +/* helper functions for platform driver */ + +int hisi_crg_probe(struct platform_device *pdev); +void hisi_crg_remove(struct platform_device *pdev); + #endif /* __HISI_CRG_H */ diff --git a/drivers/clk/hisilicon/reset.c b/drivers/clk/hisilicon/reset.c index 93cee17db8b1..c7d4c9ea7183 100644 --- a/drivers/clk/hisilicon/reset.c +++ b/drivers/clk/hisilicon/reset.c @@ -6,11 +6,15 @@ */ #include <linux/io.h> +#include <linux/kernel.h> #include <linux/of_address.h> #include <linux/platform_device.h> #include <linux/reset-controller.h> #include <linux/slab.h> #include <linux/spinlock.h> + +#include "clk.h" +#include "crg.h" #include "reset.h" #define HISI_RESET_BIT_MASK 0x1f @@ -116,3 +120,41 @@ void hisi_reset_exit(struct hisi_reset_controller *rstc) reset_controller_unregister(&rstc->rcdev); } EXPORT_SYMBOL_GPL(hisi_reset_exit); + +int hisi_crg_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct hisi_crg_dev *crg; + int ret; + + crg = devm_kmalloc(dev, sizeof(*crg), GFP_KERNEL); + if (!crg) + return -ENOMEM; + + ret = hisi_clk_probe(pdev); + if (ret) + return ret; + + crg->rstc = hisi_reset_init(pdev); + if (!crg->rstc) { + ret = -ENOMEM; + goto err; + } + + platform_set_drvdata(pdev, crg); + return 0; + +err: + hisi_clk_remove(pdev); + return ret; +} +EXPORT_SYMBOL_GPL(hisi_crg_probe); + +void hisi_crg_remove(struct platform_device *pdev) +{ + struct hisi_crg_dev *crg = platform_get_drvdata(pdev); + + hisi_reset_exit(crg->rstc); + hisi_clk_remove(pdev); +} +EXPORT_SYMBOL_GPL(hisi_crg_remove);
Helper functions extract common operations on platform drivers. During migration to devm APIs, (virtual) fixed clocks were found hard on devm APIs, since they often depended by crucial peripherals, thus require early initialization before device probing, and cannot use devm APIs. One solution to this problem is to add a "fixed-clock" node to device tree, independent to clock device, and make those peripherals depend on that. However, there is also some devices that do use fixed clocks provided by drivers, for example clk-hi3660.c . To simplify codes, we migrate clocks of other types to devm APIs, while keep fixed clocks self-managed, alongside with struct hisi_clock_data, and remove devm-managed hisi_clock_data. `hisi_clk_alloc` will be removed in the following patch. Signed-off-by: David Yang <mmyangfl@gmail.com> --- drivers/clk/hisilicon/clk.c | 157 ++++++++++++++++++++++++++++++++++ drivers/clk/hisilicon/clk.h | 46 +++++++++- drivers/clk/hisilicon/crg.h | 5 ++ drivers/clk/hisilicon/reset.c | 42 +++++++++ 4 files changed, 248 insertions(+), 2 deletions(-)