| Message ID | 1475138152-859-11-git-send-email-rnayak@codeaurora.org (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | Stephen Boyd |
| Headers | show |
On 09/29, Rajendra Nayak wrote: > Add a helper API that will allow clk providers to turn their clk_hw > structures into struct clk pointer. > Did I suggest this? > Signed-off-by: Rajendra Nayak <rnayak@codeaurora.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 820a939..a084132 100644 > --- a/drivers/clk/clk.c > +++ b/drivers/clk/clk.c > @@ -186,6 +186,12 @@ const char *clk_hw_get_name(const struct clk_hw *hw) > } > EXPORT_SYMBOL_GPL(clk_hw_get_name); > > +struct clk *clk_hw_get_clk(const struct clk_hw *hw) > +{ > + return hw->clk; I'd prefer we actually went through all the work and actually allocated another clk structure here. We can add another string or two for the dev_id and con_id too for debug/accouting purposes. The hw->clk is sort of a remnant of the clk_core introduction. I can't recall the exact plan (i.e. I should write it down somewhere once I do) but I think we want to get rid of hw->clk and have everyone use clk_hw_get_clk() instead. Then for traversals from the clk_hw pointer to a clk pointer are very clear.
On 11/03/2016 03:52 AM, Stephen Boyd wrote: > On 09/29, Rajendra Nayak wrote: >> Add a helper API that will allow clk providers to turn their clk_hw >> structures into struct clk pointer. >> > > Did I suggest this? I did this back when you suggested we store clk_hw's for all clocks associated with a gdsc instead of extracting them from the clients device tree node. > >> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.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 820a939..a084132 100644 >> --- a/drivers/clk/clk.c >> +++ b/drivers/clk/clk.c >> @@ -186,6 +186,12 @@ const char *clk_hw_get_name(const struct clk_hw *hw) >> } >> EXPORT_SYMBOL_GPL(clk_hw_get_name); >> >> +struct clk *clk_hw_get_clk(const struct clk_hw *hw) >> +{ >> + return hw->clk; > > I'd prefer we actually went through all the work and actually > allocated another clk structure here. We can add another string > or two for the dev_id and con_id too for debug/accouting > purposes. I don't quite get the part of allocating another clk structure, can you please elaborate? > > The hw->clk is sort of a remnant of the clk_core introduction. I > can't recall the exact plan (i.e. I should write it down > somewhere once I do) but I think we want to get rid of hw->clk > and have everyone use clk_hw_get_clk() instead. Then for > traversals from the clk_hw pointer to a clk pointer are very > clear. >
On 11/03, Rajendra Nayak wrote: > > > On 11/03/2016 03:52 AM, Stephen Boyd wrote: > > On 09/29, Rajendra Nayak wrote: > >> Add a helper API that will allow clk providers to turn their clk_hw > >> structures into struct clk pointer. > >> > > > > Did I suggest this? > > I did this back when you suggested we store clk_hw's for all > clocks associated with a gdsc instead of extracting them from > the clients device tree node. > > > > >> Signed-off-by: Rajendra Nayak <rnayak@codeaurora.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 820a939..a084132 100644 > >> --- a/drivers/clk/clk.c > >> +++ b/drivers/clk/clk.c > >> @@ -186,6 +186,12 @@ const char *clk_hw_get_name(const struct clk_hw *hw) > >> } > >> EXPORT_SYMBOL_GPL(clk_hw_get_name); > >> > >> +struct clk *clk_hw_get_clk(const struct clk_hw *hw) > >> +{ > >> + return hw->clk; > > > > I'd prefer we actually went through all the work and actually > > allocated another clk structure here. We can add another string > > or two for the dev_id and con_id too for debug/accouting > > purposes. > > I don't quite get the part of allocating another clk structure, > can you please elaborate? > With "per-user" clks we want to have accounting for all the struct clk consumers of a particular clk_hw instance. Given that this function lets someone get a clk pointer from a clk_hw structure, we want to make it follow the same "per-user" accounting that we already have for each time the call is made. Also, we want to make sure callers call clk_put() on the clk when they're done with it. If we don't allocate a new one here that clk_put() will destroy the one we allocate and assign during registration time. So I'm saying we want a wrapper to __clk_create_clk() for clk providers to use to generate a clk pointer if needed.
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index 820a939..a084132 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -186,6 +186,12 @@ const char *clk_hw_get_name(const struct clk_hw *hw) } EXPORT_SYMBOL_GPL(clk_hw_get_name); +struct clk *clk_hw_get_clk(const struct clk_hw *hw) +{ + return hw->clk; +} +EXPORT_SYMBOL_GPL(clk_hw_get_clk); + struct clk_hw *__clk_get_hw(struct clk *clk) { return !clk ? NULL : clk->core->hw; diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h index a39c0c5..77d05f1 100644 --- a/include/linux/clk-provider.h +++ b/include/linux/clk-provider.h @@ -729,6 +729,7 @@ void devm_clk_hw_unregister(struct device *dev, struct clk_hw *hw); /* helper functions */ const char *__clk_get_name(const struct clk *clk); const char *clk_hw_get_name(const struct clk_hw *hw); +struct clk *clk_hw_get_clk(const struct clk_hw *hw); struct clk_hw *__clk_get_hw(struct clk *clk); unsigned int clk_hw_get_num_parents(const struct clk_hw *hw); struct clk_hw *clk_hw_get_parent(const struct clk_hw *hw);
Add a helper API that will allow clk providers to turn their clk_hw structures into struct clk pointer. Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org> --- drivers/clk/clk.c | 6 ++++++ include/linux/clk-provider.h | 1 + 2 files changed, 7 insertions(+)