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.
Signed-off-by: Jay Buddhabhatti <[email protected]>
---
drivers/clk/zynqmp/divider.c | 70 ++++++------------------------------
1 file changed, 10 insertions(+), 60 deletions(-)
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
@@ -110,52 +110,6 @@ static unsigned long zynqmp_clk_divider_recalc_rate(struct clk_hw *hw,
return DIV_ROUND_UP_ULL(parent_rate, value);
}
-static void zynqmp_get_divider2_val(struct clk_hw *hw,
- unsigned long rate,
- struct zynqmp_clk_divider *divider,
- u32 *bestdiv)
-{
- int div1;
- int div2;
- long error = LONG_MAX;
- unsigned long div1_prate;
- struct clk_hw *div1_parent_hw;
- struct zynqmp_clk_divider *pdivider;
- struct clk_hw *div2_parent_hw = clk_hw_get_parent(hw);
-
- if (!div2_parent_hw)
- return;
-
- pdivider = to_zynqmp_clk_divider(div2_parent_hw);
- if (!pdivider)
- return;
-
- div1_parent_hw = clk_hw_get_parent(div2_parent_hw);
- if (!div1_parent_hw)
- return;
-
- div1_prate = clk_hw_get_rate(div1_parent_hw);
- *bestdiv = 1;
- for (div1 = 1; div1 <= pdivider->max_div;) {
- for (div2 = 1; div2 <= divider->max_div;) {
- long new_error = ((div1_prate / div1) / div2) - rate;
-
- if (abs(new_error) < abs(error)) {
- *bestdiv = div2;
- error = new_error;
- }
- if (divider->flags & CLK_DIVIDER_POWER_OF_TWO)
- div2 = div2 << 1;
- else
- div2++;
- }
- if (pdivider->flags & CLK_DIVIDER_POWER_OF_TWO)
- div1 = div1 << 1;
- else
- div1++;
- }
-}
-
/**
* zynqmp_clk_divider_round_rate() - Round rate of divider clock
* @hw: handle between common and hardware-specific interfaces
@@ -174,6 +128,8 @@ static long zynqmp_clk_divider_round_rate(struct clk_hw *hw,
u32 div_type = divider->div_type;
u32 bestdiv;
int ret;
+ u8 width = 0;
+ u16 max;
/* if read only, just return current value */
if (divider->flags & CLK_DIVIDER_READ_ONLY) {
@@ -193,23 +149,17 @@ static long zynqmp_clk_divider_round_rate(struct clk_hw *hw,
return DIV_ROUND_UP_ULL((u64)*prate, bestdiv);
}
- bestdiv = 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 automatically
- * set to optimum based on required total divider value.
- */
- if (div_type == TYPE_DIV2 &&
- (clk_hw_get_flags(hw) & CLK_SET_RATE_PARENT)) {
- zynqmp_get_divider2_val(hw, rate, divider, &bestdiv);
+ max = divider->max_div;
+ while (max != 0) {
+ if ((max & 1) == 1)
+ width++;
+ max = max >> 1;
}
- if ((clk_hw_get_flags(hw) & CLK_SET_RATE_PARENT) && divider->is_frac)
- bestdiv = rate % *prate ? 1 : bestdiv;
+ rate = divider_round_rate(hw, rate, prate, NULL, width, divider->flags);
- bestdiv = min_t(u32, bestdiv, divider->max_div);
- *prate = rate * bestdiv;
+ if (divider->is_frac && (clk_hw_get_flags(hw) & CLK_SET_RATE_PARENT) && (rate % *prate))
+ *prate = rate;
return rate;
}
--
2.17.1
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.
>
> Signed-off-by: Jay Buddhabhatti <[email protected]>
> ---
> drivers/clk/zynqmp/divider.c | 70 ++++++------------------------------
> 1 file changed, 10 insertions(+), 60 deletions(-)
Can't argue against removing that many lines!
>
> 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 clk_hw *hw,
> return DIV_ROUND_UP_ULL((u64)*prate, bestdiv);
> }
>
> - bestdiv = 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 automatically
> - * set to optimum based on required total divider value.
> - */
> - if (div_type == TYPE_DIV2 &&
> - (clk_hw_get_flags(hw) & CLK_SET_RATE_PARENT)) {
> - zynqmp_get_divider2_val(hw, rate, divider, &bestdiv);
> + max = divider->max_div;
> + while (max != 0) {
> + if ((max & 1) == 1)
> + width++;
> + max = max >> 1;
Is this ffs()?
> }
>
> - if ((clk_hw_get_flags(hw) & CLK_SET_RATE_PARENT) && divider->is_frac)
> - bestdiv = rate % *prate ? 1 : bestdiv;
> + rate = divider_round_rate(hw, rate, prate, NULL, width, divider->flags);
>
> - bestdiv = min_t(u32, bestdiv, divider->max_div);
> - *prate = rate * bestdiv;
> + if (divider->is_frac && (clk_hw_get_flags(hw) & CLK_SET_RATE_PARENT) && (rate % *prate))
> + *prate = rate;
Hi Stephen,
> -----Original Message-----
> From: Stephen Boyd <[email protected]>
> Sent: Tuesday, October 24, 2023 9:08 AM
> To: Buddhabhatti, Jay <[email protected]>; Simek, Michal
> <[email protected]>; [email protected]
> Cc: [email protected]; [email protected]; linux-
> [email protected]; Buddhabhatti, Jay <[email protected]>
> Subject: Re: [PATCH RESEND 2/2] drivers: clk: zynqmp: update divider round
> rate logic
>
> 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.
> >
> > Signed-off-by: Jay Buddhabhatti <[email protected]>
> > ---
> > drivers/clk/zynqmp/divider.c | 70
> > ++++++------------------------------
> > 1 file changed, 10 insertions(+), 60 deletions(-)
>
> Can't argue against removing that many lines!
>
> >
> > 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
> clk_hw *hw,
> > return DIV_ROUND_UP_ULL((u64)*prate, bestdiv);
> > }
> >
> > - bestdiv = 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 automatically
> > - * set to optimum based on required total divider value.
> > - */
> > - if (div_type == TYPE_DIV2 &&
> > - (clk_hw_get_flags(hw) & CLK_SET_RATE_PARENT)) {
> > - zynqmp_get_divider2_val(hw, rate, divider, &bestdiv);
> > + max = divider->max_div;
> > + while (max != 0) {
> > + if ((max & 1) == 1)
> > + width++;
> > + max = max >> 1;
>
> Is this ffs()?
[Jay] We need last set bit to get max width. I will use fls() to get most significant set bit for this. Thanks for suggestion.
Thanks,
Jay
>
> > }
> >
> > - if ((clk_hw_get_flags(hw) & CLK_SET_RATE_PARENT) && divider->is_frac)
> > - bestdiv = rate % *prate ? 1 : bestdiv;
> > + rate = divider_round_rate(hw, rate, prate, NULL, width,
> > + divider->flags);
> >
> > - bestdiv = min_t(u32, bestdiv, divider->max_div);
> > - *prate = rate * bestdiv;
> > + if (divider->is_frac && (clk_hw_get_flags(hw) & CLK_SET_RATE_PARENT)
> && (rate % *prate))
> > + *prate = rate;