linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] clk: add api to get clk consummer from clk_hw
@ 2020-05-19 17:04 Jerome Brunet
  2020-05-27  9:17 ` Stephen Boyd
  2020-05-27 20:07 ` Martin Blumenstingl
  0 siblings, 2 replies; 6+ messages in thread
From: Jerome Brunet @ 2020-05-19 17:04 UTC (permalink / raw)
  To: Stephen Boyd, Martin Blumenstingl; +Cc: Jerome Brunet, linux-clk, linux-kernel

clk_register() is deprecated. Using 'clk' member of struct clk_hw is
discouraged. With this constrainst, it is difficult for driver to
register clocks using the clk_hw API and then use the clock with
the consummer API

This add a simple helper, clk_hw_get_clk(), to get a struct clk from
a struct clk_hw. Like other clk_get() variant, each call to this helper
must be balanced with a call to clk_put().

Cc: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/clk/clk.c            | 17 +++++++++++++++++
 include/linux/clk-provider.h |  1 +
 2 files changed, 18 insertions(+)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 6fd23ce3cb03..d9946e192cbc 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -3625,6 +3625,23 @@ struct clk *clk_hw_create_clk(struct device *dev, struct clk_hw *hw,
 	return clk;
 }
 
+/**
+ * clk_hw_get_clk: get clk consummer given an clk_hw
+ * @hw: clk_hw associated with the clk being consumed
+ *
+ * Returns: new clk consummer
+ * This is the function to be used by providers which need
+ * to get a consummer clk and act on the clock element
+ * Calls to this function must be balanced with calls clk_put()
+ */
+struct clk *clk_hw_get_clk(struct clk_hw *hw)
+{
+	struct device *dev = hw->core->dev;
+
+	return clk_hw_create_clk(dev, hw, dev_name(dev), NULL);
+}
+EXPORT_SYMBOL(clk_hw_get_clk);
+
 static int clk_cpy_name(const char **dst_p, const char *src, bool must_exist)
 {
 	const char *dst;
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index bd1ee9039558..a8f466bdda1a 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -1088,6 +1088,7 @@ static inline struct clk_hw *__clk_get_hw(struct clk *clk)
 	return (struct clk_hw *)clk;
 }
 #endif
+struct clk *clk_hw_get_clk(struct clk_hw *hw);
 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,
-- 
2.25.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] clk: add api to get clk consummer from clk_hw
  2020-05-19 17:04 [PATCH] clk: add api to get clk consummer from clk_hw Jerome Brunet
@ 2020-05-27  9:17 ` Stephen Boyd
  2020-05-28 18:50   ` Jerome Brunet
  2020-05-27 20:07 ` Martin Blumenstingl
  1 sibling, 1 reply; 6+ messages in thread
From: Stephen Boyd @ 2020-05-27  9:17 UTC (permalink / raw)
  To: Jerome Brunet, Martin Blumenstingl; +Cc: Jerome Brunet, linux-clk, linux-kernel

Quoting Jerome Brunet (2020-05-19 10:04:40)
> clk_register() is deprecated. Using 'clk' member of struct clk_hw is
> discouraged. With this constrainst, it is difficult for driver to

s/constrainst/constraint/

> register clocks using the clk_hw API and then use the clock with
> the consummer API

s/consummer/consumer/

> 
> This add a simple helper, clk_hw_get_clk(), to get a struct clk from
> a struct clk_hw. Like other clk_get() variant, each call to this helper
> must be balanced with a call to clk_put().
> 
> Cc: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>

I like it!

> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 6fd23ce3cb03..d9946e192cbc 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -3625,6 +3625,23 @@ struct clk *clk_hw_create_clk(struct device *dev, struct clk_hw *hw,
>         return clk;
>  }
>  
> +/**
> + * clk_hw_get_clk: get clk consummer given an clk_hw

s/consummer/consumer/

> + * @hw: clk_hw associated with the clk being consumed
> + *
> + * Returns: new clk consummer
> + * This is the function to be used by providers which need
> + * to get a consummer clk and act on the clock element

s/consummer/consumer/

> + * Calls to this function must be balanced with calls clk_put()

calls to clk_put()

> + */
> +struct clk *clk_hw_get_clk(struct clk_hw *hw)

Can it also take a const char *id argument? That will let us "name" the
clk structure for situations where we want to keep track of who is using
the clk pointer for things. If that doesn't seem useful then I suppose
we can pass a string like "clk_hw_get_clk" for con_id below and hope it
doesn't become useful later.

> +{
> +       struct device *dev = hw->core->dev;
> +
> +       return clk_hw_create_clk(dev, hw, dev_name(dev), NULL);
> +}
> +EXPORT_SYMBOL(clk_hw_get_clk);
> +
>  static int clk_cpy_name(const char **dst_p, const char *src, bool must_exist)
>  {
>         const char *dst;

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] clk: add api to get clk consummer from clk_hw
  2020-05-19 17:04 [PATCH] clk: add api to get clk consummer from clk_hw Jerome Brunet
  2020-05-27  9:17 ` Stephen Boyd
