[PATCHv2 0/4] Memory mapped architected timers

[PATCHv2 0/4] Memory mapped architected timers

[Stephen Boyd]

This patchset adds support for memory mapped architected timers. We
don't have any other global broadcast timer in our system, so we use the
mmio timer during low power modes. The first patch is the binding.
The next two patches lay some groundwork so that the last patch is simpler.
The final patch adds support for mmio timers.

Patches are based on a recent patch from Mark that removes the
physical count reading (clocksource: arch_timer: use virtual counter,
message id <1364404312-4427-4-git-send-email-mark.rutland@arm.com>).

Updates since v1:
 * Assigned counter reading function and commented why for arm64
 * Updated DT binding to replace frame-id with frame-number and use status
   property

Stephen Boyd (4):
  Documentation: Add memory mapped ARM architected timer binding
  ARM: arch_timers: Pass clock event to set_mode callback
  clocksource: arch_timer: Push the read/write wrappers deeper
  clocksource: arch_timer: Add support for memory mapped timers

 .../devicetree/bindings/arm/arch_timer.txt         |  59 ++-
 arch/arm/include/asm/arch_timer.h                  |   5 +-
 arch/arm64/include/asm/arch_timer.h                |   4 +-
 drivers/clocksource/arm_arch_timer.c               | 455 +++++++++++++++++----
 include/clocksource/arm_arch_timer.h               |   4 +-
 5 files changed, 449 insertions(+), 78 deletions(-)

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

[PATCHv2 1/4] Documentation: Add memory mapped ARM architected timer binding

[Stephen Boyd]

Add a binding for the arm architected timer hardware's memory
mapped interface. The mmio timer hardware is made up of one base
frame and a collection of up to 8 timer frames, where each of the
8 timer frames can have either one or two views. A frame
typically maps to a privilege level (user/kernel, hypervisor,
secure). The first view has full access to the registers within a
frame, while the second view can be restricted to particular
registers within a frame. Each frame must support a physical
timer. It's optional for a frame to support a virtual timer.

Cc: devicetree-discuss at lists.ozlabs.org
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Marc Zyngier <Marc.Zyngier@arm.com>
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 .../devicetree/bindings/arm/arch_timer.txt         | 59 ++++++++++++++++++++--
 1 file changed, 56 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/arm/arch_timer.txt b/Documentation/devicetree/bindings/arm/arch_timer.txt
index 20746e5..ac20cde 100644
--- a/Documentation/devicetree/bindings/arm/arch_timer.txt
+++ b/Documentation/devicetree/bindings/arm/arch_timer.txt
@@ -1,10 +1,14 @@
 * ARM architected timer
 
-ARM cores may have a per-core architected timer, which provides per-cpu timers.
+ARM cores may have a per-core architected timer, which provides per-cpu timers,
+or a memory mapped architected timer, which provides up to 8 frames with a
+physical and optional virtual timer per frame.
 
-The timer is attached to a GIC to deliver its per-processor interrupts.
+The per-core architected timer is attached to a GIC to deliver its
+per-processor interrupts via PPIs. The memory mapped timer is attached to a GIC
+to deliver its interrupts via SPIs.
 
-** Timer node properties:
+** CP15 Timer node properties:
 
 - compatible : Should at least contain one of
 	"arm,armv7-timer"
@@ -26,3 +30,52 @@ Example:
 			     <1 10 0xf08>;
 		clock-frequency = <100000000>;
 	};
+
+** Memory mapped timer node properties
+
+- compatible : Should at least contain "arm,armv7-timer-mem".
+
+- clock-frequency : The frequency of the main counter, in Hz. Optional.
+
+- reg : The control frame base address.
+
+Note that #address-cells, #size-cells, and ranges shall be present to ensure
+the CPU can address a frame's registers.
+
+Frame:
+
+- frame-number: 0 to 7.
+
+- interrupts : Interrupt list for physical and virtual timers in that order.
+  The virtual timer interrupt is optional.
+
+- reg : The first and second view base addresses in that order. The second view
+  base address is optional.
+
+- status : "disabled" indicates the frame is not available for use.
+
+Example:
+
+	timer at f0000000 {
+		compatible = "arm,armv7-timer-mem";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges;
+		reg = <0xf0000000 0x1000>;
+		clock-frequency = <50000000>;
+
+		frame at f0001000 {
+			frame-number = <0>
+			interrupts = <0 13 0x8>,
+				     <0 14 0x8>;
+			reg = <0xf0001000 0x1000>,
+			      <0xf0002000 0x1000>;
+		};
+
+		frame at f0003000 {
+			frame-number = <1>
+			interrupts = <0 15 0x8>;
+			reg = <0xf0003000 0x1000>;
+			status = "disabled";
+		};
+	};
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

[PATCHv2 1/4] Documentation: Add memory mapped ARM architected timer binding

[Rob Herring]

On Fri, Apr 12, 2013 at 7:27 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> Add a binding for the arm architected timer hardware's memory
> mapped interface. The mmio timer hardware is made up of one base
> frame and a collection of up to 8 timer frames, where each of the
> 8 timer frames can have either one or two views. A frame
> typically maps to a privilege level (user/kernel, hypervisor,
> secure). The first view has full access to the registers within a
> frame, while the second view can be restricted to particular
> registers within a frame. Each frame must support a physical
> timer. It's optional for a frame to support a virtual timer.
>
> Cc: devicetree-discuss at lists.ozlabs.org
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Marc Zyngier <Marc.Zyngier@arm.com>
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
>  .../devicetree/bindings/arm/arch_timer.txt         | 59 ++++++++++++++++++++--
>  1 file changed, 56 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/arm/arch_timer.txt b/Documentation/devicetree/bindings/arm/arch_timer.txt
> index 20746e5..ac20cde 100644
> --- a/Documentation/devicetree/bindings/arm/arch_timer.txt
> +++ b/Documentation/devicetree/bindings/arm/arch_timer.txt
> @@ -1,10 +1,14 @@
>  * ARM architected timer
>
> -ARM cores may have a per-core architected timer, which provides per-cpu timers.
> +ARM cores may have a per-core architected timer, which provides per-cpu timers,
> +or a memory mapped architected timer, which provides up to 8 frames with a
> +physical and optional virtual timer per frame.
>
> -The timer is attached to a GIC to deliver its per-processor interrupts.
> +The per-core architected timer is attached to a GIC to deliver its
> +per-processor interrupts via PPIs. The memory mapped timer is attached to a GIC
> +to deliver its interrupts via SPIs.
>
> -** Timer node properties:
> +** CP15 Timer node properties:
>
>  - compatible : Should at least contain one of
>         "arm,armv7-timer"
> @@ -26,3 +30,52 @@ Example:
>                              <1 10 0xf08>;
>                 clock-frequency = <100000000>;
>         };
> +
> +** Memory mapped timer node properties
> +
> +- compatible : Should at least contain "arm,armv7-timer-mem".

Everything about this timer is architecturally defined? If not, let's
use a more specific name.

> +
> +- clock-frequency : The frequency of the main counter, in Hz. Optional.
> +
> +- reg : The control frame base address.
> +
> +Note that #address-cells, #size-cells, and ranges shall be present to ensure
> +the CPU can address a frame's registers.
> +
> +Frame:
> +
> +- frame-number: 0 to 7.

I'd really like to get rid of the frame numbers and sub-nodes. Is the
frame number significant to software?

> +- interrupts : Interrupt list for physical and virtual timers in that order.
> +  The virtual timer interrupt is optional.

Is that optional per frame?

Rob

