[PATCH v4] clocksource: arch_timer: Allow the device tree to specify uninitialized timer registers

[PATCH v4] clocksource: arch_timer: Allow the device tree to specify uninitialized timer registers

[Sonny Rao]

From: Doug Anderson <dianders@chromium.org>

Some 32-bit (ARMv7) systems are architected like this:

* The firmware doesn't know and doesn't care about hypervisor mode and
  we don't want to add the complexity of hypervisor there.

* The firmware isn't involved in SMP bringup or resume.

* The ARCH timer come up with an uninitialized offset (CNTVOFF)
  between the virtual and physical counters.  Each core gets a
  different random offset.

* The device boots in "Secure SVC" mode.

* Nothing has touched the reset value of CNTHCTL.PL1PCEN or
  CNTHCTL.PL1PCTEN (both default to 1 at reset)

On systems like the above, it doesn't make sense to use the virtual
counter.  There's nobody managing the offset and each time a core goes
down and comes back up it will get reinitialized to some other random
value.

This adds an optional property which can inform the kernel of this
situation, and firmware is free to remove the property if it is going
to initialize the CNTVOFF registers when each CPU comes out of reset.

Currently, the best course of action in this case is to use the
physical timer, which is why it is important that CNTHCTL hasn't been
changed from its reset value and it's a reasonable assumption given
that the firmware has never entered HYP mode.

Note that it's been said that on ARMv8 systems the firmware and
kernel really can't be architected as described above.  That means
using the physical timer like this really only makes sense for ARMv7
systems.

Signed-off-by: Doug Anderson <dianders@chromium.org>
Signed-off-by: Sonny Rao <sonnyrao@chromium.org>
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
---
Changes in v2:
- Add "#ifdef CONFIG_ARM" as per Will Deacon

Changes in v3:
- change property name to arm,cntvoff-not-fw-configured and specify
  that the value of CNTHCTL.PL1PC(T)EN must still be the reset value
  of 1 as per Mark Rutland

Changes in v4:
- change property name to arm,cpu-registers-not-fw-configured and
  specify that all cpu registers must have architected reset values
  per Mark Rutland
- change from "#ifdef CONFIG_ARM" to "if (IS_ENABLED(CONFIG_ARM))" per
  Arnd Bergmann
---
 Documentation/devicetree/bindings/arm/arch_timer.txt | 8 ++++++++
 drivers/clocksource/arm_arch_timer.c                 | 8 ++++++++
 2 files changed, 16 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/arch_timer.txt b/Documentation/devicetree/bindings/arm/arch_timer.txt
index 37b2caf..256b4d8 100644
--- a/Documentation/devicetree/bindings/arm/arch_timer.txt
+++ b/Documentation/devicetree/bindings/arm/arch_timer.txt
@@ -22,6 +22,14 @@ to deliver its interrupts via SPIs.
 - always-on : a boolean property. If present, the timer is powered through an
   always-on power domain, therefore it never loses context.
 
