pxa: convert to common clock framework

PATCH New Philipp Zabel 5 participants 9 messages Mar 16, 2012 – Mar 19, 2012

Subsystems (4)

Arm (32-bit) Arm sub-architectures Common clk PXA2xx/PXA3xx support

lore

Share & tools

lore

Philipp Zabel

author

2012-03-16 17:38 UTC #0 lore raw

PATCH 4/4 pxa: convert to common clock framework

This patch:
* moves the PXA-specific delay out of clk_enable into callbacks,
* removes the custom clk API functions which are now provided by the common clk framework,
* converts from struct clkops to struct clk_ops,
* removes the dummy clock enable/disable functions,
* changes clock definition macros to define the cross-linked struct clk / struct clk_pxa pair, and
* initializes the statically defined clocks with __clk_init

Signed-off-by Philipp Zabel <[email protected]>

---
 arch/arm/Kconfig                 |    1 +
 arch/arm/mach-pxa/clock-pxa25x.c |   30 ++++++-----
 arch/arm/mach-pxa/clock-pxa27x.c |   16 +++---
 arch/arm/mach-pxa/clock-pxa2xx.c |   26 ++++++++-
 arch/arm/mach-pxa/clock-pxa3xx.c |   49 ++++++++++++-----
 arch/arm/mach-pxa/clock.c        |   76 +-------------------------
 arch/arm/mach-pxa/clock.h        |  108 +++++++++++++++++++++++--------------
 arch/arm/mach-pxa/eseries.c      |   11 +----
 arch/arm/mach-pxa/pxa25x.c       |   22 ++++++++
 arch/arm/mach-pxa/pxa27x.c       |   30 +++++++++++
 arch/arm/mach-pxa/pxa3xx.c       |   26 +++++++++
 11 files changed, 232 insertions(+), 163 deletions(-)

arch/arm/Kconfig +1 -0

index 3cc5897..ab46c97 100644
@@ -684,6 +684,7 @@ config ARCH_PXA
 	select ARCH_HAS_CPUFREQ
 	select CLKDEV_LOOKUP
 	select CLKSRC_MMIO
+	select COMMON_CLK
 	select ARCH_REQUIRE_GPIOLIB
 	select GENERIC_CLOCKEVENTS
 	select GPIO_PXA

arch/arm/mach-pxa/clock-pxa25x.c +17 ~~-13~~

index 53fd568..de2a2e7 100644
@@ -9,6 +9,7 @@
 #include <linux/module.h>
 #include <linux/kernel.h>
 
+#include <mach/mfp-pxa25x.h>
 #include <mach/pxa2xx-regs.h>
 
 #include "clock.h"

@@ -67,21 +68,20 @@ unsigned int pxa25x_get_clk_frequency_khz(int info)
 	return (turbo & 1) ? (N/1000) : (M/1000);
 }
 
-static unsigned long clk_pxa25x_mem_getrate(struct clk *clk)
+static unsigned long clk_pxa25x_mem_recalc_rate(struct clk_hw *hw,
+						unsigned long parent_rate)
 {
 	return L_clk_mult[(CCCR >> 0) & 0x1f] * BASE_CLK;
 }
 
