Re: [PATCH 1/8] Documentation: devicetree: Update Exynos MCT bindings description

[Stephen Warren <swarren@wwwdotorg.org>]

On 08/20/2013 07:52 AM, Tomasz Figa wrote:
> This patch updates description of device tree bindings for Exynos MCT
> (multicore timers). Namely:
>  - added note about simplified specification of local timer interrupts,
>    when using single per-processor interrupt for all local timers,
>  - changed first example that was incorrectly suggesting that global
>    timer interrupts are optional,
>  - simplified example interrupt map,
>  - added example showing simplified local timer interrupt specification.

> diff --git a/Documentation/devicetree/bindings/timer/samsung,exynos4210-mct.txt b/Documentation/devicetree/bindings/timer/samsung,exynos4210-mct.txt

> -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.
> +  For MCT block that uses a per-processor interrupt for local timers, such
> +  as ones compatible with "samusng,exynos4412-mct", only one local timer

samsung is typo'd there.

> +Example 2: In this example, the timer interrupts are connected to two separate
> +	   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>;
> +		interrupts = <0>, <1>, <2>, <3>, <4>, <5>;
>  
>  		mct_map: mct-map {
> -			#interrupt-cells = <2>;
> +			#interrupt-cells = <1>;

>  			#address-cells = <0>;
>  			#size-cells = <0>;

I don't believe you need either of those two properties in a node solely
used as an interrupt map.

Also, why not put the interrupt-map property directly into the main mct
node; I don't believe there's any requirement nor advantage to it being
a separate node.

> -			interrupt-map = <0x0 0 &combiner 23 3>,
> -					<0x4 0 &gic 0 120 0>,
> -					<0x5 0 &gic 0 121 0>;
> +			interrupt-map = <0 &gic 0 57 0>,
> +					<1 &gic 0 69 0>,
> +					<2 &combiner 12 6>,
> +					<3 &combiner 12 7>,
> +					<4 &gic 0 42 0>,
> +					<5 &gic 0 48 0>;
>  		};
>  	};

> +Example 3: In this example, the IP contains four local timers, but using
> +	   a per-processor interrupt to handle them. Either all the local
> +	   timer interrupts can be specified, with the same interrupt specifier
> +	   value or just the first one.

That sounds like it should be two separate examples.

Actually, there's already a 2-timer example above using separate
interrupts, so why not make this example *just* be for the
single-interrupt case?

> +	mct@10050000 {
> +		compatible = "samsung,exynos4412-mct";
> +		reg = <0x10050000 0x800>;
> +		interrupts = <0 57 0>, <0 69 0>, <0 70 0>, <0 71 0>,
> +			     <0 42 0>/*, <0 42 0>, <0 42 0>, <0 42 0>*/;
> +	};