[PATCH 0/5] ARM: firmware: improvements to Trusted Foundations support

[PATCH 0/5] ARM: firmware: improvements to Trusted Foundations support

[Alexandre Courbot]

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

[PATCH 1/5] ARM: trusted_foundations: fix vendor prefix typos

[Alexandre Courbot]

of_register_trusted_foundations() and the firmware Kconfig used
the wrong vendor prefix for Trusted Logic Mobility.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 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

[PATCH 2/5] ARM: trusted_foundations: fallback when TF support is missing

[Alexandre Courbot]

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 <acourbot@nvidia.com>
---
 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

[PATCH 3/5] ARM: firmware: enable Trusted Foundations by default

[Alexandre Courbot]

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 <acourbot@nvidia.com>
---
 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

Re: [PATCH 3/5] ARM: firmware: enable Trusted Foundations by default

[Stephen Warren]

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.

Re: [PATCH 3/5] ARM: firmware: enable Trusted Foundations by default

[Alexandre Courbot]

On Thu, Jan 23, 2014 at 5:42 AM, Stephen Warren <swarren@wwwdotorg.org> 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

[PATCH 4/5] ARM: trusted_foundations: implement do_idle()

[Alexandre Courbot]

Support the do_idle() firmware call, which is necessary to properly
support cpuidle.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 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

Re: [PATCH 4/5] ARM: trusted_foundations: implement do_idle()

[Stephen Warren]

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?

Re: [PATCH 4/5] ARM: trusted_foundations: implement do_idle()

[Alexandre Courbot]

On Thu, Jan 23, 2014 at 5:43 AM, Stephen Warren <swarren@wwwdotorg.org> 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.

[PATCH 5/5] ARM: tegra: cpuidle: use firmware call for power down

[Alexandre Courbot]

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 <acourbot@nvidia.com>
---
 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

Re: [PATCH 5/5] ARM: tegra: cpuidle: use firmware call for power down

[Stephen Warren]

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?

Re: [PATCH 5/5] ARM: tegra: cpuidle: use firmware call for power down

[Alexandre Courbot]

On Thu, Jan 23, 2014 at 5:45 AM, Stephen Warren <swarren@wwwdotorg.org> 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?

Re: [PATCH 5/5] ARM: tegra: cpuidle: use firmware call for power down

[Stephen Warren]

On 01/23/2014 12:39 AM, Alexandre Courbot wrote:
> On Thu, Jan 23, 2014 at 5:45 AM, Stephen Warren <swarren@wwwdotorg.org> 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?

Re: [PATCH 5/5] ARM: tegra: cpuidle: use firmware call for power down

[Alexandre Courbot]

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 <swarren@wwwdotorg.org> 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.

Re: [PATCH 0/5] ARM: firmware: improvements to Trusted Foundations support

[Stephen Warren]

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.