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 the do_idle() firmware
operation that is needed for cpuidle to be supported. Tegra's cpuidle
driver is also updated accordingly.
These patches should be the last step before the device trees for NVIDIA
SHIELD and Tegra Note 7 can be submitted.
Alexandre Courbot (5):
ARM: trusted_foundations: fix vendor prefix typos
ARM: trusted_foundations: fallback when TF support is missing
ARM: firmware: enable Trusted Foundations by default
ARM: trusted_foundations: implement do_idle()
ARM: tegra: cpuidle: use firmware call for power down
arch/arm/configs/tegra_defconfig | 1 -
arch/arm/firmware/Kconfig | 3 ++-
arch/arm/firmware/trusted_foundations.c | 20 +++++++++++++++++++-
arch/arm/include/asm/trusted_foundations.h | 13 +++++++++----
arch/arm/mach-tegra/cpuidle-tegra114.c | 3 +++
5 files changed, 33 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
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
Invoke the do_idle() firmware call before suspending a CPU so that the
underlying firmware (if any) can take necessary action.
Signed-off-by: Alexandre Courbot <[email protected]>
---
arch/arm/mach-tegra/cpuidle-tegra114.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/arm/mach-tegra/cpuidle-tegra114.c b/arch/arm/mach-tegra/cpuidle-tegra114.c
index e0b87300243d..c42fd41065d2 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,6 +46,8 @@ static int tegra114_idle_power_down(struct cpuidle_device *dev,
clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &dev->cpu);
+ call_firmware_op(do_idle);
+
cpu_suspend(0, tegra30_sleep_cpu_secondary_finish);
clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &dev->cpu);
--
1.8.5.3
Support the do_idle() firmware call, which is necessary to properly
support cpuidle.
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..7c14af809995 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_LP0 0xffffffe3
+#define TF_CPU_PM_LP1 0xffffffe6
+#define TF_CPU_PM_LP1_NO_MC_CLK 0xffffffe5
+#define TF_CPU_PM_LP2 0xffffffe4
+#define TF_CPU_PM_LP2_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_do_idle(void)
+{
+ tf_generic_smc(TF_CPU_PM, TF_CPU_PM_LP2_NOFLUSH_L2, cpu_boot_addr);
return 0;
}
static const struct firmware_ops trusted_foundations_ops = {
.set_cpu_boot_addr = tf_set_cpu_boot_addr,
+ .do_idle = tf_do_idle,
};
void register_trusted_foundations(struct trusted_foundations_platform_data *pd)
--
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.
Doing so allows us to remove it from tegra_defconfig.
Signed-off-by: Alexandre Courbot <[email protected]>
---
arch/arm/configs/tegra_defconfig | 1 -
arch/arm/firmware/Kconfig | 1 +
2 files changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/configs/tegra_defconfig b/arch/arm/configs/tegra_defconfig
index 5fdc9a09d339..e38653876541 100644
--- a/arch/arm/configs/tegra_defconfig
+++ b/arch/arm/configs/tegra_defconfig
@@ -33,7 +33,6 @@ CONFIG_PCI=y
CONFIG_PCI_MSI=y
CONFIG_PCI_TEGRA=y
CONFIG_PCIEPORTBUS=y
-CONFIG_TRUSTED_FOUNDATIONS=y
CONFIG_SMP=y
CONFIG_PREEMPT=y
CONFIG_AEABI=y
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
On 01/21/2014 03:10 AM, Alexandre Courbot wrote:
> 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.
>
> Doing so allows us to remove it from tegra_defconfig.
> arch/arm/configs/tegra_defconfig | 1 -
> arch/arm/firmware/Kconfig | 1 +
Can we split out the defconfig and code changes into separate patches?
They need to go through seperate branches, possibly even separate repos.
The defconfig change might not even be necessary; at some point I'll
just rebuild it via "make tegra_defconfig; make savedefconfig" on top of
some linux-next that includes the Kconfig change, and it'll happen
automatically. Still, I guess there's no harm explicitly sending the patch.
On 01/21/2014 03:10 AM, Alexandre Courbot wrote:
> Support the do_idle() firmware call, which is necessary to properly
> support cpuidle.
> diff --git a/arch/arm/firmware/trusted_foundations.c b/arch/arm/firmware/trusted_foundations.c
> +#define TF_CPU_PM 0xfffffffc
> +#define TF_CPU_PM_LP0 0xffffffe3
> +#define TF_CPU_PM_LP1 0xffffffe6
> +#define TF_CPU_PM_LP1_NO_MC_CLK 0xffffffe5
> +#define TF_CPU_PM_LP2 0xffffffe4
> +#define TF_CPU_PM_LP2_NOFLUSH_L2 0xffffffe7
Hmm. This must be Tegra-specific, not generic to any TF client, since
aren't the names of the suspend states (LP0, LP1, LP2) entirely specific
to Tegra?
On 01/21/2014 03:10 AM, Alexandre Courbot wrote:
> Invoke the do_idle() firmware call before suspending a CPU so that the
> underlying firmware (if any) can take necessary action.
> diff --git a/arch/arm/mach-tegra/cpuidle-tegra114.c b/arch/arm/mach-tegra/cpuidle-tegra114.c
> @@ -45,6 +46,8 @@ static int tegra114_idle_power_down(struct cpuidle_device *dev,
>
> clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &dev->cpu);
>
> + call_firmware_op(do_idle);
> +
> cpu_suspend(0, tegra30_sleep_cpu_secondary_finish);
>
> clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &dev->cpu);
Don't you need to have the kernel also *not* do something when entering
idle; doesn't the FW op replace some of the register writes that the
kernel would otherwise be doing?
On 01/21/2014 03:10 AM, 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 the do_idle() firmware
> operation that is needed for cpuidle to be supported. Tegra's cpuidle
> driver is also updated accordingly.
>
> These patches should be the last step before the device trees for NVIDIA
> SHIELD and Tegra Note 7 can be submitted.
Russell, once these patches are reviewed, should Alex submit them to the
ARM patch tracker, or will you Ack them so they can go through the Tegra
tree? Either way I can put them in a separate branch based on 3.14-rc1
in order to easily resolve any conflicts.
On Thu, Jan 23, 2014 at 5:42 AM, Stephen Warren <[email protected]> wrote:
> On 01/21/2014 03:10 AM, Alexandre Courbot wrote:
>> 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.
>>
>> Doing so allows us to remove it from tegra_defconfig.
>
>> arch/arm/configs/tegra_defconfig | 1 -
>> arch/arm/firmware/Kconfig | 1 +
>
> Can we split out the defconfig and code changes into separate patches?
> They need to go through seperate branches, possibly even separate repos.
>
> The defconfig change might not even be necessary; at some point I'll
> just rebuild it via "make tegra_defconfig; make savedefconfig" on top of
> some linux-next that includes the Kconfig change, and it'll happen
> automatically. Still, I guess there's no harm explicitly sending the patch.
If we can do without the defconfig change then I will just omit it in
the next version - I don't like sending too many oneliners. :P
On Thu, Jan 23, 2014 at 5:43 AM, Stephen Warren <[email protected]> wrote:
> On 01/21/2014 03:10 AM, Alexandre Courbot wrote:
>> Support the do_idle() firmware call, which is necessary to properly
>> support cpuidle.
>
>> diff --git a/arch/arm/firmware/trusted_foundations.c b/arch/arm/firmware/trusted_foundations.c
>
>> +#define TF_CPU_PM 0xfffffffc
>> +#define TF_CPU_PM_LP0 0xffffffe3
>> +#define TF_CPU_PM_LP1 0xffffffe6
>> +#define TF_CPU_PM_LP1_NO_MC_CLK 0xffffffe5
>> +#define TF_CPU_PM_LP2 0xffffffe4
>> +#define TF_CPU_PM_LP2_NOFLUSH_L2 0xffffffe7
>
> Hmm. This must be Tegra-specific, not generic to any TF client, since
> aren't the names of the suspend states (LP0, LP1, LP2) entirely specific
> to Tegra?
The names are negligence on my part, actually. I arbitrarily named
them that way without thinking this was Tegra-only denomination. The
downstream kernel does not even use these, they hardcode the values
directly. Will fix that, thanks for spotting it.
On Thu, Jan 23, 2014 at 5:45 AM, Stephen Warren <[email protected]> wrote:
> On 01/21/2014 03:10 AM, Alexandre Courbot wrote:
>> Invoke the do_idle() firmware call before suspending a CPU so that the
>> underlying firmware (if any) can take necessary action.
>
>> diff --git a/arch/arm/mach-tegra/cpuidle-tegra114.c b/arch/arm/mach-tegra/cpuidle-tegra114.c
>
>> @@ -45,6 +46,8 @@ static int tegra114_idle_power_down(struct cpuidle_device *dev,
>>
>> clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &dev->cpu);
>>
>> + call_firmware_op(do_idle);
>> +
>> cpu_suspend(0, tegra30_sleep_cpu_secondary_finish);
>>
>> clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &dev->cpu);
>
> Don't you need to have the kernel also *not* do something when entering
> idle; doesn't the FW op replace some of the register writes that the
> kernel would otherwise be doing?
It seems like the operation is actually to inform the firmware that we
are going to suspend the CPU. Downstream kernel also uses it that way.
But you are right in that we should expect do_idle() to actually
perform the suspend operation. Maybe a prepare_idle() operation should
be added to the firmware interface for this purpose?
On 01/23/2014 12:39 AM, Alexandre Courbot wrote:
> On Thu, Jan 23, 2014 at 5:45 AM, Stephen Warren <[email protected]> wrote:
>> On 01/21/2014 03:10 AM, Alexandre Courbot wrote:
>>> Invoke the do_idle() firmware call before suspending a CPU so that the
>>> underlying firmware (if any) can take necessary action.
>>
>>> diff --git a/arch/arm/mach-tegra/cpuidle-tegra114.c b/arch/arm/mach-tegra/cpuidle-tegra114.c
>>
>>> @@ -45,6 +46,8 @@ static int tegra114_idle_power_down(struct cpuidle_device *dev,
>>>
>>> clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &dev->cpu);
>>>
>>> + call_firmware_op(do_idle);
>>> +
>>> cpu_suspend(0, tegra30_sleep_cpu_secondary_finish);
>>>
>>> clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &dev->cpu);
>>
>> Don't you need to have the kernel also *not* do something when entering
>> idle; doesn't the FW op replace some of the register writes that the
>> kernel would otherwise be doing?
>
> It seems like the operation is actually to inform the firmware that we
> are going to suspend the CPU. Downstream kernel also uses it that way.
> But you are right in that we should expect do_idle() to actually
> perform the suspend operation. Maybe a prepare_idle() operation should
> be added to the firmware interface for this purpose?
That sounds like a reasonable change. Is it easy to plumb in?
On 02/06/2014 01:28 AM, Stephen Warren wrote:
> On 01/23/2014 12:39 AM, Alexandre Courbot wrote:
>> On Thu, Jan 23, 2014 at 5:45 AM, Stephen Warren <[email protected]> wrote:
>>> On 01/21/2014 03:10 AM, Alexandre Courbot wrote:
>>>> Invoke the do_idle() firmware call before suspending a CPU so that the
>>>> underlying firmware (if any) can take necessary action.
>>>
>>>> diff --git a/arch/arm/mach-tegra/cpuidle-tegra114.c b/arch/arm/mach-tegra/cpuidle-tegra114.c
>>>
>>>> @@ -45,6 +46,8 @@ static int tegra114_idle_power_down(struct cpuidle_device *dev,
>>>>
>>>> clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &dev->cpu);
>>>>
>>>> + call_firmware_op(do_idle);
>>>> +
>>>> cpu_suspend(0, tegra30_sleep_cpu_secondary_finish);
>>>>
>>>> clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &dev->cpu);
>>>
>>> Don't you need to have the kernel also *not* do something when entering
>>> idle; doesn't the FW op replace some of the register writes that the
>>> kernel would otherwise be doing?
>>
>> It seems like the operation is actually to inform the firmware that we
>> are going to suspend the CPU. Downstream kernel also uses it that way.
>> But you are right in that we should expect do_idle() to actually
>> perform the suspend operation. Maybe a prepare_idle() operation should
>> be added to the firmware interface for this purpose?
>
> That sounds like a reasonable change. Is it easy to plumb in?
I think so. Will post a v2 of this soon.
Thanks,
Alex.