> +
> +- reg : The first and second view base addresses in that order. The second view
> +  base address is optional.
> +
> +- status : "disabled" indicates the frame is not available for use.
> +
> +Example:
> +
> +       timer at f0000000 {
> +               compatible = "arm,armv7-timer-mem";
> +               #address-cells = <1>;
> +               #size-cells = <1>;
> +               ranges;
> +               reg = <0xf0000000 0x1000>;
> +               clock-frequency = <50000000>;
> +
> +               frame at f0001000 {
> +                       frame-number = <0>
> +                       interrupts = <0 13 0x8>,
> +                                    <0 14 0x8>;
> +                       reg = <0xf0001000 0x1000>,
> +                             <0xf0002000 0x1000>;
> +               };
> +
> +               frame at f0003000 {
> +                       frame-number = <1>
> +                       interrupts = <0 15 0x8>;
> +                       reg = <0xf0003000 0x1000>;
> +                       status = "disabled";
> +               };
> +       };
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> hosted by The Linux Foundation
>
> _______________________________________________
> devicetree-discuss mailing list
> devicetree-discuss at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss

[PATCHv2 1/4] Documentation: Add memory mapped ARM architected timer binding

[Stephen Boyd]

On 04/15/13 14:20, Rob Herring wrote:
> On Fri, Apr 12, 2013 at 7:27 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
>> @@ -26,3 +30,52 @@ Example:
>>                              <1 10 0xf08>;
>>                 clock-frequency = <100000000>;
>>         };
>> +
>> +** Memory mapped timer node properties
>> +
>> +- compatible : Should at least contain "arm,armv7-timer-mem".
> Everything about this timer is architecturally defined? If not, let's
> use a more specific name.

I'm not sure I'm following you, but everything described here is part of
the ARM definition. What would be a more specific name?

>
>> +
>> +- clock-frequency : The frequency of the main counter, in Hz. Optional.
>> +
>> +- reg : The control frame base address.
>> +
>> +Note that #address-cells, #size-cells, and ranges shall be present to ensure
>> +the CPU can address a frame's registers.
>> +
>> +Frame:
>> +
>> +- frame-number: 0 to 7.
> I'd really like to get rid of the frame numbers and sub-nodes. Is the
> frame number significant to software?

We need the frame number to read and write registers in the control
frame (the first base in the parent node). We currently use it to
determine if a frame has support for the virtual timer by reading the
CNTTIDR (a register with 4 bits per frame describing capabilities). If
we wanted to control access to the second view of a frame we would also
need to configure the CNTPL0ACRn register that pertains to the frame
we're controlling. Without a frame number we wouldn't know which
register to write.

>
>> +- interrupts : Interrupt list for physical and virtual timers in that order.
>> +  The virtual timer interrupt is optional.
> Is that optional per frame?

Yes the virtual and physical timer interrupt is per-frame and the
virtual interrupt is optional.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

[PATCHv2 1/4] Documentation: Add memory mapped ARM architected timer binding

[Stephen Boyd]

Rob,

Can I get your ack on this binding or do you think we need to change
something?

Thanks,
Stephen

On 04/15/13 14:33, Stephen Boyd wrote:
> On 04/15/13 14:20, Rob Herring wrote:
>> On Fri, Apr 12, 2013 at 7:27 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
>>> @@ -26,3 +30,52 @@ Example:
>>>                              <1 10 0xf08>;
>>>                 clock-frequency = <100000000>;
>>>         };
>>> +
>>> +** Memory mapped timer node properties
>>> +
>>> +- compatible : Should at least contain "arm,armv7-timer-mem".
>> Everything about this timer is architecturally defined? If not, let's
>> use a more specific name.
> I'm not sure I'm following you, but everything described here is part of
> the ARM definition. What would be a more specific name?
>
>>> +
>>> +- clock-frequency : The frequency of the main counter, in Hz. Optional.
>>> +
>>> +- reg : The control frame base address.
>>> +
>>> +Note that #address-cells, #size-cells, and ranges shall be present to ensure
>>> +the CPU can address a frame's registers.
>>> +
>>> +Frame:
>>> +
>>> +- frame-number: 0 to 7.
>> I'd really like to get rid of the frame numbers and sub-nodes. Is the
>> frame number significant to software?
> We need the frame number to read and write registers in the control
> frame (the first base in the parent node). We currently use it to
> determine if a frame has support for the virtual timer by reading the
> CNTTIDR (a register with 4 bits per frame describing capabilities). If
> we wanted to control access to the second view of a frame we would also
> need to configure the CNTPL0ACRn register that pertains to the frame
> we're controlling. Without a frame number we wouldn't know which
> register to write.
>
>>> +- interrupts : Interrupt list for physical and virtual timers in that order.
>>> +  The virtual timer interrupt is optional.
>> Is that optional per frame?
> Yes the virtual and physical timer interrupt is per-frame and the
> virtual interrupt is optional.
>


-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

[PATCHv2 1/4] Documentation: Add memory mapped ARM architected timer binding

[Rob Herring]

On 04/15/2013 04:33 PM, Stephen Boyd wrote:
> On 04/15/13 14:20, Rob Herring wrote:
>> On Fri, Apr 12, 2013 at 7:27 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
>>> @@ -26,3 +30,52 @@ Example:
>>>                              <1 10 0xf08>;
>>>                 clock-frequency = <100000000>;
>>>         };
>>> +
>>> +** Memory mapped timer node properties
>>> +
>>> +- compatible : Should at least contain "arm,armv7-timer-mem".
>> Everything about this timer is architecturally defined? If not, let's
>> use a more specific name.
> 
> I'm not sure I'm following you, but everything described here is part of
> the ARM definition. What would be a more specific name?

Something that corresponds to the particular implementation like
cortex-a15 (obviously not an example that has this). I'm fine with
leaving this for now, but would like to see that added when specific SOC
dts are added. Better to be specific in case we need to use it for
something like errata work-arounds. Of course we haven't done that so
far with the arch timer bindings...

>>> +
>>> +- clock-frequency : The frequency of the main counter, in Hz. Optional.
>>> +
>>> +- reg : The control frame base address.
>>> +
>>> +Note that #address-cells, #size-cells, and ranges shall be present to ensure
>>> +the CPU can address a frame's registers.
>>> +
>>> +Frame:
>>> +
>>> +- frame-number: 0 to 7.
>> I'd really like to get rid of the frame numbers and sub-nodes. Is the
>> frame number significant to software?
> 
> We need the frame number to read and write registers in the control
> frame (the first base in the parent node). We currently use it to
> determine if a frame has support for the virtual timer by reading the
> CNTTIDR (a register with 4 bits per frame describing capabilities). If
> we wanted to control access to the second view of a frame we would also
> need to configure the CNTPL0ACRn register that pertains to the frame
> we're controlling. Without a frame number we wouldn't know which
> register to write.

I've gone thru the memory mapped part of the spec now, so I think I
understand things better. I see how the frame number is needed, but...

The control base is only accessible in secure or hyp mode. How does a
guest know that it is a guest and can't map the control base? Seems like
we need to allow a subset of the binding that is just a reg and
interrupts properties without the frame sub nodes.

Rob

> 
>>
>>> +- interrupts : Interrupt list for physical and virtual timers in that order.
>>> +  The virtual timer interrupt is optional.
>> Is that optional per frame?
> 
> Yes the virtual and physical timer interrupt is per-frame and the
> virtual interrupt is optional.
> 

[PATCHv2 1/4] Documentation: Add memory mapped ARM architected timer binding

[Stephen Boyd]

