All of lore.kernel.org
 help / color / mirror / Atom feed
From: robherring2@gmail.com (Rob Herring)
To: linux-arm-kernel@lists.infradead.org
Subject: [RESEND PATCH 1/1] clk: add DT support for clock gating control
Date: Thu, 12 Jul 2012 22:19:56 -0500	[thread overview]
Message-ID: <4FFF93DC.30103@gmail.com> (raw)
In-Reply-To: <4FFECC4E.4070001@googlemail.com>

On 07/12/2012 08:08 AM, Sebastian Hesselbarh wrote:
> On 07/12/2012 02:14 PM, Rob Herring wrote:
>>> +Required child properties:
>>> +- reg : should contain the individual bit and polarity to control
>>> +        the clock gate. A polarity of 0 means that by setting the
>>> +        bit to 1 the clock passes through the clock gate while
>>> +    setting the bit to 0 disables the clock. Any other value
>>> +         for polarity inverts the meaning of the control bit.
>>
>> This is a bit of overloading reg to specify the polarity.
> 
> Well, yes it is overloading but still matches reg somehow, as the
> extra information is required to access the resource. But I agree,
> expecially wrt more-than-one-bit clk-gate (see below).
> 

You can define your own property names.

>>> +        /* SATA clock gate with different parent clock */
>>> +        cg_sata: clockgate at 3 {
>>> +            reg =<3 0>; /* register bit 3, normal polarity */
>>> +            clocks =<&sata_clk>;
>>> +        };
>>
>> I'm not sure I like the node per bit. What about a bit mask for valid
>> bits and polarities. Then add a clock cell to specify the bit or index.
>>
>> i.MX has 2-bit enable fields for its leaf clocks, so how and if you
>> would support that is something to think about.
> 
> Yeah, I thought of "what if the clk_gate needs to be enabled with more
> than 1 bit" already. But this is a short-comming of the current clk-gate
> implementation.

What's implemented in Linux should not define the binding. The binding
should describe the hardware.

> Just to get it right, i.MX requires to set more than one bit to change
> the state of the gate for one leaf clock?

It's basically ON, OFF, and "ON in run/OFF in wfi".

Perhaps the iMX case is unique enough we don't try to make it use a
common binding.

> If this is true, that would require a change of the generic clk-gate
> anyway.

True, but not your problem to implement. A binding doesn't necessarily
mean there is a full Linux implementation. We just don't want to create
something only to find others need something completely different.

Rob

> I had a look at pinctrl-bindings.txt maybe this is the way to go for
> clock gating control, too. That would require clk-gate to handle an
> 'active' and 'gated' state and leave it to a clock gate control to
> actually set the required bits in any registers. This would allow
> other special implementations of clock gating controllers to reuse
> clk-gate DT description. Additionally, there could be a
> simple-clock-gating-control that can set states by reg address and
> for each controlled gate a mask, enable value, and disable value.
> 
> Sebastian

WARNING: multiple messages have this Message-ID (diff)
From: Rob Herring <robherring2@gmail.com>
To: Sebastian Hesselbarh <sebastian.hesselbarth@googlemail.com>
Cc: Grant Likely <grant.likely@secretlab.ca>,
	Rob Landley <rob@landley.net>, Mike Turquette <mturquette@ti.com>,
	devicetree-discuss@lists.ozlabs.org, linux-doc@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [RESEND PATCH 1/1] clk: add DT support for clock gating control
Date: Thu, 12 Jul 2012 22:19:56 -0500	[thread overview]
Message-ID: <4FFF93DC.30103@gmail.com> (raw)
In-Reply-To: <4FFECC4E.4070001@googlemail.com>

On 07/12/2012 08:08 AM, Sebastian Hesselbarh wrote:
> On 07/12/2012 02:14 PM, Rob Herring wrote:
>>> +Required child properties:
>>> +- reg : should contain the individual bit and polarity to control
>>> +        the clock gate. A polarity of 0 means that by setting the
>>> +        bit to 1 the clock passes through the clock gate while
>>> +    setting the bit to 0 disables the clock. Any other value
>>> +         for polarity inverts the meaning of the control bit.
>>
>> This is a bit of overloading reg to specify the polarity.
> 
> Well, yes it is overloading but still matches reg somehow, as the
> extra information is required to access the resource. But I agree,
> expecially wrt more-than-one-bit clk-gate (see below).
> 

You can define your own property names.

>>> +        /* SATA clock gate with different parent clock */
>>> +        cg_sata: clockgate@3 {
>>> +            reg =<3 0>; /* register bit 3, normal polarity */
>>> +            clocks =<&sata_clk>;
>>> +        };
>>
>> I'm not sure I like the node per bit. What about a bit mask for valid
>> bits and polarities. Then add a clock cell to specify the bit or index.
>>
>> i.MX has 2-bit enable fields for its leaf clocks, so how and if you
>> would support that is something to think about.
> 
> Yeah, I thought of "what if the clk_gate needs to be enabled with more
> than 1 bit" already. But this is a short-comming of the current clk-gate
> implementation.

What's implemented in Linux should not define the binding. The binding
should describe the hardware.

> Just to get it right, i.MX requires to set more than one bit to change
> the state of the gate for one leaf clock?

It's basically ON, OFF, and "ON in run/OFF in wfi".

Perhaps the iMX case is unique enough we don't try to make it use a
common binding.

> If this is true, that would require a change of the generic clk-gate
> anyway.

True, but not your problem to implement. A binding doesn't necessarily
mean there is a full Linux implementation. We just don't want to create
something only to find others need something completely different.

Rob

> I had a look at pinctrl-bindings.txt maybe this is the way to go for
> clock gating control, too. That would require clk-gate to handle an
> 'active' and 'gated' state and leave it to a clock gate control to
> actually set the required bits in any registers. This would allow
> other special implementations of clock gating controllers to reuse
> clk-gate DT description. Additionally, there could be a
> simple-clock-gating-control that can set states by reg address and
> for each controlled gate a mask, enable value, and disable value.
> 
> Sebastian



  reply	other threads:[~2012-07-13  3:19 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-07-12  7:15 [RESEND PATCH 1/1] clk: add DT support for clock gating control Sebastian Hesselbarth
2012-07-12  7:15 ` Sebastian Hesselbarth
2012-07-12 12:14 ` Rob Herring
2012-07-12 12:14   ` Rob Herring
2012-07-12 13:08   ` Sebastian Hesselbarh
2012-07-12 13:08     ` Sebastian Hesselbarh
2012-07-13  3:19     ` Rob Herring [this message]
2012-07-13  3:19       ` Rob Herring
2012-07-13  9:42       ` Sebastian Hesselbarh
2012-07-13  9:42         ` Sebastian Hesselbarh
2012-07-14  5:00         ` Rob Herring
2012-07-14  5:00           ` Rob Herring
2012-07-14  5:00           ` Rob Herring
2012-07-15 20:45         ` Rob Landley
2012-07-15 20:45           ` Rob Landley

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=4FFF93DC.30103@gmail.com \
    --to=robherring2@gmail.com \
    --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.