+** Optional properties:
+
+- arm,cpu-registers-not-fw-configured : Firmware does not initialize
+  any of the generic timer CPU registers, which contain their
+  architecturally-defined reset values. Only supported for 32-bit
+  systems which follow the ARMv7 architected reset values.
+
+
 Example:
 
 	timer {
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 8daf056..799139f 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -654,6 +654,14 @@ static void __init arch_timer_init(struct device_node *np)
 	arch_timer_detect_rate(NULL, np);
 
 	/*
+	 * If we cannot rely on firmware initializing the timer registers then
+	 * we should use the physical timers instead.
+	 */
+	if (IS_ENABLED(CONFIG_ARM) &&
+	    of_property_read_bool(np, "arm,cpu-registers-not-fw-configured"))
+			arch_timer_use_virtual = false;
+
+	/*
 	 * If HYP mode is available, we know that the physical timer
 	 * has been configured to be accessible from PL1. Use it, so
 	 * that a guest can use the virtual timer instead.
-- 
1.8.3.2

[PATCH v4] clocksource: arch_timer: Allow the device tree to specify uninitialized timer registers

[Sonny Rao]

From: Doug Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>

Some 32-bit (ARMv7) systems are architected like this:

* The firmware doesn't know and doesn't care about hypervisor mode and
  we don't want to add the complexity of hypervisor there.

* The firmware isn't involved in SMP bringup or resume.

* The ARCH timer come up with an uninitialized offset (CNTVOFF)
  between the virtual and physical counters.  Each core gets a
  different random offset.

* The device boots in "Secure SVC" mode.

* Nothing has touched the reset value of CNTHCTL.PL1PCEN or
  CNTHCTL.PL1PCTEN (both default to 1 at reset)

On systems like the above, it doesn't make sense to use the virtual
counter.  There's nobody managing the offset and each time a core goes
down and comes back up it will get reinitialized to some other random
value.

This adds an optional property which can inform the kernel of this
situation, and firmware is free to remove the property if it is going
to initialize the CNTVOFF registers when each CPU comes out of reset.

Currently, the best course of action in this case is to use the
physical timer, which is why it is important that CNTHCTL hasn't been
changed from its reset value and it's a reasonable assumption given
that the firmware has never entered HYP mode.

Note that it's been said that on ARMv8 systems the firmware and
kernel really can't be architected as described above.  That means
using the physical timer like this really only makes sense for ARMv7
systems.

Signed-off-by: Doug Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Signed-off-by: Sonny Rao <sonnyrao-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Reviewed-by: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
---
Changes in v2:
- Add "#ifdef CONFIG_ARM" as per Will Deacon

Changes in v3:
- change property name to arm,cntvoff-not-fw-configured and specify
  that the value of CNTHCTL.PL1PC(T)EN must still be the reset value
  of 1 as per Mark Rutland

Changes in v4:
- change property name to arm,cpu-registers-not-fw-configured and
  specify that all cpu registers must have architected reset values
  per Mark Rutland
- change from "#ifdef CONFIG_ARM" to "if (IS_ENABLED(CONFIG_ARM))" per
  Arnd Bergmann
---
 Documentation/devicetree/bindings/arm/arch_timer.txt | 8 ++++++++
 drivers/clocksource/arm_arch_timer.c                 | 8 ++++++++
 2 files changed, 16 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/arch_timer.txt b/Documentation/devicetree/bindings/arm/arch_timer.txt
index 37b2caf..256b4d8 100644
--- a/Documentation/devicetree/bindings/arm/arch_timer.txt
+++ b/Documentation/devicetree/bindings/arm/arch_timer.txt
@@ -22,6 +22,14 @@ to deliver its interrupts via SPIs.
 - always-on : a boolean property. If present, the timer is powered through an
   always-on power domain, therefore it never loses context.
 
+** Optional properties:
+
+- arm,cpu-registers-not-fw-configured : Firmware does not initialize
+  any of the generic timer CPU registers, which contain their
+  architecturally-defined reset values. Only supported for 32-bit
+  systems which follow the ARMv7 architected reset values.
+
+
 Example:
 
 	timer {
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 8daf056..799139f 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -654,6 +654,14 @@ static void __init arch_timer_init(struct device_node *np)
 	arch_timer_detect_rate(NULL, np);
 
 	/*
+	 * If we cannot rely on firmware initializing the timer registers then
+	 * we should use the physical timers instead.
+	 */
+	if (IS_ENABLED(CONFIG_ARM) &&
+	    of_property_read_bool(np, "arm,cpu-registers-not-fw-configured"))
+			arch_timer_use_virtual = false;
+
+	/*
 	 * If HYP mode is available, we know that the physical timer
 	 * has been configured to be accessible from PL1. Use it, so
 	 * that a guest can use the virtual timer instead.
-- 
1.8.3.2

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4] clocksource: arch_timer: Allow the device tree to specify uninitialized timer registers

[Sonny Rao]

From: Doug Anderson <dianders@chromium.org>

Some 32-bit (ARMv7) systems are architected like this:

* The firmware doesn't know and doesn't care about hypervisor mode and
  we don't want to add the complexity of hypervisor there.

* The firmware isn't involved in SMP bringup or resume.

* The ARCH timer come up with an uninitialized offset (CNTVOFF)
  between the virtual and physical counters.  Each core gets a
  different random offset.

* The device boots in "Secure SVC" mode.

* Nothing has touched the reset value of CNTHCTL.PL1PCEN or
  CNTHCTL.PL1PCTEN (both default to 1 at reset)

On systems like the above, it doesn't make sense to use the virtual
counter.  There's nobody managing the offset and each time a core goes
down and comes back up it will get reinitialized to some other random
value.

This adds an optional property which can inform the kernel of this
situation, and firmware is free to remove the property if it is going
to initialize the CNTVOFF registers when each CPU comes out of reset.

Currently, the best course of action in this case is to use the
physical timer, which is why it is important that CNTHCTL hasn't been
changed from its reset value and it's a reasonable assumption given
that the firmware has never entered HYP mode.

Note that it's been said that on ARMv8 systems the firmware and
kernel really can't be architected as described above.  That means
using the physical timer like this really only makes sense for ARMv7
systems.

Signed-off-by: Doug Anderson <dianders@chromium.org>
Signed-off-by: Sonny Rao <sonnyrao@chromium.org>
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
---
Changes in v2:
- Add "#ifdef CONFIG_ARM" as per Will Deacon

Changes in v3:
- change property name to arm,cntvoff-not-fw-configured and specify
  that the value of CNTHCTL.PL1PC(T)EN must still be the reset value
  of 1 as per Mark Rutland

Changes in v4:
- change property name to arm,cpu-registers-not-fw-configured and
  specify that all cpu registers must have architected reset values
  per Mark Rutland
- change from "#ifdef CONFIG_ARM" to "if (IS_ENABLED(CONFIG_ARM))" per
  Arnd Bergmann
---
 Documentation/devicetree/bindings/arm/arch_timer.txt | 8 ++++++++
 drivers/clocksource/arm_arch_timer.c                 | 8 ++++++++
 2 files changed, 16 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/arch_timer.txt b/Documentation/devicetree/bindings/arm/arch_timer.txt
index 37b2caf..256b4d8 100644
--- a/Documentation/devicetree/bindings/arm/arch_timer.txt
+++ b/Documentation/devicetree/bindings/arm/arch_timer.txt
@@ -22,6 +22,14 @@ to deliver its interrupts via SPIs.
 - always-on : a boolean property. If present, the timer is powered through an
   always-on power domain, therefore it never loses context.
 
+** Optional properties:
+
+- arm,cpu-registers-not-fw-configured : Firmware does not initialize
+  any of the generic timer CPU registers, which contain their
+  architecturally-defined reset values. Only supported for 32-bit
+  systems which follow the ARMv7 architected reset values.
+
+
 Example:
 
 	timer {
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 8daf056..799139f 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -654,6 +654,14 @@ static void __init arch_timer_init(struct device_node *np)
 	arch_timer_detect_rate(NULL, np);
 
 	/*
+	 * If we cannot rely on firmware initializing the timer registers then
+	 * we should use the physical timers instead.
+	 */
+	if (IS_ENABLED(CONFIG_ARM) &&
+	    of_property_read_bool(np, "arm,cpu-registers-not-fw-configured"))
+			arch_timer_use_virtual = false;
+
+	/*
 	 * If HYP mode is available, we know that the physical timer
 	 * has been configured to be accessible from PL1. Use it, so
 	 * that a guest can use the virtual timer instead.
-- 
1.8.3.2

Re: [PATCH v4] clocksource: arch_timer: Allow the device tree to specify uninitialized timer registers

[Doug Anderson]

Daniel,

On Wed, Oct 8, 2014 at 12:33 AM, Sonny Rao <sonnyrao@chromium.org> wrote:
> From: Doug Anderson <dianders@chromium.org>
>
> Some 32-bit (ARMv7) systems are architected like this:
>
> * The firmware doesn't know and doesn't care about hypervisor mode and
>   we don't want to add the complexity of hypervisor there.
>
> * The firmware isn't involved in SMP bringup or resume.
>
> * The ARCH timer come up with an uninitialized offset (CNTVOFF)
>   between the virtual and physical counters.  Each core gets a
>   different random offset.
>
> * The device boots in "Secure SVC" mode.
>
> * Nothing has touched the reset value of CNTHCTL.PL1PCEN or
>   CNTHCTL.PL1PCTEN (both default to 1 at reset)
>
> On systems like the above, it doesn't make sense to use the virtual
> counter.  There's nobody managing the offset and each time a core goes
> down and comes back up it will get reinitialized to some other random
> value.
>
> This adds an optional property which can inform the kernel of this
> situation, and firmware is free to remove the property if it is going
> to initialize the CNTVOFF registers when each CPU comes out of reset.
>
> Currently, the best course of action in this case is to use the
> physical timer, which is why it is important that CNTHCTL hasn't been
> changed from its reset value and it's a reasonable assumption given
> that the firmware has never entered HYP mode.
>
> Note that it's been said that on ARMv8 systems the firmware and
> kernel really can't be architected as described above.  That means
> using the physical timer like this really only makes sense for ARMv7
> systems.
>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> Signed-off-by: Sonny Rao <sonnyrao@chromium.org>
> Reviewed-by: Mark Rutland <mark.rutland@arm.com>
> ---
> Changes in v2:
> - Add "#ifdef CONFIG_ARM" as per Will Deacon
>
> Changes in v3:
> - change property name to arm,cntvoff-not-fw-configured and specify
>   that the value of CNTHCTL.PL1PC(T)EN must still be the reset value
>   of 1 as per Mark Rutland
>
> Changes in v4:
> - change property name to arm,cpu-registers-not-fw-configured and
>   specify that all cpu registers must have architected reset values
>   per Mark Rutland
> - change from "#ifdef CONFIG_ARM" to "if (IS_ENABLED(CONFIG_ARM))" per
>   Arnd Bergmann
> ---
>  Documentation/devicetree/bindings/arm/arch_timer.txt | 8 ++++++++
>  drivers/clocksource/arm_arch_timer.c                 | 8 ++++++++
>  2 files changed, 16 insertions(+)

Do you know what the status of this patch is?  This patch and Sonny's
patch at <https://patchwork.kernel.org/patch/5051901/> are needed on
Rockchip rk3288 for some specific things:

1. To make SMP happy with coreboot.

2. To (I assume) make SMP happy after S2R (no matter which firmware is
used since I don't think anyone has PSCI for rk3288).

...we still need a DTS entry atop these patches, but that's trivial to
add once these land.

Thanks!

Re: [PATCH v4] clocksource: arch_timer: Allow the device tree to specify uninitialized timer registers

[Doug Anderson]

Daniel,

On Wed, Oct 8, 2014 at 12:33 AM, Sonny Rao <sonnyrao-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
> From: Doug Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
>
> Some 32-bit (ARMv7) systems are architected like this:
>
> * The firmware doesn't know and doesn't care about hypervisor mode and
>   we don't want to add the complexity of hypervisor there.
>
> * The firmware isn't involved in SMP bringup or resume.
>
> * The ARCH timer come up with an uninitialized offset (CNTVOFF)
>   between the virtual and physical counters.  Each core gets a
>   different random offset.
>
> * The device boots in "Secure SVC" mode.
>
> * Nothing has touched the reset value of CNTHCTL.PL1PCEN or
>   CNTHCTL.PL1PCTEN (both default to 1 at reset)
>
> On systems like the above, it doesn't make sense to use the virtual
> counter.  There's nobody managing the offset and each time a core goes
> down and comes back up it will get reinitialized to some other random
> value.
>
> This adds an optional property which can inform the kernel of this
> situation, and firmware is free to remove the property if it is going
> to initialize the CNTVOFF registers when each CPU comes out of reset.
>
> Currently, the best course of action in this case is to use the
> physical timer, which is why it is important that CNTHCTL hasn't been
> changed from its reset value and it's a reasonable assumption given
> that the firmware has never entered HYP mode.
>
> Note that it's been said that on ARMv8 systems the firmware and
> kernel really can't be architected as described above.  That means
> using the physical timer like this really only makes sense for ARMv7
> systems.
>
> Signed-off-by: Doug Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> Signed-off-by: Sonny Rao <sonnyrao-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> Reviewed-by: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
> ---
> Changes in v2:
> - Add "#ifdef CONFIG_ARM" as per Will Deacon
>
> Changes in v3:
> - change property name to arm,cntvoff-not-fw-configured and specify
>   that the value of CNTHCTL.PL1PC(T)EN must still be the reset value
>   of 1 as per Mark Rutland
>
> Changes in v4:
> - change property name to arm,cpu-registers-not-fw-configured and
>   specify that all cpu registers must have architected reset values
>   per Mark Rutland
> - change from "#ifdef CONFIG_ARM" to "if (IS_ENABLED(CONFIG_ARM))" per
>   Arnd Bergmann
> ---
>  Documentation/devicetree/bindings/arm/arch_timer.txt | 8 ++++++++
>  drivers/clocksource/arm_arch_timer.c                 | 8 ++++++++
>  2 files changed, 16 insertions(+)

Do you know what the status of this patch is?  This patch and Sonny's
patch at <https://patchwork.kernel.org/patch/5051901/> are needed on
Rockchip rk3288 for some specific things:

1. To make SMP happy with coreboot.

2. To (I assume) make SMP happy after S2R (no matter which firmware is
used since I don't think anyone has PSCI for rk3288).

...we still need a DTS entry atop these patches, but that's trivial to
add once these land.

Thanks!
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4] clocksource: arch_timer: Allow the device tree to specify uninitialized timer registers

[Doug Anderson]

Daniel,

On Wed, Oct 8, 2014 at 12:33 AM, Sonny Rao <sonnyrao@chromium.org> wrote:
> From: Doug Anderson <dianders@chromium.org>
>
> Some 32-bit (ARMv7) systems are architected like this:
>
> * The firmware doesn't know and doesn't care about hypervisor mode and
>   we don't want to add the complexity of hypervisor there.
>
> * The firmware isn't involved in SMP bringup or resume.
>
> * The ARCH timer come up with an uninitialized offset (CNTVOFF)
>   between the virtual and physical counters.  Each core gets a
>   different random offset.
>
> * The device boots in "Secure SVC" mode.
>
> * Nothing has touched the reset value of CNTHCTL.PL1PCEN or
>   CNTHCTL.PL1PCTEN (both default to 1 at reset)
>
> On systems like the above, it doesn't make sense to use the virtual
> counter.  There's nobody managing the offset and each time a core goes
> down and comes back up it will get reinitialized to some other random
> value.
>
> This adds an optional property which can inform the kernel of this
> situation, and firmware is free to remove the property if it is going
> to initialize the CNTVOFF registers when each CPU comes out of reset.
>
> Currently, the best course of action in this case is to use the
> physical timer, which is why it is important that CNTHCTL hasn't been
> changed from its reset value and it's a reasonable assumption given
> that the firmware has never entered HYP mode.
>
> Note that it's been said that on ARMv8 systems the firmware and
> kernel really can't be architected as described above.  That means
> using the physical timer like this really only makes sense for ARMv7
> systems.
>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> Signed-off-by: Sonny Rao <sonnyrao@chromium.org>
> Reviewed-by: Mark Rutland <mark.rutland@arm.com>
> ---
> Changes in v2:
> - Add "#ifdef CONFIG_ARM" as per Will Deacon
>
> Changes in v3:
> - change property name to arm,cntvoff-not-fw-configured and specify
>   that the value of CNTHCTL.PL1PC(T)EN must still be the reset value
>   of 1 as per Mark Rutland
>
> Changes in v4:
> - change property name to arm,cpu-registers-not-fw-configured and
>   specify that all cpu registers must have architected reset values
>   per Mark Rutland
> - change from "#ifdef CONFIG_ARM" to "if (IS_ENABLED(CONFIG_ARM))" per
>   Arnd Bergmann
> ---
>  Documentation/devicetree/bindings/arm/arch_timer.txt | 8 ++++++++
>  drivers/clocksource/arm_arch_timer.c                 | 8 ++++++++
>  2 files changed, 16 insertions(+)

Do you know what the status of this patch is?  This patch and Sonny's
patch at <https://patchwork.kernel.org/patch/5051901/> are needed on
Rockchip rk3288 for some specific things:

1. To make SMP happy with coreboot.

2. To (I assume) make SMP happy after S2R (no matter which firmware is
used since I don't think anyone has PSCI for rk3288).

...we still need a DTS entry atop these patches, but that's trivial to
add once these land.

Thanks!

Re: [PATCH v4] clocksource: arch_timer: Allow the device tree to specify uninitialized timer registers

[Daniel Lezcano]

On 11/20/2014 12:01 AM, Doug Anderson wrote:
> Daniel,
>
> On Wed, Oct 8, 2014 at 12:33 AM, Sonny Rao <sonnyrao@chromium.org> wrote:
>> From: Doug Anderson <dianders@chromium.org>
>>
>> Some 32-bit (ARMv7) systems are architected like this:
>>
>> * The firmware doesn't know and doesn't care about hypervisor mode and
>>    we don't want to add the complexity of hypervisor there.
>>
>> * The firmware isn't involved in SMP bringup or resume.
>>
>> * The ARCH timer come up with an uninitialized offset (CNTVOFF)
>>    between the virtual and physical counters.  Each core gets a
>>    different random offset.
>>
>> * The device boots in "Secure SVC" mode.
>>
>> * Nothing has touched the reset value of CNTHCTL.PL1PCEN or
>>    CNTHCTL.PL1PCTEN (both default to 1 at reset)
>>
>> On systems like the above, it doesn't make sense to use the virtual
>> counter.  There's nobody managing the offset and each time a core goes
>> down and comes back up it will get reinitialized to some other random
>> value.
>>
>> This adds an optional property which can inform the kernel of this
>> situation, and firmware is free to remove the property if it is going
>> to initialize the CNTVOFF registers when each CPU comes out of reset.
>>
>> Currently, the best course of action in this case is to use the
>> physical timer, which is why it is important that CNTHCTL hasn't been
>> changed from its reset value and it's a reasonable assumption given
>> that the firmware has never entered HYP mode.
>>
>> Note that it's been said that on ARMv8 systems the firmware and
>> kernel really can't be architected as described above.  That means
>> using the physical timer like this really only makes sense for ARMv7
>> systems.
>>
>> Signed-off-by: Doug Anderson <dianders@chromium.org>
>> Signed-off-by: Sonny Rao <sonnyrao@chromium.org>
>> Reviewed-by: Mark Rutland <mark.rutland@arm.com>
>> ---
>> Changes in v2:
>> - Add "#ifdef CONFIG_ARM" as per Will Deacon
>>
>> Changes in v3:
>> - change property name to arm,cntvoff-not-fw-configured and specify
>>    that the value of CNTHCTL.PL1PC(T)EN must still be the reset value
>>    of 1 as per Mark Rutland
>>
>> Changes in v4:
>> - change property name to arm,cpu-registers-not-fw-configured and
>>    specify that all cpu registers must have architected reset values
>>    per Mark Rutland
>> - change from "#ifdef CONFIG_ARM" to "if (IS_ENABLED(CONFIG_ARM))" per
>>    Arnd Bergmann
>> ---
>>   Documentation/devicetree/bindings/arm/arch_timer.txt | 8 ++++++++
>>   drivers/clocksource/arm_arch_timer.c                 | 8 ++++++++
>>   2 files changed, 16 insertions(+)
>
> Do you know what the status of this patch is?  This patch and Sonny's
> patch at <https://patchwork.kernel.org/patch/5051901/> are needed on
> Rockchip rk3288 for some specific things:
>
> 1. To make SMP happy with coreboot.
>
> 2. To (I assume) make SMP happy after S2R (no matter which firmware is
> used since I don't think anyone has PSCI for rk3288).
>
> ...we still need a DTS entry atop these patches, but that's trivial to
> add once these land.

Hi Doug,

sounds like I forgot it. Thanks for the heads up.

Applied for 3.19.

   -- Daniel


-- 
  <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH v4] clocksource: arch_timer: Allow the device tree to specify uninitialized timer registers

[Daniel Lezcano]

On 11/20/2014 12:01 AM, Doug Anderson wrote:
> Daniel,
>
> On Wed, Oct 8, 2014 at 12:33 AM, Sonny Rao <sonnyrao-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
>> From: Doug Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
>>
>> Some 32-bit (ARMv7) systems are architected like this:
>>
>> * The firmware doesn't know and doesn't care about hypervisor mode and
>>    we don't want to add the complexity of hypervisor there.
>>
>> * The firmware isn't involved in SMP bringup or resume.
>>
>> * The ARCH timer come up with an uninitialized offset (CNTVOFF)
>>    between the virtual and physical counters.  Each core gets a
>>    different random offset.
>>
>> * The device boots in "Secure SVC" mode.
>>
>> * Nothing has touched the reset value of CNTHCTL.PL1PCEN or
>>    CNTHCTL.PL1PCTEN (both default to 1 at reset)
>>
>> On systems like the above, it doesn't make sense to use the virtual
>> counter.  There's nobody managing the offset and each time a core goes
>> down and comes back up it will get reinitialized to some other random
>> value.
>>
>> This adds an optional property which can inform the kernel of this
>> situation, and firmware is free to remove the property if it is going
>> to initialize the CNTVOFF registers when each CPU comes out of reset.
>>
>> Currently, the best course of action in this case is to use the
>> physical timer, which is why it is important that CNTHCTL hasn't been
>> changed from its reset value and it's a reasonable assumption given
>> that the firmware has never entered HYP mode.
>>
>> Note that it's been said that on ARMv8 systems the firmware and
>> kernel really can't be architected as described above.  That means
>> using the physical timer like this really only makes sense for ARMv7
>> systems.
>>
>> Signed-off-by: Doug Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
>> Signed-off-by: Sonny Rao <sonnyrao-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
>> Reviewed-by: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
>> ---
>> Changes in v2:
>> - Add "#ifdef CONFIG_ARM" as per Will Deacon
>>
>> Changes in v3:
>> - change property name to arm,cntvoff-not-fw-configured and specify
>>    that the value of CNTHCTL.PL1PC(T)EN must still be the reset value
>>    of 1 as per Mark Rutland
>>
>> Changes in v4:
>> - change property name to arm,cpu-registers-not-fw-configured and
>>    specify that all cpu registers must have architected reset values
>>    per Mark Rutland
>> - change from "#ifdef CONFIG_ARM" to "if (IS_ENABLED(CONFIG_ARM))" per
>>    Arnd Bergmann
>> ---
>>   Documentation/devicetree/bindings/arm/arch_timer.txt | 8 ++++++++
>>   drivers/clocksource/arm_arch_timer.c                 | 8 ++++++++
>>   2 files changed, 16 insertions(+)
>
> Do you know what the status of this patch is?  This patch and Sonny's
> patch at <https://patchwork.kernel.org/patch/5051901/> are needed on
> Rockchip rk3288 for some specific things:
>
> 1. To make SMP happy with coreboot.
>
> 2. To (I assume) make SMP happy after S2R (no matter which firmware is
> used since I don't think anyone has PSCI for rk3288).
>
> ...we still need a DTS entry atop these patches, but that's trivial to
> add once these land.

Hi Doug,

sounds like I forgot it. Thanks for the heads up.

Applied for 3.19.

   -- Daniel


-- 
  <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4] clocksource: arch_timer: Allow the device tree to specify uninitialized timer registers

[Daniel Lezcano]

On 11/20/2014 12:01 AM, Doug Anderson wrote:
> Daniel,
>
> On Wed, Oct 8, 2014 at 12:33 AM, Sonny Rao <sonnyrao@chromium.org> wrote:
>> From: Doug Anderson <dianders@chromium.org>
>>
>> Some 32-bit (ARMv7) systems are architected like this:
>>
>> * The firmware doesn't know and doesn't care about hypervisor mode and
>>    we don't want to add the complexity of hypervisor there.
>>
>> * The firmware isn't involved in SMP bringup or resume.
>>
>> * The ARCH timer come up with an uninitialized offset (CNTVOFF)
>>    between the virtual and physical counters.  Each core gets a
>>    different random offset.
>>
>> * The device boots in "Secure SVC" mode.
>>
>> * Nothing has touched the reset value of CNTHCTL.PL1PCEN or
>>    CNTHCTL.PL1PCTEN (both default to 1 at reset)
>>
>> On systems like the above, it doesn't make sense to use the virtual
>> counter.  There's nobody managing the offset and each time a core goes
>> down and comes back up it will get reinitialized to some other random
>> value.
>>
>> This adds an optional property which can inform the kernel of this
>> situation, and firmware is free to remove the property if it is going
>> to initialize the CNTVOFF registers when each CPU comes out of reset.
>>
>> Currently, the best course of action in this case is to use the
>> physical timer, which is why it is important that CNTHCTL hasn't been
>> changed from its reset value and it's a reasonable assumption given
>> that the firmware has never entered HYP mode.
>>
>> Note that it's been said that on ARMv8 systems the firmware and
>> kernel really can't be architected as described above.  That means
>> using the physical timer like this really only makes sense for ARMv7
>> systems.
>>
>> Signed-off-by: Doug Anderson <dianders@chromium.org>
>> Signed-off-by: Sonny Rao <sonnyrao@chromium.org>
>> Reviewed-by: Mark Rutland <mark.rutland@arm.com>
>> ---
>> Changes in v2:
>> - Add "#ifdef CONFIG_ARM" as per Will Deacon
>>
>> Changes in v3:
>> - change property name to arm,cntvoff-not-fw-configured and specify
>>    that the value of CNTHCTL.PL1PC(T)EN must still be the reset value
>>    of 1 as per Mark Rutland
>>
>> Changes in v4:
>> - change property name to arm,cpu-registers-not-fw-configured and
>>    specify that all cpu registers must have architected reset values
>>    per Mark Rutland
>> - change from "#ifdef CONFIG_ARM" to "if (IS_ENABLED(CONFIG_ARM))" per
>>    Arnd Bergmann
>> ---
>>   Documentation/devicetree/bindings/arm/arch_timer.txt | 8 ++++++++
>>   drivers/clocksource/arm_arch_timer.c                 | 8 ++++++++
>>   2 files changed, 16 insertions(+)
>
> Do you know what the status of this patch is?  This patch and Sonny's
> patch at <https://patchwork.kernel.org/patch/5051901/> are needed on
> Rockchip rk3288 for some specific things:
>
> 1. To make SMP happy with coreboot.
>
> 2. To (I assume) make SMP happy after S2R (no matter which firmware is
> used since I don't think anyone has PSCI for rk3288).
>
> ...we still need a DTS entry atop these patches, but that's trivial to
> add once these land.

Hi Doug,

sounds like I forgot it. Thanks for the heads up.

Applied for 3.19.

   -- Daniel


-- 
  <http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH v4] clocksource: arch_timer: Allow the device tree to specify uninitialized timer registers

[Daniel Lezcano]

Hi Doug, Olof,

IIUC, it sounds like this patch is needed from some other patches in 
arm-soc. Olof was proposing to take this patch through its tree to 
facilitate the integration.

Olof, is it this patch you were worried about ?

Thanks

   -- Daniel

On 11/20/2014 12:01 AM, Doug Anderson wrote:
> Daniel,
>
> On Wed, Oct 8, 2014 at 12:33 AM, Sonny Rao <sonnyrao@chromium.org> wrote:
>> From: Doug Anderson <dianders@chromium.org>
>>
>> Some 32-bit (ARMv7) systems are architected like this:
>>
>> * The firmware doesn't know and doesn't care about hypervisor mode and
>>    we don't want to add the complexity of hypervisor there.
>>
>> * The firmware isn't involved in SMP bringup or resume.
>>
>> * The ARCH timer come up with an uninitialized offset (CNTVOFF)
>>    between the virtual and physical counters.  Each core gets a
>>    different random offset.
>>
>> * The device boots in "Secure SVC" mode.
>>
>> * Nothing has touched the reset value of CNTHCTL.PL1PCEN or
>>    CNTHCTL.PL1PCTEN (both default to 1 at reset)
>>
>> On systems like the above, it doesn't make sense to use the virtual
>> counter.  There's nobody managing the offset and each time a core goes
>> down and comes back up it will get reinitialized to some other random
>> value.
>>
>> This adds an optional property which can inform the kernel of this
>> situation, and firmware is free to remove the property if it is going
>> to initialize the CNTVOFF registers when each CPU comes out of reset.
>>
>> Currently, the best course of action in this case is to use the
>> physical timer, which is why it is important that CNTHCTL hasn't been
>> changed from its reset value and it's a reasonable assumption given
>> that the firmware has never entered HYP mode.
>>
>> Note that it's been said that on ARMv8 systems the firmware and
>> kernel really can't be architected as described above.  That means
>> using the physical timer like this really only makes sense for ARMv7
>> systems.
>>
>> Signed-off-by: Doug Anderson <dianders@chromium.org>
>> Signed-off-by: Sonny Rao <sonnyrao@chromium.org>
>> Reviewed-by: Mark Rutland <mark.rutland@arm.com>
>> ---
>> Changes in v2:
>> - Add "#ifdef CONFIG_ARM" as per Will Deacon
>>
>> Changes in v3:
>> - change property name to arm,cntvoff-not-fw-configured and specify
>>    that the value of CNTHCTL.PL1PC(T)EN must still be the reset value
>>    of 1 as per Mark Rutland
>>
>> Changes in v4:
>> - change property name to arm,cpu-registers-not-fw-configured and
>>    specify that all cpu registers must have architected reset values
>>    per Mark Rutland
>> - change from "#ifdef CONFIG_ARM" to "if (IS_ENABLED(CONFIG_ARM))" per
>>    Arnd Bergmann
>> ---
>>   Documentation/devicetree/bindings/arm/arch_timer.txt | 8 ++++++++
>>   drivers/clocksource/arm_arch_timer.c                 | 8 ++++++++
>>   2 files changed, 16 insertions(+)
>
> Do you know what the status of this patch is?  This patch and Sonny's
> patch at <https://patchwork.kernel.org/patch/5051901/> are needed on
> Rockchip rk3288 for some specific things:
>
> 1. To make SMP happy with coreboot.
>
> 2. To (I assume) make SMP happy after S2R (no matter which firmware is
> used since I don't think anyone has PSCI for rk3288).
>
> ...we still need a DTS entry atop these patches, but that's trivial to
> add once these land.
>
> Thanks!
>


-- 
  <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

[PATCH v4] clocksource: arch_timer: Allow the device tree to specify uninitialized timer registers

[Daniel Lezcano]

Hi Doug, Olof,

IIUC, it sounds like this patch is needed from some other patches in 
arm-soc. Olof was proposing to take this patch through its tree to 
facilitate the integration.

Olof, is it this patch you were worried about ?

Thanks

   -- Daniel

On 11/20/2014 12:01 AM, Doug Anderson wrote:
> Daniel,
>
> On Wed, Oct 8, 2014 at 12:33 AM, Sonny Rao <sonnyrao@chromium.org> wrote:
>> From: Doug Anderson <dianders@chromium.org>
>>
>> Some 32-bit (ARMv7) systems are architected like this:
>>
>> * The firmware doesn't know and doesn't care about hypervisor mode and
>>    we don't want to add the complexity of hypervisor there.
>>
>> * The firmware isn't involved in SMP bringup or resume.
>>
>> * The ARCH timer come up with an uninitialized offset (CNTVOFF)
>>    between the virtual and physical counters.  Each core gets a
>>    different random offset.
>>
>> * The device boots in "Secure SVC" mode.
>>
>> * Nothing has touched the reset value of CNTHCTL.PL1PCEN or
>>    CNTHCTL.PL1PCTEN (both default to 1 at reset)
>>
>> On systems like the above, it doesn't make sense to use the virtual
>> counter.  There's nobody managing the offset and each time a core goes
>> down and comes back up it will get reinitialized to some other random
>> value.
>>
>> This adds an optional property which can inform the kernel of this
>> situation, and firmware is free to remove the property if it is going
>> to initialize the CNTVOFF registers when each CPU comes out of reset.
>>
>> Currently, the best course of action in this case is to use the
>> physical timer, which is why it is important that CNTHCTL hasn't been
>> changed from its reset value and it's a reasonable assumption given
>> that the firmware has never entered HYP mode.
>>
>> Note that it's been said that on ARMv8 systems the firmware and
>> kernel really can't be architected as described above.  That means
>> using the physical timer like this really only makes sense for ARMv7
>> systems.
>>
>> Signed-off-by: Doug Anderson <dianders@chromium.org>
>> Signed-off-by: Sonny Rao <sonnyrao@chromium.org>
>> Reviewed-by: Mark Rutland <mark.rutland@arm.com>
>> ---
>> Changes in v2:
>> - Add "#ifdef CONFIG_ARM" as per Will Deacon
>>
>> Changes in v3:
>> - change property name to arm,cntvoff-not-fw-configured and specify
>>    that the value of CNTHCTL.PL1PC(T)EN must still be the reset value
>>    of 1 as per Mark Rutland
>>
>> Changes in v4:
>> - change property name to arm,cpu-registers-not-fw-configured and
>>    specify that all cpu registers must have architected reset values
>>    per Mark Rutland
>> - change from "#ifdef CONFIG_ARM" to "if (IS_ENABLED(CONFIG_ARM))" per
>>    Arnd Bergmann
>> ---
>>   Documentation/devicetree/bindings/arm/arch_timer.txt | 8 ++++++++
>>   drivers/clocksource/arm_arch_timer.c                 | 8 ++++++++
>>   2 files changed, 16 insertions(+)
>
> Do you know what the status of this patch is?  This patch and Sonny's
> patch at <https://patchwork.kernel.org/patch/5051901/> are needed on
> Rockchip rk3288 for some specific things:
>
> 1. To make SMP happy with coreboot.
>
> 2. To (I assume) make SMP happy after S2R (no matter which firmware is
> used since I don't think anyone has PSCI for rk3288).
>
> ...we still need a DTS entry atop these patches, but that's trivial to
> add once these land.
>
> Thanks!
>


-- 
  <http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH v4] clocksource: arch_timer: Allow the device tree to specify uninitialized timer registers

[Heiko Stübner]

Hi Daniel,

Am Mittwoch, 26. November 2014, 12:51:08 schrieb Daniel Lezcano:
> Hi Doug, Olof,
> 
> IIUC, it sounds like this patch is needed from some other patches in
> arm-soc. Olof was proposing to take this patch through its tree to
> facilitate the integration.
> 
> Olof, is it this patch you were worried about ?

I think this is one of two patches in question.

"clocksource: arch_timer: Fix code to use physical timers when requested" [0] 
would be the second one.

And the patch for arm-soc that Olof means would be "ARM: dts: rk3288: add 
arm,cpu-registers-not-fw-configured" [1].


Heiko

[0] 
https://git.linaro.org/people/daniel.lezcano/linux.git/commit/04f71c2cae54dc26b2a236c787ea8d56c174150b
[1] https://lkml.org/lkml/2014/11/25/975

> 
> Thanks
> 
>    -- Daniel
> 
> On 11/20/2014 12:01 AM, Doug Anderson wrote:
> > Daniel,
> > 
> > On Wed, Oct 8, 2014 at 12:33 AM, Sonny Rao <sonnyrao@chromium.org> wrote:
> >> From: Doug Anderson <dianders@chromium.org>
> >> 
> >> Some 32-bit (ARMv7) systems are architected like this:
> >> 
> >> * The firmware doesn't know and doesn't care about hypervisor mode and
> >> 
> >>    we don't want to add the complexity of hypervisor there.
> >> 
> >> * The firmware isn't involved in SMP bringup or resume.
> >> 
> >> * The ARCH timer come up with an uninitialized offset (CNTVOFF)
> >> 
> >>    between the virtual and physical counters.  Each core gets a
> >>    different random offset.
> >> 
> >> * The device boots in "Secure SVC" mode.
> >> 
> >> * Nothing has touched the reset value of CNTHCTL.PL1PCEN or
> >> 
> >>    CNTHCTL.PL1PCTEN (both default to 1 at reset)
> >> 
> >> On systems like the above, it doesn't make sense to use the virtual
> >> counter.  There's nobody managing the offset and each time a core goes
> >> down and comes back up it will get reinitialized to some other random
> >> value.
> >> 
> >> This adds an optional property which can inform the kernel of this
> >> situation, and firmware is free to remove the property if it is going
> >> to initialize the CNTVOFF registers when each CPU comes out of reset.
> >> 
> >> Currently, the best course of action in this case is to use the
> >> physical timer, which is why it is important that CNTHCTL hasn't been
> >> changed from its reset value and it's a reasonable assumption given
> >> that the firmware has never entered HYP mode.
> >> 
> >> Note that it's been said that on ARMv8 systems the firmware and
> >> kernel really can't be architected as described above.  That means
> >> using the physical timer like this really only makes sense for ARMv7
> >> systems.
> >> 
> >> Signed-off-by: Doug Anderson <dianders@chromium.org>
> >> Signed-off-by: Sonny Rao <sonnyrao@chromium.org>
> >> Reviewed-by: Mark Rutland <mark.rutland@arm.com>
> >> ---
> >> Changes in v2:
> >> - Add "#ifdef CONFIG_ARM" as per Will Deacon
> >> 
> >> Changes in v3:
> >> - change property name to arm,cntvoff-not-fw-configured and specify
> >> 
> >>    that the value of CNTHCTL.PL1PC(T)EN must still be the reset value
> >>    of 1 as per Mark Rutland
> >> 
> >> Changes in v4:
> >> - change property name to arm,cpu-registers-not-fw-configured and
> >> 
> >>    specify that all cpu registers must have architected reset values
> >>    per Mark Rutland
> >> 
> >> - change from "#ifdef CONFIG_ARM" to "if (IS_ENABLED(CONFIG_ARM))" per
> >> 
> >>    Arnd Bergmann
> >> 
> >> ---
> >> 
> >>   Documentation/devicetree/bindings/arm/arch_timer.txt | 8 ++++++++
> >>   drivers/clocksource/arm_arch_timer.c                 | 8 ++++++++
> >>   2 files changed, 16 insertions(+)
> > 
> > Do you know what the status of this patch is?  This patch and Sonny's
> > patch at <https://patchwork.kernel.org/patch/5051901/> are needed on
> > Rockchip rk3288 for some specific things:
> > 
> > 1. To make SMP happy with coreboot.
> > 
> > 2. To (I assume) make SMP happy after S2R (no matter which firmware is
> > used since I don't think anyone has PSCI for rk3288).
> > 
> > ...we still need a DTS entry atop these patches, but that's trivial to
> > add once these land.
> > 
> > Thanks!

Re: [PATCH v4] clocksource: arch_timer: Allow the device tree to specify uninitialized timer registers

[Heiko Stübner]

Hi Daniel,

Am Mittwoch, 26. November 2014, 12:51:08 schrieb Daniel Lezcano:
> Hi Doug, Olof,
> 
> IIUC, it sounds like this patch is needed from some other patches in
> arm-soc. Olof was proposing to take this patch through its tree to
> facilitate the integration.
> 
> Olof, is it this patch you were worried about ?

I think this is one of two patches in question.

"clocksource: arch_timer: Fix code to use physical timers when requested" [0] 
would be the second one.

And the patch for arm-soc that Olof means would be "ARM: dts: rk3288: add 
arm,cpu-registers-not-fw-configured" [1].


Heiko

[0] 
https://git.linaro.org/people/daniel.lezcano/linux.git/commit/04f71c2cae54dc26b2a236c787ea8d56c174150b
[1] https://lkml.org/lkml/2014/11/25/975

> 
> Thanks
> 
>    -- Daniel
> 
> On 11/20/2014 12:01 AM, Doug Anderson wrote:
> > Daniel,
> > 
> > On Wed, Oct 8, 2014 at 12:33 AM, Sonny Rao <sonnyrao@chromium.org> wrote:
> >> From: Doug Anderson <dianders@chromium.org>
> >> 
> >> Some 32-bit (ARMv7) systems are architected like this:
> >> 
> >> * The firmware doesn't know and doesn't care about hypervisor mode and
> >> 
> >>    we don't want to add the complexity of hypervisor there.
> >> 
> >> * The firmware isn't involved in SMP bringup or resume.
> >> 
> >> * The ARCH timer come up with an uninitialized offset (CNTVOFF)
> >> 
> >>    between the virtual and physical counters.  Each core gets a
> >>    different random offset.
> >> 
> >> * The device boots in "Secure SVC" mode.
> >> 
> >> * Nothing has touched the reset value of CNTHCTL.PL1PCEN or
> >> 
> >>    CNTHCTL.PL1PCTEN (both default to 1 at reset)
> >> 
> >> On systems like the above, it doesn't make sense to use the virtual
> >> counter.  There's nobody managing the offset and each time a core goes
> >> down and comes back up it will get reinitialized to some other random
> >> value.
> >> 
> >> This adds an optional property which can inform the kernel of this
> >> situation, and firmware is free to remove the property if it is going
> >> to initialize the CNTVOFF registers when each CPU comes out of reset.
> >> 
> >> Currently, the best course of action in this case is to use the
> >> physical timer, which is why it is important that CNTHCTL hasn't been
> >> changed from its reset value and it's a reasonable assumption given
> >> that the firmware has never entered HYP mode.
> >> 
> >> Note that it's been said that on ARMv8 systems the firmware and
> >> kernel really can't be architected as described above.  That means
> >> using the physical timer like this really only makes sense for ARMv7
> >> systems.
> >> 
> >> Signed-off-by: Doug Anderson <dianders@chromium.org>
> >> Signed-off-by: Sonny Rao <sonnyrao@chromium.org>
> >> Reviewed-by: Mark Rutland <mark.rutland@arm.com>
> >> ---
> >> Changes in v2:
> >> - Add "#ifdef CONFIG_ARM" as per Will Deacon
> >> 
> >> Changes in v3:
> >> - change property name to arm,cntvoff-not-fw-configured and specify
> >> 
> >>    that the value of CNTHCTL.PL1PC(T)EN must still be the reset value
> >>    of 1 as per Mark Rutland
> >> 
> >> Changes in v4:
> >> - change property name to arm,cpu-registers-not-fw-configured and
> >> 
> >>    specify that all cpu registers must have architected reset values
> >>    per Mark Rutland
> >> 
> >> - change from "#ifdef CONFIG_ARM" to "if (IS_ENABLED(CONFIG_ARM))" per
> >> 
> >>    Arnd Bergmann
> >> 
> >> ---
> >> 
> >>   Documentation/devicetree/bindings/arm/arch_timer.txt | 8 ++++++++
> >>   drivers/clocksource/arm_arch_timer.c                 | 8 ++++++++
> >>   2 files changed, 16 insertions(+)
> > 
> > Do you know what the status of this patch is?  This patch and Sonny's
> > patch at <https://patchwork.kernel.org/patch/5051901/> are needed on
> > Rockchip rk3288 for some specific things:
> > 
> > 1. To make SMP happy with coreboot.
> > 
> > 2. To (I assume) make SMP happy after S2R (no matter which firmware is
> > used since I don't think anyone has PSCI for rk3288).
> > 
> > ...we still need a DTS entry atop these patches, but that's trivial to
> > add once these land.
> > 
> > Thanks!

[PATCH v4] clocksource: arch_timer: Allow the device tree to specify uninitialized timer registers

[Heiko Stübner]

Hi Daniel,

Am Mittwoch, 26. November 2014, 12:51:08 schrieb Daniel Lezcano:
> Hi Doug, Olof,
> 
> IIUC, it sounds like this patch is needed from some other patches in
> arm-soc. Olof was proposing to take this patch through its tree to
> facilitate the integration.
> 
> Olof, is it this patch you were worried about ?

I think this is one of two patches in question.

"clocksource: arch_timer: Fix code to use physical timers when requested" [0] 
would be the second one.

And the patch for arm-soc that Olof means would be "ARM: dts: rk3288: add 
arm,cpu-registers-not-fw-configured" [1].


Heiko

[0] 
https://git.linaro.org/people/daniel.lezcano/linux.git/commit/04f71c2cae54dc26b2a236c787ea8d56c174150b
[1] https://lkml.org/lkml/2014/11/25/975

> 
> Thanks
> 
>    -- Daniel
> 
> On 11/20/2014 12:01 AM, Doug Anderson wrote:
> > Daniel,
> > 
> > On Wed, Oct 8, 2014 at 12:33 AM, Sonny Rao <sonnyrao@chromium.org> wrote:
> >> From: Doug Anderson <dianders@chromium.org>
> >> 
> >> Some 32-bit (ARMv7) systems are architected like this:
> >> 
> >> * The firmware doesn't know and doesn't care about hypervisor mode and
> >> 
> >>    we don't want to add the complexity of hypervisor there.
> >> 
> >> * The firmware isn't involved in SMP bringup or resume.
> >> 
> >> * The ARCH timer come up with an uninitialized offset (CNTVOFF)
> >> 
> >>    between the virtual and physical counters.  Each core gets a
> >>    different random offset.
> >> 
> >> * The device boots in "Secure SVC" mode.
> >> 
> >> * Nothing has touched the reset value of CNTHCTL.PL1PCEN or
> >> 
> >>    CNTHCTL.PL1PCTEN (both default to 1 at reset)
> >> 
> >> On systems like the above, it doesn't make sense to use the virtual
> >> counter.  There's nobody managing the offset and each time a core goes
> >> down and comes back up it will get reinitialized to some other random
> >> value.
> >> 
> >> This adds an optional property which can inform the kernel of this
> >> situation, and firmware is free to remove the property if it is going
> >> to initialize the CNTVOFF registers when each CPU comes out of reset.
> >> 
> >> Currently, the best course of action in this case is to use the
> >> physical timer, which is why it is important that CNTHCTL hasn't been
> >> changed from its reset value and it's a reasonable assumption given
> >> that the firmware has never entered HYP mode.
> >> 
> >> Note that it's been said that on ARMv8 systems the firmware and
> >> kernel really can't be architected as described above.  That means
> >> using the physical timer like this really only makes sense for ARMv7
> >> systems.
> >> 
> >> Signed-off-by: Doug Anderson <dianders@chromium.org>
> >> Signed-off-by: Sonny Rao <sonnyrao@chromium.org>
> >> Reviewed-by: Mark Rutland <mark.rutland@arm.com>
> >> ---
> >> Changes in v2:
> >> - Add "#ifdef CONFIG_ARM" as per Will Deacon
> >> 
> >> Changes in v3:
> >> - change property name to arm,cntvoff-not-fw-configured and specify
> >> 
> >>    that the value of CNTHCTL.PL1PC(T)EN must still be the reset value
> >>    of 1 as per Mark Rutland
> >> 
> >> Changes in v4:
> >> - change property name to arm,cpu-registers-not-fw-configured and
> >> 
> >>    specify that all cpu registers must have architected reset values
> >>    per Mark Rutland
> >> 
> >> - change from "#ifdef CONFIG_ARM" to "if (IS_ENABLED(CONFIG_ARM))" per
> >> 
> >>    Arnd Bergmann
> >> 
> >> ---
> >> 
> >>   Documentation/devicetree/bindings/arm/arch_timer.txt | 8 ++++++++
> >>   drivers/clocksource/arm_arch_timer.c                 | 8 ++++++++
> >>   2 files changed, 16 insertions(+)
> > 
> > Do you know what the status of this patch is?  This patch and Sonny's
> > patch at <https://patchwork.kernel.org/patch/5051901/> are needed on
> > Rockchip rk3288 for some specific things:
> > 
> > 1. To make SMP happy with coreboot.
> > 
> > 2. To (I assume) make SMP happy after S2R (no matter which firmware is
> > used since I don't think anyone has PSCI for rk3288).
> > 
> > ...we still need a DTS entry atop these patches, but that's trivial to
> > add once these land.
> > 
> > Thanks!

Re: [PATCH v4] clocksource: arch_timer: Allow the device tree to specify uninitialized timer registers

[Daniel Lezcano]

On 11/26/2014 01:06 PM, Heiko Stübner wrote:
> Hi Daniel,
>
> Am Mittwoch, 26. November 2014, 12:51:08 schrieb Daniel Lezcano:
>> Hi Doug, Olof,
>>
>> IIUC, it sounds like this patch is needed from some other patches in
>> arm-soc. Olof was proposing to take this patch through its tree to
>> facilitate the integration.
>>
>> Olof, is it this patch you were worried about ?
>
> I think this is one of two patches in question.
>
> "clocksource: arch_timer: Fix code to use physical timers when requested" [0]
> would be the second one.
>
> And the patch for arm-soc that Olof means would be "ARM: dts: rk3288: add
> arm,cpu-registers-not-fw-configured" [1].

Ok, so IIUC, "clocksource: arch_timer: Fix code to use physical timers 
when requested" should go via arm's tree, right ?



> Heiko
>
> [0]
> https://git.linaro.org/people/daniel.lezcano/linux.git/commit/04f71c2cae54dc26b2a236c787ea8d56c174150b
> [1] https://lkml.org/lkml/2014/11/25/975
>
>>
>> Thanks
>>
>>     -- Daniel
>>
>> On 11/20/2014 12:01 AM, Doug Anderson wrote:
>>> Daniel,
>>>
>>> On Wed, Oct 8, 2014 at 12:33 AM, Sonny Rao <sonnyrao@chromium.org> wrote:
>>>> From: Doug Anderson <dianders@chromium.org>
>>>>
>>>> Some 32-bit (ARMv7) systems are architected like this:
>>>>
>>>> * The firmware doesn't know and doesn't care about hypervisor mode and
>>>>
>>>>     we don't want to add the complexity of hypervisor there.
>>>>
>>>> * The firmware isn't involved in SMP bringup or resume.
>>>>
>>>> * The ARCH timer come up with an uninitialized offset (CNTVOFF)
>>>>
>>>>     between the virtual and physical counters.  Each core gets a
>>>>     different random offset.
>>>>
>>>> * The device boots in "Secure SVC" mode.
>>>>
>>>> * Nothing has touched the reset value of CNTHCTL.PL1PCEN or
>>>>
>>>>     CNTHCTL.PL1PCTEN (both default to 1 at reset)
>>>>
>>>> On systems like the above, it doesn't make sense to use the virtual
>>>> counter.  There's nobody managing the offset and each time a core goes
>>>> down and comes back up it will get reinitialized to some other random
>>>> value.
>>>>
>>>> This adds an optional property which can inform the kernel of this
>>>> situation, and firmware is free to remove the property if it is going
>>>> to initialize the CNTVOFF registers when each CPU comes out of reset.
>>>>
>>>> Currently, the best course of action in this case is to use the
>>>> physical timer, which is why it is important that CNTHCTL hasn't been
>>>> changed from its reset value and it's a reasonable assumption given
>>>> that the firmware has never entered HYP mode.
>>>>
>>>> Note that it's been said that on ARMv8 systems the firmware and
>>>> kernel really can't be architected as described above.  That means
>>>> using the physical timer like this really only makes sense for ARMv7
>>>> systems.
>>>>
>>>> Signed-off-by: Doug Anderson <dianders@chromium.org>
>>>> Signed-off-by: Sonny Rao <sonnyrao@chromium.org>
>>>> Reviewed-by: Mark Rutland <mark.rutland@arm.com>
>>>> ---
>>>> Changes in v2:
>>>> - Add "#ifdef CONFIG_ARM" as per Will Deacon
>>>>
>>>> Changes in v3:
>>>> - change property name to arm,cntvoff-not-fw-configured and specify
>>>>
>>>>     that the value of CNTHCTL.PL1PC(T)EN must still be the reset value
>>>>     of 1 as per Mark Rutland
>>>>
>>>> Changes in v4:
>>>> - change property name to arm,cpu-registers-not-fw-configured and
>>>>
>>>>     specify that all cpu registers must have architected reset values
>>>>     per Mark Rutland
>>>>
>>>> - change from "#ifdef CONFIG_ARM" to "if (IS_ENABLED(CONFIG_ARM))" per
>>>>
>>>>     Arnd Bergmann
>>>>
>>>> ---
>>>>
>>>>    Documentation/devicetree/bindings/arm/arch_timer.txt | 8 ++++++++
>>>>    drivers/clocksource/arm_arch_timer.c                 | 8 ++++++++
>>>>    2 files changed, 16 insertions(+)
>>>
>>> Do you know what the status of this patch is?  This patch and Sonny's
>>> patch at <https://patchwork.kernel.org/patch/5051901/> are needed on
>>> Rockchip rk3288 for some specific things:
>>>
>>> 1. To make SMP happy with coreboot.
>>>
>>> 2. To (I assume) make SMP happy after S2R (no matter which firmware is
>>> used since I don't think anyone has PSCI for rk3288).
>>>
>>> ...we still need a DTS entry atop these patches, but that's trivial to
>>> add once these land.
>>>
>>> Thanks!
>


-- 
  <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH v4] clocksource: arch_timer: Allow the device tree to specify uninitialized timer registers

[Daniel Lezcano]

On 11/26/2014 01:06 PM, Heiko Stübner wrote:
> Hi Daniel,
>
> Am Mittwoch, 26. November 2014, 12:51:08 schrieb Daniel Lezcano:
>> Hi Doug, Olof,
>>
>> IIUC, it sounds like this patch is needed from some other patches in
>> arm-soc. Olof was proposing to take this patch through its tree to
>> facilitate the integration.
>>
>> Olof, is it this patch you were worried about ?
>
> I think this is one of two patches in question.
>
> "clocksource: arch_timer: Fix code to use physical timers when requested" [0]
> would be the second one.
>
> And the patch for arm-soc that Olof means would be "ARM: dts: rk3288: add
> arm,cpu-registers-not-fw-configured" [1].

Ok, so IIUC, "clocksource: arch_timer: Fix code to use physical timers 
when requested" should go via arm's tree, right ?



> Heiko
>
> [0]
> https://git.linaro.org/people/daniel.lezcano/linux.git/commit/04f71c2cae54dc26b2a236c787ea8d56c174150b
> [1] https://lkml.org/lkml/2014/11/25/975
>
>>
>> Thanks
>>
>>     -- Daniel
>>
>> On 11/20/2014 12:01 AM, Doug Anderson wrote:
>>> Daniel,
>>>
>>> On Wed, Oct 8, 2014 at 12:33 AM, Sonny Rao <sonnyrao@chromium.org> wrote:
>>>> From: Doug Anderson <dianders@chromium.org>
>>>>
>>>> Some 32-bit (ARMv7) systems are architected like this:
>>>>
>>>> * The firmware doesn't know and doesn't care about hypervisor mode and
>>>>
>>>>     we don't want to add the complexity of hypervisor there.
>>>>
>>>> * The firmware isn't involved in SMP bringup or resume.
>>>>
>>>> * The ARCH timer come up with an uninitialized offset (CNTVOFF)
>>>>
>>>>     between the virtual and physical counters.  Each core gets a
>>>>     different random offset.
>>>>
>>>> * The device boots in "Secure SVC" mode.
>>>>
>>>> * Nothing has touched the reset value of CNTHCTL.PL1PCEN or
>>>>
>>>>     CNTHCTL.PL1PCTEN (both default to 1 at reset)
>>>>
>>>> On systems like the above, it doesn't make sense to use the virtual
>>>> counter.  There's nobody managing the offset and each time a core goes
>>>> down and comes back up it will get reinitialized to some other random
>>>> value.
>>>>
>>>> This adds an optional property which can inform the kernel of this
>>>> situation, and firmware is free to remove the property if it is going
>>>> to initialize the CNTVOFF registers when each CPU comes out of reset.
>>>>
>>>> Currently, the best course of action in this case is to use the
>>>> physical timer, which is why it is important that CNTHCTL hasn't been
>>>> changed from its reset value and it's a reasonable assumption given
>>>> that the firmware has never entered HYP mode.
>>>>
>>>> Note that it's been said that on ARMv8 systems the firmware and
>>>> kernel really can't be architected as described above.  That means
>>>> using the physical timer like this really only makes sense for ARMv7
>>>> systems.
>>>>
>>>> Signed-off-by: Doug Anderson <dianders@chromium.org>
>>>> Signed-off-by: Sonny Rao <sonnyrao@chromium.org>
>>>> Reviewed-by: Mark Rutland <mark.rutland@arm.com>
>>>> ---
>>>> Changes in v2:
>>>> - Add "#ifdef CONFIG_ARM" as per Will Deacon
>>>>
>>>> Changes in v3:
>>>> - change property name to arm,cntvoff-not-fw-configured and specify
>>>>
>>>>     that the value of CNTHCTL.PL1PC(T)EN must still be the reset value
>>>>     of 1 as per Mark Rutland
>>>>
>>>> Changes in v4:
>>>> - change property name to arm,cpu-registers-not-fw-configured and
>>>>
>>>>     specify that all cpu registers must have architected reset values
>>>>     per Mark Rutland
>>>>
>>>> - change from "#ifdef CONFIG_ARM" to "if (IS_ENABLED(CONFIG_ARM))" per
>>>>
>>>>     Arnd Bergmann
>>>>
>>>> ---
>>>>
>>>>    Documentation/devicetree/bindings/arm/arch_timer.txt | 8 ++++++++
>>>>    drivers/clocksource/arm_arch_timer.c                 | 8 ++++++++
>>>>    2 files changed, 16 insertions(+)
>>>
>>> Do you know what the status of this patch is?  This patch and Sonny's
>>> patch at <https://patchwork.kernel.org/patch/5051901/> are needed on
>>> Rockchip rk3288 for some specific things:
>>>
>>> 1. To make SMP happy with coreboot.
>>>
>>> 2. To (I assume) make SMP happy after S2R (no matter which firmware is
>>> used since I don't think anyone has PSCI for rk3288).
>>>
>>> ...we still need a DTS entry atop these patches, but that's trivial to
>>> add once these land.
>>>
>>> Thanks!
>


-- 
  <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

[PATCH v4] clocksource: arch_timer: Allow the device tree to specify uninitialized timer registers

[Daniel Lezcano]

On 11/26/2014 01:06 PM, Heiko St?bner wrote:
> Hi Daniel,
>
> Am Mittwoch, 26. November 2014, 12:51:08 schrieb Daniel Lezcano:
>> Hi Doug, Olof,
>>
>> IIUC, it sounds like this patch is needed from some other patches in
>> arm-soc. Olof was proposing to take this patch through its tree to
>> facilitate the integration.
>>
>> Olof, is it this patch you were worried about ?
>
> I think this is one of two patches in question.
>
> "clocksource: arch_timer: Fix code to use physical timers when requested" [0]
> would be the second one.
>
> And the patch for arm-soc that Olof means would be "ARM: dts: rk3288: add
> arm,cpu-registers-not-fw-configured" [1].

Ok, so IIUC, "clocksource: arch_timer: Fix code to use physical timers 
when requested" should go via arm's tree, right ?



> Heiko
>
> [0]
> https://git.linaro.org/people/daniel.lezcano/linux.git/commit/04f71c2cae54dc26b2a236c787ea8d56c174150b
> [1] https://lkml.org/lkml/2014/11/25/975
>
>>
>> Thanks
>>
>>     -- Daniel
>>
>> On 11/20/2014 12:01 AM, Doug Anderson wrote:
>>> Daniel,
>>>
>>> On Wed, Oct 8, 2014 at 12:33 AM, Sonny Rao <sonnyrao@chromium.org> wrote:
>>>> From: Doug Anderson <dianders@chromium.org>
>>>>
>>>> Some 32-bit (ARMv7) systems are architected like this:
>>>>
>>>> * The firmware doesn't know and doesn't care about hypervisor mode and
>>>>
>>>>     we don't want to add the complexity of hypervisor there.
>>>>
>>>> * The firmware isn't involved in SMP bringup or resume.
>>>>
>>>> * The ARCH timer come up with an uninitialized offset (CNTVOFF)
>>>>
>>>>     between the virtual and physical counters.  Each core gets a
>>>>     different random offset.
>>>>
>>>> * The device boots in "Secure SVC" mode.
>>>>
>>>> * Nothing has touched the reset value of CNTHCTL.PL1PCEN or
>>>>
>>>>     CNTHCTL.PL1PCTEN (both default to 1 at reset)
>>>>
>>>> On systems like the above, it doesn't make sense to use the virtual
>>>> counter.  There's nobody managing the offset and each time a core goes
>>>> down and comes back up it will get reinitialized to some other random
>>>> value.
>>>>
>>>> This adds an optional property which can inform the kernel of this
>>>> situation, and firmware is free to remove the property if it is going
>>>> to initialize the CNTVOFF registers when each CPU comes out of reset.
>>>>
>>>> Currently, the best course of action in this case is to use the
>>>> physical timer, which is why it is important that CNTHCTL hasn't been
>>>> changed from its reset value and it's a reasonable assumption given
>>>> that the firmware has never entered HYP mode.
>>>>
>>>> Note that it's been said that on ARMv8 systems the firmware and
>>>> kernel really can't be architected as described above.  That means
>>>> using the physical timer like this really only makes sense for ARMv7
>>>> systems.
>>>>
>>>> Signed-off-by: Doug Anderson <dianders@chromium.org>
>>>> Signed-off-by: Sonny Rao <sonnyrao@chromium.org>
>>>> Reviewed-by: Mark Rutland <mark.rutland@arm.com>
>>>> ---
>>>> Changes in v2:
>>>> - Add "#ifdef CONFIG_ARM" as per Will Deacon
>>>>
>>>> Changes in v3:
>>>> - change property name to arm,cntvoff-not-fw-configured and specify
>>>>
>>>>     that the value of CNTHCTL.PL1PC(T)EN must still be the reset value
>>>>     of 1 as per Mark Rutland
>>>>
>>>> Changes in v4:
>>>> - change property name to arm,cpu-registers-not-fw-configured and
>>>>
>>>>     specify that all cpu registers must have architected reset values
>>>>     per Mark Rutland
>>>>
>>>> - change from "#ifdef CONFIG_ARM" to "if (IS_ENABLED(CONFIG_ARM))" per
>>>>
>>>>     Arnd Bergmann
>>>>
>>>> ---
>>>>
>>>>    Documentation/devicetree/bindings/arm/arch_timer.txt | 8 ++++++++
>>>>    drivers/clocksource/arm_arch_timer.c                 | 8 ++++++++
>>>>    2 files changed, 16 insertions(+)
>>>
>>> Do you know what the status of this patch is?  This patch and Sonny's
>>> patch at <https://patchwork.kernel.org/patch/5051901/> are needed on
>>> Rockchip rk3288 for some specific things:
>>>
>>> 1. To make SMP happy with coreboot.
>>>
>>> 2. To (I assume) make SMP happy after S2R (no matter which firmware is
>>> used since I don't think anyone has PSCI for rk3288).
>>>
>>> ...we still need a DTS entry atop these patches, but that's trivial to
>>> add once these land.
>>>
>>> Thanks!
>


-- 
  <http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH v4] clocksource: arch_timer: Allow the device tree to specify uninitialized timer registers

[Heiko Stübner]

Am Mittwoch, 26. November 2014, 13:30:58 schrieb Daniel Lezcano:
> On 11/26/2014 01:06 PM, Heiko Stübner wrote:
> > Hi Daniel,
> > 
> > Am Mittwoch, 26. November 2014, 12:51:08 schrieb Daniel Lezcano:
> >> Hi Doug, Olof,
> >> 
> >> IIUC, it sounds like this patch is needed from some other patches in
> >> arm-soc. Olof was proposing to take this patch through its tree to
> >> facilitate the integration.
> >> 
> >> Olof, is it this patch you were worried about ?
> > 
> > I think this is one of two patches in question.
> > 
> > "clocksource: arch_timer: Fix code to use physical timers when requested"
> > [0] would be the second one.
> > 
> > And the patch for arm-soc that Olof means would be "ARM: dts: rk3288: add
> > arm,cpu-registers-not-fw-configured" [1].
> 
> Ok, so IIUC, "clocksource: arch_timer: Fix code to use physical timers
> when requested" should go via arm's tree, right ?

If I'm reading Olof's irc-comments from yesterday correctly, that is right and 
the 3 patches should go in together:

- "clocksource: arch_timer: Fix code to use physical timers
  when requested" fixes the use of physical timers in general
- "clocksource: arch_timer: Allow the device tree to specify uninitialized
  timer registers" allows this to be set from dt
- "ARM: dts: rk3288: add arm,cpu-registers-not-fw-configured" enables this on
  rk3288


Heiko

Re: [PATCH v4] clocksource: arch_timer: Allow the device tree to specify uninitialized timer registers

[Heiko Stübner]

Am Mittwoch, 26. November 2014, 13:30:58 schrieb Daniel Lezcano:
> On 11/26/2014 01:06 PM, Heiko Stübner wrote:
> > Hi Daniel,
> > 
> > Am Mittwoch, 26. November 2014, 12:51:08 schrieb Daniel Lezcano:
> >> Hi Doug, Olof,
> >> 
> >> IIUC, it sounds like this patch is needed from some other patches in
> >> arm-soc. Olof was proposing to take this patch through its tree to
> >> facilitate the integration.
> >> 
> >> Olof, is it this patch you were worried about ?
> > 
> > I think this is one of two patches in question.
> > 
> > "clocksource: arch_timer: Fix code to use physical timers when requested"
> > [0] would be the second one.
> > 
> > And the patch for arm-soc that Olof means would be "ARM: dts: rk3288: add
> > arm,cpu-registers-not-fw-configured" [1].
> 
> Ok, so IIUC, "clocksource: arch_timer: Fix code to use physical timers
> when requested" should go via arm's tree, right ?

If I'm reading Olof's irc-comments from yesterday correctly, that is right and 
the 3 patches should go in together:

- "clocksource: arch_timer: Fix code to use physical timers
  when requested" fixes the use of physical timers in general
- "clocksource: arch_timer: Allow the device tree to specify uninitialized
  timer registers" allows this to be set from dt
- "ARM: dts: rk3288: add arm,cpu-registers-not-fw-configured" enables this on
  rk3288


Heiko
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4] clocksource: arch_timer: Allow the device tree to specify uninitialized timer registers

[Heiko Stübner]

Am Mittwoch, 26. November 2014, 13:30:58 schrieb Daniel Lezcano:
> On 11/26/2014 01:06 PM, Heiko St?bner wrote:
> > Hi Daniel,
> > 
> > Am Mittwoch, 26. November 2014, 12:51:08 schrieb Daniel Lezcano:
> >> Hi Doug, Olof,
> >> 
> >> IIUC, it sounds like this patch is needed from some other patches in
> >> arm-soc. Olof was proposing to take this patch through its tree to
> >> facilitate the integration.
> >> 
> >> Olof, is it this patch you were worried about ?
> > 
> > I think this is one of two patches in question.
> > 
> > "clocksource: arch_timer: Fix code to use physical timers when requested"
> > [0] would be the second one.
> > 
> > And the patch for arm-soc that Olof means would be "ARM: dts: rk3288: add
> > arm,cpu-registers-not-fw-configured" [1].
> 
> Ok, so IIUC, "clocksource: arch_timer: Fix code to use physical timers
> when requested" should go via arm's tree, right ?

If I'm reading Olof's irc-comments from yesterday correctly, that is right and 
the 3 patches should go in together:

- "clocksource: arch_timer: Fix code to use physical timers
  when requested" fixes the use of physical timers in general
- "clocksource: arch_timer: Allow the device tree to specify uninitialized
  timer registers" allows this to be set from dt
- "ARM: dts: rk3288: add arm,cpu-registers-not-fw-configured" enables this on
  rk3288


Heiko

Re: [PATCH v4] clocksource: arch_timer: Allow the device tree to specify uninitialized timer registers

[Daniel Lezcano]

On 10/08/2014 09:33 AM, Sonny Rao wrote:
> From: Doug Anderson <dianders@chromium.org>
>
> Some 32-bit (ARMv7) systems are architected like this:
>
> * The firmware doesn't know and doesn't care about hypervisor mode and
>    we don't want to add the complexity of hypervisor there.
>
> * The firmware isn't involved in SMP bringup or resume.
>
> * The ARCH timer come up with an uninitialized offset (CNTVOFF)
>    between the virtual and physical counters.  Each core gets a
>    different random offset.
>
> * The device boots in "Secure SVC" mode.
>
> * Nothing has touched the reset value of CNTHCTL.PL1PCEN or
>    CNTHCTL.PL1PCTEN (both default to 1 at reset)
>
> On systems like the above, it doesn't make sense to use the virtual
> counter.  There's nobody managing the offset and each time a core goes
> down and comes back up it will get reinitialized to some other random
> value.
>
> This adds an optional property which can inform the kernel of this
> situation, and firmware is free to remove the property if it is going
> to initialize the CNTVOFF registers when each CPU comes out of reset.
>
> Currently, the best course of action in this case is to use the
> physical timer, which is why it is important that CNTHCTL hasn't been
> changed from its reset value and it's a reasonable assumption given
> that the firmware has never entered HYP mode.
>
> Note that it's been said that on ARMv8 systems the firmware and
> kernel really can't be architected as described above.  That means
> using the physical timer like this really only makes sense for ARMv7
> systems.
>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> Signed-off-by: Sonny Rao <sonnyrao@chromium.org>
> Reviewed-by: Mark Rutland <mark.rutland@arm.com>

Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>

I would be nice to have Catalin's ack.

Thanks

   -- Daniel

> ---
> Changes in v2:
> - Add "#ifdef CONFIG_ARM" as per Will Deacon
>
> Changes in v3:
> - change property name to arm,cntvoff-not-fw-configured and specify
>    that the value of CNTHCTL.PL1PC(T)EN must still be the reset value
>    of 1 as per Mark Rutland
>
> Changes in v4:
> - change property name to arm,cpu-registers-not-fw-configured and
>    specify that all cpu registers must have architected reset values
>    per Mark Rutland
> - change from "#ifdef CONFIG_ARM" to "if (IS_ENABLED(CONFIG_ARM))" per
>    Arnd Bergmann
> ---
>   Documentation/devicetree/bindings/arm/arch_timer.txt | 8 ++++++++
>   drivers/clocksource/arm_arch_timer.c                 | 8 ++++++++
>   2 files changed, 16 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/arm/arch_timer.txt b/Documentation/devicetree/bindings/arm/arch_timer.txt
> index 37b2caf..256b4d8 100644
> --- a/Documentation/devicetree/bindings/arm/arch_timer.txt
> +++ b/Documentation/devicetree/bindings/arm/arch_timer.txt
> @@ -22,6 +22,14 @@ to deliver its interrupts via SPIs.
>   - always-on : a boolean property. If present, the timer is powered through an
>     always-on power domain, therefore it never loses context.
>
> +** Optional properties:
> +
> +- arm,cpu-registers-not-fw-configured : Firmware does not initialize
> +  any of the generic timer CPU registers, which contain their
> +  architecturally-defined reset values. Only supported for 32-bit
> +  systems which follow the ARMv7 architected reset values.
> +
> +
>   Example:
>
>   	timer {
> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> index 8daf056..799139f 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -654,6 +654,14 @@ static void __init arch_timer_init(struct device_node *np)
>   	arch_timer_detect_rate(NULL, np);
>
>   	/*
> +	 * If we cannot rely on firmware initializing the timer registers then
> +	 * we should use the physical timers instead.
> +	 */
> +	if (IS_ENABLED(CONFIG_ARM) &&
> +	    of_property_read_bool(np, "arm,cpu-registers-not-fw-configured"))
> +			arch_timer_use_virtual = false;
> +
> +	/*
>   	 * If HYP mode is available, we know that the physical timer
>   	 * has been configured to be accessible from PL1. Use it, so
>   	 * that a guest can use the virtual timer instead.
>


-- 
  <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

[PATCH v4] clocksource: arch_timer: Allow the device tree to specify uninitialized timer registers

[Daniel Lezcano]

On 10/08/2014 09:33 AM, Sonny Rao wrote:
> From: Doug Anderson <dianders@chromium.org>
>
> Some 32-bit (ARMv7) systems are architected like this:
>
> * The firmware doesn't know and doesn't care about hypervisor mode and
>    we don't want to add the complexity of hypervisor there.
>
> * The firmware isn't involved in SMP bringup or resume.
>
> * The ARCH timer come up with an uninitialized offset (CNTVOFF)
>    between the virtual and physical counters.  Each core gets a
>    different random offset.
>
> * The device boots in "Secure SVC" mode.
>
> * Nothing has touched the reset value of CNTHCTL.PL1PCEN or
>    CNTHCTL.PL1PCTEN (both default to 1 at reset)
>
> On systems like the above, it doesn't make sense to use the virtual
> counter.  There's nobody managing the offset and each time a core goes
> down and comes back up it will get reinitialized to some other random
> value.
>
> This adds an optional property which can inform the kernel of this
> situation, and firmware is free to remove the property if it is going
> to initialize the CNTVOFF registers when each CPU comes out of reset.
>
> Currently, the best course of action in this case is to use the
> physical timer, which is why it is important that CNTHCTL hasn't been
> changed from its reset value and it's a reasonable assumption given
> that the firmware has never entered HYP mode.
>
> Note that it's been said that on ARMv8 systems the firmware and
> kernel really can't be architected as described above.  That means
> using the physical timer like this really only makes sense for ARMv7
> systems.
>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> Signed-off-by: Sonny Rao <sonnyrao@chromium.org>
> Reviewed-by: Mark Rutland <mark.rutland@arm.com>

Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>

I would be nice to have Catalin's ack.

Thanks

   -- Daniel

> ---
> Changes in v2:
> - Add "#ifdef CONFIG_ARM" as per Will Deacon
>
> Changes in v3:
> - change property name to arm,cntvoff-not-fw-configured and specify
>    that the value of CNTHCTL.PL1PC(T)EN must still be the reset value
>    of 1 as per Mark Rutland
>
> Changes in v4:
> - change property name to arm,cpu-registers-not-fw-configured and
>    specify that all cpu registers must have architected reset values
>    per Mark Rutland
> - change from "#ifdef CONFIG_ARM" to "if (IS_ENABLED(CONFIG_ARM))" per
>    Arnd Bergmann
> ---
>   Documentation/devicetree/bindings/arm/arch_timer.txt | 8 ++++++++
>   drivers/clocksource/arm_arch_timer.c                 | 8 ++++++++
>   2 files changed, 16 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/arm/arch_timer.txt b/Documentation/devicetree/bindings/arm/arch_timer.txt
> index 37b2caf..256b4d8 100644
> --- a/Documentation/devicetree/bindings/arm/arch_timer.txt
> +++ b/Documentation/devicetree/bindings/arm/arch_timer.txt
> @@ -22,6 +22,14 @@ to deliver its interrupts via SPIs.
>   - always-on : a boolean property. If present, the timer is powered through an
>     always-on power domain, therefore it never loses context.
>
> +** Optional properties:
> +
> +- arm,cpu-registers-not-fw-configured : Firmware does not initialize
> +  any of the generic timer CPU registers, which contain their
> +  architecturally-defined reset values. Only supported for 32-bit
> +  systems which follow the ARMv7 architected reset values.
> +
> +
>   Example:
>
>   	timer {
> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> index 8daf056..799139f 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -654,6 +654,14 @@ static void __init arch_timer_init(struct device_node *np)
>   	arch_timer_detect_rate(NULL, np);
>
>   	/*
> +	 * If we cannot rely on firmware initializing the timer registers then
> +	 * we should use the physical timers instead.
> +	 */
> +	if (IS_ENABLED(CONFIG_ARM) &&
> +	    of_property_read_bool(np, "arm,cpu-registers-not-fw-configured"))
> +			arch_timer_use_virtual = false;
> +
> +	/*
>   	 * If HYP mode is available, we know that the physical timer
>   	 * has been configured to be accessible from PL1. Use it, so
>   	 * that a guest can use the virtual timer instead.
>


-- 
  <http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH v4] clocksource: arch_timer: Allow the device tree to specify uninitialized timer registers

[Daniel Lezcano]

On 11/26/2014 01:48 PM, Heiko Stübner wrote:
> Am Mittwoch, 26. November 2014, 13:30:58 schrieb Daniel Lezcano:
>> On 11/26/2014 01:06 PM, Heiko Stübner wrote:
>>> Hi Daniel,
>>>
>>> Am Mittwoch, 26. November 2014, 12:51:08 schrieb Daniel Lezcano:
>>>> Hi Doug, Olof,
>>>>
>>>> IIUC, it sounds like this patch is needed from some other patches in
>>>> arm-soc. Olof was proposing to take this patch through its tree to
>>>> facilitate the integration.
>>>>
>>>> Olof, is it this patch you were worried about ?
>>>
>>> I think this is one of two patches in question.
>>>
>>> "clocksource: arch_timer: Fix code to use physical timers when requested"
>>> [0] would be the second one.
>>>
>>> And the patch for arm-soc that Olof means would be "ARM: dts: rk3288: add
>>> arm,cpu-registers-not-fw-configured" [1].
>>
>> Ok, so IIUC, "clocksource: arch_timer: Fix code to use physical timers
>> when requested" should go via arm's tree, right ?
>
> If I'm reading Olof's irc-comments from yesterday correctly, that is right and
> the 3 patches should go in together:
>
> - "clocksource: arch_timer: Fix code to use physical timers
>    when requested" fixes the use of physical timers in general
> - "clocksource: arch_timer: Allow the device tree to specify uninitialized
>    timer registers" allows this to be set from dt
> - "ARM: dts: rk3288: add arm,cpu-registers-not-fw-configured" enables this on
>    rk3288

Ok, then I drop them from my tree and will let Olof to handle them.

Thanks !

   -- Daniel


-- 
  <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH v4] clocksource: arch_timer: Allow the device tree to specify uninitialized timer registers

[Daniel Lezcano]

On 11/26/2014 01:48 PM, Heiko Stübner wrote:
> Am Mittwoch, 26. November 2014, 13:30:58 schrieb Daniel Lezcano:
>> On 11/26/2014 01:06 PM, Heiko Stübner wrote:
>>> Hi Daniel,
>>>
>>> Am Mittwoch, 26. November 2014, 12:51:08 schrieb Daniel Lezcano:
>>>> Hi Doug, Olof,
>>>>
>>>> IIUC, it sounds like this patch is needed from some other patches in
>>>> arm-soc. Olof was proposing to take this patch through its tree to
>>>> facilitate the integration.
>>>>
>>>> Olof, is it this patch you were worried about ?
>>>
>>> I think this is one of two patches in question.
>>>
>>> "clocksource: arch_timer: Fix code to use physical timers when requested"
>>> [0] would be the second one.
>>>
>>> And the patch for arm-soc that Olof means would be "ARM: dts: rk3288: add
>>> arm,cpu-registers-not-fw-configured" [1].
>>
>> Ok, so IIUC, "clocksource: arch_timer: Fix code to use physical timers
>> when requested" should go via arm's tree, right ?
>
> If I'm reading Olof's irc-comments from yesterday correctly, that is right and
> the 3 patches should go in together:
>
> - "clocksource: arch_timer: Fix code to use physical timers
>    when requested" fixes the use of physical timers in general
> - "clocksource: arch_timer: Allow the device tree to specify uninitialized
>    timer registers" allows this to be set from dt
> - "ARM: dts: rk3288: add arm,cpu-registers-not-fw-configured" enables this on
>    rk3288

Ok, then I drop them from my tree and will let Olof to handle them.

Thanks !

   -- Daniel


-- 
  <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

[PATCH v4] clocksource: arch_timer: Allow the device tree to specify uninitialized timer registers

[Daniel Lezcano]

On 11/26/2014 01:48 PM, Heiko St?bner wrote:
> Am Mittwoch, 26. November 2014, 13:30:58 schrieb Daniel Lezcano:
>> On 11/26/2014 01:06 PM, Heiko St?bner wrote:
>>> Hi Daniel,
>>>
>>> Am Mittwoch, 26. November 2014, 12:51:08 schrieb Daniel Lezcano:
>>>> Hi Doug, Olof,
>>>>
>>>> IIUC, it sounds like this patch is needed from some other patches in
>>>> arm-soc. Olof was proposing to take this patch through its tree to
>>>> facilitate the integration.
>>>>
>>>> Olof, is it this patch you were worried about ?
>>>
>>> I think this is one of two patches in question.
>>>
>>> "clocksource: arch_timer: Fix code to use physical timers when requested"
>>> [0] would be the second one.
>>>
>>> And the patch for arm-soc that Olof means would be "ARM: dts: rk3288: add
>>> arm,cpu-registers-not-fw-configured" [1].
>>
>> Ok, so IIUC, "clocksource: arch_timer: Fix code to use physical timers
>> when requested" should go via arm's tree, right ?
>
> If I'm reading Olof's irc-comments from yesterday correctly, that is right and
> the 3 patches should go in together:
>
> - "clocksource: arch_timer: Fix code to use physical timers
>    when requested" fixes the use of physical timers in general
> - "clocksource: arch_timer: Allow the device tree to specify uninitialized
>    timer registers" allows this to be set from dt
> - "ARM: dts: rk3288: add arm,cpu-registers-not-fw-configured" enables this on
>    rk3288

Ok, then I drop them from my tree and will let Olof to handle them.

Thanks !

   -- Daniel


-- 
  <http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH v4] clocksource: arch_timer: Allow the device tree to specify uninitialized timer registers

[Daniel Lezcano]

On 11/26/2014 01:55 PM, Heiko Stübner wrote:
> Am Mittwoch, 26. November 2014, 13:49:57 schrieb Daniel Lezcano:
>> On 11/26/2014 01:48 PM, Heiko Stübner wrote:
>>> Am Mittwoch, 26. November 2014, 13:30:58 schrieb Daniel Lezcano:
>>>> On 11/26/2014 01:06 PM, Heiko Stübner wrote:
>>>>> Hi Daniel,
>>>>>
>>>>> Am Mittwoch, 26. November 2014, 12:51:08 schrieb Daniel Lezcano:
>>>>>> Hi Doug, Olof,
>>>>>>
>>>>>> IIUC, it sounds like this patch is needed from some other patches in
>>>>>> arm-soc. Olof was proposing to take this patch through its tree to
>>>>>> facilitate the integration.
>>>>>>
>>>>>> Olof, is it this patch you were worried about ?
>>>>>
>>>>> I think this is one of two patches in question.
>>>>>
>>>>> "clocksource: arch_timer: Fix code to use physical timers when
>>>>> requested"
>>>>> [0] would be the second one.
>>>>>
>>>>> And the patch for arm-soc that Olof means would be "ARM: dts: rk3288:
>>>>> add
>>>>> arm,cpu-registers-not-fw-configured" [1].
>>>>
>>>> Ok, so IIUC, "clocksource: arch_timer: Fix code to use physical timers
>>>> when requested" should go via arm's tree, right ?
>>>
>>> If I'm reading Olof's irc-comments from yesterday correctly, that is right
>>> and the 3 patches should go in together:
>>>
>>> - "clocksource: arch_timer: Fix code to use physical timers
>>>
>>>     when requested" fixes the use of physical timers in general
>>>
>>> - "clocksource: arch_timer: Allow the device tree to specify uninitialized
>>>
>>>     timer registers" allows this to be set from dt
>>>
>>> - "ARM: dts: rk3288: add arm,cpu-registers-not-fw-configured" enables this
>>> on>
>>>     rk3288
>>
>> Ok, then I drop them from my tree and will let Olof to handle them.
>
> But maybe you could give them an Ack :-)

Already done :)

   -- Daniel

-- 
  <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH v4] clocksource: arch_timer: Allow the device tree to specify uninitialized timer registers

[Daniel Lezcano]

On 11/26/2014 01:55 PM, Heiko Stübner wrote:
> Am Mittwoch, 26. November 2014, 13:49:57 schrieb Daniel Lezcano:
>> On 11/26/2014 01:48 PM, Heiko Stübner wrote:
>>> Am Mittwoch, 26. November 2014, 13:30:58 schrieb Daniel Lezcano:
>>>> On 11/26/2014 01:06 PM, Heiko Stübner wrote:
>>>>> Hi Daniel,
>>>>>
>>>>> Am Mittwoch, 26. November 2014, 12:51:08 schrieb Daniel Lezcano:
>>>>>> Hi Doug, Olof,
>>>>>>
>>>>>> IIUC, it sounds like this patch is needed from some other patches in
>>>>>> arm-soc. Olof was proposing to take this patch through its tree to
>>>>>> facilitate the integration.
>>>>>>
>>>>>> Olof, is it this patch you were worried about ?
>>>>>
>>>>> I think this is one of two patches in question.
>>>>>
>>>>> "clocksource: arch_timer: Fix code to use physical timers when
>>>>> requested"
>>>>> [0] would be the second one.
>>>>>
>>>>> And the patch for arm-soc that Olof means would be "ARM: dts: rk3288:
>>>>> add
>>>>> arm,cpu-registers-not-fw-configured" [1].
>>>>
>>>> Ok, so IIUC, "clocksource: arch_timer: Fix code to use physical timers
>>>> when requested" should go via arm's tree, right ?
>>>
>>> If I'm reading Olof's irc-comments from yesterday correctly, that is right
>>> and the 3 patches should go in together:
>>>
>>> - "clocksource: arch_timer: Fix code to use physical timers
>>>
>>>     when requested" fixes the use of physical timers in general
>>>
>>> - "clocksource: arch_timer: Allow the device tree to specify uninitialized
>>>
>>>     timer registers" allows this to be set from dt
>>>
>>> - "ARM: dts: rk3288: add arm,cpu-registers-not-fw-configured" enables this
>>> on>
>>>     rk3288
>>
>> Ok, then I drop them from my tree and will let Olof to handle them.
>
> But maybe you could give them an Ack :-)

Already done :)

   -- Daniel

-- 
  <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

[PATCH v4] clocksource: arch_timer: Allow the device tree to specify uninitialized timer registers

[Daniel Lezcano]

On 11/26/2014 01:55 PM, Heiko St?bner wrote:
> Am Mittwoch, 26. November 2014, 13:49:57 schrieb Daniel Lezcano:
>> On 11/26/2014 01:48 PM, Heiko St?bner wrote:
>>> Am Mittwoch, 26. November 2014, 13:30:58 schrieb Daniel Lezcano:
>>>> On 11/26/2014 01:06 PM, Heiko St?bner wrote:
>>>>> Hi Daniel,
>>>>>
>>>>> Am Mittwoch, 26. November 2014, 12:51:08 schrieb Daniel Lezcano:
>>>>>> Hi Doug, Olof,
>>>>>>
>>>>>> IIUC, it sounds like this patch is needed from some other patches in
>>>>>> arm-soc. Olof was proposing to take this patch through its tree to
>>>>>> facilitate the integration.
>>>>>>
>>>>>> Olof, is it this patch you were worried about ?
>>>>>
>>>>> I think this is one of two patches in question.
>>>>>
>>>>> "clocksource: arch_timer: Fix code to use physical timers when
>>>>> requested"
>>>>> [0] would be the second one.
>>>>>
>>>>> And the patch for arm-soc that Olof means would be "ARM: dts: rk3288:
>>>>> add
>>>>> arm,cpu-registers-not-fw-configured" [1].
>>>>
>>>> Ok, so IIUC, "clocksource: arch_timer: Fix code to use physical timers
>>>> when requested" should go via arm's tree, right ?
>>>
>>> If I'm reading Olof's irc-comments from yesterday correctly, that is right
>>> and the 3 patches should go in together:
>>>
>>> - "clocksource: arch_timer: Fix code to use physical timers
>>>
>>>     when requested" fixes the use of physical timers in general
>>>
>>> - "clocksource: arch_timer: Allow the device tree to specify uninitialized
>>>
>>>     timer registers" allows this to be set from dt
>>>
>>> - "ARM: dts: rk3288: add arm,cpu-registers-not-fw-configured" enables this
>>> on>
>>>     rk3288
>>
>> Ok, then I drop them from my tree and will let Olof to handle them.
>
> But maybe you could give them an Ack :-)

Already done :)

   -- Daniel

-- 
  <http://www.linaro.org/> Linaro.org ? Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH v4] clocksource: arch_timer: Allow the device tree to specify uninitialized timer registers

[Heiko Stübner]

Am Mittwoch, 26. November 2014, 13:49:57 schrieb Daniel Lezcano:
> On 11/26/2014 01:48 PM, Heiko Stübner wrote:
> > Am Mittwoch, 26. November 2014, 13:30:58 schrieb Daniel Lezcano:
> >> On 11/26/2014 01:06 PM, Heiko Stübner wrote:
> >>> Hi Daniel,
> >>> 
> >>> Am Mittwoch, 26. November 2014, 12:51:08 schrieb Daniel Lezcano:
> >>>> Hi Doug, Olof,
> >>>> 
> >>>> IIUC, it sounds like this patch is needed from some other patches in
> >>>> arm-soc. Olof was proposing to take this patch through its tree to
> >>>> facilitate the integration.
> >>>> 
> >>>> Olof, is it this patch you were worried about ?
> >>> 
> >>> I think this is one of two patches in question.
> >>> 
> >>> "clocksource: arch_timer: Fix code to use physical timers when
> >>> requested"
> >>> [0] would be the second one.
> >>> 
> >>> And the patch for arm-soc that Olof means would be "ARM: dts: rk3288:
> >>> add
> >>> arm,cpu-registers-not-fw-configured" [1].
> >> 
> >> Ok, so IIUC, "clocksource: arch_timer: Fix code to use physical timers
> >> when requested" should go via arm's tree, right ?
> > 
> > If I'm reading Olof's irc-comments from yesterday correctly, that is right
> > and the 3 patches should go in together:
> > 
> > - "clocksource: arch_timer: Fix code to use physical timers
> > 
> >    when requested" fixes the use of physical timers in general
> > 
> > - "clocksource: arch_timer: Allow the device tree to specify uninitialized
> > 
> >    timer registers" allows this to be set from dt
> > 
> > - "ARM: dts: rk3288: add arm,cpu-registers-not-fw-configured" enables this
> > on> 
> >    rk3288
> 
> Ok, then I drop them from my tree and will let Olof to handle them.

But maybe you could give them an Ack :-)


Heiko

Re: [PATCH v4] clocksource: arch_timer: Allow the device tree to specify uninitialized timer registers

[Heiko Stübner]

Am Mittwoch, 26. November 2014, 13:49:57 schrieb Daniel Lezcano:
> On 11/26/2014 01:48 PM, Heiko Stübner wrote:
> > Am Mittwoch, 26. November 2014, 13:30:58 schrieb Daniel Lezcano:
> >> On 11/26/2014 01:06 PM, Heiko Stübner wrote:
> >>> Hi Daniel,
> >>> 
> >>> Am Mittwoch, 26. November 2014, 12:51:08 schrieb Daniel Lezcano:
> >>>> Hi Doug, Olof,
> >>>> 
> >>>> IIUC, it sounds like this patch is needed from some other patches in
> >>>> arm-soc. Olof was proposing to take this patch through its tree to
> >>>> facilitate the integration.
> >>>> 
> >>>> Olof, is it this patch you were worried about ?
> >>> 
> >>> I think this is one of two patches in question.
> >>> 
> >>> "clocksource: arch_timer: Fix code to use physical timers when
> >>> requested"
> >>> [0] would be the second one.
> >>> 
> >>> And the patch for arm-soc that Olof means would be "ARM: dts: rk3288:
> >>> add
> >>> arm,cpu-registers-not-fw-configured" [1].
> >> 
> >> Ok, so IIUC, "clocksource: arch_timer: Fix code to use physical timers
> >> when requested" should go via arm's tree, right ?
> > 
> > If I'm reading Olof's irc-comments from yesterday correctly, that is right
> > and the 3 patches should go in together:
> > 
> > - "clocksource: arch_timer: Fix code to use physical timers
> > 
> >    when requested" fixes the use of physical timers in general
> > 
> > - "clocksource: arch_timer: Allow the device tree to specify uninitialized
> > 
> >    timer registers" allows this to be set from dt
> > 
> > - "ARM: dts: rk3288: add arm,cpu-registers-not-fw-configured" enables this
> > on> 
> >    rk3288
> 
> Ok, then I drop them from my tree and will let Olof to handle them.

But maybe you could give them an Ack :-)


Heiko
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4] clocksource: arch_timer: Allow the device tree to specify uninitialized timer registers

[Heiko Stübner]

Am Mittwoch, 26. November 2014, 13:49:57 schrieb Daniel Lezcano:
> On 11/26/2014 01:48 PM, Heiko St?bner wrote:
> > Am Mittwoch, 26. November 2014, 13:30:58 schrieb Daniel Lezcano:
> >> On 11/26/2014 01:06 PM, Heiko St?bner wrote:
> >>> Hi Daniel,
> >>> 
> >>> Am Mittwoch, 26. November 2014, 12:51:08 schrieb Daniel Lezcano:
> >>>> Hi Doug, Olof,
> >>>> 
> >>>> IIUC, it sounds like this patch is needed from some other patches in
> >>>> arm-soc. Olof was proposing to take this patch through its tree to
> >>>> facilitate the integration.
> >>>> 
> >>>> Olof, is it this patch you were worried about ?
> >>> 
> >>> I think this is one of two patches in question.
> >>> 
> >>> "clocksource: arch_timer: Fix code to use physical timers when
> >>> requested"
> >>> [0] would be the second one.
> >>> 
> >>> And the patch for arm-soc that Olof means would be "ARM: dts: rk3288:
> >>> add
> >>> arm,cpu-registers-not-fw-configured" [1].
> >> 
> >> Ok, so IIUC, "clocksource: arch_timer: Fix code to use physical timers
> >> when requested" should go via arm's tree, right ?
> > 
> > If I'm reading Olof's irc-comments from yesterday correctly, that is right
> > and the 3 patches should go in together:
> > 
> > - "clocksource: arch_timer: Fix code to use physical timers
> > 
> >    when requested" fixes the use of physical timers in general
> > 
> > - "clocksource: arch_timer: Allow the device tree to specify uninitialized
> > 
> >    timer registers" allows this to be set from dt
> > 
> > - "ARM: dts: rk3288: add arm,cpu-registers-not-fw-configured" enables this
> > on> 
> >    rk3288
> 
> Ok, then I drop them from my tree and will let Olof to handle them.

But maybe you could give them an Ack :-)


Heiko

Re: [PATCH v4] clocksource: arch_timer: Allow the device tree to specify uninitialized timer registers

[Yingjoe Chen]

On Wed, 2014-10-08 at 15:33 +0800, Sonny Rao wrote:
> From: Doug Anderson <dianders@chromium.org>
> 
> Some 32-bit (ARMv7) systems are architected like this:
> 
> * The firmware doesn't know and doesn't care about hypervisor mode and
>   we don't want to add the complexity of hypervisor there.
> 
> * The firmware isn't involved in SMP bringup or resume.
> 
> * The ARCH timer come up with an uninitialized offset (CNTVOFF)
>   between the virtual and physical counters.  Each core gets a
>   different random offset.
> 
> * The device boots in "Secure SVC" mode.
> 
> * Nothing has touched the reset value of CNTHCTL.PL1PCEN or
>   CNTHCTL.PL1PCTEN (both default to 1 at reset)
> 
> On systems like the above, it doesn't make sense to use the virtual
> counter.  There's nobody managing the offset and each time a core goes
> down and comes back up it will get reinitialized to some other random
> value.
> 
> This adds an optional property which can inform the kernel of this
> situation, and firmware is free to remove the property if it is going
> to initialize the CNTVOFF registers when each CPU comes out of reset.
> 
> Currently, the best course of action in this case is to use the
> physical timer, which is why it is important that CNTHCTL hasn't been
> changed from its reset value and it's a reasonable assumption given
> that the firmware has never entered HYP mode.
> 
> Note that it's been said that on ARMv8 systems the firmware and
> kernel really can't be architected as described above.  That means
> using the physical timer like this really only makes sense for ARMv7
> systems.
> 
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> Signed-off-by: Sonny Rao <sonnyrao@chromium.org>
> Reviewed-by: Mark Rutland <mark.rutland@arm.com>
> ---
> Changes in v2:
> - Add "#ifdef CONFIG_ARM" as per Will Deacon
> 
> Changes in v3:
> - change property name to arm,cntvoff-not-fw-configured and specify
>   that the value of CNTHCTL.PL1PC(T)EN must still be the reset value
>   of 1 as per Mark Rutland
> 
> Changes in v4:
> - change property name to arm,cpu-registers-not-fw-configured and
>   specify that all cpu registers must have architected reset values
>   per Mark Rutland
> - change from "#ifdef CONFIG_ARM" to "if (IS_ENABLED(CONFIG_ARM))" per
>   Arnd Bergmann
> ---
>  Documentation/devicetree/bindings/arm/arch_timer.txt | 8 ++++++++
>  drivers/clocksource/arm_arch_timer.c                 | 8 ++++++++
>  2 files changed, 16 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/arch_timer.txt b/Documentation/devicetree/bindings/arm/arch_timer.txt
> index 37b2caf..256b4d8 100644
> --- a/Documentation/devicetree/bindings/arm/arch_timer.txt
> +++ b/Documentation/devicetree/bindings/arm/arch_timer.txt
> @@ -22,6 +22,14 @@ to deliver its interrupts via SPIs.
>  - always-on : a boolean property. If present, the timer is powered through an
>    always-on power domain, therefore it never loses context.
> 
> +** Optional properties:
> +
> +- arm,cpu-registers-not-fw-configured : Firmware does not initialize
> +  any of the generic timer CPU registers, which contain their
> +  architecturally-defined reset values. Only supported for 32-bit
> +  systems which follow the ARMv7 architected reset values.
> +
> +

Hi,

Sorry for the (very) late reply.
I just realize today MT8135 need this and the other patch [1] to boot
SMP correctly. I've applied both patches and they works fine. Thanks :)

However, I'm not sure if we really need to add new property.
arm_arch_timer driver will only use virtual timer when virtual PPI
interrupt is provided, so the following patch to timer dtsi will also
works. I think if the firmware doesn't support virtual timer, it make
sense to not supply virtual interrupt.

        timer {
                compatible = "arm,armv7-timer";
                interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_LOW>,
-                            <GIC_PPI 14 IRQ_TYPE_LEVEL_LOW>,
-                            <GIC_PPI 11 IRQ_TYPE_LEVEL_LOW>,
-                            <GIC_PPI 10 IRQ_TYPE_LEVEL_LOW>;
+                            <GIC_PPI 14 IRQ_TYPE_LEVEL_LOW>;
                clock-frequency = <13000000>;
        };

Joe.C

[1]
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-November/305436.html

Re: [PATCH v4] clocksource: arch_timer: Allow the device tree to specify uninitialized timer registers

[Yingjoe Chen]

On Wed, 2014-10-08 at 15:33 +0800, Sonny Rao wrote:
> From: Doug Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> 
> Some 32-bit (ARMv7) systems are architected like this:
> 
> * The firmware doesn't know and doesn't care about hypervisor mode and
>   we don't want to add the complexity of hypervisor there.
> 
> * The firmware isn't involved in SMP bringup or resume.
> 
> * The ARCH timer come up with an uninitialized offset (CNTVOFF)
>   between the virtual and physical counters.  Each core gets a
>   different random offset.
> 
> * The device boots in "Secure SVC" mode.
> 
> * Nothing has touched the reset value of CNTHCTL.PL1PCEN or
>   CNTHCTL.PL1PCTEN (both default to 1 at reset)
> 
> On systems like the above, it doesn't make sense to use the virtual
> counter.  There's nobody managing the offset and each time a core goes
> down and comes back up it will get reinitialized to some other random
> value.
> 
> This adds an optional property which can inform the kernel of this
> situation, and firmware is free to remove the property if it is going
> to initialize the CNTVOFF registers when each CPU comes out of reset.
> 
> Currently, the best course of action in this case is to use the
> physical timer, which is why it is important that CNTHCTL hasn't been
> changed from its reset value and it's a reasonable assumption given
> that the firmware has never entered HYP mode.
> 
> Note that it's been said that on ARMv8 systems the firmware and
> kernel really can't be architected as described above.  That means
> using the physical timer like this really only makes sense for ARMv7
> systems.
> 
> Signed-off-by: Doug Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> Signed-off-by: Sonny Rao <sonnyrao-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> Reviewed-by: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
> ---
> Changes in v2:
> - Add "#ifdef CONFIG_ARM" as per Will Deacon
> 
> Changes in v3:
> - change property name to arm,cntvoff-not-fw-configured and specify
>   that the value of CNTHCTL.PL1PC(T)EN must still be the reset value
>   of 1 as per Mark Rutland
> 
> Changes in v4:
> - change property name to arm,cpu-registers-not-fw-configured and
>   specify that all cpu registers must have architected reset values
>   per Mark Rutland
> - change from "#ifdef CONFIG_ARM" to "if (IS_ENABLED(CONFIG_ARM))" per
>   Arnd Bergmann
> ---
>  Documentation/devicetree/bindings/arm/arch_timer.txt | 8 ++++++++
>  drivers/clocksource/arm_arch_timer.c                 | 8 ++++++++
>  2 files changed, 16 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/arch_timer.txt b/Documentation/devicetree/bindings/arm/arch_timer.txt
> index 37b2caf..256b4d8 100644
> --- a/Documentation/devicetree/bindings/arm/arch_timer.txt
> +++ b/Documentation/devicetree/bindings/arm/arch_timer.txt
> @@ -22,6 +22,14 @@ to deliver its interrupts via SPIs.
>  - always-on : a boolean property. If present, the timer is powered through an
>    always-on power domain, therefore it never loses context.
> 
> +** Optional properties:
> +
> +- arm,cpu-registers-not-fw-configured : Firmware does not initialize
> +  any of the generic timer CPU registers, which contain their
> +  architecturally-defined reset values. Only supported for 32-bit
> +  systems which follow the ARMv7 architected reset values.
> +
> +

Hi,

Sorry for the (very) late reply.
I just realize today MT8135 need this and the other patch [1] to boot
SMP correctly. I've applied both patches and they works fine. Thanks :)

However, I'm not sure if we really need to add new property.
arm_arch_timer driver will only use virtual timer when virtual PPI
interrupt is provided, so the following patch to timer dtsi will also
works. I think if the firmware doesn't support virtual timer, it make
sense to not supply virtual interrupt.

        timer {
                compatible = "arm,armv7-timer";
                interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_LOW>,
-                            <GIC_PPI 14 IRQ_TYPE_LEVEL_LOW>,
-                            <GIC_PPI 11 IRQ_TYPE_LEVEL_LOW>,
-                            <GIC_PPI 10 IRQ_TYPE_LEVEL_LOW>;
+                            <GIC_PPI 14 IRQ_TYPE_LEVEL_LOW>;
                clock-frequency = <13000000>;
        };

Joe.C

[1]
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-November/305436.html


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4] clocksource: arch_timer: Allow the device tree to specify uninitialized timer registers

[Yingjoe Chen]

On Wed, 2014-10-08 at 15:33 +0800, Sonny Rao wrote:
> From: Doug Anderson <dianders@chromium.org>
> 
> Some 32-bit (ARMv7) systems are architected like this:
> 
> * The firmware doesn't know and doesn't care about hypervisor mode and
>   we don't want to add the complexity of hypervisor there.
> 
> * The firmware isn't involved in SMP bringup or resume.
> 
> * The ARCH timer come up with an uninitialized offset (CNTVOFF)
>   between the virtual and physical counters.  Each core gets a
>   different random offset.
> 
> * The device boots in "Secure SVC" mode.
> 
> * Nothing has touched the reset value of CNTHCTL.PL1PCEN or
>   CNTHCTL.PL1PCTEN (both default to 1 at reset)
> 
> On systems like the above, it doesn't make sense to use the virtual
> counter.  There's nobody managing the offset and each time a core goes
> down and comes back up it will get reinitialized to some other random
> value.
> 
> This adds an optional property which can inform the kernel of this
> situation, and firmware is free to remove the property if it is going
> to initialize the CNTVOFF registers when each CPU comes out of reset.
> 
> Currently, the best course of action in this case is to use the
> physical timer, which is why it is important that CNTHCTL hasn't been
> changed from its reset value and it's a reasonable assumption given
> that the firmware has never entered HYP mode.
> 
> Note that it's been said that on ARMv8 systems the firmware and
> kernel really can't be architected as described above.  That means
> using the physical timer like this really only makes sense for ARMv7
> systems.
> 
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> Signed-off-by: Sonny Rao <sonnyrao@chromium.org>
> Reviewed-by: Mark Rutland <mark.rutland@arm.com>
> ---
> Changes in v2:
> - Add "#ifdef CONFIG_ARM" as per Will Deacon
> 
> Changes in v3:
> - change property name to arm,cntvoff-not-fw-configured and specify
>   that the value of CNTHCTL.PL1PC(T)EN must still be the reset value
>   of 1 as per Mark Rutland
> 
> Changes in v4:
> - change property name to arm,cpu-registers-not-fw-configured and
>   specify that all cpu registers must have architected reset values
>   per Mark Rutland
> - change from "#ifdef CONFIG_ARM" to "if (IS_ENABLED(CONFIG_ARM))" per
>   Arnd Bergmann
> ---
>  Documentation/devicetree/bindings/arm/arch_timer.txt | 8 ++++++++
>  drivers/clocksource/arm_arch_timer.c                 | 8 ++++++++
>  2 files changed, 16 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/arch_timer.txt b/Documentation/devicetree/bindings/arm/arch_timer.txt
> index 37b2caf..256b4d8 100644
> --- a/Documentation/devicetree/bindings/arm/arch_timer.txt
> +++ b/Documentation/devicetree/bindings/arm/arch_timer.txt
> @@ -22,6 +22,14 @@ to deliver its interrupts via SPIs.
>  - always-on : a boolean property. If present, the timer is powered through an
>    always-on power domain, therefore it never loses context.
> 
> +** Optional properties:
> +
> +- arm,cpu-registers-not-fw-configured : Firmware does not initialize
> +  any of the generic timer CPU registers, which contain their
> +  architecturally-defined reset values. Only supported for 32-bit
> +  systems which follow the ARMv7 architected reset values.
> +
> +

Hi,

Sorry for the (very) late reply.
I just realize today MT8135 need this and the other patch [1] to boot
SMP correctly. I've applied both patches and they works fine. Thanks :)

However, I'm not sure if we really need to add new property.
arm_arch_timer driver will only use virtual timer when virtual PPI
interrupt is provided, so the following patch to timer dtsi will also
works. I think if the firmware doesn't support virtual timer, it make
sense to not supply virtual interrupt.

        timer {
                compatible = "arm,armv7-timer";
                interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_LOW>,
-                            <GIC_PPI 14 IRQ_TYPE_LEVEL_LOW>,
-                            <GIC_PPI 11 IRQ_TYPE_LEVEL_LOW>,
-                            <GIC_PPI 10 IRQ_TYPE_LEVEL_LOW>;
+                            <GIC_PPI 14 IRQ_TYPE_LEVEL_LOW>;
                clock-frequency = <13000000>;
        };

Joe.C

[1]
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-November/305436.html

Re: [PATCH v4] clocksource: arch_timer: Allow the device tree to specify uninitialized timer registers

[Doug Anderson]

Yingjoe,

On Wed, Nov 26, 2014 at 6:41 AM, Yingjoe Chen <yingjoe.chen@mediatek.com> wrote:
> Sorry for the (very) late reply.
> I just realize today MT8135 need this and the other patch [1] to boot
> SMP correctly. I've applied both patches and they works fine. Thanks :)

Excellent.  It's helpful to include a Tested-by: tag in your email.
You'd have a line with just "Tested-by: Yingjoe Chen
<yingjoe.chen@mediatek.com>"


> However, I'm not sure if we really need to add new property.
> arm_arch_timer driver will only use virtual timer when virtual PPI
> interrupt is provided, so the following patch to timer dtsi will also
> works. I think if the firmware doesn't support virtual timer, it make
> sense to not supply virtual interrupt.
>
>         timer {
>                 compatible = "arm,armv7-timer";
>                 interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_LOW>,
> -                            <GIC_PPI 14 IRQ_TYPE_LEVEL_LOW>,
> -                            <GIC_PPI 11 IRQ_TYPE_LEVEL_LOW>,
> -                            <GIC_PPI 10 IRQ_TYPE_LEVEL_LOW>;
> +                            <GIC_PPI 14 IRQ_TYPE_LEVEL_LOW>;
>                 clock-frequency = <13000000>;
>         };

Once you have Sonny's patch then I believe that the above would work.
However we rejected something like this because device tree is
supposed to describe the hardware.  The hardware really does provide
the virtual timer interrupts and they really are at PPI 11 and PPI 10.
It's just that firmware doesn't handle things properly so they can't
be used.

NOTE: If we add the "arm,cpu-registers-not-fw-configured" to the
device tree and firmware actually works out how to configure things
(like if somehow has firmware that has a hypervisor) then it can
easily remove this device tree property before calling through to the
kernel.  It would be much harder for the firmware to add back in the
"PPI 11" and "PPI 10" entries to the timer.

-Doug

Re: [PATCH v4] clocksource: arch_timer: Allow the device tree to specify uninitialized timer registers

[Doug Anderson]

Yingjoe,

On Wed, Nov 26, 2014 at 6:41 AM, Yingjoe Chen <yingjoe.chen-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> Sorry for the (very) late reply.
> I just realize today MT8135 need this and the other patch [1] to boot
> SMP correctly. I've applied both patches and they works fine. Thanks :)

Excellent.  It's helpful to include a Tested-by: tag in your email.
You'd have a line with just "Tested-by: Yingjoe Chen
<yingjoe.chen-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>"


> However, I'm not sure if we really need to add new property.
> arm_arch_timer driver will only use virtual timer when virtual PPI
> interrupt is provided, so the following patch to timer dtsi will also
> works. I think if the firmware doesn't support virtual timer, it make
> sense to not supply virtual interrupt.
>
>         timer {
>                 compatible = "arm,armv7-timer";
>                 interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_LOW>,
> -                            <GIC_PPI 14 IRQ_TYPE_LEVEL_LOW>,
> -                            <GIC_PPI 11 IRQ_TYPE_LEVEL_LOW>,
> -                            <GIC_PPI 10 IRQ_TYPE_LEVEL_LOW>;
> +                            <GIC_PPI 14 IRQ_TYPE_LEVEL_LOW>;
>                 clock-frequency = <13000000>;
>         };

Once you have Sonny's patch then I believe that the above would work.
However we rejected something like this because device tree is
supposed to describe the hardware.  The hardware really does provide
the virtual timer interrupts and they really are at PPI 11 and PPI 10.
It's just that firmware doesn't handle things properly so they can't
be used.

NOTE: If we add the "arm,cpu-registers-not-fw-configured" to the
device tree and firmware actually works out how to configure things
(like if somehow has firmware that has a hypervisor) then it can
easily remove this device tree property before calling through to the
kernel.  It would be much harder for the firmware to add back in the
"PPI 11" and "PPI 10" entries to the timer.

-Doug
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4] clocksource: arch_timer: Allow the device tree to specify uninitialized timer registers

[Doug Anderson]

Yingjoe,

On Wed, Nov 26, 2014 at 6:41 AM, Yingjoe Chen <yingjoe.chen@mediatek.com> wrote:
> Sorry for the (very) late reply.
> I just realize today MT8135 need this and the other patch [1] to boot
> SMP correctly. I've applied both patches and they works fine. Thanks :)

Excellent.  It's helpful to include a Tested-by: tag in your email.
You'd have a line with just "Tested-by: Yingjoe Chen
<yingjoe.chen@mediatek.com>"


> However, I'm not sure if we really need to add new property.
> arm_arch_timer driver will only use virtual timer when virtual PPI
> interrupt is provided, so the following patch to timer dtsi will also
> works. I think if the firmware doesn't support virtual timer, it make
> sense to not supply virtual interrupt.
>
>         timer {
>                 compatible = "arm,armv7-timer";
>                 interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_LOW>,
> -                            <GIC_PPI 14 IRQ_TYPE_LEVEL_LOW>,
> -                            <GIC_PPI 11 IRQ_TYPE_LEVEL_LOW>,
> -                            <GIC_PPI 10 IRQ_TYPE_LEVEL_LOW>;
> +                            <GIC_PPI 14 IRQ_TYPE_LEVEL_LOW>;
>                 clock-frequency = <13000000>;
>         };

Once you have Sonny's patch then I believe that the above would work.
However we rejected something like this because device tree is
supposed to describe the hardware.  The hardware really does provide
the virtual timer interrupts and they really are at PPI 11 and PPI 10.
It's just that firmware doesn't handle things properly so they can't
be used.

NOTE: If we add the "arm,cpu-registers-not-fw-configured" to the
device tree and firmware actually works out how to configure things
(like if somehow has firmware that has a hypervisor) then it can
easily remove this device tree property before calling through to the
kernel.  It would be much harder for the firmware to add back in the
"PPI 11" and "PPI 10" entries to the timer.

-Doug

Re: [PATCH v4] clocksource: arch_timer: Allow the device tree to specify uninitialized timer registers

[Yingjoe Chen]

Hi,

On Wed, 2014-11-26 at 08:14 -0800, Doug Anderson wrote:
> Yingjoe,
> 
> On Wed, Nov 26, 2014 at 6:41 AM, Yingjoe Chen <yingjoe.chen@mediatek.com> wrote:
> > Sorry for the (very) late reply.
> > I just realize today MT8135 need this and the other patch [1] to boot
> > SMP correctly. I've applied both patches and they works fine. Thanks :)
> 
> Excellent.  It's helpful to include a Tested-by: tag in your email.
> You'd have a line with just "Tested-by: Yingjoe Chen
> <yingjoe.chen@mediatek.com>"

sure, here's my tested-by for the 2 patches

Tested-by: Yingjoe Chen <yingjoe.chen@mediatek.com>

I'll remember to add it next time :)


> > However, I'm not sure if we really need to add new property.
> > arm_arch_timer driver will only use virtual timer when virtual PPI
> > interrupt is provided, so the following patch to timer dtsi will also
> > works. I think if the firmware doesn't support virtual timer, it make
> > sense to not supply virtual interrupt.
> >
> >         timer {
> >                 compatible = "arm,armv7-timer";
> >                 interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_LOW>,
> > -                            <GIC_PPI 14 IRQ_TYPE_LEVEL_LOW>,
> > -                            <GIC_PPI 11 IRQ_TYPE_LEVEL_LOW>,
> > -                            <GIC_PPI 10 IRQ_TYPE_LEVEL_LOW>;
> > +                            <GIC_PPI 14 IRQ_TYPE_LEVEL_LOW>;
> >                 clock-frequency = <13000000>;
> >         };
> 
> Once you have Sonny's patch then I believe that the above would work.
> However we rejected something like this because device tree is
> supposed to describe the hardware.  The hardware really does provide
> the virtual timer interrupts and they really are at PPI 11 and PPI 10.
> It's just that firmware doesn't handle things properly so they can't
> be used.
> 
> NOTE: If we add the "arm,cpu-registers-not-fw-configured" to the
> device tree and firmware actually works out how to configure things
> (like if somehow has firmware that has a hypervisor) then it can
> easily remove this device tree property before calling through to the
> kernel.  It would be much harder for the firmware to add back in the
> "PPI 11" and "PPI 10" entries to the timer.
> 
> -Doug

I see your point, that's good for me then.
Thanks.

Joe.C

Re: [PATCH v4] clocksource: arch_timer: Allow the device tree to specify uninitialized timer registers

[Yingjoe Chen]

Hi,

On Wed, 2014-11-26 at 08:14 -0800, Doug Anderson wrote:
> Yingjoe,
> 
> On Wed, Nov 26, 2014 at 6:41 AM, Yingjoe Chen <yingjoe.chen@mediatek.com> wrote:
> > Sorry for the (very) late reply.
> > I just realize today MT8135 need this and the other patch [1] to boot
> > SMP correctly. I've applied both patches and they works fine. Thanks :)
> 
> Excellent.  It's helpful to include a Tested-by: tag in your email.
> You'd have a line with just "Tested-by: Yingjoe Chen
> <yingjoe.chen@mediatek.com>"

sure, here's my tested-by for the 2 patches

Tested-by: Yingjoe Chen <yingjoe.chen@mediatek.com>

I'll remember to add it next time :)


> > However, I'm not sure if we really need to add new property.
> > arm_arch_timer driver will only use virtual timer when virtual PPI
> > interrupt is provided, so the following patch to timer dtsi will also
> > works. I think if the firmware doesn't support virtual timer, it make
> > sense to not supply virtual interrupt.
> >
> >         timer {
> >                 compatible = "arm,armv7-timer";
> >                 interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_LOW>,
> > -                            <GIC_PPI 14 IRQ_TYPE_LEVEL_LOW>,
> > -                            <GIC_PPI 11 IRQ_TYPE_LEVEL_LOW>,
> > -                            <GIC_PPI 10 IRQ_TYPE_LEVEL_LOW>;
> > +                            <GIC_PPI 14 IRQ_TYPE_LEVEL_LOW>;
> >                 clock-frequency = <13000000>;
> >         };
> 
> Once you have Sonny's patch then I believe that the above would work.
> However we rejected something like this because device tree is
> supposed to describe the hardware.  The hardware really does provide
> the virtual timer interrupts and they really are at PPI 11 and PPI 10.
> It's just that firmware doesn't handle things properly so they can't
> be used.
> 
> NOTE: If we add the "arm,cpu-registers-not-fw-configured" to the
> device tree and firmware actually works out how to configure things
> (like if somehow has firmware that has a hypervisor) then it can
> easily remove this device tree property before calling through to the
> kernel.  It would be much harder for the firmware to add back in the
> "PPI 11" and "PPI 10" entries to the timer.
> 
> -Doug

I see your point, that's good for me then.
Thanks.

Joe.C

[PATCH v4] clocksource: arch_timer: Allow the device tree to specify uninitialized timer registers

[Yingjoe Chen]

Hi,

On Wed, 2014-11-26 at 08:14 -0800, Doug Anderson wrote:
> Yingjoe,
> 
> On Wed, Nov 26, 2014 at 6:41 AM, Yingjoe Chen <yingjoe.chen@mediatek.com> wrote:
> > Sorry for the (very) late reply.
> > I just realize today MT8135 need this and the other patch [1] to boot
> > SMP correctly. I've applied both patches and they works fine. Thanks :)
> 
> Excellent.  It's helpful to include a Tested-by: tag in your email.
> You'd have a line with just "Tested-by: Yingjoe Chen
> <yingjoe.chen@mediatek.com>"

sure, here's my tested-by for the 2 patches

Tested-by: Yingjoe Chen <yingjoe.chen@mediatek.com>

I'll remember to add it next time :)


> > However, I'm not sure if we really need to add new property.
> > arm_arch_timer driver will only use virtual timer when virtual PPI
> > interrupt is provided, so the following patch to timer dtsi will also
> > works. I think if the firmware doesn't support virtual timer, it make
> > sense to not supply virtual interrupt.
> >
> >         timer {
> >                 compatible = "arm,armv7-timer";
> >                 interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_LOW>,
> > -                            <GIC_PPI 14 IRQ_TYPE_LEVEL_LOW>,
> > -                            <GIC_PPI 11 IRQ_TYPE_LEVEL_LOW>,
> > -                            <GIC_PPI 10 IRQ_TYPE_LEVEL_LOW>;
> > +                            <GIC_PPI 14 IRQ_TYPE_LEVEL_LOW>;
> >                 clock-frequency = <13000000>;
> >         };
> 
> Once you have Sonny's patch then I believe that the above would work.
> However we rejected something like this because device tree is
> supposed to describe the hardware.  The hardware really does provide
> the virtual timer interrupts and they really are at PPI 11 and PPI 10.
> It's just that firmware doesn't handle things properly so they can't
> be used.
> 
> NOTE: If we add the "arm,cpu-registers-not-fw-configured" to the
> device tree and firmware actually works out how to configure things
> (like if somehow has firmware that has a hypervisor) then it can
> easily remove this device tree property before calling through to the
> kernel.  It would be much harder for the firmware to add back in the
> "PPI 11" and "PPI 10" entries to the timer.
> 
> -Doug

I see your point, that's good for me then.
Thanks.

Joe.C

Re: [PATCH v4] clocksource: arch_timer: Allow the device tree to specify uninitialized timer registers

[Catalin Marinas]

On Wed, Nov 26, 2014 at 12:49:42PM +0000, Daniel Lezcano wrote:
> On 10/08/2014 09:33 AM, Sonny Rao wrote:
> > From: Doug Anderson <dianders@chromium.org>
> >
> > Some 32-bit (ARMv7) systems are architected like this:
> >
> > * The firmware doesn't know and doesn't care about hypervisor mode and
> >    we don't want to add the complexity of hypervisor there.
> >
> > * The firmware isn't involved in SMP bringup or resume.
> >
> > * The ARCH timer come up with an uninitialized offset (CNTVOFF)
> >    between the virtual and physical counters.  Each core gets a
> >    different random offset.
> >
> > * The device boots in "Secure SVC" mode.
> >
> > * Nothing has touched the reset value of CNTHCTL.PL1PCEN or
> >    CNTHCTL.PL1PCTEN (both default to 1 at reset)
> >
> > On systems like the above, it doesn't make sense to use the virtual
> > counter.  There's nobody managing the offset and each time a core goes
> > down and comes back up it will get reinitialized to some other random
> > value.
> >
> > This adds an optional property which can inform the kernel of this
> > situation, and firmware is free to remove the property if it is going
> > to initialize the CNTVOFF registers when each CPU comes out of reset.
> >
> > Currently, the best course of action in this case is to use the
> > physical timer, which is why it is important that CNTHCTL hasn't been
> > changed from its reset value and it's a reasonable assumption given
> > that the firmware has never entered HYP mode.
> >
> > Note that it's been said that on ARMv8 systems the firmware and
> > kernel really can't be architected as described above.  That means
> > using the physical timer like this really only makes sense for ARMv7
> > systems.
> >
> > Signed-off-by: Doug Anderson <dianders@chromium.org>
> > Signed-off-by: Sonny Rao <sonnyrao@chromium.org>
> > Reviewed-by: Mark Rutland <mark.rutland@arm.com>
> 
> Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> 
> I would be nice to have Catalin's ack.

FWIW:

Acked-by: Catalin Marinas <catalin.marinas@arm.com>

Re: [PATCH v4] clocksource: arch_timer: Allow the device tree to specify uninitialized timer registers

[Catalin Marinas]

On Wed, Nov 26, 2014 at 12:49:42PM +0000, Daniel Lezcano wrote:
> On 10/08/2014 09:33 AM, Sonny Rao wrote:
> > From: Doug Anderson <dianders@chromium.org>
> >
> > Some 32-bit (ARMv7) systems are architected like this:
> >
> > * The firmware doesn't know and doesn't care about hypervisor mode and
> >    we don't want to add the complexity of hypervisor there.
> >
> > * The firmware isn't involved in SMP bringup or resume.
> >
> > * The ARCH timer come up with an uninitialized offset (CNTVOFF)
> >    between the virtual and physical counters.  Each core gets a
> >    different random offset.
> >
> > * The device boots in "Secure SVC" mode.
> >
> > * Nothing has touched the reset value of CNTHCTL.PL1PCEN or
> >    CNTHCTL.PL1PCTEN (both default to 1 at reset)
> >
> > On systems like the above, it doesn't make sense to use the virtual
> > counter.  There's nobody managing the offset and each time a core goes
> > down and comes back up it will get reinitialized to some other random
> > value.
> >
> > This adds an optional property which can inform the kernel of this
> > situation, and firmware is free to remove the property if it is going
> > to initialize the CNTVOFF registers when each CPU comes out of reset.
> >
> > Currently, the best course of action in this case is to use the
> > physical timer, which is why it is important that CNTHCTL hasn't been
> > changed from its reset value and it's a reasonable assumption given
> > that the firmware has never entered HYP mode.
> >
> > Note that it's been said that on ARMv8 systems the firmware and
> > kernel really can't be architected as described above.  That means
> > using the physical timer like this really only makes sense for ARMv7
> > systems.
> >
> > Signed-off-by: Doug Anderson <dianders@chromium.org>
> > Signed-off-by: Sonny Rao <sonnyrao@chromium.org>
> > Reviewed-by: Mark Rutland <mark.rutland@arm.com>
> 
> Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> 
> I would be nice to have Catalin's ack.

FWIW:

Acked-by: Catalin Marinas <catalin.marinas@arm.com>

[PATCH v4] clocksource: arch_timer: Allow the device tree to specify uninitialized timer registers

[Catalin Marinas]

On Wed, Nov 26, 2014 at 12:49:42PM +0000, Daniel Lezcano wrote:
> On 10/08/2014 09:33 AM, Sonny Rao wrote:
> > From: Doug Anderson <dianders@chromium.org>
> >
> > Some 32-bit (ARMv7) systems are architected like this:
> >
> > * The firmware doesn't know and doesn't care about hypervisor mode and
> >    we don't want to add the complexity of hypervisor there.
> >
> > * The firmware isn't involved in SMP bringup or resume.
> >
> > * The ARCH timer come up with an uninitialized offset (CNTVOFF)
> >    between the virtual and physical counters.  Each core gets a
> >    different random offset.
> >
> > * The device boots in "Secure SVC" mode.
> >
> > * Nothing has touched the reset value of CNTHCTL.PL1PCEN or
> >    CNTHCTL.PL1PCTEN (both default to 1 at reset)
> >
> > On systems like the above, it doesn't make sense to use the virtual
> > counter.  There's nobody managing the offset and each time a core goes
> > down and comes back up it will get reinitialized to some other random
> > value.
> >
> > This adds an optional property which can inform the kernel of this
> > situation, and firmware is free to remove the property if it is going
> > to initialize the CNTVOFF registers when each CPU comes out of reset.
> >
> > Currently, the best course of action in this case is to use the
> > physical timer, which is why it is important that CNTHCTL hasn't been
> > changed from its reset value and it's a reasonable assumption given
> > that the firmware has never entered HYP mode.
> >
> > Note that it's been said that on ARMv8 systems the firmware and
> > kernel really can't be architected as described above.  That means
> > using the physical timer like this really only makes sense for ARMv7
> > systems.
> >
> > Signed-off-by: Doug Anderson <dianders@chromium.org>
> > Signed-off-by: Sonny Rao <sonnyrao@chromium.org>
> > Reviewed-by: Mark Rutland <mark.rutland@arm.com>
> 
> Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> 
> I would be nice to have Catalin's ack.

FWIW:

Acked-by: Catalin Marinas <catalin.marinas@arm.com>

Re: [PATCH v4] clocksource: arch_timer: Allow the device tree to specify uninitialized timer registers

[Olof Johansson]

On Wed, Oct 08, 2014 at 12:33:47AM -0700, Sonny Rao wrote:
> From: Doug Anderson <dianders@chromium.org>
> 
> Some 32-bit (ARMv7) systems are architected like this:
> 
> * The firmware doesn't know and doesn't care about hypervisor mode and
>   we don't want to add the complexity of hypervisor there.
> 
> * The firmware isn't involved in SMP bringup or resume.
> 
> * The ARCH timer come up with an uninitialized offset (CNTVOFF)
>   between the virtual and physical counters.  Each core gets a
>   different random offset.
> 
> * The device boots in "Secure SVC" mode.
> 
> * Nothing has touched the reset value of CNTHCTL.PL1PCEN or
>   CNTHCTL.PL1PCTEN (both default to 1 at reset)
> 
> On systems like the above, it doesn't make sense to use the virtual
> counter.  There's nobody managing the offset and each time a core goes
> down and comes back up it will get reinitialized to some other random
> value.
> 
> This adds an optional property which can inform the kernel of this
> situation, and firmware is free to remove the property if it is going
> to initialize the CNTVOFF registers when each CPU comes out of reset.
> 
> Currently, the best course of action in this case is to use the
> physical timer, which is why it is important that CNTHCTL hasn't been
> changed from its reset value and it's a reasonable assumption given
> that the firmware has never entered HYP mode.
> 
> Note that it's been said that on ARMv8 systems the firmware and
> kernel really can't be architected as described above.  That means
> using the physical timer like this really only makes sense for ARMv7
> systems.
> 
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> Signed-off-by: Sonny Rao <sonnyrao@chromium.org>
> Reviewed-by: Mark Rutland <mark.rutland@arm.com>
> ---
> Changes in v2:
> - Add "#ifdef CONFIG_ARM" as per Will Deacon
> 
> Changes in v3:
> - change property name to arm,cntvoff-not-fw-configured and specify
>   that the value of CNTHCTL.PL1PC(T)EN must still be the reset value
>   of 1 as per Mark Rutland
> 
> Changes in v4:
> - change property name to arm,cpu-registers-not-fw-configured and
>   specify that all cpu registers must have architected reset values
>   per Mark Rutland
> - change from "#ifdef CONFIG_ARM" to "if (IS_ENABLED(CONFIG_ARM))" per
>   Arnd Bergmann


Applied to next/drivers (and next/dt for rk3288 dependency). Thanks, all!


-Olof

Re: [PATCH v4] clocksource: arch_timer: Allow the device tree to specify uninitialized timer registers

[Olof Johansson]

On Wed, Oct 08, 2014 at 12:33:47AM -0700, Sonny Rao wrote:
> From: Doug Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> 
> Some 32-bit (ARMv7) systems are architected like this:
> 
> * The firmware doesn't know and doesn't care about hypervisor mode and
>   we don't want to add the complexity of hypervisor there.
> 
> * The firmware isn't involved in SMP bringup or resume.
> 
> * The ARCH timer come up with an uninitialized offset (CNTVOFF)
>   between the virtual and physical counters.  Each core gets a
>   different random offset.
> 
> * The device boots in "Secure SVC" mode.
> 
> * Nothing has touched the reset value of CNTHCTL.PL1PCEN or
>   CNTHCTL.PL1PCTEN (both default to 1 at reset)
> 
> On systems like the above, it doesn't make sense to use the virtual
> counter.  There's nobody managing the offset and each time a core goes
> down and comes back up it will get reinitialized to some other random
> value.
> 
> This adds an optional property which can inform the kernel of this
> situation, and firmware is free to remove the property if it is going
> to initialize the CNTVOFF registers when each CPU comes out of reset.
> 
> Currently, the best course of action in this case is to use the
> physical timer, which is why it is important that CNTHCTL hasn't been
> changed from its reset value and it's a reasonable assumption given
> that the firmware has never entered HYP mode.
> 
> Note that it's been said that on ARMv8 systems the firmware and
> kernel really can't be architected as described above.  That means
> using the physical timer like this really only makes sense for ARMv7
> systems.
> 
> Signed-off-by: Doug Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> Signed-off-by: Sonny Rao <sonnyrao-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> Reviewed-by: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
> ---
> Changes in v2:
> - Add "#ifdef CONFIG_ARM" as per Will Deacon
> 
> Changes in v3:
> - change property name to arm,cntvoff-not-fw-configured and specify
>   that the value of CNTHCTL.PL1PC(T)EN must still be the reset value
>   of 1 as per Mark Rutland
> 
> Changes in v4:
> - change property name to arm,cpu-registers-not-fw-configured and
>   specify that all cpu registers must have architected reset values
>   per Mark Rutland
> - change from "#ifdef CONFIG_ARM" to "if (IS_ENABLED(CONFIG_ARM))" per
>   Arnd Bergmann


Applied to next/drivers (and next/dt for rk3288 dependency). Thanks, all!


-Olof
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4] clocksource: arch_timer: Allow the device tree to specify uninitialized timer registers

[Olof Johansson]

On Wed, Oct 08, 2014 at 12:33:47AM -0700, Sonny Rao wrote:
> From: Doug Anderson <dianders@chromium.org>
> 
> Some 32-bit (ARMv7) systems are architected like this:
> 
> * The firmware doesn't know and doesn't care about hypervisor mode and
>   we don't want to add the complexity of hypervisor there.
> 
> * The firmware isn't involved in SMP bringup or resume.
> 
> * The ARCH timer come up with an uninitialized offset (CNTVOFF)
>   between the virtual and physical counters.  Each core gets a
>   different random offset.
> 
> * The device boots in "Secure SVC" mode.
> 
> * Nothing has touched the reset value of CNTHCTL.PL1PCEN or
>   CNTHCTL.PL1PCTEN (both default to 1 at reset)
> 
> On systems like the above, it doesn't make sense to use the virtual
> counter.  There's nobody managing the offset and each time a core goes
> down and comes back up it will get reinitialized to some other random
> value.
> 
> This adds an optional property which can inform the kernel of this
> situation, and firmware is free to remove the property if it is going
> to initialize the CNTVOFF registers when each CPU comes out of reset.
> 
> Currently, the best course of action in this case is to use the
> physical timer, which is why it is important that CNTHCTL hasn't been
> changed from its reset value and it's a reasonable assumption given
> that the firmware has never entered HYP mode.
> 
> Note that it's been said that on ARMv8 systems the firmware and
> kernel really can't be architected as described above.  That means
> using the physical timer like this really only makes sense for ARMv7
> systems.
> 
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> Signed-off-by: Sonny Rao <sonnyrao@chromium.org>
> Reviewed-by: Mark Rutland <mark.rutland@arm.com>
> ---
> Changes in v2:
> - Add "#ifdef CONFIG_ARM" as per Will Deacon
> 
> Changes in v3:
> - change property name to arm,cntvoff-not-fw-configured and specify
>   that the value of CNTHCTL.PL1PC(T)EN must still be the reset value
>   of 1 as per Mark Rutland
> 
> Changes in v4:
> - change property name to arm,cpu-registers-not-fw-configured and
>   specify that all cpu registers must have architected reset values
>   per Mark Rutland
> - change from "#ifdef CONFIG_ARM" to "if (IS_ENABLED(CONFIG_ARM))" per
>   Arnd Bergmann


Applied to next/drivers (and next/dt for rk3288 dependency). Thanks, all!


-Olof