[PATCH v4 0/6] clk: fractional-divider: do a clean up

[PATCH v4 0/6] 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 6 is an amendment to existing user of the fractional divider outside of
clock framework. Greg, it would be nice to have your Ack on this if no
objections.

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 (6):
  clk: fractional-divider: fix sparse warnings
  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 | 90 ++++++++++++++++++++++--------------
 drivers/clk/rockchip/clk.c           |  6 ++-
 drivers/tty/serial/8250/8250_dw.c    |  4 --
 include/linux/clk-provider.h         |  3 +-
 5 files changed, 62 insertions(+), 42 deletions(-)

-- 
2.1.4

[PATCH v4 1/6] clk: fractional-divider: fix sparse warnings

[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 | 44 +++++++++++++++++++++---------------
 1 file changed, 26 insertions(+), 18 deletions(-)

diff --git a/drivers/clk/clk-fractional-divider.c b/drivers/clk/clk-fractional-divider.c
index 140eb58..ba2fdc1 100644
--- a/drivers/clk/clk-fractional-divider.c
+++ b/drivers/clk/clk-fractional-divider.c
@@ -21,17 +21,18 @@ 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 flags = 0;
-	u32 val, m, n;
+	unsigned long flags;
+	unsigned long m, n;
+	u32 val;
 	u64 ret;
 
-	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;
@@ -65,29 +66,36 @@ static long clk_fd_round_rate(struct clk_hw *hw, unsigned long rate,
 	return rate;
 }
 
+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 flags;
 	unsigned long div;
-	unsigned n, m;
-	u32 val;
+	unsigned long m, n;
 
 	div = gcd(parent_rate, rate);
 	m = rate / div;
 	n = parent_rate / div;
 
-	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

[PATCH v4 2/6] 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 ba2fdc1..7cfcc56 100644
--- a/drivers/clk/clk-fractional-divider.c
+++ b/drivers/clk/clk-fractional-divider.c
@@ -47,18 +47,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.1.4

[PATCH v4 3/6] 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 7cfcc56..16f42ae 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 2116e2b..bb3c626 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -496,13 +496,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.1.4

[PATCH v4 4/6] 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.

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.1.4

[PATCH v4 5/6] 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 | 36 +++++++++++++++++++++++-------------
 2 files changed, 24 insertions(+), 13 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 16f42ae..9dbe337 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)
 
@@ -50,20 +51,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 void clk_fd_update(struct clk_fractional_divider *fd,
@@ -82,12 +93,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;
-	unsigned long div;
 	unsigned long m, n;
 
-	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.1.4

[PATCH v4 6/6] 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.

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.1.4

Re: [PATCH v4 4/6] clk: rockchip: save width in struct clk_fractional_divider

[Heiko Stübner]

Am Dienstag, 14. Juli 2015, 18:12:01 schrieb Andy Shevchenko:
> The ->mwidth and ->nwidth fields will be used by clk-fractional-divider when
> it will be switched to rational base approximation algorithm.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Looks sane to me

Reviewed-by: Heiko Stuebner <heiko@sntech.de>


> ---
>  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;

Re: [PATCH v4 1/6] clk: fractional-divider: fix sparse warnings

[Stephen Boyd]

On 07/14, Andy Shevchenko wrote:
> 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 | 44 +++++++++++++++++++++---------------

This can be "fixed" with far fewer lines of code.

 drivers/clk/clk-fractional-divider.c | 8 ++++++++
  1 file changed, 8 insertions(+)

diff --git a/drivers/clk/clk-fractional-divider.c b/drivers/clk/clk-fractional-divider.c
index 140eb5844dc4..e85f856b8592 100644
--- a/drivers/clk/clk-fractional-divider.c
+++ b/drivers/clk/clk-fractional-divider.c
@@ -27,11 +27,15 @@ static unsigned long clk_fd_recalc_rate(struct clk_hw *hw,
 
 	if (fd->lock)
 		spin_lock_irqsave(fd->lock, flags);
+	else
+		__acquire(fd->lock);
 
 	val = clk_readl(fd->reg);
 
 	if (fd->lock)
 		spin_unlock_irqrestore(fd->lock, flags);
+	else
+		__release(fd->lock);
 
 	m = (val & fd->mmask) >> fd->mshift;
 	n = (val & fd->nmask) >> fd->nshift;
@@ -80,6 +84,8 @@ static int clk_fd_set_rate(struct clk_hw *hw, unsigned long rate,
 
 	if (fd->lock)
 		spin_lock_irqsave(fd->lock, flags);
+	else
+		__acquire(fd->lock);
 
 	val = clk_readl(fd->reg);
 	val &= ~(fd->mmask | fd->nmask);
@@ -88,6 +94,8 @@ static int clk_fd_set_rate(struct clk_hw *hw, unsigned long rate,
 
 	if (fd->lock)
 		spin_unlock_irqrestore(fd->lock, flags);
+	else
+		__release(fd->lock);
 
 	return 0;
 }

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v4 3/6] clk: fractional-divider: keep mwidth and nwidth internally

[Stephen Boyd]

On 07/14, Andy Shevchenko wrote:
> diff --git a/drivers/clk/clk-fractional-divider.c b/drivers/clk/clk-fractional-divider.c
> index 7cfcc56..16f42ae 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;

Please do the shifts in the GENMASK.
-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v4 3/6] clk: fractional-divider: keep mwidth and nwidth internally

[Andy Shevchenko]

On Tue, 2015-07-21 at 17:41 -0700, Stephen Boyd wrote:
> On 07/14, Andy Shevchenko wrote:
> > diff --git a/drivers/clk/clk-fractional-divider.c b/drivers/clk/clk
> > -fractional-divider.c
> > index 7cfcc56..16f42ae 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;
> 
> Please do the shifts in the GENMASK.

It's not optimal. Waste of performance.

32-bit case on 32-bit machine (similar to other cases on x86).

a) GENMASK(x - 1, 0) << y

        movl    $32, %ecx
        subb    4(%esp), %cl
        movl    $-1, %eax
        shrl    %cl, %eax
        movzbl  8(%esp), %ecx
        sall    %cl, %eax
        ret

