[RFC PATCH 0/3] Yet another GIC OF binding series

[RFC PATCH 0/3] Yet another GIC OF binding series

[Rob Herring]

From: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>

Grant,

Here's yet another patch series for GIC binding and init. 

You keep saying we should have a common DT function scanning for interrupt
controller nodes and calling their initialization functions. But that will
not work until we have dynamic assignment of linux irq numbers. So either
everyone should just stop trying to do DT bindings for GIC/VIC until that
is in place, or we need an interim solution. This is another attempt at
the latter. I reworked gic_of_init intending for it to be the interrupt
controller specific initialization function that DT interrupt controller
scanning code would call. For now, it is just called by the platform code.
The platform initialization looks something like this:

struct of_intc_desc desc;
memset(&desc, 0, sizeof(desc));
desc.controller = of_find_compatible_node(NULL, NULL, "arm,cortex-a9-gic");
gic_of_init(&desc);

I've addressed your previous comments and in particular initializing more
than 1 GIC is supported now.

Rob

Rob Herring (3):
  dt: irq: add interrupt controller descriptor struct
  ARM: gic: allow irq_start to be 0
  ARM: gic: add OF based initialization

 Documentation/devicetree/bindings/arm/gic.txt |   28 +++++++++++++++++++
 arch/arm/common/gic.c                         |   36 ++++++++++++++++++++++++-
 arch/arm/include/asm/hardware/gic.h           |    2 +
 include/linux/of_irq.h                        |    6 ++++
 4 files changed, 71 insertions(+), 1 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/gic.txt

-- 
1.7.4.1

[PATCH v2] ARM: gic: add OF based initialization

[Rob Herring]

From: Rob Herring <rob.herring@calxeda.com>

This adds gic initialization using device tree data. The initialization
functions are intended to be called by a generic OF interrupt
controller parsing function once the right pieces are in place.

PPI binding support based on work by Marc Zyngier.

Signed-off-by: Rob Herring <rob.herring@calxeda.com>
---
Changes in v2:
- Add bindings and documentation for PPI
- change interrupt-cells to 2
- Move init of np pointer to after intc_desc ptr NULL check

 Documentation/devicetree/bindings/arm/gic.txt |   62 +++++++++++++++++++++++++
 arch/arm/common/gic.c                         |   41 ++++++++++++++++
 arch/arm/include/asm/hardware/gic.h           |    3 +
 3 files changed, 106 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/gic.txt

diff --git a/Documentation/devicetree/bindings/arm/gic.txt b/Documentation/devicetree/bindings/arm/gic.txt
new file mode 100644
index 0000000..85be7c9
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/gic.txt
@@ -0,0 +1,62 @@
+* ARM Generic Interrupt Controller
+
+ARM SMP cores are often associated with a GIC, providing per processor
+interrupts (PPI), shared processor interrupts (SPI) and software
+generated interrupts (SGI).
+
+Primary GIC is attached directly to the CPU and typically has PPIs. Secondary GICs are
+cascaded into the upward interrupt controller and do not have PPIs.
+
+Main node required properties:
+
+- compatible : should be one of:
+	"arm,cortex-a9-gic"
+	"arm,arm11mp-gic"
+- interrupt-controller : Identifies the node as an interrupt controller
+- #interrupt-cells : Specifies the number of cells needed to encode an
+  interrupt source.  The type shall be a <u32> and the value shall be 2.
+- reg : Specifies base physical address(s) and size of the GIC registers. The
+  first 2 values are the GIC distributor register base and size. The 2nd 2
+  values are the GIC cpu interface register base and size.
+- #size-cells : Shall be <0> as PPIs don't have size for reg property. Optional
+  for secondary GICs.
+
+Optional
+- interrupts	: optional, used on secondary GICs only
+
+PPI sub-nodes required properties:
+
+- compatible : should match the main node with "-ppi" appended. one of:
+	"arm,cortex-a9-gic-ppi"
+	"arm,arm11mp-gic-ppi"
+- interrupt-controller : Identifies the node as an interrupt controller
+- #interrupt-cells : Specifies the number of cells needed to encode an
+  interrupt source.  The type shall be a <u32> and the value shall be 1 as PPIs
+  don't have edge/level settings.
+- reg : Specifies cpu number the PPIs are attached to.
+
+Example:
+
+	intc: interrupt-controller@fff11000 {
+		compatible = "arm,cortex-a9-gic";
+		#interrupt-cells = <2>;
+		#size-cells = <0>;
+		#address-cells = <1>;
+		interrupt-controller;
+		reg = <0xfff11000 0x1000>,
+		      <0xfff10100 0x100>;
+		      
+		gicppi0: gic-ppi@0 {
+			compatible = "arm,cortex-a9-gic-ppi";
+			#interrupt-cells = <1>;
+			interrupt-controller;
+			reg = <0>;
+		};
+		gicppi1: gic-ppi@1 {
+			compatible = "arm,cortex-a9-gic-ppi";
+			#interrupt-cells = <1>;
+			interrupt-controller;
+			reg = <1>;
+		};
+	};
+
diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
index f13298e..a796299 100644
--- a/arch/arm/common/gic.c
+++ b/arch/arm/common/gic.c
@@ -28,6 +28,10 @@
 #include <linux/smp.h>
 #include <linux/cpumask.h>
 #include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/irqdomain.h>
 
 #include <asm/irq.h>
 #include <asm/mach/irq.h>
