From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mike Turquette Subject: Re: [PATCH v2 3/4] clk: Provide an always-on clock domain framework Date: Wed, 25 Feb 2015 10:26:29 -0800 Message-ID: <20150225182629.421.37835@quantum> References: <1424276101-30137-1-git-send-email-lee.jones@linaro.org> <1424276101-30137-4-git-send-email-lee.jones@linaro.org> <20150223172344.421.62815@quantum> <20150225154808.GD6688@x1> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: In-Reply-To: <20150225154808.GD6688@x1> Sender: linux-kernel-owner@vger.kernel.org To: Lee Jones , Rob Herring Cc: "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" , Stephen Boyd , kernel@stlinux.com, "devicetree@vger.kernel.org" List-Id: devicetree@vger.kernel.org Quoting Lee Jones (2015-02-25 07:48:08) > On Wed, 25 Feb 2015, Rob Herring wrote: >=20 > > On Mon, Feb 23, 2015 at 11:23 AM, Mike Turquette wrote: > > > Quoting Lee Jones (2015-02-18 08:15:00) > > >> Much h/w contain clocks which if turned off would prove fatal. = The > > >> only way to recover is to restart the board(s). This driver tak= es > > >> references to clocks which are required to be always-on in order= to > > >> prevent the common clk framework from trying to turn them off du= ring > > >> the clk_disabled_unused() procedure. > >=20 > > [...] > >=20 > > >> +static int ao_clock_domain_probe(struct platform_device *pdev) > > >> +{ > > >> + struct device_node *np =3D pdev->dev.of_node; > > >> + int nclks, i; > > >> + > > >> + nclks =3D of_count_phandle_with_args(np, "clocks", "#clo= ck-cells"); > > > > > > Minor nitpick: please use of_clk_get_parent_count. I spent a soli= d 5 > > > minutes writing that function and I need people to use it so I ca= n get a > > > return on my investment. > > > > > > Otherwise the patch looks good. I believe that this method is tar= geting > > > always-on clock in a production environment, which is different f= rom the > > > CLK_IGNORE_UNUSED stuff which typically is helpful while bringing= up new > > > hardware or dealing with a platform that has incomplete driver su= pport. > >=20 > > There is also the usecase of keep clocks on until I load a module t= hat > > properly handles my hardware (e.g simplefb). We have a simplefb nod= e > > with clocks and the simplefb driver jumps thru some hoops to hand-o= ff > > clocks to the real driver. I don't really like it and don't want to > > see more examples. And there is the case of I thought I would never > > manage this clock, but kernel subsystems evolve and now I want to > > manage a clock. This should not require a DT update to do so. > >=20 > > Neither of these may be Lee's usecase, but I want to see them cover= ed > > by the binding. > >=20 > > > I wonder if there is a clever way for existing clock providers > > > (expressed in DT) to use this without having to create a separate= node > > > of clocks with the "always-on-clk-domain" flag. Possibly the comm= on > > > clock binding could declare some always-on flag that is standardi= zed? > > > Then the framework core could use this code easily. Not sure if t= hat is > > > a good idea though... > >=20 > > I would prefer to see the always on clocks just listed within the > > clock controller's node rather than creating made up nodes with clo= ck > > properties. >=20 > > This should be always-on until claimed IMO, but that > > aspect is the OS's problem, not a DT problem. >=20 > I disagree with this point. There are likely to be many unclaimed, > but perfectly gateable clocks in a system, which will consume power > unnecessarily. The clk framework does the right thing by turning all > unclaimed clocks off IMHO. This only leaves a small use-case where w= e > need to artificially claim some which must not be gated. I might have misread both of your mails, but I think you two are actually in agreement. You both support a common property which lists the always-on clocks inside of the common clock binding, no? >=20 > The other way to do is, as you mentioned is list the clocks which mus= t > stay on in the clock source node, but this will still require a > binding. It will also require a much more complicated framework > driver. >=20 > clkprovider@xxxxxxxx { > always-on-clks =3D <1, 2, 4, 5, 7>; > }; This should pose no burden on the driver. Since always-on-clks is in th= e common clock binding it should be handled by the framework core. At clk_register-time we can check for always-on-clks, walk the list and se= e if we have a match. It's ugly O(n^2) but it works. Thoughts? Mike > --=20 > Lee Jones > Linaro STMicroelectronics Landing Team Lead > Linaro.org =E2=94=82 Open source software for ARM SoCs > Follow Linaro: Facebook | Twitter | Blog