-const struct clkops clk_pxa25x_mem_ops = {
-	.enable		= clk_dummy_enable,
-	.disable	= clk_dummy_disable,
-	.getrate	= clk_pxa25x_mem_getrate,
+const struct clk_ops clk_pxa25x_mem_ops = {
+	.recalc_rate	= clk_pxa25x_mem_recalc_rate,
 };
 
-const struct clkops clk_pxa25x_lcd_ops = {
+const struct clk_ops clk_pxa25x_lcd_ops = {
 	.enable		= clk_pxa2xx_cken_enable,
 	.disable	= clk_pxa2xx_cken_disable,
-	.getrate	= clk_pxa25x_mem_getrate,
+	.recalc_rate	= clk_pxa25x_mem_recalc_rate,
 };
 
 static unsigned long gpio12_config_32k[] = {

@@ -92,17 +92,19 @@ static unsigned long gpio12_config_gpio[] = {
 	GPIO12_GPIO,
 };
 
-static void clk_gpio12_enable(struct clk *clk)
+static int clk_gpio12_enable(struct clk_hw *hw)
 {
 	pxa2xx_mfp_config(gpio12_config_32k, 1);
+
+	return 0;
 }
 
-static void clk_gpio12_disable(struct clk *clk)
+static void clk_gpio12_disable(struct clk_hw *hw)
 {
 	pxa2xx_mfp_config(gpio12_config_gpio, 1);
 }
 
-const struct clkops clk_pxa25x_gpio12_ops = {
+const struct clk_ops clk_pxa25x_gpio12_ops = {
 	.enable         = clk_gpio12_enable,
 	.disable        = clk_gpio12_disable,
 };

@@ -115,17 +117,19 @@ static unsigned long gpio11_config_gpio[] = {
 	GPIO11_GPIO,
 };
 
-static void clk_gpio11_enable(struct clk *clk)
+static int clk_gpio11_enable(struct clk_hw *hw)
 {
 	pxa2xx_mfp_config(gpio11_config_3m6, 1);
+
+	return 0;
 }
 
-static void clk_gpio11_disable(struct clk *clk)
+static void clk_gpio11_disable(struct clk_hw *hw)
 {
 	pxa2xx_mfp_config(gpio11_config_gpio, 1);
 }
 
-const struct clkops clk_pxa25x_gpio11_ops = {
+const struct clk_ops clk_pxa25x_gpio11_ops = {
 	.enable         = clk_gpio11_enable,
 	.disable        = clk_gpio11_disable,
 };

arch/arm/mach-pxa/clock-pxa27x.c +8 -8

index 734864d..6b5ebba 100644
@@ -63,7 +63,8 @@ unsigned int pxa27x_get_clk_frequency_khz(int info)
 /*
  * Return the current mem clock frequency as reflected by CCCR[A], B, and L
  */
-static unsigned long clk_pxa27x_mem_getrate(struct clk *clk)
+static unsigned long clk_pxa27x_mem_recalc_rate(struct clk_hw *hw,
+						unsigned long parent_rate)
 {
 	unsigned long ccsr, clkcfg;
 	unsigned int l, L, m, M;

@@ -85,10 +86,8 @@ static unsigned long clk_pxa27x_mem_getrate(struct clk *clk)
 	return M;
 }
 
-const struct clkops clk_pxa27x_mem_ops = {
-	.enable		= clk_dummy_enable,
-	.disable	= clk_dummy_disable,
-	.getrate	= clk_pxa27x_mem_getrate,
+const struct clk_ops clk_pxa27x_mem_ops = {
+	.recalc_rate	= clk_pxa27x_mem_recalc_rate,
 };
 
 /*

@@ -110,14 +109,15 @@ static unsigned int pxa27x_get_lcdclk_frequency_10khz(void)
 	return K / 10000;
 }
 
-static unsigned long clk_pxa27x_lcd_getrate(struct clk *clk)
+static unsigned long clk_pxa27x_lcd_recalc_rate(struct clk_hw *hw,
+						unsigned long parent_rate)
 {
 	return pxa27x_get_lcdclk_frequency_10khz() * 10000;
 }
 
-const struct clkops clk_pxa27x_lcd_ops = {
+const struct clk_ops clk_pxa27x_lcd_ops = {
 	.enable		= clk_pxa2xx_cken_enable,
 	.disable	= clk_pxa2xx_cken_disable,
-	.getrate	= clk_pxa27x_lcd_getrate,
+	.recalc_rate	= clk_pxa27x_lcd_recalc_rate,
 };

arch/arm/mach-pxa/clock-pxa2xx.c +23 -3

index 1d5859d..5ab5cb6 100644
@@ -6,6 +6,8 @@
  * published by the Free Software Foundation.
  */
 
+#include <linux/clk-provider.h>
+#include <linux/delay.h>
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/init.h>

@@ -15,19 +17,37 @@
 
 #include "clock.h"
 
-void clk_pxa2xx_cken_enable(struct clk *clk)
+int clk_pxa2xx_cken_enable(struct clk_hw *hw)
 {
+	struct clk_pxa *clk = to_clk_pxa(hw);
+
 	CKEN |= 1 << clk->cken;
+
+	if (clk->delay)
+		udelay(clk->delay);
+
+	return 0;
 }
 
-void clk_pxa2xx_cken_disable(struct clk *clk)
+void clk_pxa2xx_cken_disable(struct clk_hw *hw)
 {
+	struct clk_pxa *clk = to_clk_pxa(hw);
+
 	CKEN &= ~(1 << clk->cken);
 }
 
-const struct clkops clk_pxa2xx_cken_ops = {
+static unsigned long clk_pxa2xx_cken_recalc_rate(struct clk_hw *hw,
+						unsigned long parent_rate)
+{
+	struct clk_pxa *clk = to_clk_pxa(hw);
+
+	return clk->rate;
+}
+
+const struct clk_ops clk_pxa2xx_cken_ops = {
 	.enable		= clk_pxa2xx_cken_enable,
 	.disable	= clk_pxa2xx_cken_disable,
+	.recalc_rate	= clk_pxa2xx_cken_recalc_rate,
 };
 
 #ifdef CONFIG_PM

arch/arm/mach-pxa/clock-pxa3xx.c +34 ~~-15~~

119 lines not on this page · raw message

arch/arm/mach-pxa/clock.c +3 ~~-73~~

85 lines not on this page · raw message

arch/arm/mach-pxa/clock.h +67 ~~-41~~

147 lines not on this page · raw message

arch/arm/mach-pxa/eseries.c +1 ~~-10~~

index 3c29343..9214e03 100644
@@ -126,17 +126,8 @@ struct resource eseries_tmio_resources[] = {
 };
 
 /* Some e-series hardware cannot control the 32K clock */
-static void clk_32k_dummy(struct clk *clk)
-{
-}
-
-static const struct clkops clk_32k_dummy_ops = {
-	.enable         = clk_32k_dummy,
-	.disable        = clk_32k_dummy,
-};
-
 static struct clk tmio_dummy_clk = {
-	.ops	= &clk_32k_dummy_ops,
+	.ops	= &clk_dummy_ops,
 	.rate	= 32768,
 };

arch/arm/mach-pxa/pxa25x.c +22 -0

index 2c2eb41..a5a42bc 100644
@@ -16,6 +16,7 @@
  * initialization stuff for PXA machines which can be overridden later if
  * need be.
  */
+#include <linux/clk-private.h>
 #include <linux/gpio.h>
 #include <linux/gpio-pxa.h>
 #include <linux/module.h>

@@ -241,6 +242,27 @@ static int __init pxa25x_init(void)
 
 		reset_status = RCSR;
 
+		__clk_init(NULL, &clk_pxa25x_hwuart);
+		__clk_init(NULL, &clk_pxa25x_ffuart);
+		__clk_init(NULL, &clk_pxa25x_btuart);
+		__clk_init(NULL, &clk_pxa25x_stuart);
+		__clk_init(NULL, &clk_pxa25x_usb);
+		__clk_init(NULL, &clk_pxa25x_mmc);
+		__clk_init(NULL, &clk_pxa25x_i2c);
+		__clk_init(NULL, &clk_pxa25x_ssp);
+		__clk_init(NULL, &clk_pxa25x_nssp);
+		__clk_init(NULL, &clk_pxa25x_assp);
+		__clk_init(NULL, &clk_pxa25x_pwm0);
+		__clk_init(NULL, &clk_pxa25x_pwm1);
+		__clk_init(NULL, &clk_pxa25x_ac97);
+		__clk_init(NULL, &clk_pxa25x_i2s);
+		__clk_init(NULL, &clk_pxa25x_ficp);
+
+		__clk_init(NULL, &clk_pxa25x_lcd);
+		__clk_init(NULL, &clk_pxa25x_gpio11);
+		__clk_init(NULL, &clk_pxa25x_gpio12);
+		__clk_init(NULL, &clk_pxa25x_mem);
+
 		clkdev_add_table(pxa25x_clkregs, ARRAY_SIZE(pxa25x_clkregs));
 
 		if ((ret = pxa_init_dma(IRQ_DMA, 16)))

arch/arm/mach-pxa/pxa27x.c +30 -0

index b9dc6ac..8f98413 100644
@@ -11,6 +11,7 @@
  * it under the terms of the GNU General Public License version 2 as
  * published by the Free Software Foundation.
  */
+#include <linux/clk-private.h>
 #include <linux/gpio.h>
 #include <linux/gpio-pxa.h>
 #include <linux/module.h>

@@ -339,6 +340,35 @@ static int __init pxa27x_init(void)
 
 		reset_status = RCSR;
 
+		__clk_init(NULL, &clk_pxa27x_ffuart);
+		__clk_init(NULL, &clk_pxa27x_btuart);
+		__clk_init(NULL, &clk_pxa27x_stuart);
+		__clk_init(NULL, &clk_pxa27x_i2s);
+		__clk_init(NULL, &clk_pxa27x_i2c);
+		__clk_init(NULL, &clk_pxa27x_usb);
+		__clk_init(NULL, &clk_pxa27x_mmc);
+		__clk_init(NULL, &clk_pxa27x_ficp);
+		__clk_init(NULL, &clk_pxa27x_usbhost);
+		__clk_init(NULL, &clk_pxa27x_pwri2c);
+		__clk_init(NULL, &clk_pxa27x_keypad);
+		__clk_init(NULL, &clk_pxa27x_ssp1);
+		__clk_init(NULL, &clk_pxa27x_ssp2);
+		__clk_init(NULL, &clk_pxa27x_ssp3);
+		__clk_init(NULL, &clk_pxa27x_pwm0);
+		__clk_init(NULL, &clk_pxa27x_pwm1);
+		__clk_init(NULL, &clk_pxa27x_ac97);
+		__clk_init(NULL, &clk_pxa27x_ac97conf);
+		__clk_init(NULL, &clk_pxa27x_msl);
+		__clk_init(NULL, &clk_pxa27x_usim);
+		__clk_init(NULL, &clk_pxa27x_memstk);
+		__clk_init(NULL, &clk_pxa27x_im);
+		__clk_init(NULL, &clk_pxa27x_memc);
+
+		__clk_init(NULL, &clk_pxa27x_lcd);
+		__clk_init(NULL, &clk_pxa27x_camera);
+		__clk_init(NULL, &clk_pxa27x_mem);
+		__clk_init(NULL, &clk_dummy);
+
 		clkdev_add_table(pxa27x_clkregs, ARRAY_SIZE(pxa27x_clkregs));
 
 		if ((ret = pxa_init_dma(IRQ_DMA, 32)))

arch/arm/mach-pxa/pxa3xx.c +26 -0

index 25a093e..de6c85c 100644
@@ -12,6 +12,8 @@
  * it under the terms of the GNU General Public License version 2 as
  * published by the Free Software Foundation.
  */
+#include <linux/clkdev.h>
+#include <linux/clk-private.h>
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/init.h>

@@ -453,6 +455,30 @@ static int __init pxa3xx_init(void)
 		 */
 		ASCR &= ~(ASCR_RDH | ASCR_D1S | ASCR_D2S | ASCR_D3S);
 
+		__clk_init(NULL, &clk_pxa3xx_pout);
+		__clk_init(NULL, &clk_dummy);
+		__clk_init(NULL, &clk_pxa3xx_lcd);
+		__clk_init(NULL, &clk_pxa3xx_camera);
+		__clk_init(NULL, &clk_pxa3xx_ac97);
+		__clk_init(NULL, &clk_pxa3xx_ffuart);
+		__clk_init(NULL, &clk_pxa3xx_btuart);
+		__clk_init(NULL, &clk_pxa3xx_stuart);
+		__clk_init(NULL, &clk_pxa3xx_i2c);
+		__clk_init(NULL, &clk_pxa3xx_udc);
+		__clk_init(NULL, &clk_pxa3xx_usbh);
+		__clk_init(NULL, &clk_pxa3xx_u2d);
+		__clk_init(NULL, &clk_pxa3xx_keypad);
+		__clk_init(NULL, &clk_pxa3xx_ssp1);
+		__clk_init(NULL, &clk_pxa3xx_ssp2);
+		__clk_init(NULL, &clk_pxa3xx_ssp3);
+		__clk_init(NULL, &clk_pxa3xx_ssp4);
+		__clk_init(NULL, &clk_pxa3xx_pwm0);
+		__clk_init(NULL, &clk_pxa3xx_pwm1);
+		__clk_init(NULL, &clk_pxa3xx_mmc1);
+		__clk_init(NULL, &clk_pxa3xx_mmc2);
+		__clk_init(NULL, &clk_pxa3xx_smemc);
+		__clk_init(NULL, &clk_pxa3xx_gpio);
+
 		clkdev_add_table(pxa3xx_clkregs, ARRAY_SIZE(pxa3xx_clkregs));
 
 		if ((ret = pxa_init_dma(IRQ_DMA, 32)))

Signature · 1 line

1.7.9.1

Turquette, Mike

2012-03-16 22:03 UTC #1 lore raw

PATCH 4/4 pxa: convert to common clock framework

Quoted 7 lines · Philipp Zabel

?#define DEFINE_CK(_name, _cken, _ops) ? ? ? ? ? ? ? ? ?\
-struct clk clk_##_name = { ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
- ? ? ? ? ? ? ? .ops ? ?= _ops, ? ? ? ? ? ? ? ? ? ? ? ? \
+ ? ? ? struct clk clk_##_name; ? ? ? ? ? ? ? ? ? ? ? ? \
+ ? ? ? static struct clk_pxa clk_pxa_##_name = { ? ? ? \
+ ? ? ? ? ? ? ? .hw = { ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
+ ? ? ? ? ? ? ? ? ? ? ? .clk = &clk_##_name, ? ? ? ? ? ?\

Quoted 8 lines

+ ? ? ? ? ? ? ? }, ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
? ? ? ? ? ? ? ?.cken ? = CKEN_##_cken, ? ? ? ? ? ? ? ? \
+ ? ? ? }; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
+ ? ? ? static struct clk clk_##_name = { ? ? ? ? ? ? ? \
+ ? ? ? ? ? ? ? .name ? = #_name, ? ? ? ? ? ? ? ? ? ? ? \
+ ? ? ? ? ? ? ? .ops ? ?= _ops, ? ? ? ? ? ? ? ? ? ? ? ? \
+ ? ? ? ? ? ? ? .hw ? ? = &clk_pxa_##_name.hw, ? ? ? ? ?\
? ? ? ?}

Hi Philipp,

It looks like your macros do not initialize the .parent_names, .num_parents or .flags members of struct clk. __clk_init expects those to be initialized before being called. The kerneldoc for __clk_init states this:
http://git.linaro.org/gitweb?p=people/mturquette/linux.git;a=blob;f=include/linux/clk-private.h;h=e2cce6fa604686ea3bca0496998322bacaed854d;hb=c5836d8acd9846ec1675b269d994d96a8ee44df4#l49

You can use the macros for the basic clocks types as templates. The gate and mux macros are good to look at since those show how to handle a single-parent and multi-parent clock, respectively:
http://git.linaro.org/gitweb?p=people/mturquette/linux.git;a=blob;f=include/linux/clk-private.h;h=5e4312b6f5ccb1072273ce94e235874c74dd852d;hb=2508b44cd5c134691dc5d679f728bbf82af2abe0#l83
and
http://git.linaro.org/gitweb?p=people/mturquette/linux.git;a=blob;f=include/linux/clk-private.h;h=5e4312b6f5ccb1072273ce94e235874c74dd852d;hb=2508b44cd5c134691dc5d679f728bbf82af2abe0#l148

Quoted 10 lines

?#define DEFINE_CLK(_name, _ops, _rate, _delay) ? ? ? ? \
-struct clk clk_##_name = { ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
- ? ? ? ? ? ? ? .ops ? ?= _ops, ? ? ? ? ? ? ? ? ? ? ? ? \
- ? ? ? ? ? ? ? .rate ? = _rate, ? ? ? ? ? ? ? ? ? ? ? ?\
+ ? ? ? struct clk clk_##_name; ? ? ? ? ? ? ? ? ? ? ? ? \
+ ? ? ? static struct clk_pxa clk_pxa_##_name = { ? ? ? \
+ ? ? ? ? ? ? ? .hw = { ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
+ ? ? ? ? ? ? ? ? ? ? ? .clk = &clk_##_name, ? ? ? ? ? ?\
+ ? ? ? ? ? ? ? }, ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\

Quoted 8 lines

? ? ? ? ? ? ? ?.delay ?= _delay, ? ? ? ? ? ? ? ? ? ? ? \
+ ? ? ? ? ? ? ? .rate ? = _rate, ? ? ? ? ? ? ? ? ? ? ? ?\
+ ? ? ? }; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
+ ? ? ? static struct clk clk_##_name = { ? ? ? ? ? ? ? \
+ ? ? ? ? ? ? ? .name ? = #_name, ? ? ? ? ? ? ? ? ? ? ? \
+ ? ? ? ? ? ? ? .ops ? ?= _ops, ? ? ? ? ? ? ? ? ? ? ? ? \
+ ? ? ? ? ? ? ? .hw ? ? = &clk_pxa_##_name.hw, ? ? ? ? ?\
? ? ? ?}

ditto

Quoted 11 lines

?#define DEFINE_PXA2_CKEN(_name, _cken, _rate, _delay) ?\
-struct clk clk_##_name = { ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
- ? ? ? ? ? ? ? .ops ? ?= &clk_pxa2xx_cken_ops, ? ? ? ? \
- ? ? ? ? ? ? ? .rate ? = _rate, ? ? ? ? ? ? ? ? ? ? ? ?\
+ ? ? ? struct clk clk_##_name; ? ? ? ? ? ? ? ? ? ? ? ? \
+ ? ? ? static struct clk_pxa clk_pxa_##_name = { ? ? ? \
+ ? ? ? ? ? ? ? .hw = { ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
+ ? ? ? ? ? ? ? ? ? ? ? .clk = &clk_##_name, ? ? ? ? ? ?\
+ ? ? ? ? ? ? ? }, ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
? ? ? ? ? ? ? ?.cken ? = CKEN_##_cken, ? ? ? ? ? ? ? ? \

Quoted 8 lines

? ? ? ? ? ? ? ?.delay ?= _delay, ? ? ? ? ? ? ? ? ? ? ? \
+ ? ? ? ? ? ? ? .rate ? = _rate, ? ? ? ? ? ? ? ? ? ? ? ?\
+ ? ? ? }; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
+ ? ? ? static struct clk clk_##_name = { ? ? ? ? ? ? ? \
+ ? ? ? ? ? ? ? .name ? = #_name, ? ? ? ? ? ? ? ? ? ? ? \
+ ? ? ? ? ? ? ? .ops ? ?= &clk_pxa2xx_cken_ops, ? ? ? ? \
+ ? ? ? ? ? ? ? .hw ? ? = &clk_pxa_##_name.hw, ? ? ? ? ?\
? ? ? ?}

ditto

Regards,
Mike

Philipp Zabel

author

2012-03-17 10:24 UTC #2 lore raw

PATCH 4/4 pxa: convert to common clock framework

Hi Mike,

Quoted 17 lines · Mike

Quoted 7 lines · Philipp Zabel
?#define DEFINE_CK(_name, _cken, _ops) ? ? ? ? ? ? ? ? ?\
-struct clk clk_##_name = { ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
- ? ? ? ? ? ? ? .ops ? ?= _ops, ? ? ? ? ? ? ? ? ? ? ? ? \
+ ? ? ? struct clk clk_##_name; ? ? ? ? ? ? ? ? ? ? ? ? \
+ ? ? ? static struct clk_pxa clk_pxa_##_name = { ? ? ? \
+ ? ? ? ? ? ? ? .hw = { ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
+ ? ? ? ? ? ? ? ? ? ? ? .clk = &clk_##_name, ? ? ? ? ? ?\
Quoted 8 lines
+ ? ? ? ? ? ? ? }, ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
? ? ? ? ? ? ? ?.cken ? = CKEN_##_cken, ? ? ? ? ? ? ? ? \
+ ? ? ? }; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
+ ? ? ? static struct clk clk_##_name = { ? ? ? ? ? ? ? \
+ ? ? ? ? ? ? ? .name ? = #_name, ? ? ? ? ? ? ? ? ? ? ? \
+ ? ? ? ? ? ? ? .ops ? ?= _ops, ? ? ? ? ? ? ? ? ? ? ? ? \
+ ? ? ? ? ? ? ? .hw ? ? = &clk_pxa_##_name.hw, ? ? ? ? ?\
? ? ? ?}

Quoted 7 lines

Hi Philipp,
It looks like your macros do not initialize the .parent_names, .num_parents or .flags members of struct clk. ?__clk_init expects those to be initialized before being called. ?The kerneldoc for __clk_init states this:
http://git.linaro.org/gitweb?p=people/mturquette/linux.git;a=blob;f=include/linux/clk-private.h;h=e2cce6fa604686ea3bca0496998322bacaed854d;hb=c5836d8acd9846ec1675b269d994d96a8ee44df4#l49

Yes, thank you! That was the problem.

Quoted 6 lines

You can use the macros for the basic clocks types as templates. ?The gate and mux macros are good to look at since those show how to handle a single-parent and multi-parent clock, respectively:
http://git.linaro.org/gitweb?p=people/mturquette/linux.git;a=blob;f=include/linux/clk-private.h;h=5e4312b6f5ccb1072273ce94e235874c74dd852d;hb=2508b44cd5c134691dc5d679f728bbf82af2abe0#l83
and
http://git.linaro.org/gitweb?p=people/mturquette/linux.git;a=blob;f=include/linux/clk-private.h;h=5e4312b6f5ccb1072273ce94e235874c74dd852d;hb=2508b44cd5c134691dc5d679f728bbf82af2abe0#l148

If there was a fixed fractional divider clk element, a lot of the peripheral clocks could be derived from a fixed clk representing the 312 MHz PPLL on PXA27x (fixed -> divider -> gate), using just the basic clock templates.

