From: Stephen Boyd <sboyd@codeaurora.org>
To: Mark Rutland <mark.rutland@arm.com>
Cc: "linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"linux-arm-msm@vger.kernel.org" <linux-arm-msm@vger.kernel.org>,
"devicetree-discuss@lists.ozlabs.org"
<devicetree-discuss@lists.ozlabs.org>,
Marc Zyngier <Marc.Zyngier@arm.com>
Subject: Re: [PATCH 1/4] Documentation: Add memory mapped ARM architected timer binding
Date: Tue, 09 Apr 2013 09:42:38 -0700 [thread overview]
Message-ID: <516444FE.2080200@codeaurora.org> (raw)
In-Reply-To: <20130409090843.GS23725@e106331-lin.cambridge.arm.com>
On 04/09/13 02:08, Mark Rutland wrote:
> On Tue, Apr 09, 2013 at 03:30:20AM +0100, Stephen Boyd wrote:
>>
>> -** Timer node properties:
>> +** CP15 Timer node properties:
>>
>> - compatible : Should at least contain one of
>> "arm,armv7-timer"
>> @@ -26,3 +30,55 @@ Example:
>> <1 10 0xf08>;
>> clock-frequency = <100000000>;
>> };
>> +
>> +** Memory mapped timer node properties
>> +
>> +- compatible : Should at least contain "arm,armv7-timer-mem".
>> +
>> +- #address-cells : Must be 1.
> What about LPAE systems?
>
> How about something like the following:
>
> #address-cells : If the ranges property is empty, the same value as the
> parent node's #address-cells property. Otherwise, a
> value such that the ranges property specifies a
> mapping to the parent node's address space.
Yes that is much better. I wasn't trying to preclude LPAE or 64 bit systems.
>> +
>> +- #size-cells : Must be 1.
>> +
>> +- ranges : Indicates parent and child bus address space are the same.
>> +
> Similarly, what if someone wants to write a more complex mapping for some
> reason?
>
> We should be able to handle it if we use the standard accessors.
Maybe I should just leave this part out? They are standard DT properties
so I could assume DT writers know what to do.
>> +- clock-frequency : The frequency of the main counter, in Hz. Optional.
>> +
>> +- reg : The control frame base address.
>> +
>> +Frame:
>> +
>> +- frame-id: Encoded as follows:
>> + bits[3:0] frame number: 0 to 7.
>> + bits[10:8] frame usage:
>> + 0 - user/kernel
>> + 1 - hyp
>> + 2 - secure
>> +
> Could we not just have a disabled status property for those frames claimed by a
> higher level (either secure firmware or hypervisor)? Or have I missed something
> here?
Using disabled status would work. I was also thinking maybe we should
use a compatible string in each frame's node. Then we could match
against compatible children like "frame-user", "frame-kernel",
"frame-hyp", "frame-secure", "frame-user-kernel", etc. It allows us
flexibility if we should need to add something else in the future.
Also to get the frame number, I was thinking maybe we should expand the
reg property to have two address cells. Then we could have reg = <0
0xf0001000 0x1000>.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
WARNING: multiple messages have this Message-ID (diff)
From: sboyd@codeaurora.org (Stephen Boyd)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/4] Documentation: Add memory mapped ARM architected timer binding
Date: Tue, 09 Apr 2013 09:42:38 -0700 [thread overview]
Message-ID: <516444FE.2080200@codeaurora.org> (raw)
In-Reply-To: <20130409090843.GS23725@e106331-lin.cambridge.arm.com>
On 04/09/13 02:08, Mark Rutland wrote:
> On Tue, Apr 09, 2013 at 03:30:20AM +0100, Stephen Boyd wrote:
>>
>> -** Timer node properties:
>> +** CP15 Timer node properties:
>>
>> - compatible : Should at least contain one of
>> "arm,armv7-timer"
>> @@ -26,3 +30,55 @@ Example:
>> <1 10 0xf08>;
>> clock-frequency = <100000000>;
>> };
>> +
>> +** Memory mapped timer node properties
>> +
>> +- compatible : Should at least contain "arm,armv7-timer-mem".
>> +
>> +- #address-cells : Must be 1.
> What about LPAE systems?
>
> How about something like the following:
>
> #address-cells : If the ranges property is empty, the same value as the
> parent node's #address-cells property. Otherwise, a
> value such that the ranges property specifies a
> mapping to the parent node's address space.
Yes that is much better. I wasn't trying to preclude LPAE or 64 bit systems.
>> +
>> +- #size-cells : Must be 1.
>> +
>> +- ranges : Indicates parent and child bus address space are the same.
>> +
> Similarly, what if someone wants to write a more complex mapping for some
> reason?
>
> We should be able to handle it if we use the standard accessors.
Maybe I should just leave this part out? They are standard DT properties
so I could assume DT writers know what to do.
>> +- clock-frequency : The frequency of the main counter, in Hz. Optional.
>> +
>> +- reg : The control frame base address.
>> +
>> +Frame:
>> +
>> +- frame-id: Encoded as follows:
>> + bits[3:0] frame number: 0 to 7.
>> + bits[10:8] frame usage:
>> + 0 - user/kernel
>> + 1 - hyp
>> + 2 - secure
>> +
> Could we not just have a disabled status property for those frames claimed by a
> higher level (either secure firmware or hypervisor)? Or have I missed something
> here?
Using disabled status would work. I was also thinking maybe we should
use a compatible string in each frame's node. Then we could match
against compatible children like "frame-user", "frame-kernel",
"frame-hyp", "frame-secure", "frame-user-kernel", etc. It allows us
flexibility if we should need to add something else in the future.
Also to get the frame number, I was thinking maybe we should expand the
reg property to have two address cells. Then we could have reg = <0
0xf0001000 0x1000>.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
next prev parent reply other threads:[~2013-04-09 16:42 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-04-09 2:30 [PATCH 0/4] Memory mapped architected timers Stephen Boyd
2013-04-09 2:30 ` Stephen Boyd
2013-04-09 2:30 ` [PATCH 1/4] Documentation: Add memory mapped ARM architected timer binding Stephen Boyd
2013-04-09 2:30 ` Stephen Boyd
2013-04-09 9:08 ` Mark Rutland
2013-04-09 9:08 ` Mark Rutland
2013-04-09 16:42 ` Stephen Boyd [this message]
2013-04-09 16:42 ` Stephen Boyd
2013-04-10 10:13 ` Mark Rutland
2013-04-10 10:13 ` Mark Rutland
2013-04-11 2:52 ` Stephen Boyd
2013-04-11 2:52 ` Stephen Boyd
2013-04-11 11:24 ` Mark Rutland
2013-04-11 11:24 ` Mark Rutland
2013-04-11 21:48 ` Stephen Boyd
2013-04-11 21:48 ` Stephen Boyd
2013-04-09 2:30 ` [PATCH 2/4] ARM: arch_timers: Pass clock event to set_mode callback Stephen Boyd
2013-04-09 2:30 ` Stephen Boyd
2013-04-09 2:30 ` [PATCH 3/4] clocksource: arch_timer: Push the read/write wrappers deeper Stephen Boyd
2013-04-09 2:30 ` Stephen Boyd
2013-04-09 9:38 ` Mark Rutland
2013-04-09 9:38 ` Mark Rutland
2013-04-09 9:38 ` Mark Rutland
2013-04-09 14:49 ` Stephen Boyd
2013-04-09 14:49 ` Stephen Boyd
2013-04-09 2:30 ` [PATCH 4/4] clocksource: arch_timer: Add support for memory mapped timers Stephen Boyd
2013-04-09 2:30 ` Stephen Boyd
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=516444FE.2080200@codeaurora.org \
--to=sboyd@codeaurora.org \
--cc=Marc.Zyngier@arm.com \
--cc=devicetree-discuss@lists.ozlabs.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.