These (mostly minor) patches fix a few typos, improve points that
were agreed upon when the Trusted Foundation series was initially
submitted, and more importantly add support for a prepare_idle()
firmware operation that informs the firmware a CPU is doing idle.
Tegra's cpuidle driver is then also updated accordingly.
These patches should be the last step before device trees for NVIDIA
SHIELD and Tegra Note 7 can be submitted.
Changes since v1:
- Do not remove TF support from tegra_defconfig (will automatically be taken
care of during next configuration update)
- Add a new prepare_idle() operation to firmware_ops that informs the firmware
a CPU is going idle (vs. asking the firmware to do it itself as do_idle()
does)
- Fix idle states names in TF implementation of prepare_idle to sound less
Tegra-specific
Alexandre Courbot (6):
ARM: trusted_foundations: fix vendor prefix typos
ARM: trusted_foundations: fallback when TF support is missing
ARM: firmware: enable Trusted Foundations by default
ARM: firmware: add prepare_idle() operation
ARM: trusted_foundations: implement prepare_idle()
ARM: tegra: cpuidle: use firmware for power down
arch/arm/firmware/Kconfig | 3 ++-
arch/arm/firmware/trusted_foundations.c | 20 +++++++++++++++++++-
arch/arm/include/asm/firmware.h | 4 ++++
arch/arm/include/asm/trusted_foundations.h | 13 +++++++++----
arch/arm/mach-tegra/cpuidle-tegra114.c | 11 ++++++++++-
5 files changed, 44 insertions(+), 7 deletions(-)
--
1.8.5.3
of_register_trusted_foundations() and the firmware Kconfig used
the wrong vendor prefix for Trusted Logic Mobility.
Signed-off-by: Alexandre Courbot <[email protected]>
---
arch/arm/firmware/Kconfig | 2 +-
arch/arm/include/asm/trusted_foundations.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/firmware/Kconfig b/arch/arm/firmware/Kconfig
index bb00ccf00d66..bb126594995e 100644
--- a/arch/arm/firmware/Kconfig
+++ b/arch/arm/firmware/Kconfig
@@ -20,7 +20,7 @@ config TRUSTED_FOUNDATIONS
This option allows the kernel to invoke the secure monitor whenever
required on devices using Trusted Foundations. See
arch/arm/include/asm/trusted_foundations.h or the
- tl,trusted-foundations device tree binding documentation for details
+ tlm,trusted-foundations device tree binding documentation for details
on how to use it.
Say n if you don't know what this is about.
diff --git a/arch/arm/include/asm/trusted_foundations.h b/arch/arm/include/asm/trusted_foundations.h
index 3bd36e2c5f2e..997862fd5d77 100644
--- a/arch/arm/include/asm/trusted_foundations.h
+++ b/arch/arm/include/asm/trusted_foundations.h
@@ -59,7 +59,7 @@ static inline void of_register_trusted_foundations(void)
* If we find the target should enable TF but does not support it,
* fail as the system won't be able to do much anyway
*/
- if (of_find_compatible_node(NULL, NULL, "tl,trusted-foundations"))
+ if (of_find_compatible_node(NULL, NULL, "tlm,trusted-foundations"))
register_trusted_foundations(NULL);
}
#endif /* CONFIG_TRUSTED_FOUNDATIONS */
--
1.8.5.3
As discussed previously (https://lkml.org/lkml/2013/11/26/289), enable
Trusted Foundation support by default since it already depends on a
supporting architecture being selected.
Signed-off-by: Alexandre Courbot <[email protected]>
---
arch/arm/firmware/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/firmware/Kconfig b/arch/arm/firmware/Kconfig
index bb126594995e..ad396af68e47 100644
--- a/arch/arm/firmware/Kconfig
+++ b/arch/arm/firmware/Kconfig
@@ -11,6 +11,7 @@ menu "Firmware options"
config TRUSTED_FOUNDATIONS
bool "Trusted Foundations secure monitor support"
depends on ARCH_SUPPORTS_TRUSTED_FOUNDATIONS
+ default y
help
Some devices (including most Tegra-based consumer devices on the
market) are booted with the Trusted Foundations secure monitor
--
1.8.5.3
Support the prepare_idle() firmware call, which is necessary to properly
support CPU idling.
Signed-off-by: Alexandre Courbot <[email protected]>
---
arch/arm/firmware/trusted_foundations.c | 20 +++++++++++++++++++-
1 file changed, 19 insertions(+), 1 deletion(-)
diff --git a/arch/arm/firmware/trusted_foundations.c b/arch/arm/firmware/trusted_foundations.c
index ef1e3d8f4af0..3fb1b5a1dce9 100644
--- a/arch/arm/firmware/trusted_foundations.c
+++ b/arch/arm/firmware/trusted_foundations.c
@@ -22,6 +22,15 @@
#define TF_SET_CPU_BOOT_ADDR_SMC 0xfffff200
+#define TF_CPU_PM 0xfffffffc
+#define TF_CPU_PM_S3 0xffffffe3
+#define TF_CPU_PM_S2 0xffffffe6
+#define TF_CPU_PM_S2_NO_MC_CLK 0xffffffe5
+#define TF_CPU_PM_S1 0xffffffe4
+#define TF_CPU_PM_S1_NOFLUSH_L2 0xffffffe7
+
+static unsigned long cpu_boot_addr;
+
static void __naked tf_generic_smc(u32 type, u32 arg1, u32 arg2)
{
asm volatile(
@@ -41,13 +50,22 @@ static void __naked tf_generic_smc(u32 type, u32 arg1, u32 arg2)
static int tf_set_cpu_boot_addr(int cpu, unsigned long boot_addr)
{
- tf_generic_smc(TF_SET_CPU_BOOT_ADDR_SMC, boot_addr, 0);
+ cpu_boot_addr = boot_addr;
+ tf_generic_smc(TF_SET_CPU_BOOT_ADDR_SMC, cpu_boot_addr, 0);
+
+ return 0;
+}
+
+static int tf_prepare_idle(void)
+{
+ tf_generic_smc(TF_CPU_PM, TF_CPU_PM_S1_NOFLUSH_L2, cpu_boot_addr);
return 0;
}
static const struct firmware_ops trusted_foundations_ops = {
.set_cpu_boot_addr = tf_set_cpu_boot_addr,
+ .prepare_idle = tf_prepare_idle,
};
void register_trusted_foundations(struct trusted_foundations_platform_data *pd)
--
1.8.5.3
Attempt to invoke the prepare_idle() and do_idle() firmware calls
to power down a CPU so an underlying firmware gets informed of
the idle operation and performs it by itself if designed in such a way.
Signed-off-by: Alexandre Courbot <[email protected]>
---
arch/arm/mach-tegra/cpuidle-tegra114.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/arch/arm/mach-tegra/cpuidle-tegra114.c b/arch/arm/mach-tegra/cpuidle-tegra114.c
index e0b87300243d..558067ddc186 100644
--- a/arch/arm/mach-tegra/cpuidle-tegra114.c
+++ b/arch/arm/mach-tegra/cpuidle-tegra114.c
@@ -19,6 +19,7 @@
#include <linux/cpuidle.h>
#include <linux/cpu_pm.h>
#include <linux/clockchips.h>
+#include <asm/firmware.h>
#include <asm/cpuidle.h>
#include <asm/suspend.h>
@@ -45,7 +46,15 @@ static int tegra114_idle_power_down(struct cpuidle_device *dev,
clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &dev->cpu);
- cpu_suspend(0, tegra30_sleep_cpu_secondary_finish);
+ call_firmware_op(prepare_idle);
+
+ switch (call_firmware_op(do_idle)) {
+ case -ENOSYS:
+ cpu_suspend(0, tegra30_sleep_cpu_secondary_finish);
+ break;
+ default:
+ break;
+ }
clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &dev->cpu);
--
1.8.5.3
Some firmwares do not put the CPU into idle mode themselves, but still
need to be informed that the CPU is about to enter idle mode before this
happens. Add a prepare_idle() operation to the firmware_ops structure to
handle such cases.
Signed-off-by: Alexandre Courbot <[email protected]>
---
arch/arm/include/asm/firmware.h | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/arm/include/asm/firmware.h b/arch/arm/include/asm/firmware.h
index 15631300c238..2c9f10df7568 100644
--- a/arch/arm/include/asm/firmware.h
+++ b/arch/arm/include/asm/firmware.h
@@ -22,6 +22,10 @@
*/
struct firmware_ops {
/*
+ * Inform the firmware we intend to enter CPU idle mode
+ */
+ int (*prepare_idle)(void);
+ /*
* Enters CPU idle mode
*/
int (*do_idle)(void);
--
1.8.5.3
When Trusted Foundations is detected as present on the system, but
Trusted Foundations support is not built into the kernel, the kernel
used to issue a panic very early during boot, leaving little clue to the
user as to what is going wrong.
It turns out that even without TF support built-in, the kernel can boot
on a TF-enabled system provided that SMP and cpuidle are disabled. This
patch does this and continue booting on one CPU, leaving the user with a
usable (however degraded) system.
Signed-off-by: Alexandre Courbot <[email protected]>
---
arch/arm/include/asm/trusted_foundations.h | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/arch/arm/include/asm/trusted_foundations.h b/arch/arm/include/asm/trusted_foundations.h
index 997862fd5d77..b5f7705abcb0 100644
--- a/arch/arm/include/asm/trusted_foundations.h
+++ b/arch/arm/include/asm/trusted_foundations.h
@@ -30,6 +30,8 @@
#include <linux/printk.h>
#include <linux/bug.h>
#include <linux/of.h>
+#include <linux/cpu.h>
+#include <linux/smp.h>
struct trusted_foundations_platform_data {
unsigned int version_major;
@@ -47,10 +49,13 @@ static inline void register_trusted_foundations(
struct trusted_foundations_platform_data *pd)
{
/*
- * If we try to register TF, this means the system needs it to continue.
- * Its absence if thus a fatal error.
+ * If the system requires TF and we cannot provide it, continue booting
+ * but disable features that cannot be provided.
*/
- panic("No support for Trusted Foundations, stopping...\n");
+ pr_err("No support for Trusted Foundations, continuing in degraded mode.\n");
+ pr_err("Secondary processors as well as CPU PM will be disabled.\n");
+ setup_max_cpus = 0;
+ cpu_idle_poll_ctrl(true);
}
static inline void of_register_trusted_foundations(void)
--
1.8.5.3
On Fri, Feb 07, 2014 at 01:35:06PM +0900, Alexandre Courbot wrote:
> Attempt to invoke the prepare_idle() and do_idle() firmware calls
> to power down a CPU so an underlying firmware gets informed of
> the idle operation and performs it by itself if designed in such a way.
>
> Signed-off-by: Alexandre Courbot <[email protected]>
> ---
> arch/arm/mach-tegra/cpuidle-tegra114.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-tegra/cpuidle-tegra114.c b/arch/arm/mach-tegra/cpuidle-tegra114.c
> index e0b87300243d..558067ddc186 100644
> --- a/arch/arm/mach-tegra/cpuidle-tegra114.c
> +++ b/arch/arm/mach-tegra/cpuidle-tegra114.c
> @@ -19,6 +19,7 @@
> #include <linux/cpuidle.h>
> #include <linux/cpu_pm.h>
> #include <linux/clockchips.h>
> +#include <asm/firmware.h>
>
> #include <asm/cpuidle.h>
> #include <asm/suspend.h>
> @@ -45,7 +46,15 @@ static int tegra114_idle_power_down(struct cpuidle_device *dev,
>
> clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &dev->cpu);
>
> - cpu_suspend(0, tegra30_sleep_cpu_secondary_finish);
> + call_firmware_op(prepare_idle);
> +
> + switch (call_firmware_op(do_idle)) {
> + case -ENOSYS:
> + cpu_suspend(0, tegra30_sleep_cpu_secondary_finish);
> + break;
> + default:
> + break;
Do you expect other cases down the road? If not, this is a simple if instead:
/* Only call cpu_suspend if TF didn't handle the pre-suspend logic */
if (call_firmware_op(do_idle) == -ENOSYS)
cpu_suspend(....);
-Olof
On Fri, Feb 07, 2014 at 01:35:00PM +0900, Alexandre Courbot wrote:
> These (mostly minor) patches fix a few typos, improve points that
> were agreed upon when the Trusted Foundation series was initially
> submitted, and more importantly add support for a prepare_idle()
> firmware operation that informs the firmware a CPU is doing idle.
> Tegra's cpuidle driver is then also updated accordingly.
>
> These patches should be the last step before device trees for NVIDIA
> SHIELD and Tegra Note 7 can be submitted.
>
> Changes since v1:
> - Do not remove TF support from tegra_defconfig (will automatically be taken
> care of during next configuration update)
> - Add a new prepare_idle() operation to firmware_ops that informs the firmware
> a CPU is going idle (vs. asking the firmware to do it itself as do_idle()
> does)
> - Fix idle states names in TF implementation of prepare_idle to sound less
> Tegra-specific
1-5:
Acked-by: Olof Johansson <[email protected]>
Stephen asked if he should merge through Russell's tree or ours; since
tegra is the only user I don't have a problem merging this (via tegra
pull requests) but I'm giving Russell veto power. :)
-Olof
On 02/12/2014 10:36 AM, Olof Johansson wrote:
> On Fri, Feb 07, 2014 at 01:35:06PM +0900, Alexandre Courbot wrote:
>> Attempt to invoke the prepare_idle() and do_idle() firmware calls
>> to power down a CPU so an underlying firmware gets informed of
>> the idle operation and performs it by itself if designed in such a way.
>>
>> Signed-off-by: Alexandre Courbot <[email protected]>
>> ---
>> arch/arm/mach-tegra/cpuidle-tegra114.c | 11 ++++++++++-
>> 1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/mach-tegra/cpuidle-tegra114.c b/arch/arm/mach-tegra/cpuidle-tegra114.c
>> index e0b87300243d..558067ddc186 100644
>> --- a/arch/arm/mach-tegra/cpuidle-tegra114.c
>> +++ b/arch/arm/mach-tegra/cpuidle-tegra114.c
>> @@ -19,6 +19,7 @@
>> #include <linux/cpuidle.h>
>> #include <linux/cpu_pm.h>
>> #include <linux/clockchips.h>
>> +#include <asm/firmware.h>
>>
>> #include <asm/cpuidle.h>
>> #include <asm/suspend.h>
>> @@ -45,7 +46,15 @@ static int tegra114_idle_power_down(struct cpuidle_device *dev,
>>
>> clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &dev->cpu);
>>
>> - cpu_suspend(0, tegra30_sleep_cpu_secondary_finish);
>> + call_firmware_op(prepare_idle);
>> +
>> + switch (call_firmware_op(do_idle)) {
>> + case -ENOSYS:
>> + cpu_suspend(0, tegra30_sleep_cpu_secondary_finish);
>> + break;
>> + default:
>> + break;
>
> Do you expect other cases down the road? If not, this is a simple if instead:
>
> /* Only call cpu_suspend if TF didn't handle the pre-suspend logic */
> if (call_firmware_op(do_idle) == -ENOSYS)
> cpu_suspend(....);
I might have been overdoing it indeed. Let me submit a v3 just for this
one patch.
Thanks!
Alex.
Attempt to invoke the prepare_idle() and do_idle() firmware calls
to power down a CPU so an underlying firmware gets informed of
the idle operation and performs it by itself if designed in such a way.
Signed-off-by: Alexandre Courbot <[email protected]>
---
This is the only v3 patch in the series.
Changes since v2:
- Use an if statement instead of a switch to handle the single return
code we need to care about.
arch/arm/mach-tegra/cpuidle-tegra114.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/arm/mach-tegra/cpuidle-tegra114.c b/arch/arm/mach-tegra/cpuidle-tegra114.c
index e0b87300243d..b5fb7c110c64 100644
--- a/arch/arm/mach-tegra/cpuidle-tegra114.c
+++ b/arch/arm/mach-tegra/cpuidle-tegra114.c
@@ -19,6 +19,7 @@
#include <linux/cpuidle.h>
#include <linux/cpu_pm.h>
#include <linux/clockchips.h>
+#include <asm/firmware.h>
#include <asm/cpuidle.h>
#include <asm/suspend.h>
@@ -45,7 +46,11 @@ static int tegra114_idle_power_down(struct cpuidle_device *dev,
clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &dev->cpu);
- cpu_suspend(0, tegra30_sleep_cpu_secondary_finish);
+ call_firmware_op(prepare_idle);
+
+ /* Do suspend by ourselves if the firmware does not implement it */
+ if (call_firmware_op(do_idle) == -ENOSYS)
+ cpu_suspend(0, tegra30_sleep_cpu_secondary_finish);
clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &dev->cpu);
--
1.8.5.4
On Tue, Feb 11, 2014 at 6:43 PM, Alexandre Courbot <[email protected]> wrote:
> Attempt to invoke the prepare_idle() and do_idle() firmware calls
> to power down a CPU so an underlying firmware gets informed of
> the idle operation and performs it by itself if designed in such a way.
>
> Signed-off-by: Alexandre Courbot <[email protected]>
Acked-by: Olof Johansson <[email protected]>
-Olof
On 02/11/2014 06:38 PM, Olof Johansson wrote:
> On Fri, Feb 07, 2014 at 01:35:00PM +0900, Alexandre Courbot wrote:
>> These (mostly minor) patches fix a few typos, improve points that
>> were agreed upon when the Trusted Foundation series was initially
>> submitted, and more importantly add support for a prepare_idle()
>> firmware operation that informs the firmware a CPU is doing idle.
>> Tegra's cpuidle driver is then also updated accordingly.
>>
>> These patches should be the last step before device trees for NVIDIA
>> SHIELD and Tegra Note 7 can be submitted.
>>
>> Changes since v1:
>> - Do not remove TF support from tegra_defconfig (will automatically be taken
>> care of during next configuration update)
>> - Add a new prepare_idle() operation to firmware_ops that informs the firmware
>> a CPU is going idle (vs. asking the firmware to do it itself as do_idle()
>> does)
>> - Fix idle states names in TF implementation of prepare_idle to sound less
>> Tegra-specific
>
> 1-5:
>
> Acked-by: Olof Johansson <[email protected]>
>
> Stephen asked if he should merge through Russell's tree or ours; since
> tegra is the only user I don't have a problem merging this (via tegra
> pull requests) but I'm giving Russell veto power. :)
Russell, I intend to apply this series to the Tegra tree on Fri US time,
unless I hear otherwise from you. Thanks.
Hi Alexandre,
On 07.02.2014 05:35, Alexandre Courbot wrote:
> Some firmwares do not put the CPU into idle mode themselves, but still
> need to be informed that the CPU is about to enter idle mode before this
> happens. Add a prepare_idle() operation to the firmware_ops structure to
> handle such cases.
>
> Signed-off-by: Alexandre Courbot <[email protected]>
> ---
> arch/arm/include/asm/firmware.h | 4 ++++
> 1 file changed, 4 insertions(+)
I wonder if .do_idle() couldn't simply return an appropriate error code
to let the upper layer know that it should proceed with normal CPU idle
activation, while still letting the firmware know that the CPU is going
to idle.
Best regards,
Tomasz
On 02/13/2014 04:01 AM, Tomasz Figa wrote:
> Hi Alexandre,
>
> On 07.02.2014 05:35, Alexandre Courbot wrote:
>> Some firmwares do not put the CPU into idle mode themselves, but still
>> need to be informed that the CPU is about to enter idle mode before this
>> happens. Add a prepare_idle() operation to the firmware_ops structure to
>> handle such cases.
>>
>> Signed-off-by: Alexandre Courbot <[email protected]>
>> ---
>> arch/arm/include/asm/firmware.h | 4 ++++
>> 1 file changed, 4 insertions(+)
>
> I wonder if .do_idle() couldn't simply return an appropriate error code
> to let the upper layer know that it should proceed with normal CPU idle
> activation, while still letting the firmware know that the CPU is going
> to idle.
That seems to disagree with the naming of the operation, and the
semantics I assume it has, though. It seems clearer to add an explicit
separate op for this.
On 02/13/2014 08:01 PM, Tomasz Figa wrote:
> Hi Alexandre,
>
> On 07.02.2014 05:35, Alexandre Courbot wrote:
>> Some firmwares do not put the CPU into idle mode themselves, but still
>> need to be informed that the CPU is about to enter idle mode before this
>> happens. Add a prepare_idle() operation to the firmware_ops structure to
>> handle such cases.
>>
>> Signed-off-by: Alexandre Courbot <[email protected]>
>> ---
>> arch/arm/include/asm/firmware.h | 4 ++++
>> 1 file changed, 4 insertions(+)
>
> I wonder if .do_idle() couldn't simply return an appropriate error code
> to let the upper layer know that it should proceed with normal CPU idle
> activation, while still letting the firmware know that the CPU is going
> to idle.
In our particular case I agree it would be enough to use do_idle() to
let the firmware know about the operation and have it return -ENOSYS so
the kernel actually performs it. I'm afraid this might not fulfill all
needs though (e.g. one can imagine a firmware where the OS needs to take
action between the notification and the actual shutdown), and as Stephen
pointed out that would make the name of the function ambiguous at best.
I'd rather keep it the current way for clarity.
On 14.02.2014 06:16, Alexandre Courbot wrote:
> On 02/13/2014 08:01 PM, Tomasz Figa wrote:
>> Hi Alexandre,
>>
>> On 07.02.2014 05:35, Alexandre Courbot wrote:
>>> Some firmwares do not put the CPU into idle mode themselves, but still
>>> need to be informed that the CPU is about to enter idle mode before this
>>> happens. Add a prepare_idle() operation to the firmware_ops structure to
>>> handle such cases.
>>>
>>> Signed-off-by: Alexandre Courbot <[email protected]>
>>> ---
>>> arch/arm/include/asm/firmware.h | 4 ++++
>>> 1 file changed, 4 insertions(+)
>>
>> I wonder if .do_idle() couldn't simply return an appropriate error code
>> to let the upper layer know that it should proceed with normal CPU idle
>> activation, while still letting the firmware know that the CPU is going
>> to idle.
>
> In our particular case I agree it would be enough to use do_idle() to
> let the firmware know about the operation and have it return -ENOSYS so
> the kernel actually performs it. I'm afraid this might not fulfill all
> needs though (e.g. one can imagine a firmware where the OS needs to take
> action between the notification and the actual shutdown), and as Stephen
> pointed out that would make the name of the function ambiguous at best.
> I'd rather keep it the current way for clarity.
>
OK. I'm not strongly against this, just wanted some more thought on
this, so please move on.
Best regards,
Tomasz
On 02/06/2014 09:35 PM, Alexandre Courbot wrote:
> These (mostly minor) patches fix a few typos, improve points that
> were agreed upon when the Trusted Foundation series was initially
> submitted, and more importantly add support for a prepare_idle()
> firmware operation that informs the firmware a CPU is doing idle.
> Tegra's cpuidle driver is then also updated accordingly.
>
> These patches should be the last step before device trees for NVIDIA
> SHIELD and Tegra Note 7 can be submitted.
I've applied this series to Tegra's for-3.15/tf branch (using V3 of
patch 6/6)