regards
Philipp

Sascha Hauer

2012-03-17 16:07 UTC #3 lore raw

PATCH 4/4 pxa: convert to common clock framework

Quoted 37 lines · Philipp Zabel

Hi Mike,
Quoted 17 lines · Mike
Quoted 7 lines · Philipp Zabel
?#define DEFINE_CK(_name, _cken, _ops) ? ? ? ? ? ? ? ? ?\
-struct clk clk_##_name = { ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
- ? ? ? ? ? ? ? .ops ? ?= _ops, ? ? ? ? ? ? ? ? ? ? ? ? \
+ ? ? ? struct clk clk_##_name; ? ? ? ? ? ? ? ? ? ? ? ? \
+ ? ? ? static struct clk_pxa clk_pxa_##_name = { ? ? ? \
+ ? ? ? ? ? ? ? .hw = { ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
+ ? ? ? ? ? ? ? ? ? ? ? .clk = &clk_##_name, ? ? ? ? ? ?\
Quoted 8 lines
+ ? ? ? ? ? ? ? }, ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
? ? ? ? ? ? ? ?.cken ? = CKEN_##_cken, ? ? ? ? ? ? ? ? \
+ ? ? ? }; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
+ ? ? ? static struct clk clk_##_name = { ? ? ? ? ? ? ? \
+ ? ? ? ? ? ? ? .name ? = #_name, ? ? ? ? ? ? ? ? ? ? ? \
+ ? ? ? ? ? ? ? .ops ? ?= _ops, ? ? ? ? ? ? ? ? ? ? ? ? \
+ ? ? ? ? ? ? ? .hw ? ? = &clk_pxa_##_name.hw, ? ? ? ? ?\
? ? ? ?}
Quoted 7 lines
Hi Philipp,
It looks like your macros do not initialize the .parent_names, .num_parents or .flags members of struct clk. ?__clk_init expects those to be initialized before being called. ?The kerneldoc for __clk_init states this:
http://git.linaro.org/gitweb?p=people/mturquette/linux.git;a=blob;f=include/linux/clk-private.h;h=e2cce6fa604686ea3bca0496998322bacaed854d;hb=c5836d8acd9846ec1675b269d994d96a8ee44df4#l49
Yes, thank you! That was the problem.
Quoted 6 lines
You can use the macros for the basic clocks types as templates. ?The gate and mux macros are good to look at since those show how to handle a single-parent and multi-parent clock, respectively:
http://git.linaro.org/gitweb?p=people/mturquette/linux.git;a=blob;f=include/linux/clk-private.h;h=5e4312b6f5ccb1072273ce94e235874c74dd852d;hb=2508b44cd5c134691dc5d679f728bbf82af2abe0#l83
and
http://git.linaro.org/gitweb?p=people/mturquette/linux.git;a=blob;f=include/linux/clk-private.h;h=5e4312b6f5ccb1072273ce94e235874c74dd852d;hb=2508b44cd5c134691dc5d679f728bbf82af2abe0#l148

