* [PATCH v2 0/2] clk: fractional-divider: do a clean up @ 2015-03-30 18:57 Andy Shevchenko 2015-03-30 18:57 ` [PATCH v2 1/2] clk: fractional-divider: switch to rational best approximation Andy Shevchenko 2015-03-30 18:57 ` [PATCH v2 2/2] clk: fractional-divider: fix sparse warnings Andy Shevchenko 0 siblings, 2 replies; 6+ messages in thread From: Andy Shevchenko @ 2015-03-30 18:57 UTC (permalink / raw) To: Stephen Boyd, linux-kernel, heikki.krogerus; +Cc: Andy Shevchenko Patches are self-explanatory I think. So, just changelog is provided here. It would be really nice to queue them to v4.1. Changelog v2: - move to rational_best_approximation() and mult_frac() - add patch 2/2 Andy Shevchenko (2): clk: fractional-divider: switch to rational best approximation clk: fractional-divider: fix sparse warnings drivers/clk/Kconfig | 3 +- drivers/clk/clk-fractional-divider.c | 72 ++++++++++++++++++------------------ 2 files changed, 36 insertions(+), 39 deletions(-) -- 2.1.4 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/2] clk: fractional-divider: switch to rational best approximation 2015-03-30 18:57 [PATCH v2 0/2] clk: fractional-divider: do a clean up Andy Shevchenko @ 2015-03-30 18:57 ` Andy Shevchenko 2015-03-31 0:32 ` Stephen Boyd 2015-03-30 18:57 ` [PATCH v2 2/2] clk: fractional-divider: fix sparse warnings Andy Shevchenko 1 sibling, 1 reply; 6+ messages in thread From: Andy Shevchenko @ 2015-03-30 18:57 UTC (permalink / raw) To: Stephen Boyd, linux-kernel, heikki.krogerus; +Cc: Andy Shevchenko This patch converts the code to use rational best approximation algorithm which is more precise. Suggested-by: Stephen Boyd <sboyd@codeaurora.org> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- drivers/clk/Kconfig | 3 +-- drivers/clk/clk-fractional-divider.c | 39 ++++++++++++++---------------------- 2 files changed, 16 insertions(+), 26 deletions(-) diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig index 0b474a0..46d90a9 100644 --- a/drivers/clk/Kconfig +++ b/drivers/clk/Kconfig @@ -14,6 +14,7 @@ config COMMON_CLK select HAVE_CLK_PREPARE select CLKDEV_LOOKUP select SRCU + select RATIONAL ---help--- The common clock framework is a single definition of struct clk, useful across many platforms, as well as an @@ -63,7 +64,6 @@ config COMMON_CLK_SI5351 tristate "Clock driver for SiLabs 5351A/B/C" depends on I2C select REGMAP_I2C - select RATIONAL ---help--- This driver supports Silicon Labs 5351A/B/C programmable clock generators. @@ -139,7 +139,6 @@ config COMMON_CLK_CDCE706 tristate "Clock driver for TI CDCE706 clock synthesizer" depends on I2C select REGMAP_I2C - select RATIONAL ---help--- This driver supports TI CDCE706 programmable 3-PLL clock synthesizer. diff --git a/drivers/clk/clk-fractional-divider.c b/drivers/clk/clk-fractional-divider.c index 6aa72d9..9c53704 100644 --- a/drivers/clk/clk-fractional-divider.c +++ b/drivers/clk/clk-fractional-divider.c @@ -7,13 +7,14 @@ * * Adjustable fractional divider clock implementation. * Output rate = (m / n) * parent_rate. + * Uses rational best approximation algorithm. */ #include <linux/clk-provider.h> #include <linux/module.h> #include <linux/device.h> #include <linux/slab.h> -#include <linux/gcd.h> +#include <linux/rational.h> #define to_clk_fd(_hw) container_of(_hw, struct clk_fractional_divider, hw) @@ -21,9 +22,9 @@ static unsigned long clk_fd_recalc_rate(struct clk_hw *hw, unsigned long parent_rate) { struct clk_fractional_divider *fd = to_clk_fd(hw); + unsigned long m, n; unsigned long flags = 0; - u32 val, m, n; - u64 ret; + u32 val; if (fd->lock) spin_lock_irqsave(fd->lock, flags); @@ -39,30 +40,22 @@ static unsigned long clk_fd_recalc_rate(struct clk_hw *hw, if (!n || !m) return parent_rate; - ret = (u64)parent_rate * m; - do_div(ret, n); - - return ret; + return mult_frac(parent_rate, m, n); } static long clk_fd_round_rate(struct clk_hw *hw, unsigned long rate, - unsigned long *prate) + unsigned long *parent_rate) { struct clk_fractional_divider *fd = to_clk_fd(hw); - unsigned maxn = (fd->nmask >> fd->nshift) + 1; - unsigned div; + unsigned long m, n; - if (!rate || rate >= *prate) - return *prate; + if (!rate || rate >= *parent_rate) + return *parent_rate; - div = gcd(*prate, rate); - - while ((*prate / div) > maxn) { - div <<= 1; - rate <<= 1; - } + rational_best_approximation(rate, *parent_rate, + fd->mmask >> fd->mshift, fd->nmask >> fd->nshift, &m, &n); - return rate; + return mult_frac(rate, m, n); } static int clk_fd_set_rate(struct clk_hw *hw, unsigned long rate, @@ -70,13 +63,11 @@ 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 div; - unsigned n, m; + unsigned long n, m; u32 val; - div = gcd(parent_rate, rate); - m = rate / div; - n = parent_rate / div; + rational_best_approximation(rate, parent_rate, + fd->mmask >> fd->mshift, fd->nmask >> fd->nshift, &m, &n); if (fd->lock) spin_lock_irqsave(fd->lock, flags); -- 2.1.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] clk: fractional-divider: switch to rational best approximation 2015-03-30 18:57 ` [PATCH v2 1/2] clk: fractional-divider: switch to rational best approximation Andy Shevchenko @ 2015-03-31 0:32 ` Stephen Boyd 2015-03-31 7:42 ` Andy Shevchenko 0 siblings, 1 reply; 6+ messages in thread From: Stephen Boyd @ 2015-03-31 0:32 UTC (permalink / raw) To: Andy Shevchenko, linux-kernel, heikki.krogerus On 03/30/15 11:57, Andy Shevchenko wrote: > This patch converts the code to use rational best approximation algorithm which > is more precise. > > Suggested-by: Stephen Boyd <sboyd@codeaurora.org> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > drivers/clk/Kconfig | 3 +-- > drivers/clk/clk-fractional-divider.c | 39 ++++++++++++++---------------------- > 2 files changed, 16 insertions(+), 26 deletions(-) > > diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig > index 0b474a0..46d90a9 100644 > --- a/drivers/clk/Kconfig > +++ b/drivers/clk/Kconfig > @@ -14,6 +14,7 @@ config COMMON_CLK > select HAVE_CLK_PREPARE > select CLKDEV_LOOKUP > select SRCU > + select RATIONAL > ---help--- > The common clock framework is a single definition of struct > clk, useful across many platforms, as well as an > @@ -63,7 +64,6 @@ config COMMON_CLK_SI5351 > tristate "Clock driver for SiLabs 5351A/B/C" > depends on I2C > select REGMAP_I2C > - select RATIONAL > ---help--- > This driver supports Silicon Labs 5351A/B/C programmable clock > generators. > @@ -139,7 +139,6 @@ config COMMON_CLK_CDCE706 > tristate "Clock driver for TI CDCE706 clock synthesizer" > depends on I2C > select REGMAP_I2C > - select RATIONAL > ---help--- > This driver supports TI CDCE706 programmable 3-PLL clock synthesizer. Will some kernel janitor find us if we leave the selects here? I know you removed it because COMMON_CLK must be Y here and we'd always select RATIONAL, but it feels better to leave it alone and actually split off the basic clock types into individual configs so that the tiny kernel users don't have unused code lying around. That could be future work someday and then we might forget to select RATIONAL again. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] clk: fractional-divider: switch to rational best approximation 2015-03-31 0:32 ` Stephen Boyd @ 2015-03-31 7:42 ` Andy Shevchenko 2015-03-31 9:33 ` Andy Shevchenko 0 siblings, 1 reply; 6+ messages in thread From: Andy Shevchenko @ 2015-03-31 7:42 UTC (permalink / raw) To: Stephen Boyd; +Cc: Andy Shevchenko, linux-kernel, Krogerus, Heikki On Tue, Mar 31, 2015 at 3:32 AM, Stephen Boyd <sboyd@codeaurora.org> wrote: > On 03/30/15 11:57, Andy Shevchenko wrote: >> This patch converts the code to use rational best approximation algorithm which >> is more precise. >> --- a/drivers/clk/Kconfig >> +++ b/drivers/clk/Kconfig >> @@ -14,6 +14,7 @@ config COMMON_CLK >> select HAVE_CLK_PREPARE >> select CLKDEV_LOOKUP >> select SRCU >> + select RATIONAL >> ---help--- >> The common clock framework is a single definition of struct >> clk, useful across many platforms, as well as an >> @@ -63,7 +64,6 @@ config COMMON_CLK_SI5351 >> tristate "Clock driver for SiLabs 5351A/B/C" >> depends on I2C >> select REGMAP_I2C >> - select RATIONAL >> ---help--- >> This driver supports Silicon Labs 5351A/B/C programmable clock >> generators. >> @@ -139,7 +139,6 @@ config COMMON_CLK_CDCE706 >> tristate "Clock driver for TI CDCE706 clock synthesizer" >> depends on I2C >> select REGMAP_I2C >> - select RATIONAL >> ---help--- >> This driver supports TI CDCE706 programmable 3-PLL clock synthesizer. > > Will some kernel janitor find us if we leave the selects here? I know > you removed it because COMMON_CLK must be Y here and we'd always select > RATIONAL, but it feels better to leave it alone and actually split off > the basic clock types into individual configs so that the tiny kernel > users don't have unused code lying around. That could be future work > someday and then we might forget to select RATIONAL again. I'm fine with that as well. So, do I need to resend or you apply modified version? -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] clk: fractional-divider: switch to rational best approximation 2015-03-31 7:42 ` Andy Shevchenko @ 2015-03-31 9:33 ` Andy Shevchenko 0 siblings, 0 replies; 6+ messages in thread From: Andy Shevchenko @ 2015-03-31 9:33 UTC (permalink / raw) To: Andy Shevchenko; +Cc: Stephen Boyd, linux-kernel, Krogerus, Heikki On Tue, 2015-03-31 at 10:42 +0300, Andy Shevchenko wrote: > On Tue, Mar 31, 2015 at 3:32 AM, Stephen Boyd <sboyd@codeaurora.org> wrote: > > On 03/30/15 11:57, Andy Shevchenko wrote: > >> This patch converts the code to use rational best approximation algorithm which > >> is more precise. > > >> --- a/drivers/clk/Kconfig > >> +++ b/drivers/clk/Kconfig > >> @@ -14,6 +14,7 @@ config COMMON_CLK > >> select HAVE_CLK_PREPARE > >> select CLKDEV_LOOKUP > >> select SRCU > >> + select RATIONAL > >> ---help--- > >> The common clock framework is a single definition of struct > >> clk, useful across many platforms, as well as an > >> @@ -63,7 +64,6 @@ config COMMON_CLK_SI5351 > >> tristate "Clock driver for SiLabs 5351A/B/C" > >> depends on I2C > >> select REGMAP_I2C > >> - select RATIONAL > >> ---help--- > >> This driver supports Silicon Labs 5351A/B/C programmable clock > >> generators. > >> @@ -139,7 +139,6 @@ config COMMON_CLK_CDCE706 > >> tristate "Clock driver for TI CDCE706 clock synthesizer" > >> depends on I2C > >> select REGMAP_I2C > >> - select RATIONAL > >> ---help--- > >> This driver supports TI CDCE706 programmable 3-PLL clock synthesizer. > > > > Will some kernel janitor find us if we leave the selects here? I know > > you removed it because COMMON_CLK must be Y here and we'd always select > > RATIONAL, but it feels better to leave it alone and actually split off > > the basic clock types into individual configs so that the tiny kernel > > users don't have unused code lying around. That could be future work > > someday and then we might forget to select RATIONAL again. > > I'm fine with that as well. So, do I need to resend or you apply > modified version? There are couple of issues with arithmetics, such as we can't use mult_frac() because of potential overflow on 32 bit kernels, so I will resend new version. -- Andy Shevchenko <andriy.shevchenko@intel.com> Intel Finland Oy ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 2/2] clk: fractional-divider: fix sparse warnings 2015-03-30 18:57 [PATCH v2 0/2] clk: fractional-divider: do a clean up Andy Shevchenko 2015-03-30 18:57 ` [PATCH v2 1/2] clk: fractional-divider: switch to rational best approximation Andy Shevchenko @ 2015-03-30 18:57 ` Andy Shevchenko 1 sibling, 0 replies; 6+ messages in thread From: Andy Shevchenko @ 2015-03-30 18:57 UTC (permalink / raw) To: Stephen Boyd, linux-kernel, heikki.krogerus; +Cc: Andy Shevchenko Sparse complains about possible imbalance in locking. drivers/clk/clk-fractional-divider.c:37:9: warning: context imbalance in 'clk_fd_recalc_rate' - different lock contexts for basic block drivers/clk/clk-fractional-divider.c:61:12: warning: context imbalance in 'clk_fd_set_rate' - different lock contexts for basic block Let's rewrite code to fix this and make it more straight. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- drivers/clk/clk-fractional-divider.c | 35 +++++++++++++++++++++-------------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/drivers/clk/clk-fractional-divider.c b/drivers/clk/clk-fractional-divider.c index 9c53704..3766da8 100644 --- a/drivers/clk/clk-fractional-divider.c +++ b/drivers/clk/clk-fractional-divider.c @@ -26,13 +26,13 @@ static unsigned long clk_fd_recalc_rate(struct clk_hw *hw, unsigned long flags = 0; u32 val; - if (fd->lock) + if (fd->lock) { spin_lock_irqsave(fd->lock, flags); - - val = clk_readl(fd->reg); - - if (fd->lock) + val = clk_readl(fd->reg); spin_unlock_irqrestore(fd->lock, flags); + } else { + val = clk_readl(fd->reg); + } m = (val & fd->mmask) >> fd->mshift; n = (val & fd->nmask) >> fd->nshift; @@ -58,27 +58,34 @@ static long clk_fd_round_rate(struct clk_hw *hw, unsigned long rate, return mult_frac(rate, m, n); } +static void clk_fd_update(struct clk_fractional_divider *fd, + unsigned long m, unsigned long n) +{ + u32 val; + + val = clk_readl(fd->reg); + val &= ~(fd->mmask | fd->nmask); + val |= (m << fd->mshift) | (n << fd->nshift); + clk_writel(val, fd->reg); +} + static int clk_fd_set_rate(struct clk_hw *hw, unsigned long rate, unsigned long parent_rate) { struct clk_fractional_divider *fd = to_clk_fd(hw); unsigned long flags = 0; unsigned long n, m; - u32 val; rational_best_approximation(rate, parent_rate, fd->mmask >> fd->mshift, fd->nmask >> fd->nshift, &m, &n); - if (fd->lock) + if (fd->lock) { spin_lock_irqsave(fd->lock, flags); - - val = clk_readl(fd->reg); - val &= ~(fd->mmask | fd->nmask); - val |= (m << fd->mshift) | (n << fd->nshift); - clk_writel(val, fd->reg); - - if (fd->lock) + clk_fd_update(fd, m, n); spin_unlock_irqrestore(fd->lock, flags); + } else { + clk_fd_update(fd, m, n); + } return 0; } -- 2.1.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-03-31 9:33 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-03-30 18:57 [PATCH v2 0/2] clk: fractional-divider: do a clean up Andy Shevchenko 2015-03-30 18:57 ` [PATCH v2 1/2] clk: fractional-divider: switch to rational best approximation Andy Shevchenko 2015-03-31 0:32 ` Stephen Boyd 2015-03-31 7:42 ` Andy Shevchenko 2015-03-31 9:33 ` Andy Shevchenko 2015-03-30 18:57 ` [PATCH v2 2/2] clk: fractional-divider: fix sparse warnings Andy Shevchenko
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).