@@ -394,3 +398,40 @@ void gic_raise_softirq(const struct cpumask *mask, unsigned int irq)
 	writel_relaxed(map << 16 | irq, gic_data[0].dist_base + GIC_DIST_SOFTINT);
 }
 #endif
+
+#ifdef CONFIG_OF
+static int gic_cnt __initdata = 0;
+
+void __init gic_of_init(struct of_intc_desc *d)
+{
+	struct device_node *np;
+	void __iomem *cpu_base;
+	void __iomem *dist_base;
+	int irq;
+
+	if (WARN_ON(!d || !d->controller))
+		return;
+
+	np = d->controller;
+	dist_base = of_iomap(np, 0);
+	WARN(!dist_base, "unable to map gic dist registers\n");
+
+	cpu_base = of_iomap(np, 1);
+	WARN(!cpu_base, "unable to map gic cpu registers\n");
+
+	gic_init(gic_cnt, d->irq_base, dist_base, cpu_base);
+	irq_domain_add_simple(d->controller, d->irq_base);
+
+	if (d->parent) {
+		irq = irq_of_parse_and_map(np, 0);
+		gic_cascade_irq(gic_cnt, irq);
+	}
+	gic_cnt++;
+}
+
+void __init gic_of_ppi_init(struct of_intc_desc *d)
+{
+	irq_domain_add_simple(d->controller, d->irq_base);
+}
+
+#endif
diff --git a/arch/arm/include/asm/hardware/gic.h b/arch/arm/include/asm/hardware/gic.h
index 435d3f8..a6594d4 100644
--- a/arch/arm/include/asm/hardware/gic.h
+++ b/arch/arm/include/asm/hardware/gic.h
@@ -37,6 +37,9 @@ extern void __iomem *gic_cpu_base_addr;
 extern struct irq_chip gic_arch_extn;
 
 void gic_init(unsigned int, unsigned int, void __iomem *, void __iomem *);
+struct of_intc_desc;
+void gic_of_init(struct of_intc_desc *d);
+void gic_of_ppi_init(struct of_intc_desc *d);
 void gic_secondary_init(unsigned int);
 void gic_cascade_irq(unsigned int gic_nr, unsigned int irq);
 void gic_raise_softirq(const struct cpumask *mask, unsigned int irq);
-- 
1.7.4.1

Re: [PATCH v2] ARM: gic: add OF based initialization

[Rob Herring]

