mbox series

[v8,00/19] Consolidate and improve NVIDIA Tegra CPUIDLE driver(s)

Message ID 20191203004116.11771-1-digetx@gmail.com
Headers show
Series Consolidate and improve NVIDIA Tegra CPUIDLE driver(s) | expand

Message

Dmitry Osipenko Dec. 3, 2019, 12:40 a.m. UTC
Hello,

This series does the following:

  1. Unifies Tegra20/30/114 drivers into a single driver and moves it out
     into common drivers/cpuidle/ directory.

  2. Enables CPU cluster power-down idling state on Tegra30.

In the end there is a quite nice clean up of the Tegra CPUIDLE drivers
and of the Tegra's arch code in general. Please review, thanks!

Changelog:

v8: - Rebased on recent linux-next, now making use of
      cpuidle_driver_state_disabled(). Added new patch to make this new API
      usable by the updated Tegra cpuidle driver:

        cpuidle: Avoid NULL dereference in cpuidle_driver_state_disabled()

    - Added new patch to handle case where LP2 isn't available:

        cpuidle: tegra: Disable CC6 state if LP2 unavailable

v7: - drivers/cpuidle/cpuidle-tegra.c now includes an explicit comment that
      clarifies the new terminology that is used for naming of the idling
      states. This change was suggested by Peter De Schrijver in the review
      comment to v6. See the comment to struct tegra_idle_driver in the code.

    - (!) This series is now based on top of the "NVIDIA Tegra20 CPUFreq
      driver major update" patchset. The conflict between these two series
      is trivial to resolve, but still it's worth to mention about that.

v6: - Addressed request from Thierry Reding to change the way patches are
      organized by making changes in a more incremental manner.

    - tegra_sleep_cpu() now checks for the secondary CPUs to be offline
      in the "Make outer_disable() open-coded" patch.

v5: - Rebased on a recent linux-next, fixed one minor conflict in Kconfig.

    - Improved commit's message of the "Support CPU cluster power-down state
      on Tegra30" patch.

    - The "Support CPU cluster power-down state on Tegra30" patch is also
      got split and now there is additional "Make outer_disable() open-coded"
      patch.

    - Made minor cosmetic changes to the "Introduce unified driver for
      NVIDIA Tegra SoCs" patch by improving error message and renaming
      one variable.

v4: - Fixed compilation with !CONFIG_CACHE_L2X0 (and tested that it still
      works).

    - Replaced ktime_compare() with ktime_before() in the new driver,
      for consistency.

v3: - Addressed review comments that were made by Jon Hunter to v2 by
      splitting patches into smaller (and simpler) chunks, better
      documenting changes in the commit messages and using proper error
      codes in the code.

      Warnings are replaced with a useful error messages in the code of
      "Introduce unified driver for NVIDIA Tegra SoCs" patch.

      Secondary CPUs parking timeout increased to 100ms because I found
      that it actually may happen to take more than 1ms if CPU is running
      on a *very* low frequency.

      Added diagnostic messages that are reporting Flow Controller state
      when CPU parking fails.

      Further polished cpuidle driver's code.

      The coupled state entering is now aborted if there is a pending SGI
      (Software Generated Interrupt) because it will be lost after GIC's
      power-cycling. Like it was done by the old Tegra20 CPUIDLE driver.

v2: - Added patches to enable the new cpuidle driver in the defconfigs:

        ARM: multi_v7_defconfig: Enable Tegra cpuidle driver
        ARM: tegra: Enable Tegra cpuidle driver in tegra_defconfig

    - Dropped patches that removed CPUIDLE_FLAG_TIMER_STOP from the idling
      states because that flag actually doesn't have any negative effects,
      but still is correct for the case of a local CPU timer on older Tegra
      SoCs:

        cpuidle: tegra: Remove CPUIDLE_FLAG_TIMER_STOP from Tegra114/124 idle-state
        cpuidle: tegra: Remove CPUIDLE_FLAG_TIMER_STOP from all states

    - The "Add unified driver for NVIDIA Tegra SoCs" patch got more polish.
      Tegra30 and Terga114 states are now squashed into a single common C7
      state (following Parker TRM terminology, see 17.2.2.2 Power Management
      States), more comments added, etc minor changes.

Dmitry Osipenko (19):
  ARM: tegra: Compile sleep-tegra20/30.S unconditionally
  ARM: tegra: Add tegra_pm_park_secondary_cpu()
  ARM: tegra: Remove pen-locking from cpuidle-tegra20
  ARM: tegra: Change tegra_set_cpu_in_lp2() type to void
  ARM: tegra: Propagate error from tegra_idle_lp2_last()
  ARM: tegra: Expose PM functions required for new cpuidle driver
  ARM: tegra: Rename some of the newly exposed PM functions
  ARM: tegra: Make outer_disable() open-coded
  arm: tegra20: cpuidle: Handle case where secondary CPU hangs on
    entering LP2
  arm: tegra20: cpuidle: Make abort_flag atomic
  arm: tegra20/30: cpuidle: Remove unnecessary memory barrier
  cpuidle: Avoid NULL dereference in cpuidle_driver_state_disabled()
  cpuidle: Refactor and move out NVIDIA Tegra20 driver into
    drivers/cpuidle
  cpuidle: tegra: Squash Tegra30 driver into the common driver
  cpuidle: tegra: Support CPU cluster power-down state on Tegra30
  cpuidle: tegra: Squash Tegra114 driver into the common driver
  cpuidle: tegra: Disable CC6 state if LP2 unavailable
  ARM: multi_v7_defconfig: Enable Tegra cpuidle driver
  ARM: tegra: Enable Tegra cpuidle driver in tegra_defconfig

 arch/arm/configs/multi_v7_defconfig           |   1 +
 arch/arm/configs/tegra_defconfig              |   1 +
 arch/arm/mach-tegra/Makefile                  |  19 +-
 arch/arm/mach-tegra/cpuidle-tegra114.c        |  89 -----
 arch/arm/mach-tegra/cpuidle-tegra20.c         | 212 ----------
 arch/arm/mach-tegra/cpuidle-tegra30.c         | 132 ------
 arch/arm/mach-tegra/cpuidle.c                 |  50 ---
 arch/arm/mach-tegra/cpuidle.h                 |  21 -
 arch/arm/mach-tegra/irq.c                     |   3 +-
 arch/arm/mach-tegra/pm.c                      |  54 ++-
 arch/arm/mach-tegra/pm.h                      |   4 -
 arch/arm/mach-tegra/reset-handler.S           |  11 -
 arch/arm/mach-tegra/reset.h                   |   9 +-
 arch/arm/mach-tegra/sleep-tegra20.S           | 170 --------
 arch/arm/mach-tegra/sleep-tegra30.S           |   6 +-
 arch/arm/mach-tegra/sleep.h                   |  15 -
 arch/arm/mach-tegra/tegra.c                   |   7 +-
 drivers/cpuidle/Kconfig.arm                   |   8 +
 drivers/cpuidle/Makefile                      |   1 +
 drivers/cpuidle/cpuidle-tegra.c               | 376 ++++++++++++++++++
 drivers/cpuidle/driver.c                      |   4 +
 include/soc/tegra/cpuidle.h                   |   2 +-
 .../mach-tegra => include/soc/tegra}/irq.h    |   8 +-
 include/soc/tegra/pm.h                        |  31 ++
 24 files changed, 473 insertions(+), 761 deletions(-)
 delete mode 100644 arch/arm/mach-tegra/cpuidle-tegra114.c
 delete mode 100644 arch/arm/mach-tegra/cpuidle-tegra20.c
 delete mode 100644 arch/arm/mach-tegra/cpuidle-tegra30.c
 delete mode 100644 arch/arm/mach-tegra/cpuidle.c
 delete mode 100644 arch/arm/mach-tegra/cpuidle.h
 create mode 100644 drivers/cpuidle/cpuidle-tegra.c
 rename {arch/arm/mach-tegra => include/soc/tegra}/irq.h (59%)

