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: <YgVoM2QZTJAbu6WR@builder.lan>
Date:   Thu, 10 Feb 2022 13:32:03 -0600
From:   Bjorn Andersson <bjorn.andersson@...aro.org>
To:     Stephen Boyd <sboyd@...nel.org>
Cc:     Taniya Das <tdas@...eaurora.org>,
        Michael Turquette <mturquette@...libre.com>,
        Rajendra Nayak <rnayak@...eaurora.org>,
        linux-arm-msm@...r.kernel.org, linux-soc@...r.kernel.org,
        linux-clk@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [v1 1/2] clk: qcom: gdsc: Use the default transition delay for
 GDSCs

On Thu 10 Feb 01:28 CST 2022, Stephen Boyd wrote:

> Quoting Bjorn Andersson (2022-02-09 14:35:08)
> > On Wed 09 Feb 11:25 CST 2022, Taniya Das wrote:
> > 
> > > Do not update the transition delay and use the default reset values.
> > > 
> > > Fixes: 45dd0e55317cc ("clk: qcom: Add support for GDSCs)
> > > Signed-off-by: Taniya Das <tdas@...eaurora.org>
> > > ---
> > >  drivers/clk/qcom/gdsc.c | 6 +++++-
> > >  drivers/clk/qcom/gdsc.h | 1 +
> > >  2 files changed, 6 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
> > > index 7e1dd8ccfa38..e7b213450640 100644
> > > --- a/drivers/clk/qcom/gdsc.c
> > > +++ b/drivers/clk/qcom/gdsc.c
> > > @@ -380,7 +380,11 @@ static int gdsc_init(struct gdsc *sc)
> > >        */
> > >       mask = HW_CONTROL_MASK | SW_OVERRIDE_MASK |
> > >              EN_REST_WAIT_MASK | EN_FEW_WAIT_MASK | CLK_DIS_WAIT_MASK;
> > > -     val = EN_REST_WAIT_VAL | EN_FEW_WAIT_VAL | CLK_DIS_WAIT_VAL;
> > > +
> > > +     regmap_read(sc->regmap, sc->gdscr, &val);
> > > +
> > > +     if (!(sc->flags & DEFAULT_TRANSITION_DELAY))
> > 
> > I dug a little bit more into this and noticed that on various platforms
> > CLK_DIS_WAIT_VAL for the GPU_CX GDSC is supposed to be 8 (whereas both
> > hw default and CLK_DIS_WAIT_VAL is 2).
> > 
> > I'm not able to find anything helpful in the git log describing what the
> > value does, but it seems that a "just use hw default" flag won't cut it
> > for this scenario.
> > 
> 
> I'd prefer we invert the logic so that we don't need to litter this flag
> all over the place. I recall that the wait values were incorrect a long
> time ago on early gdsc using designs but hopefully they've been fixed
> now and we can simply use the default power on reset (POR) values.

Are you suggesting that we make it the default to not update the values
and then provide means to specify it where needed?

I like that suggestion.

But as mentioned in my reply yesterday, GPU_CX on several platforms
needs a different CLK_DIS_WAIT_VAL - and not the same value.

Are these values ever 0? Or could we simply add the three numbers to
struct gdsc and have 0 denote "use hw default"?

Regards,
Bjorn

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