Quoted 4 lines

If there was a fixed fractional divider clk element, a lot of the peripheral clocks could be derived from a fixed clk representing the 312 MHz PPLL on PXA27x (fixed -> divider -> gate), using just the basic clock templates.

You could try the following. It is not thoroughly tested but may be a starting point. I need this aswell for i.MX so I will continue to work on it soon.

Sascha

8<-------------------------------------------------------

clk: add a fixed factor clock

Having fixed factors/dividers in hardware is a common pattern, so add a clock doing this. Currently no rate propagation is supported.

Signed-off-by Sascha Hauer <[email protected]>

---
 drivers/clk/Makefile           |    2 +-
 drivers/clk/clk-fixed-factor.c |   97 ++++++++++++++++++++++++++++++++++++++++
 include/linux/clk-provider.h   |    4 ++
 3 files changed, 102 insertions(+), 1 deletions(-)
 create mode 100644 drivers/clk/clk-fixed-factor.c

drivers/clk/Makefile +1 -1

index ba9a779..54f6f5b 100644
@@ -1,4 +1,4 @@
 
 obj-$(CONFIG_CLKDEV_LOOKUP)	+= clkdev.o
 obj-$(CONFIG_COMMON_CLK)	+= clk.o clk-fixed-rate.o clk-gate.o \
