2012-05-17 10:22:26

by Rajendra Nayak

[permalink] [raw]
Subject: [PATCH 0/2] clk: Add support for more default divider types

Mike,

The 2 patches here add support for 'power of two' and 'table based'
dividers to clk-divider.c.
These divider types are useful on OMAP and extending clk-divider.c
to support these should help other platforms too which have
such divider types.

regards,
Rajendra

Rajendra Nayak (2):
clk: Add support for power of two type dividers
clk: Add support for rate table based dividers

drivers/clk/clk-divider.c | 129 +++++++++++++++++++++++++++++++++++-------
include/linux/clk-private.h | 3 +-
include/linux/clk-provider.h | 10 +++-
3 files changed, 120 insertions(+), 22 deletions(-)


2012-05-17 10:22:30

by Rajendra Nayak

[permalink] [raw]
Subject: [PATCH 2/2] clk: Add support for rate table based dividers

Some divider clks do not have any obvious relationship
between the divider and the value programmed in the
register. For instance, say a value of 1 could signify divide
by 6 and a value of 2 could signify divide by 4 etc.
Also there are dividers where not all values possible
based on the bitfield width are valid. For instance
a 3 bit wide bitfield can be used to program a value
from 0 to 7. However its possible that only 0 to 4
are valid values.

All these cases need the platform code to pass a simple
table of divider/value tuple, so the framework knows
the exact value to be written based on the divider
calculation and can also do better error checking.

This patch adds support for such rate table based
dividers.

Signed-off-by: Rajendra Nayak <[email protected]>
---
drivers/clk/clk-divider.c | 67 ++++++++++++++++++++++++++++++++++++++++--
include/linux/clk-private.h | 3 +-
include/linux/clk-provider.h | 10 +++++-
3 files changed, 75 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
index e548c43..e4911ee 100644
--- a/drivers/clk/clk-divider.c
+++ b/drivers/clk/clk-divider.c
@@ -32,30 +32,69 @@
#define div_mask(d) ((1 << (d->width)) - 1)
#define is_power_of_two(i) !(i & ~i)

+static unsigned int _get_table_maxdiv(const struct clk_div_table *table)
+{
+ unsigned int maxdiv;
+ const struct clk_div_table *clkt;
+
+ for (clkt = table; clkt->div; clkt++)
+ if (clkt->div > maxdiv)
+ maxdiv = clkt->div;
+ return maxdiv;
+}
+
static unsigned int _get_maxdiv(struct clk_divider *divider)
{
if (divider->flags & CLK_DIVIDER_ONE_BASED)
return div_mask(divider);
if (divider->flags & CLK_DIVIDER_POWER_OF_TWO)
return 1 << div_mask(divider);
+ if (divider->table)
+ return _get_table_maxdiv(divider->table);
return div_mask(divider) + 1;
}

+static unsigned int _get_table_div(const struct clk_div_table *table,
+ unsigned int val)
+{
+ const struct clk_div_table *clkt;
+
+ for (clkt = table; clkt->div; clkt++)
+ if (clkt->val == val)
+ return clkt->div;
+ return 0;
+}
+
static unsigned int _get_div(struct clk_divider *divider, unsigned int val)
{
if (divider->flags & CLK_DIVIDER_ONE_BASED)
return val;
if (divider->flags & CLK_DIVIDER_POWER_OF_TWO)
return 1 << val;
+ if (divider->table)
+ return _get_table_div(divider->table, val);
return val + 1;
}

+static unsigned int _get_table_val(const struct clk_div_table *table,
+ unsigned int div)
+{
+ const struct clk_div_table *clkt;
+
+ for (clkt = table; clkt->div; clkt++)
+ if (clkt->div == div)
+ return clkt->val;
+ return 0;
+}
+
static unsigned int _get_val(struct clk_divider *divider, u8 div)
{
if (divider->flags & CLK_DIVIDER_ONE_BASED)
return div;
if (divider->flags & CLK_DIVIDER_POWER_OF_TWO)
return __ffs(div);
+ if (divider->table)
+ return _get_table_val(divider->table, div);
return div - 1;
}