On 04/25/13 14:47, Rob Herring wrote:
> On 04/15/2013 04:33 PM, Stephen Boyd wrote:
>> On 04/15/13 14:20, Rob Herring wrote:
>>> On Fri, Apr 12, 2013 at 7:27 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
>>>> @@ -26,3 +30,52 @@ Example:
>>>>                              <1 10 0xf08>;
>>>>                 clock-frequency = <100000000>;
>>>>         };
>>>> +
>>>> +** Memory mapped timer node properties
>>>> +
>>>> +- compatible : Should at least contain "arm,armv7-timer-mem".
>>> Everything about this timer is architecturally defined? If not, let's
>>> use a more specific name.
>> I'm not sure I'm following you, but everything described here is part of
>> the ARM definition. What would be a more specific name?
> Something that corresponds to the particular implementation like
> cortex-a15 (obviously not an example that has this). I'm fine with
> leaving this for now, but would like to see that added when specific SOC
> dts are added. Better to be specific in case we need to use it for
> something like errata work-arounds. Of course we haven't done that so
> far with the arch timer bindings...

Agreed. I'm under the impression that most of our compatible fields
should be more specific than they currently are so we can workaround hw
bugs like you say. Perhaps the catch all generic one should just be
"arm,arm-timer-mem" since it isn't tied to any particular CPU type?

>
>>>> +
>>>> +- clock-frequency : The frequency of the main counter, in Hz. Optional.
>>>> +
>>>> +- reg : The control frame base address.
>>>> +
>>>> +Note that #address-cells, #size-cells, and ranges shall be present to ensure
>>>> +the CPU can address a frame's registers.
>>>> +
>>>> +Frame:
>>>> +
>>>> +- frame-number: 0 to 7.
>>> I'd really like to get rid of the frame numbers and sub-nodes. Is the
>>> frame number significant to software?
>> We need the frame number to read and write registers in the control
>> frame (the first base in the parent node). We currently use it to
>> determine if a frame has support for the virtual timer by reading the
>> CNTTIDR (a register with 4 bits per frame describing capabilities). If
>> we wanted to control access to the second view of a frame we would also
>> need to configure the CNTPL0ACRn register that pertains to the frame
>> we're controlling. Without a frame number we wouldn't know which
>> register to write.
> I've gone thru the memory mapped part of the spec now, so I think I
> understand things better. I see how the frame number is needed, but...
>
> The control base is only accessible in secure or hyp mode. How does a
> guest know that it is a guest and can't map the control base? Seems like
> we need to allow a subset of the binding that is just a reg and
> interrupts properties without the frame sub nodes.

I don't see that part. My understanding is that the control base is
accessible in non-secure mode and by the guests. There are certain
registers within that base that are only accessible in secure mode
though: CNTFRQ and CNTNSAR. Also some registers are configurable:
CNTACRn and CNTVOFFN. CNTVOFFN is only accessible in the hypervisor.

We don't really care about CNTFRQ because it's duplicated into each
view. We do care about CNTNSAR. Luckily the spec "just works" there in
the sense that we can use CNTTIDR in conjunction with CNTACRn and
determine if we have access to a frame we're interested in if the
CNTTIDR bits say the frame is present and the CNTACRn register says we
can access it. If not then it must be locked down for secure users.

Unfortunately hardware doesn't have a way to say that a particular frame
is reserved for the hypervisor or the guest kernel/userspace. We need
some help from software, so we have the status property express that a
particular frame is available. We have to assume the DT is going to be
different depending on if you're the hypervisor or the guest. That's a
valid assumption right? Otherwise I hope we can do some trapping of the
guest's mapping to the control base and then rewrite what they read so
that they only see the frame that we want to be available to them.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

[PATCHv2 1/4] Documentation: Add memory mapped ARM architected timer binding

[Rob Herring]

On 04/25/2013 05:48 PM, Stephen Boyd wrote:
> On 04/25/13 14:47, Rob Herring wrote:
>> On 04/15/2013 04:33 PM, Stephen Boyd wrote:
>>> On 04/15/13 14:20, Rob Herring wrote:
>>>> On Fri, Apr 12, 2013 at 7:27 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
>>>>> @@ -26,3 +30,52 @@ Example:
>>>>>                              <1 10 0xf08>;
>>>>>                 clock-frequency = <100000000>;
>>>>>         };
>>>>> +
>>>>> +** Memory mapped timer node properties
>>>>> +
>>>>> +- compatible : Should at least contain "arm,armv7-timer-mem".
>>>> Everything about this timer is architecturally defined? If not, let's
>>>> use a more specific name.
>>> I'm not sure I'm following you, but everything described here is part of
>>> the ARM definition. What would be a more specific name?
>> Something that corresponds to the particular implementation like
>> cortex-a15 (obviously not an example that has this). I'm fine with
>> leaving this for now, but would like to see that added when specific SOC
>> dts are added. Better to be specific in case we need to use it for
>> something like errata work-arounds. Of course we haven't done that so
>> far with the arch timer bindings...
> 
> Agreed. I'm under the impression that most of our compatible fields
> should be more specific than they currently are so we can workaround hw
> bugs like you say. Perhaps the catch all generic one should just be
> "arm,arm-timer-mem" since it isn't tied to any particular CPU type?
> 
>>
>>>>> +
>>>>> +- clock-frequency : The frequency of the main counter, in Hz. Optional.
>>>>> +
>>>>> +- reg : The control frame base address.
>>>>> +
>>>>> +Note that #address-cells, #size-cells, and ranges shall be present to ensure
>>>>> +the CPU can address a frame's registers.
>>>>> +
>>>>> +Frame:
>>>>> +
>>>>> +- frame-number: 0 to 7.
>>>> I'd really like to get rid of the frame numbers and sub-nodes. Is the
>>>> frame number significant to software?
>>> We need the frame number to read and write registers in the control
>>> frame (the first base in the parent node). We currently use it to
>>> determine if a frame has support for the virtual timer by reading the
>>> CNTTIDR (a register with 4 bits per frame describing capabilities). If
>>> we wanted to control access to the second view of a frame we would also
>>> need to configure the CNTPL0ACRn register that pertains to the frame
>>> we're controlling. Without a frame number we wouldn't know which
>>> register to write.
>> I've gone thru the memory mapped part of the spec now, so I think I
>> understand things better. I see how the frame number is needed, but...
>>
>> The control base is only accessible in secure or hyp mode. How does a
>> guest know that it is a guest and can't map the control base? Seems like
>> we need to allow a subset of the binding that is just a reg and
>> interrupts properties without the frame sub nodes.
> 
> I don't see that part. My understanding is that the control base is
> accessible in non-secure mode and by the guests. There are certain
> registers within that base that are only accessible in secure mode
> though: CNTFRQ and CNTNSAR. Also some registers are configurable:
> CNTACRn and CNTVOFFN. CNTVOFFN is only accessible in the hypervisor.

The example is section E.8 seems to say otherwise. Perhaps Mark R can
comment further.

> We don't really care about CNTFRQ because it's duplicated into each
> view. We do care about CNTNSAR. Luckily the spec "just works" there in
> the sense that we can use CNTTIDR in conjunction with CNTACRn and
> determine if we have access to a frame we're interested in if the
> CNTTIDR bits say the frame is present and the CNTACRn register says we
> can access it. If not then it must be locked down for secure users.
> 
> Unfortunately hardware doesn't have a way to say that a particular frame
> is reserved for the hypervisor or the guest kernel/userspace. We need
> some help from software, so we have the status property express that a
> particular frame is available. We have to assume the DT is going to be
> different depending on if you're the hypervisor or the guest. That's a
> valid assumption right? Otherwise I hope we can do some trapping of the
> guest's mapping to the control base and then rewrite what they read so
> that they only see the frame that we want to be available to them.

