From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752403Ab2AQXt1 (ORCPT ); Tue, 17 Jan 2012 18:49:27 -0500 Received: from mail-pz0-f46.google.com ([209.85.210.46]:49815 "EHLO mail-pz0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750895Ab2AQXtZ convert rfc822-to-8bit (ORCPT ); Tue, 17 Jan 2012 18:49:25 -0500 MIME-Version: 1.0 In-Reply-To: References: <1323727329-4989-1-git-send-email-grant.likely@secretlab.ca> <1323727329-4989-4-git-send-email-grant.likely@secretlab.ca> <74CDBE0F657A3D45AFBB94109FB122FF17801D2353@HQMAIL01.nvidia.com> From: Grant Likely Date: Tue, 17 Jan 2012 16:49:04 -0700 X-Google-Sender-Auth: NIEO9LMfPGWlCuXMXht5JAzfpPU Message-ID: Subject: Re: [RFC v2 4/9] of: add clock providers To: "Turquette, Mike" Cc: Stephen Warren , "linux-kernel@vger.kernel.org" , "devicetree-discuss@lists.ozlabs.org" , Rob Herring , Sascha Hauer Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Jan 17, 2012 at 4:37 PM, Turquette, Mike wrote: > On Tue, Jan 17, 2012 at 2:47 PM, Grant Likely wrote: >> On Tue, Jan 17, 2012 at 1:44 PM, Stephen Warren 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 >>From the description, I assumed that the i2s_mux was part of the i2s controller driver which would select the appropriate clock based on the configured mode. If it is better to implement it as a separate clock driver, then yes the i2s controller must have knowledge of that and call clk_set_parent appropriately. Regardless, some driver needs to explicitly understand the relationship between pinmux and clock source. g.