Subject: [PATCH 02/18] arm64: GICv3 device tree binding documentation

From: [email-masked] (Christoffer Dall)
Date: Sun, 16 Feb 2014 17:21:20 -0800

On Wed, Feb 05, 2014 at 01:30:34PM +0000, Marc Zyngier wrote:
> Add the necessary documentation to support GICv3.
> 
> Acked-by: Catalin Marinas <[email-masked]>
> Signed-off-by: Marc Zyngier <[email-masked]>
> ---
>  Documentation/devicetree/bindings/arm/gic-v3.txt | 81 ++++++++++++++++++++++++
>  1 file changed, 81 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/arm/gic-v3.txt
> 
> diff --git a/Documentation/devicetree/bindings/arm/gic-v3.txt b/Documentation/devicetree/bindings/arm/gic-v3.txt
> new file mode 100644
> index 0000000..93852f6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/gic-v3.txt
> @@ -0,0 +1,81 @@
> +* ARM Generic Interrupt Controller, version 3
> +
> +AArch64 SMP cores are often associated with a GICv3, providing private
> +peripheral interrupts (PPI), shared peripheral interrupts (SPI),
> +software generated interrupts (SGI), and locality-specific peripheral
> +Interrupts (LPI).

Super-over-ridiculous-nit: capitalize the definitions, e.g. Private
Peripheral Interrupts (PPI)

> +
> +Main node required properties:
> +
> +- compatible : should at least contain  "arm,gic-v3".
> +- interrupt-controller : Identifies the node as an interrupt controller
> +- #interrupt-cells : Specifies the number of cells needed to encode an
> +  interrupt source. Must be a single cell with a value of at least 3.
> +
> +  The 1st cell is the interrupt type; 0 for SPI interrupts, 1 for PPI
> +  interrupts. Other values are reserved for future use.
> +
> +  The 2nd cell contains the interrupt number for the interrupt type.
> +  SPI interrupts are in the range [0-987]. PPI interrupts are in the
> +  range [0-15].
> +
> +  The 3rd cell is the flags, encoded as follows:
> +	bits[3:0] trigger type and level flags.
> +		1 = edge triggered
> +		2 = edge triggered (deprecated, for compatibility with GICv2)
> +		4 = level triggered
> +		8 = level triggered (deprecated, for compatibility with GICv2)
> +
> +  Cells 4 and beyond are reserved for future use. Where the 1st cell
> +  has a value of 0 or 1, cells 4 and beyond act as padding, and may be
> +  ignored. It is recommended that padding cells have a value of 0.

another-super-nit: Saying "If the 1st cell" or "When the 1st cell"
sounds more clear to me, but I may be wrong.

> +
> +- reg : Specifies base physical address(s) and size of the GIC
> +  registers, in the following order:
> +  - GIC Distributor interface (GICD)
> +  - GIC Redistributors (GICR), one range per redistributor region
> +  - GIC CPU interface (GICC)
> +  - GIC Hypervisor interface (GICH)
> +  - GIC Virtual CPU interface (GICV)
> +
> +  GICC, GICH and GICV are optional.
> +
> +- interrupts : Interrupt source of the VGIC maintenance interrupt.
> +
> +Optional
> +
> +- redistributor-stride : If using padding pages, specifies the stride
> +  of consecutive redistributors. Must be a multiple of 64kB.

How is this going to be used?  What would the size of the padding then
be?  I couldn't find anything about this in the specs.

Or does this mean the total size of the set of contiguous pages for each
of the redistributors?  That seems to make the math in the example below
work.

> +
> +- #redistributor-regions: The number of independent contiguous regions
> +  occupied by the redistributors. Required if more than one such
> +  region is present.
> +
> +Examples:
> +
> +	gic: interrupt-controller at 2cf00000 {
> +		compatible = "arm,gic-v3";
> +		#interrupt-cells = <3>;
> +		interrupt-controller;
> +		reg = <0x0 0x2f000000 0 0x10000>,	// GICD
> +		      <0x0 0x2f100000 0 0x200000>,	// GICR
> +		      <0x0 0x2c000000 0 0x2000>,	// GICC
> +		      <0x0 0x2c010000 0 0x2000>,	// GICH
> +		      <0x0 0x2c020000 0 0x2000>;	// GICV
> +		interrupts = <1 9 4>;
> +	};
> +
> +	gic: interrupt-controller at 2c010000 {
> +		compatible = "arm,gic-v3";
> +		#interrupt-cells = <3>;
> +		interrupt-controller;
> +		redistributor-stride = 0x40000;		// 256kB stride

I raised the point in patch 1 about the stide value being read as a
64-bit value, which would imply that the example should be the
following, I think:

+		redistributor-stride = <0x0 0x40000>;		// 256kB stride

> +		#redistributor-regions = <2>;
> +		reg = <0x0 0x2c010000 0 0x10000>,	// GICD
> +		      <0x0 0x2d000000 0 0x800000>,	// GICR 1: CPUs 0-31
> +		      <0x0 0x2e000000 0 0x800000>;	// GICR 2: CPUs 32-63
> +		      <0x0 0x2c040000 0 0x2000>,	// GICC
> +		      <0x0 0x2c060000 0 0x2000>,	// GICH
> +		      <0x0 0x2c080000 0 0x2000>;	// GICV
> +		interrupts = <1 9 4>;
> +	};
> -- 
> 1.8.3.4
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Otherwise it looks good.