public inbox for linux-arm-kernel@lists.infradead.org 
 help / color / mirror / Atom feed
From: swarren@wwwdotorg•org (Stephen Warren)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH 1/8] Documentation: devicetree: Update Exynos MCT bindings description
Date: Tue, 20 Aug 2013 14:41:15 -0600	[thread overview]
Message-ID: <5213D46B.3060105@wwwdotorg.org> (raw)
In-Reply-To: <1562176.XFHI3umCn7@amdc1227>

On 08/20/2013 11:12 AM, Tomasz Figa wrote:
> On Tuesday 20 of August 2013 11:00:53 Stephen Warren wrote:
>> 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.
> 
> Oops. ;)
> 
>>> +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 at 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.
> 
> Well, you don't need #size-cells, as it is not used for interrupt-map 
> property.
> 
> As for #address-cell property, you need it, as it defines how many cells 
> are used in interrupt map specifier for unit address. See ePAPR 2.4.3.1 or 
> [1] for a description of interrupt-map property format.
> 
> [1] - http://devicetree.org/Device_Tree_Usage#Advanced_Interrupt_Mapping

Uggh. Yes, you're right.

>> 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.
> 
> It is more readable, as you don't mix virtual (helper) properties, with 
> those describing the hardware. Otherwise both ways are technically correct, 
> but not for all cases, i.e. only when #address-cells and #interrupt-cells 
> properties aren't used for device's own purposes.

Hmm. I see the argument.

>>> +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?
> 
> Well, I wanted to show that both ways of specification would be equivalent 
> here. If you insist on making it a single example, then I can send next 
> version with this changed.

Oh! I didn't see the /* */ at all in the third example...

I think it'd be more obvious if you wrote the whole property out twice:

+		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>*/;
+		/* or: */
+		interrupts = <0 57 0>, <0 69 0>, <0 70 0>, <0 71 0>,
+			     <0 42 0>;

  reply	other threads:[~2013-08-20 20:41 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-20 13:52 [PATCH 0/8] Exynos device tree clean-up for 3.12 Tomasz Figa
2013-08-20 13:52 ` [PATCH 1/8] Documentation: devicetree: Update Exynos MCT bindings description Tomasz Figa
2013-08-20 17:00   ` Stephen Warren
2013-08-20 17:12     ` Tomasz Figa
2013-08-20 20:41       ` Stephen Warren [this message]
2013-08-20 22:13         ` Tomasz Figa
2013-08-26 17:21   ` [PATCH v2 " Tomasz Figa
2013-08-20 13:52 ` [PATCH 2/8] ARM: dts: exynos4: Drop interrupt controller properties from MCT nodes Tomasz Figa
2013-08-20 13:52 ` [PATCH 3/8] ARM: dts: exynos4x12: Move MCT node to exynos4x12.dtsi Tomasz Figa
2013-08-20 13:52 ` [PATCH 4/8] ARM: dts: exynos4: Simplify MCT interrupt map Tomasz Figa
2013-08-20 13:52 ` [PATCH 5/8] ARM: dts: exynos4x12: Move mshc node to exynos4x12.dtsi Tomasz Figa
2013-08-20 13:52 ` [PATCH 6/8] ARM: dts: exynos4x12: Keep mshc node disabled by default Tomasz Figa
2013-08-20 13:52 ` [PATCH 7/8] ARM: dts: exynos: Fix missing spaces after labels Tomasz Figa
2013-08-20 13:52 ` [PATCH 8/8] ARM: dts: exynos4: Add missing unit-address of sysreg node Tomasz Figa
2013-08-20 14:20 ` [PATCH 0/8] Exynos device tree clean-up for 3.12 Sylwester Nawrocki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5213D46B.3060105@wwwdotorg.org \
    --to=swarren@wwwdotorg$(echo .)org \
    --cc=linux-arm-kernel@lists$(echo .)infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox