Hi Stephen,

On 30 November 2018 09:09 Stephen Boyd wrote:

Quoting Phil Edworthy (2018-11-20 06:14:45)

quoted

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.

Ok. I guess that works by virtue of how -ENOENT is returned by various
functions that are called deeper in the clk_get() path? I'm cautiously
optimistic. So cautious, we should probably add a comment to these optional
functions that indicate they rely on the functions they call to return -ENOENT
under the various conditions that make a clk optional.

Yes, it does indeed rely on how clk_get() is implemented.
Specifically, that if __of_clk_get_by_name() returns -EINVAL, the error is
superseded by clk_get_sys() returning -ENOENT.
As you say, a comment may help here.

quoted

diff --git a/include/linux/clk.h b/include/linux/clk.h index

a7773b5c0b9f..3ea3c78f62dd 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,

quoted

  */
 struct clk *devm_clk_get(struct device *dev, const char *id);

+/**
+ * devm_clk_get_optional - lookup and obtain a managed reference to an

optional

quoted

+ *                        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

Please add () around devm_clk_get() so we know it's a function.

Will do.

quoted

+ * 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)

quoted

        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,

quoted

                                                 struct clk_bulk_data
*clks)  { @@ -862,6 +879,16 @@ static inline void
clk_bulk_disable_unprepare(int num_clks,
        clk_bulk_unprepare(num_clks, clks);  }

+static inline struct clk *clk_get_optional(struct device *dev, const
+char *id)

Any kernel doc for this function?

I took my cue from the surrounding functions, let me know if I have to add it.

Thanks
Phil

quoted

+{
+       struct clk *clk = clk_get(dev, id);
+
+       if (clk == ERR_PTR(-ENOENT))
+               clk = NULL;
+
+       return clk;
+}
+

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel