clocksource: dw_apb_timer: Add common DTS glue for dw_apb_timer

PATCH Superseded dinguyen at altera.com 3 participants 5 messages Jul 11, 2012

Subsystems (5)

Device tree bindings Arm (32-bit) Arm sub-architectures Real time clock (RTC) +1 more subsystems

lore

Share & tools

lore

dinguyen at altera.com

author

2012-07-11 16:12 UTC #0 lore raw

PATCH v3 1/1 clocksource: dw_apb_timer: Add common DTS glue for dw_apb_timer

From: Dinh Nguyen <[email protected]>

Make a common device tree glue for clocksource/dw_apb_timer. Move mach-picoxcell/time.c to be a generic device tree application of the dw_apb_timer.

Configure mach-picoxcell to use the dw_apb_timer_of device tree implementation in drivers/clocksource.

Signed-off-by Pavel Machek <[email protected]>

Signed-off-by Dinh Nguyen <[email protected]>

Acked-by Jamie Iles <[email protected]>

---
 Documentation/devicetree/bindings/rtc/dw-apb.txt   |   24 ++++++++++
 arch/arm/Kconfig                                   |    1 +
 arch/arm/mach-picoxcell/Makefile                   |    1 -
 arch/arm/mach-picoxcell/common.c                   |    3 +-
 arch/arm/mach-picoxcell/common.h                   |    2 +-
 drivers/clocksource/Kconfig                        |    3 ++
 drivers/clocksource/Makefile                       |    1 +
 .../clocksource/dw_apb_timer_of.c                  |   47 ++++++++++++--------
 8 files changed, 60 insertions(+), 22 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/rtc/dw-apb.txt
 rename arch/arm/mach-picoxcell/time.c => drivers/clocksource/dw_apb_timer_of.c (61%)

Documentation/devicetree/bindings/rtc/dw-apb.txt new file +24 -0

new file mode 100644
index 0000000..bbc459c
@@ -0,0 +1,24 @@
+* Designware APB timer
+
+Required properties:
+- compatible: "snps,dw-apb-timer-sp" or "snps,dw-apb-timer-osc"
+- reg: physical base address of the controller and length of memory mapped
+  region.
+- interrupts: IRQ line for the timer.
+- clock-frequency: The frequency in HZ of the timer.
+
+Example:
+
+		timer1: timer at ffc09000 {
+				compatible = "snps,dw-apb-timer-sp";
+				interrupts = <0 168 4>;
+				clock-frequency = <200000000>;
+				reg = <0xffc09000 0x1000>;
+			};
+
+		timer2: timer at ffd00000 {
+				compatible = "snps,dw-apb-timer-osc";
+				interrupts = <0 169 4>;
+				clock-frequency = <200000000>;
+				reg = <0xffd00000 0x1000>;
+			};

arch/arm/Kconfig +1 -0

index a91009c..57eb6ef 100644
@@ -658,6 +658,7 @@ config ARCH_PICOXCELL
 	select ARM_VIC
 	select CPU_V6K
 	select DW_APB_TIMER
+	select DW_APB_TIMER_OF
 	select GENERIC_CLOCKEVENTS
 	select GENERIC_GPIO
 	select HAVE_TCM

arch/arm/mach-picoxcell/Makefile +0 -1

index e5ec4a8..8e39f80 100644
@@ -1,2 +1 @@
 obj-y	:= common.o
-obj-y	+= time.o

arch/arm/mach-picoxcell/common.c +2 -1

index a2e8ae8..8f9a0b4 100644
@@ -14,6 +14,7 @@
 #include <linux/of_address.h>
 #include <linux/of_irq.h>
 #include <linux/of_platform.h>
+#include <linux/dw_apb_timer.h>
 
 #include <asm/mach/arch.h>
 #include <asm/hardware/vic.h>

@@ -97,7 +98,7 @@ DT_MACHINE_START(PICOXCELL, "Picochip picoXcell")
 	.nr_irqs	= NR_IRQS_LEGACY,
 	.init_irq	= picoxcell_init_irq,
 	.handle_irq	= vic_handle_irq,
-	.timer		= &picoxcell_timer,
+	.timer		= &dw_apb_timer,
 	.init_machine	= picoxcell_init_machine,
 	.dt_compat	= picoxcell_dt_match,
 	.restart	= picoxcell_wdt_restart,

