Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.34 as permitted sender) client-ip=23.128.96.34;
Message-ID: <3ff911fa4c417a81462813f975ac757f.sboyd@kernel.org>
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
In-Reply-To: <20231016113002.15929-3-jay.buddhabhatti@amd.com>
References: <20231016113002.15929-1-jay.buddhabhatti@amd.com> <20231016113002.15929-3-jay.buddhabhatti@amd.com>
Subject: Re: [PATCH RESEND 2/2] drivers: clk: zynqmp: update divider round rate logic
From:   Stephen Boyd <sboyd@kernel.org>
Cc:     linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        linux-kernel@vger.kernel.org,
        Jay Buddhabhatti <jay.buddhabhatti@amd.com>
To:     Jay Buddhabhatti <jay.buddhabhatti@amd.com>, michal.simek@amd.com,
        mturquette@baylibre.com
Date:   Mon, 23 Oct 2023 20:37:49 -0700
User-Agent: alot/0.10
Precedence: bulk

Quoting Jay Buddhabhatti (2023-10-16 04:30:02)
> Currently zynqmp divider round rate is considering single parent and
> calculating rate and parent rate accordingly. But if divider clock flag
> is set to SET_RATE_PARENT then its not trying to traverse through all
> parent rate and not selecting best parent rate from that. So use common
> divider_round_rate() which is traversing through all clock parents and
> its rate and calculating proper parent rate.
>=20
> Signed-off-by: Jay Buddhabhatti <jay.buddhabhatti@amd.com>
> ---
>  drivers/clk/zynqmp/divider.c | 70 ++++++------------------------------
>  1 file changed, 10 insertions(+), 60 deletions(-)

Can't argue against removing that many lines!

>=20
> diff --git a/drivers/clk/zynqmp/divider.c b/drivers/clk/zynqmp/divider.c
> index 33a3b2a22659..a42c183d7e5d 100644
> --- a/drivers/clk/zynqmp/divider.c
> +++ b/drivers/clk/zynqmp/divider.c
> @@ -193,23 +149,17 @@ static long zynqmp_clk_divider_round_rate(struct cl=
k_hw *hw,
>                 return DIV_ROUND_UP_ULL((u64)*prate, bestdiv);
>         }
> =20
> -       bestdiv =3D zynqmp_divider_get_val(*prate, rate, divider->flags);
> -
> -       /*
> -        * In case of two divisors, compute best divider values and return
> -        * divider2 value based on compute value. div1 will  be automatic=
ally
> -        * set to optimum based on required total divider value.
> -        */
> -       if (div_type =3D=3D TYPE_DIV2 &&
> -           (clk_hw_get_flags(hw) & CLK_SET_RATE_PARENT)) {
> -               zynqmp_get_divider2_val(hw, rate, divider, &bestdiv);
> +       max =3D divider->max_div;
> +       while (max !=3D 0) {
> +               if ((max & 1) =3D=3D 1)
> +                       width++;
> +               max =3D max >> 1;

Is this ffs()?

>         }
> =20
> -       if ((clk_hw_get_flags(hw) & CLK_SET_RATE_PARENT) && divider->is_f=
rac)
> -               bestdiv =3D rate % *prate ? 1 : bestdiv;
> +       rate =3D divider_round_rate(hw, rate, prate, NULL, width, divider=
->flags);
> =20
> -       bestdiv =3D min_t(u32, bestdiv, divider->max_div);
> -       *prate =3D rate * bestdiv;
> +       if (divider->is_frac && (clk_hw_get_flags(hw) & CLK_SET_RATE_PARE=
NT) && (rate % *prate))
> +               *prate =3D rate;