On 08/25/2011 04:49 PM, Rob Herring wrote:
> From: Rob Herring <rob.herring@calxeda.com>
> 
> This adds gic initialization using device tree data. The initialization
> functions are intended to be called by a generic OF interrupt
> controller parsing function once the right pieces are in place.
> 
> PPI binding support based on work by Marc Zyngier.
> 
> Signed-off-by: Rob Herring <rob.herring@calxeda.com>
> ---
> Changes in v2:
> - Add bindings and documentation for PPI
> - change interrupt-cells to 2
> - Move init of np pointer to after intc_desc ptr NULL check
> 
>  Documentation/devicetree/bindings/arm/gic.txt |   62 +++++++++++++++++++++++++
>  arch/arm/common/gic.c                         |   41 ++++++++++++++++
>  arch/arm/include/asm/hardware/gic.h           |    3 +
>  3 files changed, 106 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/arm/gic.txt
> 
> diff --git a/Documentation/devicetree/bindings/arm/gic.txt b/Documentation/devicetree/bindings/arm/gic.txt
> new file mode 100644
> index 0000000..85be7c9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/gic.txt
> @@ -0,0 +1,62 @@
> +* ARM Generic Interrupt Controller
> +
> +ARM SMP cores are often associated with a GIC, providing per processor
> +interrupts (PPI), shared processor interrupts (SPI) and software
> +generated interrupts (SGI).
> +
> +Primary GIC is attached directly to the CPU and typically has PPIs. Secondary GICs are
> +cascaded into the upward interrupt controller and do not have PPIs.
> +
> +Main node required properties:
> +
> +- compatible : should be one of:
> +	"arm,cortex-a9-gic"
> +	"arm,arm11mp-gic"
> +- interrupt-controller : Identifies the node as an interrupt controller
> +- #interrupt-cells : Specifies the number of cells needed to encode an
> +  interrupt source.  The type shall be a <u32> and the value shall be 2.

I need to add the following level sense info. I'm using the same values
as IRQF_TRIGGER_* as suggested by Grant.

 The 2nd cell contains the level-sense information, encoded as follows:
                    1 = low-to-high edge triggered
                    2 = high-to-low edge triggered
                    4 = active high level-sensitive
                    8 = active low level-sensitive

> +- reg : Specifies base physical address(s) and size of the GIC registers. The
> +  first 2 values are the GIC distributor register base and size. The 2nd 2
> +  values are the GIC cpu interface register base and size.
> +- #size-cells : Shall be <0> as PPIs don't have size for reg property. Optional
> +  for secondary GICs.
> +
> +Optional
> +- interrupts	: optional, used on secondary GICs only
> +
> +PPI sub-nodes required properties:
> +
> +- compatible : should match the main node with "-ppi" appended. one of:
> +	"arm,cortex-a9-gic-ppi"
> +	"arm,arm11mp-gic-ppi"
> +- interrupt-controller : Identifies the node as an interrupt controller
> +- #interrupt-cells : Specifies the number of cells needed to encode an
> +  interrupt source.  The type shall be a <u32> and the value shall be 1 as PPIs
> +  don't have edge/level settings.
> +- reg : Specifies cpu number the PPIs are attached to.
> +
> +Example:
> +
> +	intc: interrupt-controller@fff11000 {
> +		compatible = "arm,cortex-a9-gic";
> +		#interrupt-cells = <2>;
> +		#size-cells = <0>;
> +		#address-cells = <1>;
> +		interrupt-controller;
> +		reg = <0xfff11000 0x1000>,
> +		      <0xfff10100 0x100>;
> +		      
> +		gicppi0: gic-ppi@0 {
> +			compatible = "arm,cortex-a9-gic-ppi";
> +			#interrupt-cells = <1>;
> +			interrupt-controller;
> +			reg = <0>;
> +		};
> +		gicppi1: gic-ppi@1 {
> +			compatible = "arm,cortex-a9-gic-ppi";
> +			#interrupt-cells = <1>;
> +			interrupt-controller;
> +			reg = <1>;
> +		};
> +	};
> +
> diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
> index f13298e..a796299 100644
> --- a/arch/arm/common/gic.c
> +++ b/arch/arm/common/gic.c
> @@ -28,6 +28,10 @@
>  #include <linux/smp.h>
>  #include <linux/cpumask.h>
>  #include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/irqdomain.h>
>  
>  #include <asm/irq.h>
>  #include <asm/mach/irq.h>
> @@ -394,3 +398,40 @@ void gic_raise_softirq(const struct cpumask *mask, unsigned int irq)
>  	writel_relaxed(map << 16 | irq, gic_data[0].dist_base + GIC_DIST_SOFTINT);
>  }
>  #endif
> +
> +#ifdef CONFIG_OF
> +static int gic_cnt __initdata = 0;
> +
> +void __init gic_of_init(struct of_intc_desc *d)
> +{
> +	struct device_node *np;
> +	void __iomem *cpu_base;
> +	void __iomem *dist_base;
> +	int irq;
> +
> +	if (WARN_ON(!d || !d->controller))
> +		return;
> +
> +	np = d->controller;
> +	dist_base = of_iomap(np, 0);
> +	WARN(!dist_base, "unable to map gic dist registers\n");
> +
> +	cpu_base = of_iomap(np, 1);
> +	WARN(!cpu_base, "unable to map gic cpu registers\n");
> +
> +	gic_init(gic_cnt, d->irq_base, dist_base, cpu_base);
> +	irq_domain_add_simple(d->controller, d->irq_base);
> +
> +	if (d->parent) {
> +		irq = irq_of_parse_and_map(np, 0);
> +		gic_cascade_irq(gic_cnt, irq);
> +	}
> +	gic_cnt++;
> +}
> +
> +void __init gic_of_ppi_init(struct of_intc_desc *d)
> +{
> +	irq_domain_add_simple(d->controller, d->irq_base);
> +}
> +
> +#endif
> diff --git a/arch/arm/include/asm/hardware/gic.h b/arch/arm/include/asm/hardware/gic.h
> index 435d3f8..a6594d4 100644
> --- a/arch/arm/include/asm/hardware/gic.h
> +++ b/arch/arm/include/asm/hardware/gic.h
> @@ -37,6 +37,9 @@ extern void __iomem *gic_cpu_base_addr;
>  extern struct irq_chip gic_arch_extn;
>  
>  void gic_init(unsigned int, unsigned int, void __iomem *, void __iomem *);
> +struct of_intc_desc;
> +void gic_of_init(struct of_intc_desc *d);
> +void gic_of_ppi_init(struct of_intc_desc *d);
>  void gic_secondary_init(unsigned int);
>  void gic_cascade_irq(unsigned int gic_nr, unsigned int irq);
>  void gic_raise_softirq(const struct cpumask *mask, unsigned int irq);

