Hi Thomas,

On 11/03/2012 03:45 PM, Thomas Abraham wrote:
Allow the MCT controller base address and interrupts to be obtained from
device tree and remove unused static definitions of these. The non-dt support
for Exynos5250 is removed but retained for Exynos4210 based platforms.

Cc: Changhwan Youn<[email protected]>
Signed-off-by: Thomas Abraham<[email protected]>
---
  .../bindings/timer/samsung,exynos4210-mct.txt      |   70 ++++++++++++++++++++
  arch/arm/mach-exynos/include/mach/irqs.h           |    6 --
  arch/arm/mach-exynos/mct.c                         |   42 ++++++++----
  3 files changed, 99 insertions(+), 19 deletions(-)
  create mode 100644 
Documentation/devicetree/bindings/timer/samsung,exynos4210-mct.txt

diff --git a/Documentation/devicetree/bindings/timer/samsung,exynos4210-mct.txt 
b/Documentation/devicetree/bindings/timer/samsung,exynos4210-mct.txt
new file mode 100644
index 0000000..c53fd93
--- /dev/null
+++ b/Documentation/devicetree/bindings/timer/samsung,exynos4210-mct.txt
@@ -0,0 +1,70 @@
+Samsung's Multi Core Timer (MCT)
+
+The Samsung's Multi Core Timer (MCT) module includes two main blocks, the
+global timer and CPU local timers. The global timer is a 64-bit free running
+up-counter and can generate 4 interrupts when the counter reaches one of the
+four preset counter values. The CPU local timers are 32-bit free running
+down-counters and generates an interrupt when the counter expires. There is

s/generates/generate ?

+one CPU local timer instantiated in MCT for every CPU in the system.
+
+Required properties:
+
+- compatible: should be "samsung,exynos4210-mct".
+- reg: base address of the mct controller and length of the address space
+  it occupies.
+- interrupts: the list of interrupts generated by the controller. The following
+  should be the order of the interrupts specified. The local timer interrupts
+  should be specified after the four global timer interrupts have been
+  specified.
+
+       0: Global Timer Interrupt 0
+       1: Global Timer Interrupt 1
+       2: Global Timer Interrupt 2
+       3: Global Timer Interrupt 3
+       4: Local Timer Interrupt 0
+       5: Local Timer Interrupt 1
+       6: ..
+       7: ..
+       i: Local Timer Interrupt n
+
+- samsung,mct-nr-local-irqs: The number of local timer interrupts supported
+  by the MCT controller.
+
+Example 1: In this example, the system uses only the first global timer
+          interrupt generated by MCT and the remaining three global timer
+          interrupts are unused. Two local timer interrupts have been
+          specified.
+
+       mct@10050000 {
+               compatible = "samsung,exynos4210-mct";
+               reg =<0x10050000 0x800>;
+               interrupts =<0 57 0>,<0 0 0>,<0 0 0>,<0 0 0>,
+                       <0 42 0>,<0 48 0>;
+               samsung,mct-nr-local-irqs =<4>;

Then this means the MCT supports 4 local interrupts but only 2 are
specified here ? Doesn't the code below expect

        samsung,mct-nr-local-irqs = <2>;

in this case ? Or should interrupts really be

        interrupts =<0 57 0>, <0 0 0>, <0 0 0>, <0 0 0>,
                <0 42 0>, <0 48 0>, <0 0 0>, <0 0 0>;
?
+       };
+
+Example 2: In this example, the MCT global and local timer interrupts are
+          connected to two seperate interrupt controllers. Hence, an
+          interrupt-map is created to map the interrupts to the respective
+          interrupt controllers.
+
+       mct@101C0000 {
+               compatible = "samsung,exynos4210-mct";
+               reg =<0x101C0000 0x800>;
+               interrupt-controller;
+               #interrups-cells =<2>;
+               interrupt-parent =<&mct_map>;
+               interrupts =<0 0>,<1 0>,<2 0>,<3 0>,
+                       <4 0>,<5 0>;
+               samsung,mct-nr-local-irqs =<2>;

Here the samsung,mct-nr-local-irqs' value matches what's specified in the interrupts property.

+
+               mct_map: mct-map {
+                       compatible = "samsung,mct-map";

Do we need a compatible property in sub-nodes like this one ?
Wouldn't it be sufficient to reference this node, for example by name ?

+                       #interrupt-cells =<2>;
+                       #address-cells =<0>;
+                       #size-cells =<0>;
+                       interrupt-map =<0x0 0&combiner 23 3>,
+                                       <0x4 0&gic 0 120 0>,
+                                       <0x5 0&gic 0 121 0>;
+               };
+       };
diff --git a/arch/arm/mach-exynos/include/mach/irqs.h 
b/arch/arm/mach-exynos/include/mach/irqs.h
index 6da3115..03c9f04 100644
--- a/arch/arm/mach-exynos/include/mach/irqs.h
+++ b/arch/arm/mach-exynos/include/mach/irqs.h
@@ -30,8 +30,6 @@

  /* For EXYNOS4 and EXYNOS5 */

-#define EXYNOS_IRQ_MCT_LOCALTIMER      IRQ_PPI(12)
-
  #define EXYNOS_IRQ_EINT16_31          IRQ_SPI(32)

  /* For EXYNOS4 SoCs */
@@ -320,8 +318,6 @@
  #define EXYNOS5_IRQ_CEC                       IRQ_SPI(114)
  #define EXYNOS5_IRQ_SATA              IRQ_SPI(115)

