From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932088Ab2EUSbG (ORCPT ); Mon, 21 May 2012 14:31:06 -0400 Received: from mail-yx0-f174.google.com ([209.85.213.174]:48984 "EHLO mail-yx0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750728Ab2EUSbE (ORCPT ); Mon, 21 May 2012 14:31:04 -0400 Message-ID: <4FBA89E3.7010106@gmail.com> Date: Mon, 21 May 2012 13:30:59 -0500 From: Rob Herring User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:12.0) Gecko/20120430 Thunderbird/12.0.1 MIME-Version: 1.0 To: Shawn Guo CC: Shawn Guo , Mike Turquette , Grant Likely , "arm@kernel.org" , "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" Subject: Re: [GIT PULL] DT clk binding support References: <4FB80F32.5090309@gmail.com> <20120520030653.GB5810@S2100-06.ap.freescale.net> <4FB9A5E7.2070000@gmail.com> <20120521064901.GE8140@S2101-09.ap.freescale.net> In-Reply-To: <20120521064901.GE8140@S2101-09.ap.freescale.net> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 05/21/2012 01:49 AM, Shawn Guo wrote: > On Sun, May 20, 2012 at 09:18:15PM -0500, Rob Herring wrote: >>> As I said, any clock in the clock tree except root clock is not only >>> a clock provider but also a consumer. If you define "clocks" as a >>> required property for clock consumers, you are essentially asking users >>> to either define the whole clock tree in the device tree or stay away >>> from device tree completely. >> >> So what are you proposing that a clock consumer have? The very >> definition of a clock consumer is that it has a clocks property. >> > To support the cases that the clock tree is defined by clock driver > and device tree together, the "clocks" property could be reasonably > absent in case the parent clock (provider) is being defined in driver > than DT. And for such clocks, the "clock-names" than "clocks" should > be required to find the clock provider/parent. > > I do not like the idea to look for clock with name too much, but I > do not see other way around to support those platforms that have clock > tree definition split in clock driver and device tree. > >>> >>> Are you sure this is the right thing to do? If I remember correctly, >>> Grant's position is it should be pretty reasonable to have most of >>> the clock tree defined in clock driver and only define those leaf >>> clocks which are very likely to become the clock providers for other >>> peripherals. >> >> The minimum is you have to have a provider and consumer. It may be a >> single provider that provides all clocks for a chip. If you don't want a >> provider, then just define a clock-frequency property. >>> >>> Let me put a terrible example here. Since clock tree is actually SoC >>> specific, I can reasonably choose to define the entire imx6q clock tree >>> and all the clk lookups for imx6q peripherals in clk-imx6q driver. >>> On imx6q-sabrelite board, the audio codec sgtl5000 uses cko (an imx6q >>> clock available on pad) as the clock source. That said, I need a board >>> specific clk lookup here, which should be the best user of clock DT >>> bindings. But sadly, with the current bindings, I can not give the >>> required "clocks" property for sgtl5000 node. >> >> I don't understand your example. For the sgtl5000 on the sabrelite, you >> would provide a phandle and cell entry that is interpreted as the cko >> pin. > > With the bindings here, I need something like below in device tree to > replace the clk lookup registration that is currently done in imx6q > sabrelite specific setup code. However the problem here is I have cko > defined in clock driver, and thus I can not give phandle to cko in > device tree. What I suggest is for such cases, we could require > clock-names = "cko" than clocks = <&cko>, and of_clk_get() should also > be able to find the clock with looking for the clk name. > > imx6q-sabrelite.dts: > > codec: sgtl5000@0a { > compatible = "fsl,sgtl5000"; > reg = <0x0a>; > clocks = <&cko>; > }; > > mach-imx6q.c, imx6q_sabrelite_cko1_setup(): > > cko1 = clk_get_sys(NULL, "cko1"); > clk_register_clkdev(cko1, NULL, "0-000a"); > What!? This is a terrible abuse/hack of the clock binding and is in no way what was intended. You cannot use half of the clock binding. You have to have a provider. The primary binding is a phandle reference. clock-names is just auxiliary data. Rob From mboxrd@z Thu Jan 1 00:00:00 1970 From: robherring2@gmail.com (Rob Herring) Date: Mon, 21 May 2012 13:30:59 -0500 Subject: [GIT PULL] DT clk binding support In-Reply-To: <20120521064901.GE8140@S2101-09.ap.freescale.net> References: <4FB80F32.5090309@gmail.com> <20120520030653.GB5810@S2100-06.ap.freescale.net> <4FB9A5E7.2070000@gmail.com> <20120521064901.GE8140@S2101-09.ap.freescale.net> Message-ID: <4FBA89E3.7010106@gmail.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 05/21/2012 01:49 AM, Shawn Guo wrote: > On Sun, May 20, 2012 at 09:18:15PM -0500, Rob Herring wrote: >>> As I said, any clock in the clock tree except root clock is not only >>> a clock provider but also a consumer. If you define "clocks" as a >>> required property for clock consumers, you are essentially asking users >>> to either define the whole clock tree in the device tree or stay away >>> from device tree completely. >> >> So what are you proposing that a clock consumer have? The very >> definition of a clock consumer is that it has a clocks property. >> > To support the cases that the clock tree is defined by clock driver > and device tree together, the "clocks" property could be reasonably > absent in case the parent clock (provider) is being defined in driver > than DT. And for such clocks, the "clock-names" than "clocks" should > be required to find the clock provider/parent. > > I do not like the idea to look for clock with name too much, but I > do not see other way around to support those platforms that have clock > tree definition split in clock driver and device tree. > >>> >>> Are you sure this is the right thing to do? If I remember correctly, >>> Grant's position is it should be pretty reasonable to have most of >>> the clock tree defined in clock driver and only define those leaf >>> clocks which are very likely to become the clock providers for other >>> peripherals. >> >> The minimum is you have to have a provider and consumer. It may be a >> single provider that provides all clocks for a chip. If you don't want a >> provider, then just define a clock-frequency property. >>> >>> Let me put a terrible example here. Since clock tree is actually SoC >>> specific, I can reasonably choose to define the entire imx6q clock tree >>> and all the clk lookups for imx6q peripherals in clk-imx6q driver. >>> On imx6q-sabrelite board, the audio codec sgtl5000 uses cko (an imx6q >>> clock available on pad) as the clock source. That said, I need a board >>> specific clk lookup here, which should be the best user of clock DT >>> bindings. But sadly, with the current bindings, I can not give the >>> required "clocks" property for sgtl5000 node. >> >> I don't understand your example. For the sgtl5000 on the sabrelite, you >> would provide a phandle and cell entry that is interpreted as the cko >> pin. > > With the bindings here, I need something like below in device tree to > replace the clk lookup registration that is currently done in imx6q > sabrelite specific setup code. However the problem here is I have cko > defined in clock driver, and thus I can not give phandle to cko in > device tree. What I suggest is for such cases, we could require > clock-names = "cko" than clocks = <&cko>, and of_clk_get() should also > be able to find the clock with looking for the clk name. > > imx6q-sabrelite.dts: > > codec: sgtl5000 at 0a { > compatible = "fsl,sgtl5000"; > reg = <0x0a>; > clocks = <&cko>; > }; > > mach-imx6q.c, imx6q_sabrelite_cko1_setup(): > > cko1 = clk_get_sys(NULL, "cko1"); > clk_register_clkdev(cko1, NULL, "0-000a"); > What!? This is a terrible abuse/hack of the clock binding and is in no way what was intended. You cannot use half of the clock binding. You have to have a provider. The primary binding is a phandle reference. clock-names is just auxiliary data. Rob