| Message ID | 20231016113002.15929-3-jay.buddhabhatti@amd.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | update for versal net platform | expand |
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 <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! > > 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 <sboyd@kernel.org> > Sent: Tuesday, October 24, 2023 9:08 AM > To: Buddhabhatti, Jay <jay.buddhabhatti@amd.com>; Simek, Michal > <michal.simek@amd.com>; mturquette@baylibre.com > Cc: linux-clk@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux- > kernel@vger.kernel.org; Buddhabhatti, Jay <jay.buddhabhatti@amd.com> > 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 <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! > > > > > 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;
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; }
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 <jay.buddhabhatti@amd.com> --- drivers/clk/zynqmp/divider.c | 70 ++++++------------------------------ 1 file changed, 10 insertions(+), 60 deletions(-)