[PATCH][RFC] sh: helper code for 4-bit divisors

[PATCH][RFC] sh: helper code for 4-bit divisors

[Magnus Damm]

[-- Attachment #1: Type: text/plain, Size: 1087 bytes --]

Hey Paul,

Attached are two versions of helper code for 4-bit divisors. The
sh7785 clock code is modified as an example.

linux-2.6.31-pre-sh-clk-cpg-div4-sh7785-a-20090528.patch
 include/asm/clock.h            |   42 +++++++
 kernel/cpu/clock-cpg.c         |   66 +++++++++++
 kernel/cpu/sh4a/clock-sh7785.c |  241 +++++++++++++----------------------------
 3 files changed, 188 insertions(+), 161 deletions(-)

linux-2.6.31-pre-sh-clk-cpg-div4-sh7785-b-20090528.patch
 include/asm/clock.h            |   40 +++++++
 kernel/cpu/clock-cpg.c         |   69 +++++++++++++
 kernel/cpu/sh4a/clock-sh7785.c |  206 ++++++++---------------------------------
 3 files changed, 154 insertions(+), 161 deletions(-)

I prefer version b (the last) since it allows us to write more compact
per-processor code. Also, I think using offsetof is a quick and cheap
way to get private data without using much memory. I may be the only
person that likes wrapping stuff like this though. Any thoughts?

If we can agree on something then i'd like to convert the SuperH
Mobile processors using this.

/ magnus

[-- Attachment #2: linux-2.6.31-pre-sh-clk-cpg-div4-sh7785-a-20090528.patch --]
[-- Type: application/octet-stream, Size: 11251 bytes --]

--- 0001/arch/sh/include/asm/clock.h
+++ work/arch/sh/include/asm/clock.h	2009-05-28 13:02:03.000000000 +0900
@@ -3,6 +3,7 @@
 
 #include <linux/list.h>
 #include <linux/seq_file.h>
+#include <linux/cpufreq.h>
 #include <linux/clk.h>
 #include <linux/err.h>
 
@@ -117,4 +118,45 @@ long clk_rate_table_round(struct clk *cl
 			  struct cpufreq_frequency_table *freq_table,
 			  unsigned long rate);
 
+
+/* helper data structures and function for 4-bit width clock divisors */
+struct sh_clk_cpg_div4_table {
+	unsigned int div[1 << 4];
+	unsigned int mult[1 << 4];
+	struct clk_div_mult_table t;
+};
+
+struct sh_clk_cpg_div4_data {
+	unsigned int shift;
+	void __iomem *reg;
+
+	/* allowable divisor bitmap */
+	unsigned long div_bitmap;
+
+	/* Supportable frequencies + termination entry */
+	struct cpufreq_frequency_table freq_table[(1 << 4)+1];
+
+	/* Configuration table */
+	struct sh_clk_cpg_div4_table *config_table;
+};
+
+#define SH_CLK_CPG_DIV4_DATA(_reg, _shift, _div_bitmap)	\
+{							\
+	.reg            = (void __iomem *)_reg,		\
+	.shift          = _shift,			\
+	.div_bitmap     = _div_bitmap,			\
+							\
+	.freq_table[0]  = {				\
+		.index = 0,				\
+		.frequency = CPUFREQ_TABLE_END,		\
+	},						\
+}
+
+int sh_clk_cpg_div4_register(struct clk *clk,
+			      int nr,
+			      struct sh_clk_cpg_div4_data *data,
+			      struct sh_clk_cpg_div4_table *table);
+
+
+
 #endif /* __ASM_SH_CLOCK_H */
--- 0001/arch/sh/kernel/cpu/clock-cpg.c
+++ work/arch/sh/kernel/cpu/clock-cpg.c	2009-05-28 13:00:35.000000000 +0900
@@ -1,7 +1,73 @@
 #include <linux/clk.h>
 #include <linux/compiler.h>
+#include <linux/io.h>
 #include <asm/clock.h>
 
+static unsigned long sh_clk_cpg_div4_recalc(struct clk *clk)
+{
+	struct sh_clk_cpg_div4_data *data = clk->priv;
+	unsigned int idx = (__raw_readl(data->reg) >> data->shift) & 0x000f;
+
+	clk_rate_table_build(clk, data->freq_table,
+			     ARRAY_SIZE(data->config_table->div),
+			     &data->config_table->t, &data->div_bitmap);
+
+	return data->freq_table[idx].frequency;
+}
+
+static long sh_clk_cpg_div4_round_rate(struct clk *clk, unsigned long rate)
+{
+	struct sh_clk_cpg_div4_data *data = clk->priv;
+
+	return clk_rate_table_round(clk, data->freq_table, rate);
+}
+
+static struct clk_ops sh_clk_cpg_div4_clk_ops = {
+	.recalc		= sh_clk_cpg_div4_recalc,
+	.round_rate	= sh_clk_cpg_div4_round_rate,
+};
+
+int __init sh_clk_cpg_div4_register(struct clk *clk,
+				    int nr,
+				    struct sh_clk_cpg_div4_data *data,
+				    struct sh_clk_cpg_div4_table *table)
+{
+	struct clk *clkp;
+	struct sh_clk_cpg_div4_data *datap;
+	int ret = 0;
+	int k;
+
+	for (k = 0; k < ARRAY_SIZE(table->div); k++) {
+		if (!table->div[k])
+			table->div[k] = 1;
+		if (!table->mult[k])
+			table->mult[k] = 1;
+	}
+
+	table->t.divisors = table->div;
+	table->t.nr_divisors = ARRAY_SIZE(table->div);
+	table->t.multipliers = table->mult;
+	table->t.nr_multipliers = ARRAY_SIZE(table->mult);
+
+	for (k = 0; !ret && (k < nr); k++) {
+		clkp = clk + k;
+		datap = data + k;
+
+		datap->config_table = table;
+
+		BUG_ON(!clkp->parent || !clkp->name);
+
+		clkp->id = -1;
+		clkp->ops = &sh_clk_cpg_div4_clk_ops;
+		clkp->priv = datap;
+		clkp->flags = CLK_ENABLE_ON_INIT;
+
+		ret = clk_register(clkp);
+	}
+
+	return ret;
+}
+
 #ifdef CONFIG_SH_CLK_CPG_LEGACY
 static struct clk master_clk = {
 	.name		= "master_clk",
--- 0001/arch/sh/kernel/cpu/sh4a/clock-sh7785.c
+++ work/arch/sh/kernel/cpu/sh4a/clock-sh7785.c	2009-05-28 13:00:06.000000000 +0900
@@ -17,65 +17,14 @@
 #include <asm/clock.h>
 #include <asm/freq.h>
 
-static unsigned int div2[] = { 1, 2, 4, 6, 8, 12, 16, 18,
-			       24, 32, 36, 48 };
-
-static struct clk_div_mult_table cpg_div = {
-	.divisors = div2,
-	.nr_divisors = ARRAY_SIZE(div2),
-};
-
-struct clk_priv {
-	unsigned int			shift;
-
-	/* allowable divisor bitmap */
-	unsigned long			div_bitmap;
-
-	/* Supportable frequencies + termination entry */
-	struct cpufreq_frequency_table	freq_table[ARRAY_SIZE(div2)+1];
-};
-
-#define FRQMR_CLK_DATA(_name, _shift, _div_bitmap)	\
-static struct clk_priv _name##_data = {			\
-	.shift		= _shift,			\
-	.div_bitmap	= _div_bitmap,			\
-							\
-	.freq_table[0]	= {				\
-		.index = 0,				\
-		.frequency = CPUFREQ_TABLE_END,		\
-	},						\
-}
-
-FRQMR_CLK_DATA(pfc,  0, 0x0f80);
-FRQMR_CLK_DATA(s3fc, 4, 0x0ff0);
-FRQMR_CLK_DATA(s2fc, 8, 0x0030);
-FRQMR_CLK_DATA(mfc, 12, 0x000c);
-FRQMR_CLK_DATA(bfc, 16, 0x0fe0);
-FRQMR_CLK_DATA(sfc, 20, 0x000c);
-FRQMR_CLK_DATA(ufc, 24, 0x000c);
-FRQMR_CLK_DATA(ifc, 28, 0x000e);
-
-static unsigned long frqmr_recalc(struct clk *clk)
-{
-	struct clk_priv *data = clk->priv;
-	unsigned int idx = (__raw_readl(FRQMR1) >> data->shift) & 0x000f;
-
-	clk_rate_table_build(clk, data->freq_table, ARRAY_SIZE(div2),
-			     &cpg_div, &data->div_bitmap);
-	
-	return data->freq_table[idx].frequency;
-}
-
-static long frqmr_round_rate(struct clk *clk, unsigned long rate)
-{
-	struct clk_priv *data = clk->priv;
-
-	return clk_rate_table_round(clk, data->freq_table, rate);
-}
-
-static struct clk_ops frqmr_clk_ops = {
-	.recalc			= frqmr_recalc,
-	.round_rate		= frqmr_round_rate,
+/*
+ * Default rate for the root input clock, reset this with clk_set_rate()
+ * from the platform code.
+ */
+static struct clk extal_clk = {
+	.name		= "extal",
+	.id		= -1,
+	.rate		= 33333333,
 };
 
 static unsigned long pll_recalc(struct clk *clk)
@@ -92,16 +41,6 @@ static struct clk_ops pll_clk_ops = {
 	.recalc		= pll_recalc,
 };
 
-/*
- * Default rate for the root input clock, reset this with clk_set_rate()
- * from the platform code.
- */
-static struct clk extal_clk = {
-	.name		= "extal",
-	.id		= -1,
-	.rate		= 33333333,
-};
-
 static struct clk pll_clk = {
 	.name		= "pll_clk",
 	.id		= -1,
@@ -110,87 +49,62 @@ static struct clk pll_clk = {
 	.flags		= CLK_ENABLE_ON_INIT,
 };
 
-static struct clk cpu_clk = {
-	.name		= "cpu_clk",		/* Ick */
-	.id		= -1,
-	.ops		= &frqmr_clk_ops,
-	.parent		= &pll_clk,
-	.flags		= CLK_ENABLE_ON_INIT,
-	.priv		= &ifc_data,
-};
-
-static struct clk shyway_clk = {
-	.name		= "shyway_clk",		/* SHck */
-	.id		= -1,
-	.ops		= &frqmr_clk_ops,
-	.parent		= &pll_clk,
-	.flags		= CLK_ENABLE_ON_INIT,
-	.priv		= &sfc_data,
-};
-
-static struct clk peripheral_clk = {
-	.name		= "peripheral_clk",	/* Pck */
-	.id		= -1,
-	.ops		= &frqmr_clk_ops,
-	.parent		= &pll_clk,
-	.flags		= CLK_ENABLE_ON_INIT,
-	.priv		= &pfc_data,
-};
-
-static struct clk ddr_clk = {
-	.name		= "ddr_clk",		/* DDRck */
-	.id		= -1,
-	.ops		= &frqmr_clk_ops,
-	.parent		= &pll_clk,
-	.flags		= CLK_ENABLE_ON_INIT,
-	.priv		= &mfc_data,
-};
-
-static struct clk bus_clk = {
-	.name		= "bus_clk",		/* Bck */
-	.id		= -1,
-	.ops		= &frqmr_clk_ops,
-	.parent		= &pll_clk,
-	.flags		= CLK_ENABLE_ON_INIT,
-	.priv		= &bfc_data,
+static struct clk *clks[] = {
+	&extal_clk,
+	&pll_clk,
 };
 
-static struct clk ga_clk = {
-	.name		= "ga_clk",		/* GAck */
-	.id		= -1,
-	.ops		= &frqmr_clk_ops,
-	.parent		= &pll_clk,
-	.priv		= &s2fc_data,
+struct sh_clk_cpg_div4_table div4_table = {
+	.div = { 1, 2, 4, 6, 8, 12, 16, 18, 24, 32, 36, 48, },
 };
 
-static struct clk du_clk = {
-	.name		= "du_clk",		/* DUck */
-	.id		= -1,
-	.ops		= &frqmr_clk_ops,
-	.parent		= &pll_clk,
-	.priv		= &s3fc_data,
-};
+enum { DIV4_I, DIV4_U, DIV4_SH, DIV4_B, DIV4_DDR, DIV4_GA,
+	DIV4_DU, DIV4_P, DIV4_NR };
 
-static struct clk umem_clk = {
-	.name		= "umem_clk",		/* uck */
-	.id		= -1,
-	.ops		= &frqmr_clk_ops,
-	.parent		= &pll_clk,
-	.flags		= CLK_ENABLE_ON_INIT,
-	.priv		= &ufc_data,
-};
-
-static struct clk *clks[] = {
-	&extal_clk,
-	&pll_clk,
-	&cpu_clk,
-	&shyway_clk,
-	&peripheral_clk,
-	&ddr_clk,
-	&bus_clk,
-	&ga_clk,
-	&du_clk,
-	&umem_clk,
+struct sh_clk_cpg_div4_data div4_data[DIV4_NR] = {
+	[DIV4_P] = SH_CLK_CPG_DIV4_DATA(FRQMR1, 0, 0x0f80),
+	[DIV4_DU] = SH_CLK_CPG_DIV4_DATA(FRQMR1, 4, 0x0ff0),
+	[DIV4_GA] = SH_CLK_CPG_DIV4_DATA(FRQMR1, 8, 0x0030),
+	[DIV4_DDR] = SH_CLK_CPG_DIV4_DATA(FRQMR1, 12, 0x000c),
+	[DIV4_B] = SH_CLK_CPG_DIV4_DATA(FRQMR1, 16, 0x0fe0),
+	[DIV4_SH] = SH_CLK_CPG_DIV4_DATA(FRQMR1, 20, 0x000c),
+	[DIV4_U] = SH_CLK_CPG_DIV4_DATA(FRQMR1, 24, 0x000c),
+	[DIV4_I] = SH_CLK_CPG_DIV4_DATA(FRQMR1, 28, 0x000e),
+};
+
+struct clk div4_clks[DIV4_NR] = {
+	[DIV4_I] = {
+		.name		= "cpu_clk",		/* Ick */
+		.parent		= &pll_clk,
+	},
+	[DIV4_SH] = {
+		.name		= "shyway_clk",		/* SHck */
+		.parent		= &pll_clk,
+	},
+	[DIV4_P] = {
+		.name		= "peripheral_clk",	/* Pck */
+		.parent		= &pll_clk,
+	},
+	[DIV4_DDR] = {
+		.name		= "ddr_clk",		/* DDRck */
+		.parent		= &pll_clk,
+	},
+	[DIV4_B] = {
+		.name		= "bus_clk",		/* Bck */
+		.parent		= &pll_clk,
+	},
+	[DIV4_GA] = {
+		.name		= "ga_clk",		/* GAck */
+		.parent		= &pll_clk,
+	},
+	[DIV4_DU] = {
+		.name		= "du_clk",		/* DUck */
+		.parent		= &pll_clk,
+	},
+	[DIV4_U] = {
+		.name		= "umem_clk",		/* uck */
+		.parent		= &pll_clk,
+	},
 };
 
 static int mstpcr_clk_enable(struct clk *clk)
@@ -229,22 +143,22 @@ static struct clk_ops mstpcr_clk_ops = {
 
 static struct clk mstpcr_clks[] = {
 	/* MSTPCR0 */
-	CLK("scif_fck", 5, &peripheral_clk, MSTPCR0, 29, 0),
-	CLK("scif_fck", 4, &peripheral_clk, MSTPCR0, 28, 0),
-	CLK("scif_fck", 3, &peripheral_clk, MSTPCR0, 27, 0),
-	CLK("scif_fck", 2, &peripheral_clk, MSTPCR0, 26, 0),
-	CLK("scif_fck", 1, &peripheral_clk, MSTPCR0, 25, 0),
-	CLK("scif_fck", 0, &peripheral_clk, MSTPCR0, 24, 0),
-	CLK("ssi_fck", 1, &peripheral_clk, MSTPCR0, 21, 0),
-	CLK("ssi_fck", 0, &peripheral_clk, MSTPCR0, 20, 0),
-	CLK("hac_fck", 1, &peripheral_clk, MSTPCR0, 17, 0),
-	CLK("hac_fck", 0, &peripheral_clk, MSTPCR0, 16, 0),
-	CLK("mmcif_fck", -1, &peripheral_clk, MSTPCR0, 13, 0),
-	CLK("flctl_fck", -1, &peripheral_clk, MSTPCR0, 12, 0),
-	CLK("tmu345_fck", -1, &peripheral_clk, MSTPCR0, 9, 0),
-	CLK("tmu012_fck", -1, &peripheral_clk, MSTPCR0, 8, 0),
-	CLK("siof_fck", -1, &peripheral_clk, MSTPCR0, 3, 0),
-	CLK("hspi_fck", -1, &peripheral_clk, MSTPCR0, 2, 0),
+	CLK("scif_fck", 5, &div4_clks[DIV4_P], MSTPCR0, 29, 0),
+	CLK("scif_fck", 4, &div4_clks[DIV4_P], MSTPCR0, 28, 0),
+	CLK("scif_fck", 3, &div4_clks[DIV4_P], MSTPCR0, 27, 0),
+	CLK("scif_fck", 2, &div4_clks[DIV4_P], MSTPCR0, 26, 0),
+	CLK("scif_fck", 1, &div4_clks[DIV4_P], MSTPCR0, 25, 0),
+	CLK("scif_fck", 0, &div4_clks[DIV4_P], MSTPCR0, 24, 0),
+	CLK("ssi_fck", 1, &div4_clks[DIV4_P], MSTPCR0, 21, 0),
+	CLK("ssi_fck", 0, &div4_clks[DIV4_P], MSTPCR0, 20, 0),
+	CLK("hac_fck", 1, &div4_clks[DIV4_P], MSTPCR0, 17, 0),
+	CLK("hac_fck", 0, &div4_clks[DIV4_P], MSTPCR0, 16, 0),
+	CLK("mmcif_fck", -1, &div4_clks[DIV4_P], MSTPCR0, 13, 0),
+	CLK("flctl_fck", -1, &div4_clks[DIV4_P], MSTPCR0, 12, 0),
+	CLK("tmu345_fck", -1, &div4_clks[DIV4_P], MSTPCR0, 9, 0),
+	CLK("tmu012_fck", -1, &div4_clks[DIV4_P], MSTPCR0, 8, 0),
+	CLK("siof_fck", -1, &div4_clks[DIV4_P], MSTPCR0, 3, 0),
+	CLK("hspi_fck", -1, &div4_clks[DIV4_P], MSTPCR0, 2, 0),
 
 	/* MSTPCR1 */
 	CLK("hudi_fck", -1, NULL, MSTPCR1, 19, 0),
@@ -260,6 +174,11 @@ int __init arch_clk_init(void)
 
 	for (i = 0; i < ARRAY_SIZE(clks); i++)
 		ret |= clk_register(clks[i]);
+
+	if (!ret)
+		ret = sh_clk_cpg_div4_register(div4_clks, DIV4_NR,
+					       div4_data, &div4_table);
+
 	for (i = 0; i < ARRAY_SIZE(mstpcr_clks); i++)
 		ret |= clk_register(&mstpcr_clks[i]);
 

[-- Attachment #3: linux-2.6.31-pre-sh-clk-cpg-div4-sh7785-b-20090528.patch --]
[-- Type: application/octet-stream, Size: 10598 bytes --]

--- 0001/arch/sh/include/asm/clock.h
+++ work/arch/sh/include/asm/clock.h	2009-05-28 13:31:34.000000000 +0900
@@ -3,6 +3,7 @@
 
 #include <linux/list.h>
 #include <linux/seq_file.h>
+#include <linux/cpufreq.h>
 #include <linux/clk.h>
 #include <linux/err.h>
 
@@ -117,4 +118,43 @@ long clk_rate_table_round(struct clk *cl
 			  struct cpufreq_frequency_table *freq_table,
 			  unsigned long rate);
 
+
+/* helper data structures and function for 4-bit width clock divisors */
+struct sh_clk_cpg_div4_table {
+	unsigned int div[1 << 4];
+	unsigned int mult[1 << 4];
+	struct clk_div_mult_table t;
+};
+
+struct sh_clk_cpg_div4 {
+	struct clk clk;
+
+	/* allowable divisor bitmap */
+	unsigned long div_bitmap;
+
+	/* Supportable frequencies + termination entry */
+	struct cpufreq_frequency_table freq_table[(1 << 4)+1];
+};
+
+#define SH_CLK_CPG_DIV4(_name, _reg, _shift, _div_bitmap)	\
+{								\
+	.clk = {						\
+		.name = _name,					\
+		.enable_reg = (void __iomem *)_reg,		\
+		.enable_bit = _shift,				\
+	},							\
+	.div_bitmap     = _div_bitmap,				\
+	.freq_table[0]  = {					\
+		.index = 0,					\
+		.frequency = CPUFREQ_TABLE_END,			\
+	},							\
+}
+
+int sh_clk_cpg_div4_register(struct sh_clk_cpg_div4 *div4,
+			     int nr,
+			     struct clk *parent,
+			     struct sh_clk_cpg_div4_table *table);
+
+
+
 #endif /* __ASM_SH_CLOCK_H */
--- 0001/arch/sh/kernel/cpu/clock-cpg.c
+++ work/arch/sh/kernel/cpu/clock-cpg.c	2009-05-28 13:41:28.000000000 +0900
@@ -1,7 +1,76 @@
 #include <linux/clk.h>
 #include <linux/compiler.h>
+#include <linux/io.h>
 #include <asm/clock.h>
 
+
+static struct sh_clk_cpg_div4 *clk2div4(struct clk *clk)
+{
+	return (void *)clk - offsetof(struct sh_clk_cpg_div4, clk);
+}
+
+static unsigned long sh_clk_cpg_div4_recalc(struct clk *clk)
+{
+	struct sh_clk_cpg_div4 *div4 = clk2div4(clk);
+	struct sh_clk_cpg_div4_table *table = clk->priv;
+	unsigned int idx;
+
+	clk_rate_table_build(clk, div4->freq_table, ARRAY_SIZE(table->div),
+			     &table->t, &div4->div_bitmap);
+
+	idx = (__raw_readl(clk->enable_reg) >> clk->enable_bit) & 0x000f;
+
+	return div4->freq_table[idx].frequency;
+}
+
+static long sh_clk_cpg_div4_round_rate(struct clk *clk, unsigned long rate)
+{
+	return clk_rate_table_round(clk, clk2div4(clk)->freq_table, rate);
+}
+
+static struct clk_ops sh_clk_cpg_div4_clk_ops = {
+	.recalc		= sh_clk_cpg_div4_recalc,
+	.round_rate	= sh_clk_cpg_div4_round_rate,
+};
+
+int __init sh_clk_cpg_div4_register(struct sh_clk_cpg_div4 *div4,
+				    int nr,
+				    struct clk *parent,
+				    struct sh_clk_cpg_div4_table *table)
+{
+	struct clk *clkp;
+	struct sh_clk_cpg_div4 *div4p;
+	int ret = 0;
+	int k;
+
+	for (k = 0; k < ARRAY_SIZE(table->div); k++) {
+		if (!table->div[k])
+			table->div[k] = 1;
+		if (!table->mult[k])
+			table->mult[k] = 1;
+	}
+
+	table->t.divisors = table->div;
+	table->t.nr_divisors = ARRAY_SIZE(table->div);
+	table->t.multipliers = table->mult;
+	table->t.nr_multipliers = ARRAY_SIZE(table->mult);
+
+	for (k = 0; !ret && (k < nr); k++) {
+		div4p = div4 + k;
+		clkp = &div4p->clk;
+
+		clkp->id = -1;
+		clkp->ops = &sh_clk_cpg_div4_clk_ops;
+		clkp->parent = parent;
+		clkp->priv = table;
+		clkp->flags = CLK_ENABLE_ON_INIT;
+
+		ret = clk_register(clkp);
+	}
+
+	return ret;
+}
+
 #ifdef CONFIG_SH_CLK_CPG_LEGACY
 static struct clk master_clk = {
 	.name		= "master_clk",
--- 0001/arch/sh/kernel/cpu/sh4a/clock-sh7785.c
+++ work/arch/sh/kernel/cpu/sh4a/clock-sh7785.c	2009-05-28 13:32:37.000000000 +0900
@@ -17,65 +17,14 @@
 #include <asm/clock.h>
 #include <asm/freq.h>
 
-static unsigned int div2[] = { 1, 2, 4, 6, 8, 12, 16, 18,
-			       24, 32, 36, 48 };
-
-static struct clk_div_mult_table cpg_div = {
-	.divisors = div2,
-	.nr_divisors = ARRAY_SIZE(div2),
-};
-
-struct clk_priv {
-	unsigned int			shift;
-
-	/* allowable divisor bitmap */
-	unsigned long			div_bitmap;
-
-	/* Supportable frequencies + termination entry */
-	struct cpufreq_frequency_table	freq_table[ARRAY_SIZE(div2)+1];
-};
-
-#define FRQMR_CLK_DATA(_name, _shift, _div_bitmap)	\
-static struct clk_priv _name##_data = {			\
-	.shift		= _shift,			\
-	.div_bitmap	= _div_bitmap,			\
-							\
-	.freq_table[0]	= {				\
-		.index = 0,				\
-		.frequency = CPUFREQ_TABLE_END,		\
-	},						\
-}
-
-FRQMR_CLK_DATA(pfc,  0, 0x0f80);
-FRQMR_CLK_DATA(s3fc, 4, 0x0ff0);
-FRQMR_CLK_DATA(s2fc, 8, 0x0030);
-FRQMR_CLK_DATA(mfc, 12, 0x000c);
-FRQMR_CLK_DATA(bfc, 16, 0x0fe0);
-FRQMR_CLK_DATA(sfc, 20, 0x000c);
-FRQMR_CLK_DATA(ufc, 24, 0x000c);
-FRQMR_CLK_DATA(ifc, 28, 0x000e);
-
-static unsigned long frqmr_recalc(struct clk *clk)
-{
-	struct clk_priv *data = clk->priv;
-	unsigned int idx = (__raw_readl(FRQMR1) >> data->shift) & 0x000f;
-
-	clk_rate_table_build(clk, data->freq_table, ARRAY_SIZE(div2),
-			     &cpg_div, &data->div_bitmap);
-	
-	return data->freq_table[idx].frequency;
-}
-
-static long frqmr_round_rate(struct clk *clk, unsigned long rate)
-{
-	struct clk_priv *data = clk->priv;
-
-	return clk_rate_table_round(clk, data->freq_table, rate);
-}
-
-static struct clk_ops frqmr_clk_ops = {
-	.recalc			= frqmr_recalc,
-	.round_rate		= frqmr_round_rate,
+/*
+ * Default rate for the root input clock, reset this with clk_set_rate()
+ * from the platform code.
+ */
+static struct clk extal_clk = {
+	.name		= "extal",
+	.id		= -1,
+	.rate		= 33333333,
 };
 
 static unsigned long pll_recalc(struct clk *clk)
@@ -92,16 +41,6 @@ static struct clk_ops pll_clk_ops = {
 	.recalc		= pll_recalc,
 };
 
-/*
- * Default rate for the root input clock, reset this with clk_set_rate()
- * from the platform code.
- */
-static struct clk extal_clk = {
-	.name		= "extal",
-	.id		= -1,
-	.rate		= 33333333,
-};
-
 static struct clk pll_clk = {
 	.name		= "pll_clk",
 	.id		= -1,
@@ -110,87 +49,27 @@ static struct clk pll_clk = {
 	.flags		= CLK_ENABLE_ON_INIT,
 };
 
-static struct clk cpu_clk = {
-	.name		= "cpu_clk",		/* Ick */
-	.id		= -1,
-	.ops		= &frqmr_clk_ops,
-	.parent		= &pll_clk,
-	.flags		= CLK_ENABLE_ON_INIT,
-	.priv		= &ifc_data,
-};
-
-static struct clk shyway_clk = {
-	.name		= "shyway_clk",		/* SHck */
-	.id		= -1,
-	.ops		= &frqmr_clk_ops,
-	.parent		= &pll_clk,
-	.flags		= CLK_ENABLE_ON_INIT,
-	.priv		= &sfc_data,
-};
-
-static struct clk peripheral_clk = {
-	.name		= "peripheral_clk",	/* Pck */
-	.id		= -1,
-	.ops		= &frqmr_clk_ops,
-	.parent		= &pll_clk,
-	.flags		= CLK_ENABLE_ON_INIT,
-	.priv		= &pfc_data,
-};
-
-static struct clk ddr_clk = {
-	.name		= "ddr_clk",		/* DDRck */
-	.id		= -1,
-	.ops		= &frqmr_clk_ops,
-	.parent		= &pll_clk,
-	.flags		= CLK_ENABLE_ON_INIT,
-	.priv		= &mfc_data,
-};
-
-static struct clk bus_clk = {
-	.name		= "bus_clk",		/* Bck */
-	.id		= -1,
-	.ops		= &frqmr_clk_ops,
-	.parent		= &pll_clk,
-	.flags		= CLK_ENABLE_ON_INIT,
-	.priv		= &bfc_data,
+static struct clk *clks[] = {
+	&extal_clk,
+	&pll_clk,
 };
 
-static struct clk ga_clk = {
-	.name		= "ga_clk",		/* GAck */
-	.id		= -1,
-	.ops		= &frqmr_clk_ops,
-	.parent		= &pll_clk,
-	.priv		= &s2fc_data,
+struct sh_clk_cpg_div4_table div4_table = {
+	.div = { 1, 2, 4, 6, 8, 12, 16, 18, 24, 32, 36, 48, },
 };
 
-static struct clk du_clk = {
-	.name		= "du_clk",		/* DUck */
-	.id		= -1,
-	.ops		= &frqmr_clk_ops,
-	.parent		= &pll_clk,
-	.priv		= &s3fc_data,
-};
+enum { DIV4_I, DIV4_U, DIV4_SH, DIV4_B, DIV4_DDR, DIV4_GA,
+	DIV4_DU, DIV4_P, DIV4_NR };
 
-static struct clk umem_clk = {
-	.name		= "umem_clk",		/* uck */
-	.id		= -1,
-	.ops		= &frqmr_clk_ops,
-	.parent		= &pll_clk,
-	.flags		= CLK_ENABLE_ON_INIT,
-	.priv		= &ufc_data,
-};
-
-static struct clk *clks[] = {
-	&extal_clk,
-	&pll_clk,
-	&cpu_clk,
-	&shyway_clk,
-	&peripheral_clk,
-	&ddr_clk,
-	&bus_clk,
-	&ga_clk,
-	&du_clk,
-	&umem_clk,
+struct sh_clk_cpg_div4 div4[DIV4_NR] = {
+	[DIV4_P] = SH_CLK_CPG_DIV4("peripheral_clk", FRQMR1, 0, 0x0f80),
+	[DIV4_DU] = SH_CLK_CPG_DIV4("du_clk", FRQMR1, 4, 0x0ff0),
+	[DIV4_GA] = SH_CLK_CPG_DIV4("ga_clk", FRQMR1, 8, 0x0030),
+	[DIV4_DDR] = SH_CLK_CPG_DIV4("ddr_clk", FRQMR1, 12, 0x000c),
+	[DIV4_B] = SH_CLK_CPG_DIV4("bus_clk", FRQMR1, 16, 0x0fe0),
+	[DIV4_SH] = SH_CLK_CPG_DIV4("shyway_clk", FRQMR1, 20, 0x000c),
+	[DIV4_U] = SH_CLK_CPG_DIV4("umem_clk", FRQMR1, 24, 0x000c),
+	[DIV4_I] = SH_CLK_CPG_DIV4("cpu_clk", FRQMR1, 28, 0x000e),
 };
 
 static int mstpcr_clk_enable(struct clk *clk)
@@ -229,22 +108,22 @@ static struct clk_ops mstpcr_clk_ops = {
 
 static struct clk mstpcr_clks[] = {
 	/* MSTPCR0 */
-	CLK("scif_fck", 5, &peripheral_clk, MSTPCR0, 29, 0),
-	CLK("scif_fck", 4, &peripheral_clk, MSTPCR0, 28, 0),
-	CLK("scif_fck", 3, &peripheral_clk, MSTPCR0, 27, 0),
-	CLK("scif_fck", 2, &peripheral_clk, MSTPCR0, 26, 0),
-	CLK("scif_fck", 1, &peripheral_clk, MSTPCR0, 25, 0),
-	CLK("scif_fck", 0, &peripheral_clk, MSTPCR0, 24, 0),
-	CLK("ssi_fck", 1, &peripheral_clk, MSTPCR0, 21, 0),
-	CLK("ssi_fck", 0, &peripheral_clk, MSTPCR0, 20, 0),
-	CLK("hac_fck", 1, &peripheral_clk, MSTPCR0, 17, 0),
-	CLK("hac_fck", 0, &peripheral_clk, MSTPCR0, 16, 0),
-	CLK("mmcif_fck", -1, &peripheral_clk, MSTPCR0, 13, 0),
-	CLK("flctl_fck", -1, &peripheral_clk, MSTPCR0, 12, 0),
-	CLK("tmu345_fck", -1, &peripheral_clk, MSTPCR0, 9, 0),
-	CLK("tmu012_fck", -1, &peripheral_clk, MSTPCR0, 8, 0),
-	CLK("siof_fck", -1, &peripheral_clk, MSTPCR0, 3, 0),
-	CLK("hspi_fck", -1, &peripheral_clk, MSTPCR0, 2, 0),
+	CLK("scif_fck", 5, &div4[DIV4_P].clk, MSTPCR0, 29, 0),
+	CLK("scif_fck", 4, &div4[DIV4_P].clk, MSTPCR0, 28, 0),
+	CLK("scif_fck", 3, &div4[DIV4_P].clk, MSTPCR0, 27, 0),
+	CLK("scif_fck", 2, &div4[DIV4_P].clk, MSTPCR0, 26, 0),
+	CLK("scif_fck", 1, &div4[DIV4_P].clk, MSTPCR0, 25, 0),
+	CLK("scif_fck", 0, &div4[DIV4_P].clk, MSTPCR0, 24, 0),
+	CLK("ssi_fck", 1, &div4[DIV4_P].clk, MSTPCR0, 21, 0),
+	CLK("ssi_fck", 0, &div4[DIV4_P].clk, MSTPCR0, 20, 0),
+	CLK("hac_fck", 1, &div4[DIV4_P].clk, MSTPCR0, 17, 0),
+	CLK("hac_fck", 0, &div4[DIV4_P].clk, MSTPCR0, 16, 0),
+	CLK("mmcif_fck", -1, &div4[DIV4_P].clk, MSTPCR0, 13, 0),
+	CLK("flctl_fck", -1, &div4[DIV4_P].clk, MSTPCR0, 12, 0),
+	CLK("tmu345_fck", -1, &div4[DIV4_P].clk, MSTPCR0, 9, 0),
+	CLK("tmu012_fck", -1, &div4[DIV4_P].clk, MSTPCR0, 8, 0),
+	CLK("siof_fck", -1, &div4[DIV4_P].clk, MSTPCR0, 3, 0),
+	CLK("hspi_fck", -1, &div4[DIV4_P].clk, MSTPCR0, 2, 0),
 
 	/* MSTPCR1 */
 	CLK("hudi_fck", -1, NULL, MSTPCR1, 19, 0),
@@ -260,6 +139,11 @@ int __init arch_clk_init(void)
 
 	for (i = 0; i < ARRAY_SIZE(clks); i++)
 		ret |= clk_register(clks[i]);
+
+	if (!ret)
+		ret = sh_clk_cpg_div4_register(div4, DIV4_NR,
+					       &pll_clk, &div4_table);
+
 	for (i = 0; i < ARRAY_SIZE(mstpcr_clks); i++)
 		ret |= clk_register(&mstpcr_clks[i]);
 

Re: [PATCH][RFC] sh: helper code for 4-bit divisors

[Paul Mundt]

On Thu, May 28, 2009 at 01:59:31PM +0900, Magnus Damm wrote:
> Hey Paul,
> 
> Attached are two versions of helper code for 4-bit divisors. The
> sh7785 clock code is modified as an example.
> 
> linux-2.6.31-pre-sh-clk-cpg-div4-sh7785-a-20090528.patch
>  include/asm/clock.h            |   42 +++++++
>  kernel/cpu/clock-cpg.c         |   66 +++++++++++
>  kernel/cpu/sh4a/clock-sh7785.c |  241 +++++++++++++----------------------------
>  3 files changed, 188 insertions(+), 161 deletions(-)
> 
> linux-2.6.31-pre-sh-clk-cpg-div4-sh7785-b-20090528.patch
>  include/asm/clock.h            |   40 +++++++
>  kernel/cpu/clock-cpg.c         |   69 +++++++++++++
>  kernel/cpu/sh4a/clock-sh7785.c |  206 ++++++++---------------------------------
>  3 files changed, 154 insertions(+), 161 deletions(-)
> 
> I prefer version b (the last) since it allows us to write more compact
> per-processor code. Also, I think using offsetof is a quick and cheap
> way to get private data without using much memory. I may be the only
> person that likes wrapping stuff like this though. Any thoughts?
> 
> If we can agree on something then i'd like to convert the SuperH
> Mobile processors using this.
> 
I don't particularly mind which one you want to use, but at least forcing
the clock flags in the register function must die. CLK_ENABLE_ON_INIT is
only ever supposed to be used sparingly, not as a blanket thing. I have
modified the SH-Mobile clocks previously also to fix this up, and I will
not apply any patch that default-enables everything. I do not want to see
any of these clock flags used by registration functions, ever. These need
to be decided on an individual basis by each clock. If you want to have
macros for building those up, fine, but the flag setting must be done
explicitly for each clock.

The sh_clk_cpg_div4_data structure I am also not terribly keen on.
For starters, the frequency table has no place in this data structure, at
all. Nothing about it is specific to div4 clocks, and all this does is
encourage alternate tables to be constructed elsewhere. There is nothing
in this structure that can basically not be included in the struct clk
directly, and we can of course throw the CPG ifdef around those fields to
make life easier on people who aren't using the CPG.

I was hesitant to add a private data pointer to struct clk in the first
place because I knew as soon as that was done people would use that as an
excuse for building up all manner of unspeakable helper structures rather
than just thinking about what needs to be in struct clk and what doesn't.
If you want to have smaller data structures for making things easier to
manipulate, that is perfectly fine, but struct clk is the container that
they ultimately belong in, not buried under the private data pointer.

Once this is generalized enough I think I will just kill the private data
pointer entirely to avoid this sort of abuse happening in the future.

Re: [PATCH][RFC] sh: helper code for 4-bit divisors

[Magnus Damm]

On Thu, May 28, 2009 at 2:25 PM, Paul Mundt <lethal@linux-sh.org> wrote:
> On Thu, May 28, 2009 at 01:59:31PM +0900, Magnus Damm wrote:
>> Hey Paul,
>>
>> Attached are two versions of helper code for 4-bit divisors. The
>> sh7785 clock code is modified as an example.
>>
>> linux-2.6.31-pre-sh-clk-cpg-div4-sh7785-a-20090528.patch
>>  include/asm/clock.h            |   42 +++++++
>>  kernel/cpu/clock-cpg.c         |   66 +++++++++++
>>  kernel/cpu/sh4a/clock-sh7785.c |  241 +++++++++++++----------------------------
>>  3 files changed, 188 insertions(+), 161 deletions(-)
>>
>> linux-2.6.31-pre-sh-clk-cpg-div4-sh7785-b-20090528.patch
>>  include/asm/clock.h            |   40 +++++++
>>  kernel/cpu/clock-cpg.c         |   69 +++++++++++++
>>  kernel/cpu/sh4a/clock-sh7785.c |  206 ++++++++---------------------------------
>>  3 files changed, 154 insertions(+), 161 deletions(-)
>>
>> I prefer version b (the last) since it allows us to write more compact
>> per-processor code. Also, I think using offsetof is a quick and cheap
>> way to get private data without using much memory. I may be the only
>> person that likes wrapping stuff like this though. Any thoughts?
>>
>> If we can agree on something then i'd like to convert the SuperH
>> Mobile processors using this.
>>
> I don't particularly mind which one you want to use, but at least forcing
> the clock flags in the register function must die. CLK_ENABLE_ON_INIT is
> only ever supposed to be used sparingly, not as a blanket thing. I have
> modified the SH-Mobile clocks previously also to fix this up, and I will
> not apply any patch that default-enables everything. I do not want to see
> any of these clock flags used by registration functions, ever. These need
> to be decided on an individual basis by each clock. If you want to have
> macros for building those up, fine, but the flag setting must be done
> explicitly for each clock.

Ok, sure. I agree. Maybe the current cpg clocks for sh7785 can be
tuned a bit then, maybe the peripheral clock doesn't need to use
CLK_ENABLE_ON_INIT?

> The sh_clk_cpg_div4_data structure I am also not terribly keen on.
> For starters, the frequency table has no place in this data structure, at
> all. Nothing about it is specific to div4 clocks, and all this does is
> encourage alternate tables to be constructed elsewhere. There is nothing
> in this structure that can basically not be included in the struct clk
> directly, and we can of course throw the CPG ifdef around those fields to
> make life easier on people who aren't using the CPG.

The only specific thing for div4 clocks is that it's for 4 bit
divisors which means that the frequency table size is adjusted to
that.

> I was hesitant to add a private data pointer to struct clk in the first
> place because I knew as soon as that was done people would use that as an
> excuse for building up all manner of unspeakable helper structures rather
> than just thinking about what needs to be in struct clk and what doesn't.
> If you want to have smaller data structures for making things easier to
> manipulate, that is perfectly fine, but struct clk is the container that
> they ultimately belong in, not buried under the private data pointer.

I guess the concept of a private pointer is to allow different
implementations to hook in whatever private data they need.
It makes sense to use the same structure for all implementations if
they are close enough, but I'm not sure if it's a good plan to embeed
a huge frequency table in struct clk at this point. Maybe after moving
stop bits from clocks to wakeup/idle.

> Once this is generalized enough I think I will just kill the private data
> pointer entirely to avoid this sort of abuse happening in the future.

Your view of the clock framework seems to be that all SuperH clocks
are close enough so we can just throw everything into struct clock.
I'd say that we have at least 4 different types of clocks: PLL, DIV4,
DIV6 and MSTP. We can of course use the same data structure for
everything, I just don't see why since they are all have different
needs.

I'm more for making struct clk as small as possible and let each
specialized clock type wrap whatever it needs around the data type.
With or without private data pointer in struct clk. =)

/ magnus

Re: [PATCH][RFC] sh: helper code for 4-bit divisors

[Paul Mundt]

On Thu, May 28, 2009 at 02:54:59PM +0900, Magnus Damm wrote:
> On Thu, May 28, 2009 at 2:25 PM, Paul Mundt <lethal@linux-sh.org> wrote:
> > I don't particularly mind which one you want to use, but at least forcing
> > the clock flags in the register function must die. CLK_ENABLE_ON_INIT is
> > only ever supposed to be used sparingly, not as a blanket thing. I have
> > modified the SH-Mobile clocks previously also to fix this up, and I will
> > not apply any patch that default-enables everything. I do not want to see
> > any of these clock flags used by registration functions, ever. These need
> > to be decided on an individual basis by each clock. If you want to have
> > macros for building those up, fine, but the flag setting must be done
> > explicitly for each clock.
> 
> Ok, sure. I agree. Maybe the current cpg clocks for sh7785 can be
> tuned a bit then, maybe the peripheral clock doesn't need to use
> CLK_ENABLE_ON_INIT?
> 
Yes, the peripheral clock can have this disabled, it's children will
force it to be enabled, the reason it has that bit at all is because at
the time I was converting from the legacy clocks, there were no sibling
clocks.

> > The sh_clk_cpg_div4_data structure I am also not terribly keen on.
> > For starters, the frequency table has no place in this data structure, at
> > all. Nothing about it is specific to div4 clocks, and all this does is
> > encourage alternate tables to be constructed elsewhere. There is nothing
> > in this structure that can basically not be included in the struct clk
> > directly, and we can of course throw the CPG ifdef around those fields to
> > make life easier on people who aren't using the CPG.
> 
> The only specific thing for div4 clocks is that it's for 4 bit
> divisors which means that the frequency table size is adjusted to
> that.
> 
Yes, we need to be a bit more intelligent about handling the frequency
table, but that again is a totally unrelated issue.

> I guess the concept of a private pointer is to allow different
> implementations to hook in whatever private data they need.
> It makes sense to use the same structure for all implementations if
> they are close enough, but I'm not sure if it's a good plan to embeed
> a huge frequency table in struct clk at this point. Maybe after moving
> stop bits from clocks to wakeup/idle.
> 
Likewise.

> > Once this is generalized enough I think I will just kill the private data
> > pointer entirely to avoid this sort of abuse happening in the future.
> 
> Your view of the clock framework seems to be that all SuperH clocks
> are close enough so we can just throw everything into struct clock.
> I'd say that we have at least 4 different types of clocks: PLL, DIV4,
> DIV6 and MSTP. We can of course use the same data structure for
> everything, I just don't see why since they are all have different
> needs.
> 
MSTP is not now nor has it ever been a clock. It got shoehorned in to the
clock framework because we had no better place for it, if your generic
platform stuff gets integrated, the MSTP mess will need a separate
abstraction and will be tied in to that. We only use the clock framework
now because piggybacking on clk_enable()/disable() for a virtualized
function clock that maps to an MSTP bit is convenient. However, we have
many blocks with MSTP bits that don't have any clocks of their own in the
first place. Fixing up this coupling is beneficial.

Beyond that, PLL, DIV4, and DIV6 are different. I do not expect that one
size will fit all, but I more than expect that a reasonably flexible
struct clk will work for the 95% case, and we can special case the
scenarios that do not map in to that. There is absolutely no point in
having a struct clk in the first place if you want it to be some sort of
neutered rate/refcount container where all of the real clock information
is stashed off in some random data structure. Divisors and multipliers
are common to _every_ clock, so this is data that you are going to have
to reproduce in every one of your offshoot data structures anyways, with
the only variation being in size. A maze of fairly similar data
structures is a total disaster, and likewise will not be merged, so don't
even waste your time.

For the root clocks it is acceptable to have special data structures if
and only if they can not fit within the struct clk abstraction, or they
happen to need a lot more data (which is probably true in the PLL cases).
But extending this methodology to every clock on the system only creates
an unmaintainable mess.

> I'm more for making struct clk as small as possible and let each
> specialized clock type wrap whatever it needs around the data type.
> With or without private data pointer in struct clk. =)

Which is precisely what struct clk was never intended for in the first
place. It is meant to be a useful abstraction, not a refcount wrapper.