linux-kernel - Re: [PATCH v4 2/3] clk: qcom: clk-rcg2: add support for rcg2 freq multi ops

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]

Date:   Wed, 14 Jun 2023 17:28:08 -0700
From:   Stephen Boyd <sboyd@...nel.org>
To:     Christian Marangi <ansuelsmth@...il.com>,
        Konrad Dybcio <konrad.dybcio@...aro.org>
Cc:     Andy Gross <agross@...nel.org>,
        Bjorn Andersson <andersson@...nel.org>,
        Michael Turquette <mturquette@...libre.com>,
        linux-arm-msm@...r.kernel.org, linux-clk@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 2/3] clk: qcom: clk-rcg2: add support for rcg2 freq multi ops

Quoting Christian Marangi (2023-05-29 05:34:57)
> On Mon, May 29, 2023 at 02:12:23PM +0200, Konrad Dybcio wrote:
> > On 28.05.2023 14:37, Christian Marangi wrote:
> > > On Sat, May 27, 2023 at 06:11:16PM +0200, Konrad Dybcio wrote:
> > >> On 27.04.2023 17:07, Christian Marangi wrote:
> > >>> +  * Force the first conf if we can't find a correct config.
> > >>> +  */
> > >>> + if (unlikely(i == f->num_confs))
> > >>> +         best_conf = f->confs;
> > >> Is that a supported scenario or would it be a device driver / clock
> > >> driver error?
> > >>
> > > 
> > > It's to handle case for the 2 continue in the loop and arriving in a
> > > situation where best_conf was never set?
> > > 
> > > Should we return a warning and an ERR_PTR? Idea was to provide a best
> > > effort selection.
> > Hm.. I'm not sure what's the expected behavior here.. Stephen?
> > 
> 
> I have this implementation rady, if you want I can send this revision
> and discuss that in v5 directly. It's WARN and returning -EINVAL.

I'd only have a WARN if you never expect to hit that case. Otherwise, it
should return -EINVAL and not warn. At a quick glance it sounds like
some sort of rounding policy, so just make sure the
round_rate/determine_rate implementation agrees with what set_rate()
will do and it should be good.