Yeah, I believe the only way to prevent access within non-secure world
is with the MMU. So I guess the example is just policy that the
hypervisor would/may not create a stage2 mapping. You still have the
same issue that the guest should not be passed the control base. You
could make the reg property optional, but then what do you do with the
node name?

Certainly the guest dtb will be different.

Rob

[PATCHv2 1/4] Documentation: Add memory mapped ARM architected timer binding

[Stephen Boyd]

On 04/25/13 16:06, Rob Herring wrote:
> On 04/25/2013 05:48 PM, Stephen Boyd wrote:
>
>> We don't really care about CNTFRQ because it's duplicated into each
>> view. We do care about CNTNSAR. Luckily the spec "just works" there in
>> the sense that we can use CNTTIDR in conjunction with CNTACRn and
>> determine if we have access to a frame we're interested in if the
>> CNTTIDR bits say the frame is present and the CNTACRn register says we
>> can access it. If not then it must be locked down for secure users.
>>
>> Unfortunately hardware doesn't have a way to say that a particular frame
>> is reserved for the hypervisor or the guest kernel/userspace. We need
>> some help from software, so we have the status property express that a
>> particular frame is available. We have to assume the DT is going to be
>> different depending on if you're the hypervisor or the guest. That's a
>> valid assumption right? Otherwise I hope we can do some trapping of the
>> guest's mapping to the control base and then rewrite what they read so
>> that they only see the frame that we want to be available to them.
> Yeah, I believe the only way to prevent access within non-secure world
> is with the MMU. So I guess the example is just policy that the
> hypervisor would/may not create a stage2 mapping. You still have the
> same issue that the guest should not be passed the control base. You
> could make the reg property optional, but then what do you do with the
> node name?

I don't follow. Why shouldn't we tell the guest about the hardware
that's there? Shouldn't they be able to safely assume they can access
the control base just like a non-guest kernel running in PL1 would be
able to?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

[PATCHv2 1/4] Documentation: Add memory mapped ARM architected timer binding

[Mark Rutland]

On Fri, Apr 26, 2013 at 12:06:21AM +0100, Rob Herring wrote:
> On 04/25/2013 05:48 PM, Stephen Boyd wrote:
> > On 04/25/13 14:47, Rob Herring wrote:
> >> On 04/15/2013 04:33 PM, Stephen Boyd wrote:
> >>> On 04/15/13 14:20, Rob Herring wrote:
> >>>> On Fri, Apr 12, 2013 at 7:27 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> >>>>> @@ -26,3 +30,52 @@ Example:
> >>>>>                              <1 10 0xf08>;
> >>>>>                 clock-frequency = <100000000>;
> >>>>>         };
> >>>>> +
> >>>>> +** Memory mapped timer node properties
> >>>>> +
> >>>>> +- compatible : Should at least contain "arm,armv7-timer-mem".
> >>>> Everything about this timer is architecturally defined? If not, let's
> >>>> use a more specific name.
> >>> I'm not sure I'm following you, but everything described here is part of
> >>> the ARM definition. What would be a more specific name?
> >> Something that corresponds to the particular implementation like
> >> cortex-a15 (obviously not an example that has this). I'm fine with
> >> leaving this for now, but would like to see that added when specific SOC
> >> dts are added. Better to be specific in case we need to use it for
> >> something like errata work-arounds. Of course we haven't done that so
> >> far with the arch timer bindings...
> > 
> > Agreed. I'm under the impression that most of our compatible fields
> > should be more specific than they currently are so we can workaround hw
> > bugs like you say. Perhaps the catch all generic one should just be
> > "arm,arm-timer-mem" since it isn't tied to any particular CPU type?
> > 
> >>
> >>>>> +
> >>>>> +- clock-frequency : The frequency of the main counter, in Hz. Optional.
> >>>>> +
> >>>>> +- reg : The control frame base address.
> >>>>> +
> >>>>> +Note that #address-cells, #size-cells, and ranges shall be present to ensure
> >>>>> +the CPU can address a frame's registers.
> >>>>> +
> >>>>> +Frame:
> >>>>> +
> >>>>> +- frame-number: 0 to 7.
> >>>> I'd really like to get rid of the frame numbers and sub-nodes. Is the
> >>>> frame number significant to software?
> >>> We need the frame number to read and write registers in the control
> >>> frame (the first base in the parent node). We currently use it to
> >>> determine if a frame has support for the virtual timer by reading the
> >>> CNTTIDR (a register with 4 bits per frame describing capabilities). If
> >>> we wanted to control access to the second view of a frame we would also
> >>> need to configure the CNTPL0ACRn register that pertains to the frame
> >>> we're controlling. Without a frame number we wouldn't know which
> >>> register to write.
> >> I've gone thru the memory mapped part of the spec now, so I think I
> >> understand things better. I see how the frame number is needed, but...
> >>
> >> The control base is only accessible in secure or hyp mode. How does a
> >> guest know that it is a guest and can't map the control base? Seems like
> >> we need to allow a subset of the binding that is just a reg and
> >> interrupts properties without the frame sub nodes.
> > 
> > I don't see that part. My understanding is that the control base is
> > accessible in non-secure mode and by the guests. There are certain
> > registers within that base that are only accessible in secure mode
> > though: CNTFRQ and CNTNSAR. Also some registers are configurable:
> > CNTACRn and CNTVOFFN. CNTVOFFN is only accessible in the hypervisor.
> 
> The example is section E.8 seems to say otherwise. Perhaps Mark R can
> comment further.

I believe E.8 is an example system, not the general case.

However, I don't see anything limiting accesses to CNTVOFFn to the hypervisor.
As far as I can see, if the guest has access to the CNTCTLBase frame, it has
the same access privileges as the host (and hypervisor).

CNTACRn and CNTVOFFn only seem to be configurable with regard to
secure/non-secure accesses.

> 
> > We don't really care about CNTFRQ because it's duplicated into each
> > view. We do care about CNTNSAR. Luckily the spec "just works" there in
> > the sense that we can use CNTTIDR in conjunction with CNTACRn and
> > determine if we have access to a frame we're interested in if the
> > CNTTIDR bits say the frame is present and the CNTACRn register says we
> > can access it. If not then it must be locked down for secure users.
> > 
> > Unfortunately hardware doesn't have a way to say that a particular frame
> > is reserved for the hypervisor or the guest kernel/userspace. We need
> > some help from software, so we have the status property express that a
> > particular frame is available. We have to assume the DT is going to be
> > different depending on if you're the hypervisor or the guest. That's a
> > valid assumption right? Otherwise I hope we can do some trapping of the
> > guest's mapping to the control base and then rewrite what they read so
> > that they only see the frame that we want to be available to them.
> 
> Yeah, I believe the only way to prevent access within non-secure world
> is with the MMU. So I guess the example is just policy that the
> hypervisor would/may not create a stage2 mapping. You still have the
> same issue that the guest should not be passed the control base. You
> could make the reg property optional, but then what do you do with the
> node name?

I'm not sure on if and how we'd want to expose this to guests. I'm adding Marc
Zyngier to Cc, he may have some relevant thoughts.

Mark.

[PATCHv2 2/4] ARM: arch_timers: Pass clock event to set_mode callback

[Stephen Boyd]

