On 05/23/2014 07:53 PM, Mike Turquette wrote:

Quoting Alex Elder (2014-05-20 05:52:39)

quoted

@@ -743,11 +746,16 @@ struct clk *kona_clk_setup(struct kona_clk *bcm_clk)
        clk = clk_register(NULL, &bcm_clk->hw);
        if (IS_ERR(clk)) {
                pr_err("%s: error registering clock %s (%ld)\n", __func__,
-                       init_data->name, PTR_ERR(clk));
+                       name, PTR_ERR(clk));
                goto out_teardown;
        }
        BUG_ON(!clk);
 
+       /*  Make it so we can look the clock up using clk_find() */

s/clk_find/clk_get/ ?

quoted

+       bcm_clk->cl.con_id = name;
+       bcm_clk->cl.clk = clk;
+       clkdev_add(&bcm_clk->cl);

This is not so nice. I'll explain more below.

OK, despite what I said before, I do need this, or
something like it, so I can look up clocks by name.
(Continued below.)

quoted

+
        return clk;
 out_teardown:
        bcm_clk_teardown(bcm_clk);

diff --git a/drivers/clk/bcm/clk-kona.c b/drivers/clk/bcm/clk-kona.c
index d8a7f38..fd070d6 100644
--- a/drivers/clk/bcm/clk-kona.c
+++ b/drivers/clk/bcm/clk-kona.c

@@ -1195,6 +1195,48 @@ static bool __peri_clk_init(struct kona_clk *bcm_clk)
        return true;
 }
 
+static bool __kona_clk_init(struct kona_clk *bcm_clk);
+static bool __kona_prereq_init(struct kona_clk *bcm_clk)
+{
+       struct clk *clk;
+       struct clk_hw *hw;
+       struct kona_clk *prereq;
+
+       BUG_ON(clk_is_initialized(bcm_clk));
+
+       if (!bcm_clk->p.prereq)
+               return true;
+
+       clk = clk_get(NULL, bcm_clk->p.prereq);

The clkdev global namespace is getting polluted with all of these new
prereq clocks. If there was an associated struct device *dev with them
then it wouldn't be a problem, but you might get collisions with other
clock drivers that also use NULL for the device.

Yes I recognize this.  Ideally a CCU would have a device struct
associated with it that I could use, because the name of a clock
is unique within that context.  But I have no such device available.
(Please correct me if I'm wrong.  I don't want to make one up, and
I would like to use it if it exists.)

It would be a lot nicer for the clocks that require a prereq clock to
just use clk_get(dev, "well_known_name"); in the same way that drivers
use it, without considering it a special case.

I can do something like that if I can get a meaningful device
structure.  Do you have any suggestions?

Other than this issue, I've implemented all of the previous
initialization routines using ->prepare() instead, and it
works fine.  I'm going to send an updated series out tomorrow.
I want to look it over again after a good night's sleep...

					-Alex