-#define EXYNOS5_IRQ_MCT_L0             IRQ_SPI(120)
-#define EXYNOS5_IRQ_MCT_L1             IRQ_SPI(121)
  #define EXYNOS5_IRQ_MMC44             IRQ_SPI(123)
  #define EXYNOS5_IRQ_MDMA1             IRQ_SPI(124)
  #define EXYNOS5_IRQ_FIMC_LITE0                IRQ_SPI(125)
@@ -411,8 +407,6 @@
  #define EXYNOS5_IRQ_PMU_CPU1          COMBINER_IRQ(22, 4)

  #define EXYNOS5_IRQ_EINT0             COMBINER_IRQ(23, 0)
-#define EXYNOS5_IRQ_MCT_G0             COMBINER_IRQ(23, 3)
-#define EXYNOS5_IRQ_MCT_G1             COMBINER_IRQ(23, 4)

  #define EXYNOS5_IRQ_EINT1             COMBINER_IRQ(24, 0)
  #define EXYNOS5_IRQ_SYSMMU_LITE1_0    COMBINER_IRQ(24, 1)
diff --git a/arch/arm/mach-exynos/mct.c b/arch/arm/mach-exynos/mct.c
index d65d0c7..f7792b8 100644
--- a/arch/arm/mach-exynos/mct.c
+++ b/arch/arm/mach-exynos/mct.c
@@ -19,6 +19,9 @@
  #include<linux/platform_device.h>
  #include<linux/delay.h>
  #include<linux/percpu.h>
+#include<linux/of.h>
+#include<linux/of_irq.h>
+#include<linux/of_address.h>

  #include<asm/hardware/gic.h>
  #include<asm/localtimer.h>
@@ -483,14 +486,16 @@ static struct local_timer_ops exynos4_mct_tick_ops 
__cpuinitdata = {
  };
  #endif /* CONFIG_LOCAL_TIMERS */

-static void __init exynos4_timer_resources(void)
+static void __init exynos4_timer_resources(struct device_node *np)
  {
        struct clk *mct_clk;
        mct_clk = clk_get(NULL, "xtal");

        clk_rate = clk_get_rate(mct_clk);

-       reg_base = S5P_VA_SYSTIMER;
+       reg_base = (np) ? of_iomap(np, 0) : S5P_VA_SYSTIMER;

nit: Parentheses around np look redundant.

+       if (!reg_base)
+               panic("%s: unable to ioremap mct address space\n", __func__);

How about adding a line like:

#define pr_fmt(fmt) "%s: " fmt, __func__

on top of this file and dropping "%s: " and __func__ in those panic() calls ? It would make the logs more consistent across whole file.

  #ifdef CONFIG_LOCAL_TIMERS
        if (mct_int_type == MCT_INT_PPI) {
@@ -509,23 +514,34 @@ static void __init exynos4_timer_resources(void)

  static void __init exynos4_timer_init(void)
  {
-       if (soc_is_exynos4210()) {
+       struct device_node *np;
+       u32 nr_irqs, i;
+
+       np = of_find_compatible_node(NULL, NULL, "samsung,exynos4210-mct");
+       if (np) {
+               if (of_machine_is_compatible("samsung,exynos4210") ||
+                       of_machine_is_compatible("samsung,exynos5250"))
+                       mct_int_type = MCT_INT_SPI;
+               else
+                       mct_int_type = MCT_INT_PPI;

Does it make sense to specify this mct_int_type as a property of
the mct node ?

+
+               if (of_property_read_u32(np, "samsung,mct-nr-local-irqs",
+                                               &nr_irqs))
+                       panic("%s: number of local irqs not specified\n",
+                                               __func__);
+
+               mct_irqs[MCT_G0_IRQ] = irq_of_parse_and_map(np, MCT_G0_IRQ);
+               for (i = 0; i<  nr_irqs; i++)
+                       mct_irqs[MCT_L0_IRQ + i] =
+                               irq_of_parse_and_map(np, MCT_L0_IRQ + i);
+       } else if (soc_is_exynos4210()) {
                mct_irqs[MCT_G0_IRQ] = EXYNOS4_IRQ_MCT_G0;
                mct_irqs[MCT_L0_IRQ] = EXYNOS4_IRQ_MCT_L0;
                mct_irqs[MCT_L1_IRQ] = EXYNOS4_IRQ_MCT_L1;
                mct_int_type = MCT_INT_SPI;
-       } else if (soc_is_exynos5250()) {
-               mct_irqs[MCT_G0_IRQ] = EXYNOS5_IRQ_MCT_G0;
-               mct_irqs[MCT_L0_IRQ] = EXYNOS5_IRQ_MCT_L0;
-               mct_irqs[MCT_L1_IRQ] = EXYNOS5_IRQ_MCT_L1;
-               mct_int_type = MCT_INT_SPI;
-       } else {
-               mct_irqs[MCT_G0_IRQ] = EXYNOS4_IRQ_MCT_G0;
-               mct_irqs[MCT_L0_IRQ] = EXYNOS_IRQ_MCT_LOCALTIMER;
-               mct_int_type = MCT_INT_PPI;
        }

-       exynos4_timer_resources();
+       exynos4_timer_resources(np);
        exynos4_clocksource_init();
        exynos4_clockevent_init();
  }

--

Regards,
Sylwester
_______________________________________________
devicetree-discuss mailing list
[email protected]
https://lists.ozlabs.org/listinfo/devicetree-discuss

Reply via email to