From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1759286Ab2EWBip (ORCPT ); Tue, 22 May 2012 21:38:45 -0400 Received: from wolverine01.qualcomm.com ([199.106.114.254]:36352 "EHLO wolverine01.qualcomm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754165Ab2EWBio (ORCPT ); Tue, 22 May 2012 21:38:44 -0400 X-IronPort-AV: E=McAfee;i="5400,1158,6719"; a="193731512" Message-ID: <49cd509b59c8172c880b0c8a50c0f02c.squirrel@www.codeaurora.org> In-Reply-To: <4FBB9A08.2050803@gmail.com> References: <4FB80F32.5090309@gmail.com> <20120520030653.GB5810@S2100-06.ap.freescale.net> <4FB9A5E7.2070000@gmail.com> <20120521064901.GE8140@S2101-09.ap.freescale.net> <4FBA89E3.7010106@gmail.com> <20120521232616.GF8140@S2101-09.ap.freescale.net> <4FBAD545.7060803@gmail.com> <20120522021535.GG8140@S2101-09.ap.freescale.net> <4FBB134D.6050409@codeaurora.org> <4FBB9A08.2050803@gmail.com> Date: Tue, 22 May 2012 18:38:44 -0700 (PDT) Subject: Re: [GIT PULL] DT clk binding support From: "Saravana Kannan" To: "Rob Herring" Cc: "Stephen Boyd" , "Shawn Guo" , "Mike Turquette" , "linux-kernel@vger.kernel.org" , "Grant Likely" , "arm@kernel.org" , "Shawn Guo" , "linux-arm-kernel@lists.infradead.org" , "Saravana Kannan" User-Agent: SquirrelMail/1.4.17 MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7BIT X-Priority: 3 (Normal) Importance: Normal Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, May 22, 2012 6:52 am, Rob Herring wrote: > On 05/21/2012 11:17 PM, Stephen Boyd wrote: >> On 05/21/12 19:15, Shawn Guo wrote: >>> On Mon, May 21, 2012 at 06:52:37PM -0500, Rob Herring wrote: >>>> As Grant states: "This proposed binding is only about one thing: >>>> attaching clock providers to clock consumers." This means you have to >>>> have at least a single provider and a single consumer defined in the >>>> DT. >>>> >>> I just read through Grant's comments over again. I agree with the >>> statement which implicitly requires the clk provider defined in DT. >>> However, for some case, this provider in DT is just a skeleton which >>> is backed by clock driver where the provider is actually defined. >>> >>> Looking at Grant's comment below, the second option is also to match >>> the clock in driver just using name. The only difference to my >>> proposal is the name here is given by the argument of phandle pointing >>> to that skeleton provider node. >>> >>> I'm fine with that. So go ahead with your bindings. >>> >> >> Can we do what the regulator framework has done and have a common >> binding of -clk = <&phandle>? Something like: >> >> core-clk = <&uart3_clk> >> >> and then have clk_get() use the of node of the device passed in to find >> a property named %s-clk and find the clock with the matching phandle. > > Sigh... That is what we had in previous versions from over a year ago > and we moved away from that approach. The current binding has been > reviewed multiple times in the last 6 months... > > The current approach is aligned with how interrupts are handled (with > the addition of a phandle). I think not having per clock property names > is easier to parse and easier to document. > >> This looks like it's trying to cover both the end consumers (uart uses >> uart3_clk) and the internal clock tree consumers (a crystal oscillator >> connects to a PLL or a mux has multiple parents). We can certainly use >> these bindings for muxes and internal parent-child relationships but I >> would prefer we use different bindings for consumer bindings that match >> what regulators do today. > > The binding supports either defining every last internal clock or just > the leaf clocks. I took the former route on highbank since I don't have > a lot of clocks. If I was doing imx or omap for example, I'd probably > just define all the clock controller outputs. > If only the leaf nodes are defined in DT, then how is the clock platform driver implementer supposed to instantiate the rest of the tree and connect it up with the partial list of clocks in DT? So, they have to switch back and forth between DT and the .c file which defines the rest and make sure the parent<->child names match? To me it looks that it might better to decouple the description of the clock HW from the mapping of a clock leaf to a consumer device. If we just use a string to identify the clock that's consumed by a device, we can achieve this decoupling at a clean boundary -- clock consumers devices (UART) vs clock producer devices (clock controller in the SoC, in a PMIC, audio codec, etc). With the decoupling, we don't have the inconsistency of having some of the clocks of a clock producer device incompletely defined in DT and the rest of the clocks of the same clock producer device hard coded in the kernel. So, you either put your entire clock tree in the SoC in the DT or put all of it in the kernel but you aren't forced to put just some of them in the DT just to get DT working. I see no benefit in defining only some of the clocks in DT -- it just adds more confusion in the clock tree definition. What am I missing? Thanks, Saravana -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum. From mboxrd@z Thu Jan 1 00:00:00 1970 From: skannan@codeaurora.org (Saravana Kannan) Date: Tue, 22 May 2012 18:38:44 -0700 (PDT) Subject: [GIT PULL] DT clk binding support In-Reply-To: <4FBB9A08.2050803@gmail.com> References: <4FB80F32.5090309@gmail.com> <20120520030653.GB5810@S2100-06.ap.freescale.net> <4FB9A5E7.2070000@gmail.com> <20120521064901.GE8140@S2101-09.ap.freescale.net> <4FBA89E3.7010106@gmail.com> <20120521232616.GF8140@S2101-09.ap.freescale.net> <4FBAD545.7060803@gmail.com> <20120522021535.GG8140@S2101-09.ap.freescale.net> <4FBB134D.6050409@codeaurora.org> <4FBB9A08.2050803@gmail.com> Message-ID: <49cd509b59c8172c880b0c8a50c0f02c.squirrel@www.codeaurora.org> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Tue, May 22, 2012 6:52 am, Rob Herring wrote: > On 05/21/2012 11:17 PM, Stephen Boyd wrote: >> On 05/21/12 19:15, Shawn Guo wrote: >>> On Mon, May 21, 2012 at 06:52:37PM -0500, Rob Herring wrote: >>>> As Grant states: "This proposed binding is only about one thing: >>>> attaching clock providers to clock consumers." This means you have to >>>> have at least a single provider and a single consumer defined in the >>>> DT. >>>> >>> I just read through Grant's comments over again. I agree with the >>> statement which implicitly requires the clk provider defined in DT. >>> However, for some case, this provider in DT is just a skeleton which >>> is backed by clock driver where the provider is actually defined. >>> >>> Looking at Grant's comment below, the second option is also to match >>> the clock in driver just using name. The only difference to my >>> proposal is the name here is given by the argument of phandle pointing >>> to that skeleton provider node. >>> >>> I'm fine with that. So go ahead with your bindings. >>> >> >> Can we do what the regulator framework has done and have a common >> binding of -clk = <&phandle>? Something like: >> >> core-clk = <&uart3_clk> >> >> and then have clk_get() use the of node of the device passed in to find >> a property named %s-clk and find the clock with the matching phandle. > > Sigh... That is what we had in previous versions from over a year ago > and we moved away from that approach. The current binding has been > reviewed multiple times in the last 6 months... > > The current approach is aligned with how interrupts are handled (with > the addition of a phandle). I think not having per clock property names > is easier to parse and easier to document. > >> This looks like it's trying to cover both the end consumers (uart uses >> uart3_clk) and the internal clock tree consumers (a crystal oscillator >> connects to a PLL or a mux has multiple parents). We can certainly use >> these bindings for muxes and internal parent-child relationships but I >> would prefer we use different bindings for consumer bindings that match >> what regulators do today. > > The binding supports either defining every last internal clock or just > the leaf clocks. I took the former route on highbank since I don't have > a lot of clocks. If I was doing imx or omap for example, I'd probably > just define all the clock controller outputs. > If only the leaf nodes are defined in DT, then how is the clock platform driver implementer supposed to instantiate the rest of the tree and connect it up with the partial list of clocks in DT? So, they have to switch back and forth between DT and the .c file which defines the rest and make sure the parent<->child names match? To me it looks that it might better to decouple the description of the clock HW from the mapping of a clock leaf to a consumer device. If we just use a string to identify the clock that's consumed by a device, we can achieve this decoupling at a clean boundary -- clock consumers devices (UART) vs clock producer devices (clock controller in the SoC, in a PMIC, audio codec, etc). With the decoupling, we don't have the inconsistency of having some of the clocks of a clock producer device incompletely defined in DT and the rest of the clocks of the same clock producer device hard coded in the kernel. So, you either put your entire clock tree in the SoC in the DT or put all of it in the kernel but you aren't forced to put just some of them in the DT just to get DT working. I see no benefit in defining only some of the clocks in DT -- it just adds more confusion in the clock tree definition. What am I missing? Thanks, Saravana -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.