Re: [RFC PATCH 0/3] Yet another GIC OF binding series

[Shawn Guo]

On Tue, Aug 09, 2011 at 03:16:57PM -0500, Rob Herring wrote:
> From: Rob Herring <rob.herring@calxeda.com>
> 
> Grant,
> 
> Here's yet another patch series for GIC binding and init. 
> 
> You keep saying we should have a common DT function scanning for interrupt
> controller nodes and calling their initialization functions. But that will
> not work until we have dynamic assignment of linux irq numbers. So either
> everyone should just stop trying to do DT bindings for GIC/VIC until that
> is in place, or we need an interim solution.

Yes, we need a way out.  I based my i.MX6Q series on this patch set.
With the v2 of "ARM: gic: add OF based initialization" in,

Acked-by: Shawn Guo <shawn.guo@linaro.org>
Tested-by: Shawn Guo <shawn.guo@linaro.org>

Regards,
Shawn

> This is another attempt at
> the latter. I reworked gic_of_init intending for it to be the interrupt
> controller specific initialization function that DT interrupt controller
> scanning code would call. For now, it is just called by the platform code.
> The platform initialization looks something like this:
> 
> struct of_intc_desc desc;
> memset(&desc, 0, sizeof(desc));
> desc.controller = of_find_compatible_node(NULL, NULL, "arm,cortex-a9-gic");
> gic_of_init(&desc);
> 
> I've addressed your previous comments and in particular initializing more
> than 1 GIC is supported now.
> 
> Rob
> 
> Rob Herring (3):
>   dt: irq: add interrupt controller descriptor struct
>   ARM: gic: allow irq_start to be 0
>   ARM: gic: add OF based initialization
> 
>  Documentation/devicetree/bindings/arm/gic.txt |   28 +++++++++++++++++++
>  arch/arm/common/gic.c                         |   36 ++++++++++++++++++++++++-
>  arch/arm/include/asm/hardware/gic.h           |    2 +
>  include/linux/of_irq.h                        |    6 ++++
>  4 files changed, 71 insertions(+), 1 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/arm/gic.txt
> 
> -- 
> 1.7.4.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

[RFC PATCH 1/3] dt: irq: add interrupt controller descriptor struct

[Rob Herring]

From: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>

In preparation to scan and initialize interrupt controllers from a
device-tree, create struct to pass to interrupt controller initialization
functions.

irq_base should go away with dynamic linux irq assignment.

Signed-off-by: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
---
 include/linux/of_irq.h |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/include/linux/of_irq.h b/include/linux/of_irq.h
