From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lee Jones Subject: Re: [STLinux Kernel] [PATCH 3/4] clk: Provide always-on clock support Date: Mon, 2 Mar 2015 10:32:28 +0000 Message-ID: <20150302103228.GD32347@x1> References: <1425071674-16995-1-git-send-email-lee.jones@linaro.org> <1425071674-16995-4-git-send-email-lee.jones@linaro.org> <20150302083652.GE31325@x1> <20150302101859.GB32347@x1> <54F43A98.509@st.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: Content-Disposition: inline In-Reply-To: <54F43A98.509@st.com> Sender: linux-kernel-owner@vger.kernel.org To: Maxime Coquelin Cc: Jassi Brar , Devicetree List , Mike Turquette , kernel@stlinux.com, Stephen Boyd , lkml , Jassi Brar , "linux-arm-kernel@lists.infradead.org" List-Id: devicetree@vger.kernel.org On Mon, 02 Mar 2015, Maxime Coquelin wrote: > On 03/02/2015 11:18 AM, Lee Jones wrote: > >On Mon, 02 Mar 2015, Jassi Brar wrote: > > > >>On Mon, Mar 2, 2015 at 2:06 PM, Lee Jones wr= ote: > >>>On Sat, 28 Feb 2015, Jassi Brar wrote: > >>> > >>>>On 28 February 2015 at 02:44, Lee Jones wr= ote: > >>>>>Lots of platforms contain clocks which if turned off would prove= fatal. > >>>>>The only way to recover from these catastrophic failures is to r= estart > >>>>>the board(s). Now, when a clock is registered with the framewor= k it is > >>>>>compared against a list of provided always-on clock names which = must be > >>>>>kept ungated. If it matches, we enable the existing CLK_IGNORE_= UNUSED > >>>>>flag, which will prevent the common clk framework from attemptin= g to > >>>>>gate it during the clk_disable_unused() procedure. > >>>>> > >>>>If a clock is critical on a certain board, it could be got+enable= d > >>>>during early boot so there is always a user. > >>>I tried this. There was push-back from the DT maintainers. > >>> > >>> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-Febr= uary/324417.html > >>> > >>Thanks, I wasn't aware of the history. > >> > >>>>To be able to do that from DT, maybe add a new, say, CLK_ALWAYS_O= N > >>>>flag could be made to initialize the clock with one phantom user > >>>>already. Or just reuse the CLK_IGNORE_UNUSED? > >>>How is that different to what this set is doing? > >>> > >>The phantom user - that's there but none can see it. > >> > >>How about? > >> > >>+ of_property_for_each_string(np, "clock-always-on", prop, cl= kname) { > >>+ clk =3D __clk_lookup(clkname); > >>+ if (!clk) > >>+ continue; > >>+ > >>+ clk->core->enable_count =3D 1; > >>+ clk->core->prepare_count =3D 1; > >>+ } > >This is only fractionally different from the current implementation. > > > >I believe the current way it slightly nicer, as we don't have to fak= e > >the user count. This solution is saying "one of the drivers is stil= l > >consuming this clock", instead, in the original implementation we're > >saying "we know there are no consumers of this clock, but keep it on > >anyway due to [insert reason here]". > > > So maybe introducing a new "CLK_DISABLE_NEVER" flag will be more > explicit than hacking around "CLK_IGNORE_UNUSED" one? I am okay with that if Mike is; however, personally I think CLK_IGNORE_UNUSED is pretty descriptive. If clocks are unused (reference count reaches zero) the framework will disable the clock -- fair enough. So in order to prevent that from happening in cases where these clocks are used by entities other than device drivers (perhaps these devices are even invisible to Linux) we have to tell the framework to "ignore the fact that this clock _appears_ to be unused". Hence CLK_IGNORE_UNUSED. I'm struggling to think of a solid reason for inventing new flags when we have one that is perfectly suitable/descriptive. --=20 Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org =E2=94=82 Open source software for ARM SoCs =46ollow Linaro: Facebook | Twitter | Blog