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;
Feedback-ID: i8771445c:Fastmail
Date:   Mon, 3 Jul 2023 09:25:59 +0200
To:     Frank Oltmanns <frank@oltmanns.dev>
Cc:     Michael Turquette <mturquette@baylibre.com>,
        Stephen Boyd <sboyd@kernel.org>, Chen-Yu Tsai <wens@csie.org>,
        Jernej Skrabec <jernej.skrabec@gmail.com>,
        Samuel Holland <samuel@sholland.org>,
        Andre Przywara <andre.przywara@arm.com>,
        Roman Beranek <me@crly.cz>, linux-clk@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org, linux-sunxi@lists.linux.dev,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 5/8] clk: sunxi-ng: nkm: Support finding closest rate
Message-ID: <zvxviyera3zxe2ro5ej52vz2o2vrinu6zo7osddojbys4tuqhd@3f2okcbpf5jr>
From:   Maxime Ripard <maxime@cerno.tech>
References: <20230702-pll-mipi_set_rate_parent-v3-0-46dcb8aa9cbc@oltmanns.dev>
 <20230702-pll-mipi_set_rate_parent-v3-5-46dcb8aa9cbc@oltmanns.dev>
 <87wmzhsbu0.fsf@oltmanns.dev>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="vstofkqfz34ty6ly"
Content-Disposition: inline
In-Reply-To: <87wmzhsbu0.fsf@oltmanns.dev>
Precedence: bulk