index cd2e61c..de91ff9 100644
--- a/include/linux/of_irq.h
+++ b/include/linux/of_irq.h
@@ -33,6 +33,12 @@ struct of_irq {
 	u32 specifier[OF_MAX_IRQ_SPEC]; /* Specifier copy */
 };
 
+struct of_intc_desc {
+	struct device_node *controller;
+	struct device_node *parent;
+	int irq_base;
+};
+
 /*
  * Workarounds only applied to 32bit powermac machines
  */
-- 
1.7.4.1

Re: [RFC PATCH 1/3] dt: irq: add interrupt controller descriptor struct

[Jamie Iles]

On Tue, Aug 09, 2011 at 03:16:58PM -0500, Rob Herring wrote:
> From: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
> 
> In preparation to scan and initialize interrupt controllers from a
> device-tree, create struct to pass to interrupt controller initialization
> functions.
> 
> irq_base should go away with dynamic linux irq assignment.
> 
> Signed-off-by: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
> ---
>  include/linux/of_irq.h |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/of_irq.h b/include/linux/of_irq.h
> index cd2e61c..de91ff9 100644
> --- a/include/linux/of_irq.h
> +++ b/include/linux/of_irq.h
> @@ -33,6 +33,12 @@ struct of_irq {
>  	u32 specifier[OF_MAX_IRQ_SPEC]; /* Specifier copy */
>  };
>  
> +struct of_intc_desc {
> +	struct device_node *controller;
> +	struct device_node *parent;

Do we need parent here?  I think that of_irq_find_parent() will do the 
job.

Other than that nit the series looks nice to me.

Jamie

Re: [RFC PATCH 1/3] dt: irq: add interrupt controller descriptor struct

[Rob Herring]

On 08/10/2011 08:14 AM, Jamie Iles wrote:
> On Tue, Aug 09, 2011 at 03:16:58PM -0500, Rob Herring wrote:
>> From: Rob Herring <rob.herring@calxeda.com>
>>
>> In preparation to scan and initialize interrupt controllers from a
>> device-tree, create struct to pass to interrupt controller initialization
>> functions.
>>
>> irq_base should go away with dynamic linux irq assignment.
>>
>> Signed-off-by: Rob Herring <rob.herring@calxeda.com>
>> ---
>>  include/linux/of_irq.h |    6 ++++++
>>  1 files changed, 6 insertions(+), 0 deletions(-)
>>
>> diff --git a/include/linux/of_irq.h b/include/linux/of_irq.h
>> index cd2e61c..de91ff9 100644
>> --- a/include/linux/of_irq.h
>> +++ b/include/linux/of_irq.h
>> @@ -33,6 +33,12 @@ struct of_irq {
>>  	u32 specifier[OF_MAX_IRQ_SPEC]; /* Specifier copy */
>>  };
>>  
>> +struct of_intc_desc {
>> +	struct device_node *controller;
>> +	struct device_node *parent;
> 
> Do we need parent here?  I think that of_irq_find_parent() will do the 
> job.
> 

With interrupt controller scanning code, it will avoid calling
of_irq_find_parent twice. That code will have to maintain a list of
nodes and parents anyway in order to initialize controllers with no
parent first.

Rob

> Other than that nit the series looks nice to me.
> 
> Jamie

[RFC PATCH 2/3] ARM: gic: allow irq_start to be 0

[Rob Herring]

From: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>

There's really no need to set irq_start per platform for the primary gic.
The SGIs and PPIs are not handled as normal irqs, so how irqs 0-31 are
setup doesn't really matter. So allow irq_start to be set to 0 to match
the linux irq numbering.

Signed-off-by: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
---
 arch/arm/common/gic.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
index 3227ca9..f13298e 100644
--- a/arch/arm/common/gic.c
+++ b/arch/arm/common/gic.c
@@ -353,7 +353,7 @@ void __init gic_init(unsigned int gic_nr, unsigned int irq_start,
 	gic = &gic_data[gic_nr];
 	gic->dist_base = dist_base;
 	gic->cpu_base = cpu_base;
-	gic->irq_offset = (irq_start - 1) & ~31;
+	gic->irq_offset = irq_start ? (irq_start - 1) & ~31 : 0;
 
 	if (gic_nr == 0)
 		gic_cpu_base_addr = cpu_base;
-- 
1.7.4.1

[RFC PATCH 3/3] ARM: gic: add OF based initialization

[Rob Herring]

From: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>

This adds gic initialization using device tree data. An example device tree
binding looks like this:

intc: interrupt-controller@fff11000 {
        compatible = "arm,cortex-a9-gic";
        #interrupt-cells = <1>;
        interrupt-controller;
        reg = <0xfff11000 0x1000>,
              <0xfff10100 0x100>;
};

Signed-off-by: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
---
 Documentation/devicetree/bindings/arm/gic.txt |   28 ++++++++++++++++++++
 arch/arm/common/gic.c                         |   34 +++++++++++++++++++++++++
 arch/arm/include/asm/hardware/gic.h           |    2 +
 3 files changed, 64 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/gic.txt

diff --git a/Documentation/devicetree/bindings/arm/gic.txt b/Documentation/devicetree/bindings/arm/gic.txt
new file mode 100644
index 0000000..78012e3
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/gic.txt
@@ -0,0 +1,28 @@
+* ARM Generic Interrupt Controller
+
+Some ARM cores have an interrupt controller called GIC. The ARM GIC
+representation in the device tree should be done as under:-
+
+Required properties:
+
+- compatible : should be one of:
+	"arm,cortex-a9-gic"
+	"arm,arm11mp-gic"
+- interrupt-controller : Identifies the node as an interrupt controller
+- #interrupt-cells : Specifies the number of cells needed to encode an
+  interrupt source.  The type shall be a <u32> and the value shall be 1.
+- reg : Specifies base physical address(s) and size of the GIC registers. The
+  first 2 values are the GIC distributor register base and size. The 2nd 2
+  values are the GIC cpu interface register base and size.
+
+Example:
+
+intc: interrupt-controller@fff11000 {
+        compatible = "arm,cortex-a9-gic";
+        #interrupt-cells = <1>;
+        interrupt-controller;
+        reg = <0xfff11000 0x1000>,
+              <0xfff10100 0x100>;
+};
+
+
diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
index f13298e..0626bb7 100644
--- a/arch/arm/common/gic.c
+++ b/arch/arm/common/gic.c
@@ -28,6 +28,10 @@
 #include <linux/smp.h>
 #include <linux/cpumask.h>
 #include <linux/io.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/irqdomain.h>
 
 #include <asm/irq.h>
 #include <asm/mach/irq.h>
@@ -394,3 +398,33 @@ void gic_raise_softirq(const struct cpumask *mask, unsigned int irq)
 	writel_relaxed(map << 16 | irq, gic_data[0].dist_base + GIC_DIST_SOFTINT);
 }
 #endif
+
+#ifdef CONFIG_OF
+static int gic_cnt __initdata = 0;
+
+void __init gic_of_init(struct of_intc_desc *d)
+{
+	struct device_node *np = d->controller;
+	void __iomem *cpu_base;
+	void __iomem *dist_base;
+	int irq;
+
+	if (WARN_ON(!d || !d->controller))
+		return;
+
+	dist_base = of_iomap(np, 0);
+	WARN(!dist_base, "unable to map gic dist registers\n");
+
+	cpu_base = of_iomap(np, 1);
+	WARN(!cpu_base, "unable to map gic cpu registers\n");
+
+	gic_init(gic_cnt, d->irq_base, dist_base, cpu_base);
+	irq_domain_add_simple(d->controller, d->irq_base);
+
+	if (d->parent) {
+		irq = irq_of_parse_and_map(np, 0);
+		gic_cascade_irq(gic_cnt, irq);
+	}
+	gic_cnt++;
+}
+#endif
diff --git a/arch/arm/include/asm/hardware/gic.h b/arch/arm/include/asm/hardware/gic.h
index 435d3f8..64ef90d 100644
--- a/arch/arm/include/asm/hardware/gic.h
+++ b/arch/arm/include/asm/hardware/gic.h
@@ -37,6 +37,8 @@ extern void __iomem *gic_cpu_base_addr;
 extern struct irq_chip gic_arch_extn;
 
 void gic_init(unsigned int, unsigned int, void __iomem *, void __iomem *);
+struct of_intc_desc;
+void gic_of_init(struct of_intc_desc *d);
 void gic_secondary_init(unsigned int);
 void gic_cascade_irq(unsigned int gic_nr, unsigned int irq);
 void gic_raise_softirq(const struct cpumask *mask, unsigned int irq);
-- 
1.7.4.1

Re: [RFC PATCH 3/3] ARM: gic: add OF based initialization

[Marc Zyngier]

On 09/08/11 21:17, Rob Herring wrote:
> From: Rob Herring <rob.herring@calxeda.com>
> 
> This adds gic initialization using device tree data. An example device tree
> binding looks like this:
> 
> intc: interrupt-controller@fff11000 {
>         compatible = "arm,cortex-a9-gic";
>         #interrupt-cells = <1>;
>         interrupt-controller;
>         reg = <0xfff11000 0x1000>,
>               <0xfff10100 0x100>;
> };

I'm afraid I still object to this. PPIs are an important part of the
GIC, and this binding totally ignores the per-cpu aspect.

How do you represent the connection between a CPU local timer and the
GIC? Even worse, how to represent a device connected to only *one* of
the CPUs? PPIs are difficult to represent on the Linux side. But we
shouldn't ignore them in the DT binding.

Cheers,

	M.
-- 
Jazz is not dead. It just smells funny...

Re: [RFC PATCH 3/3] ARM: gic: add OF based initialization

[Rob Herring]

On 08/10/2011 03:08 AM, Marc Zyngier wrote:
> On 09/08/11 21:17, Rob Herring wrote:
>> From: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
>>
>> This adds gic initialization using device tree data. An example device tree
>> binding looks like this:
>>
>> intc: interrupt-controller@fff11000 {
>>         compatible = "arm,cortex-a9-gic";
>>         #interrupt-cells = <1>;
>>         interrupt-controller;
>>         reg = <0xfff11000 0x1000>,
>>               <0xfff10100 0x100>;
>> };
> 
> I'm afraid I still object to this. PPIs are an important part of the
> GIC, and this binding totally ignores the per-cpu aspect.

BTW, we've already got 2 flavors of gic bindings in the dts files.

> How do you represent the connection between a CPU local timer and the
> GIC? Even worse, how to represent a device connected to only *one* of
> the CPUs? PPIs are difficult to represent on the Linux side. But we
> shouldn't ignore them in the DT binding.
> 

Do you have an updated gic binding patchset based on your latest PPI
patch series?

This problem also extends to the TWD itself. How do we represent that?
Is it 1 node per core or 1 node total. Then is timer and watchdog in 1
node, or is it split to separate nodes?

Rob

Re: [RFC PATCH 0/3] Yet another GIC OF binding series

[Thomas Abraham]

On 10 August 2011 01:46, Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> From: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
>
> Grant,
>
> Here's yet another patch series for GIC binding and init.
>
> You keep saying we should have a common DT function scanning for interrupt
> controller nodes and calling their initialization functions. But that will
> not work until we have dynamic assignment of linux irq numbers. So either
> everyone should just stop trying to do DT bindings for GIC/VIC until that
> is in place, or we need an interim solution. This is another attempt at
> the latter. I reworked gic_of_init intending for it to be the interrupt
> controller specific initialization function that DT interrupt controller
> scanning code would call. For now, it is just called by the platform code.
> The platform initialization looks something like this:
>
> struct of_intc_desc desc;
> memset(&desc, 0, sizeof(desc));
> desc.controller = of_find_compatible_node(NULL, NULL, "arm,cortex-a9-gic");
> gic_of_init(&desc);
>
> I've addressed your previous comments and in particular initializing more
> than 1 GIC is supported now.

Works fine for Exynos4.

Tested-by: Thomas Abraham <thomas.abraham-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

>
> Rob
>
> Rob Herring (3):
>  dt: irq: add interrupt controller descriptor struct
>  ARM: gic: allow irq_start to be 0
>  ARM: gic: add OF based initialization
>
>  Documentation/devicetree/bindings/arm/gic.txt |   28 +++++++++++++++++++
>  arch/arm/common/gic.c                         |   36 ++++++++++++++++++++++++-
>  arch/arm/include/asm/hardware/gic.h           |    2 +
>  include/linux/of_irq.h                        |    6 ++++
>  4 files changed, 71 insertions(+), 1 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/arm/gic.txt
>
> --
> 1.7.4.1
>
>