From mboxrd@z Thu Jan 1 00:00:00 1970 From: Masahiro Yamada Date: Fri, 22 Jan 2016 01:48:43 +0900 Subject: [U-Boot] [RFC PATCH 6/6] clk: add fixed rate clock driver In-Reply-To: References: <1450437315-26333-1-git-send-email-yamada.masahiro@socionext.com> <1450437315-26333-7-git-send-email-yamada.masahiro@socionext.com> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 2016-01-20 13:35 GMT+09:00 Simon Glass : > Hi Masahiro, > > On 18 January 2016 at 22:15, Masahiro Yamada > wrote: >> 2015-12-28 23:20 GMT+09:00 Simon Glass : >>> Hi Masahiro, >>> >>> On 18 December 2015 at 04:15, Masahiro Yamada >>> wrote: >>>> This commit intends to implement "fixed-clock" as in Linux. >>>> (drivers/clk/clk-fixed-rate.c in Linux) >>>> >>>> If you need a very simple clock to just provide fixed clock rate >>>> like a crystal oscillator, you do not have to write a new driver. >>>> This driver can support it. >>>> >>>> Note: >>>> As you see in dts/ directories, fixed clocks are often collected in >>>> one container node like this: >>>> >>>> clocks { >>>> refclk_a: refclk_a { >>>> #clock-cells = <0>; >>>> compatible = "fixed-clock"; >>>> clock-frequency = <10000000>; >>>> }; >>>> >>>> refclk_b: refclk_b { >>>> #clock-cells = <0>; >>>> compatible = "fixed-clock"; >>>> clock-frequency = <20000000>; >>>> }; >>>> }; >>>> >>>> This does not work in the current DM of U-Boot, unfortunately. >>>> The "clocks" node must have 'compatible = "simple-bus";' or something >>>> to bind children. >>> >>> I suppose we could explicitly probe the children of the 'clocks' node >>> somewhere. What do you suggest? >>> >>>> >>>> Most of developers would be unhappy about adding such a compatible >>>> string only in U-Boot because we generally want to use the same set >>>> of device trees beyond projects. >>> >>> I'm not sure we need to change it, but if we did, we could try to >>> upstream the change. >>> >>>> >>>> Signed-off-by: Masahiro Yamada >>>> --- >>>> >>>> I do not understand why we need both .get_rate and .get_periph_rate. >>>> >>>> I set both in this driver, but I am not sure if I am doing right. >>> >>> This is to avoid needing a new clock device for every single clock >>> divider in the SoC. For example, it is common to have a PLL be used by >>> 20-30 devices. In U-Boot we can encode the device number as a >>> peripheral ID, Then we can adjust those dividers by settings the >>> clock's rate on a per-peripheral basis. Thus we need only one clock >>> device instead of 20-30. >>> >>> In the case of your clock I think you could return -ENOSYS for >>> get_periph_rate(). >> >> I've just posted v2. >> >> I am still keeping both .get_rate() and .get_periph_rate(). >> >> If I follow your suggestion, each clock consumer must know the >> detail of its clock providers to choose the correct one, >> either .get_rate() or .get_periph_rate(). >> >> Or do you want drivers to do like this? >> >> >> >> clock_cells = clk_get_nr_cells(...); >> >> if (clock_cells == 0) >> rate = clk_get_rate(...); >> else >> rate = clk_get_periph_rate(...); >> >> >> >> For the proper use of these two, the details of clocks must be >> hard-coded in drivers. >> They, of course should be describe in device tree and clock providers. > > In general drivers don't use PLLs directly. So I doubt this case will > arise. Do you have an example? > No, I don't. You commented "In the case of your clock I think you could return -ENOSYS for get_periph_rate().", so I just imagined there is a case where drivers call clk_get_rate(), not clk_get_periph_rate(). -- Best Regards Masahiro Yamada