[PATCH v5 0/5] clk: fractional-divider: do a clean up

[PATCH v5 0/5] clk: fractional-divider: do a clean up

[Andy Shevchenko]

The series provides a clean up for clk-fractional-diveder together with moving
it to use rational best approximation algorithm. I think the patches are
self-explanatory.

The series was tested with 8250_dw UART driver on Intel Braswell.

Patch 5 is an amendment to existing user of the fractional divider outside of
clock framework.

Changelog v5:
 - drop patch 1 (there is another fix already in upstream)
 - add tags (Ack, Review)

Changelog v4:
 - remove dependency to clk_div_mask() and use GENMASK() instead
 - apply changes to rockchip
 - amend 8250_dw driver to use proposed changes

Changelog v3:
 - add patch 2/3 to simplify further usage
 - don't use mult_frac() due to potential overflow on 32 bit kernels
 - guarantee in ->round_rate() that m and n will not overflow

Changelog v2:
 - move to rational_best_approximation() and mult_frac()
 - add patch 2/2

Andy Shevchenko (5):
  clk: fractional-divider: rename prate -> parent_rate
  clk: fractional-divider: keep mwidth and nwidth internally
  clk: rockchip: save width in struct clk_fractional_divider
  clk: fractional-divider: switch to rational best approximation
  serial: 8250_dw: allow lower reference frequencies

 drivers/clk/Kconfig                  |  1 +
 drivers/clk/clk-fractional-divider.c | 53 ++++++++++++++++++++++--------------
 drivers/clk/rockchip/clk.c           |  6 ++--
 drivers/tty/serial/8250/8250_dw.c    |  4 ---
 include/linux/clk-provider.h         |  3 +-
 5 files changed, 40 insertions(+), 27 deletions(-)

-- 
2.4.6

[PATCH v5 1/5] clk: fractional-divider: rename prate -> parent_rate

[Andy Shevchenko]

Rename function parameter to be more explicit what it is for. This also makes
it in align with struct clk_ops.

There is no functional change.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/clk/clk-fractional-divider.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/clk-fractional-divider.c b/drivers/clk/clk-fractional-divider.c
index e85f856..1af9c1e 100644
--- a/drivers/clk/clk-fractional-divider.c
+++ b/drivers/clk/clk-fractional-divider.c
@@ -50,18 +50,18 @@ static unsigned long clk_fd_recalc_rate(struct clk_hw *hw,
 }
 
 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;
 
-	if (!rate || rate >= *prate)
-		return *prate;
+	if (!rate || rate >= *parent_rate)
+		return *parent_rate;
 
-	div = gcd(*prate, rate);
+	div = gcd(*parent_rate, rate);
 