There isn't any reason why we don't pass the event here and we'll
need it in the near future for memory mapped arch timers anyway.

Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Marc Zyngier <Marc.Zyngier@arm.com>
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 drivers/clocksource/arm_arch_timer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 2abb861..545891b 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -140,7 +140,7 @@ static int __cpuinit arch_timer_setup(struct clock_event_device *clk)
 
 	clk->cpumask = cpumask_of(smp_processor_id());
 
-	clk->set_mode(CLOCK_EVT_MODE_SHUTDOWN, NULL);
+	clk->set_mode(CLOCK_EVT_MODE_SHUTDOWN, clk);
 
 	clockevents_config_and_register(clk, arch_timer_rate,
 					0xf, 0x7fffffff);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

[PATCHv2 3/4] clocksource: arch_timer: Push the read/write wrappers deeper

[Stephen Boyd]

We're going to introduce support to read and write the memory
mapped timer registers in the next patch, so push the cp15
read/write functions one level deeper. This simplifies the next
patch and makes it clearer what's going on.

Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Marc Zyngier <Marc.Zyngier@arm.com>
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 arch/arm/include/asm/arch_timer.h    |  5 ++--
 arch/arm64/include/asm/arch_timer.h  |  4 ++--
 drivers/clocksource/arm_arch_timer.c | 44 ++++++++++++++++++++++++------------
 3 files changed, 34 insertions(+), 19 deletions(-)

diff --git a/arch/arm/include/asm/arch_timer.h b/arch/arm/include/asm/arch_timer.h
index 35fea17..23d65f5 100644
--- a/arch/arm/include/asm/arch_timer.h
+++ b/arch/arm/include/asm/arch_timer.h
@@ -18,7 +18,8 @@ int arch_timer_sched_clock_init(void);
  * nicely work out which register we want, and chuck away the rest of
  * the code. At least it does so with a recent GCC (4.6.3).
  */