@ 2020-05-27 20:07 ` Martin Blumenstingl
  2020-05-28 18:58   ` Jerome Brunet
  1 sibling, 1 reply; 6+ messages in thread
From: Martin Blumenstingl @ 2020-05-27 20:07 UTC (permalink / raw)
  To: Jerome Brunet; +Cc: Stephen Boyd, linux-clk, linux-kernel

Hi Jerome,

On Tue, May 19, 2020 at 7:09 PM Jerome Brunet <jbrunet@baylibre.com> wrote:
[...]
> + * Calls to this function must be balanced with calls clk_put()
> + */
> +struct clk *clk_hw_get_clk(struct clk_hw *hw)
I haven't looked at it myself yet, but would it be hard to have a
devm_ variant of this function as well?
a non-devm managed function would add boilerplate to the meson-mx-sdhc-mmc code

also this may or may not simplify how to fetch the struct device
pointer for this use-case.
(that said, I only know about drivers for Amlogic related IP and there
the devm_ variant can be used, but I don't know about other potential
consumers of this new API)


Thank you!
Martin

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] clk: add api to get clk consummer from clk_hw
  2020-05-27  9:17 ` Stephen Boyd
@ 2020-05-28 18:50   ` Jerome Brunet
  0 siblings, 0 replies; 6+ messages in thread
From: Jerome Brunet @ 2020-05-28 18:50 UTC (permalink / raw)
  To: Stephen Boyd, Martin Blumenstingl; +Cc: linux-clk, linux-kernel


On Wed 27 May 2020 at 11:17, Stephen Boyd <sboyd@kernel.org> wrote:

> Quoting Jerome Brunet (2020-05-19 10:04:40)
>> clk_register() is deprecated. Using 'clk' member of struct clk_hw is
>> discouraged. With this constrainst, it is difficult for driver to
>
> s/constrainst/constraint/
>
>> register clocks using the clk_hw API and then use the clock with
>> the consummer API
>
> s/consummer/consumer/
>
>> 
>> This add a simple helper, clk_hw_get_clk(), to get a struct clk from
>> a struct clk_hw. Like other clk_get() variant, each call to this helper
>> must be balanced with a call to clk_put().
>> 
>> Cc: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
>> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
>
> I like it!
>
>> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
>> index 6fd23ce3cb03..d9946e192cbc 100644
>> --- a/drivers/clk/clk.c
>> +++ b/drivers/clk/clk.c
>> @@ -3625,6 +3625,23 @@ struct clk *clk_hw_create_clk(struct device *dev, struct clk_hw *hw,
>>         return clk;
>>  }
>>  
>> +/**
>> + * clk_hw_get_clk: get clk consummer given an clk_hw
>
> s/consummer/consumer/
>
>> + * @hw: clk_hw associated with the clk being consumed
>> + *
>> + * Returns: new clk consummer
>> + * This is the function to be used by providers which need
>> + * to get a consummer clk and act on the clock element
>
> s/consummer/consumer/
>
>> + * Calls to this function must be balanced with calls clk_put()
>
> calls to clk_put()
>
>> + */
>> +struct clk *clk_hw_get_clk(struct clk_hw *hw)

Sorry about all the typos ...

>
> Can it also take a const char *id argument? That will let us "name" the
> clk structure for situations where we want to keep track of who is using
> the clk pointer for things. If that doesn't seem useful then I suppose
> we can pass a string like "clk_hw_get_clk" for con_id below and hope it
> doesn't become useful later.

Sure I can add the argument. no worries

>
>> +{
>> +       struct device *dev = hw->core->dev;
>> +
>> +       return clk_hw_create_clk(dev, hw, dev_name(dev), NULL);
>> +}
>> +EXPORT_SYMBOL(clk_hw_get_clk);
>> +
>>  static int clk_cpy_name(const char **dst_p, const char *src, bool must_exist)
>>  {
>>         const char *dst;


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] clk: add api to get clk consummer from clk_hw
  2020-05-27 20:07 ` Martin Blumenstingl
@ 2020-05-28 18:58   ` Jerome Brunet
  2020-05-28 23:03     ` Stephen Boyd
  0 siblings, 1 reply; 6+ messages in thread
From: Jerome Brunet @ 2020-05-28 18:58 UTC (permalink / raw)
  To: Martin Blumenstingl; +Cc: Stephen Boyd, linux-clk, linux-kernel


On Wed 27 May 2020 at 22:07, Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote:

> Hi Jerome,
>
> On Tue, May 19, 2020 at 7:09 PM Jerome Brunet <jbrunet@baylibre.com> wrote:
> [...]
>> + * Calls to this function must be balanced with calls clk_put()
>> + */
>> +struct clk *clk_hw_get_clk(struct clk_hw *hw)
> I haven't looked at it myself yet, but would it be hard to have a
> devm_ variant of this function as well?

Seems easy enough.
Stephen is this OK with you ?

I'm just wondering if this devm_ function should use the device pointer
embedded in the clk_hw structure or have it as an argument ?

The 1st option seems simpler but I'm not sure it is correct.

Thoughts ?

> a non-devm managed function would add boilerplate to the meson-mx-sdhc-mmc code
>
> also this may or may not simplify how to fetch the struct device
> pointer for this use-case.
> (that said, I only know about drivers for Amlogic related IP and there
> the devm_ variant can be used, but I don't know about other potential
> consumers of this new API)
>
>
> Thank you!
> Martin


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] clk: add api to get clk consummer from clk_hw
  2020-05-28 18:58   ` Jerome Brunet
@ 2020-05-28 23:03     ` Stephen Boyd
  0 siblings, 0 replies; 6+ messages in thread
From: Stephen Boyd @ 2020-05-28 23:03 UTC (permalink / raw)
  To: Jerome Brunet, Martin Blumenstingl; +Cc: linux-clk, linux-kernel

Quoting Jerome Brunet (2020-05-28 11:58:45)
> 
> On Wed 27 May 2020 at 22:07, Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote:
> 
> > Hi Jerome,
> >
> > On Tue, May 19, 2020 at 7:09 PM Jerome Brunet <jbrunet@baylibre.com> wrote:
> > [...]
> >> + * Calls to this function must be balanced with calls clk_put()
> >> + */
> >> +struct clk *clk_hw_get_clk(struct clk_hw *hw)
> > I haven't looked at it myself yet, but would it be hard to have a
> > devm_ variant of this function as well?
> 
> Seems easy enough.
> Stephen is this OK with you ?
> 
> I'm just wondering if this devm_ function should use the device pointer
> embedded in the clk_hw structure or have it as an argument ?
> 
> The 1st option seems simpler but I'm not sure it is correct.
> 
> Thoughts ?
> 

devm API sounds OK to me. For now we can use the one embedded in the
clk_hw structure and if we have to we can replace it with the one that
the caller passes in. Hopefully we never need to do that because then it
means we have drivers passing around clk_hw pointers instead of having
the caller use proper clk_get() style APIs.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-05-28 23:03 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-19 17:04 [PATCH] clk: add api to get clk consummer from clk_hw Jerome Brunet
2020-05-27  9:17 ` Stephen Boyd
2020-05-28 18:50   ` Jerome Brunet
2020-05-27 20:07 ` Martin Blumenstingl
2020-05-28 18:58   ` Jerome Brunet
2020-05-28 23:03     ` Stephen Boyd

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).