From mboxrd@z Thu Jan 1 00:00:00 1970 From: Masahiro Yamada Date: Tue, 19 Jan 2016 14:15:33 +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 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. -- Best Regards Masahiro Yamada