Comments

Michał Mirosław Dec. 7, 2019, 7:31 p.m. UTC | #1
On Tue, Dec 03, 2019 at 03:40:57AM +0300, Dmitry Osipenko wrote:
> Hello,
> 
> This series does the following:
> 
>   1. Unifies Tegra20/30/114 drivers into a single driver and moves it out
>      into common drivers/cpuidle/ directory.
> 
>   2. Enables CPU cluster power-down idling state on Tegra30.
> 
> In the end there is a quite nice clean up of the Tegra CPUIDLE drivers
> and of the Tegra's arch code in general. Please review, thanks!
> 
> Changelog:
> 
> v8: - Rebased on recent linux-next, now making use of
>       cpuidle_driver_state_disabled(). [...]

Dear Dmitry

This rebase was not complete, at least in patch 14 direct access to
.disabled field (now removed in Linus' master) are left. The fixups
needed are below (only compile-tested for now).

Best Regards,
Michał Mirosław

diff --git a/drivers/cpuidle/cpuidle-tegra.c b/drivers/cpuidle/cpuidle-tegra.c
index 077ff15e32fb..2d325d3aab57 100644
--- a/drivers/cpuidle/cpuidle-tegra.c
+++ b/drivers/cpuidle/cpuidle-tegra.c
@@ -306,13 +306,12 @@ static struct cpuidle_driver tegra_idle_driver = {
  */
 void tegra_cpuidle_pcie_irqs_in_use(void)
 {
-	if (tegra_idle_driver.states[TEGRA_CC6].disabled ||
+	if ((tegra_idle_driver.states[TEGRA_CC6].flags & CPUIDLE_FLAG_UNUSABLE) &&
 	    tegra_get_chip_id() != TEGRA20)
 		return;
 
 	pr_info("disabling CC6 state, since PCIe IRQs are in use\n");
 	cpuidle_driver_state_disabled(&tegra_idle_driver, TEGRA_CC6, true);
-	tegra_idle_driver.states[TEGRA_CC6].disabled = true;
 }
 
 static void tegra_cpuidle_setup_tegra114_c7_state(void)
@@ -328,7 +327,7 @@ static int tegra_cpuidle_probe(struct platform_device *pdev)
 {
 	/* LP2 could be disabled in device-tree */
 	if (tegra_pmc_get_suspend_mode() < TEGRA_SUSPEND_LP2)
-		tegra_idle_driver.states[TEGRA_CC6].disabled = true;
+		cpuidle_driver_state_disabled(&tegra_idle_driver, TEGRA_CC6, true);
 
 	/*
 	 * Required suspend-resume functionality, which is provided by the
@@ -337,9 +336,9 @@ static int tegra_cpuidle_probe(struct platform_device *pdev)
 	 */
 	if (!IS_ENABLED(CONFIG_PM_SLEEP)) {
 		if (!tegra_cpuidle_using_firmware())
-			tegra_idle_driver.states[TEGRA_C7].disabled = true;
+			cpuidle_driver_state_disabled(&tegra_idle_driver, TEGRA_C7, true);
 
-		tegra_idle_driver.states[TEGRA_CC6].disabled = true;
+		cpuidle_driver_state_disabled(&tegra_idle_driver, TEGRA_CC6, true);
 	}
 
 	/*
@@ -349,7 +348,7 @@ static int tegra_cpuidle_probe(struct platform_device *pdev)
 	switch (tegra_get_chip_id()) {
 	case TEGRA20:
 		/* Tegra20 isn't capable to power-off individual CPU cores */
-		tegra_idle_driver.states[TEGRA_C7].disabled = true;
+		cpuidle_driver_state_disabled(&tegra_idle_driver, TEGRA_C7, true);
 		break;
 	case TEGRA30:
 		break;
@@ -358,7 +357,7 @@ static int tegra_cpuidle_probe(struct platform_device *pdev)
 		tegra_cpuidle_setup_tegra114_c7_state();
 
 		/* coupled CC6 (LP2) state isn't implemented yet */
-		tegra_idle_driver.states[TEGRA_CC6].disabled = true;
+		cpuidle_driver_state_disabled(&tegra_idle_driver, TEGRA_CC6, true);
 		break;
 	default:
 		return -EINVAL;
Michał Mirosław Dec. 7, 2019, 9:52 p.m. UTC | #2
On Tue, Dec 03, 2019 at 03:40:57AM +0300, Dmitry Osipenko wrote:
> Hello,
> 
> This series does the following:
> 
>   1. Unifies Tegra20/30/114 drivers into a single driver and moves it out
>      into common drivers/cpuidle/ directory.
> 
>   2. Enables CPU cluster power-down idling state on Tegra30.
> 
> In the end there is a quite nice clean up of the Tegra CPUIDLE drivers
> and of the Tegra's arch code in general. Please review, thanks!

I did a quick smoke test for this series on top of Linus' master:
 - rebuilding with the patches applied, CONFIG_ARM_TEGRA_CPUIDLE=n - works
 - building with CONFIG_ARM_TEGRA_CPUIDLE=y - doesn't boot

The hang is somewhere early in the boot process, before simplefb can
take the console and show any logs. If I get BOOTFB to work again I might
be able to get some more info.

Best Regards,
Michał Mirosław
Dmitry Osipenko Dec. 7, 2019, 10:56 p.m. UTC | #3
07.12.2019 22:31, Michał Mirosław пишет:
> On Tue, Dec 03, 2019 at 03:40:57AM +0300, Dmitry Osipenko wrote:
>> Hello,
>>
>> This series does the following:
>>
>>   1. Unifies Tegra20/30/114 drivers into a single driver and moves it out
>>      into common drivers/cpuidle/ directory.
>>
>>   2. Enables CPU cluster power-down idling state on Tegra30.
>>
>> In the end there is a quite nice clean up of the Tegra CPUIDLE drivers
>> and of the Tegra's arch code in general. Please review, thanks!
>>
>> Changelog:
>>
>> v8: - Rebased on recent linux-next, now making use of
>>       cpuidle_driver_state_disabled(). [...]
> 
> Dear Dmitry
> 
> This rebase was not complete, at least in patch 14 direct access to
> .disabled field (now removed in Linus' master) are left. The fixups
> needed are below (only compile-tested for now).

Hello Michał,

CPUIDLE core got changed after this series was sent out, I'll rebase it
one more time. Thank you for the attention.

[snip]
Dmitry Osipenko Dec. 7, 2019, 10:56 p.m. UTC | #4
08.12.2019 00:52, Michał Mirosław пишет:
> On Tue, Dec 03, 2019 at 03:40:57AM +0300, Dmitry Osipenko wrote:
>> Hello,
>>
>> This series does the following:
>>
>>   1. Unifies Tegra20/30/114 drivers into a single driver and moves it out
>>      into common drivers/cpuidle/ directory.
>>
>>   2. Enables CPU cluster power-down idling state on Tegra30.
>>
>> In the end there is a quite nice clean up of the Tegra CPUIDLE drivers
>> and of the Tegra's arch code in general. Please review, thanks!
> 
> I did a quick smoke test for this series on top of Linus' master:
>  - rebuilding with the patches applied, CONFIG_ARM_TEGRA_CPUIDLE=n - works
>  - building with CONFIG_ARM_TEGRA_CPUIDLE=y - doesn't boot
> 
> The hang is somewhere early in the boot process, before simplefb can
> take the console and show any logs. If I get BOOTFB to work again I might
> be able to get some more info.

Thank you very much for trying these patches!

Could you please try to make ARM_TEGRA_CPUIDLE "tristate" in the Kconfig
and compile it as a loadable module? That way you'll get framebuffer
shown before the hang happens.

Does LP2 suspend/resume work for you? There should be
"nvidia,suspend-mode = <2>" in the PMC's node of device-tree.
Michał Mirosław Dec. 9, 2019, 4:04 p.m. UTC | #5
On Sun, Dec 08, 2019 at 01:56:14AM +0300, Dmitry Osipenko wrote:
> 08.12.2019 00:52, Michał Mirosław пишет:
> > On Tue, Dec 03, 2019 at 03:40:57AM +0300, Dmitry Osipenko wrote:
> >> Hello,
> >>
> >> This series does the following:
> >>
> >>   1. Unifies Tegra20/30/114 drivers into a single driver and moves it out
> >>      into common drivers/cpuidle/ directory.
> >>
> >>   2. Enables CPU cluster power-down idling state on Tegra30.
> >>
> >> In the end there is a quite nice clean up of the Tegra CPUIDLE drivers
> >> and of the Tegra's arch code in general. Please review, thanks!
> > 
> > I did a quick smoke test for this series on top of Linus' master:
> >  - rebuilding with the patches applied, CONFIG_ARM_TEGRA_CPUIDLE=n - works
> >  - building with CONFIG_ARM_TEGRA_CPUIDLE=y - doesn't boot
> > 
> > The hang is somewhere early in the boot process, before simplefb can
> > take the console and show any logs. If I get BOOTFB to work again I might
> > be able to get some more info.
> 
> Thank you very much for trying these patches!
> 
> Could you please try to make ARM_TEGRA_CPUIDLE "tristate" in the Kconfig
> and compile it as a loadable module? That way you'll get framebuffer
> shown before the hang happens.
> 
> Does LP2 suspend/resume work for you? There should be
> "nvidia,suspend-mode = <2>" in the PMC's node of device-tree.

Not at the moment. I also tried suspend-mode = <1> and <0>, but it
made no difference.

Best Regards,
Michał Mirosław
Dmitry Osipenko Dec. 9, 2019, 9:22 p.m. UTC | #6
09.12.2019 19:04, Michał Mirosław пишет:
> On Sun, Dec 08, 2019 at 01:56:14AM +0300, Dmitry Osipenko wrote:
>> 08.12.2019 00:52, Michał Mirosław пишет:
>>> On Tue, Dec 03, 2019 at 03:40:57AM +0300, Dmitry Osipenko wrote:
>>>> Hello,
>>>>
>>>> This series does the following:
>>>>
>>>>   1. Unifies Tegra20/30/114 drivers into a single driver and moves it out
>>>>      into common drivers/cpuidle/ directory.
>>>>
>>>>   2. Enables CPU cluster power-down idling state on Tegra30.
>>>>
>>>> In the end there is a quite nice clean up of the Tegra CPUIDLE drivers
>>>> and of the Tegra's arch code in general. Please review, thanks!
>>>
>>> I did a quick smoke test for this series on top of Linus' master:
>>>  - rebuilding with the patches applied, CONFIG_ARM_TEGRA_CPUIDLE=n - works
>>>  - building with CONFIG_ARM_TEGRA_CPUIDLE=y - doesn't boot
>>>
>>> The hang is somewhere early in the boot process, before simplefb can
>>> take the console and show any logs. If I get BOOTFB to work again I might
>>> be able to get some more info.
>>
>> Thank you very much for trying these patches!
>>
>> Could you please try to make ARM_TEGRA_CPUIDLE "tristate" in the Kconfig
>> and compile it as a loadable module? That way you'll get framebuffer
>> shown before the hang happens.
>>
>> Does LP2 suspend/resume work for you? There should be
>> "nvidia,suspend-mode = <2>" in the PMC's node of device-tree.
> 
> Not at the moment. I also tried suspend-mode = <1> and <0>, but it
> made no difference.

If LP2 doesn't work, then it explains why you're getting the hang.

Are you using TF300T for the testing? I'm recalling that LP2 worked for
you sometime ago on TF300T, maybe some offending change was introduced
since then. Could you please try to do the git bisection or at least
find out what is the last good kernel version?

I rebased this series on a recent linux-next and you could find the
rebased patches here [1].

[1] https://github.com/grate-driver/linux/commits/master

With [1] you should be able to remove "nvidia,suspend-mode" property
from the device-tree to get cpuidle working with the disabled CC6 state
(LP2). Could you please check that at least disabled CC6 works for you?
Michał Mirosław Dec. 10, 2019, 2:35 a.m. UTC | #7
On Tue, Dec 10, 2019 at 12:22:18AM +0300, Dmitry Osipenko wrote:
> 09.12.2019 19:04, Michał Mirosław пишет:
> > On Sun, Dec 08, 2019 at 01:56:14AM +0300, Dmitry Osipenko wrote:
> >> 08.12.2019 00:52, Michał Mirosław пишет:
> >>> On Tue, Dec 03, 2019 at 03:40:57AM +0300, Dmitry Osipenko wrote:
> >>>> Hello,
> >>>>
> >>>> This series does the following:
> >>>>
> >>>>   1. Unifies Tegra20/30/114 drivers into a single driver and moves it out
> >>>>      into common drivers/cpuidle/ directory.
> >>>>
> >>>>   2. Enables CPU cluster power-down idling state on Tegra30.
> >>>>
> >>>> In the end there is a quite nice clean up of the Tegra CPUIDLE drivers
> >>>> and of the Tegra's arch code in general. Please review, thanks!
> >>>
> >>> I did a quick smoke test for this series on top of Linus' master:
> >>>  - rebuilding with the patches applied, CONFIG_ARM_TEGRA_CPUIDLE=n - works
> >>>  - building with CONFIG_ARM_TEGRA_CPUIDLE=y - doesn't boot
> >>>
> >>> The hang is somewhere early in the boot process, before simplefb can
> >>> take the console and show any logs. If I get BOOTFB to work again I might
> >>> be able to get some more info.
> >>
> >> Thank you very much for trying these patches!
> >>
> >> Could you please try to make ARM_TEGRA_CPUIDLE "tristate" in the Kconfig
> >> and compile it as a loadable module? That way you'll get framebuffer
> >> shown before the hang happens.
> >>
> >> Does LP2 suspend/resume work for you? There should be
> >> "nvidia,suspend-mode = <2>" in the PMC's node of device-tree.
> > 
> > Not at the moment. I also tried suspend-mode = <1> and <0>, but it
> > made no difference.
> 
> If LP2 doesn't work, then it explains why you're getting the hang.
> 
> Are you using TF300T for the testing? I'm recalling that LP2 worked for
> you sometime ago on TF300T, maybe some offending change was introduced
> since then. Could you please try to do the git bisection or at least
> find out what is the last good kernel version?
> 
> I rebased this series on a recent linux-next and you could find the
> rebased patches here [1].
> 
> [1] https://github.com/grate-driver/linux/commits/master
> 
> With [1] you should be able to remove "nvidia,suspend-mode" property
> from the device-tree to get cpuidle working with the disabled CC6 state
> (LP2). Could you please check that at least disabled CC6 works for you?

I tested suspend with your tree merged, but CONFIG_TEGRA_CPUIDLE=n. LP2
seems to work [1]. The same tree with CONFIG_TEGRA_CPUIDLE=y doesn't
boot. I'll try comparing DTs, but other than that I'm blocked on BOOTFB now.

Best Regards,
Michał Mirosław

[1] rtcwake -s 3 -d /dev/rtc0 -v -m mem

(...)
[ 2710.157919] PM: suspend entry (deep)
[ 2710.161205] Filesystems sync: 0.000 seconds
[ 2710.176677] Freezing user space processes ... (elapsed 0.001 seconds) done.
[ 2710.178342] OOM killer disabled.
[ 2710.178527] Freezing remaining freezable tasks ... (elapsed 0.001 seconds) done.
[ 2710.347871] Disabling non-boot CPUs ...
[ 2710.349160] IRQ 18: no longer affine to CPU1
[ 2710.352499] IRQ 19: no longer affine to CPU2
[ 2710.370059] IRQ 20: no longer affine to CPU3
[ 2710.371284] Entering suspend state LP2
[ 2710.371556] Enabling non-boot CPUs ...
[ 2710.373157] CPU1 is up
[ 2710.374598] CPU2 is up
[ 2710.375996] CPU3 is up
[ 2710.462876] OOM killer enabled.
[ 2710.463018] Restarting tasks ...
[ 2710.463880] tegra-devfreq 6000c800.actmon: Failed to get emc clock
[ 2710.464509] done.
[ 2710.552824] asus-ec 1-0015: model         : ASUS-TF201-PAD
[ 2710.558345] asus-ec 1-0015: FW version    : PAD-EC20T-0216
[ 2710.562942] asus-ec 1-0015: Config format : ECFG-0001
[ 2710.567651] asus-ec 1-0015: HW version    : TF201-PAD-SKU1
[ 2710.572488] asus-ec 1-0015: EC FW behaviour: susb on when system wakeup
[ 2710.769796] atkbd serio1: no of_node; not parsing pinctrl DT
[ 2710.835629] asus-ec 5-0019: model         : ASUS-TF201-DOCK
[ 2710.838686] asus-ec 5-0019: FW version    : DOCK-EC20N-0207
[ 2710.841865] asus-ec 5-0019: Config format : ECFG-0001
[ 2710.844271] asus-ec 5-0019: HW version    : PCBA-SKU-2
[ 2710.847950] asus-ec 5-0019: EC FW behaviour: susb on when receive ec_req
[ 2711.040935] PM: suspend exit
Dmitry Osipenko Dec. 10, 2019, 4:02 p.m. UTC | #8
10.12.2019 05:35, Michał Mirosław пишет:
> On Tue, Dec 10, 2019 at 12:22:18AM +0300, Dmitry Osipenko wrote:
>> 09.12.2019 19:04, Michał Mirosław пишет:
>>> On Sun, Dec 08, 2019 at 01:56:14AM +0300, Dmitry Osipenko wrote:
>>>> 08.12.2019 00:52, Michał Mirosław пишет:
>>>>> On Tue, Dec 03, 2019 at 03:40:57AM +0300, Dmitry Osipenko wrote:
>>>>>> Hello,
>>>>>>
>>>>>> This series does the following:
>>>>>>
>>>>>>   1. Unifies Tegra20/30/114 drivers into a single driver and moves it out
>>>>>>      into common drivers/cpuidle/ directory.
>>>>>>
>>>>>>   2. Enables CPU cluster power-down idling state on Tegra30.
>>>>>>
>>>>>> In the end there is a quite nice clean up of the Tegra CPUIDLE drivers
>>>>>> and of the Tegra's arch code in general. Please review, thanks!
>>>>>
>>>>> I did a quick smoke test for this series on top of Linus' master:
>>>>>  - rebuilding with the patches applied, CONFIG_ARM_TEGRA_CPUIDLE=n - works
>>>>>  - building with CONFIG_ARM_TEGRA_CPUIDLE=y - doesn't boot
>>>>>
>>>>> The hang is somewhere early in the boot process, before simplefb can
>>>>> take the console and show any logs. If I get BOOTFB to work again I might
>>>>> be able to get some more info.
>>>>
>>>> Thank you very much for trying these patches!
>>>>
>>>> Could you please try to make ARM_TEGRA_CPUIDLE "tristate" in the Kconfig
>>>> and compile it as a loadable module? That way you'll get framebuffer
>>>> shown before the hang happens.
>>>>
>>>> Does LP2 suspend/resume work for you? There should be
>>>> "nvidia,suspend-mode = <2>" in the PMC's node of device-tree.
>>>
>>> Not at the moment. I also tried suspend-mode = <1> and <0>, but it
>>> made no difference.
>>
>> If LP2 doesn't work, then it explains why you're getting the hang.
>>
>> Are you using TF300T for the testing? I'm recalling that LP2 worked for
>> you sometime ago on TF300T, maybe some offending change was introduced
>> since then. Could you please try to do the git bisection or at least
>> find out what is the last good kernel version?
>>
>> I rebased this series on a recent linux-next and you could find the
>> rebased patches here [1].
>>
>> [1] https://github.com/grate-driver/linux/commits/master
>>
>> With [1] you should be able to remove "nvidia,suspend-mode" property
>> from the device-tree to get cpuidle working with the disabled CC6 state
>> (LP2). Could you please check that at least disabled CC6 works for you?
> 
> I tested suspend with your tree merged, but CONFIG_TEGRA_CPUIDLE=n. LP2
> seems to work [1]. The same tree with CONFIG_TEGRA_CPUIDLE=y doesn't
> boot. I'll try comparing DTs, but other than that I'm blocked on BOOTFB now.

That's an interesting result.

> [1] rtcwake -s 3 -d /dev/rtc0 -v -m mem
> 
> (...)
> [ 2710.157919] PM: suspend entry (deep)
> [ 2710.161205] Filesystems sync: 0.000 seconds
> [ 2710.176677] Freezing user space processes ... (elapsed 0.001 seconds) done.
> [ 2710.178342] OOM killer disabled.
> [ 2710.178527] Freezing remaining freezable tasks ... (elapsed 0.001 seconds) done.
> [ 2710.347871] Disabling non-boot CPUs ...
> [ 2710.349160] IRQ 18: no longer affine to CPU1
> [ 2710.352499] IRQ 19: no longer affine to CPU2
> [ 2710.370059] IRQ 20: no longer affine to CPU3
> [ 2710.371284] Entering suspend state LP2
> [ 2710.371556] Enabling non-boot CPUs ...
> [ 2710.373157] CPU1 is up
> [ 2710.374598] CPU2 is up
> [ 2710.375996] CPU3 is up
> [ 2710.462876] OOM killer enabled.
> [ 2710.463018] Restarting tasks ...
> [ 2710.463880] tegra-devfreq 6000c800.actmon: Failed to get emc clock
> [ 2710.464509] done.
> [ 2710.552824] asus-ec 1-0015: model         : ASUS-TF201-PAD
> [ 2710.558345] asus-ec 1-0015: FW version    : PAD-EC20T-0216
> [ 2710.562942] asus-ec 1-0015: Config format : ECFG-0001
> [ 2710.567651] asus-ec 1-0015: HW version    : TF201-PAD-SKU1
> [ 2710.572488] asus-ec 1-0015: EC FW behaviour: susb on when system wakeup
> [ 2710.769796] atkbd serio1: no of_node; not parsing pinctrl DT
> [ 2710.835629] asus-ec 5-0019: model         : ASUS-TF201-DOCK
> [ 2710.838686] asus-ec 5-0019: FW version    : DOCK-EC20N-0207
> [ 2710.841865] asus-ec 5-0019: Config format : ECFG-0001
> [ 2710.844271] asus-ec 5-0019: HW version    : PCBA-SKU-2
> [ 2710.847950] asus-ec 5-0019: EC FW behaviour: susb on when receive ec_req
> [ 2711.040935] PM: suspend exit
> 

Could you please try this change on top of recent grate-linux (it should
allow display to light up before the hang):

diff --git a/drivers/cpuidle/cpuidle-tegra.c
b/drivers/cpuidle/cpuidle-tegra.c
index db9ccba5a74c..21317b4e16c1 100644
--- a/drivers/cpuidle/cpuidle-tegra.c
+++ b/drivers/cpuidle/cpuidle-tegra.c
@@ -22,6 +22,7 @@
 #include <linux/ktime.h>
 #include <linux/platform_device.h>
 #include <linux/types.h>
+#include <linux/workqueue.h>

 #include <linux/clk/tegra.h>
 #include <linux/firmware/trusted_foundations.h>
@@ -332,7 +333,7 @@ static void tegra_cpuidle_setup_tegra114_c7_state(void)
 	s->exit_latency = 500;
 }

-static int tegra_cpuidle_probe(struct platform_device *pdev)
+static void tegra_cpuidle_probe_work(struct work_struct *work)
 {
 	/* LP2 could be disabled in device-tree */
 	if (tegra_pmc_get_suspend_mode() < TEGRA_SUSPEND_LP2)
@@ -372,10 +373,18 @@ static int tegra_cpuidle_probe(struct
platform_device *pdev)
 		break;

 	default:
-		return -EINVAL;
+		return;
 	}

-	return cpuidle_register(&tegra_idle_driver, cpu_possible_mask);
+	cpuidle_register(&tegra_idle_driver, cpu_possible_mask);
+}
+
+static DECLARE_DELAYED_WORK(delayed_probe, tegra_cpuidle_probe_work);
+
+static int tegra_cpuidle_probe(struct platform_device *pdev)
+{
+	schedule_delayed_work(&delayed_probe, 5 * HZ);
+	return 0;
 }

 static struct platform_driver tegra_cpuidle_driver = {
Dmitry Osipenko Dec. 10, 2019, 4:21 p.m. UTC | #9
10.12.2019 19:02, Dmitry Osipenko пишет:
> 10.12.2019 05:35, Michał Mirosław пишет:
>> On Tue, Dec 10, 2019 at 12:22:18AM +0300, Dmitry Osipenko wrote:
>>> 09.12.2019 19:04, Michał Mirosław пишет:
>>>> On Sun, Dec 08, 2019 at 01:56:14AM +0300, Dmitry Osipenko wrote:
>>>>> 08.12.2019 00:52, Michał Mirosław пишет:
>>>>>> On Tue, Dec 03, 2019 at 03:40:57AM +0300, Dmitry Osipenko wrote:
>>>>>>> Hello,
>>>>>>>
>>>>>>> This series does the following:
>>>>>>>
>>>>>>>   1. Unifies Tegra20/30/114 drivers into a single driver and moves it out
>>>>>>>      into common drivers/cpuidle/ directory.
>>>>>>>
>>>>>>>   2. Enables CPU cluster power-down idling state on Tegra30.
>>>>>>>
>>>>>>> In the end there is a quite nice clean up of the Tegra CPUIDLE drivers
>>>>>>> and of the Tegra's arch code in general. Please review, thanks!
>>>>>>
>>>>>> I did a quick smoke test for this series on top of Linus' master:
>>>>>>  - rebuilding with the patches applied, CONFIG_ARM_TEGRA_CPUIDLE=n - works
>>>>>>  - building with CONFIG_ARM_TEGRA_CPUIDLE=y - doesn't boot
>>>>>>
>>>>>> The hang is somewhere early in the boot process, before simplefb can
>>>>>> take the console and show any logs. If I get BOOTFB to work again I might
>>>>>> be able to get some more info.
>>>>>
>>>>> Thank you very much for trying these patches!
>>>>>
>>>>> Could you please try to make ARM_TEGRA_CPUIDLE "tristate" in the Kconfig
>>>>> and compile it as a loadable module? That way you'll get framebuffer
>>>>> shown before the hang happens.
>>>>>
>>>>> Does LP2 suspend/resume work for you? There should be
>>>>> "nvidia,suspend-mode = <2>" in the PMC's node of device-tree.
>>>>
>>>> Not at the moment. I also tried suspend-mode = <1> and <0>, but it
>>>> made no difference.
>>>
>>> If LP2 doesn't work, then it explains why you're getting the hang.
>>>
>>> Are you using TF300T for the testing? I'm recalling that LP2 worked for
>>> you sometime ago on TF300T, maybe some offending change was introduced
>>> since then. Could you please try to do the git bisection or at least
>>> find out what is the last good kernel version?
>>>
>>> I rebased this series on a recent linux-next and you could find the
>>> rebased patches here [1].
>>>
>>> [1] https://github.com/grate-driver/linux/commits/master
>>>
>>> With [1] you should be able to remove "nvidia,suspend-mode" property
>>> from the device-tree to get cpuidle working with the disabled CC6 state
>>> (LP2). Could you please check that at least disabled CC6 works for you?
>>
>> I tested suspend with your tree merged, but CONFIG_TEGRA_CPUIDLE=n. LP2
>> seems to work [1]. The same tree with CONFIG_TEGRA_CPUIDLE=y doesn't
>> boot. I'll try comparing DTs, but other than that I'm blocked on BOOTFB now.
> 
> That's an interesting result.
> 
>> [1] rtcwake -s 3 -d /dev/rtc0 -v -m mem
>>
>> (...)
>> [ 2710.157919] PM: suspend entry (deep)
>> [ 2710.161205] Filesystems sync: 0.000 seconds
>> [ 2710.176677] Freezing user space processes ... (elapsed 0.001 seconds) done.
>> [ 2710.178342] OOM killer disabled.
>> [ 2710.178527] Freezing remaining freezable tasks ... (elapsed 0.001 seconds) done.
>> [ 2710.347871] Disabling non-boot CPUs ...
>> [ 2710.349160] IRQ 18: no longer affine to CPU1
>> [ 2710.352499] IRQ 19: no longer affine to CPU2
>> [ 2710.370059] IRQ 20: no longer affine to CPU3
>> [ 2710.371284] Entering suspend state LP2
>> [ 2710.371556] Enabling non-boot CPUs ...
>> [ 2710.373157] CPU1 is up
>> [ 2710.374598] CPU2 is up
>> [ 2710.375996] CPU3 is up
>> [ 2710.462876] OOM killer enabled.
>> [ 2710.463018] Restarting tasks ...
>> [ 2710.463880] tegra-devfreq 6000c800.actmon: Failed to get emc clock
>> [ 2710.464509] done.
>> [ 2710.552824] asus-ec 1-0015: model         : ASUS-TF201-PAD
>> [ 2710.558345] asus-ec 1-0015: FW version    : PAD-EC20T-0216
>> [ 2710.562942] asus-ec 1-0015: Config format : ECFG-0001
>> [ 2710.567651] asus-ec 1-0015: HW version    : TF201-PAD-SKU1
>> [ 2710.572488] asus-ec 1-0015: EC FW behaviour: susb on when system wakeup
>> [ 2710.769796] atkbd serio1: no of_node; not parsing pinctrl DT
>> [ 2710.835629] asus-ec 5-0019: model         : ASUS-TF201-DOCK
>> [ 2710.838686] asus-ec 5-0019: FW version    : DOCK-EC20N-0207
>> [ 2710.841865] asus-ec 5-0019: Config format : ECFG-0001
>> [ 2710.844271] asus-ec 5-0019: HW version    : PCBA-SKU-2
>> [ 2710.847950] asus-ec 5-0019: EC FW behaviour: susb on when receive ec_req
>> [ 2711.040935] PM: suspend exit
>>
> 
> Could you please try this change on top of recent grate-linux (it should
> allow display to light up before the hang):
> 
> diff --git a/drivers/cpuidle/cpuidle-tegra.c
> b/drivers/cpuidle/cpuidle-tegra.c
> index db9ccba5a74c..21317b4e16c1 100644
> --- a/drivers/cpuidle/cpuidle-tegra.c
> +++ b/drivers/cpuidle/cpuidle-tegra.c
> @@ -22,6 +22,7 @@
>  #include <linux/ktime.h>
>  #include <linux/platform_device.h>
>  #include <linux/types.h>
> +#include <linux/workqueue.h>
> 
>  #include <linux/clk/tegra.h>
>  #include <linux/firmware/trusted_foundations.h>
> @@ -332,7 +333,7 @@ static void tegra_cpuidle_setup_tegra114_c7_state(void)
>  	s->exit_latency = 500;
>  }
> 
> -static int tegra_cpuidle_probe(struct platform_device *pdev)
> +static void tegra_cpuidle_probe_work(struct work_struct *work)
>  {
>  	/* LP2 could be disabled in device-tree */
>  	if (tegra_pmc_get_suspend_mode() < TEGRA_SUSPEND_LP2)
> @@ -372,10 +373,18 @@ static int tegra_cpuidle_probe(struct
> platform_device *pdev)
>  		break;
> 
>  	default:
> -		return -EINVAL;
> +		return;
>  	}
> 
> -	return cpuidle_register(&tegra_idle_driver, cpu_possible_mask);
> +	cpuidle_register(&tegra_idle_driver, cpu_possible_mask);
> +}
> +
> +static DECLARE_DELAYED_WORK(delayed_probe, tegra_cpuidle_probe_work);
> +
> +static int tegra_cpuidle_probe(struct platform_device *pdev)
> +{
> +	schedule_delayed_work(&delayed_probe, 5 * HZ);
> +	return 0;
>  }
> 
>  static struct platform_driver tegra_cpuidle_driver = {
> 

Also, do you have CONFIG_ARM_TEGRA20_CPUFREQ=y? Please try to disable it
if it's enabled and if you enabled CPU OPPs in the device-tree.
Peter De Schrijver Dec. 11, 2019, 8:51 a.m. UTC | #10
On Tue, Dec 03, 2019 at 03:40:57AM +0300, Dmitry Osipenko wrote:
> Hello,
> 
> This series does the following:
> 
>   1. Unifies Tegra20/30/114 drivers into a single driver and moves it out
>      into common drivers/cpuidle/ directory.
> 
>   2. Enables CPU cluster power-down idling state on Tegra30.
> 
> In the end there is a quite nice clean up of the Tegra CPUIDLE drivers
> and of the Tegra's arch code in general. Please review, thanks!
> 

Acked-By Peter De Schrijver <pdeschrijver@nvidia.com> for the series.

Peter.
Dmitry Osipenko Dec. 12, 2019, 1:58 a.m. UTC | #11
11.12.2019 11:51, Peter De Schrijver пишет:
> On Tue, Dec 03, 2019 at 03:40:57AM +0300, Dmitry Osipenko wrote:
>> Hello,
>>
>> This series does the following:
>>
>>   1. Unifies Tegra20/30/114 drivers into a single driver and moves it out
>>      into common drivers/cpuidle/ directory.
>>
>>   2. Enables CPU cluster power-down idling state on Tegra30.
>>
>> In the end there is a quite nice clean up of the Tegra CPUIDLE drivers
>> and of the Tegra's arch code in general. Please review, thanks!
>>
> 
> Acked-By Peter De Schrijver <pdeschrijver@nvidia.com> for the series.
> 
> Peter.
> 

Thank you very much! Will be awesome if you could take a look at the
cpufreq patches as well [1].

There are also some other older unreviewed clk patches on the list that
I'd want you to take a look. I'll revisit and re-send them soon. Maybe
will add the PLLX LP1 patches [2] into cpufreq series.

[1] https://patchwork.ozlabs.org/project/linux-tegra/list/?series=143592

[2] https://patchwork.ozlabs.org/project/linux-tegra/list/?series=139741
Dmitry Osipenko Dec. 18, 2019, 7:10 a.m. UTC | #12
10.12.2019 19:21, Dmitry Osipenko пишет:
> 10.12.2019 19:02, Dmitry Osipenko пишет:
>> 10.12.2019 05:35, Michał Mirosław пишет:
>>> On Tue, Dec 10, 2019 at 12:22:18AM +0300, Dmitry Osipenko wrote:
>>>> 09.12.2019 19:04, Michał Mirosław пишет:
>>>>> On Sun, Dec 08, 2019 at 01:56:14AM +0300, Dmitry Osipenko wrote:
>>>>>> 08.12.2019 00:52, Michał Mirosław пишет:
>>>>>>> On Tue, Dec 03, 2019 at 03:40:57AM +0300, Dmitry Osipenko wrote:
>>>>>>>> Hello,
>>>>>>>>
>>>>>>>> This series does the following:
>>>>>>>>
>>>>>>>>   1. Unifies Tegra20/30/114 drivers into a single driver and moves it out
>>>>>>>>      into common drivers/cpuidle/ directory.
>>>>>>>>
>>>>>>>>   2. Enables CPU cluster power-down idling state on Tegra30.
>>>>>>>>
>>>>>>>> In the end there is a quite nice clean up of the Tegra CPUIDLE drivers
>>>>>>>> and of the Tegra's arch code in general. Please review, thanks!
>>>>>>>
>>>>>>> I did a quick smoke test for this series on top of Linus' master:
>>>>>>>  - rebuilding with the patches applied, CONFIG_ARM_TEGRA_CPUIDLE=n - works
>>>>>>>  - building with CONFIG_ARM_TEGRA_CPUIDLE=y - doesn't boot
>>>>>>>
>>>>>>> The hang is somewhere early in the boot process, before simplefb can
>>>>>>> take the console and show any logs. If I get BOOTFB to work again I might
>>>>>>> be able to get some more info.
>>>>>>
>>>>>> Thank you very much for trying these patches!
>>>>>>
>>>>>> Could you please try to make ARM_TEGRA_CPUIDLE "tristate" in the Kconfig
>>>>>> and compile it as a loadable module? That way you'll get framebuffer
>>>>>> shown before the hang happens.
>>>>>>
>>>>>> Does LP2 suspend/resume work for you? There should be
>>>>>> "nvidia,suspend-mode = <2>" in the PMC's node of device-tree.
>>>>>
>>>>> Not at the moment. I also tried suspend-mode = <1> and <0>, but it
>>>>> made no difference.
>>>>
>>>> If LP2 doesn't work, then it explains why you're getting the hang.
>>>>
>>>> Are you using TF300T for the testing? I'm recalling that LP2 worked for
>>>> you sometime ago on TF300T, maybe some offending change was introduced
>>>> since then. Could you please try to do the git bisection or at least
>>>> find out what is the last good kernel version?
>>>>
>>>> I rebased this series on a recent linux-next and you could find the
>>>> rebased patches here [1].
>>>>
>>>> [1] https://github.com/grate-driver/linux/commits/master
>>>>
>>>> With [1] you should be able to remove "nvidia,suspend-mode" property
>>>> from the device-tree to get cpuidle working with the disabled CC6 state
>>>> (LP2). Could you please check that at least disabled CC6 works for you?
>>>
>>> I tested suspend with your tree merged, but CONFIG_TEGRA_CPUIDLE=n. LP2
>>> seems to work [1]. The same tree with CONFIG_TEGRA_CPUIDLE=y doesn't
>>> boot. I'll try comparing DTs, but other than that I'm blocked on BOOTFB now.
>>
>> That's an interesting result.
>>
>>> [1] rtcwake -s 3 -d /dev/rtc0 -v -m mem
>>>
>>> (...)
>>> [ 2710.157919] PM: suspend entry (deep)
>>> [ 2710.161205] Filesystems sync: 0.000 seconds
>>> [ 2710.176677] Freezing user space processes ... (elapsed 0.001 seconds) done.
>>> [ 2710.178342] OOM killer disabled.
>>> [ 2710.178527] Freezing remaining freezable tasks ... (elapsed 0.001 seconds) done.
>>> [ 2710.347871] Disabling non-boot CPUs ...
>>> [ 2710.349160] IRQ 18: no longer affine to CPU1
>>> [ 2710.352499] IRQ 19: no longer affine to CPU2
>>> [ 2710.370059] IRQ 20: no longer affine to CPU3
>>> [ 2710.371284] Entering suspend state LP2
>>> [ 2710.371556] Enabling non-boot CPUs ...
>>> [ 2710.373157] CPU1 is up
>>> [ 2710.374598] CPU2 is up
>>> [ 2710.375996] CPU3 is up
>>> [ 2710.462876] OOM killer enabled.
>>> [ 2710.463018] Restarting tasks ...
>>> [ 2710.463880] tegra-devfreq 6000c800.actmon: Failed to get emc clock
>>> [ 2710.464509] done.
>>> [ 2710.552824] asus-ec 1-0015: model         : ASUS-TF201-PAD
>>> [ 2710.558345] asus-ec 1-0015: FW version    : PAD-EC20T-0216
>>> [ 2710.562942] asus-ec 1-0015: Config format : ECFG-0001
>>> [ 2710.567651] asus-ec 1-0015: HW version    : TF201-PAD-SKU1
>>> [ 2710.572488] asus-ec 1-0015: EC FW behaviour: susb on when system wakeup
>>> [ 2710.769796] atkbd serio1: no of_node; not parsing pinctrl DT
>>> [ 2710.835629] asus-ec 5-0019: model         : ASUS-TF201-DOCK
>>> [ 2710.838686] asus-ec 5-0019: FW version    : DOCK-EC20N-0207
>>> [ 2710.841865] asus-ec 5-0019: Config format : ECFG-0001
>>> [ 2710.844271] asus-ec 5-0019: HW version    : PCBA-SKU-2
>>> [ 2710.847950] asus-ec 5-0019: EC FW behaviour: susb on when receive ec_req
>>> [ 2711.040935] PM: suspend exit
>>>
>>
>> Could you please try this change on top of recent grate-linux (it should
>> allow display to light up before the hang):
>>
>> diff --git a/drivers/cpuidle/cpuidle-tegra.c
>> b/drivers/cpuidle/cpuidle-tegra.c
>> index db9ccba5a74c..21317b4e16c1 100644
>> --- a/drivers/cpuidle/cpuidle-tegra.c
>> +++ b/drivers/cpuidle/cpuidle-tegra.c
>> @@ -22,6 +22,7 @@
>>  #include <linux/ktime.h>
>>  #include <linux/platform_device.h>
>>  #include <linux/types.h>
>> +#include <linux/workqueue.h>
>>
>>  #include <linux/clk/tegra.h>
>>  #include <linux/firmware/trusted_foundations.h>
>> @@ -332,7 +333,7 @@ static void tegra_cpuidle_setup_tegra114_c7_state(void)
>>  	s->exit_latency = 500;
>>  }
>>
>> -static int tegra_cpuidle_probe(struct platform_device *pdev)
>> +static void tegra_cpuidle_probe_work(struct work_struct *work)
>>  {
>>  	/* LP2 could be disabled in device-tree */
>>  	if (tegra_pmc_get_suspend_mode() < TEGRA_SUSPEND_LP2)
>> @@ -372,10 +373,18 @@ static int tegra_cpuidle_probe(struct
>> platform_device *pdev)
>>  		break;
>>
>>  	default:
>> -		return -EINVAL;
>> +		return;
>>  	}
>>
>> -	return cpuidle_register(&tegra_idle_driver, cpu_possible_mask);
>> +	cpuidle_register(&tegra_idle_driver, cpu_possible_mask);
>> +}
>> +
>> +static DECLARE_DELAYED_WORK(delayed_probe, tegra_cpuidle_probe_work);
>> +
>> +static int tegra_cpuidle_probe(struct platform_device *pdev)
>> +{
>> +	schedule_delayed_work(&delayed_probe, 5 * HZ);
>> +	return 0;
>>  }
>>
>>  static struct platform_driver tegra_cpuidle_driver = {
>>
> 
> Also, do you have CONFIG_ARM_TEGRA20_CPUFREQ=y? Please try to disable it
> if it's enabled and if you enabled CPU OPPs in the device-tree.
> 

I'm going to drop the "Support CPU cluster power-down state on Tegra30"
for now to not block the rest of the patches.

Will get back to that patch once we'll fix the TF300T problem, perhaps
something needs to be changed for the Trusted Foundations firmware handling.