linux-kernel - Re: [PATCH v2 3/4] clk: qcom: ipq5332: Use icc-clk for enabling NoC related clocks

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZpjxobF6LZMMN8A9@hu-varada-blr.qualcomm.com>
Date: Thu, 18 Jul 2024 16:12:41 +0530
From: Varadarajan Narayanan <quic_varada@...cinc.com>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
CC: <andersson@...nel.org>, <mturquette@...libre.com>, <sboyd@...nel.org>,
        <robh@...nel.org>, <krzk+dt@...nel.org>, <conor+dt@...nel.org>,
        <gregkh@...uxfoundation.org>, <konrad.dybcio@...aro.org>,
        <djakov@...nel.org>, <quic_wcheng@...cinc.com>,
        <linux-arm-msm@...r.kernel.org>, <linux-clk@...r.kernel.org>,
        <devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <linux-usb@...r.kernel.org>, <linux-pm@...r.kernel.org>
Subject: Re: [PATCH v2 3/4] clk: qcom: ipq5332: Use icc-clk for enabling NoC
 related clocks

On Sat, Jul 13, 2024 at 07:21:29PM +0300, Dmitry Baryshkov wrote:
> On Thu, Jul 11, 2024 at 05:02:38PM GMT, Varadarajan Narayanan wrote:
> > Use the icc-clk framework to enable few clocks to be able to
> > create paths and use the peripherals connected on those NoCs.
> >
> > Signed-off-by: Varadarajan Narayanan <quic_varada@...cinc.com>
> > ---
> >  drivers/clk/qcom/gcc-ipq5332.c | 36 +++++++++++++++++++++++++++++-----
> >  1 file changed, 31 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/clk/qcom/gcc-ipq5332.c b/drivers/clk/qcom/gcc-ipq5332.c
> > index f98591148a97..6d7672cae0f7 100644
> > --- a/drivers/clk/qcom/gcc-ipq5332.c
> > +++ b/drivers/clk/qcom/gcc-ipq5332.c
> > @@ -4,12 +4,14 @@
> >   */
> >
> >  #include <linux/clk-provider.h>
> > +#include <linux/interconnect-provider.h>
> >  #include <linux/mod_devicetable.h>
> >  #include <linux/module.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/regmap.h>
> >
> >  #include <dt-bindings/clock/qcom,ipq5332-gcc.h>
> > +#include <dt-bindings/interconnect/qcom,ipq5332.h>
> >
> >  #include "clk-alpha-pll.h"
> >  #include "clk-branch.h"
> > @@ -131,12 +133,14 @@ static struct clk_alpha_pll gpll4_main = {
> >  			 * (will be added soon), so the clock framework
> >  			 * disables this source. But some of the clocks
> >  			 * initialized by boot loaders uses this source. So we
> > -			 * need to keep this clock ON. Add the
> > -			 * CLK_IGNORE_UNUSED flag so the clock will not be
> > -			 * disabled. Once the consumer in kernel is added, we
> > -			 * can get rid of this flag.
> > +			 * need to keep this clock ON.
> > +			 *
> > +			 * After initial bootup, when the ICC framework turns
> > +			 * off unused paths, as part of the icc-clk dependencies
> > +			 * this clock gets disabled resulting in a hang. Marking
> > +			 * it as critical to ensure it is not turned off.
>
> Previous comment was pretty clear: there are missing consumers, the flag
> will be removed once they are added. Current comment doesn't make sense.
> What is the reason for the device hang if we have all the consumers in
> place?

Earlier, since there were no consumers for this clock, it got
disabled via clk_disable_unused() and CLK_IGNORE_UNUSED helped
prevent that.

Now, since this clk is getting used indirectly via icc-clk
framework, it doesn't qualify for being disabled by
clk_disable_unused(). However, when icc_sync_state is called, if
it sees there are no consumers for a path and that path gets
disabled, the relevant clocks get disabled and eventually this
clock also gets disabled. To avoid this have changed 'flags' from
CLK_IGNORE_UNUSED -> CLK_IS_CRITICAL.

Thanks
Varada

> >  			 */
> > -			.flags = CLK_IGNORE_UNUSED,
> > +			.flags = CLK_IS_CRITICAL,
> >  		},
> >  	},
> >  };
>
>
> --
> With best wishes
> Dmitry