arch/arm/mach-picoxcell/common.h +1 -1

index 83d55ab..a65cb02 100644
@@ -12,6 +12,6 @@
 
 #include <asm/mach/time.h>
 
-extern struct sys_timer picoxcell_timer;
+extern struct sys_timer dw_apb_timer;
 
 #endif /* __PICOXCELL_COMMON_H__ */

drivers/clocksource/Kconfig +3 -0

index 99c6b20..e62bc7e 100644
@@ -16,6 +16,9 @@ config CLKSRC_MMIO
 config DW_APB_TIMER
 	bool
 
+config DW_APB_TIMER_OF
+	bool
+
 config CLKSRC_DBX500_PRCMU
 	bool "Clocksource PRCMU Timer"
 	depends on UX500_SOC_DB8500

drivers/clocksource/Makefile +1 -0

index dd3e661..2cdaf7d 100644
@@ -10,4 +10,5 @@ obj-$(CONFIG_EM_TIMER_STI)	+= em_sti.o
 obj-$(CONFIG_CLKBLD_I8253)	+= i8253.o
 obj-$(CONFIG_CLKSRC_MMIO)	+= mmio.o
 obj-$(CONFIG_DW_APB_TIMER)	+= dw_apb_timer.o
+obj-$(CONFIG_DW_APB_TIMER_OF)	+= dw_apb_timer_of.o
 obj-$(CONFIG_CLKSRC_DBX500_PRCMU)	+= clksrc-dbx500-prcmu.o
\ No newline at end of file

drivers/clocksource/dw_apb_timer_of.c renamed from arch/arm/mach-picoxcell/time.c +28 ~~-19~~

116 lines not on this page · raw message

Signature · 1 line

1.7.9.5

Pavel Machek

2012-07-11 16:20 UTC #1 lore raw

PATCH v3 1/1 clocksource: dw_apb_timer: Add common DTS glue for dw_apb_timer

Hi!

Quoted 10 lines

Make a common device tree glue for clocksource/dw_apb_timer. Move mach-picoxcell/time.c to be a generic device tree application of the dw_apb_timer.
Configure mach-picoxcell to use the dw_apb_timer_of device tree implementation in drivers/clocksource.
Signed-off-by Pavel Machek <[email protected]>
Signed-off-by Dinh Nguyen <[email protected]>
Acked-by Jamie Iles <[email protected]>

Ok, this is getting complex.

Documentation/devicetree/bindings/rtc/dw-apb.txt line 8

Quoted 11 lines

Documentation/devicetree/bindings/rtc/dw-apb.txt new file +8 -0

@@ -0,0 +1,24 @@
+* Designware APB timer
+
+Required properties:
+- compatible: "snps,dw-apb-timer-sp" or "snps,dw-apb-timer-osc"
+- reg: physical base address of the controller and length of memory mapped
+  region.
+- interrupts: IRQ line for the timer.
+- clock-frequency: The frequency in HZ of the timer.

I know device-tree maintainers requested -freq -> -frequency,

drivers/clocksource/dw_apb_timer_of.c

Quoted 2 lines

drivers/clocksource/dw_apb_timer_of.c renamed from arch/arm/mach-picoxcell/time.c +0 -0

But then this will need updating to accept "-frequency",

drivers/clocksource/dw_apb_timer_of.c static void picoxcell_add_clocksource(struct device_node *source_timer) · line 107

Quoted 6 lines

drivers/clocksource/dw_apb_timer_of.c renamed from arch/arm/mach-picoxcell/time.c +2 -1