-	while ((*prate / div) > maxn) {
+	while ((*parent_rate / div) > maxn) {
 		div <<= 1;
 		rate <<= 1;
 	}
-- 
2.4.6

Re: [PATCH v5 1/5] clk: fractional-divider: rename prate -> parent_rate

[Heikki Krogerus]

On Mon, Jul 27, 2015 at 02:48:49PM +0300, Andy Shevchenko wrote:
> Rename function parameter to be more explicit what it is for. This also makes
> it in align with struct clk_ops.
> 
> There is no functional change.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>


Thanks,

-- 
heikki

[PATCH v5 2/5] clk: fractional-divider: keep mwidth and nwidth internally

[Andy Shevchenko]

The patch adds mwidth and nwidth fields to the struct clk_fractional_divider
for further usage. While here, use GENMASK() instead of open coding this
functionality.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/clk/clk-fractional-divider.c | 6 ++++--
 include/linux/clk-provider.h         | 3 ++-
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/clk-fractional-divider.c b/drivers/clk/clk-fractional-divider.c
index 1af9c1e..0282c76 100644
--- a/drivers/clk/clk-fractional-divider.c
+++ b/drivers/clk/clk-fractional-divider.c
@@ -128,9 +128,11 @@ struct clk *clk_register_fractional_divider(struct device *dev,
 
 	fd->reg = reg;
 	fd->mshift = mshift;
-	fd->mmask = (BIT(mwidth) - 1) << mshift;
+	fd->mwidth = mwidth;
+	fd->mmask = GENMASK(mwidth - 1, 0) << mshift;
 	fd->nshift = nshift;
-	fd->nmask = (BIT(nwidth) - 1) << nshift;
+	fd->nwidth = nwidth;
+	fd->nmask = GENMASK(nwidth - 1, 0) << nshift;
 	fd->flags = clk_divider_flags;
 	fd->lock = lock;
 	fd->hw.init = &init;
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 06a56e5..b4d45d9 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -500,13 +500,14 @@ struct clk *clk_register_fixed_factor(struct device *dev, const char *name,
  *
  * Clock with adjustable fractional divider affecting its output frequency.
  */
-
 struct clk_fractional_divider {
 	struct clk_hw	hw;
 	void __iomem	*reg;
 	u8		mshift;
+	u8		mwidth;
 	u32		mmask;
 	u8		nshift;
+	u8		nwidth;
 	u32		nmask;
 	u8		flags;
 	spinlock_t	*lock;
-- 
2.4.6

Re: [PATCH v5 2/5] clk: fractional-divider: keep mwidth and nwidth internally

[Heikki Krogerus]

On Mon, Jul 27, 2015 at 02:48:50PM +0300, Andy Shevchenko wrote:
> The patch adds mwidth and nwidth fields to the struct clk_fractional_divider
> for further usage. While here, use GENMASK() instead of open coding this
> functionality.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>


Thanks,

-- 
heikki

[PATCH v5 3/5] clk: rockchip: save width in struct clk_fractional_divider

[Andy Shevchenko]

The ->mwidth and ->nwidth fields will be used by clk-fractional-divider when it
will be switched to rational base approximation algorithm.

Reviewed-by: Heiko Stuebner <heiko@sntech.de>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/clk/rockchip/clk.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/rockchip/clk.c b/drivers/clk/rockchip/clk.c
index 2493881..be6c7fd 100644
--- a/drivers/clk/rockchip/clk.c
+++ b/drivers/clk/rockchip/clk.c
@@ -135,9 +135,11 @@ static struct clk *rockchip_clk_register_frac_branch(const char *name,
 	div->flags = div_flags;
 	div->reg = base + muxdiv_offset;
 	div->mshift = 16;
-	div->mmask = 0xffff0000;
+	div->mwidth = 16;
+	div->mmask = GENMASK(div->mwidth - 1, 0) << div->mshift;
 	div->nshift = 0;
-	div->nmask = 0xffff;
+	div->nwidth = 16;
+	div->nmask = GENMASK(div->nwidth - 1, 0) << div->nshift;
 	div->lock = lock;
 	div_ops = &clk_fractional_divider_ops;
 
-- 
2.4.6

[PATCH v5 4/5] clk: fractional-divider: switch to rational best approximation

[Andy Shevchenko]

This patch converts the code to use rational best approximation algorithm which
is much more precise.

Suggested-by: Stephen Boyd <sboyd@codeaurora.org>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/clk/Kconfig                  |  1 +
 drivers/clk/clk-fractional-divider.c | 41 +++++++++++++++++++++++-------------
 2 files changed, 27 insertions(+), 15 deletions(-)

diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index 42f7120..bd9f312 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
diff --git a/drivers/clk/clk-fractional-divider.c b/drivers/clk/clk-fractional-divider.c
index 0282c76..5c4955e 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)
 
@@ -22,7 +23,8 @@ static unsigned long clk_fd_recalc_rate(struct clk_hw *hw,
 {
 	struct clk_fractional_divider *fd = to_clk_fd(hw);
 	unsigned long flags = 0;
-	u32 val, m, n;
+	unsigned long m, n;
+	u32 val;
 	u64 ret;
 
 	if (fd->lock)
@@ -53,20 +55,30 @@ static long clk_fd_round_rate(struct clk_hw *hw, unsigned long rate,
 			      unsigned long *parent_rate)
 {
 	struct clk_fractional_divider *fd = to_clk_fd(hw);
-	unsigned maxn = (fd->nmask >> fd->nshift) + 1;
-	unsigned div;
+	unsigned long scale;
+	unsigned long m, n;
+	u64 ret;
 
 	if (!rate || rate >= *parent_rate)
 		return *parent_rate;
 
-	div = gcd(*parent_rate, rate);
+	/*
+	 * Get rate closer to *parent_rate to guarantee there is no overflow
+	 * for m and n. In the result it will be the nearest rate left shifted
+	 * by (scale - fd->nwidth) bits.
+	 */
+	scale = fls_long(*parent_rate / rate - 1);
+	if (scale > fd->nwidth)
+		rate <<= scale - fd->nwidth;
 
-	while ((*parent_rate / div) > maxn) {
-		div <<= 1;
-		rate <<= 1;
-	}
+	rational_best_approximation(rate, *parent_rate,
+			GENMASK(fd->mwidth - 1, 0), GENMASK(fd->nwidth - 1, 0),
+			&m, &n);
 
-	return rate;
+	ret = (u64)*parent_rate * m;
+	do_div(ret, n);
+
+	return ret;
 }
 
 static int clk_fd_set_rate(struct clk_hw *hw, unsigned long rate,
@@ -74,13 +86,12 @@ 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 m, n;
 	u32 val;
 
-	div = gcd(parent_rate, rate);
-	m = rate / div;
-	n = parent_rate / div;
+	rational_best_approximation(rate, parent_rate,
+			GENMASK(fd->mwidth - 1, 0), GENMASK(fd->nwidth - 1, 0),
+			&m, &n);
 
 	if (fd->lock)
 		spin_lock_irqsave(fd->lock, flags);
-- 
2.4.6

Re: [PATCH v5 4/5] clk: fractional-divider: switch to rational best approximation

[Heikki Krogerus]

On Mon, Jul 27, 2015 at 02:48:52PM +0300, Andy Shevchenko wrote:
> This patch converts the code to use rational best approximation algorithm which
> is much more precise.
> 
> Suggested-by: Stephen Boyd <sboyd@codeaurora.org>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>


Thanks,

-- 
heikki

[PATCH v5 5/5] serial: 8250_dw: allow lower reference frequencies

[Andy Shevchenko]

We have couple of standard but rare used baudrates which are not supported by
1,8432MHz reference frequency. Besides that user can potentially ask for any
baudrate (via BOTHER flag) and we currently don't fully support that. Since
clk-fractional-divider is moved to use rational best approximation for
reference frequency we may amend the driver to support whatever user wants.

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/tty/serial/8250/8250_dw.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index d48b506..c30e4ba 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -246,10 +246,6 @@ static void dw8250_set_termios(struct uart_port *p, struct ktermios *termios,
 	if (IS_ERR(d->clk) || !old)
 		goto out;
 
-	/* Not requesting clock rates below 1.8432Mhz */
-	if (baud < 115200)
-		baud = 115200;
-
 	clk_disable_unprepare(d->clk);
 	rate = clk_round_rate(d->clk, baud * 16);
 	ret = clk_set_rate(d->clk, rate);
-- 
2.4.6