Quoting Sebastian Hesselbarth (2014-05-13 13:19:58)

On 05/13/2014 09:49 PM, Mike Turquette wrote:

quoted

Quoting Sebastian Hesselbarth (2014-05-11 13:24:34)

quoted

Currently, most DT clock drivers pick a unique node name to allow unique
clock names. As ePAPR recommends node names to be generic, we therefore
provide a helper to generate a unique clock name from the DT node name
plus reg property or a magic number instead. This is basically the same
we already do for proper devices and may vanish as soon as there is some
(early) device support for clocks available.

Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>

[...]

quoted

quoted

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index dff0373f53c1..b449a635dbfa 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c

@@ -17,6 +17,7 @@
 #include <linux/list.h>
 #include <linux/slab.h>
 #include <linux/of.h>
+#include <linux/of_address.h>
 #include <linux/device.h>
 #include <linux/init.h>
 #include <linux/sched.h>

@@ -2543,6 +2544,34 @@ const char *of_clk_get_parent_name(struct device_node *np, int index)
 }
 EXPORT_SYMBOL_GPL(of_clk_get_parent_name);
 
+/**
+ * of_clk_create_name() - Allocate and create a unique clock name
+ * @np: Device node pointer of the clock node
+ *
+ * This will allocate and create a unique clock name based on the
+ * reg property value. As a last resort, it will use the node name
+ * followed by a unique number. The caller has to deallocate the
+ * buffer.
+ */
+char *of_clk_create_name(struct device_node *np)
+{
+       static atomic_t clk_no_reg_magic;
+       const __be32 *reg;
+       u64 addr;
+       int magic;
+
+       reg = of_get_property(np, "reg", NULL);
+       if (reg) {
+               addr = of_translate_address(np, reg);
+               return kasprintf(GFP_KERNEL, "%llx.%s",
+                                (unsigned long long)addr, np->name);
+       }
+
+       magic = atomic_add_return(1, &clk_no_reg_magic);
+       return kasprintf(GFP_KERNEL, "%s.%d", np->name, magic);

For the case where we the reg property is present we use reg.name, but
for the case were the reg property is missing we use name.magic. Is it
intentional to switch the string and integer pairs?

Doing so avoids the case where magic might collide with a simple bus
clock (e.g. 'clk@1'), but I wanted to double check that it was
intentional.

Mike,

yes it is intentional and copies what is done for platform_device names.

Unfortunately, as much as I prefer this patch someday, it doesn't work
with the rest of the helpers as expected. While this can generate unique
and generic clock names, especially of_clk_get_parent_name() picks
either an clock-output-names named clock _or_ the node name ignoring the
above auto-generated name of course.

If you agree with the general approach here, we should still postpone
this for the next cycle when I have more time to look at the details.
I prefer to rename the nodes and use clock-output-names where required
for the Berlin clock nodes now.

Yes, that sounds fine. I'd also like the DT maintainers to take a look
at this and weigh in.

Thanks,
Mike

Sebastian

quoted

quoted

+}
+EXPORT_SYMBOL_GPL(of_clk_create_name);
+
 struct clock_provider {
        of_clk_init_cb_t clk_init_cb;
        struct device_node *np;

diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 511917416fb0..c6f3ca1cd81c 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h

@@ -514,6 +514,7 @@ struct clk *of_clk_src_simple_get(struct of_phandle_args *clkspec,
 struct clk *of_clk_src_onecell_get(struct of_phandle_args *clkspec, void *data);
 int of_clk_get_parent_count(struct device_node *np);
 const char *of_clk_get_parent_name(struct device_node *np, int index);
+char *of_clk_create_name(struct device_node *np);
 
 void of_clk_init(const struct of_device_id *matches);
 

@@ -543,6 +544,10 @@ static inline const char *of_clk_get_parent_name(struct device_node *np,
 {
        return NULL;
 }
+static inline char *of_clk_create_name(struct device_node *np)
+{
+       return NULL;
+}
 #define of_clk_init(matches) \
        { while (0); }
 #endif /* CONFIG_OF */

-- 
1.9.1