b) GENMASK(x + y - 1, y)

        pushl   %esi
        pushl   %ebx
        movl    $1, %edx
        movzbl  12(%esp), %eax
        movzbl  16(%esp), %esi
        movl    $-1, %ebx
        subl    %eax, %edx
        movl    %ebx, %eax
        subl    %esi, %edx
        leal    31(%edx), %ecx
        shrl    %cl, %eax
        movl    %esi, %ecx
        sall    %cl, %ebx
        andl    %ebx, %eax
        popl    %ebx
        popl    %esi
        ret


-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH v4 3/6] clk: fractional-divider: keep mwidth and nwidth internally

[Stephen Boyd]

On 07/22, Andy Shevchenko wrote:
> On Tue, 2015-07-21 at 17:41 -0700, Stephen Boyd wrote:
> > On 07/14, Andy Shevchenko wrote:
> > > diff --git a/drivers/clk/clk-fractional-divider.c b/drivers/clk/clk
> > > -fractional-divider.c
> > > index 7cfcc56..16f42ae 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;
> > 
> > Please do the shifts in the GENMASK.
> 
> It's not optimal. Waste of performance.
> 
> 32-bit case on 32-bit machine (similar to other cases on x86).
> 
> a) GENMASK(x - 1, 0) << y
> 
>         movl    $32, %ecx
>         subb    4(%esp), %cl
>         movl    $-1, %eax
>         shrl    %cl, %eax
>         movzbl  8(%esp), %ecx
>         sall    %cl, %eax
>         ret
> 
> b) GENMASK(x + y - 1, y)
> 
>         pushl   %esi
>         pushl   %ebx
>         movl    $1, %edx
>         movzbl  12(%esp), %eax
>         movzbl  16(%esp), %esi
>         movl    $-1, %ebx
>         subl    %eax, %edx
>         movl    %ebx, %eax
>         subl    %esi, %edx
>         leal    31(%edx), %ecx
>         shrl    %cl, %eax
>         movl    %esi, %ecx
>         sall    %cl, %ebx
>         andl    %ebx, %eax
>         popl    %ebx
>         popl    %esi
>         ret
> 

This is not hotpath code, so why do we care?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH v4 3/6] clk: fractional-divider: keep mwidth and nwidth internally

[Andy Shevchenko]

On Wed, 2015-07-22 at 16:45 -0700, Stephen Boyd wrote:
> On 07/22, Andy Shevchenko wrote:
> > On Tue, 2015-07-21 at 17:41 -0700, Stephen Boyd wrote:
> > > On 07/14, Andy Shevchenko wrote:
> > > > diff --git a/drivers/clk/clk-fractional-divider.c 
> > > > b/drivers/clk/clk
> > > > -fractional-divider.c
> > > > index 7cfcc56..16f42ae 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;
> > > 
> > > Please do the shifts in the GENMASK.
> > 
> > It's not optimal. Waste of performance.
> > 
> > 32-bit case on 32-bit machine (similar to other cases on x86).
> > 
> > a) GENMASK(x - 1, 0) << y
> > 
> >         movl    $32, %ecx
> >         subb    4(%esp), %cl
> >         movl    $-1, %eax
> >         shrl    %cl, %eax
> >         movzbl  8(%esp), %ecx
> >         sall    %cl, %eax
> >         ret
> > 
> > b) GENMASK(x + y - 1, y)
> > 
> >         pushl   %esi
> >         pushl   %ebx
> >         movl    $1, %edx
> >         movzbl  12(%esp), %eax
> >         movzbl  16(%esp), %esi
> >         movl    $-1, %ebx
> >         subl    %eax, %edx
> >         movl    %ebx, %eax
> >         subl    %esi, %edx
> >         leal    31(%edx), %ecx
> >         shrl    %cl, %eax
> >         movl    %esi, %ecx
> >         sall    %cl, %ebx
> >         andl    %ebx, %eax
> >         popl    %ebx
> >         popl    %esi
> >         ret
> > 
> 
> This is not hotpath code, so why do we care?

I care about memory foot print of kernel's .text as well.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH v4 6/6] serial: 8250_dw: allow lower reference frequencies

[Greg Kroah-Hartman]

On Tue, Jul 14, 2015 at 06:12:03PM +0300, Andy Shevchenko wrote:
> 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.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/tty/serial/8250/8250_dw.c | 4 ----
>  1 file changed, 4 deletions(-)

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>