[PATCH 05/10] clk: Add support for simple dividers
Uwe Kleine-König
u.kleine-koenig at pengutronix.de
Mon Apr 18 05:49:09 EDT 2011
On Fri, Apr 15, 2011 at 09:08:10PM +0200, Sascha Hauer wrote:
> This patch adds support for the most common type of divider,
> which expects a register, width and shift values to desacribe
> the location of the divider. The divider can be zero based
> or one based (div = reg_val + 1 vs. div = reg_val).
>
> Signed-off-by: Sascha Hauer <s.hauer at pengutronix.de>
> Cc: Jeremy Kerr <jeremy.kerr at canonical.com>
> ---
> drivers/clk/Kconfig | 3 +
> drivers/clk/Makefile | 1 +
> drivers/clk/clk-divider.c | 132 +++++++++++++++++++++++++++++++++++++++++++++
> include/linux/clk.h | 31 +++++++++++
> 4 files changed, 167 insertions(+), 0 deletions(-)
> create mode 100644 drivers/clk/clk-divider.c
>
> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> index 6e3ae54..76bb4c9 100644
> --- a/drivers/clk/Kconfig
> +++ b/drivers/clk/Kconfig
> @@ -5,3 +5,6 @@ config CLKDEV_LOOKUP
>
> config USE_COMMON_STRUCT_CLK
> bool
> +
> +config USE_COMMON_CLK_DIVIDER
> + bool
> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> index a1a06d3..723d884 100644
> --- a/drivers/clk/Makefile
> +++ b/drivers/clk/Makefile
> @@ -1,3 +1,4 @@
>
> obj-$(CONFIG_CLKDEV_LOOKUP) += clkdev.o
> obj-$(CONFIG_USE_COMMON_STRUCT_CLK) += clk.o
> +obj-$(CONFIG_USE_COMMON_CLK_DIVIDER) += clk-divider.o
> diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
> new file mode 100644
> index 0000000..2de94df
> --- /dev/null
> +++ b/drivers/clk/clk-divider.c
> @@ -0,0 +1,132 @@
> +/*
> + * Copyright (C) 2011 Sascha Hauer <s.hauer at pengutronix.de>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * Standard functionality for the common clock API.
> + */
> +#include <linux/module.h>
> +#include <linux/clk.h>
> +#include <linux/io.h>
> +
> +#define to_clk_divider(clk) (container_of(clk, struct clk_divider, clk))
> +
> +static unsigned long clk_divider_get_rate(struct clk *clk)
> +{
> + struct clk_divider *divider = to_clk_divider(clk);
> + unsigned long rate = clk_get_rate(divider->parent);
> + unsigned int div;
> +
> + div = readl(divider->reg) >> divider->shift;
> + div &= (1 << divider->width) - 1;
> +
> + if (!(divider->flags & CLK_DIVIDER_FLAG_ONE_BASED))
> + div++;
> +
> + return rate / div;
DIV_ROUND_UP(rate, div)? (There is DIV_ROUND_CLOSEST, too, but I think
you need to round up, see below.)
> +}
> +
> +static int clk_divider_bestdiv(struct clk *clk, unsigned long rate,
> + unsigned long *best_parent_rate)
conceptually this returns an unsigned value
> +{
> + struct clk_divider *divider = to_clk_divider(clk);
> + int i, bestdiv = 0;
> + unsigned long parent_rate, best = 0, now, maxdiv;
> +
> + maxdiv = (1 << divider->width);
Is divider->width == 32 a valid scenario? If so, this overflows.
> +
> + if (divider->flags & CLK_DIVIDER_FLAG_ONE_BASED)
> + maxdiv--;
> +
> + /*
> + * The maximum divider we can use without overflowing
> + * unsigned long in rate * i below
> + */
> + maxdiv = min(ULONG_MAX / rate, maxdiv);
> +
> + for (i = 1; i <= maxdiv; i++) {
> + parent_rate = clk_round_rate(divider->parent, (rate + 1) * i);
> + now = parent_rate / i;
This is buggy, take rate = 25, i = 4. You request the parent to set
104. If you get that, the resulting rate is 26. (And even if
clk_round_rate only return 101 this might be too much.)
I think you need:
parent_rate = clk_round_rate(divider->parent, rate * i)
now = DIV_ROUND_UP(parent_rate, i);
I think you really need DIV_ROUND_UP here, DIV_ROUND_CLOSEST won't work.
Consider that a rate of 25 is requested, again with i=4. So the parent
is asked for 100 and might return 97. With DIV_ROUND_CLOSEST you report
to be able to set 24 now, but in fact it's more (24.25). If now further
up in the recursion another divider considers 24 to be a great value to
make 12Hz the value actually provided is too big.
> +
> + if (now <= rate && now >= best) {
> + bestdiv = i;
> + best = now;
> + }
> + }
> +
> + if (!bestdiv) {
> + bestdiv = (1 << divider->width);
> + parent_rate = clk_round_rate(divider->parent, 1);
> + } else {
> + parent_rate = best * bestdiv;
> + }
> +
> + if (best_parent_rate)
> + *best_parent_rate = parent_rate;
> +
> + return bestdiv;
> +}
> +
> +static long clk_divider_round_rate(struct clk *clk, unsigned long rate)
> +{
> + unsigned long best_parent_rate;
> + int div = clk_divider_bestdiv(clk, rate, &best_parent_rate);
> +
> + return best_parent_rate / div;
> +}
> +
> +static int clk_divider_set_rate(struct clk *clk, unsigned long rate)
> +{
> + unsigned long best_parent_rate;
> + struct clk_divider *divider = to_clk_divider(clk);
> + unsigned int div;
> + int ret;
> + unsigned long flags = 0;
> + u32 val;
> +
> + div = clk_divider_bestdiv(clk, rate, &best_parent_rate);
> +
> + if (rate != best_parent_rate / div)
> + return -EINVAL;
This is too harsh, isn't it. Or can you expect to only get values that
are returned by round_rate? Again you need DIV_ROUND_UP.
> +
> + ret = clk_set_rate(divider->parent, best_parent_rate);
> + if (ret)
> + return ret;
> +
> + if (divider->lock)
> + spin_lock_irqsave(divider->lock, flags);
> +
> + if (!(divider->flags & CLK_DIVIDER_FLAG_ONE_BASED))
> + div--;
> +
> + val = readl(divider->reg);
> + val &= ~(((1 << divider->width) - 1) << divider->shift);
> + val |= div << divider->shift;
> + writel(val, divider->reg);
> +
> + if (divider->lock)
> + spin_unlock_irqrestore(divider->lock, flags);
> +
> + return 0;
> +}
> +
> +static struct clk *clk_divider_get_parent(struct clk *clk)
> +{
> + struct clk_divider *divider = to_clk_divider(clk);
> +
> + return divider->parent;
> +}
> +
> +struct clk_ops clk_divider_ops = {
> + .prepare = clk_parent_prepare,
> + .unprepare = clk_parent_unprepare,
> + .enable = clk_parent_enable,
> + .disable = clk_parent_disable,
> + .get_rate = clk_divider_get_rate,
> + .round_rate = clk_divider_round_rate,
> + .set_rate = clk_divider_set_rate,
> + .get_parent = clk_divider_get_parent,
> +};
> +EXPORT_SYMBOL_GPL(clk_divider_ops);
> diff --git a/include/linux/clk.h b/include/linux/clk.h
> index d014341..6f9771b 100644
> --- a/include/linux/clk.h
> +++ b/include/linux/clk.h
> @@ -180,6 +180,37 @@ unsigned long clk_parent_get_rate(struct clk *clk);
> long clk_parent_round_rate(struct clk *clk, unsigned long rate);
> int clk_parent_set_rate(struct clk *clk, unsigned long rate);
>
> +/**
> + * clock divider
> + *
> + * @clk clock source
> + * @reg register containing the divider
> + * @shift shift to the divider
> + * @width width of the divider
> + * @lock register lock
"optional register lock"?
> + * @parent parent clock
@flags is missing here. Maybe use the same ordering as in the struct?
> + *
> + * This clock implements get_rate/set_rate/round_rate. prepare/unprepare and
> + * enable/disable are passed through to the parent.
> + *
> + * The divider is calculated as div = reg_val + 1
> + * or if CLK_DIVIDER_FLAG_ONE_BASED is set as div = reg_val
> + * (with reg_val == 0 considered invalid)
> + */
> +struct clk_divider {
> + struct clk clk;
> + void __iomem *reg;
> + unsigned char shift;
> + unsigned char width;
> + unsigned int div;
> + struct clk *parent;
> + spinlock_t *lock;
> +#define CLK_DIVIDER_FLAG_ONE_BASED (1 << 0)
> + unsigned int flags;
> +};
> +
> +extern struct clk_ops clk_divider_ops;
> +
> #else /* !CONFIG_USE_COMMON_STRUCT_CLK */
>
> /*
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
More information about the linux-arm-kernel
mailing list