* [PATCH 0/3] clk: let clock perform allocation in init
@ 2019-09-24 12:39 Jerome Brunet
2019-09-24 12:39 ` [PATCH 1/3] clk: actually call the clock init before any other callback of the clock Jerome Brunet
` (4 more replies)
0 siblings, 5 replies; 14+ messages in thread
From: Jerome Brunet @ 2019-09-24 12:39 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd; +Cc: Jerome Brunet, linux-clk, linux-kernel
This patchset is a follow up on this pinky swear [0].
Its purpose is:
* Clarify the acceptable use of clk_ops init() callback
* Let the init() callback return an error code in case anything
fail.
* Add the terminate() counter part of of init() to release the
resources which may have been claimed in init()
After discussing with Stephen at LPC, I decided to drop the 2 last patches
of the RFC [1]. I can live without it for now and nobody expressed a
critical need to get the proposed placeholder.
[0]: https://lkml.kernel.org/r/CAEG3pNB-143Pr_xCTPj=tURhpiTiJqi61xfDGDVdU7zG5H-2tA@mail.gmail.com
[1]: https://lkml.kernel.org/r/20190828102012.4493-1-jbrunet@baylibre.com
Jerome Brunet (3):
clk: actually call the clock init before any other callback of the
clock
clk: let init callback return an error code
clk: add terminate callback to clk_ops
drivers/clk/clk.c | 38 ++++++++++++++++++---------
drivers/clk/meson/clk-mpll.c | 4 ++-
drivers/clk/meson/clk-phase.c | 4 ++-
drivers/clk/meson/clk-pll.c | 4 ++-
drivers/clk/meson/sclk-div.c | 4 ++-
drivers/clk/microchip/clk-core.c | 8 ++++--
drivers/clk/mmp/clk-frac.c | 4 ++-
drivers/clk/mmp/clk-mix.c | 4 ++-
drivers/clk/qcom/clk-hfpll.c | 6 +++--
drivers/clk/rockchip/clk-pll.c | 28 ++++++++++++--------
drivers/clk/ti/clock.h | 2 +-
drivers/clk/ti/clockdomain.c | 8 +++---
drivers/net/phy/mdio-mux-meson-g12a.c | 4 ++-
include/linux/clk-provider.h | 13 ++++++---
14 files changed, 90 insertions(+), 41 deletions(-)
--
2.21.0
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH 1/3] clk: actually call the clock init before any other callback of the clock 2019-09-24 12:39 [PATCH 0/3] clk: let clock perform allocation in init Jerome Brunet @ 2019-09-24 12:39 ` Jerome Brunet 2019-12-24 2:59 ` Stephen Boyd 2019-09-24 12:39 ` [PATCH 2/3] clk: let init callback return an error code Jerome Brunet ` (3 subsequent siblings) 4 siblings, 1 reply; 14+ messages in thread From: Jerome Brunet @ 2019-09-24 12:39 UTC (permalink / raw) To: Michael Turquette, Stephen Boyd; +Cc: Jerome Brunet, linux-clk, linux-kernel __clk_init_parent() will call the .get_parent() callback of the clock so .init() must run before. Fixes: 541debae0adf ("clk: call the clock init() callback before any other ops callback") Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> --- drivers/clk/clk.c | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index 1c677d7f7f53..232144cca6cf 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -3294,6 +3294,21 @@ static int __clk_core_init(struct clk_core *core) goto out; } + /* + * optional platform-specific magic + * + * The .init callback is not used by any of the basic clock types, but + * exists for weird hardware that must perform initialization magic. + * Please consider other ways of solving initialization problems before + * using this callback, as its use is discouraged. + * + * If it exist, this callback should called before any other callback of + * the clock + */ + if (core->ops->init) + core->ops->init(core->hw); + + core->parent = __clk_init_parent(core); /* @@ -3318,17 +3333,6 @@ static int __clk_core_init(struct clk_core *core) core->orphan = true; } - /* - * optional platform-specific magic - * - * The .init callback is not used by any of the basic clock types, but - * exists for weird hardware that must perform initialization magic. - * Please consider other ways of solving initialization problems before - * using this callback, as its use is discouraged. - */ - if (core->ops->init) - core->ops->init(core->hw); - /* * Set clk's accuracy. The preferred method is to use * .recalc_accuracy. For simple clocks and lazy developers the default -- 2.21.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] clk: actually call the clock init before any other callback of the clock 2019-09-24 12:39 ` [PATCH 1/3] clk: actually call the clock init before any other callback of the clock Jerome Brunet @ 2019-12-24 2:59 ` Stephen Boyd 0 siblings, 0 replies; 14+ messages in thread From: Stephen Boyd @ 2019-12-24 2:59 UTC (permalink / raw) To: Jerome Brunet, Michael Turquette; +Cc: Jerome Brunet, linux-clk, linux-kernel Quoting Jerome Brunet (2019-09-24 05:39:52) > __clk_init_parent() will call the .get_parent() callback of the clock > so .init() must run before. > > Fixes: 541debae0adf ("clk: call the clock init() callback before any other ops callback") > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> > --- Applied to clk-next ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/3] clk: let init callback return an error code 2019-09-24 12:39 [PATCH 0/3] clk: let clock perform allocation in init Jerome Brunet 2019-09-24 12:39 ` [PATCH 1/3] clk: actually call the clock init before any other callback of the clock Jerome Brunet @ 2019-09-24 12:39 ` Jerome Brunet 2019-09-24 13:38 ` Ankur Tyagi ` (2 more replies) 2019-09-24 12:39 ` [PATCH 3/3] clk: add terminate callback to clk_ops Jerome Brunet ` (2 subsequent siblings) 4 siblings, 3 replies; 14+ messages in thread From: Jerome Brunet @ 2019-09-24 12:39 UTC (permalink / raw) To: Michael Turquette, Stephen Boyd Cc: Jerome Brunet, linux-clk, linux-kernel, Heiko Stuebner, Tero Kristo, Andrew Lunn, Florian Fainelli, Heiner Kallweit, David S. Miller, netdev, linux-amlogic, linux-arm-msm, linux-rockchip, linux-omap If the init callback is allowed to request resources, it needs a return value to report the outcome of such a request. Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> --- Sorry about the spam. This patch change quite a few files so I have tried to Cc the relevant people. Stephen, You may notice that I have added a couple of the network people. You need an Ack from one of them since the Amlogic G12a mdio mux has a clock which uses the .init() callback drivers/clk/clk.c | 17 ++++++++++------ drivers/clk/meson/clk-mpll.c | 4 +++- drivers/clk/meson/clk-phase.c | 4 +++- drivers/clk/meson/clk-pll.c | 4 +++- drivers/clk/meson/sclk-div.c | 4 +++- drivers/clk/microchip/clk-core.c | 8 ++++++-- drivers/clk/mmp/clk-frac.c | 4 +++- drivers/clk/mmp/clk-mix.c | 4 +++- drivers/clk/qcom/clk-hfpll.c | 6 ++++-- drivers/clk/rockchip/clk-pll.c | 28 ++++++++++++++++----------- drivers/clk/ti/clock.h | 2 +- drivers/clk/ti/clockdomain.c | 8 +++++--- drivers/net/phy/mdio-mux-meson-g12a.c | 4 +++- include/linux/clk-provider.h | 10 +++++++--- 14 files changed, 72 insertions(+), 35 deletions(-) diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index 232144cca6cf..df853a98fad2 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -3298,16 +3298,21 @@ static int __clk_core_init(struct clk_core *core) * optional platform-specific magic * * The .init callback is not used by any of the basic clock types, but - * exists for weird hardware that must perform initialization magic. - * Please consider other ways of solving initialization problems before - * using this callback, as its use is discouraged. + * exists for weird hardware that must perform initialization magic for + * CCF to get an accurate view of clock for any other callbacks. It may + * also be used needs to perform dynamic allocations. Such allocation + * must be freed in the terminate() callback. + * This callback shall not be used to initialize the parameters state, + * such as rate, parent, etc ... * * If it exist, this callback should called before any other callback of * the clock */ - if (core->ops->init) - core->ops->init(core->hw); - + if (core->ops->init) { + ret = core->ops->init(core->hw); + if (ret) + goto out; + } core->parent = __clk_init_parent(core); diff --git a/drivers/clk/meson/clk-mpll.c b/drivers/clk/meson/clk-mpll.c index 2d39a8bc367c..fc9df4860872 100644 --- a/drivers/clk/meson/clk-mpll.c +++ b/drivers/clk/meson/clk-mpll.c @@ -129,7 +129,7 @@ static int mpll_set_rate(struct clk_hw *hw, return 0; } -static void mpll_init(struct clk_hw *hw) +static int mpll_init(struct clk_hw *hw) { struct clk_regmap *clk = to_clk_regmap(hw); struct meson_clk_mpll_data *mpll = meson_clk_mpll_data(clk); @@ -151,6 +151,8 @@ static void mpll_init(struct clk_hw *hw) /* Set the magic misc bit if required */ if (MESON_PARM_APPLICABLE(&mpll->misc)) meson_parm_write(clk->map, &mpll->misc, 1); + + return 0; } const struct clk_ops meson_clk_mpll_ro_ops = { diff --git a/drivers/clk/meson/clk-phase.c b/drivers/clk/meson/clk-phase.c index 80c3ada193a4..fe22e171121a 100644 --- a/drivers/clk/meson/clk-phase.c +++ b/drivers/clk/meson/clk-phase.c @@ -78,7 +78,7 @@ meson_clk_triphase_data(struct clk_regmap *clk) return (struct meson_clk_triphase_data *)clk->data; } -static void meson_clk_triphase_sync(struct clk_hw *hw) +static int meson_clk_triphase_sync(struct clk_hw *hw) { struct clk_regmap *clk = to_clk_regmap(hw); struct meson_clk_triphase_data *tph = meson_clk_triphase_data(clk); @@ -88,6 +88,8 @@ static void meson_clk_triphase_sync(struct clk_hw *hw) val = meson_parm_read(clk->map, &tph->ph0); meson_parm_write(clk->map, &tph->ph1, val); meson_parm_write(clk->map, &tph->ph2, val); + + return 0; } static int meson_clk_triphase_get_phase(struct clk_hw *hw) diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c index ddb1e5634739..489092dde3a6 100644 --- a/drivers/clk/meson/clk-pll.c +++ b/drivers/clk/meson/clk-pll.c @@ -277,7 +277,7 @@ static int meson_clk_pll_wait_lock(struct clk_hw *hw) return -ETIMEDOUT; } -static void meson_clk_pll_init(struct clk_hw *hw) +static int meson_clk_pll_init(struct clk_hw *hw) { struct clk_regmap *clk = to_clk_regmap(hw); struct meson_clk_pll_data *pll = meson_clk_pll_data(clk); @@ -288,6 +288,8 @@ static void meson_clk_pll_init(struct clk_hw *hw) pll->init_count); meson_parm_write(clk->map, &pll->rst, 0); } + + return 0; } static int meson_clk_pll_is_enabled(struct clk_hw *hw) diff --git a/drivers/clk/meson/sclk-div.c b/drivers/clk/meson/sclk-div.c index 3acf03780221..76d31c0a3342 100644 --- a/drivers/clk/meson/sclk-div.c +++ b/drivers/clk/meson/sclk-div.c @@ -216,7 +216,7 @@ static int sclk_div_is_enabled(struct clk_hw *hw) return 0; } -static void sclk_div_init(struct clk_hw *hw) +static int sclk_div_init(struct clk_hw *hw) { struct clk_regmap *clk = to_clk_regmap(hw); struct meson_sclk_div_data *sclk = meson_sclk_div_data(clk); @@ -231,6 +231,8 @@ static void sclk_div_init(struct clk_hw *hw) sclk->cached_div = val + 1; sclk_div_get_duty_cycle(hw, &sclk->cached_duty); + + return 0; } const struct clk_ops meson_sclk_div_ops = { diff --git a/drivers/clk/microchip/clk-core.c b/drivers/clk/microchip/clk-core.c index 567755d6f844..1b4f023cdc8b 100644 --- a/drivers/clk/microchip/clk-core.c +++ b/drivers/clk/microchip/clk-core.c @@ -266,10 +266,12 @@ static void roclk_disable(struct clk_hw *hw) writel(REFO_ON | REFO_OE, PIC32_CLR(refo->ctrl_reg)); } -static void roclk_init(struct clk_hw *hw) +static int roclk_init(struct clk_hw *hw) { /* initialize clock in disabled state */ roclk_disable(hw); + + return 0; } static u8 roclk_get_parent(struct clk_hw *hw) @@ -880,7 +882,7 @@ static int sclk_set_parent(struct clk_hw *hw, u8 index) return err; } -static void sclk_init(struct clk_hw *hw) +static int sclk_init(struct clk_hw *hw) { struct pic32_sys_clk *sclk = clkhw_to_sys_clk(hw); unsigned long flags; @@ -899,6 +901,8 @@ static void sclk_init(struct clk_hw *hw) writel(v, sclk->slew_reg); spin_unlock_irqrestore(&sclk->core->reg_lock, flags); } + + return 0; } /* sclk with post-divider */ diff --git a/drivers/clk/mmp/clk-frac.c b/drivers/clk/mmp/clk-frac.c index 90bf181f191a..fabc09aca6c4 100644 --- a/drivers/clk/mmp/clk-frac.c +++ b/drivers/clk/mmp/clk-frac.c @@ -109,7 +109,7 @@ static int clk_factor_set_rate(struct clk_hw *hw, unsigned long drate, return 0; } -static void clk_factor_init(struct clk_hw *hw) +static int clk_factor_init(struct clk_hw *hw) { struct mmp_clk_factor *factor = to_clk_factor(hw); struct mmp_clk_factor_masks *masks = factor->masks; @@ -146,6 +146,8 @@ static void clk_factor_init(struct clk_hw *hw) if (factor->lock) spin_unlock_irqrestore(factor->lock, flags); + + return 0; } static const struct clk_ops clk_factor_ops = { diff --git a/drivers/clk/mmp/clk-mix.c b/drivers/clk/mmp/clk-mix.c index 90814b2613c0..d2cd36c54474 100644 --- a/drivers/clk/mmp/clk-mix.c +++ b/drivers/clk/mmp/clk-mix.c @@ -419,12 +419,14 @@ static int mmp_clk_set_rate(struct clk_hw *hw, unsigned long rate, } } -static void mmp_clk_mix_init(struct clk_hw *hw) +static int mmp_clk_mix_init(struct clk_hw *hw) { struct mmp_clk_mix *mix = to_clk_mix(hw); if (mix->table) _filter_clk_table(mix, mix->table, mix->table_size); + + return 0; } const struct clk_ops mmp_clk_mix_ops = { diff --git a/drivers/clk/qcom/clk-hfpll.c b/drivers/clk/qcom/clk-hfpll.c index 3c04805f2a55..e847d586a73a 100644 --- a/drivers/clk/qcom/clk-hfpll.c +++ b/drivers/clk/qcom/clk-hfpll.c @@ -196,7 +196,7 @@ static unsigned long clk_hfpll_recalc_rate(struct clk_hw *hw, return l_val * parent_rate; } -static void clk_hfpll_init(struct clk_hw *hw) +static int clk_hfpll_init(struct clk_hw *hw) { struct clk_hfpll *h = to_clk_hfpll(hw); struct hfpll_data const *hd = h->d; @@ -206,7 +206,7 @@ static void clk_hfpll_init(struct clk_hw *hw) regmap_read(regmap, hd->mode_reg, &mode); if (mode != (PLL_BYPASSNL | PLL_RESET_N | PLL_OUTCTRL)) { __clk_hfpll_init_once(hw); - return; + return 0; } if (hd->status_reg) { @@ -218,6 +218,8 @@ static void clk_hfpll_init(struct clk_hw *hw) __clk_hfpll_init_once(hw); } } + + return 0; } static int hfpll_is_enabled(struct clk_hw *hw) diff --git a/drivers/clk/rockchip/clk-pll.c b/drivers/clk/rockchip/clk-pll.c index 198417d56300..10560d963baf 100644 --- a/drivers/clk/rockchip/clk-pll.c +++ b/drivers/clk/rockchip/clk-pll.c @@ -282,7 +282,7 @@ static int rockchip_rk3036_pll_is_enabled(struct clk_hw *hw) return !(pllcon & RK3036_PLLCON1_PWRDOWN); } -static void rockchip_rk3036_pll_init(struct clk_hw *hw) +static int rockchip_rk3036_pll_init(struct clk_hw *hw) { struct rockchip_clk_pll *pll = to_rockchip_clk_pll(hw); const struct rockchip_pll_rate_table *rate; @@ -290,14 +290,14 @@ static void rockchip_rk3036_pll_init(struct clk_hw *hw) unsigned long drate; if (!(pll->flags & ROCKCHIP_PLL_SYNC_RATE)) - return; + return 0; drate = clk_hw_get_rate(hw); rate = rockchip_get_pll_settings(pll, drate); /* when no rate setting for the current rate, rely on clk_set_rate */ if (!rate) - return; + return 0; rockchip_rk3036_pll_get_params(pll, &cur); @@ -319,13 +319,15 @@ static void rockchip_rk3036_pll_init(struct clk_hw *hw) if (!parent) { pr_warn("%s: parent of %s not available\n", __func__, __clk_get_name(hw->clk)); - return; + return 0; } pr_debug("%s: pll %s: rate params do not match rate table, adjusting\n", __func__, __clk_get_name(hw->clk)); rockchip_rk3036_pll_set_params(pll, rate); } + + return 0; } static const struct clk_ops rockchip_rk3036_pll_clk_norate_ops = { @@ -515,7 +517,7 @@ static int rockchip_rk3066_pll_is_enabled(struct clk_hw *hw) return !(pllcon & RK3066_PLLCON3_PWRDOWN); } -static void rockchip_rk3066_pll_init(struct clk_hw *hw) +static int rockchip_rk3066_pll_init(struct clk_hw *hw) { struct rockchip_clk_pll *pll = to_rockchip_clk_pll(hw); const struct rockchip_pll_rate_table *rate; @@ -523,14 +525,14 @@ static void rockchip_rk3066_pll_init(struct clk_hw *hw) unsigned long drate; if (!(pll->flags & ROCKCHIP_PLL_SYNC_RATE)) - return; + return 0; drate = clk_hw_get_rate(hw); rate = rockchip_get_pll_settings(pll, drate); /* when no rate setting for the current rate, rely on clk_set_rate */ if (!rate) - return; + return 0; rockchip_rk3066_pll_get_params(pll, &cur); @@ -543,6 +545,8 @@ static void rockchip_rk3066_pll_init(struct clk_hw *hw) __func__, clk_hw_get_name(hw)); rockchip_rk3066_pll_set_params(pll, rate); } + + return 0; } static const struct clk_ops rockchip_rk3066_pll_clk_norate_ops = { @@ -761,7 +765,7 @@ static int rockchip_rk3399_pll_is_enabled(struct clk_hw *hw) return !(pllcon & RK3399_PLLCON3_PWRDOWN); } -static void rockchip_rk3399_pll_init(struct clk_hw *hw) +static int rockchip_rk3399_pll_init(struct clk_hw *hw) { struct rockchip_clk_pll *pll = to_rockchip_clk_pll(hw); const struct rockchip_pll_rate_table *rate; @@ -769,14 +773,14 @@ static void rockchip_rk3399_pll_init(struct clk_hw *hw) unsigned long drate; if (!(pll->flags & ROCKCHIP_PLL_SYNC_RATE)) - return; + return 0; drate = clk_hw_get_rate(hw); rate = rockchip_get_pll_settings(pll, drate); /* when no rate setting for the current rate, rely on clk_set_rate */ if (!rate) - return; + return 0; rockchip_rk3399_pll_get_params(pll, &cur); @@ -798,13 +802,15 @@ static void rockchip_rk3399_pll_init(struct clk_hw *hw) if (!parent) { pr_warn("%s: parent of %s not available\n", __func__, __clk_get_name(hw->clk)); - return; + return 0; } pr_debug("%s: pll %s: rate params do not match rate table, adjusting\n", __func__, __clk_get_name(hw->clk)); rockchip_rk3399_pll_set_params(pll, rate); } + + return 0; } static const struct clk_ops rockchip_rk3399_pll_clk_norate_ops = { diff --git a/drivers/clk/ti/clock.h b/drivers/clk/ti/clock.h index e4b8392ff63c..adef9c16e43b 100644 --- a/drivers/clk/ti/clock.h +++ b/drivers/clk/ti/clock.h @@ -252,7 +252,7 @@ extern const struct clk_ops omap_gate_clk_ops; extern struct ti_clk_features ti_clk_features; -void omap2_init_clk_clkdm(struct clk_hw *hw); +int omap2_init_clk_clkdm(struct clk_hw *hw); int omap2_clkops_enable_clkdm(struct clk_hw *hw); void omap2_clkops_disable_clkdm(struct clk_hw *hw); diff --git a/drivers/clk/ti/clockdomain.c b/drivers/clk/ti/clockdomain.c index 423a99b9f10c..ee56306f79d5 100644 --- a/drivers/clk/ti/clockdomain.c +++ b/drivers/clk/ti/clockdomain.c @@ -101,16 +101,16 @@ void omap2_clkops_disable_clkdm(struct clk_hw *hw) * * Convert a clockdomain name stored in a struct clk 'clk' into a * clockdomain pointer, and save it into the struct clk. Intended to be - * called during clk_register(). No return value. + * called during clk_register(). Returns 0 on success, -EERROR otherwise. */ -void omap2_init_clk_clkdm(struct clk_hw *hw) +int omap2_init_clk_clkdm(struct clk_hw *hw) { struct clk_hw_omap *clk = to_clk_hw_omap(hw); struct clockdomain *clkdm; const char *clk_name; if (!clk->clkdm_name) - return; + return 0; clk_name = __clk_get_name(hw->clk); @@ -123,6 +123,8 @@ void omap2_init_clk_clkdm(struct clk_hw *hw) pr_debug("clock: could not associate clk %s to clkdm %s\n", clk_name, clk->clkdm_name); } + + return 0; } static void __init of_ti_clockdomain_setup(struct device_node *node) diff --git a/drivers/net/phy/mdio-mux-meson-g12a.c b/drivers/net/phy/mdio-mux-meson-g12a.c index 7a9ad54582e1..bf86c9c7a288 100644 --- a/drivers/net/phy/mdio-mux-meson-g12a.c +++ b/drivers/net/phy/mdio-mux-meson-g12a.c @@ -123,7 +123,7 @@ static int g12a_ephy_pll_is_enabled(struct clk_hw *hw) return (val & PLL_CTL0_LOCK_DIG) ? 1 : 0; } -static void g12a_ephy_pll_init(struct clk_hw *hw) +static int g12a_ephy_pll_init(struct clk_hw *hw) { struct g12a_ephy_pll *pll = g12a_ephy_pll_to_dev(hw); @@ -136,6 +136,8 @@ static void g12a_ephy_pll_init(struct clk_hw *hw) writel(0x20200000, pll->base + ETH_PLL_CTL5); writel(0x0000c002, pll->base + ETH_PLL_CTL6); writel(0x00000023, pll->base + ETH_PLL_CTL7); + + return 0; } static const struct clk_ops g12a_ephy_pll_ops = { diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h index 2fdfe8061363..b82ec4c4ca95 100644 --- a/include/linux/clk-provider.h +++ b/include/linux/clk-provider.h @@ -190,8 +190,12 @@ struct clk_duty { * * @init: Perform platform-specific initialization magic. * This is not not used by any of the basic clock types. - * Please consider other ways of solving initialization problems - * before using this callback, as its use is discouraged. + * This callback exist for HW which needs to perform some + * initialisation magic for CCF to get an accurate view of the + * clock. It may also be used dynamic resource allocation is + * required. It shall not used to deal with clock parameters, + * such as rate or parents. + * Returns 0 on success, -EERROR otherwise. * * @debug_init: Set up type-specific debugfs entries for this clock. This * is called once, after the debugfs directory entry for this @@ -243,7 +247,7 @@ struct clk_ops { struct clk_duty *duty); int (*set_duty_cycle)(struct clk_hw *hw, struct clk_duty *duty); - void (*init)(struct clk_hw *hw); + int (*init)(struct clk_hw *hw); void (*debug_init)(struct clk_hw *hw, struct dentry *dentry); }; -- 2.21.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* RE: [PATCH 2/3] clk: let init callback return an error code 2019-09-24 12:39 ` [PATCH 2/3] clk: let init callback return an error code Jerome Brunet @ 2019-09-24 13:38 ` Ankur Tyagi 2019-09-24 14:10 ` Jerome Brunet 2019-09-24 16:19 ` Andrew Lunn 2019-09-29 19:33 ` Heiko Stuebner 2 siblings, 1 reply; 14+ messages in thread From: Ankur Tyagi @ 2019-09-24 13:38 UTC (permalink / raw) To: Jerome Brunet, Michael Turquette, Stephen Boyd Cc: linux-clk, linux-kernel, Heiko Stuebner, Tero Kristo, Andrew Lunn, Florian Fainelli, Heiner Kallweit, David S. Miller, netdev, linux-amlogic, linux-arm-msm, linux-rockchip, linux-omap Hi, I am no expert here but just looked at the patch and found few discrepancy that I have mentioned inline. > -----Original Message----- > From: linux-omap-owner@vger.kernel.org <linux-omap- > owner@vger.kernel.org> On Behalf Of Jerome Brunet > Sent: Wednesday, 25 September 2019 12:40 AM > To: Michael Turquette <mturquette@baylibre.com>; Stephen Boyd > <sboyd@kernel.org> > Cc: Jerome Brunet <jbrunet@baylibre.com>; linux-clk@vger.kernel.org; linux- > kernel@vger.kernel.org; Heiko Stuebner <heiko@sntech.de>; Tero Kristo <t- > kristo@ti.com>; Andrew Lunn <andrew@lunn.ch>; Florian Fainelli > <f.fainelli@gmail.com>; Heiner Kallweit <hkallweit1@gmail.com>; David S. > Miller <davem@davemloft.net>; netdev@vger.kernel.org; linux- > amlogic@lists.infradead.org; linux-arm-msm@vger.kernel.org; linux- > rockchip@lists.infradead.org; linux-omap@vger.kernel.org > Subject: [PATCH 2/3] clk: let init callback return an error code > > If the init callback is allowed to request resources, it needs a return > value to report the outcome of such a request. > > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> > --- > > Sorry about the spam. > This patch change quite a few files so I have tried to Cc the > relevant people. Stephen, You may notice that I have added a > couple of the network people. You need an Ack from one of them > since the Amlogic G12a mdio mux has a clock which uses the .init() > callback > > drivers/clk/clk.c | 17 ++++++++++------ > drivers/clk/meson/clk-mpll.c | 4 +++- > drivers/clk/meson/clk-phase.c | 4 +++- > drivers/clk/meson/clk-pll.c | 4 +++- > drivers/clk/meson/sclk-div.c | 4 +++- > drivers/clk/microchip/clk-core.c | 8 ++++++-- > drivers/clk/mmp/clk-frac.c | 4 +++- > drivers/clk/mmp/clk-mix.c | 4 +++- > drivers/clk/qcom/clk-hfpll.c | 6 ++++-- > drivers/clk/rockchip/clk-pll.c | 28 ++++++++++++++++----------- > drivers/clk/ti/clock.h | 2 +- > drivers/clk/ti/clockdomain.c | 8 +++++--- > drivers/net/phy/mdio-mux-meson-g12a.c | 4 +++- > include/linux/clk-provider.h | 10 +++++++--- > 14 files changed, 72 insertions(+), 35 deletions(-) > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > index 232144cca6cf..df853a98fad2 100644 > --- a/drivers/clk/clk.c > +++ b/drivers/clk/clk.c > @@ -3298,16 +3298,21 @@ static int __clk_core_init(struct clk_core *core) > * optional platform-specific magic > * > * The .init callback is not used by any of the basic clock types, but > - * exists for weird hardware that must perform initialization magic. > - * Please consider other ways of solving initialization problems before > - * using this callback, as its use is discouraged. > + * exists for weird hardware that must perform initialization magic for > + * CCF to get an accurate view of clock for any other callbacks. It may > + * also be used needs to perform dynamic allocations. Such allocation > + * must be freed in the terminate() callback. > + * This callback shall not be used to initialize the parameters state, > + * such as rate, parent, etc ... > * > * If it exist, this callback should called before any other callback of > * the clock > */ > -if (core->ops->init) > -core->ops->init(core->hw); > - > +if (core->ops->init) { > +ret = core->ops->init(core->hw); > +if (ret) > +goto out; > +} > > core->parent = __clk_init_parent(core); > > diff --git a/drivers/clk/meson/clk-mpll.c b/drivers/clk/meson/clk-mpll.c > index 2d39a8bc367c..fc9df4860872 100644 > --- a/drivers/clk/meson/clk-mpll.c > +++ b/drivers/clk/meson/clk-mpll.c > @@ -129,7 +129,7 @@ static int mpll_set_rate(struct clk_hw *hw, > return 0; > } > > -static void mpll_init(struct clk_hw *hw) > +static int mpll_init(struct clk_hw *hw) > { > struct clk_regmap *clk = to_clk_regmap(hw); > struct meson_clk_mpll_data *mpll = meson_clk_mpll_data(clk); > @@ -151,6 +151,8 @@ static void mpll_init(struct clk_hw *hw) > /* Set the magic misc bit if required */ > if (MESON_PARM_APPLICABLE(&mpll->misc)) > meson_parm_write(clk->map, &mpll->misc, 1); > + > +return 0; > } > > const struct clk_ops meson_clk_mpll_ro_ops = { > diff --git a/drivers/clk/meson/clk-phase.c b/drivers/clk/meson/clk-phase.c > index 80c3ada193a4..fe22e171121a 100644 > --- a/drivers/clk/meson/clk-phase.c > +++ b/drivers/clk/meson/clk-phase.c > @@ -78,7 +78,7 @@ meson_clk_triphase_data(struct clk_regmap *clk) > return (struct meson_clk_triphase_data *)clk->data; > } > > -static void meson_clk_triphase_sync(struct clk_hw *hw) > +static int meson_clk_triphase_sync(struct clk_hw *hw) > { > struct clk_regmap *clk = to_clk_regmap(hw); > struct meson_clk_triphase_data *tph = meson_clk_triphase_data(clk); > @@ -88,6 +88,8 @@ static void meson_clk_triphase_sync(struct clk_hw *hw) > val = meson_parm_read(clk->map, &tph->ph0); > meson_parm_write(clk->map, &tph->ph1, val); > meson_parm_write(clk->map, &tph->ph2, val); > + > +return 0; > } > > static int meson_clk_triphase_get_phase(struct clk_hw *hw) > diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c > index ddb1e5634739..489092dde3a6 100644 > --- a/drivers/clk/meson/clk-pll.c > +++ b/drivers/clk/meson/clk-pll.c > @@ -277,7 +277,7 @@ static int meson_clk_pll_wait_lock(struct clk_hw *hw) > return -ETIMEDOUT; > } > > -static void meson_clk_pll_init(struct clk_hw *hw) > +static int meson_clk_pll_init(struct clk_hw *hw) > { > struct clk_regmap *clk = to_clk_regmap(hw); > struct meson_clk_pll_data *pll = meson_clk_pll_data(clk); > @@ -288,6 +288,8 @@ static void meson_clk_pll_init(struct clk_hw *hw) > pll->init_count); > meson_parm_write(clk->map, &pll->rst, 0); > } > + > +return 0; > } > > static int meson_clk_pll_is_enabled(struct clk_hw *hw) > diff --git a/drivers/clk/meson/sclk-div.c b/drivers/clk/meson/sclk-div.c > index 3acf03780221..76d31c0a3342 100644 > --- a/drivers/clk/meson/sclk-div.c > +++ b/drivers/clk/meson/sclk-div.c > @@ -216,7 +216,7 @@ static int sclk_div_is_enabled(struct clk_hw *hw) > return 0; > } > > -static void sclk_div_init(struct clk_hw *hw) > +static int sclk_div_init(struct clk_hw *hw) > { > struct clk_regmap *clk = to_clk_regmap(hw); > struct meson_sclk_div_data *sclk = meson_sclk_div_data(clk); > @@ -231,6 +231,8 @@ static void sclk_div_init(struct clk_hw *hw) > sclk->cached_div = val + 1; > > sclk_div_get_duty_cycle(hw, &sclk->cached_duty); > + > +return 0; > } > > const struct clk_ops meson_sclk_div_ops = { > diff --git a/drivers/clk/microchip/clk-core.c b/drivers/clk/microchip/clk-core.c > index 567755d6f844..1b4f023cdc8b 100644 > --- a/drivers/clk/microchip/clk-core.c > +++ b/drivers/clk/microchip/clk-core.c > @@ -266,10 +266,12 @@ static void roclk_disable(struct clk_hw *hw) > writel(REFO_ON | REFO_OE, PIC32_CLR(refo->ctrl_reg)); > } > > -static void roclk_init(struct clk_hw *hw) > +static int roclk_init(struct clk_hw *hw) > { > /* initialize clock in disabled state */ > roclk_disable(hw); > + > +return 0; > } > > static u8 roclk_get_parent(struct clk_hw *hw) > @@ -880,7 +882,7 @@ static int sclk_set_parent(struct clk_hw *hw, u8 index) > return err; > } > > -static void sclk_init(struct clk_hw *hw) > +static int sclk_init(struct clk_hw *hw) > { > struct pic32_sys_clk *sclk = clkhw_to_sys_clk(hw); > unsigned long flags; > @@ -899,6 +901,8 @@ static void sclk_init(struct clk_hw *hw) > writel(v, sclk->slew_reg); > spin_unlock_irqrestore(&sclk->core->reg_lock, flags); > } > + > +return 0; > } > > /* sclk with post-divider */ > diff --git a/drivers/clk/mmp/clk-frac.c b/drivers/clk/mmp/clk-frac.c > index 90bf181f191a..fabc09aca6c4 100644 > --- a/drivers/clk/mmp/clk-frac.c > +++ b/drivers/clk/mmp/clk-frac.c > @@ -109,7 +109,7 @@ static int clk_factor_set_rate(struct clk_hw *hw, > unsigned long drate, > return 0; > } > > -static void clk_factor_init(struct clk_hw *hw) > +static int clk_factor_init(struct clk_hw *hw) > { > struct mmp_clk_factor *factor = to_clk_factor(hw); > struct mmp_clk_factor_masks *masks = factor->masks; > @@ -146,6 +146,8 @@ static void clk_factor_init(struct clk_hw *hw) > > if (factor->lock) > spin_unlock_irqrestore(factor->lock, flags); > + > +return 0; > } > > static const struct clk_ops clk_factor_ops = { > diff --git a/drivers/clk/mmp/clk-mix.c b/drivers/clk/mmp/clk-mix.c > index 90814b2613c0..d2cd36c54474 100644 > --- a/drivers/clk/mmp/clk-mix.c > +++ b/drivers/clk/mmp/clk-mix.c > @@ -419,12 +419,14 @@ static int mmp_clk_set_rate(struct clk_hw *hw, > unsigned long rate, > } > } > > -static void mmp_clk_mix_init(struct clk_hw *hw) > +static int mmp_clk_mix_init(struct clk_hw *hw) > { > struct mmp_clk_mix *mix = to_clk_mix(hw); > > if (mix->table) > _filter_clk_table(mix, mix->table, mix->table_size); > + > +return 0; > } > > const struct clk_ops mmp_clk_mix_ops = { > diff --git a/drivers/clk/qcom/clk-hfpll.c b/drivers/clk/qcom/clk-hfpll.c > index 3c04805f2a55..e847d586a73a 100644 > --- a/drivers/clk/qcom/clk-hfpll.c > +++ b/drivers/clk/qcom/clk-hfpll.c > @@ -196,7 +196,7 @@ static unsigned long clk_hfpll_recalc_rate(struct clk_hw > *hw, > return l_val * parent_rate; > } > > -static void clk_hfpll_init(struct clk_hw *hw) > +static int clk_hfpll_init(struct clk_hw *hw) > { > struct clk_hfpll *h = to_clk_hfpll(hw); > struct hfpll_data const *hd = h->d; > @@ -206,7 +206,7 @@ static void clk_hfpll_init(struct clk_hw *hw) > regmap_read(regmap, hd->mode_reg, &mode); > if (mode != (PLL_BYPASSNL | PLL_RESET_N | PLL_OUTCTRL)) { > __clk_hfpll_init_once(hw); > -return; > +return 0; > } > > if (hd->status_reg) { > @@ -218,6 +218,8 @@ static void clk_hfpll_init(struct clk_hw *hw) > __clk_hfpll_init_once(hw); > } > } > + > +return 0; > } > > static int hfpll_is_enabled(struct clk_hw *hw) > diff --git a/drivers/clk/rockchip/clk-pll.c b/drivers/clk/rockchip/clk-pll.c > index 198417d56300..10560d963baf 100644 > --- a/drivers/clk/rockchip/clk-pll.c > +++ b/drivers/clk/rockchip/clk-pll.c > @@ -282,7 +282,7 @@ static int rockchip_rk3036_pll_is_enabled(struct clk_hw > *hw) > return !(pllcon & RK3036_PLLCON1_PWRDOWN); > } > > -static void rockchip_rk3036_pll_init(struct clk_hw *hw) > +static int rockchip_rk3036_pll_init(struct clk_hw *hw) > { > struct rockchip_clk_pll *pll = to_rockchip_clk_pll(hw); > const struct rockchip_pll_rate_table *rate; > @@ -290,14 +290,14 @@ static void rockchip_rk3036_pll_init(struct clk_hw > *hw) > unsigned long drate; > > if (!(pll->flags & ROCKCHIP_PLL_SYNC_RATE)) > -return; > +return 0; > > drate = clk_hw_get_rate(hw); > rate = rockchip_get_pll_settings(pll, drate); > > /* when no rate setting for the current rate, rely on clk_set_rate */ > if (!rate) > -return; > +return 0; > > rockchip_rk3036_pll_get_params(pll, &cur); > > @@ -319,13 +319,15 @@ static void rockchip_rk3036_pll_init(struct clk_hw > *hw) > if (!parent) { > pr_warn("%s: parent of %s not available\n", > __func__, __clk_get_name(hw->clk)); > -return; > +return 0; > } > > pr_debug("%s: pll %s: rate params do not match rate table, > adjusting\n", > __func__, __clk_get_name(hw->clk)); > rockchip_rk3036_pll_set_params(pll, rate); > } > + > +return 0; > } > > static const struct clk_ops rockchip_rk3036_pll_clk_norate_ops = { > @@ -515,7 +517,7 @@ static int rockchip_rk3066_pll_is_enabled(struct clk_hw > *hw) > return !(pllcon & RK3066_PLLCON3_PWRDOWN); > } > > -static void rockchip_rk3066_pll_init(struct clk_hw *hw) > +static int rockchip_rk3066_pll_init(struct clk_hw *hw) > { > struct rockchip_clk_pll *pll = to_rockchip_clk_pll(hw); > const struct rockchip_pll_rate_table *rate; > @@ -523,14 +525,14 @@ static void rockchip_rk3066_pll_init(struct clk_hw > *hw) > unsigned long drate; > > if (!(pll->flags & ROCKCHIP_PLL_SYNC_RATE)) > -return; > +return 0; > > drate = clk_hw_get_rate(hw); > rate = rockchip_get_pll_settings(pll, drate); > > /* when no rate setting for the current rate, rely on clk_set_rate */ > if (!rate) > -return; > +return 0; > > rockchip_rk3066_pll_get_params(pll, &cur); > > @@ -543,6 +545,8 @@ static void rockchip_rk3066_pll_init(struct clk_hw *hw) > __func__, clk_hw_get_name(hw)); > rockchip_rk3066_pll_set_params(pll, rate); > } > + > +return 0; > } > > static const struct clk_ops rockchip_rk3066_pll_clk_norate_ops = { > @@ -761,7 +765,7 @@ static int rockchip_rk3399_pll_is_enabled(struct clk_hw > *hw) > return !(pllcon & RK3399_PLLCON3_PWRDOWN); > } > > -static void rockchip_rk3399_pll_init(struct clk_hw *hw) > +static int rockchip_rk3399_pll_init(struct clk_hw *hw) > { > struct rockchip_clk_pll *pll = to_rockchip_clk_pll(hw); > const struct rockchip_pll_rate_table *rate; > @@ -769,14 +773,14 @@ static void rockchip_rk3399_pll_init(struct clk_hw > *hw) > unsigned long drate; > > if (!(pll->flags & ROCKCHIP_PLL_SYNC_RATE)) > -return; > +return 0; > > drate = clk_hw_get_rate(hw); > rate = rockchip_get_pll_settings(pll, drate); > > /* when no rate setting for the current rate, rely on clk_set_rate */ > if (!rate) > -return; > +return 0; > > rockchip_rk3399_pll_get_params(pll, &cur); > > @@ -798,13 +802,15 @@ static void rockchip_rk3399_pll_init(struct clk_hw > *hw) > if (!parent) { > pr_warn("%s: parent of %s not available\n", > __func__, __clk_get_name(hw->clk)); > -return; > +return 0; > } > > pr_debug("%s: pll %s: rate params do not match rate table, > adjusting\n", > __func__, __clk_get_name(hw->clk)); > rockchip_rk3399_pll_set_params(pll, rate); > } > + > +return 0; > } > > static const struct clk_ops rockchip_rk3399_pll_clk_norate_ops = { > diff --git a/drivers/clk/ti/clock.h b/drivers/clk/ti/clock.h > index e4b8392ff63c..adef9c16e43b 100644 > --- a/drivers/clk/ti/clock.h > +++ b/drivers/clk/ti/clock.h > @@ -252,7 +252,7 @@ extern const struct clk_ops omap_gate_clk_ops; > > extern struct ti_clk_features ti_clk_features; > > -void omap2_init_clk_clkdm(struct clk_hw *hw); > +int omap2_init_clk_clkdm(struct clk_hw *hw); > int omap2_clkops_enable_clkdm(struct clk_hw *hw); > void omap2_clkops_disable_clkdm(struct clk_hw *hw); > > diff --git a/drivers/clk/ti/clockdomain.c b/drivers/clk/ti/clockdomain.c > index 423a99b9f10c..ee56306f79d5 100644 > --- a/drivers/clk/ti/clockdomain.c > +++ b/drivers/clk/ti/clockdomain.c > @@ -101,16 +101,16 @@ void omap2_clkops_disable_clkdm(struct clk_hw > *hw) > * > * Convert a clockdomain name stored in a struct clk 'clk' into a > * clockdomain pointer, and save it into the struct clk. Intended to be > - * called during clk_register(). No return value. > + * called during clk_register(). Returns 0 on success, -EERROR otherwise. > */ > -void omap2_init_clk_clkdm(struct clk_hw *hw) > +int omap2_init_clk_clkdm(struct clk_hw *hw) > { > struct clk_hw_omap *clk = to_clk_hw_omap(hw); > struct clockdomain *clkdm; > const char *clk_name; > > if (!clk->clkdm_name) > -return; > +return 0; > > clk_name = __clk_get_name(hw->clk); > > @@ -123,6 +123,8 @@ void omap2_init_clk_clkdm(struct clk_hw *hw) > pr_debug("clock: could not associate clk %s to clkdm %s\n", > clk_name, clk->clkdm_name); > } > + > +return 0; > } Where is it returning -EERROR as mentioned in the description? > > static void __init of_ti_clockdomain_setup(struct device_node *node) > diff --git a/drivers/net/phy/mdio-mux-meson-g12a.c b/drivers/net/phy/mdio- > mux-meson-g12a.c > index 7a9ad54582e1..bf86c9c7a288 100644 > --- a/drivers/net/phy/mdio-mux-meson-g12a.c > +++ b/drivers/net/phy/mdio-mux-meson-g12a.c > @@ -123,7 +123,7 @@ static int g12a_ephy_pll_is_enabled(struct clk_hw *hw) > return (val & PLL_CTL0_LOCK_DIG) ? 1 : 0; > } > > -static void g12a_ephy_pll_init(struct clk_hw *hw) > +static int g12a_ephy_pll_init(struct clk_hw *hw) > { > struct g12a_ephy_pll *pll = g12a_ephy_pll_to_dev(hw); > > @@ -136,6 +136,8 @@ static void g12a_ephy_pll_init(struct clk_hw *hw) > writel(0x20200000, pll->base + ETH_PLL_CTL5); > writel(0x0000c002, pll->base + ETH_PLL_CTL6); > writel(0x00000023, pll->base + ETH_PLL_CTL7); > + > +return 0; > } > > static const struct clk_ops g12a_ephy_pll_ops = { > diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h > index 2fdfe8061363..b82ec4c4ca95 100644 > --- a/include/linux/clk-provider.h > +++ b/include/linux/clk-provider.h > @@ -190,8 +190,12 @@ struct clk_duty { > * > * @init:Perform platform-specific initialization magic. > *This is not not used by any of the basic clock types. > - *Please consider other ways of solving initialization problems > - *before using this callback, as its use is discouraged. > + *This callback exist for HW which needs to perform some > + *initialisation magic for CCF to get an accurate view of the > + *clock. It may also be used dynamic resource allocation is > + *required. It shall not used to deal with clock parameters, > + *such as rate or parents. > + *Returns 0 on success, -EERROR otherwise. Aren't all functions returning 0 always? > * > * @debug_init:Set up type-specific debugfs entries for this clock. This > *is called once, after the debugfs directory entry for this > @@ -243,7 +247,7 @@ struct clk_ops { > struct clk_duty *duty); > int(*set_duty_cycle)(struct clk_hw *hw, > struct clk_duty *duty); > -void(*init)(struct clk_hw *hw); > +int(*init)(struct clk_hw *hw); > void(*debug_init)(struct clk_hw *hw, struct dentry *dentry); > }; > > -- > 2.21.0 ________________________________ This email is confidential and may contain information subject to legal privilege. If you are not the intended recipient please advise us of our error by return e-mail then delete this email and any attached files. You may not copy, disclose or use the contents in any way. The views expressed in this email may not be those of Gallagher Group Ltd or subsidiary companies thereof. ________________________________ ^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH 2/3] clk: let init callback return an error code 2019-09-24 13:38 ` Ankur Tyagi @ 2019-09-24 14:10 ` Jerome Brunet 0 siblings, 0 replies; 14+ messages in thread From: Jerome Brunet @ 2019-09-24 14:10 UTC (permalink / raw) To: Ankur Tyagi, Michael Turquette, Stephen Boyd Cc: linux-clk, linux-kernel, Heiko Stuebner, Tero Kristo, Andrew Lunn, Florian Fainelli, Heiner Kallweit, David S. Miller, netdev, linux-amlogic, linux-arm-msm, linux-rockchip, linux-omap On Tue 24 Sep 2019 at 13:38, Ankur Tyagi <Ankur.Tyagi@gallagher.com> wrote: > Hi, > > I am no expert here but just looked at the patch and found few > discrepancy that I have mentioned inline. > [...] > > Aren't all functions returning 0 always? > Yes, on purpose. This patch is an API conversion to let the init() callback of the clock ops return an error code or 0. The patch is not meant to change anything in the prior behavior of the clock drivers which is why every exit path return 0 with this change. IOW, yes there are all returning 0 for now, but it will eventually change. >> * >> * @debug_init:Set up type-specific debugfs entries for this clock. This >> *is called once, after the debugfs directory entry for this >> @@ -243,7 +247,7 @@ struct clk_ops { >> struct clk_duty *duty); >> int(*set_duty_cycle)(struct clk_hw *hw, >> struct clk_duty *duty); >> -void(*init)(struct clk_hw *hw); >> +int(*init)(struct clk_hw *hw); >> void(*debug_init)(struct clk_hw *hw, struct dentry *dentry); >> }; >> >> -- >> 2.21.0 > > ________________________________ > This email is confidential and may contain information subject to legal privilege. If you are not the intended recipient please advise us of our error by return e-mail then delete this email and any attached files. You may not copy, disclose or use the contents in any way. The views expressed in this email may not be those of Gallagher Group Ltd or subsidiary companies thereof. > ________________________________ ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] clk: let init callback return an error code 2019-09-24 12:39 ` [PATCH 2/3] clk: let init callback return an error code Jerome Brunet 2019-09-24 13:38 ` Ankur Tyagi @ 2019-09-24 16:19 ` Andrew Lunn 2019-09-29 19:33 ` Heiko Stuebner 2 siblings, 0 replies; 14+ messages in thread From: Andrew Lunn @ 2019-09-24 16:19 UTC (permalink / raw) To: Jerome Brunet Cc: Michael Turquette, Stephen Boyd, linux-clk, linux-kernel, Heiko Stuebner, Tero Kristo, Florian Fainelli, Heiner Kallweit, David S. Miller, netdev, linux-amlogic, linux-arm-msm, linux-rockchip, linux-omap On Tue, Sep 24, 2019 at 02:39:53PM +0200, Jerome Brunet wrote: > If the init callback is allowed to request resources, it needs a return > value to report the outcome of such a request. > > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> > --- > > Sorry about the spam. > This patch change quite a few files so I have tried to Cc the > relevant people. Stephen, You may notice that I have added a > couple of the network people. You need an Ack from one of them > since the Amlogic G12a mdio mux has a clock which uses the .init() > callback > static void __init of_ti_clockdomain_setup(struct device_node *node) > diff --git a/drivers/net/phy/mdio-mux-meson-g12a.c b/drivers/net/phy/mdio-mux-meson-g12a.c > index 7a9ad54582e1..bf86c9c7a288 100644 > --- a/drivers/net/phy/mdio-mux-meson-g12a.c > +++ b/drivers/net/phy/mdio-mux-meson-g12a.c > @@ -123,7 +123,7 @@ static int g12a_ephy_pll_is_enabled(struct clk_hw *hw) > return (val & PLL_CTL0_LOCK_DIG) ? 1 : 0; > } > > -static void g12a_ephy_pll_init(struct clk_hw *hw) > +static int g12a_ephy_pll_init(struct clk_hw *hw) > { > struct g12a_ephy_pll *pll = g12a_ephy_pll_to_dev(hw); > > @@ -136,6 +136,8 @@ static void g12a_ephy_pll_init(struct clk_hw *hw) > writel(0x20200000, pll->base + ETH_PLL_CTL5); > writel(0x0000c002, pll->base + ETH_PLL_CTL6); > writel(0x00000023, pll->base + ETH_PLL_CTL7); > + > + return 0; > } Reviewed-by: Andrew Lunn <andrew@lunn.ch> It should be safe to merge this via the clk tree. You would probably know about an possible merge conflicts, since you wrote this driver! Andrew ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] clk: let init callback return an error code 2019-09-24 12:39 ` [PATCH 2/3] clk: let init callback return an error code Jerome Brunet 2019-09-24 13:38 ` Ankur Tyagi 2019-09-24 16:19 ` Andrew Lunn @ 2019-09-29 19:33 ` Heiko Stuebner 2 siblings, 0 replies; 14+ messages in thread From: Heiko Stuebner @ 2019-09-29 19:33 UTC (permalink / raw) To: Jerome Brunet Cc: Michael Turquette, Stephen Boyd, linux-clk, linux-kernel, Tero Kristo, Andrew Lunn, Florian Fainelli, Heiner Kallweit, David S. Miller, netdev, linux-amlogic, linux-arm-msm, linux-rockchip, linux-omap Am Dienstag, 24. September 2019, 14:39:53 CEST schrieb Jerome Brunet: > If the init callback is allowed to request resources, it needs a return > value to report the outcome of such a request. > > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> > --- [...] > drivers/clk/rockchip/clk-pll.c | 28 ++++++++++++++++----------- for the Rockchip part Acked-by: Heiko Stuebner <heiko@sntech.de> ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/3] clk: add terminate callback to clk_ops 2019-09-24 12:39 [PATCH 0/3] clk: let clock perform allocation in init Jerome Brunet 2019-09-24 12:39 ` [PATCH 1/3] clk: actually call the clock init before any other callback of the clock Jerome Brunet 2019-09-24 12:39 ` [PATCH 2/3] clk: let init callback return an error code Jerome Brunet @ 2019-09-24 12:39 ` Jerome Brunet 2019-12-24 2:59 ` Stephen Boyd 2019-11-18 10:08 ` [PATCH 0/3] clk: let clock perform allocation in init Jerome Brunet 2019-11-29 15:36 ` Jerome Brunet 4 siblings, 1 reply; 14+ messages in thread From: Jerome Brunet @ 2019-09-24 12:39 UTC (permalink / raw) To: Michael Turquette, Stephen Boyd; +Cc: Jerome Brunet, linux-clk, linux-kernel Add a terminate callback to the clk_ops to release the resources claimed in .init() Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> --- drivers/clk/clk.c | 7 ++++++- include/linux/clk-provider.h | 3 +++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index df853a98fad2..926f49c78b5d 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -3844,6 +3844,7 @@ static void clk_core_evict_parent_cache(struct clk_core *core) void clk_unregister(struct clk *clk) { unsigned long flags; + const struct clk_ops *ops; if (!clk || WARN_ON_ONCE(IS_ERR(clk))) return; @@ -3852,7 +3853,8 @@ void clk_unregister(struct clk *clk) clk_prepare_lock(); - if (clk->core->ops == &clk_nodrv_ops) { + ops = clk->core->ops; + if (ops == &clk_nodrv_ops) { pr_err("%s: unregistered clock: %s\n", __func__, clk->core->name); goto unlock; @@ -3865,6 +3867,9 @@ void clk_unregister(struct clk *clk) clk->core->ops = &clk_nodrv_ops; clk_enable_unlock(flags); + if (ops->terminate) + ops->terminate(clk->core->hw); + if (!hlist_empty(&clk->core->children)) { struct clk_core *child; struct hlist_node *t; diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h index b82ec4c4ca95..5a5a64785923 100644 --- a/include/linux/clk-provider.h +++ b/include/linux/clk-provider.h @@ -197,6 +197,8 @@ struct clk_duty { * such as rate or parents. * Returns 0 on success, -EERROR otherwise. * + * @terminate: Free any resource allocated by init. + * * @debug_init: Set up type-specific debugfs entries for this clock. This * is called once, after the debugfs directory entry for this * clock has been created. The dentry pointer representing that @@ -248,6 +250,7 @@ struct clk_ops { int (*set_duty_cycle)(struct clk_hw *hw, struct clk_duty *duty); int (*init)(struct clk_hw *hw); + void (*terminate)(struct clk_hw *hw); void (*debug_init)(struct clk_hw *hw, struct dentry *dentry); }; -- 2.21.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] clk: add terminate callback to clk_ops 2019-09-24 12:39 ` [PATCH 3/3] clk: add terminate callback to clk_ops Jerome Brunet @ 2019-12-24 2:59 ` Stephen Boyd 0 siblings, 0 replies; 14+ messages in thread From: Stephen Boyd @ 2019-12-24 2:59 UTC (permalink / raw) To: Jerome Brunet, Michael Turquette; +Cc: Jerome Brunet, linux-clk, linux-kernel Quoting Jerome Brunet (2019-09-24 05:39:54) > Add a terminate callback to the clk_ops to release the resources > claimed in .init() > > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com> > --- Applied to clk-next ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/3] clk: let clock perform allocation in init 2019-09-24 12:39 [PATCH 0/3] clk: let clock perform allocation in init Jerome Brunet ` (2 preceding siblings ...) 2019-09-24 12:39 ` [PATCH 3/3] clk: add terminate callback to clk_ops Jerome Brunet @ 2019-11-18 10:08 ` Jerome Brunet 2019-11-29 15:36 ` Jerome Brunet 4 siblings, 0 replies; 14+ messages in thread From: Jerome Brunet @ 2019-11-18 10:08 UTC (permalink / raw) To: Michael Turquette, Stephen Boyd; +Cc: linux-clk, linux-kernel On Tue 24 Sep 2019 at 14:39, Jerome Brunet <jbrunet@baylibre.com> wrote: > This patchset is a follow up on this pinky swear [0]. > Its purpose is: > * Clarify the acceptable use of clk_ops init() callback > * Let the init() callback return an error code in case anything > fail. > * Add the terminate() counter part of of init() to release the > resources which may have been claimed in init() > > After discussing with Stephen at LPC, I decided to drop the 2 last patches > of the RFC [1]. I can live without it for now and nobody expressed a > critical need to get the proposed placeholder. > > [0]: https://lkml.kernel.org/r/CAEG3pNB-143Pr_xCTPj=tURhpiTiJqi61xfDGDVdU7zG5H-2tA@mail.gmail.com > [1]: https://lkml.kernel.org/r/20190828102012.4493-1-jbrunet@baylibre.com > > Jerome Brunet (3): > clk: actually call the clock init before any other callback of the > clock > clk: let init callback return an error code > clk: add terminate callback to clk_ops > > drivers/clk/clk.c | 38 ++++++++++++++++++--------- > drivers/clk/meson/clk-mpll.c | 4 ++- > drivers/clk/meson/clk-phase.c | 4 ++- > drivers/clk/meson/clk-pll.c | 4 ++- > drivers/clk/meson/sclk-div.c | 4 ++- > drivers/clk/microchip/clk-core.c | 8 ++++-- > drivers/clk/mmp/clk-frac.c | 4 ++- > drivers/clk/mmp/clk-mix.c | 4 ++- > drivers/clk/qcom/clk-hfpll.c | 6 +++-- > drivers/clk/rockchip/clk-pll.c | 28 ++++++++++++-------- > drivers/clk/ti/clock.h | 2 +- > drivers/clk/ti/clockdomain.c | 8 +++--- > drivers/net/phy/mdio-mux-meson-g12a.c | 4 ++- > include/linux/clk-provider.h | 13 ++++++--- > 14 files changed, 90 insertions(+), 41 deletions(-) Hi Stephen, Is this series Ok with you ? Do you think you can take it at the beginning of the next cycle ? Thx Jerome ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/3] clk: let clock perform allocation in init 2019-09-24 12:39 [PATCH 0/3] clk: let clock perform allocation in init Jerome Brunet ` (3 preceding siblings ...) 2019-11-18 10:08 ` [PATCH 0/3] clk: let clock perform allocation in init Jerome Brunet @ 2019-11-29 15:36 ` Jerome Brunet 2019-12-03 9:05 ` Stephen Boyd 4 siblings, 1 reply; 14+ messages in thread From: Jerome Brunet @ 2019-11-29 15:36 UTC (permalink / raw) To: Michael Turquette, Stephen Boyd; +Cc: linux-clk, linux-kernel On Tue 24 Sep 2019 at 14:39, Jerome Brunet <jbrunet@baylibre.com> wrote: > This patchset is a follow up on this pinky swear [0]. > Its purpose is: > * Clarify the acceptable use of clk_ops init() callback > * Let the init() callback return an error code in case anything > fail. > * Add the terminate() counter part of of init() to release the > resources which may have been claimed in init() > > After discussing with Stephen at LPC, I decided to drop the 2 last patches > of the RFC [1]. I can live without it for now and nobody expressed a > critical need to get the proposed placeholder. > > [0]: https://lkml.kernel.org/r/CAEG3pNB-143Pr_xCTPj=tURhpiTiJqi61xfDGDVdU7zG5H-2tA@mail.gmail.com > [1]: https://lkml.kernel.org/r/20190828102012.4493-1-jbrunet@baylibre.com > Hi Stephen, Do you think we can fit this into the incoming cycle ? Cheers Jerome > Jerome Brunet (3): > clk: actually call the clock init before any other callback of the > clock > clk: let init callback return an error code > clk: add terminate callback to clk_ops > > drivers/clk/clk.c | 38 ++++++++++++++++++--------- > drivers/clk/meson/clk-mpll.c | 4 ++- > drivers/clk/meson/clk-phase.c | 4 ++- > drivers/clk/meson/clk-pll.c | 4 ++- > drivers/clk/meson/sclk-div.c | 4 ++- > drivers/clk/microchip/clk-core.c | 8 ++++-- > drivers/clk/mmp/clk-frac.c | 4 ++- > drivers/clk/mmp/clk-mix.c | 4 ++- > drivers/clk/qcom/clk-hfpll.c | 6 +++-- > drivers/clk/rockchip/clk-pll.c | 28 ++++++++++++-------- > drivers/clk/ti/clock.h | 2 +- > drivers/clk/ti/clockdomain.c | 8 +++--- > drivers/net/phy/mdio-mux-meson-g12a.c | 4 ++- > include/linux/clk-provider.h | 13 ++++++--- > 14 files changed, 90 insertions(+), 41 deletions(-) ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/3] clk: let clock perform allocation in init 2019-11-29 15:36 ` Jerome Brunet @ 2019-12-03 9:05 ` Stephen Boyd 2019-12-16 9:17 ` Jerome Brunet 0 siblings, 1 reply; 14+ messages in thread From: Stephen Boyd @ 2019-12-03 9:05 UTC (permalink / raw) To: Jerome Brunet, Michael Turquette; +Cc: linux-clk, linux-kernel Quoting Jerome Brunet (2019-11-29 07:36:28) > > On Tue 24 Sep 2019 at 14:39, Jerome Brunet <jbrunet@baylibre.com> wrote: > > > This patchset is a follow up on this pinky swear [0]. > > Its purpose is: > > * Clarify the acceptable use of clk_ops init() callback > > * Let the init() callback return an error code in case anything > > fail. > > * Add the terminate() counter part of of init() to release the > > resources which may have been claimed in init() > > > > After discussing with Stephen at LPC, I decided to drop the 2 last patches > > of the RFC [1]. I can live without it for now and nobody expressed a > > critical need to get the proposed placeholder. > > > > [0]: https://lkml.kernel.org/r/CAEG3pNB-143Pr_xCTPj=tURhpiTiJqi61xfDGDVdU7zG5H-2tA@mail.gmail.com > > [1]: https://lkml.kernel.org/r/20190828102012.4493-1-jbrunet@baylibre.com > > > > Hi Stephen, > > Do you think we can fit this into the incoming cycle ? > Sorry I missed this one. I'll apply it soon but won't be for this merge window. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/3] clk: let clock perform allocation in init 2019-12-03 9:05 ` Stephen Boyd @ 2019-12-16 9:17 ` Jerome Brunet 0 siblings, 0 replies; 14+ messages in thread From: Jerome Brunet @ 2019-12-16 9:17 UTC (permalink / raw) To: Stephen Boyd, Michael Turquette; +Cc: linux-clk, linux-kernel On Tue 03 Dec 2019 at 10:05, Stephen Boyd <sboyd@kernel.org> wrote: > Quoting Jerome Brunet (2019-11-29 07:36:28) >> >> On Tue 24 Sep 2019 at 14:39, Jerome Brunet <jbrunet@baylibre.com> wrote: >> >> > This patchset is a follow up on this pinky swear [0]. >> > Its purpose is: >> > * Clarify the acceptable use of clk_ops init() callback >> > * Let the init() callback return an error code in case anything >> > fail. >> > * Add the terminate() counter part of of init() to release the >> > resources which may have been claimed in init() >> > >> > After discussing with Stephen at LPC, I decided to drop the 2 last patches >> > of the RFC [1]. I can live without it for now and nobody expressed a >> > critical need to get the proposed placeholder. >> > >> > [0]: https://lkml.kernel.org/r/CAEG3pNB-143Pr_xCTPj=tURhpiTiJqi61xfDGDVdU7zG5H-2tA@mail.gmail.com >> > [1]: https://lkml.kernel.org/r/20190828102012.4493-1-jbrunet@baylibre.com >> > >> >> Hi Stephen, >> >> Do you think we can fit this into the incoming cycle ? >> > > Sorry I missed this one. I'll apply it soon but won't be for this merge > window. No worries, I was referring to the v5.6 cycle, not the v5.5 merge window. ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2019-12-24 2:59 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-09-24 12:39 [PATCH 0/3] clk: let clock perform allocation in init Jerome Brunet 2019-09-24 12:39 ` [PATCH 1/3] clk: actually call the clock init before any other callback of the clock Jerome Brunet 2019-12-24 2:59 ` Stephen Boyd 2019-09-24 12:39 ` [PATCH 2/3] clk: let init callback return an error code Jerome Brunet 2019-09-24 13:38 ` Ankur Tyagi 2019-09-24 14:10 ` Jerome Brunet 2019-09-24 16:19 ` Andrew Lunn 2019-09-29 19:33 ` Heiko Stuebner 2019-09-24 12:39 ` [PATCH 3/3] clk: add terminate callback to clk_ops Jerome Brunet 2019-12-24 2:59 ` Stephen Boyd 2019-11-18 10:08 ` [PATCH 0/3] clk: let clock perform allocation in init Jerome Brunet 2019-11-29 15:36 ` Jerome Brunet 2019-12-03 9:05 ` Stephen Boyd 2019-12-16 9:17 ` Jerome Brunet
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).