@@ -84,6 +123,26 @@ static unsigned long clk_divider_recalc_rate(struct clk_hw *hw,
*/
#define MULT_ROUND_UP(r, m) ((r) * (m) + (m) - 1)

+static bool _is_valid_table_div(const struct clk_div_table *table,
+ unsigned int div)
+{
+ const struct clk_div_table *clkt;
+
+ for (clkt = table; clkt->div; clkt++)
+ if (clkt->div == div)
+ return true;
+ return false;
+}
+
+static bool _is_valid_div(struct clk_divider *divider, unsigned int div)
+{
+ if (divider->flags & CLK_DIVIDER_POWER_OF_TWO)
+ return is_power_of_two(div);
+ if (divider->table)
+ return _is_valid_table_div(divider->table, div);
+ return true;
+}
+
static int clk_divider_bestdiv(struct clk_hw *hw, unsigned long rate,
unsigned long *best_parent_rate)
{
@@ -111,8 +170,7 @@ static int clk_divider_bestdiv(struct clk_hw *hw, unsigned long rate,
maxdiv = min(ULONG_MAX / rate, maxdiv);

for (i = 1; i <= maxdiv; i++) {
- if ((divider->flags & CLK_DIVIDER_POWER_OF_TWO)
- && (!is_power_of_two(i)))
+ if (!_is_valid_div(divider, i))
continue;
parent_rate = __clk_round_rate(__clk_get_parent(hw->clk),
MULT_ROUND_UP(rate, i));
@@ -186,12 +244,14 @@ EXPORT_SYMBOL_GPL(clk_divider_ops);
* @shift: number of bits to shift the bitfield
* @width: width of the bitfield
* @clk_divider_flags: divider-specific flags for this clock
+ * @table: array of divider/value pairs ending with a div set to 0
* @lock: shared register lock for this clock
*/
struct clk *clk_register_divider(struct device *dev, const char *name,
const char *parent_name, unsigned long flags,
void __iomem *reg, u8 shift, u8 width,
- u8 clk_divider_flags, spinlock_t *lock)
+ u8 clk_divider_flags, const struct clk_div_table *table,
+ spinlock_t *lock)
{
struct clk_divider *div;
struct clk *clk;
@@ -217,6 +277,7 @@ struct clk *clk_register_divider(struct device *dev, const char *name,
div->flags = clk_divider_flags;
div->lock = lock;
div->hw.init = &init;
+ div->table = table;

/* register the clock */
clk = clk_register(dev, &div->hw);
diff --git a/include/linux/clk-private.h b/include/linux/clk-private.h
index eb3f84b..2479239 100644
--- a/include/linux/clk-private.h
+++ b/include/linux/clk-private.h
@@ -105,7 +105,7 @@ struct clk {

#define DEFINE_CLK_DIVIDER(_name, _parent_name, _parent_ptr, \
_flags, _reg, _shift, _width, \
- _divider_flags, _lock) \
+ _divider_flags, _table, _lock) \
static struct clk _name; \
static const char *_name##_parent_names[] = { \
_parent_name, \
@@ -121,6 +121,7 @@ struct clk {
.shift = _shift, \
.width = _width, \
.flags = _divider_flags, \
+ .table = _table, \
.lock = _lock, \
}; \
DEFINE_CLK(_name, clk_divider_ops, _flags, \
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 4a0b483..22bc067 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -203,6 +203,11 @@ struct clk *clk_register_gate(struct device *dev, const char *name,
void __iomem *reg, u8 bit_idx,
u8 clk_gate_flags, spinlock_t *lock);

+struct clk_div_table {
+ unsigned int val;
+ unsigned int div;
+};
+
/**
* struct clk_divider - adjustable divider clock
*
@@ -210,6 +215,7 @@ struct clk *clk_register_gate(struct device *dev, const char *name,
* @reg: register containing the divider
* @shift: shift to the divider bit field
* @width: width of the divider bit field
+ * @table: array of value/divider pairs, last entry should have div = 0
* @lock: register lock
*
* Clock with an adjustable divider affecting its output frequency. Implements
@@ -229,6 +235,7 @@ struct clk_divider {
u8 shift;
u8 width;
u8 flags;
+ const struct clk_div_table *table;
spinlock_t *lock;
};

@@ -239,7 +246,8 @@ extern const struct clk_ops clk_divider_ops;
struct clk *clk_register_divider(struct device *dev, const char *name,
const char *parent_name, unsigned long flags,
void __iomem *reg, u8 shift, u8 width,
- u8 clk_divider_flags, spinlock_t *lock);
+ u8 clk_divider_flags, const struct clk_div_table *table,
+ spinlock_t *lock);

/**
* struct clk_mux - multiplexer clock
--
1.7.1

2012-05-17 10:22:28

by Rajendra Nayak

[permalink] [raw]
Subject: [PATCH 1/2] clk: Add support for power of two type dividers

Quite often dividers and the value programmed in the
register have a relation of 'power of two', something like
value div
0 1
1 2
2 4
3 8...

Add support for such dividers as part of clk-divider.

The clk-divider flag 'CLK_DIVIDER_POWER_OF_TWO' should be used
to define such clocks.

Signed-off-by: Rajendra Nayak <[email protected]>
---
drivers/clk/clk-divider.c | 66 ++++++++++++++++++++++++++++++++-------------
1 files changed, 47 insertions(+), 19 deletions(-)

diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
index 8ea11b4..e548c43 100644
--- a/drivers/clk/clk-divider.c
+++ b/drivers/clk/clk-divider.c
@@ -30,18 +30,50 @@
#define to_clk_divider(_hw) container_of(_hw, struct clk_divider, hw)

#define div_mask(d) ((1 << (d->width)) - 1)
+#define is_power_of_two(i) !(i & ~i)
+
+static unsigned int _get_maxdiv(struct clk_divider *divider)
+{
+ if (divider->flags & CLK_DIVIDER_ONE_BASED)
+ return div_mask(divider);
+ if (divider->flags & CLK_DIVIDER_POWER_OF_TWO)
+ return 1 << div_mask(divider);
+ return div_mask(divider) + 1;
+}
+
+static unsigned int _get_div(struct clk_divider *divider, unsigned int val)
+{
+ if (divider->flags & CLK_DIVIDER_ONE_BASED)
+ return val;
+ if (divider->flags & CLK_DIVIDER_POWER_OF_TWO)
+ return 1 << val;
+ return val + 1;
+}
+
+static unsigned int _get_val(struct clk_divider *divider, u8 div)
+{
+ if (divider->flags & CLK_DIVIDER_ONE_BASED)
+ return div;
+ if (divider->flags & CLK_DIVIDER_POWER_OF_TWO)
+ return __ffs(div);
+ return div - 1;
+}

static unsigned long clk_divider_recalc_rate(struct clk_hw *hw,
unsigned long parent_rate)
{
struct clk_divider *divider = to_clk_divider(hw);
- unsigned int div;
+ unsigned int div, val;

- div = readl(divider->reg) >> divider->shift;
- div &= div_mask(divider);
+ val = readl(divider->reg) >> divider->shift;
+ val &= div_mask(divider);

- if (!(divider->flags & CLK_DIVIDER_ONE_BASED))
- div++;
+ div = _get_div(divider, val);
+ if (!div) {
+ WARN(1, "%s: Invalid divisor for clock %s\n", __func__,
+ __clk_get_name(hw->clk));
+ return parent_rate;
+ }

return parent_rate / div;
}
@@ -62,10 +94,7 @@ static int clk_divider_bestdiv(struct clk_hw *hw, unsigned long rate,
if (!rate)
rate = 1;

- maxdiv = (1 << divider->width);
-
- if (divider->flags & CLK_DIVIDER_ONE_BASED)
- maxdiv--;
+ maxdiv = _get_maxdiv(divider);

if (!(__clk_get_flags(hw->clk) & CLK_SET_RATE_PARENT)) {
parent_rate = *best_parent_rate;
@@ -82,6 +111,9 @@ static int clk_divider_bestdiv(struct clk_hw *hw, unsigned long rate,
maxdiv = min(ULONG_MAX / rate, maxdiv);

for (i = 1; i <= maxdiv; i++) {
+ if ((divider->flags & CLK_DIVIDER_POWER_OF_TWO)
+ && (!is_power_of_two(i)))
+ continue;
parent_rate = __clk_round_rate(__clk_get_parent(hw->clk),
MULT_ROUND_UP(rate, i));
now = parent_rate / i;
@@ -93,9 +125,7 @@ static int clk_divider_bestdiv(struct clk_hw *hw, unsigned long rate,
}

if (!bestdiv) {
- bestdiv = (1 << divider->width);
- if (divider->flags & CLK_DIVIDER_ONE_BASED)
- bestdiv--;
+ bestdiv = _get_maxdiv(divider);
*best_parent_rate = __clk_round_rate(__clk_get_parent(hw->clk), 1);
}

@@ -115,24 +145,22 @@ static int clk_divider_set_rate(struct clk_hw *hw, unsigned long rate,
unsigned long parent_rate)
{
struct clk_divider *divider = to_clk_divider(hw);
- unsigned int div;
+ unsigned int div, value;
unsigned long flags = 0;
u32 val;

div = parent_rate / rate;
+ value = _get_val(divider, div);

- if (!(divider->flags & CLK_DIVIDER_ONE_BASED))
- div--;
-
- if (div > div_mask(divider))
- div = div_mask(divider);
+ if (value > div_mask(divider))
+ value = div_mask(divider);

if (divider->lock)
spin_lock_irqsave(divider->lock, flags);

val = readl(divider->reg);
val &= ~(div_mask(divider) << divider->shift);
- val |= div << divider->shift;
+ val |= value << divider->shift;
writel(val, divider->reg);

if (divider->lock)
--
1.7.1

2012-05-21 09:54:55

by Ben Dooks

[permalink] [raw]
Subject: Re: [PATCH 2/2] clk: Add support for rate table based dividers

On 17/05/12 11:22, Rajendra Nayak wrote:
> Some divider clks do not have any obvious relationship
> between the divider and the value programmed in the
> register. For instance, say a value of 1 could signify divide
> by 6 and a value of 2 could signify divide by 4 etc.
> Also there are dividers where not all values possible
> based on the bitfield width are valid. For instance
> a 3 bit wide bitfield can be used to program a value
> from 0 to 7. However its possible that only 0 to 4
> are valid values.
>
> All these cases need the platform code to pass a simple
> table of divider/value tuple, so the framework knows
> the exact value to be written based on the divider
> calculation and can also do better error checking.
>
> This patch adds support for such rate table based
> dividers.

I was considering the idea that you simply pass a
pointer to a set of routines and a data pointer to
the clk-divider code so that any new cases don't
require changing the drivers/clk/clk-divider.c

This would make the get max / min / special just
a function call through a struct.

> Signed-off-by: Rajendra Nayak<[email protected]>
> ---
> drivers/clk/clk-divider.c | 67 ++++++++++++++++++++++++++++++++++++++++--
> include/linux/clk-private.h | 3 +-
> include/linux/clk-provider.h | 10 +++++-
> 3 files changed, 75 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
> index e548c43..e4911ee 100644
> --- a/drivers/clk/clk-divider.c
> +++ b/drivers/clk/clk-divider.c
> @@ -32,30 +32,69 @@
> #define div_mask(d) ((1<< (d->width)) - 1)
> #define is_power_of_two(i) !(i& ~i)
>
> +static unsigned int _get_table_maxdiv(const struct clk_div_table *table)
> +{
> + unsigned int maxdiv;
> + const struct clk_div_table *clkt;
> +
> + for (clkt = table; clkt->div; clkt++)
> + if (clkt->div> maxdiv)
> + maxdiv = clkt->div;
> + return maxdiv;
> +}
> +
> static unsigned int _get_maxdiv(struct clk_divider *divider)
> {
> if (divider->flags& CLK_DIVIDER_ONE_BASED)
> return div_mask(divider);
> if (divider->flags& CLK_DIVIDER_POWER_OF_TWO)
> return 1<< div_mask(divider);
> + if (divider->table)
> + return _get_table_maxdiv(divider->table);
> return div_mask(divider) + 1;
> }
>
> +static unsigned int _get_table_div(const struct clk_div_table *table,
> + unsigned int val)
> +{
> + const struct clk_div_table *clkt;
> +
> + for (clkt = table; clkt->div; clkt++)
> + if (clkt->val == val)
> + return clkt->div;
> + return 0;
> +}
> +
> static unsigned int _get_div(struct clk_divider *divider, unsigned int val)
> {
> if (divider->flags& CLK_DIVIDER_ONE_BASED)
> return val;
> if (divider->flags& CLK_DIVIDER_POWER_OF_TWO)
> return 1<< val;
> + if (divider->table)
> + return _get_table_div(divider->table, val);
> return val + 1;
> }
>
> +static unsigned int _get_table_val(const struct clk_div_table *table,
> + unsigned int div)
> +{
> + const struct clk_div_table *clkt;
> +
> + for (clkt = table; clkt->div; clkt++)
> + if (clkt->div == div)
> + return clkt->val;
> + return 0;
> +}
> +
> static unsigned int _get_val(struct clk_divider *divider, u8 div)
> {
> if (divider->flags& CLK_DIVIDER_ONE_BASED)
> return div;
> if (divider->flags& CLK_DIVIDER_POWER_OF_TWO)
> return __ffs(div);
> + if (divider->table)
> + return _get_table_val(divider->table, div);
> return div - 1;
> }
>
> @@ -84,6 +123,26 @@ static unsigned long clk_divider_recalc_rate(struct clk_hw *hw,
> */
> #define MULT_ROUND_UP(r, m) ((r) * (m) + (m) - 1)
>
> +static bool _is_valid_table_div(const struct clk_div_table *table,
> + unsigned int div)
> +{
> + const struct clk_div_table *clkt;
> +
> + for (clkt = table; clkt->div; clkt++)
> + if (clkt->div == div)
> + return true;
> + return false;
> +}
> +
> +static bool _is_valid_div(struct clk_divider *divider, unsigned int div)
> +{
> + if (divider->flags& CLK_DIVIDER_POWER_OF_TWO)
> + return is_power_of_two(div);
> + if (divider->table)
> + return _is_valid_table_div(divider->table, div);
> + return true;
> +}
> +
> static int clk_divider_bestdiv(struct clk_hw *hw, unsigned long rate,
> unsigned long *best_parent_rate)
> {
> @@ -111,8 +170,7 @@ static int clk_divider_bestdiv(struct clk_hw *hw, unsigned long rate,
> maxdiv = min(ULONG_MAX / rate, maxdiv);
>
> for (i = 1; i<= maxdiv; i++) {
> - if ((divider->flags& CLK_DIVIDER_POWER_OF_TWO)
> - && (!is_power_of_two(i)))
> + if (!_is_valid_div(divider, i))
> continue;
> parent_rate = __clk_round_rate(__clk_get_parent(hw->clk),
> MULT_ROUND_UP(rate, i));
> @@ -186,12 +244,14 @@ EXPORT_SYMBOL_GPL(clk_divider_ops);
> * @shift: number of bits to shift the bitfield
> * @width: width of the bitfield
> * @clk_divider_flags: divider-specific flags for this clock
> + * @table: array of divider/value pairs ending with a div set to 0
> * @lock: shared register lock for this clock
> */
> struct clk *clk_register_divider(struct device *dev, const char *name,
> const char *parent_name, unsigned long flags,
> void __iomem *reg, u8 shift, u8 width,
> - u8 clk_divider_flags, spinlock_t *lock)
> + u8 clk_divider_flags, const struct clk_div_table *table,
> + spinlock_t *lock)
> {
> struct clk_divider *div;
> struct clk *clk;
> @@ -217,6 +277,7 @@ struct clk *clk_register_divider(struct device *dev, const char *name,
> div->flags = clk_divider_flags;
> div->lock = lock;
> div->hw.init =&init;
> + div->table = table;
>
> /* register the clock */
> clk = clk_register(dev,&div->hw);
> diff --git a/include/linux/clk-private.h b/include/linux/clk-private.h
> index eb3f84b..2479239 100644
> --- a/include/linux/clk-private.h
> +++ b/include/linux/clk-private.h
> @@ -105,7 +105,7 @@ struct clk {
>
> #define DEFINE_CLK_DIVIDER(_name, _parent_name, _parent_ptr, \
> _flags, _reg, _shift, _width, \
> - _divider_flags, _lock) \
> + _divider_flags, _table, _lock) \
> static struct clk _name; \
> static const char *_name##_parent_names[] = { \
> _parent_name, \
> @@ -121,6 +121,7 @@ struct clk {
> .shift = _shift, \
> .width = _width, \
> .flags = _divider_flags, \
> + .table = _table, \
> .lock = _lock, \
> }; \
> DEFINE_CLK(_name, clk_divider_ops, _flags, \
> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
> index 4a0b483..22bc067 100644
> --- a/include/linux/clk-provider.h
> +++ b/include/linux/clk-provider.h
> @@ -203,6 +203,11 @@ struct clk *clk_register_gate(struct device *dev, const char *name,
> void __iomem *reg, u8 bit_idx,
> u8 clk_gate_flags, spinlock_t *lock);
>
> +struct clk_div_table {
> + unsigned int val;
> + unsigned int div;
> +};
> +
> /**
> * struct clk_divider - adjustable divider clock
> *
> @@ -210,6 +215,7 @@ struct clk *clk_register_gate(struct device *dev, const char *name,
> * @reg: register containing the divider
> * @shift: shift to the divider bit field
> * @width: width of the divider bit field
> + * @table: array of value/divider pairs, last entry should have div = 0
> * @lock: register lock
> *
> * Clock with an adjustable divider affecting its output frequency. Implements
> @@ -229,6 +235,7 @@ struct clk_divider {
> u8 shift;
> u8 width;
> u8 flags;
> + const struct clk_div_table *table;
> spinlock_t *lock;
> };
>
> @@ -239,7 +246,8 @@ extern const struct clk_ops clk_divider_ops;
> struct clk *clk_register_divider(struct device *dev, const char *name,
> const char *parent_name, unsigned long flags,
> void __iomem *reg, u8 shift, u8 width,
> - u8 clk_divider_flags, spinlock_t *lock);
> + u8 clk_divider_flags, const struct clk_div_table *table,
> + spinlock_t *lock);
>
> /**
> * struct clk_mux - multiplexer clock


--
Ben Dooks http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius

2012-05-22 04:45:35

by Rajendra Nayak

[permalink] [raw]
Subject: Re: [PATCH 2/2] clk: Add support for rate table based dividers

Hi Ben,

On Monday 21 May 2012 03:17 PM, Ben Dooks wrote:
> On 17/05/12 11:22, Rajendra Nayak wrote:
>> Some divider clks do not have any obvious relationship
>> between the divider and the value programmed in the
>> register. For instance, say a value of 1 could signify divide
>> by 6 and a value of 2 could signify divide by 4 etc.
>> Also there are dividers where not all values possible
>> based on the bitfield width are valid. For instance
>> a 3 bit wide bitfield can be used to program a value
>> from 0 to 7. However its possible that only 0 to 4
>> are valid values.
>>
>> All these cases need the platform code to pass a simple
>> table of divider/value tuple, so the framework knows
>> the exact value to be written based on the divider
>> calculation and can also do better error checking.
>>
>> This patch adds support for such rate table based
>> dividers.
>
> I was considering the idea that you simply pass a
> pointer to a set of routines and a data pointer to
> the clk-divider code so that any new cases don't
> require changing the drivers/clk/clk-divider.c

I don;t know if I understand your comment completely.
Are you suggesting the get min/max etc be function pointers
passed by platform code (and implemented in platform code?)
so clk-divider does not need an update every time a new divider
type is added?
The idea of extending clk-divider was so its useful for more
than just OMAP, so the code in clk-divider can be reused across
multiple platforms. Did I understand your comment right?

regards,
Rajendra

>
> This would make the get max / min / special just
> a function call through a struct.
>
>> Signed-off-by: Rajendra Nayak<[email protected]>
>> ---
>> drivers/clk/clk-divider.c | 67 ++++++++++++++++++++++++++++++++++++++++--
>> include/linux/clk-private.h | 3 +-
>> include/linux/clk-provider.h | 10 +++++-
>> 3 files changed, 75 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
>> index e548c43..e4911ee 100644
>> --- a/drivers/clk/clk-divider.c
>> +++ b/drivers/clk/clk-divider.c
>> @@ -32,30 +32,69 @@
>> #define div_mask(d) ((1<< (d->width)) - 1)
>> #define is_power_of_two(i) !(i& ~i)
>>
>> +static unsigned int _get_table_maxdiv(const struct clk_div_table *table)
>> +{
>> + unsigned int maxdiv;
>> + const struct clk_div_table *clkt;
>> +
>> + for (clkt = table; clkt->div; clkt++)
>> + if (clkt->div> maxdiv)
>> + maxdiv = clkt->div;
>> + return maxdiv;
>> +}
>> +
>> static unsigned int _get_maxdiv(struct clk_divider *divider)
>> {
>> if (divider->flags& CLK_DIVIDER_ONE_BASED)
>> return div_mask(divider);
>> if (divider->flags& CLK_DIVIDER_POWER_OF_TWO)
>> return 1<< div_mask(divider);
>> + if (divider->table)
>> + return _get_table_maxdiv(divider->table);
>> return div_mask(divider) + 1;
>> }
>>
>> +static unsigned int _get_table_div(const struct clk_div_table *table,
>> + unsigned int val)
>> +{
>> + const struct clk_div_table *clkt;
>> +
>> + for (clkt = table; clkt->div; clkt++)
>> + if (clkt->val == val)
>> + return clkt->div;
>> + return 0;
>> +}
>> +
>> static unsigned int _get_div(struct clk_divider *divider, unsigned int
>> val)
>> {
>> if (divider->flags& CLK_DIVIDER_ONE_BASED)
>> return val;
>> if (divider->flags& CLK_DIVIDER_POWER_OF_TWO)
>> return 1<< val;
>> + if (divider->table)
>> + return _get_table_div(divider->table, val);
>> return val + 1;
>> }
>>
>> +static unsigned int _get_table_val(const struct clk_div_table *table,
>> + unsigned int div)
>> +{
>> + const struct clk_div_table *clkt;
>> +
>> + for (clkt = table; clkt->div; clkt++)
>> + if (clkt->div == div)
>> + return clkt->val;
>> + return 0;
>> +}
>> +
>> static unsigned int _get_val(struct clk_divider *divider, u8 div)
>> {
>> if (divider->flags& CLK_DIVIDER_ONE_BASED)
>> return div;
>> if (divider->flags& CLK_DIVIDER_POWER_OF_TWO)
>> return __ffs(div);
>> + if (divider->table)
>> + return _get_table_val(divider->table, div);
>> return div - 1;
>> }
>>
>> @@ -84,6 +123,26 @@ static unsigned long
>> clk_divider_recalc_rate(struct clk_hw *hw,
>> */
>> #define MULT_ROUND_UP(r, m) ((r) * (m) + (m) - 1)
>>
>> +static bool _is_valid_table_div(const struct clk_div_table *table,
>> + unsigned int div)
>> +{
>> + const struct clk_div_table *clkt;
>> +
>> + for (clkt = table; clkt->div; clkt++)
>> + if (clkt->div == div)
>> + return true;
>> + return false;
>> +}
>> +
>> +static bool _is_valid_div(struct clk_divider *divider, unsigned int div)
>> +{
>> + if (divider->flags& CLK_DIVIDER_POWER_OF_TWO)
>> + return is_power_of_two(div);
>> + if (divider->table)
>> + return _is_valid_table_div(divider->table, div);
>> + return true;
>> +}
>> +
>> static int clk_divider_bestdiv(struct clk_hw *hw, unsigned long rate,
>> unsigned long *best_parent_rate)
>> {
>> @@ -111,8 +170,7 @@ static int clk_divider_bestdiv(struct clk_hw *hw,
>> unsigned long rate,
>> maxdiv = min(ULONG_MAX / rate, maxdiv);
>>
>> for (i = 1; i<= maxdiv; i++) {
>> - if ((divider->flags& CLK_DIVIDER_POWER_OF_TWO)
>> - && (!is_power_of_two(i)))
>> + if (!_is_valid_div(divider, i))
>> continue;
>> parent_rate = __clk_round_rate(__clk_get_parent(hw->clk),
>> MULT_ROUND_UP(rate, i));
>> @@ -186,12 +244,14 @@ EXPORT_SYMBOL_GPL(clk_divider_ops);
>> * @shift: number of bits to shift the bitfield
>> * @width: width of the bitfield
>> * @clk_divider_flags: divider-specific flags for this clock
>> + * @table: array of divider/value pairs ending with a div set to 0
>> * @lock: shared register lock for this clock
>> */
>> struct clk *clk_register_divider(struct device *dev, const char *name,
>> const char *parent_name, unsigned long flags,
>> void __iomem *reg, u8 shift, u8 width,
>> - u8 clk_divider_flags, spinlock_t *lock)
>> + u8 clk_divider_flags, const struct clk_div_table *table,
>> + spinlock_t *lock)
>> {
>> struct clk_divider *div;
>> struct clk *clk;
>> @@ -217,6 +277,7 @@ struct clk *clk_register_divider(struct device
>> *dev, const char *name,
>> div->flags = clk_divider_flags;
>> div->lock = lock;
>> div->hw.init =&init;
>> + div->table = table;
>>
>> /* register the clock */
>> clk = clk_register(dev,&div->hw);
>> diff --git a/include/linux/clk-private.h b/include/linux/clk-private.h
>> index eb3f84b..2479239 100644
>> --- a/include/linux/clk-private.h
>> +++ b/include/linux/clk-private.h
>> @@ -105,7 +105,7 @@ struct clk {
>>
>> #define DEFINE_CLK_DIVIDER(_name, _parent_name, _parent_ptr, \
>> _flags, _reg, _shift, _width, \
>> - _divider_flags, _lock) \
>> + _divider_flags, _table, _lock) \
>> static struct clk _name; \
>> static const char *_name##_parent_names[] = { \
>> _parent_name, \
>> @@ -121,6 +121,7 @@ struct clk {
>> .shift = _shift, \
>> .width = _width, \
>> .flags = _divider_flags, \
>> + .table = _table, \
>> .lock = _lock, \
>> }; \
>> DEFINE_CLK(_name, clk_divider_ops, _flags, \
>> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
>> index 4a0b483..22bc067 100644
>> --- a/include/linux/clk-provider.h
>> +++ b/include/linux/clk-provider.h
>> @@ -203,6 +203,11 @@ struct clk *clk_register_gate(struct device *dev,
>> const char *name,
>> void __iomem *reg, u8 bit_idx,
>> u8 clk_gate_flags, spinlock_t *lock);
>>
>> +struct clk_div_table {
>> + unsigned int val;
>> + unsigned int div;
>> +};
>> +
>> /**
>> * struct clk_divider - adjustable divider clock
>> *
>> @@ -210,6 +215,7 @@ struct clk *clk_register_gate(struct device *dev,
>> const char *name,
>> * @reg: register containing the divider
>> * @shift: shift to the divider bit field
>> * @width: width of the divider bit field
>> + * @table: array of value/divider pairs, last entry should have div = 0
>> * @lock: register lock
>> *
>> * Clock with an adjustable divider affecting its output frequency.
>> Implements
>> @@ -229,6 +235,7 @@ struct clk_divider {
>> u8 shift;
>> u8 width;
>> u8 flags;
>> + const struct clk_div_table *table;
>> spinlock_t *lock;
>> };
>>
>> @@ -239,7 +246,8 @@ extern const struct clk_ops clk_divider_ops;
>> struct clk *clk_register_divider(struct device *dev, const char *name,
>> const char *parent_name, unsigned long flags,
>> void __iomem *reg, u8 shift, u8 width,
>> - u8 clk_divider_flags, spinlock_t *lock);
>> + u8 clk_divider_flags, const struct clk_div_table *table,
>> + spinlock_t *lock);
>>
>> /**
>> * struct clk_mux - multiplexer clock
>
>

2012-05-22 20:27:13

by Mike Turquette

[permalink] [raw]
Subject: Re: [PATCH 1/2] clk: Add support for power of two type dividers

On 20120517-15:52, Rajendra Nayak wrote:
> Quite often dividers and the value programmed in the
> register have a relation of 'power of two', something like
> value div
> 0 1
> 1 2
> 2 4
> 3 8...
>
> Add support for such dividers as part of clk-divider.
>
> The clk-divider flag 'CLK_DIVIDER_POWER_OF_TWO' should be used
> to define such clocks.
>
> Signed-off-by: Rajendra Nayak <[email protected]>

Hi Rajendra,

This patch looks good to me. I'll take it into my clk-next branch
towards 3.6 since I'm done pulling stuff for 3.5.

It's good to see CLK_DIVIDER_POWER_OF_TWO finally get used :-)

Regards,
Mike

2012-05-23 04:45:39

by Rajendra Nayak

[permalink] [raw]
Subject: Re: [PATCH 1/2] clk: Add support for power of two type dividers

On Wednesday 23 May 2012 01:56 AM, Mike Turquette wrote:
> On 20120517-15:52, Rajendra Nayak wrote:
>> Quite often dividers and the value programmed in the
>> register have a relation of 'power of two', something like
>> value div
>> 0 1
>> 1 2
>> 2 4
>> 3 8...
>>
>> Add support for such dividers as part of clk-divider.
>>
>> The clk-divider flag 'CLK_DIVIDER_POWER_OF_TWO' should be used
>> to define such clocks.
>>
>> Signed-off-by: Rajendra Nayak<[email protected]>
>
> Hi Rajendra,
>
> This patch looks good to me. I'll take it into my clk-next branch
> towards 3.6 since I'm done pulling stuff for 3.5.
>
> It's good to see CLK_DIVIDER_POWER_OF_TWO finally get used :-)

great, thanks Mike.

>
> Regards,
> Mike

2012-05-29 21:21:45

by Mike Turquette

[permalink] [raw]
Subject: Re: [PATCH 2/2] clk: Add support for rate table based dividers

On Thu, May 17, 2012 at 3:22 AM, Rajendra Nayak <[email protected]> wrote:
> Some divider clks do not have any obvious relationship
> between the divider and the value programmed in the
> register. For instance, say a value of 1 could signify divide
> by 6 and a value of 2 could signify divide by 4 etc.
> Also there are dividers where not all values possible
> based on the bitfield width are valid. For instance
> a 3 bit wide bitfield can be used to program a value
> from 0 to 7. However its possible that only 0 to 4
> are valid values.
>
> All these cases need the platform code to pass a simple
> table of divider/value tuple, so the framework knows
> the exact value to be written based on the divider
> calculation and can also do better error checking.
>
> This patch adds support for such rate table based
> dividers.
>
> Signed-off-by: Rajendra Nayak <[email protected]>

Thanks Rajendra. I'll take this into my clk-next.

Regards,
Mike

> ---
> ?drivers/clk/clk-divider.c ? ?| ? 67 ++++++++++++++++++++++++++++++++++++++++--
> ?include/linux/clk-private.h ?| ? ?3 +-
> ?include/linux/clk-provider.h | ? 10 +++++-
> ?3 files changed, 75 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
> index e548c43..e4911ee 100644
> --- a/drivers/clk/clk-divider.c
> +++ b/drivers/clk/clk-divider.c
> @@ -32,30 +32,69 @@
> ?#define div_mask(d) ? ?((1 << (d->width)) - 1)
> ?#define is_power_of_two(i) ? ? !(i & ~i)
>
> +static unsigned int _get_table_maxdiv(const struct clk_div_table *table)
> +{
> + ? ? ? unsigned int maxdiv;
> + ? ? ? const struct clk_div_table *clkt;
> +
> + ? ? ? for (clkt = table; clkt->div; clkt++)
> + ? ? ? ? ? ? ? if (clkt->div > maxdiv)
> + ? ? ? ? ? ? ? ? ? ? ? maxdiv = clkt->div;
> + ? ? ? return maxdiv;
> +}
> +
> ?static unsigned int _get_maxdiv(struct clk_divider *divider)
> ?{
> ? ? ? ?if (divider->flags & CLK_DIVIDER_ONE_BASED)
> ? ? ? ? ? ? ? ?return div_mask(divider);
> ? ? ? ?if (divider->flags & CLK_DIVIDER_POWER_OF_TWO)
> ? ? ? ? ? ? ? ?return 1 << div_mask(divider);
> + ? ? ? if (divider->table)
> + ? ? ? ? ? ? ? return _get_table_maxdiv(divider->table);
> ? ? ? ?return div_mask(divider) + 1;
> ?}
>
> +static unsigned int _get_table_div(const struct clk_div_table *table,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? unsigned int val)
> +{
> + ? ? ? const struct clk_div_table *clkt;
> +
> + ? ? ? for (clkt = table; clkt->div; clkt++)
> + ? ? ? ? ? ? ? if (clkt->val == val)
> + ? ? ? ? ? ? ? ? ? ? ? return clkt->div;
> + ? ? ? return 0;
> +}
> +
> ?static unsigned int _get_div(struct clk_divider *divider, unsigned int val)
> ?{
> ? ? ? ?if (divider->flags & CLK_DIVIDER_ONE_BASED)
> ? ? ? ? ? ? ? ?return val;
> ? ? ? ?if (divider->flags & CLK_DIVIDER_POWER_OF_TWO)
> ? ? ? ? ? ? ? ?return 1 << val;
> + ? ? ? if (divider->table)
> + ? ? ? ? ? ? ? return _get_table_div(divider->table, val);
> ? ? ? ?return val + 1;
> ?}
>
> +static unsigned int _get_table_val(const struct clk_div_table *table,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? unsigned int div)
> +{
> + ? ? ? const struct clk_div_table *clkt;
> +
> + ? ? ? for (clkt = table; clkt->div; clkt++)
> + ? ? ? ? ? ? ? if (clkt->div == div)
> + ? ? ? ? ? ? ? ? ? ? ? return clkt->val;
> + ? ? ? return 0;
> +}
> +
> ?static unsigned int _get_val(struct clk_divider *divider, u8 div)
> ?{
> ? ? ? ?if (divider->flags & CLK_DIVIDER_ONE_BASED)
> ? ? ? ? ? ? ? ?return div;
> ? ? ? ?if (divider->flags & CLK_DIVIDER_POWER_OF_TWO)
> ? ? ? ? ? ? ? ?return __ffs(div);
> + ? ? ? if (divider->table)
> + ? ? ? ? ? ? ? return ?_get_table_val(divider->table, div);
> ? ? ? ?return div - 1;
> ?}
>
> @@ -84,6 +123,26 @@ static unsigned long clk_divider_recalc_rate(struct clk_hw *hw,
> ?*/
> ?#define MULT_ROUND_UP(r, m) ((r) * (m) + (m) - 1)
>
> +static bool _is_valid_table_div(const struct clk_div_table *table,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?unsigned int div)
> +{
> + ? ? ? const struct clk_div_table *clkt;
> +
> + ? ? ? for (clkt = table; clkt->div; clkt++)
> + ? ? ? ? ? ? ? if (clkt->div == div)
> + ? ? ? ? ? ? ? ? ? ? ? return true;
> + ? ? ? return false;
> +}
> +
> +static bool _is_valid_div(struct clk_divider *divider, unsigned int div)
> +{
> + ? ? ? if (divider->flags & CLK_DIVIDER_POWER_OF_TWO)
> + ? ? ? ? ? ? ? return is_power_of_two(div);
> + ? ? ? if (divider->table)
> + ? ? ? ? ? ? ? return _is_valid_table_div(divider->table, div);
> + ? ? ? return true;
> +}
> +
> ?static int clk_divider_bestdiv(struct clk_hw *hw, unsigned long rate,
> ? ? ? ? ? ? ? ?unsigned long *best_parent_rate)
> ?{
> @@ -111,8 +170,7 @@ static int clk_divider_bestdiv(struct clk_hw *hw, unsigned long rate,
> ? ? ? ?maxdiv = min(ULONG_MAX / rate, maxdiv);
>
> ? ? ? ?for (i = 1; i <= maxdiv; i++) {
> - ? ? ? ? ? ? ? if ((divider->flags & CLK_DIVIDER_POWER_OF_TWO)
> - ? ? ? ? ? ? ? ? ? ? ? && (!is_power_of_two(i)))
> + ? ? ? ? ? ? ? if (!_is_valid_div(divider, i))
> ? ? ? ? ? ? ? ? ? ? ? ?continue;
> ? ? ? ? ? ? ? ?parent_rate = __clk_round_rate(__clk_get_parent(hw->clk),
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?MULT_ROUND_UP(rate, i));
> @@ -186,12 +244,14 @@ EXPORT_SYMBOL_GPL(clk_divider_ops);
> ?* @shift: number of bits to shift the bitfield
> ?* @width: width of the bitfield
> ?* @clk_divider_flags: divider-specific flags for this clock
> + * @table: array of divider/value pairs ending with a div set to 0
> ?* @lock: shared register lock for this clock
> ?*/
> ?struct clk *clk_register_divider(struct device *dev, const char *name,
> ? ? ? ? ? ? ? ?const char *parent_name, unsigned long flags,
> ? ? ? ? ? ? ? ?void __iomem *reg, u8 shift, u8 width,
> - ? ? ? ? ? ? ? u8 clk_divider_flags, spinlock_t *lock)
> + ? ? ? ? ? ? ? u8 clk_divider_flags, const struct clk_div_table *table,
> + ? ? ? ? ? ? ? spinlock_t *lock)
> ?{
> ? ? ? ?struct clk_divider *div;
> ? ? ? ?struct clk *clk;
> @@ -217,6 +277,7 @@ struct clk *clk_register_divider(struct device *dev, const char *name,
> ? ? ? ?div->flags = clk_divider_flags;
> ? ? ? ?div->lock = lock;
> ? ? ? ?div->hw.init = &init;
> + ? ? ? div->table = table;
>
> ? ? ? ?/* register the clock */
> ? ? ? ?clk = clk_register(dev, &div->hw);
> diff --git a/include/linux/clk-private.h b/include/linux/clk-private.h
> index eb3f84b..2479239 100644
> --- a/include/linux/clk-private.h
> +++ b/include/linux/clk-private.h
> @@ -105,7 +105,7 @@ struct clk {
>
> ?#define DEFINE_CLK_DIVIDER(_name, _parent_name, _parent_ptr, ? \
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?_flags, _reg, _shift, _width, ? \
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? _divider_flags, _lock) ? ? ? ? ?\
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? _divider_flags, _table, _lock) ?\
> ? ? ? ?static struct clk _name; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
> ? ? ? ?static const char *_name##_parent_names[] = { ? ? ? ? ? \
> ? ? ? ? ? ? ? ?_parent_name, ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
> @@ -121,6 +121,7 @@ struct clk {
> ? ? ? ? ? ? ? ?.shift = _shift, ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
> ? ? ? ? ? ? ? ?.width = _width, ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
> ? ? ? ? ? ? ? ?.flags = _divider_flags, ? ? ? ? ? ? ? ? ? ? ? ?\
> + ? ? ? ? ? ? ? .table = _table, ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
> ? ? ? ? ? ? ? ?.lock = _lock, ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
> ? ? ? ?}; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
> ? ? ? ?DEFINE_CLK(_name, clk_divider_ops, _flags, ? ? ? ? ? ? ?\
> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
> index 4a0b483..22bc067 100644
> --- a/include/linux/clk-provider.h
> +++ b/include/linux/clk-provider.h
> @@ -203,6 +203,11 @@ struct clk *clk_register_gate(struct device *dev, const char *name,
> ? ? ? ? ? ? ? ?void __iomem *reg, u8 bit_idx,
> ? ? ? ? ? ? ? ?u8 clk_gate_flags, spinlock_t *lock);
>
> +struct clk_div_table {
> + ? ? ? unsigned int ? ?val;
> + ? ? ? unsigned int ? ?div;
> +};
> +
> ?/**
> ?* struct clk_divider - adjustable divider clock
> ?*
> @@ -210,6 +215,7 @@ struct clk *clk_register_gate(struct device *dev, const char *name,
> ?* @reg: ? ? ? register containing the divider
> ?* @shift: ? ? shift to the divider bit field
> ?* @width: ? ? width of the divider bit field
> + * @table: ? ? array of value/divider pairs, last entry should have div = 0
> ?* @lock: ? ? ?register lock
> ?*
> ?* Clock with an adjustable divider affecting its output frequency. ?Implements
> @@ -229,6 +235,7 @@ struct clk_divider {
> ? ? ? ?u8 ? ? ? ? ? ? ?shift;
> ? ? ? ?u8 ? ? ? ? ? ? ?width;
> ? ? ? ?u8 ? ? ? ? ? ? ?flags;
> + ? ? ? const struct clk_div_table ? ? ?*table;
> ? ? ? ?spinlock_t ? ? ?*lock;
> ?};
>
> @@ -239,7 +246,8 @@ extern const struct clk_ops clk_divider_ops;
> ?struct clk *clk_register_divider(struct device *dev, const char *name,
> ? ? ? ? ? ? ? ?const char *parent_name, unsigned long flags,
> ? ? ? ? ? ? ? ?void __iomem *reg, u8 shift, u8 width,
> - ? ? ? ? ? ? ? u8 clk_divider_flags, spinlock_t *lock);
> + ? ? ? ? ? ? ? u8 clk_divider_flags, const struct clk_div_table *table,
> + ? ? ? ? ? ? ? spinlock_t *lock);
>
> ?/**
> ?* struct clk_mux - multiplexer clock
> --
> 1.7.1
>

2012-05-30 09:24:45

by Rajendra Nayak

[permalink] [raw]
Subject: Re: [PATCH 2/2] clk: Add support for rate table based dividers

On Wednesday 30 May 2012 02:45 AM, Turquette, Mike wrote:
> On Thu, May 17, 2012 at 3:22 AM, Rajendra Nayak<[email protected]> wrote:
>> Some divider clks do not have any obvious relationship
>> between the divider and the value programmed in the
>> register. For instance, say a value of 1 could signify divide
>> by 6 and a value of 2 could signify divide by 4 etc.
>> Also there are dividers where not all values possible
>> based on the bitfield width are valid. For instance
>> a 3 bit wide bitfield can be used to program a value
>> from 0 to 7. However its possible that only 0 to 4
>> are valid values.
>>
>> All these cases need the platform code to pass a simple
>> table of divider/value tuple, so the framework knows
>> the exact value to be written based on the divider
>> calculation and can also do better error checking.
>>
>> This patch adds support for such rate table based
>> dividers.
>>
>> Signed-off-by: Rajendra Nayak<[email protected]>
>
> Thanks Rajendra. I'll take this into my clk-next.

Mike,

Since this adds one additional parameter to clk_register_divider(),
it means fixing all existing users going in this merge window.
I can resend this patch fixing all the users once 3.5-rc1 is out and
you have moved clk-next, which you can then pull in.

regards,
Rajendra

>
> Regards,
> Mike
>
>> ---
>> drivers/clk/clk-divider.c | 67 ++++++++++++++++++++++++++++++++++++++++--
>> include/linux/clk-private.h | 3 +-
>> include/linux/clk-provider.h | 10 +++++-
>> 3 files changed, 75 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
>> index e548c43..e4911ee 100644
>> --- a/drivers/clk/clk-divider.c
>> +++ b/drivers/clk/clk-divider.c
>> @@ -32,30 +32,69 @@
>> #define div_mask(d) ((1<< (d->width)) - 1)
>> #define is_power_of_two(i) !(i& ~i)
>>
>> +static unsigned int _get_table_maxdiv(const struct clk_div_table *table)
>> +{
>> + unsigned int maxdiv;
>> + const struct clk_div_table *clkt;
>> +
>> + for (clkt = table; clkt->div; clkt++)
>> + if (clkt->div> maxdiv)
>> + maxdiv = clkt->div;
>> + return maxdiv;
>> +}
>> +
>> static unsigned int _get_maxdiv(struct clk_divider *divider)
>> {
>> if (divider->flags& CLK_DIVIDER_ONE_BASED)
>> return div_mask(divider);
>> if (divider->flags& CLK_DIVIDER_POWER_OF_TWO)
>> return 1<< div_mask(divider);
>> + if (divider->table)
>> + return _get_table_maxdiv(divider->table);
>> return div_mask(divider) + 1;
>> }
>>
>> +static unsigned int _get_table_div(const struct clk_div_table *table,
>> + unsigned int val)
>> +{
>> + const struct clk_div_table *clkt;
>> +
>> + for (clkt = table; clkt->div; clkt++)
>> + if (clkt->val == val)
>> + return clkt->div;
>> + return 0;
>> +}
>> +
>> static unsigned int _get_div(struct clk_divider *divider, unsigned int val)
>> {
>> if (divider->flags& CLK_DIVIDER_ONE_BASED)
>> return val;
>> if (divider->flags& CLK_DIVIDER_POWER_OF_TWO)
>> return 1<< val;
>> + if (divider->table)
>> + return _get_table_div(divider->table, val);
>> return val + 1;
>> }
>>
>> +static unsigned int _get_table_val(const struct clk_div_table *table,
>> + unsigned int div)
>> +{
>> + const struct clk_div_table *clkt;
>> +
>> + for (clkt = table; clkt->div; clkt++)
>> + if (clkt->div == div)
>> + return clkt->val;
>> + return 0;
>> +}
>> +
>> static unsigned int _get_val(struct clk_divider *divider, u8 div)
>> {
>> if (divider->flags& CLK_DIVIDER_ONE_BASED)
>> return div;
>> if (divider->flags& CLK_DIVIDER_POWER_OF_TWO)
>> return __ffs(div);
>> + if (divider->table)
>> + return _get_table_val(divider->table, div);
>> return div - 1;
>> }
>>
>> @@ -84,6 +123,26 @@ static unsigned long clk_divider_recalc_rate(struct clk_hw *hw,
>> */
>> #define MULT_ROUND_UP(r, m) ((r) * (m) + (m) - 1)
>>
>> +static bool _is_valid_table_div(const struct clk_div_table *table,
>> + unsigned int div)
>> +{
>> + const struct clk_div_table *clkt;
>> +
>> + for (clkt = table; clkt->div; clkt++)
>> + if (clkt->div == div)
>> + return true;
>> + return false;
>> +}
>> +
>> +static bool _is_valid_div(struct clk_divider *divider, unsigned int div)
>> +{
>> + if (divider->flags& CLK_DIVIDER_POWER_OF_TWO)
>> + return is_power_of_two(div);
>> + if (divider->table)
>> + return _is_valid_table_div(divider->table, div);
>> + return true;
>> +}
>> +
>> static int clk_divider_bestdiv(struct clk_hw *hw, unsigned long rate,
>> unsigned long *best_parent_rate)
>> {
>> @@ -111,8 +170,7 @@ static int clk_divider_bestdiv(struct clk_hw *hw, unsigned long rate,
>> maxdiv = min(ULONG_MAX / rate, maxdiv);
>>
>> for (i = 1; i<= maxdiv; i++) {
>> - if ((divider->flags& CLK_DIVIDER_POWER_OF_TWO)
>> -&& (!is_power_of_two(i)))
>> + if (!_is_valid_div(divider, i))
>> continue;
>> parent_rate = __clk_round_rate(__clk_get_parent(hw->clk),
>> MULT_ROUND_UP(rate, i));
>> @@ -186,12 +244,14 @@ EXPORT_SYMBOL_GPL(clk_divider_ops);
>> * @shift: number of bits to shift the bitfield
>> * @width: width of the bitfield
>> * @clk_divider_flags: divider-specific flags for this clock
>> + * @table: array of divider/value pairs ending with a div set to 0
>> * @lock: shared register lock for this clock
>> */
>> struct clk *clk_register_divider(struct device *dev, const char *name,
>> const char *parent_name, unsigned long flags,
>> void __iomem *reg, u8 shift, u8 width,
>> - u8 clk_divider_flags, spinlock_t *lock)
>> + u8 clk_divider_flags, const struct clk_div_table *table,
>> + spinlock_t *lock)
>> {
>> struct clk_divider *div;
>> struct clk *clk;
>> @@ -217,6 +277,7 @@ struct clk *clk_register_divider(struct device *dev, const char *name,
>> div->flags = clk_divider_flags;
>> div->lock = lock;
>> div->hw.init =&init;
>> + div->table = table;
>>
>> /* register the clock */
>> clk = clk_register(dev,&div->hw);
>> diff --git a/include/linux/clk-private.h b/include/linux/clk-private.h
>> index eb3f84b..2479239 100644
>> --- a/include/linux/clk-private.h
>> +++ b/include/linux/clk-private.h
>> @@ -105,7 +105,7 @@ struct clk {
>>
>> #define DEFINE_CLK_DIVIDER(_name, _parent_name, _parent_ptr, \
>> _flags, _reg, _shift, _width, \
>> - _divider_flags, _lock) \
>> + _divider_flags, _table, _lock) \
>> static struct clk _name; \
>> static const char *_name##_parent_names[] = { \
>> _parent_name, \
>> @@ -121,6 +121,7 @@ struct clk {
>> .shift = _shift, \
>> .width = _width, \
>> .flags = _divider_flags, \
>> + .table = _table, \
>> .lock = _lock, \
>> }; \
>> DEFINE_CLK(_name, clk_divider_ops, _flags, \
>> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
>> index 4a0b483..22bc067 100644
>> --- a/include/linux/clk-provider.h
>> +++ b/include/linux/clk-provider.h
>> @@ -203,6 +203,11 @@ struct clk *clk_register_gate(struct device *dev, const char *name,
>> void __iomem *reg, u8 bit_idx,
>> u8 clk_gate_flags, spinlock_t *lock);
>>
>> +struct clk_div_table {
>> + unsigned int val;
>> + unsigned int div;
>> +};
>> +
>> /**
>> * struct clk_divider - adjustable divider clock
>> *
>> @@ -210,6 +215,7 @@ struct clk *clk_register_gate(struct device *dev, const char *name,
>> * @reg: register containing the divider
>> * @shift: shift to the divider bit field
>> * @width: width of the divider bit field
>> + * @table: array of value/divider pairs, last entry should have div = 0
>> * @lock: register lock
>> *
>> * Clock with an adjustable divider affecting its output frequency. Implements
>> @@ -229,6 +235,7 @@ struct clk_divider {
>> u8 shift;
>> u8 width;
>> u8 flags;
>> + const struct clk_div_table *table;
>> spinlock_t *lock;
>> };
>>
>> @@ -239,7 +246,8 @@ extern const struct clk_ops clk_divider_ops;
>> struct clk *clk_register_divider(struct device *dev, const char *name,
>> const char *parent_name, unsigned long flags,
>> void __iomem *reg, u8 shift, u8 width,
>> - u8 clk_divider_flags, spinlock_t *lock);
>> + u8 clk_divider_flags, const struct clk_div_table *table,
>> + spinlock_t *lock);
>>
>> /**
>> * struct clk_mux - multiplexer clock
>> --
>> 1.7.1
>>

2012-05-31 04:05:22

by Saravana Kannan

[permalink] [raw]
Subject: Re: [PATCH 2/2] clk: Add support for rate table based dividers

On 05/21/2012 09:45 PM, Rajendra Nayak wrote:
> Hi Ben,
>
> On Monday 21 May 2012 03:17 PM, Ben Dooks wrote:
>> On 17/05/12 11:22, Rajendra Nayak wrote:
>>> Some divider clks do not have any obvious relationship
>>> between the divider and the value programmed in the
>>> register. For instance, say a value of 1 could signify divide
>>> by 6 and a value of 2 could signify divide by 4 etc.
>>> Also there are dividers where not all values possible
>>> based on the bitfield width are valid. For instance
>>> a 3 bit wide bitfield can be used to program a value
>>> from 0 to 7. However its possible that only 0 to 4
>>> are valid values.
>>>
>>> All these cases need the platform code to pass a simple
>>> table of divider/value tuple, so the framework knows
>>> the exact value to be written based on the divider
>>> calculation and can also do better error checking.
>>>
>>> This patch adds support for such rate table based
>>> dividers.
>>
>> I was considering the idea that you simply pass a
>> pointer to a set of routines and a data pointer to
>> the clk-divider code so that any new cases don't
>> require changing the drivers/clk/clk-divider.c
>
> I don;t know if I understand your comment completely.
> Are you suggesting the get min/max etc be function pointers
> passed by platform code (and implemented in platform code?)
> so clk-divider does not need an update every time a new divider
> type is added?
> The idea of extending clk-divider was so its useful for more
> than just OMAP, so the code in clk-divider can be reused across
> multiple platforms. Did I understand your comment right?
>
> regards,
> Rajendra
>
>>
>> This would make the get max / min / special just
>> a function call through a struct.
>>
>>> Signed-off-by: Rajendra Nayak<[email protected]>
>>> ---
>>> drivers/clk/clk-divider.c | 67
>>> ++++++++++++++++++++++++++++++++++++++++--
>>> include/linux/clk-private.h | 3 +-
>>> include/linux/clk-provider.h | 10 +++++-
>>> 3 files changed, 75 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
>>> index e548c43..e4911ee 100644
>>> --- a/drivers/clk/clk-divider.c
>>> +++ b/drivers/clk/clk-divider.c
>>> @@ -32,30 +32,69 @@
>>> #define div_mask(d) ((1<< (d->width)) - 1)
>>> #define is_power_of_two(i) !(i& ~i)
>>>
>>> +static unsigned int _get_table_maxdiv(const struct clk_div_table
>>> *table)
>>> +{
>>> + unsigned int maxdiv;
>>> + const struct clk_div_table *clkt;
>>> +
>>> + for (clkt = table; clkt->div; clkt++)
>>> + if (clkt->div> maxdiv)
>>> + maxdiv = clkt->div;
>>> + return maxdiv;
>>> +}
>>> +
>>> static unsigned int _get_maxdiv(struct clk_divider *divider)
>>> {
>>> if (divider->flags& CLK_DIVIDER_ONE_BASED)
>>> return div_mask(divider);
>>> if (divider->flags& CLK_DIVIDER_POWER_OF_TWO)
>>> return 1<< div_mask(divider);

Where are these flags defined? I don't see it in any of the patches in
the series. Is my search foo not up to par today?

I think what Ben is saying is that you provider a way (using function or
data/table pointers in clk_divider) that will allow the clk provider to
define a "divider" to "register value" mapping. Say you decide to do
that using a function pointer, then you would implement the following in
clk-divider.c.

div_to_reg_one_based
div_to_reg_pow_two

The actual clock-provider code will pick one of these or implement their
own mapping function. That way, clk-divider won't have to change for any
other convoluted variants of clk divider to register value mapping.

-Saravana

--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

2012-06-01 04:40:43

by Rajendra Nayak

[permalink] [raw]
Subject: Re: [PATCH 2/2] clk: Add support for rate table based dividers

On Thursday 31 May 2012 09:35 AM, Saravana Kannan wrote:
> On 05/21/2012 09:45 PM, Rajendra Nayak wrote:
>> Hi Ben,
>>
>> On Monday 21 May 2012 03:17 PM, Ben Dooks wrote:
>>> On 17/05/12 11:22, Rajendra Nayak wrote:
>>>> Some divider clks do not have any obvious relationship
>>>> between the divider and the value programmed in the
>>>> register. For instance, say a value of 1 could signify divide
>>>> by 6 and a value of 2 could signify divide by 4 etc.
>>>> Also there are dividers where not all values possible
>>>> based on the bitfield width are valid. For instance
>>>> a 3 bit wide bitfield can be used to program a value
>>>> from 0 to 7. However its possible that only 0 to 4
>>>> are valid values.
>>>>
>>>> All these cases need the platform code to pass a simple
>>>> table of divider/value tuple, so the framework knows
>>>> the exact value to be written based on the divider
>>>> calculation and can also do better error checking.
>>>>
>>>> This patch adds support for such rate table based
>>>> dividers.
>>>
>>> I was considering the idea that you simply pass a
>>> pointer to a set of routines and a data pointer to
>>> the clk-divider code so that any new cases don't
>>> require changing the drivers/clk/clk-divider.c
>>
>> I don;t know if I understand your comment completely.
>> Are you suggesting the get min/max etc be function pointers
>> passed by platform code (and implemented in platform code?)
>> so clk-divider does not need an update every time a new divider
>> type is added?
>> The idea of extending clk-divider was so its useful for more
>> than just OMAP, so the code in clk-divider can be reused across
>> multiple platforms. Did I understand your comment right?
>>
>> regards,
>> Rajendra
>>
>>>
>>> This would make the get max / min / special just
>>> a function call through a struct.
>>>
>>>> Signed-off-by: Rajendra Nayak<[email protected]>
>>>> ---
>>>> drivers/clk/clk-divider.c | 67
>>>> ++++++++++++++++++++++++++++++++++++++++--
>>>> include/linux/clk-private.h | 3 +-
>>>> include/linux/clk-provider.h | 10 +++++-
>>>> 3 files changed, 75 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
>>>> index e548c43..e4911ee 100644
>>>> --- a/drivers/clk/clk-divider.c
>>>> +++ b/drivers/clk/clk-divider.c
>>>> @@ -32,30 +32,69 @@
>>>> #define div_mask(d) ((1<< (d->width)) - 1)
>>>> #define is_power_of_two(i) !(i& ~i)
>>>>
>>>> +static unsigned int _get_table_maxdiv(const struct clk_div_table
>>>> *table)
>>>> +{
>>>> + unsigned int maxdiv;
>>>> + const struct clk_div_table *clkt;
>>>> +
>>>> + for (clkt = table; clkt->div; clkt++)
>>>> + if (clkt->div> maxdiv)
>>>> + maxdiv = clkt->div;
>>>> + return maxdiv;
>>>> +}
>>>> +
>>>> static unsigned int _get_maxdiv(struct clk_divider *divider)
>>>> {
>>>> if (divider->flags& CLK_DIVIDER_ONE_BASED)
>>>> return div_mask(divider);
>>>> if (divider->flags& CLK_DIVIDER_POWER_OF_TWO)
>>>> return 1<< div_mask(divider);
>
> Where are these flags defined? I don't see it in any of the patches in
> the series. Is my search foo not up to par today?

Well, the flag was already part of Mike's original series, it was just
not used.

>
> I think what Ben is saying is that you provider a way (using function or
> data/table pointers in clk_divider) that will allow the clk provider to
> define a "divider" to "register value" mapping. Say you decide to do
> that using a function pointer, then you would implement the following in
> clk-divider.c.
>
> div_to_reg_one_based
> div_to_reg_pow_two
>
> The actual clock-provider code will pick one of these or implement their
> own mapping function. That way, clk-divider won't have to change for any
> other convoluted variants of clk divider to register value mapping.

I get the point. But I am just wondering if there are any such
convoluted variants (which are platform specific).
I though the table should be able to handle *all* such variants.

>
> -Saravana
>