-static const struct of_device_id picoxcell_timer_ids[] __initconst = {
+static const struct of_device_id osctimer_ids[] __initconst = {
 	{ .compatible = "picochip,pc3x2-timer" },
+	{ .compatible = "snps,dw-apb-timer-osc" },
 	{},
 };

and dts will need to be updated, anyway, so we could update "picochip,pc3x2-timer" -> "snps,dw-apb-timer-osc" as well, right?

Alternatively, we could just keep using "-freq"....

Or am I overlooking something?

Otherwise looks ok to me.
Pavel

Signature · 2 lines

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Rob Herring

2012-07-11 17:37 UTC #2 lore raw

PATCH v3 1/1 clocksource: dw_apb_timer: Add common DTS glue for dw_apb_timer

Quoted 43 lines · Pavel Machek

Hi!
Quoted 10 lines
Make a common device tree glue for clocksource/dw_apb_timer. Move mach-picoxcell/time.c to be a generic device tree application of the dw_apb_timer.
Configure mach-picoxcell to use the dw_apb_timer_of device tree implementation in drivers/clocksource.
Signed-off-by Pavel Machek <[email protected]>
Signed-off-by Dinh Nguyen <[email protected]>
Acked-by Jamie Iles <[email protected]>
Ok, this is getting complex.
Documentation/devicetree/bindings/rtc/dw-apb.txt line 8
Quoted 11 lines
Documentation/devicetree/bindings/rtc/dw-apb.txt new file +8 -0
@@ -0,0 +1,24 @@
+* Designware APB timer
+
+Required properties:
+- compatible: "snps,dw-apb-timer-sp" or "snps,dw-apb-timer-osc"
+- reg: physical base address of the controller and length of memory mapped
+  region.
+- interrupts: IRQ line for the timer.
+- clock-frequency: The frequency in HZ of the timer.
I know device-tree maintainers requested -freq -> -frequency,
drivers/clocksource/dw_apb_timer_of.c
Quoted 2 lines
drivers/clocksource/dw_apb_timer_of.c renamed from arch/arm/mach-picoxcell/time.c +0 -0
But then this will need updating to accept "-frequency",
drivers/clocksource/dw_apb_timer_of.c static void picoxcell_add_clocksource(struct device_node *source_timer) · line 107
Quoted 6 lines
drivers/clocksource/dw_apb_timer_of.c renamed from arch/arm/mach-picoxcell/time.c +2 -1
-static const struct of_device_id picoxcell_timer_ids[] __initconst = {
+static const struct of_device_id osctimer_ids[] __initconst = {
 	{ .compatible = "picochip,pc3x2-timer" },
+	{ .compatible = "snps,dw-apb-timer-osc" },
 	{},
 };
and dts will need to be updated, anyway, so we could update "picochip,pc3x2-timer" -> "snps,dw-apb-timer-osc" as well, right?

Quoted 3 lines

Alternatively, we could just keep using "-freq"....
Or am I overlooking something?

If picoxcell is already using "-freq", then yes, you'll have to keep it for compatibility. You have to assume that a dtb cannot be changed.

Rob

Dinh Nguyen

author

2012-07-11 19:35 UTC #3 lore raw

PATCH v3 1/1 clocksource: dw_apb_timer: Add common DTS glue for dw_apb_timer

Quoted 44 lines · Rob Herring

Quoted 43 lines · Pavel Machek
Hi!
Quoted 10 lines
Make a common device tree glue for clocksource/dw_apb_timer. Move mach-picoxcell/time.c to be a generic device tree application of the dw_apb_timer.
Configure mach-picoxcell to use the dw_apb_timer_of device tree implementation in drivers/clocksource.
Signed-off-by Pavel Machek <[email protected]>
Signed-off-by Dinh Nguyen <[email protected]>
Acked-by Jamie Iles <[email protected]>
Ok, this is getting complex.
Documentation/devicetree/bindings/rtc/dw-apb.txt line 8
Quoted 11 lines
Documentation/devicetree/bindings/rtc/dw-apb.txt new file +8 -0
@@ -0,0 +1,24 @@
+* Designware APB timer
+
+Required properties:
+- compatible: "snps,dw-apb-timer-sp" or "snps,dw-apb-timer-osc"
+- reg: physical base address of the controller and length of memory mapped
+  region.
+- interrupts: IRQ line for the timer.
+- clock-frequency: The frequency in HZ of the timer.
I know device-tree maintainers requested -freq -> -frequency,
drivers/clocksource/dw_apb_timer_of.c
Quoted 2 lines
drivers/clocksource/dw_apb_timer_of.c renamed from arch/arm/mach-picoxcell/time.c +0 -0
But then this will need updating to accept "-frequency",
drivers/clocksource/dw_apb_timer_of.c static void picoxcell_add_clocksource(struct device_node *source_timer) · line 107
Quoted 6 lines
drivers/clocksource/dw_apb_timer_of.c renamed from arch/arm/mach-picoxcell/time.c +2 -1
-static const struct of_device_id picoxcell_timer_ids[] __initconst = {
+static const struct of_device_id osctimer_ids[] __initconst = {
 	{ .compatible = "picochip,pc3x2-timer" },
+	{ .compatible = "snps,dw-apb-timer-osc" },
 	{},
 };
and dts will need to be updated, anyway, so we could update "picochip,pc3x2-timer" -> "snps,dw-apb-timer-osc" as well, right?

Quoted 6 lines

Quoted 3 lines
Alternatively, we could just keep using "-freq"....
Or am I overlooking something?
If picoxcell is already using "-freq", then yes, you'll have to keep it for compatibility. You have to assume that a dtb cannot be changed.

So just to be sure, since we don't want to create churn in the dts files. Only picoxcell is using clock-freq, so to keep compatibility, something like this should be done:

- if (of_property_read_u32(np, "clock-freq", rate))
+ if (of_property_read_u32(np, "clock-freq", rate) &&
+ of_property_read_u32(np, "clock-frequency", rate))

                panic("No clock-frequency property for %s", np->name);

Going forward, platforms that use the dq_apb_timer should use clock-frequency?

Thanks,
Dinh

Quoted 3 lines

Rob

Rob Herring

2012-07-11 19:38 UTC #4 lore raw

PATCH v3 1/1 clocksource: dw_apb_timer: Add common DTS glue for dw_apb_timer

Quoted 61 lines · Dinh Nguyen

Quoted 44 lines · Rob Herring
Quoted 43 lines · Pavel Machek
Hi!
Quoted 10 lines
Make a common device tree glue for clocksource/dw_apb_timer. Move mach-picoxcell/time.c to be a generic device tree application of the dw_apb_timer.
Configure mach-picoxcell to use the dw_apb_timer_of device tree implementation in drivers/clocksource.
Signed-off-by Pavel Machek <[email protected]>
Signed-off-by Dinh Nguyen <[email protected]>
Acked-by Jamie Iles <[email protected]>
Ok, this is getting complex.
Documentation/devicetree/bindings/rtc/dw-apb.txt line 8
Quoted 11 lines
Documentation/devicetree/bindings/rtc/dw-apb.txt new file +8 -0
@@ -0,0 +1,24 @@
+* Designware APB timer
+
+Required properties:
+- compatible: "snps,dw-apb-timer-sp" or "snps,dw-apb-timer-osc"
+- reg: physical base address of the controller and length of memory mapped
+  region.
+- interrupts: IRQ line for the timer.
+- clock-frequency: The frequency in HZ of the timer.
I know device-tree maintainers requested -freq -> -frequency,
drivers/clocksource/dw_apb_timer_of.c
Quoted 2 lines
drivers/clocksource/dw_apb_timer_of.c renamed from arch/arm/mach-picoxcell/time.c +0 -0
But then this will need updating to accept "-frequency",
drivers/clocksource/dw_apb_timer_of.c static void picoxcell_add_clocksource(struct device_node *source_timer) · line 107
Quoted 6 lines
drivers/clocksource/dw_apb_timer_of.c renamed from arch/arm/mach-picoxcell/time.c +2 -1
-static const struct of_device_id picoxcell_timer_ids[] __initconst = {
+static const struct of_device_id osctimer_ids[] __initconst = {
 	{ .compatible = "picochip,pc3x2-timer" },
+	{ .compatible = "snps,dw-apb-timer-osc" },
 	{},
 };
and dts will need to be updated, anyway, so we could update "picochip,pc3x2-timer" -> "snps,dw-apb-timer-osc" as well, right?
Quoted 6 lines
Quoted 3 lines
Alternatively, we could just keep using "-freq"....
Or am I overlooking something?
If picoxcell is already using "-freq", then yes, you'll have to keep it for compatibility. You have to assume that a dtb cannot be changed.
So just to be sure, since we don't want to create churn in the dts files. Only picoxcell is using clock-freq, so to keep compatibility, something like this should be done:
(quoted diff) +2 -1
-       if (of_property_read_u32(np, "clock-freq", rate))
+       if (of_property_read_u32(np, "clock-freq", rate) &&
+               of_property_read_u32(np, "clock-frequency", rate))
                panic("No clock-frequency property for %s", np->name);

Quoted 2 lines

Going forward, platforms that use the dq_apb_timer should use clock-frequency?

Yeah, that's fine.

Rob