--vstofkqfz34ty6ly
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Mon, Jul 03, 2023 at 09:17:43AM +0200, Frank Oltmanns wrote:
>=20
> On 2023-07-02 at 19:55:24 +0200, Frank Oltmanns <frank@oltmanns.dev> wrot=
e:
> > When finding the best rate for a NKM clock, consider rates that are
> > higher than the requested rate, if the CCU_FEATURE_CLOSEST_RATE flag is
> > set.
> >
> > Accommodate ccu_mux_helper_determine_rate to this change.
> >
> > Signed-off-by: Frank Oltmanns <frank@oltmanns.dev>
> > ---
> >  drivers/clk/sunxi-ng/ccu_mux.c | 23 +++++++++++++++-----
> >  drivers/clk/sunxi-ng/ccu_nkm.c | 48 +++++++++++++++++++++++++++++++---=
--------
> >  2 files changed, 54 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/clk/sunxi-ng/ccu_mux.c b/drivers/clk/sunxi-ng/ccu_=
mux.c
> > index 1d557e323169..8594d6a4addd 100644
> > --- a/drivers/clk/sunxi-ng/ccu_mux.c
> > +++ b/drivers/clk/sunxi-ng/ccu_mux.c
> > @@ -113,7 +113,7 @@ int ccu_mux_helper_determine_rate(struct ccu_common=
 *common,
> >  	}
> >
> >  	for (i =3D 0; i < clk_hw_get_num_parents(hw); i++) {
> > -		unsigned long tmp_rate, parent_rate;
> > +		unsigned long tmp_rate, parent_rate, best_diff =3D ULONG_MAX;
> >  		struct clk_hw *parent;
> >
> >  		parent =3D clk_hw_get_parent_by_index(hw, i);
> > @@ -139,10 +139,23 @@ int ccu_mux_helper_determine_rate(struct ccu_comm=
on *common,
> >  			goto out;
> >  		}
> >
> > -		if ((req->rate - tmp_rate) < (req->rate - best_rate)) {
> > -			best_rate =3D tmp_rate;
> > -			best_parent_rate =3D parent_rate;
> > -			best_parent =3D parent;
> > +		if (common->features & CCU_FEATURE_CLOSEST_RATE) {
> > +			unsigned long tmp_diff =3D req->rate > tmp_rate ?
> > +						 req->rate - tmp_rate :
> > +						 tmp_rate - req->rate;
> > +
> > +			if (tmp_diff < best_diff) {
> > +				best_rate =3D tmp_rate;
> > +				best_parent_rate =3D parent_rate;
> > +				best_parent =3D parent;
> > +				best_diff =3D tmp_diff;
> > +			}
> > +		} else {
> > +			if ((req->rate - tmp_rate) < (req->rate - best_rate)) {
> > +				best_rate =3D tmp_rate;
> > +				best_parent_rate =3D parent_rate;
> > +				best_parent =3D parent;
> > +			}
> >  		}
> >  	}
> >
> > diff --git a/drivers/clk/sunxi-ng/ccu_nkm.c b/drivers/clk/sunxi-ng/ccu_=
nkm.c
> > index d83843e69c25..36d9e987e4d8 100644
> > --- a/drivers/clk/sunxi-ng/ccu_nkm.c
> > +++ b/drivers/clk/sunxi-ng/ccu_nkm.c
> > @@ -18,9 +18,11 @@ struct _ccu_nkm {
> >  };
> >
> >  static unsigned long ccu_nkm_find_best_with_parent_adj(unsigned long *=
parent, unsigned long rate,
> > -						       struct _ccu_nkm *nkm, struct clk_hw *phw)
> > +						       struct _ccu_nkm *nkm, struct clk_hw *phw,
> > +						       unsigned long features)
> >  {
> > -	unsigned long best_rate =3D 0, best_parent_rate =3D *parent, tmp_pare=
nt =3D *parent;
> > +	unsigned long best_rate =3D 0, best_parent_rate =3D 0, tmp_parent =3D=
 *parent;
> > +	unsigned long best_diff =3D ULONG_MAX;
> >  	unsigned long best_n =3D 0, best_k =3D 0, best_m =3D 0;
> >  	unsigned long _n, _k, _m;
> >
> > @@ -28,16 +30,26 @@ static unsigned long ccu_nkm_find_best_with_parent_=
adj(unsigned long *parent, un
> >  		for (_n =3D nkm->min_n; _n <=3D nkm->max_n; _n++) {
> >  			for (_m =3D nkm->min_m; _m <=3D nkm->max_m; _m++) {
> >  				unsigned long tmp_rate;
> > +				unsigned long tmp_diff;
> >
> >  				tmp_parent =3D clk_hw_round_rate(phw, rate * _m / (_n * _k));
> >
> >  				tmp_rate =3D tmp_parent * _n * _k / _m;
> > -				if (tmp_rate > rate)
> > -					continue;
> >
> > -				if ((rate - tmp_rate) < (rate - best_rate)) {
> > +				if (features & CCU_FEATURE_CLOSEST_RATE) {
> > +					tmp_diff =3D rate > tmp_rate ?
> > +						   rate - tmp_rate :
> > +						   tmp_rate - rate;
> > +				} else {
> > +					if (tmp_rate > rate)
> > +						continue;
> > +					tmp_diff =3D rate - tmp_diff;
>=20
> Sorry, this should of course be tmp_diff =3D rate - tmp_rate. I'll fix
> that in v4. Also I'll do tests on my phone where
> CCU_FEATURE_CLOSEST_RATE is not set (i.e., without PATCH 8), so see if
> it replicates the old behaviour. I'll also look into adding kunit tests,
> so that this doesn't happen again. I'm not sure if this is feasible, but
> I'll ask here for advise, if/when I encounter obstacles.

While this would obviously be great, I don't think we have the
infrastructure just yet to allow to easily add kunit tests for entire
clocks.

Maxime

--vstofkqfz34ty6ly
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iHUEABYKAB0WIQRcEzekXsqa64kGDp7j7w1vZxhRxQUCZKJ4BwAKCRDj7w1vZxhR
xT/EAQCAf+wzsG4PBtQDf9TUqpzI61mOL0/7PFIInvCwhd3hjAD/apsd946lQXsb
lqSSwED7GsHO1pZfkFY7qO5BQ8IyXQk=
=vCWb
-----END PGP SIGNATURE-----

--vstofkqfz34ty6ly--