* [PATCH v9 0/2] clk: Add functions to get optional clocks
@ 2018-12-03 11:13 Phil Edworthy
2018-12-03 11:13 ` [PATCH v9 1/2] clk: Add comment about __of_clk_get_by_name() error values Phil Edworthy
2018-12-03 11:13 ` [PATCH v9 2/2] clk: Add (devm_)clk_get_optional() functions Phil Edworthy
0 siblings, 2 replies; 7+ messages in thread
From: Phil Edworthy @ 2018-12-03 11:13 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Russell King
Cc: Andy Shevchenko, Geert Uytterhoeven, Uwe Kleine-König,
linux-clk, linux-kernel, linux-arm-kernel, Phil Edworthy
Quite a few drivers get an optional clock, e.g. a bus clock required to
access peripheral's registers that is always enabled on some devices.
v9:
- Add a separate patch to add a comment about __of_clk_get_by_name() error
values.
- Add brackets after devm_clk_get so people know it's a function.
- Add kernel doc for clk_get_optional().
Phil Edworthy (2):
clk: Add comment about __of_clk_get_by_name() error values
clk: Add (devm_)clk_get_optional() functions
drivers/clk/clk-devres.c | 11 +++++++++++
drivers/clk/clkdev.c | 6 ++++++
include/linux/clk.h | 36 ++++++++++++++++++++++++++++++++++++
3 files changed, 53 insertions(+)
--
2.17.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v9 1/2] clk: Add comment about __of_clk_get_by_name() error values
2018-12-03 11:13 [PATCH v9 0/2] clk: Add functions to get optional clocks Phil Edworthy
@ 2018-12-03 11:13 ` Phil Edworthy
2018-12-03 13:30 ` Andy Shevchenko
2018-12-03 11:13 ` [PATCH v9 2/2] clk: Add (devm_)clk_get_optional() functions Phil Edworthy
1 sibling, 1 reply; 7+ messages in thread
From: Phil Edworthy @ 2018-12-03 11:13 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Russell King
Cc: Andy Shevchenko, Geert Uytterhoeven, Uwe Kleine-König,
linux-clk, linux-kernel, linux-arm-kernel, Phil Edworthy
It's not immediately obvious from the code that failure to get a
clock provider can return either -ENOENT or -EINVAL. Therefore, add
a comment to highlight this.
Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
---
drivers/clk/clkdev.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c
index 9ab3db8b3988..cc5df3970cd3 100644
--- a/drivers/clk/clkdev.c
+++ b/drivers/clk/clkdev.c
@@ -52,6 +52,12 @@ struct clk *of_clk_get(struct device_node *np, int index)
}
EXPORT_SYMBOL(of_clk_get);
+/*
+ * Beware the return values when np is valid, but no clock provider is found.
+ * If name = NULL, the function returns -ENOENT.
+ * If name != NULL, the function returns -EINVAL. This is because __of_clk_get()
+ * is called even if of_property_match_string() returns an error.
+ */
static struct clk *__of_clk_get_by_name(struct device_node *np,
const char *dev_id,
const char *name)
--
2.17.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v9 2/2] clk: Add (devm_)clk_get_optional() functions
2018-12-03 11:13 [PATCH v9 0/2] clk: Add functions to get optional clocks Phil Edworthy
2018-12-03 11:13 ` [PATCH v9 1/2] clk: Add comment about __of_clk_get_by_name() error values Phil Edworthy
@ 2018-12-03 11:13 ` Phil Edworthy
1 sibling, 0 replies; 7+ messages in thread
From: Phil Edworthy @ 2018-12-03 11:13 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Russell King
Cc: Andy Shevchenko, Geert Uytterhoeven, Uwe Kleine-König,
linux-clk, linux-kernel, linux-arm-kernel, Phil Edworthy
This adds clk_get_optional() and devm_clk_get_optional() functions to get
optional clocks.
They behave the same as (devm_)clk_get() except where there is no clock
producer. In this case, instead of returning -ENOENT, the function
returns NULL. This makes error checking simpler and allows
clk_prepare_enable, etc to be called on the returned reference
without additional checks.
Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v9:
- Add brackets after devm_clk_get so people know it's a function.
- Add kernel doc for clk_get_optional().
v8:
- Remove else clause
v7:
- Instead of messing with the core functions, simply wrap them for the
_optional() versions. By putting clk_get_optional() inline in the header
file, we can get rid of the arch specific patches as well.
v6:
- Add doxygen style comment for devm_clk_get_optional() args
v5:
- No changes.
v4:
- No changes.
v3:
- No changes.
---
drivers/clk/clk-devres.c | 11 +++++++++++
include/linux/clk.h | 36 ++++++++++++++++++++++++++++++++++++
2 files changed, 47 insertions(+)
diff --git a/drivers/clk/clk-devres.c b/drivers/clk/clk-devres.c
index 12c87457eca1..8e8893168676 100644
--- a/drivers/clk/clk-devres.c
+++ b/drivers/clk/clk-devres.c
@@ -34,6 +34,17 @@ struct clk *devm_clk_get(struct device *dev, const char *id)
}
EXPORT_SYMBOL(devm_clk_get);
+struct clk *devm_clk_get_optional(struct device *dev, const char *id)
+{
+ struct clk *clk = devm_clk_get(dev, id);
+
+ if (clk == ERR_PTR(-ENOENT))
+ return NULL;
+
+ return clk;
+}
+EXPORT_SYMBOL(devm_clk_get_optional);
+
struct clk_bulk_devres {
struct clk_bulk_data *clks;
int num_clks;
diff --git a/include/linux/clk.h b/include/linux/clk.h
index a7773b5c0b9f..d8bc1a856b39 100644
--- a/include/linux/clk.h
+++ b/include/linux/clk.h
@@ -383,6 +383,17 @@ int __must_check devm_clk_bulk_get_all(struct device *dev,
*/
struct clk *devm_clk_get(struct device *dev, const char *id);
+/**
+ * devm_clk_get_optional - lookup and obtain a managed reference to an optional
+ * clock producer.
+ * @dev: device for clock "consumer"
+ * @id: clock consumer ID
+ *
+ * Behaves the same as devm_clk_get() except where there is no clock producer.
+ * In this case, instead of returning -ENOENT, the function returns NULL.
+ */
+struct clk *devm_clk_get_optional(struct device *dev, const char *id);
+
/**
* devm_get_clk_from_child - lookup and obtain a managed reference to a
* clock producer from child node.
@@ -718,6 +729,12 @@ static inline struct clk *devm_clk_get(struct device *dev, const char *id)
return NULL;
}
+static inline struct clk *devm_clk_get_optional(struct device *dev,
+ const char *id)
+{
+ return NULL;
+}
+
static inline int __must_check devm_clk_bulk_get(struct device *dev, int num_clks,
struct clk_bulk_data *clks)
{
@@ -862,6 +879,25 @@ static inline void clk_bulk_disable_unprepare(int num_clks,
clk_bulk_unprepare(num_clks, clks);
}
+/**
+ * clk_get_optional - lookup and obtain a reference to an optional clock
+ * producer.
+ * @dev: device for clock "consumer"
+ * @id: clock consumer ID
+ *
+ * Behaves the same as clk_get() except where there is no clock producer. In
+ * this case, instead of returning -ENOENT, the function returns NULL.
+ */
+static inline struct clk *clk_get_optional(struct device *dev, const char *id)
+{
+ struct clk *clk = clk_get(dev, id);
+
+ if (clk == ERR_PTR(-ENOENT))
+ return NULL;
+
+ return clk;
+}
+
#if defined(CONFIG_OF) && defined(CONFIG_COMMON_CLK)
struct clk *of_clk_get(struct device_node *np, int index);
struct clk *of_clk_get_by_name(struct device_node *np, const char *name);
--
2.17.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v9 1/2] clk: Add comment about __of_clk_get_by_name() error values
2018-12-03 11:13 ` [PATCH v9 1/2] clk: Add comment about __of_clk_get_by_name() error values Phil Edworthy
@ 2018-12-03 13:30 ` Andy Shevchenko
2018-12-06 12:30 ` Phil Edworthy
0 siblings, 1 reply; 7+ messages in thread
From: Andy Shevchenko @ 2018-12-03 13:30 UTC (permalink / raw)
To: Phil Edworthy
Cc: Michael Turquette, Stephen Boyd, Russell King,
Geert Uytterhoeven, Uwe Kleine-König, linux-clk,
linux-kernel, linux-arm-kernel
On Mon, Dec 03, 2018 at 11:13:08AM +0000, Phil Edworthy wrote:
> It's not immediately obvious from the code that failure to get a
> clock provider can return either -ENOENT or -EINVAL. Therefore, add
> a comment to highlight this.
> +/*
> + * Beware the return values when np is valid, but no clock provider is found.
> + * If name = NULL, the function returns -ENOENT.
> + * If name != NULL, the function returns -EINVAL. This is because __of_clk_get()
I would start new sentence from new line
(this will emphasize the possible variants)
* This is ...
Otherwise looks good to me:
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> + * is called even if of_property_match_string() returns an error.
> + */
> static struct clk *__of_clk_get_by_name(struct device_node *np,
> const char *dev_id,
> const char *name)
> --
> 2.17.1
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH v9 1/2] clk: Add comment about __of_clk_get_by_name() error values
2018-12-03 13:30 ` Andy Shevchenko
@ 2018-12-06 12:30 ` Phil Edworthy
2019-01-16 15:18 ` Phil Edworthy
0 siblings, 1 reply; 7+ messages in thread
From: Phil Edworthy @ 2018-12-06 12:30 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Michael Turquette, Stephen Boyd, Russell King,
Geert Uytterhoeven, Uwe Kleine-König, linux-clk,
linux-kernel, linux-arm-kernel
Hi Andy,
On 03 December 2018 13:31 Andy Shevchenko wrote:
> On Mon, Dec 03, 2018 at 11:13:08AM +0000, Phil Edworthy wrote:
> > It's not immediately obvious from the code that failure to get a clock
> > provider can return either -ENOENT or -EINVAL. Therefore, add a
> > comment to highlight this.
>
> > +/*
> > + * Beware the return values when np is valid, but no clock provider is
> found.
> > + * If name = NULL, the function returns -ENOENT.
> > + * If name != NULL, the function returns -EINVAL. This is because
> > +__of_clk_get()
>
> I would start new sentence from new line (this will emphasize the possible
> variants)
>
> * This is ...
I disagree, the explanation is specifically related to the case where the function
returns -EINVAL. Though this is a nit, so I'm not really bothered either way.
Thanks for the review!
Phil
> Otherwise looks good to me:
>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>
> > + * is called even if of_property_match_string() returns an error.
> > + */
> > static struct clk *__of_clk_get_by_name(struct device_node *np,
> > const char *dev_id,
> > const char *name)
> > --
> > 2.17.1
> >
>
> --
> With Best Regards,
> Andy Shevchenko
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH v9 1/2] clk: Add comment about __of_clk_get_by_name() error values
2018-12-06 12:30 ` Phil Edworthy
@ 2019-01-16 15:18 ` Phil Edworthy
2019-01-24 21:07 ` Andy Shevchenko
0 siblings, 1 reply; 7+ messages in thread
From: Phil Edworthy @ 2019-01-16 15:18 UTC (permalink / raw)
To: Andy Shevchenko, Michael Turquette, Uwe Kleine-König,
Stephen Boyd, Russell King, Geert Uytterhoeven
Cc: linux-clk, linux-kernel, linux-arm-kernel
Hi,
Any other comments on this patch and patch 2/2 (https://lkml.org/lkml/2018/12/3/326)?
Thanks
Phil
> -----Original Message-----
> From: Phil Edworthy
> Sent: 06 December 2018 12:31
> To: 'Andy Shevchenko' <andriy.shevchenko@linux.intel.com>
> Cc: Michael Turquette <mturquette@baylibre.com>; Stephen Boyd
> <sboyd@kernel.org>; Russell King <linux@armlinux.org.uk>; Geert
> Uytterhoeven <geert@linux-m68k.org>; Uwe Kleine-König <u.kleine-
> koenig@pengutronix.de>; linux-clk@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org
> Subject: RE: [PATCH v9 1/2] clk: Add comment about
> __of_clk_get_by_name() error values
>
> Hi Andy,
>
> On 03 December 2018 13:31 Andy Shevchenko wrote:
> > On Mon, Dec 03, 2018 at 11:13:08AM +0000, Phil Edworthy wrote:
> > > It's not immediately obvious from the code that failure to get a
> > > clock provider can return either -ENOENT or -EINVAL. Therefore, add
> > > a comment to highlight this.
> >
> > > +/*
> > > + * Beware the return values when np is valid, but no clock provider
> > > +is
> > found.
> > > + * If name = NULL, the function returns -ENOENT.
> > > + * If name != NULL, the function returns -EINVAL. This is because
> > > +__of_clk_get()
> >
> > I would start new sentence from new line (this will emphasize the
> > possible
> > variants)
> >
> > * This is ...
> I disagree, the explanation is specifically related to the case where the
> function returns -EINVAL. Though this is a nit, so I'm not really bothered
> either way.
>
> Thanks for the review!
> Phil
>
> > Otherwise looks good to me:
> >
> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> >
> > > + * is called even if of_property_match_string() returns an error.
> > > + */
> > > static struct clk *__of_clk_get_by_name(struct device_node *np,
> > > const char *dev_id,
> > > const char *name)
> > > --
> > > 2.17.1
> > >
> >
> > --
> > With Best Regards,
> > Andy Shevchenko
> >
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v9 1/2] clk: Add comment about __of_clk_get_by_name() error values
2019-01-16 15:18 ` Phil Edworthy
@ 2019-01-24 21:07 ` Andy Shevchenko
0 siblings, 0 replies; 7+ messages in thread
From: Andy Shevchenko @ 2019-01-24 21:07 UTC (permalink / raw)
To: Phil Edworthy
Cc: Michael Turquette, Uwe Kleine-König, Stephen Boyd,
Russell King, Geert Uytterhoeven, linux-clk, linux-kernel,
linux-arm-kernel
On Wed, Jan 16, 2019 at 03:18:42PM +0000, Phil Edworthy wrote:
> Hi,
>
> Any other comments on this patch and patch 2/2 (https://lkml.org/lkml/2018/12/3/326)?
Was on vacations, sorry.
> > > I would start new sentence from new line (this will emphasize the
> > > possible
> > > variants)
> > >
> > > * This is ...
> > I disagree, the explanation is specifically related to the case where the
> > function returns -EINVAL. Though this is a nit, so I'm not really bothered
> > either way.
Ah, okay. You may bear my tags on.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-01-24 21:08 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-03 11:13 [PATCH v9 0/2] clk: Add functions to get optional clocks Phil Edworthy
2018-12-03 11:13 ` [PATCH v9 1/2] clk: Add comment about __of_clk_get_by_name() error values Phil Edworthy
2018-12-03 13:30 ` Andy Shevchenko
2018-12-06 12:30 ` Phil Edworthy
2019-01-16 15:18 ` Phil Edworthy
2019-01-24 21:07 ` Andy Shevchenko
2018-12-03 11:13 ` [PATCH v9 2/2] clk: Add (devm_)clk_get_optional() functions Phil Edworthy
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).