* Re: [PATCH 3/3] ARM: mach-shmobile: clock-sh7372: Add FSIDIV clock support
2010-10-15 5:15 [PATCH 3/3] ARM: mach-shmobile: clock-sh7372: Add FSIDIV clock support Kuninori Morimoto
@ 2010-10-15 6:16 ` Paul Mundt
2010-10-15 6:42 ` [PATCH 3/3] ARM: mach-shmobile: clock-sh7372: Add FSIDIV clock Kuninori Morimoto
` (6 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Paul Mundt @ 2010-10-15 6:16 UTC (permalink / raw)
To: linux-sh
On Fri, Oct 15, 2010 at 02:15:14PM +0900, Kuninori Morimoto wrote:
> +static long fsidiv_round_rate(struct clk *clk, unsigned long rate)
> +{
> + /*
> + * FSIDIV doesn't have freq_table.
> + * it mean fsidiv can not use clk_rate_table_round.
> + * self calculate here
> + */
Is there some particular reason why you can't just construct a rate table
for FSIDIV instead? sh_clk_div6_register_ops() in drivers/sh/clk-cpg.c is
an example of how to set it up. It's provided generically in the struct
clk largely for these sorts of cases.
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 3/3] ARM: mach-shmobile: clock-sh7372: Add FSIDIV clock
2010-10-15 5:15 [PATCH 3/3] ARM: mach-shmobile: clock-sh7372: Add FSIDIV clock support Kuninori Morimoto
2010-10-15 6:16 ` Paul Mundt
@ 2010-10-15 6:42 ` Kuninori Morimoto
2010-10-15 9:39 ` [PATCH 3/3] ARM: mach-shmobile: clock-sh7372: Add FSIDIV clock support Paul Mundt
` (5 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Kuninori Morimoto @ 2010-10-15 6:42 UTC (permalink / raw)
To: linux-sh
Dear Paul
> Is there some particular reason why you can't just construct a rate table
> for FSIDIV instead? sh_clk_div6_register_ops() in drivers/sh/clk-cpg.c is
> an example of how to set it up. It's provided generically in the struct
> clk largely for these sorts of cases.
Sorry I didn't explain for detail.
The reason is treq table size.
FSIDIV dividing frequency bit is 16.
And struct cpufreq_frequency_table size is 8byte.
So, the table size will be 8 x 65535 = 512K byte if FSIDIV have it.
(ex) div6 case is 8 x 64 = 512 byte
div4 case is 8 x 16 = 128 byte
I thought 512K byte is too big for kernel.
Best regards
--
Kuninori Morimoto
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 3/3] ARM: mach-shmobile: clock-sh7372: Add FSIDIV clock support
2010-10-15 5:15 [PATCH 3/3] ARM: mach-shmobile: clock-sh7372: Add FSIDIV clock support Kuninori Morimoto
2010-10-15 6:16 ` Paul Mundt
2010-10-15 6:42 ` [PATCH 3/3] ARM: mach-shmobile: clock-sh7372: Add FSIDIV clock Kuninori Morimoto
@ 2010-10-15 9:39 ` Paul Mundt
2010-10-15 15:50 ` Paul Mundt
` (4 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Paul Mundt @ 2010-10-15 9:39 UTC (permalink / raw)
To: linux-sh
On Fri, Oct 15, 2010 at 03:42:55PM +0900, Kuninori Morimoto wrote:
> > Is there some particular reason why you can't just construct a rate table
> > for FSIDIV instead? sh_clk_div6_register_ops() in drivers/sh/clk-cpg.c is
> > an example of how to set it up. It's provided generically in the struct
> > clk largely for these sorts of cases.
>
> Sorry I didn't explain for detail.
> The reason is treq table size.
>
> FSIDIV dividing frequency bit is 16.
> And struct cpufreq_frequency_table size is 8byte.
>
> So, the table size will be 8 x 65535 = 512K byte if FSIDIV have it.
> (ex) div6 case is 8 x 64 = 512 byte
> div4 case is 8 x 16 = 128 byte
>
> I thought 512K byte is too big for kernel.
>
Ok, that's certainly a valid reason. As we'll presumably see more of
these in the future, how about something like this?
---
diff --git a/drivers/sh/clk.c b/drivers/sh/clk.c
index 813d97c..018be37 100644
--- a/drivers/sh/clk.c
+++ b/drivers/sh/clk.c
@@ -45,6 +45,8 @@ void clk_rate_table_build(struct clk *clk,
unsigned long freq;
int i;
+ clk->nr_freqs = nr_freqs;
+
for (i = 0; i < nr_freqs; i++) {
div = 1;
mult = 1;
@@ -69,30 +71,39 @@ void clk_rate_table_build(struct clk *clk,
freq_table[i].frequency = CPUFREQ_TABLE_END;
}
-long clk_rate_table_round(struct clk *clk,
- struct cpufreq_frequency_table *freq_table,
- unsigned long rate)
+struct clk_rate_round_data;
+
+struct clk_rate_round_data {
+ unsigned long rate;
+ unsigned int min, max;
+ long (*func)(unsigned int, struct clk_rate_round_data *);
+ void *arg;
+};
+
+#define for_each_frequency(pos, r, freq) \
+ for (pos = r->min, freq = r->func(pos, r->arg); \
+ pos < r->max; pos++, freq = r->func(pos, r)) \
+ if (unlikely(freq = 0)) \
+ ; \
+ else
+
+static long clk_rate_round_helper(struct clk_rate_round_data *rounder)
{
unsigned long rate_error, rate_error_prev = ~0UL;
- unsigned long rate_best_fit = rate;
- unsigned long highest, lowest;
+ unsigned long rate_best_fit = rounder->rate;
+ unsigned long highest, lowest, freq;
int i;
highest = 0;
lowest = ~0UL;
- for (i = 0; freq_table[i].frequency != CPUFREQ_TABLE_END; i++) {
- unsigned long freq = freq_table[i].frequency;
-
- if (freq = CPUFREQ_ENTRY_INVALID)
- continue;
-
+ for_each_frequency(i, rounder, freq) {
if (freq > highest)
highest = freq;
if (freq < lowest)
lowest = freq;
- rate_error = abs(freq - rate);
+ rate_error = abs(freq - rounder->rate);
if (rate_error < rate_error_prev) {
rate_best_fit = freq;
rate_error_prev = rate_error;
@@ -102,14 +113,61 @@ long clk_rate_table_round(struct clk *clk,
break;
}
- if (rate >= highest)
+ if (rounder->rate >= highest)
rate_best_fit = highest;
- if (rate <= lowest)
+ if (rounder->rate <= lowest)
rate_best_fit = lowest;
return rate_best_fit;
}
+static long clk_rate_table_iter(unsigned int pos,
+ struct clk_rate_round_data *rounder)
+{
+ struct cpufreq_frequency_table *freq_table = rounder->arg;
+ unsigned long freq = freq_table[pos].frequency;
+
+ if (freq = CPUFREQ_ENTRY_INVALID)
+ freq = 0;
+
+ return freq;
+}
+
+long clk_rate_table_round(struct clk *clk,
+ struct cpufreq_frequency_table *freq_table,
+ unsigned long rate)
+{
+ struct clk_rate_round_data table_round = {
+ .min = 0,
+ .max = clk->nr_freqs,
+ .func = clk_rate_table_iter,
+ .arg = freq_table,
+ .rate = rate,
+ };
+
+ return clk_rate_round_helper(&table_round);
+}
+
+static long clk_rate_div_range_iter(unsigned int pos,
+ struct clk_rate_round_data *rounder)
+{
+ return clk_get_rate(rounder->arg) / pos;
+}
+
+long clk_rate_div_range_round(struct clk *clk, unsigned int div_min,
+ unsigned int div_max, unsigned long rate)
+{
+ struct clk_rate_round_data div_range_round = {
+ .min = div_min,
+ .max = div_max,
+ .func = clk_rate_div_range_iter,
+ .arg = clk_get_parent(clk),
+ .rate = rate,
+ };
+
+ return clk_rate_round_helper(&div_range_round);
+}
+
int clk_rate_table_find(struct clk *clk,
struct cpufreq_frequency_table *freq_table,
unsigned long rate)
diff --git a/include/linux/sh_clk.h b/include/linux/sh_clk.h
index 8ae3770..4dca992 100644
--- a/include/linux/sh_clk.h
+++ b/include/linux/sh_clk.h
@@ -53,6 +53,7 @@ struct clk {
struct dentry *dentry;
struct clk_mapping *mapping;
struct cpufreq_frequency_table *freq_table;
+ unsigned int nr_freqs;
};
#define CLK_ENABLE_ON_INIT (1 << 0)
@@ -118,6 +119,9 @@ int clk_rate_table_find(struct clk *clk,
struct cpufreq_frequency_table *freq_table,
unsigned long rate);
+long clk_rate_div_range_round(struct clk *clk, unsigned int div_min,
+ unsigned int div_max, unsigned long rate);
+
#define SH_CLK_MSTP32(_parent, _enable_reg, _enable_bit, _flags) \
{ \
.parent = _parent, \
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 3/3] ARM: mach-shmobile: clock-sh7372: Add FSIDIV clock support
2010-10-15 5:15 [PATCH 3/3] ARM: mach-shmobile: clock-sh7372: Add FSIDIV clock support Kuninori Morimoto
` (2 preceding siblings ...)
2010-10-15 9:39 ` [PATCH 3/3] ARM: mach-shmobile: clock-sh7372: Add FSIDIV clock support Paul Mundt
@ 2010-10-15 15:50 ` Paul Mundt
2010-10-18 3:49 ` [PATCH 3/3] ARM: mach-shmobile: clock-sh7372: Add FSIDIV clock Kuninori Morimoto
` (3 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Paul Mundt @ 2010-10-15 15:50 UTC (permalink / raw)
To: linux-sh
On Fri, Oct 15, 2010 at 06:39:23PM +0900, Paul Mundt wrote:
> On Fri, Oct 15, 2010 at 03:42:55PM +0900, Kuninori Morimoto wrote:
> > So, the table size will be 8 x 65535 = 512K byte if FSIDIV have it.
> > (ex) div6 case is 8 x 64 = 512 byte
> > div4 case is 8 x 16 = 128 byte
> >
> > I thought 512K byte is too big for kernel.
> >
> Ok, that's certainly a valid reason. As we'll presumably see more of
> these in the future, how about something like this?
>
> +#define for_each_frequency(pos, r, freq) \
> + for (pos = r->min, freq = r->func(pos, r->arg); \
> + pos < r->max; pos++, freq = r->func(pos, r)) \
> + if (unlikely(freq = 0)) \
> + ; \
> + else
> +
Oops, the first r->arg needs to be changed to r or you'll blow up on the
table rounding. I've got all of these in my sh/clkfwk topic branch if you
simply wish to test that.
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 3/3] ARM: mach-shmobile: clock-sh7372: Add FSIDIV clock
2010-10-15 5:15 [PATCH 3/3] ARM: mach-shmobile: clock-sh7372: Add FSIDIV clock support Kuninori Morimoto
` (3 preceding siblings ...)
2010-10-15 15:50 ` Paul Mundt
@ 2010-10-18 3:49 ` Kuninori Morimoto
2010-10-18 8:56 ` [PATCH 3/3] ARM: mach-shmobile: clock-sh7372: Add FSIDIV clock support Paul Mundt
` (2 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Kuninori Morimoto @ 2010-10-18 3:49 UTC (permalink / raw)
To: linux-sh
Dear Paul
> On Fri, Oct 15, 2010 at 06:39:23PM +0900, Paul Mundt wrote:
> > On Fri, Oct 15, 2010 at 03:42:55PM +0900, Kuninori Morimoto wrote:
> > > So, the table size will be 8 x 65535 = 512K byte if FSIDIV have it.
> > > (ex) div6 case is 8 x 64 = 512 byte
> > > div4 case is 8 x 16 = 128 byte
> > >
> > > I thought 512K byte is too big for kernel.
> > >
> > Ok, that's certainly a valid reason. As we'll presumably see more of
> > these in the future, how about something like this?
> >
> > +#define for_each_frequency(pos, r, freq) \
> > + for (pos = r->min, freq = r->func(pos, r->arg); \
> > + pos < r->max; pos++, freq = r->func(pos, r)) \
> > + if (unlikely(freq = 0)) \
> > + ; \
> > + else
> > +
> Oops, the first r->arg needs to be changed to r or you'll blow up on the
> table rounding. I've got all of these in my sh/clkfwk topic branch if you
> simply wish to test that.
Thank you.
These patches are based on genesis tree + sh/clkfwk.
Kuninori Morimoto (2):
sh: clkfwk: modify for_each_frequency end condition
ARM: mach-shmobile: clock-sh7372: Add FSIDIV clock support
I think 1st patch is needef to solve off-by-one issue.
Best regards
--
Kuninori Morimoto
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 3/3] ARM: mach-shmobile: clock-sh7372: Add FSIDIV clock support
2010-10-15 5:15 [PATCH 3/3] ARM: mach-shmobile: clock-sh7372: Add FSIDIV clock support Kuninori Morimoto
` (4 preceding siblings ...)
2010-10-18 3:49 ` [PATCH 3/3] ARM: mach-shmobile: clock-sh7372: Add FSIDIV clock Kuninori Morimoto
@ 2010-10-18 8:56 ` Paul Mundt
2010-10-18 10:10 ` [PATCH 3/3] ARM: mach-shmobile: clock-sh7372: Add FSIDIV clock Kuninori Morimoto
2010-10-18 10:12 ` [PATCH 3/3] ARM: mach-shmobile: clock-sh7372: Add FSIDIV clock support Paul Mundt
7 siblings, 0 replies; 9+ messages in thread
From: Paul Mundt @ 2010-10-18 8:56 UTC (permalink / raw)
To: linux-sh
On Mon, Oct 18, 2010 at 12:49:44PM +0900, Kuninori Morimoto wrote:
> > On Fri, Oct 15, 2010 at 06:39:23PM +0900, Paul Mundt wrote:
> > Oops, the first r->arg needs to be changed to r or you'll blow up on the
> > table rounding. I've got all of these in my sh/clkfwk topic branch if you
> > simply wish to test that.
>
> Thank you.
> These patches are based on genesis tree + sh/clkfwk.
>
> Kuninori Morimoto (2):
> sh: clkfwk: modify for_each_frequency end condition
> ARM: mach-shmobile: clock-sh7372: Add FSIDIV clock support
>
> I think 1st patch is needef to solve off-by-one issue.
>
I'll take the first one in the sh/clkfwk topic branch and merge it in to
HEAD. The remaining FSIDIV patch I'll then roll in to the genesis tree
once the clock bits have hit mainline. Hopefully this will all be sorted
by late -rc1 or -rc2. Thanks for testing!
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 3/3] ARM: mach-shmobile: clock-sh7372: Add FSIDIV clock
2010-10-15 5:15 [PATCH 3/3] ARM: mach-shmobile: clock-sh7372: Add FSIDIV clock support Kuninori Morimoto
` (5 preceding siblings ...)
2010-10-18 8:56 ` [PATCH 3/3] ARM: mach-shmobile: clock-sh7372: Add FSIDIV clock support Paul Mundt
@ 2010-10-18 10:10 ` Kuninori Morimoto
2010-10-18 10:12 ` [PATCH 3/3] ARM: mach-shmobile: clock-sh7372: Add FSIDIV clock support Paul Mundt
7 siblings, 0 replies; 9+ messages in thread
From: Kuninori Morimoto @ 2010-10-18 10:10 UTC (permalink / raw)
To: linux-sh
Dear Paul
> I'll take the first one in the sh/clkfwk topic branch and merge it in to
> HEAD. The remaining FSIDIV patch I'll then roll in to the genesis tree
> once the clock bits have hit mainline. Hopefully this will all be sorted
> by late -rc1 or -rc2. Thanks for testing!
Thanks.
I send "HDMI sound support patch (for AP4)" in next mail.
Can you please care it if ALSA tree was merged ?
Best regards
--
Kuninori Morimoto
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 3/3] ARM: mach-shmobile: clock-sh7372: Add FSIDIV clock support
2010-10-15 5:15 [PATCH 3/3] ARM: mach-shmobile: clock-sh7372: Add FSIDIV clock support Kuninori Morimoto
` (6 preceding siblings ...)
2010-10-18 10:10 ` [PATCH 3/3] ARM: mach-shmobile: clock-sh7372: Add FSIDIV clock Kuninori Morimoto
@ 2010-10-18 10:12 ` Paul Mundt
7 siblings, 0 replies; 9+ messages in thread
From: Paul Mundt @ 2010-10-18 10:12 UTC (permalink / raw)
To: linux-sh
On Mon, Oct 18, 2010 at 07:10:50PM +0900, Kuninori Morimoto wrote:
> > HEAD. The remaining FSIDIV patch I'll then roll in to the genesis tree
> > once the clock bits have hit mainline. Hopefully this will all be sorted
> > by late -rc1 or -rc2. Thanks for testing!
>
> Thanks.
> I send "HDMI sound support patch (for AP4)" in next mail.
> Can you please care it if ALSA tree was merged ?
>
Of course, that's what I'm here for :-)
^ permalink raw reply [flat|nested] 9+ messages in thread