| Message ID | 20230529133433.56215-1-frank@oltmanns.dev (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | clk: fractional-divider: Improve approximation when zero based | expand |
Quoting Frank Oltmanns (2023-05-29 06:34:33) > Consider the CLK_FRAC_DIVIDER_ZERO_BASED flag when finding the best > approximation for m and n. By doing so, increase the range of valid > values for the numerator and denominator by 1. > > Cc: A.s. Dong <aisheng.dong@nxp.com > Signed-off-by: Frank Oltmanns <frank@oltmanns.dev> > --- > I stumpled upon this, when familiarizing myself with clk drivers. Unfortunately, > I have no boards to test this patch. It seems the only user of this flag in > mainline is drivers/clk/imx/clk-composite-7ulp.c, therefore I'm cc-ing > get_maintainers.pl --git-blame -f drivers/clk/imx/clk-composite-7ulp.c > in the hopes of a wider audience. > > Thank you for considering this contribution, Thanks for looking at this. Can you add a kunit test (or a suite of tests) to confirm that this doesn't break existing functionality and also improves a case that would have failed or been suboptimal before?
Hi Stephen, On 2023-06-12 at 14:39:00 -0700, Stephen Boyd <sboyd@kernel.org> wrote: > Quoting Frank Oltmanns (2023-05-29 06:34:33) >> Consider the CLK_FRAC_DIVIDER_ZERO_BASED flag when finding the best >> approximation for m and n. By doing so, increase the range of valid >> values for the numerator and denominator by 1. >> >> Cc: A.s. Dong <aisheng.dong@nxp.com >> Signed-off-by: Frank Oltmanns <frank@oltmanns.dev> >> --- >> I stumpled upon this, when familiarizing myself with clk drivers. Unfortunately, >> I have no boards to test this patch. It seems the only user of this flag in >> mainline is drivers/clk/imx/clk-composite-7ulp.c, therefore I'm cc-ing >> get_maintainers.pl --git-blame -f drivers/clk/imx/clk-composite-7ulp.c >> in the hopes of a wider audience. >> >> Thank you for considering this contribution, > > Thanks for looking at this. Can you add a kunit test (or a suite of > tests) to confirm that this doesn't break existing functionality and > also improves a case that would have failed or been suboptimal before? Thank you for your feedback, I've submitted a V2 that contains the tests: https://lore.kernel.org/lkml/20230613083626.227476-1-frank@oltmanns.dev/ Thanks, Frank
diff --git a/drivers/clk/clk-fractional-divider.c b/drivers/clk/clk-fractional-divider.c index 479297763e70..7da21cd2bdb1 100644 --- a/drivers/clk/clk-fractional-divider.c +++ b/drivers/clk/clk-fractional-divider.c @@ -123,6 +123,7 @@ void clk_fractional_divider_general_approximation(struct clk_hw *hw, unsigned long *m, unsigned long *n) { struct clk_fractional_divider *fd = to_clk_fd(hw); + unsigned long max_m, max_n; /* * Get rate closer to *parent_rate to guarantee there is no overflow @@ -138,9 +139,15 @@ void clk_fractional_divider_general_approximation(struct clk_hw *hw, rate <<= scale - fd->nwidth; } - rational_best_approximation(rate, *parent_rate, - GENMASK(fd->mwidth - 1, 0), GENMASK(fd->nwidth - 1, 0), - m, n); + if (fd->flags & CLK_FRAC_DIVIDER_ZERO_BASED) { + max_m = 1 << fd->mwidth; + max_n = 1 << fd->nwidth; + } else { + max_m = GENMASK(fd->mwidth - 1, 0); + max_n = GENMASK(fd->nwidth - 1, 0); + } + + rational_best_approximation(rate, *parent_rate, max_m, max_n, m, n); } static long clk_fd_round_rate(struct clk_hw *hw, unsigned long rate, @@ -169,13 +176,18 @@ static int clk_fd_set_rate(struct clk_hw *hw, unsigned long rate, { struct clk_fractional_divider *fd = to_clk_fd(hw); unsigned long flags = 0; - unsigned long m, n; + unsigned long m, n, max_m, max_n; u32 mmask, nmask; u32 val; - rational_best_approximation(rate, parent_rate, - GENMASK(fd->mwidth - 1, 0), GENMASK(fd->nwidth - 1, 0), - &m, &n); + if (fd->flags & CLK_FRAC_DIVIDER_ZERO_BASED) { + max_m = 1 << fd->mwidth; + max_n = 1 << fd->nwidth; + } else { + max_m = GENMASK(fd->mwidth - 1, 0); + max_n = GENMASK(fd->nwidth - 1, 0); + } + rational_best_approximation(rate, parent_rate, max_m, max_n, &m, &n); if (fd->flags & CLK_FRAC_DIVIDER_ZERO_BASED) { m--;
Consider the CLK_FRAC_DIVIDER_ZERO_BASED flag when finding the best approximation for m and n. By doing so, increase the range of valid values for the numerator and denominator by 1. Cc: A.s. Dong <aisheng.dong@nxp.com Signed-off-by: Frank Oltmanns <frank@oltmanns.dev> --- I stumpled upon this, when familiarizing myself with clk drivers. Unfortunately, I have no boards to test this patch. It seems the only user of this flag in mainline is drivers/clk/imx/clk-composite-7ulp.c, therefore I'm cc-ing get_maintainers.pl --git-blame -f drivers/clk/imx/clk-composite-7ulp.c in the hopes of a wider audience. Thank you for considering this contribution, Frank drivers/clk/clk-fractional-divider.c | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-)