| Message ID | 20190731084019.8451-2-narmstrong@baylibre.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | clk: meson: g12a: add support for DVFS | expand |
On Wed 31 Jul 2019 at 10:40, Neil Armstrong <narmstrong@baylibre.com> wrote: > Introduce the clk_hw_set_parent() provider call to change parent of > a clock by using the clk_hw pointers. > > This eases the clock reparenting from clock rate notifiers and > implementing DVFS with simpler code avoiding the boilerplates > functions as __clk_lookup(clk_hw_get_name()) then clk_set_parent(). > > Signed-off-by: Neil Armstrong <narmstrong@baylibre.com> > Acked-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> Looks ok to me but we will obviously need Stephen's ack to apply it > --- > drivers/clk/clk.c | 6 ++++++ > include/linux/clk-provider.h | 1 + > 2 files changed, 7 insertions(+) > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > index c0990703ce54..c11b1781d24a 100644 > --- a/drivers/clk/clk.c > +++ b/drivers/clk/clk.c > @@ -2487,6 +2487,12 @@ static int clk_core_set_parent_nolock(struct clk_core *core, > return ret; > } > > +int clk_hw_set_parent(struct clk_hw *hw, struct clk_hw *parent) > +{ > + return clk_core_set_parent_nolock(hw->core, parent->core); > +} > +EXPORT_SYMBOL_GPL(clk_hw_set_parent); > + > /** > * clk_set_parent - switch the parent of a mux clk > * @clk: the mux clk whose input we are switching > diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h > index 2ae7604783dd..dce5521a9bf6 100644 > --- a/include/linux/clk-provider.h > +++ b/include/linux/clk-provider.h > @@ -817,6 +817,7 @@ unsigned int clk_hw_get_num_parents(const struct clk_hw *hw); > struct clk_hw *clk_hw_get_parent(const struct clk_hw *hw); > struct clk_hw *clk_hw_get_parent_by_index(const struct clk_hw *hw, > unsigned int index); > +int clk_hw_set_parent(struct clk_hw *hw, struct clk_hw *new_parent); > unsigned int __clk_get_enable_count(struct clk *clk); > unsigned long clk_hw_get_rate(const struct clk_hw *hw); > unsigned long __clk_get_flags(struct clk *clk); > -- > 2.22.0
Quoting Jerome Brunet (2019-08-06 01:28:19) > On Wed 31 Jul 2019 at 10:40, Neil Armstrong <narmstrong@baylibre.com> wrote: > > > Introduce the clk_hw_set_parent() provider call to change parent of > > a clock by using the clk_hw pointers. > > > > This eases the clock reparenting from clock rate notifiers and > > implementing DVFS with simpler code avoiding the boilerplates > > functions as __clk_lookup(clk_hw_get_name()) then clk_set_parent(). > > > > Signed-off-by: Neil Armstrong <narmstrong@baylibre.com> > > Acked-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> > > Looks ok to me but we will obviously need Stephen's ack to apply it Acked-by: Stephen Boyd <sboyd@kernel.org> > > > --- > > drivers/clk/clk.c | 6 ++++++ > > include/linux/clk-provider.h | 1 + > > 2 files changed, 7 insertions(+) > > > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > > index c0990703ce54..c11b1781d24a 100644 > > --- a/drivers/clk/clk.c > > +++ b/drivers/clk/clk.c > > @@ -2487,6 +2487,12 @@ static int clk_core_set_parent_nolock(struct clk_core *core, > > return ret; > > } > > > > +int clk_hw_set_parent(struct clk_hw *hw, struct clk_hw *parent) > > +{ > > + return clk_core_set_parent_nolock(hw->core, parent->core); I wonder if you really want to call all those things in clk_core_set_parent_nolock(). For example, do you want notifiers to run again and for rates to be speculated? Maybe all you want to do is overwrite some value for the clk's parent by calling into the ops for the clk and generically parse the value that's passed as the "parent" here. I ask because it may be good to avoid doing all that work and updating bookkeeping when we're deep in a notifier. If we can clearly understand what the driver wants to do from the notifier then it's simpler to change in the future to use things such as the coordinated clk rate vaporware.
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index c0990703ce54..c11b1781d24a 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -2487,6 +2487,12 @@ static int clk_core_set_parent_nolock(struct clk_core *core, return ret; } +int clk_hw_set_parent(struct clk_hw *hw, struct clk_hw *parent) +{ + return clk_core_set_parent_nolock(hw->core, parent->core); +} +EXPORT_SYMBOL_GPL(clk_hw_set_parent); + /** * clk_set_parent - switch the parent of a mux clk * @clk: the mux clk whose input we are switching diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h index 2ae7604783dd..dce5521a9bf6 100644 --- a/include/linux/clk-provider.h +++ b/include/linux/clk-provider.h @@ -817,6 +817,7 @@ unsigned int clk_hw_get_num_parents(const struct clk_hw *hw); struct clk_hw *clk_hw_get_parent(const struct clk_hw *hw); struct clk_hw *clk_hw_get_parent_by_index(const struct clk_hw *hw, unsigned int index); +int clk_hw_set_parent(struct clk_hw *hw, struct clk_hw *new_parent); unsigned int __clk_get_enable_count(struct clk *clk); unsigned long clk_hw_get_rate(const struct clk_hw *hw); unsigned long __clk_get_flags(struct clk *clk);