-				   clk-mux.o clk-divider.o clk-test.o
+				   clk-mux.o clk-divider.o clk-fixed-factor.o clk-test.o

drivers/clk/clk-fixed-factor.c new file +97 -0

97 lines not on this page · raw message

include/linux/clk-provider.h +4 -0

index 5508897..21ca2f8 100644
@@ -257,6 +257,10 @@ struct clk *clk_register_mux(struct device *dev, const char *name,
 		void __iomem *reg, u8 shift, u8 width,
 		u8 clk_mux_flags, spinlock_t *lock);
 
+struct clk *clk_register_fixed_factor(struct device *dev, const char *name,
+		const char *parent_name, unsigned long flags,
+		unsigned int mult, unsigned int div);
+
 /**
  * clk_register - allocate a new clock, register it and return an opaque cookie
  * @dev: device that is registering this clock

Signature · 7 lines

1.7.9.1

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Philipp Zabel

author

2012-03-19 08:47 UTC #4 lore raw

PATCH 4/4 pxa: convert to common clock framework

Quoted 9 lines · Sascha Hauer

Quoted 4 lines · Philipp Zabel
If there was a fixed fractional divider clk element, a lot of the peripheral clocks could be derived from a fixed clk representing the 312 MHz PPLL on PXA27x (fixed -> divider -> gate), using just the basic clock templates.
You could try the following. It is not thoroughly tested but may be a starting point. I need this aswell for i.MX so I will continue to work on it soon.

Thanks, this is exactly what I was looking for.

I'm a bit wary of adding so many clocks that just represent fixed hardware, especially with the amount of function calls needed. But describing the clock tree correctly appeals to my sense of order.

What is the take of the PXA maintainers on this? Are there maybe plans to add device tree support for PXA?

regards
Philipp

Arnd Bergmann

2012-03-19 10:00 UTC #5 lore raw

PATCH 4/4 pxa: convert to common clock framework

Quoted 9 lines · Philipp Zabel

Thanks, this is exactly what I was looking for.
I'm a bit wary of adding so many clocks that just represent fixed hardware, especially with the amount of function calls needed. But describing the clock tree correctly appeals to my sense of order.
What is the take of the PXA maintainers on this? Are there maybe plans to add device tree support for PXA?

Haojian Zhuang has been busy posting DT patches recently. The patches ended up being too late for v3.4, but I expect the first set to get submitted for arm-soc after the merge window has ended.

There is still some infrastructure work required, e.g. in the interrupt controller, but things will be easier after -rc1 when the irqdomain and clk support is merged.

Arnd

Turquette, Mike

2012-03-17 18:06 UTC #6 lore raw

PATCH 4/4 pxa: convert to common clock framework

In reply to Philipp Zabel

Quoted 22 lines · Philipp Zabel

Hi Mike,
Quoted 19 lines · Mike
Quoted 7 lines · Philipp Zabel
?#define DEFINE_CK(_name, _cken, _ops) ? ? ? ? ? ? ? ? ?\
-struct clk clk_##_name = { ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
- ? ? ? ? ? ? ? .ops ? ?= _ops, ? ? ? ? ? ? ? ? ? ? ? ? \
+ ? ? ? struct clk clk_##_name; ? ? ? ? ? ? ? ? ? ? ? ? \
+ ? ? ? static struct clk_pxa clk_pxa_##_name = { ? ? ? \
+ ? ? ? ? ? ? ? .hw = { ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? \
+ ? ? ? ? ? ? ? ? ? ? ? .clk = &clk_##_name, ? ? ? ? ? ?\
Quoted 8 lines
+ ? ? ? ? ? ? ? }, ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
? ? ? ? ? ? ? ?.cken ? = CKEN_##_cken, ? ? ? ? ? ? ? ? \
+ ? ? ? }; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
+ ? ? ? static struct clk clk_##_name = { ? ? ? ? ? ? ? \
+ ? ? ? ? ? ? ? .name ? = #_name, ? ? ? ? ? ? ? ? ? ? ? \
+ ? ? ? ? ? ? ? .ops ? ?= _ops, ? ? ? ? ? ? ? ? ? ? ? ? \
+ ? ? ? ? ? ? ? .hw ? ? = &clk_pxa_##_name.hw, ? ? ? ? ?\
? ? ? ?}
Hi Philipp,

Quoted 7 lines

Quoted 5 lines
It looks like your macros do not initialize the .parent_names, .num_parents or .flags members of struct clk. ?__clk_init expects those to be initialized before being called. ?The kerneldoc for __clk_init states this:
http://git.linaro.org/gitweb?p=people/mturquette/linux.git;a=blob;f=include/linux/clk-private.h;h=e2cce6fa604686ea3bca0496998322bacaed854d;hb=c5836d8acd9846ec1675b269d994d96a8ee44df4#l49
Yes, thank you! That was the problem.

Awesome! Glad that it's working for you now without the hacks.

Quoted 11 lines

Quoted 6 lines
You can use the macros for the basic clocks types as templates. ?The gate and mux macros are good to look at since those show how to handle a single-parent and multi-parent clock, respectively:
http://git.linaro.org/gitweb?p=people/mturquette/linux.git;a=blob;f=include/linux/clk-private.h;h=5e4312b6f5ccb1072273ce94e235874c74dd852d;hb=2508b44cd5c134691dc5d679f728bbf82af2abe0#l83
and
http://git.linaro.org/gitweb?p=people/mturquette/linux.git;a=blob;f=include/linux/clk-private.h;h=5e4312b6f5ccb1072273ce94e235874c74dd852d;hb=2508b44cd5c134691dc5d679f728bbf82af2abe0#l148
If there was a fixed fractional divider clk element, a lot of the peripheral clocks could be derived from a fixed clk representing the 312 MHz PPLL on PXA27x (fixed -> divider -> gate), using just the basic clock templates.

We need more common clock types, no question. I have code for fixed-dividers and a standard "dummy" clock which I think will be critical for DT clock bindings when wiring up discrete devices. Sascha probably has what you need in his tree already.

All of these changes are welcome additions to the framework, but I didn't want to slow down the merge process with feature creep. Happy to take any patches from you in the future.

Regards,
Mike

Shawn Guo

2012-03-18 05:43 UTC #7 lore raw

PATCH 4/4 pxa: convert to common clock framework

In reply to Philipp Zabel

Your patch #2 is trying to avoid the layering violation by separating static clock definitions and from clk_ops implementations into different files. But ...

On Fri, Mar 16, 2012 at 06:38:03PM +0100, Philipp Zabel wrote:
...

arch/arm/mach-pxa/clock.h line 2

Quoted 7 lines

arch/arm/mach-pxa/clock.h +1 -0

index a3585a2..7c04f9b 100644
@@ -1,80 +1,106 @@
 #include <linux/clkdev.h>
+#include <linux/clk-private.h>

... This is what I read from clk-private.h.

/*
* WARNING: Do not include clk-private.h from any file that implements struct
* clk_ops. Doing so is a layering violation!
*
* ...
*/

