Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <648af616.1c0a0220.f8289.b7a9@mx.google.com>
Date:   Thu, 15 Jun 2023 13:29:24 +0200
From:   Christian Marangi <ansuelsmth@gmail.com>
To:     Stephen Boyd <sboyd@kernel.org>
Cc:     Konrad Dybcio <konrad.dybcio@linaro.org>,
        Andy Gross <agross@kernel.org>,
        Bjorn Andersson <andersson@kernel.org>,
        Michael Turquette <mturquette@baylibre.com>,
        linux-arm-msm@vger.kernel.org, linux-clk@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4 2/3] clk: qcom: clk-rcg2: add support for rcg2 freq
 multi ops
References: <20230427150717.20860-1-ansuelsmth@gmail.com>
 <20230427150717.20860-3-ansuelsmth@gmail.com>
 <82072c2b-8483-6fb6-a9d1-c9882825c9cb@linaro.org>
 <6473e34c.df0a0220.33a79.6c95@mx.google.com>
 <4afbcdd0-a11c-4826-d669-2ffc9488a8b6@linaro.org>
 <64749bf4.5d0a0220.325c0.64b5@mx.google.com>
 <d251be3e02b2fe28357c884e39fe7601.sboyd@kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <d251be3e02b2fe28357c884e39fe7601.sboyd@kernel.org>
Precedence: bulk

On Wed, Jun 14, 2023 at 05:28:08PM -0700, Stephen Boyd wrote:
> 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.

Hi, in theory the WAN path should never happen, as it means no parent
for the clk are present. So I guess with your logic printing a WARN is
correct.

About the rounding policy this is more or less problematic, it's a
CLOSEST policy, so not a CEIL or FLOOR one. The determine_rate apply the
very same selection logic of set_rate so also that should be good.

I sent v5 some time ago with the concern here addressed so if you have
time it would be good if you can check that.

-- 
	Ansuel