linux-kernel - Re: [RFC v2 4/9] of: add clock providers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJOA=zMgGsZKGQ-EfSnXDY_WoFrzG6XPZtuxANFKqdL8CAaYrw@mail.gmail.com>
Date:	Tue, 17 Jan 2012 15:37:12 -0800
From:	"Turquette, Mike" <mturquette@...com>
To:	Grant Likely <grant.likely@...retlab.ca>
Cc:	Stephen Warren <swarren@...dia.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"devicetree-discuss@...ts.ozlabs.org" 
	<devicetree-discuss@...ts.ozlabs.org>,
	Rob Herring <rob.herring@...xeda.com>,
	Sascha Hauer <kernel@...gutronix.de>
Subject: Re: [RFC v2 4/9] of: add clock providers

On Tue, Jan 17, 2012 at 2:47 PM, Grant Likely <grant.likely@...retlab.ca> wrote:
> On Tue, Jan 17, 2012 at 1:44 PM, Stephen Warren <swarren@...dia.com> wrote:
>> In other words, does the UART driver need to do something like:
>>
>> clk_reg = clk_get(dev, "register");
>> clk_parent = of_clk_get_by_name(np, "register);
>> clk_set_parent(clk_reg, clk_parent);
>>
>> Or will that all happen transparently within just the of_clk_get_by_name
>> call?
>>
>> (I suppose this question makes slightly more sense for the PLL itself,
>> since both the upstream and downstream clocks are represented in the PLL
>> node, whereas the UART's node only represents the clock consumer side,
>> so the above code isn't really possible automatically).
>
> The intent is that device only interacts with the leaf device.  If the
> clocks are arranged into a hierarchy, then the clock driver is
> responsible for any interactions with the parent clock.  Requiring the
> driver to manipulate parent clocks directly defeats the purpose of
> having a clock abstraction.

I don't think that we can get rid of all instances of drivers knowing
a bit about hierarchy.  I think we can get rid of most, but there are
cases where it is valid for a driver to know some of the details.
More on that below.

>> Somewhat related to this: How does dynamic reparenting interact with
>> the DT clock binding; is the DT just the default/initial clock setup,
>> and anything beyond that needs a custom binding and code in the consumer?
>
> As far as the clock binding goes, it only describes provider/consumer
> relationships.  The fact that such relationships may resolve to a
> hierarchy is beyond what the binding describes.  If a clock has
> multiple possible parents, then that specific clock binding should
> document how the multiple parent clocks are described and the clock
> driver is responsible for implementing the correct behaviour.

It also deserves to be said that the DT data says nothing about which
of the possible parents _should_ be the input to a mux clock.  It's
pretty common to want to make changes to hierarchy after taking a
device out of reset, since the reset values for a clock management IP
might be pretty conservative.  So someone, somewhere must know some
details about hierarchy and set things up correctly.  Maybe a "clock
driver" can do this, but for specific IPs such as the audio example
below it makes sense for that driver to have the knowledge.

> Similarly, the DT clock binding provides no generic mechanism for
> walking up the clock tree.  That behaviour must also be implemented by
> each specific clock driver.
>
>> I'm thinking of say a system with 1 I2S controller, and both an internal
>> and external I2S clock source, where perhaps the internal source needs
>> to be used to capture from an I2S interface on one set of pins (e.g.
>> analog mic) but the other clock source needs to be used to capture from
>> I2S on another set of pins (e.g. digital baseband unit connection).
>> (This example is theoretical, but I'm sure there are other dynamic clock
>> cases in practice).
>
> That is a reasonable example.  In this case, the i2c controller would
> include both in its clocks property, and the binding would document
> when and why each clock source is used.

I'm confused on this point.  How does the binding "document when and
why each clock is used"?  In the case where this I2S controller
expects to dynamically switch roles at run-time (analog mic versus
baseband) then clk_set_parent must still be invoked by the driver.  To
be clear I'm imagining the above example like:

i_I2S   e_I2S
   \       /
    I2S_mux
        |
I2S controller IP

Regards,
Mike
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/