And you include it in arch/arm/mach-pxa/clock.h which in turn is being included by the pxa clock files implementing clk_ops.

arch/arm/mach-pxa/clock.h line 4

Quoted 2 lines

arch/arm/mach-pxa/clock.h +1 -0

+#include <linux/clk-provider.h>
 #include <linux/syscore_ops.h>

Regards,
Shawn

Philipp Zabel

author

2012-03-19 08:51 UTC #8 lore raw

PATCH 4/4 pxa: convert to common clock framework

Quoted 6 lines · Shawn Guo

Your patch #2 is trying to avoid the layering violation by separating static clock definitions and from clk_ops implementations into different files. ?But ...
On Fri, Mar 16, 2012 at 06:38:03PM +0100, Philipp Zabel wrote:
...

Quoted 9 lines

Quoted 7 lines
arch/arm/mach-pxa/clock.h +0 -0
index a3585a2..7c04f9b 100644
@@ -1,80 +1,106 @@
?#include <linux/clkdev.h>
+#include <linux/clk-private.h>
... This is what I read from clk-private.h.

Thank you.
I could just remove that include line from clock.h, because it is only needed for the static clock definition macros.
The pxa[23]?x.c, where they are used, already #include <clk-private.h> directly. Or, to do it properly, clock.h could also be split in two.

regards
Philipp