Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1183775 > unrolled thread

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

Back to article view | Back to linux.kernel

Contents

  [PATCH v4 0/6] clk: fractional-divider: do a clean up Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2015-07-14 17:20 +0200
    [PATCH v4 4/6] clk: rockchip: save width in struct clk_fractional_divider Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2015-07-14 17:20 +0200
      Re: [PATCH v4 4/6] clk: rockchip: save width in struct clk_fractional_divider Heiko Stübner <heiko@sntech.de> - 2015-07-14 18:20 +0200
    [PATCH v4 6/6] serial: 8250_dw: allow lower reference frequencies Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2015-07-14 17:20 +0200
    [PATCH v4 1/6] clk: fractional-divider: fix sparse warnings Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2015-07-14 17:20 +0200
    [PATCH v4 3/6] clk: fractional-divider: keep mwidth and nwidth internally Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2015-07-14 17:20 +0200
      Re: [PATCH v4 3/6] clk: fractional-divider: keep mwidth and nwidth  internally Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2015-07-22 15:10 +0200

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

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1183776 — [PATCH v4 4/6] clk: rockchip: save width in struct clk_fractional_divider

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

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

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;

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1183777 — [PATCH v4 6/6] serial: 8250_dw: allow lower reference frequencies

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

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

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

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

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

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

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web