-static inline void arch_timer_reg_write(const int access, const int reg, u32 val)
+static inline void arch_timer_reg_write_cp15(const int access, const int reg,
+					  u32 val)
 {
 	if (access == ARCH_TIMER_PHYS_ACCESS) {
 		switch (reg) {
@@ -45,7 +46,7 @@ static inline void arch_timer_reg_write(const int access, const int reg, u32 val
 	isb();
 }
 
-static inline u32 arch_timer_reg_read(const int access, const int reg)
+static inline u32 arch_timer_reg_read_cp15(const int access, const int reg)
 {
 	u32 val = 0;
 
diff --git a/arch/arm64/include/asm/arch_timer.h b/arch/arm64/include/asm/arch_timer.h
index 5307737..95db1a9 100644
--- a/arch/arm64/include/asm/arch_timer.h
+++ b/arch/arm64/include/asm/arch_timer.h
@@ -26,7 +26,7 @@
 
 #include <clocksource/arm_arch_timer.h>
 
-static inline void arch_timer_reg_write(int access, int reg, u32 val)
+static inline void arch_timer_reg_write_cp15(int access, int reg, u32 val)
 {
 	if (access == ARCH_TIMER_PHYS_ACCESS) {
 		switch (reg) {
@@ -57,7 +57,7 @@ static inline void arch_timer_reg_write(int access, int reg, u32 val)
 	isb();
 }
 
-static inline u32 arch_timer_reg_read(int access, int reg)
+static inline u32 arch_timer_reg_read_cp15(int access, int reg)
 {
 	u32 val;
 
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 545891b..4f1f002 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -43,14 +43,26 @@ static bool arch_timer_use_virtual = true;
  * Architected system timer support.
  */
 
+static inline void arch_timer_reg_write(int access, int reg, u32 val,
+					struct clock_event_device *clk)
+{
+	arch_timer_reg_write_cp15(access, reg, val);
+}
+
+static inline u32 arch_timer_reg_read(int access, int reg,
+				      struct clock_event_device *clk)
+{
+	return arch_timer_reg_read_cp15(access, reg);
+}
+
 static inline irqreturn_t timer_handler(const int access,
 					struct clock_event_device *evt)
 {
 	unsigned long ctrl;
-	ctrl = arch_timer_reg_read(access, ARCH_TIMER_REG_CTRL);
+	ctrl = arch_timer_reg_read(access, ARCH_TIMER_REG_CTRL, evt);
 	if (ctrl & ARCH_TIMER_CTRL_IT_STAT) {
 		ctrl |= ARCH_TIMER_CTRL_IT_MASK;
-		arch_timer_reg_write(access, ARCH_TIMER_REG_CTRL, ctrl);
+		arch_timer_reg_write(access, ARCH_TIMER_REG_CTRL, ctrl, evt);
 		evt->event_handler(evt);
 		return IRQ_HANDLED;
 	}
@@ -72,15 +84,16 @@ static irqreturn_t arch_timer_handler_phys(int irq, void *dev_id)
 	return timer_handler(ARCH_TIMER_PHYS_ACCESS, evt);
 }
 
-static inline void timer_set_mode(const int access, int mode)
+static inline void timer_set_mode(const int access, int mode,
+				  struct clock_event_device *clk)
 {
 	unsigned long ctrl;
 	switch (mode) {
 	case CLOCK_EVT_MODE_UNUSED:
 	case CLOCK_EVT_MODE_SHUTDOWN:
-		ctrl = arch_timer_reg_read(access, ARCH_TIMER_REG_CTRL);
+		ctrl = arch_timer_reg_read(access, ARCH_TIMER_REG_CTRL, clk);
 		ctrl &= ~ARCH_TIMER_CTRL_ENABLE;
-		arch_timer_reg_write(access, ARCH_TIMER_REG_CTRL, ctrl);
+		arch_timer_reg_write(access, ARCH_TIMER_REG_CTRL, ctrl, clk);
 		break;
 	default:
 		break;
@@ -90,36 +103,37 @@ static inline void timer_set_mode(const int access, int mode)
 static void arch_timer_set_mode_virt(enum clock_event_mode mode,
 				     struct clock_event_device *clk)
 {
-	timer_set_mode(ARCH_TIMER_VIRT_ACCESS, mode);
+	timer_set_mode(ARCH_TIMER_VIRT_ACCESS, mode, clk);
 }
 
 static void arch_timer_set_mode_phys(enum clock_event_mode mode,
 				     struct clock_event_device *clk)
 {
-	timer_set_mode(ARCH_TIMER_PHYS_ACCESS, mode);
+	timer_set_mode(ARCH_TIMER_PHYS_ACCESS, mode, clk);
 }
 
-static inline void set_next_event(const int access, unsigned long evt)
+static inline void set_next_event(const int access, unsigned long evt,
+				  struct clock_event_device *clk)
 {
 	unsigned long ctrl;
-	ctrl = arch_timer_reg_read(access, ARCH_TIMER_REG_CTRL);
+	ctrl = arch_timer_reg_read(access, ARCH_TIMER_REG_CTRL, clk);
 	ctrl |= ARCH_TIMER_CTRL_ENABLE;
 	ctrl &= ~ARCH_TIMER_CTRL_IT_MASK;
-	arch_timer_reg_write(access, ARCH_TIMER_REG_TVAL, evt);
-	arch_timer_reg_write(access, ARCH_TIMER_REG_CTRL, ctrl);
+	arch_timer_reg_write(access, ARCH_TIMER_REG_TVAL, evt, clk);
+	arch_timer_reg_write(access, ARCH_TIMER_REG_CTRL, ctrl, clk);
 }
 
 static int arch_timer_set_next_event_virt(unsigned long evt,
-					  struct clock_event_device *unused)
+					  struct clock_event_device *clk)
 {
-	set_next_event(ARCH_TIMER_VIRT_ACCESS, evt);
+	set_next_event(ARCH_TIMER_VIRT_ACCESS, evt, clk);
 	return 0;
 }
 
 static int arch_timer_set_next_event_phys(unsigned long evt,
-					  struct clock_event_device *unused)
+					  struct clock_event_device *clk)
 {
-	set_next_event(ARCH_TIMER_PHYS_ACCESS, evt);
+	set_next_event(ARCH_TIMER_PHYS_ACCESS, evt, clk);
 	return 0;
 }
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

[PATCHv2 4/4] clocksource: arch_timer: Add support for memory mapped timers

[Stephen Boyd]

Add support for the memory mapped timers by filling in the
read/write functions and adding some parsing code. Note that we
only register one clocksource, preferring the cp15 based
clocksource over the mmio one.

To keep things simple we register one global clockevent. This
covers the case of UP and SMP systems with only mmio hardware and
systems where the memory mapped timers are used as the broadcast
timer in low power modes.

The DT binding allows for per-CPU memory mapped timers in case we
want to support that in the future, but the code isn't added
here. We also don't do much for hypervisor support, although it
should be possible to support it by searching for at least two
frames where one frame has the virtual capability and then
updating KVM timers to support it.

Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Marc Zyngier <Marc.Zyngier@arm.com>
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 drivers/clocksource/arm_arch_timer.c | 415 ++++++++++++++++++++++++++++++-----
 include/clocksource/arm_arch_timer.h |   4 +-
 2 files changed, 361 insertions(+), 58 deletions(-)

diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 4f1f002..7385fca 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -16,13 +16,39 @@
 #include <linux/clockchips.h>
 #include <linux/interrupt.h>
 #include <linux/of_irq.h>
+#include <linux/of_address.h>
 #include <linux/io.h>
+#include <linux/slab.h>
 
 #include <asm/arch_timer.h>
 #include <asm/virt.h>
 
 #include <clocksource/arm_arch_timer.h>
 
+#define CNTTIDR		0x08
+#define CNTTIDR_VIRT(n)	(BIT(1) << ((n) * 4))
+
+#define CNTVCT_LO	0x08
+#define CNTVCT_HI	0x0c
+#define CNTFRQ		0x10
+#define CNTP_TVAL	0x28
+#define CNTP_CTL	0x2c
+#define CNTV_TVAL	0x38
+#define CNTV_CTL	0x3c
+
+#define ARCH_CP15_TIMER	BIT(0)
+#define ARCH_MEM_TIMER	BIT(1)
+static unsigned arch_timers_present;
+
+static void __iomem *arch_counter_base;
+
+struct arch_timer {
+	void __iomem *base;
+	struct clock_event_device evt;
+};
+
+#define to_arch_timer(e) container_of(e, struct arch_timer, evt)
+
 static u32 arch_timer_rate;
 
 enum ppi_nr {
@@ -38,6 +64,7 @@ static int arch_timer_ppi[MAX_TIMER_PPI];
 static struct clock_event_device __percpu *arch_timer_evt;
 
 static bool arch_timer_use_virtual = true;
+static bool arch_timer_mem_use_virtual = false;
 
 /*
  * Architected system timer support.
@@ -46,13 +73,69 @@ static bool arch_timer_use_virtual = true;
 static inline void arch_timer_reg_write(int access, int reg, u32 val,
 					struct clock_event_device *clk)
 {
-	arch_timer_reg_write_cp15(access, reg, val);
+	if (access == ARCH_TIMER_MEM_PHYS_ACCESS) {
+		struct arch_timer *timer = to_arch_timer(clk);
+		switch (reg) {
+		case ARCH_TIMER_REG_CTRL:
+			writel_relaxed(val, timer->base + CNTP_CTL);
+			break;
+		case ARCH_TIMER_REG_TVAL:
+			writel_relaxed(val, timer->base + CNTP_TVAL);
+			break;
+		default:
+			BUILD_BUG();
+		}
+	} else if (access == ARCH_TIMER_MEM_VIRT_ACCESS) {
+		struct arch_timer *timer = to_arch_timer(clk);
+		switch (reg) {
+		case ARCH_TIMER_REG_CTRL:
+			writel_relaxed(val, timer->base + CNTV_CTL);
+			break;
+		case ARCH_TIMER_REG_TVAL:
+			writel_relaxed(val, timer->base + CNTV_TVAL);
+			break;
+		default:
+			BUILD_BUG();
+		}
+	} else {
+		arch_timer_reg_write_cp15(access, reg, val);
+	}
 }
 
 static inline u32 arch_timer_reg_read(int access, int reg,
 				      struct clock_event_device *clk)
 {
-	return arch_timer_reg_read_cp15(access, reg);
+	u32 val;
+
+	if (access == ARCH_TIMER_MEM_PHYS_ACCESS) {
+		struct arch_timer *timer = to_arch_timer(clk);
+		switch (reg) {
+		case ARCH_TIMER_REG_CTRL:
+			val = readl_relaxed(timer->base + CNTP_CTL);
+			break;
+		case ARCH_TIMER_REG_TVAL:
+			val = readl_relaxed(timer->base + CNTP_TVAL);
+			break;
+		default:
+			BUILD_BUG();
+		}
+	} else if (access == ARCH_TIMER_MEM_VIRT_ACCESS) {
+		struct arch_timer *timer = to_arch_timer(clk);
+		switch (reg) {
+		case ARCH_TIMER_REG_CTRL:
+			val = readl_relaxed(timer->base + CNTV_CTL);
+			break;
+		case ARCH_TIMER_REG_TVAL:
+			val = readl_relaxed(timer->base + CNTV_TVAL);
+			break;
+		default:
+			BUILD_BUG();
+		}
+	} else {
+		val = arch_timer_reg_read_cp15(access, reg);
+	}
+
+	return val;
 }
 
 static inline irqreturn_t timer_handler(const int access,
@@ -84,6 +167,20 @@ static irqreturn_t arch_timer_handler_phys(int irq, void *dev_id)
 	return timer_handler(ARCH_TIMER_PHYS_ACCESS, evt);
 }
 
+static irqreturn_t arch_timer_handler_phys_mem(int irq, void *dev_id)
+{
+	struct clock_event_device *evt = dev_id;
+
+	return timer_handler(ARCH_TIMER_MEM_PHYS_ACCESS, evt);
+}
+
+static irqreturn_t arch_timer_handler_virt_mem(int irq, void *dev_id)
+{
+	struct clock_event_device *evt = dev_id;
+
+	return timer_handler(ARCH_TIMER_MEM_VIRT_ACCESS, evt);
+}
+
 static inline void timer_set_mode(const int access, int mode,
 				  struct clock_event_device *clk)
 {
@@ -112,6 +209,18 @@ static void arch_timer_set_mode_phys(enum clock_event_mode mode,
 	timer_set_mode(ARCH_TIMER_PHYS_ACCESS, mode, clk);
 }
 
+static void arch_timer_set_mode_virt_mem(enum clock_event_mode mode,
+					 struct clock_event_device *clk)
+{
+	timer_set_mode(ARCH_TIMER_MEM_VIRT_ACCESS, mode, clk);
+}
+
+static void arch_timer_set_mode_phys_mem(enum clock_event_mode mode,
+					 struct clock_event_device *clk)
+{
+	timer_set_mode(ARCH_TIMER_MEM_PHYS_ACCESS, mode, clk);
+}
+
 static inline void set_next_event(const int access, unsigned long evt,
 				  struct clock_event_device *clk)
 {
@@ -137,27 +246,62 @@ static int arch_timer_set_next_event_phys(unsigned long evt,
 	return 0;
 }
 
-static int __cpuinit arch_timer_setup(struct clock_event_device *clk)
+static int arch_timer_set_next_event_virt_mem(unsigned long evt,
+					      struct clock_event_device *clk)
 {
-	clk->features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_C3STOP;
-	clk->name = "arch_sys_timer";
-	clk->rating = 450;
-	if (arch_timer_use_virtual) {
-		clk->irq = arch_timer_ppi[VIRT_PPI];
-		clk->set_mode = arch_timer_set_mode_virt;
-		clk->set_next_event = arch_timer_set_next_event_virt;
+	set_next_event(ARCH_TIMER_MEM_VIRT_ACCESS, evt, clk);
+	return 0;
+}
+
+static int arch_timer_set_next_event_phys_mem(unsigned long evt,
+					      struct clock_event_device *clk)
+{
+	set_next_event(ARCH_TIMER_MEM_PHYS_ACCESS, evt, clk);
+	return 0;
+}
+
+static void __cpuinit __arch_timer_setup(unsigned type,
+				       struct clock_event_device *clk)
+{
+	clk->features = CLOCK_EVT_FEAT_ONESHOT;
+
+	if (type == ARCH_CP15_TIMER) {
+		clk->features |= CLOCK_EVT_FEAT_C3STOP;
+		clk->name = "arch_sys_timer";
+		clk->rating = 450;
+		clk->cpumask = cpumask_of(smp_processor_id());
+		if (arch_timer_use_virtual) {
+			clk->irq = arch_timer_ppi[VIRT_PPI];
+			clk->set_mode = arch_timer_set_mode_virt;
+			clk->set_next_event = arch_timer_set_next_event_virt;
+		} else {
+			clk->irq = arch_timer_ppi[PHYS_SECURE_PPI];
+			clk->set_mode = arch_timer_set_mode_phys;
+			clk->set_next_event = arch_timer_set_next_event_phys;
+		}
 	} else {
-		clk->irq = arch_timer_ppi[PHYS_SECURE_PPI];
-		clk->set_mode = arch_timer_set_mode_phys;
-		clk->set_next_event = arch_timer_set_next_event_phys;
+		clk->name = "arch_mem_timer";
+		clk->rating = 400;
+		clk->cpumask = cpu_all_mask;
+		if (arch_timer_mem_use_virtual) {
+			clk->set_mode = arch_timer_set_mode_virt_mem;
+			clk->set_next_event =
+				arch_timer_set_next_event_virt_mem;
+		} else {
+			clk->set_mode = arch_timer_set_mode_phys_mem;
+			clk->set_next_event =
+				arch_timer_set_next_event_phys_mem;
+		}
 	}
 
-	clk->cpumask = cpumask_of(smp_processor_id());
-
 	clk->set_mode(CLOCK_EVT_MODE_SHUTDOWN, clk);
 
-	clockevents_config_and_register(clk, arch_timer_rate,
-					0xf, 0x7fffffff);
+	clockevents_config_and_register(clk, arch_timer_rate, 0xf, 0x7fffffff);
+}
+
+static int __cpuinit arch_timer_setup(struct clock_event_device *clk)
+{
+	__arch_timer_setup(ARCH_CP15_TIMER, clk);
 
 	if (arch_timer_use_virtual)
 		enable_percpu_irq(arch_timer_ppi[VIRT_PPI], 0);
@@ -172,27 +316,41 @@ static int __cpuinit arch_timer_setup(struct clock_event_device *clk)
 	return 0;
 }
 
-static int arch_timer_available(void)
+static void
+arch_timer_detect_rate(void __iomem *cntbase, struct device_node *np)
 {
-	u32 freq;
-
-	if (arch_timer_rate == 0) {
-		freq = arch_timer_get_cntfrq();
+	/* Who has more than one independent system counter? */
+	if (arch_timer_rate)
+		return;
 
-		/* Check the timer frequency. */
-		if (freq == 0) {
-			pr_warn("Architected timer frequency not available\n");
-			return -EINVAL;
-		}
-
-		arch_timer_rate = freq;
+	/* Try to determine the frequency from the device tree or CNTFRQ */
+	if (of_property_read_u32(np, "clock-frequency", &arch_timer_rate)) {
+		if (cntbase)
+			arch_timer_rate = readl_relaxed(cntbase + CNTFRQ);
+		else
+			arch_timer_rate = arch_timer_get_cntfrq();
 	}
 
-	pr_info_once("Architected local timer running at %lu.%02luMHz (%s).\n",
+	/* Check the timer frequency. */
+	if (arch_timer_rate == 0)
+		pr_warn("Architected timer frequency not available\n");
+}
+
+static void arch_timer_banner(unsigned type)
+{
+	pr_info("Architected %s%s%s timer(s) running at %lu.%02luMHz (%s%s%s).\n",
+		     type & ARCH_CP15_TIMER ? "cp15" : "",
+		     type == (ARCH_CP15_TIMER | ARCH_MEM_TIMER) ?  " and " : "",
+		     type & ARCH_MEM_TIMER ? "mmio" : "",
 		     (unsigned long)arch_timer_rate / 1000000,
 		     (unsigned long)(arch_timer_rate / 10000) % 100,
-		     arch_timer_use_virtual ? "virt" : "phys");
-	return 0;
+		     type & ARCH_CP15_TIMER ?
+			arch_timer_use_virtual ? "virt" : "phys" :
+			"",
+		     type == (ARCH_CP15_TIMER | ARCH_MEM_TIMER) ?  "/" : "",
+		     type & ARCH_MEM_TIMER ?
+			arch_timer_mem_use_virtual ? "virt" : "phys" :
+			"");
 }
 
 u32 arch_timer_get_rate(void)
@@ -200,19 +358,35 @@ u32 arch_timer_get_rate(void)
 	return arch_timer_rate;
 }
 
-u64 arch_timer_read_counter(void)
+static u64 arch_counter_get_mem_cntvct(void)
 {
-	return arch_counter_get_cntvct();
+	u32 vct_lo, vct_hi, tmp_hi;
+
+	do {
+		vct_hi = readl_relaxed(arch_counter_base + CNTVCT_HI);
+		vct_lo = readl_relaxed(arch_counter_base + CNTVCT_LO);
+		tmp_hi = readl_relaxed(arch_counter_base + CNTVCT_HI);
+	} while (vct_hi != tmp_hi);
+
+	return ((u64) vct_hi << 32) | vct_lo;
 }
 
+/*
+ * Default to cp15 based access because arm64 uses this function for
+ * sched_clock() before DT is probed and the cp15 method is guaranteed
+ * to exist on arm64. arm doesn't use this before DT is probed so even
+ * if we don't have the cp15 accessors we won't have a problem.
+ */
+u64 (*arch_timer_read_counter)(void) = arch_counter_get_cntvct;
+
 static cycle_t arch_counter_read(struct clocksource *cs)
 {
-	return arch_counter_get_cntvct();
+	return arch_timer_read_counter();
 }
 
 static cycle_t arch_counter_read_cc(const struct cyclecounter *cc)
 {
-	return arch_counter_get_cntvct();
+	return arch_timer_read_counter();
 }
 
 static struct clocksource clocksource_counter = {
@@ -235,6 +409,23 @@ struct timecounter *arch_timer_get_timecounter(void)
 	return &timecounter;
 }
 
+static void __init arch_counter_register(unsigned type)
+{
+	u64 start_count;
+
+	/* Register the CP15 based counter if we have one */
+	if (type & ARCH_CP15_TIMER)
+		arch_timer_read_counter = arch_counter_get_cntvct;
+	else
+		arch_timer_read_counter = arch_counter_get_mem_cntvct;
+
+	start_count = arch_timer_read_counter();
+	clocksource_register_hz(&clocksource_counter, arch_timer_rate);
+	cyclecounter.mult = clocksource_counter.mult;
+	cyclecounter.shift = clocksource_counter.shift;
+	timecounter_init(&timecounter, &cyclecounter, start_count);
+}
+
 static void __cpuinit arch_timer_stop(struct clock_event_device *clk)
 {
 	pr_debug("arch_timer_teardown disable IRQ%d cpu #%d\n",
@@ -277,22 +468,12 @@ static int __init arch_timer_register(void)
 	int err;
 	int ppi;
 
-	err = arch_timer_available();
-	if (err)
-		goto out;
-
 	arch_timer_evt = alloc_percpu(struct clock_event_device);
 	if (!arch_timer_evt) {
 		err = -ENOMEM;
 		goto out;
 	}
 
-	clocksource_register_hz(&clocksource_counter, arch_timer_rate);
-	cyclecounter.mult = clocksource_counter.mult;
-	cyclecounter.shift = clocksource_counter.shift;
-	timecounter_init(&timecounter, &cyclecounter,
-			 arch_counter_get_cntvct());
-
 	if (arch_timer_use_virtual) {
 		ppi = arch_timer_ppi[VIRT_PPI];
 		err = request_percpu_irq(ppi, arch_timer_handler_virt,
@@ -343,31 +524,51 @@ out:
 	return err;
 }
 
+static int __init arch_timer_mem_register(void __iomem *base, unsigned int irq)
+{
+	int ret;
+	irq_handler_t func;
+	struct arch_timer *t;
+
+	t = kzalloc(sizeof(*t), GFP_KERNEL);
+	if (!t)
+		return -ENOMEM;
+
+	t->base = base;
+	t->evt.irq = irq;
+	__arch_timer_setup(ARCH_MEM_TIMER, &t->evt);
+
+	if (arch_timer_mem_use_virtual)
+		func = arch_timer_handler_virt_mem;
+	else
+		func = arch_timer_handler_phys_mem;
+
+	ret = request_irq(irq, func, IRQF_TIMER, "arch_mem_timer", &t->evt);
+	if (ret)
+		kfree(t);
+
+	return ret;
+}
+
 static const struct of_device_id arch_timer_of_match[] __initconst = {
 	{ .compatible	= "arm,armv7-timer",	},
 	{ .compatible	= "arm,armv8-timer",	},
 	{},
 };
 
-int __init arch_timer_init(void)
+static int __init arch_timer_parse(void)
 {
 	struct device_node *np;
-	u32 freq;
 	int i;
 
 	np = of_find_matching_node(NULL, arch_timer_of_match);
-	if (!np) {
-		pr_err("arch_timer: can't find DT node\n");
-		return -ENODEV;
-	}
-
-	/* Try to determine the frequency from the device tree or CNTFRQ */
-	if (!of_property_read_u32(np, "clock-frequency", &freq))
-		arch_timer_rate = freq;
+	if (!np)
+		return 0;
 
+	arch_timers_present |= ARCH_CP15_TIMER;
 	for (i = PHYS_SECURE_PPI; i < MAX_TIMER_PPI; i++)
 		arch_timer_ppi[i] = irq_of_parse_and_map(np, i);
-
+	arch_timer_detect_rate(NULL, np);
 	of_node_put(np);
 
 	/*
@@ -390,3 +591,103 @@ int __init arch_timer_init(void)
 
 	return arch_timer_register();
 }
+
+static const struct of_device_id arch_timer_mem_of_match[] __initconst = {
+	{ .compatible	= "arm,armv7-timer-mem", },
+	{},
+};
+
+static int __init arch_timer_mem_parse(void)
+{
+	struct device_node *np, *frame, *best_frame = NULL;
+	void __iomem *cntctlbase, *base;
+	unsigned int irq;
+	u32 cnttidr;
+
+	np = of_find_matching_node(NULL, arch_timer_mem_of_match);
+	if (!np)
+		return 0;
+
+	arch_timers_present |= ARCH_MEM_TIMER;
+	cntctlbase = of_iomap(np, 0);
+	if (!cntctlbase) {
+		pr_err("arch_timer: Can't find CNTCTLBase\n");
+		of_node_put(np);
+		return -ENOMEM;
+	}
+
+	cnttidr = readl_relaxed(cntctlbase + CNTTIDR);
+	iounmap(cntctlbase);
+
+	/*
+	 * Try to find a virtual capable frame. Otherwise fall back to a
+	 * physical capable frame.
+	 */
+	for_each_available_child_of_node(np, frame) {
+		int n;
+
+		if (of_property_read_u32(frame, "frame-number", &n)) {
+			pr_err("arch_timer: Missing frame-number\n");
+			of_node_put(best_frame);
+			of_node_put(frame);
+			of_node_put(np);
+			return -EINVAL;
+		}
+
+		if (cnttidr & CNTTIDR_VIRT(n)) {
+			of_node_put(best_frame);
+			best_frame = frame;
+			arch_timer_mem_use_virtual = true;
+			break;
+		}
+		of_node_put(best_frame);
+		best_frame = of_node_get(frame);
+	}
+	of_node_put(np);
+
+	base = arch_counter_base = of_iomap(best_frame, 0);
+	if (!base) {
+		pr_err("arch_timer: Can't map frame's registers\n");
+		of_node_put(best_frame);
+		return -ENOMEM;
+	}
+
+	if (arch_timer_mem_use_virtual)
+		irq = irq_of_parse_and_map(best_frame, 1);
+	else
+		irq = irq_of_parse_and_map(best_frame, 0);
+	if (!irq) {
+		pr_err("arch_timer: Frame missing %s irq",
+				arch_timer_mem_use_virtual ? "virt" : "phys");
+		of_node_put(best_frame);
+		return -EINVAL;
+	}
+
+	arch_timer_detect_rate(base, best_frame);
+	of_node_put(best_frame);
+
+	return arch_timer_mem_register(base, irq);
+}
+
+int __init arch_timer_init(void)
+{
+	int err;
+
+	err = arch_timer_parse();
+	if (err)
+		return err;
+
+	err = arch_timer_mem_parse();
+	if (err)
+		return err;
+
+	if (!(arch_timers_present & (ARCH_CP15_TIMER | ARCH_MEM_TIMER))) {
+		pr_err("arch_timer: can't find DT node\n");
+		return -ENODEV;
+	}
+
+	arch_timer_banner(arch_timers_present);
+	arch_counter_register(arch_timers_present);
+
+	return 0;
+}
diff --git a/include/clocksource/arm_arch_timer.h b/include/clocksource/arm_arch_timer.h
index 3883812..c3ff7fe 100644
--- a/include/clocksource/arm_arch_timer.h
+++ b/include/clocksource/arm_arch_timer.h
@@ -28,12 +28,14 @@
 
 #define ARCH_TIMER_PHYS_ACCESS		0
 #define ARCH_TIMER_VIRT_ACCESS		1
+#define ARCH_TIMER_MEM_PHYS_ACCESS	2
+#define ARCH_TIMER_MEM_VIRT_ACCESS	3
 
 #ifdef CONFIG_ARM_ARCH_TIMER
 
 extern int arch_timer_init(void);
 extern u32 arch_timer_get_rate(void);
-extern u64 arch_timer_read_counter(void);
+extern u64 (*arch_timer_read_counter)(void);
 extern struct timecounter *arch_timer_get_timecounter(void);
 
 #else
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation