All of lore.kernel.org
 help / color / mirror / Atom feed
From: elder@linaro.org (Alex Elder)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2 2/5] clk: bcm281xx: implement prerequisite clocks
Date: Fri, 30 May 2014 09:05:31 -0500	[thread overview]
Message-ID: <5388902B.9080302@linaro.org> (raw)
In-Reply-To: <5387F8EF.3030607@linaro.org>

On 05/29/2014 10:20 PM, Alex Elder wrote:
> On 05/23/2014 07:53 PM, Mike Turquette wrote:
>> Quoting Alex Elder (2014-05-20 05:52:39)
>>> @@ -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/ ?
>>
>>> +       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.)
...

I've been thinking this morning about ways to at least
improve this.  The problem is worse than just prerequisite
clocks polluting the global name space.  Right now *all*
clocks get their name registered this way, because any one
of them could be tagged as a prerequisite, and therefore
in need of lookup by name.

If I had a device structure to associate the clock names
with it would help, but I don't have one.  There is no
other way to define a separate name space, it's either
associated with a device, or it's global.

Given all that, I could prefix or suffix the clock names
with some special string, in order to sort of carve out a
reserved portion of the global name space.

I could specify the prerequisite clock by its index in
its CCU's clocks array.  I could then manufacture a
of_phandle_args structure and use of_clk_get_from_provider()
to look up what we need, but that seems kind of kludgy.
Maybe a new function could encapsulate the messy details
of that.

Do you have any suggestions?  I can create some new
common code if appropriate, but only if it represents
missing functionality that's generally useful.

					-Alex

WARNING: multiple messages have this Message-ID (diff)
From: Alex Elder <elder-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
To: Mike Turquette
	<mturquette-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>,
	mporter-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org,
	bcm-xK7y4jjYLqYh9ZMKESR00Q@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: [PATCH v2 2/5] clk: bcm281xx: implement prerequisite clocks
Date: Fri, 30 May 2014 09:05:31 -0500	[thread overview]
Message-ID: <5388902B.9080302@linaro.org> (raw)
In-Reply-To: <5387F8EF.3030607-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

On 05/29/2014 10:20 PM, Alex Elder wrote:
> On 05/23/2014 07:53 PM, Mike Turquette wrote:
>> Quoting Alex Elder (2014-05-20 05:52:39)
>>> @@ -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/ ?
>>
>>> +       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.)
...

I've been thinking this morning about ways to at least
improve this.  The problem is worse than just prerequisite
clocks polluting the global name space.  Right now *all*
clocks get their name registered this way, because any one
of them could be tagged as a prerequisite, and therefore
in need of lookup by name.

If I had a device structure to associate the clock names
with it would help, but I don't have one.  There is no
other way to define a separate name space, it's either
associated with a device, or it's global.

Given all that, I could prefix or suffix the clock names
with some special string, in order to sort of carve out a
reserved portion of the global name space.

I could specify the prerequisite clock by its index in
its CCU's clocks array.  I could then manufacture a
of_phandle_args structure and use of_clk_get_from_provider()
to look up what we need, but that seems kind of kludgy.
Maybe a new function could encapsulate the messy details
of that.

Do you have any suggestions?  I can create some new
common code if appropriate, but only if it represents
missing functionality that's generally useful.

					-Alex


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
From: Alex Elder <elder@linaro.org>
To: Mike Turquette <mturquette@linaro.org>,
	mporter@linaro.org, bcm@fixthebug.org,
	devicetree@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 2/5] clk: bcm281xx: implement prerequisite clocks
Date: Fri, 30 May 2014 09:05:31 -0500	[thread overview]
Message-ID: <5388902B.9080302@linaro.org> (raw)
In-Reply-To: <5387F8EF.3030607@linaro.org>

On 05/29/2014 10:20 PM, Alex Elder wrote:
> On 05/23/2014 07:53 PM, Mike Turquette wrote:
>> Quoting Alex Elder (2014-05-20 05:52:39)
>>> @@ -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/ ?
>>
>>> +       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.)
...

I've been thinking this morning about ways to at least
improve this.  The problem is worse than just prerequisite
clocks polluting the global name space.  Right now *all*
clocks get their name registered this way, because any one
of them could be tagged as a prerequisite, and therefore
in need of lookup by name.

If I had a device structure to associate the clock names
with it would help, but I don't have one.  There is no
other way to define a separate name space, it's either
associated with a device, or it's global.

Given all that, I could prefix or suffix the clock names
with some special string, in order to sort of carve out a
reserved portion of the global name space.

I could specify the prerequisite clock by its index in
its CCU's clocks array.  I could then manufacture a
of_phandle_args structure and use of_clk_get_from_provider()
to look up what we need, but that seems kind of kludgy.
Maybe a new function could encapsulate the messy details
of that.

Do you have any suggestions?  I can create some new
common code if appropriate, but only if it represents
missing functionality that's generally useful.

					-Alex



  reply	other threads:[~2014-05-30 14:05 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-20 12:52 [PATCH v2 0/5] clk: bcm: prerequisite and bus clock support Alex Elder
2014-05-20 12:52 ` Alex Elder
2014-05-20 12:52 ` Alex Elder
2014-05-20 12:52 ` [PATCH v2 1/5] clk: bcm281xx: add an initialized flag Alex Elder
2014-05-20 12:52   ` Alex Elder
2014-05-20 12:52   ` Alex Elder
2014-05-24  0:33   ` Mike Turquette
2014-05-24  0:33     ` Mike Turquette
2014-05-24  0:33     ` Mike Turquette
2014-05-29 13:26     ` Alex Elder
2014-05-29 13:26       ` Alex Elder
2014-05-20 12:52 ` [PATCH v2 2/5] clk: bcm281xx: implement prerequisite clocks Alex Elder
2014-05-20 12:52   ` Alex Elder
2014-05-24  0:53   ` Mike Turquette
2014-05-24  0:53     ` Mike Turquette
2014-05-29 13:26     ` Alex Elder
2014-05-29 13:26       ` Alex Elder
2014-05-29 13:26       ` Alex Elder
2014-05-29 16:35       ` Mike Turquette
2014-05-29 16:35         ` Mike Turquette
2014-05-29 16:53         ` Alex Elder
2014-05-29 16:53           ` Alex Elder
2014-05-29 16:53           ` Alex Elder
2014-05-29 17:47           ` Mike Turquette
2014-05-29 17:47             ` Mike Turquette
2014-05-30  3:20     ` Alex Elder
2014-05-30  3:20       ` Alex Elder
2014-05-30  3:20       ` Alex Elder
2014-05-30 14:05       ` Alex Elder [this message]
2014-05-30 14:05         ` Alex Elder
2014-05-30 14:05         ` Alex Elder
2014-05-20 12:52 ` [PATCH v2 3/5] clk: bcm281xx: add bus clock support Alex Elder
2014-05-20 12:52   ` Alex Elder
2014-05-20 12:52 ` [PATCH v2 4/5] clk: bcm281xx: define a bus clock Alex Elder
2014-05-20 12:52   ` Alex Elder
     [not found] ` <1400590362-11177-1-git-send-email-elder-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2014-05-20 12:55   ` [PATCH v2 5/5] ARM: dts: add bus clock bsc3_apb for bcm281xx Alex Elder

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5388902B.9080302@linaro.org \
    --to=elder@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.