Skip to contentSkip to conversation

Tegra cpuidle driver fixes and improvements for 5.16

PATCH In review Dmitry Osipenko 3 participants 12 messages Sep 12, 2021 – Oct 4, 2021

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

cover Tegra cpuidle driver fixes and improvements for 5.16

This series adds compile-testing support to Tegra cpuidle driver and fixes a non-critical problem that should pop up only when power management patches
[1] will be merged. Previously I sent out these patches separately[2][3], but they partially missed recent kernel release. I combined the remaining patches into this series and added ack from Daniel Lezcano that he gave to the "Check whether PMC is ready" patch [3].

[1] https://patchwork.ozlabs.org/project/linux-tegra/list/?series=261973
[2] https://patchwork.ozlabs.org/project/linux-tegra/list/?series=252996
[3] https://patchwork.ozlabs.org/project/linux-tegra/list/?series=254040

Dmitry Osipenko (6):
soc/tegra: fuse: Add stubs needed for compile testing soc/tegra: irq: Add stubs needed for compile testing soc/tegra: pm: Make stubs usable for compile testing
clk: tegra: Add stubs needed for compile testing
cpuidle: tegra: Enable compile testing
cpuidle: tegra: Check whether PMC is ready

 drivers/cpuidle/Kconfig.arm     |  2 +-
 drivers/cpuidle/cpuidle-tegra.c |  3 +++
 include/linux/clk/tegra.h       |  4 ++++
 include/soc/tegra/fuse.h        | 31 ++++++++++++++++++++++++++-----
 include/soc/tegra/irq.h         |  9 ++++++++-
 include/soc/tegra/pm.h          |  2 +-
 6 files changed, 43 insertions(+), 8 deletions(-)
Signature · 1 line
2.32.0
#1 lore raw

PATCH 1/6 soc/tegra: fuse: Add stubs needed for compile testing

Add stubs needed for compile-testing of tegra-cpuidle driver.

---
 include/soc/tegra/fuse.h | 31 ++++++++++++++++++++++++++-----
 1 file changed, 26 insertions(+), 5 deletions(-)
include/soc/tegra/fuse.h +26 -5
index 990701f788bc..67d2bc856fbc 100644
@@ -6,6 +6,8 @@
 #ifndef __SOC_TEGRA_FUSE_H__
 #define __SOC_TEGRA_FUSE_H__
 
+#include <linux/types.h>
+
 #define TEGRA20		0x20
 #define TEGRA30		0x30
 #define TEGRA114	0x35
@@ -22,11 +24,6 @@
 
 #ifndef __ASSEMBLY__
 
-u32 tegra_read_chipid(void);
-u8 tegra_get_chip_id(void);
-u8 tegra_get_platform(void);
-bool tegra_is_silicon(void);
-
 enum tegra_revision {
 	TEGRA_REVISION_UNKNOWN = 0,
 	TEGRA_REVISION_A01,
@@ -57,6 +54,10 @@ extern struct tegra_sku_info tegra_sku_info;
 u32 tegra_read_straps(void);
 u32 tegra_read_ram_code(void);
 int tegra_fuse_readl(unsigned long offset, u32 *value);
+u32 tegra_read_chipid(void);
+u8 tegra_get_chip_id(void);
+u8 tegra_get_platform(void);
+bool tegra_is_silicon(void);
 #else
 static struct tegra_sku_info tegra_sku_info __maybe_unused;
 
@@ -74,6 +75,26 @@ static inline int tegra_fuse_readl(unsigned long offset, u32 *value)
 {
 	return -ENODEV;
 }
+
+static inline u32 tegra_read_chipid(void)
+{
+	return 0;
+}
+
+static inline u8 tegra_get_chip_id(void)
+{
+	return 0;
+}
+
+static inline u8 tegra_get_platform(void)
+{
+	return 0;
+}
+
+static inline bool tegra_is_silicon(void)
+{
+	return false;
+}
 #endif
 
 struct device *tegra_soc_device_register(void);
Signature · 1 line
2.32.0
#2 lore raw

PATCH 2/6 soc/tegra: irq: Add stubs needed for compile testing

In reply to Dmitry Osipenko

Add stubs needed for compile-testing of tegra-cpuidle driver.

---
 include/soc/tegra/irq.h | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)
include/soc/tegra/irq.h +8 -1
index 8eb11a7109e4..94539551c8c1 100644
@@ -6,8 +6,15 @@
 #ifndef __SOC_TEGRA_IRQ_H
 #define __SOC_TEGRA_IRQ_H
 
-#if defined(CONFIG_ARM)
+#include <linux/types.h>
+
+#if defined(CONFIG_ARM) && defined(CONFIG_ARCH_TEGRA)
 bool tegra_pending_sgi(void);
+#else
+static inline bool tegra_pending_sgi(void)
+{
+	return false;
+}
 #endif
 
 #endif /* __SOC_TEGRA_IRQ_H */
Signature · 1 line
2.32.0
#3 lore raw

PATCH 3/6 soc/tegra: pm: Make stubs usable for compile testing

In reply to Dmitry Osipenko

The PM stubs need to depend on ARCH_TEGRA in order to be usable for compile-testing of tegra-cpuidle driver. Add the dependency.

---
 include/soc/tegra/pm.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
include/soc/tegra/pm.h +1 -1
index 433878927026..ce4d0b1bd0d6 100644
@@ -17,7 +17,7 @@ enum tegra_suspend_mode {
 	TEGRA_SUSPEND_NOT_READY,
 };
 
-#if defined(CONFIG_PM_SLEEP) && defined(CONFIG_ARM)
+#if defined(CONFIG_PM_SLEEP) && defined(CONFIG_ARM) && defined(CONFIG_ARCH_TEGRA)
 enum tegra_suspend_mode
 tegra_pm_validate_suspend_mode(enum tegra_suspend_mode mode);
 
Signature · 1 line
2.32.0
#4 lore raw

PATCH 4/6 clk: tegra: Add stubs needed for compile testing

In reply to Dmitry Osipenko

Add stubs needed for compile-testing of tegra-cpuidle driver.

---
 include/linux/clk/tegra.h | 4 ++++
 1 file changed, 4 insertions(+)
include/linux/clk/tegra.h +4 -0
index d128ad1570aa..9bd06d8a5436 100644
@@ -42,7 +42,11 @@ struct tegra_cpu_car_ops {
 #endif
 };
 
+#ifdef CONFIG_ARCH_TEGRA
 extern struct tegra_cpu_car_ops *tegra_cpu_car_ops;
+#else
+static struct tegra_cpu_car_ops *tegra_cpu_car_ops __maybe_unused;
+#endif
 
 static inline void tegra_wait_cpu_in_reset(u32 cpu)
 {
Signature · 1 line
2.32.0
#5 lore raw

Re: [PATCH v1 4/6] clk: tegra: Add stubs needed for compile testing

Quoted 12 lines · Dmitry Osipenko

Add stubs needed for compile-testing of tegra-cpuidle driver.

---
 include/linux/clk/tegra.h | 4 ++++
 1 file changed, 4 insertions(+)
include/linux/clk/tegra.h +0 -0
index d128ad1570aa..9bd06d8a5436 100644

include/linux/clk/tegra.h struct tegra_cpu_car_ops { · line 49

Quoted 8 lines
include/linux/clk/tegra.h +4 -0
@@ -42,7 +42,11 @@ struct tegra_cpu_car_ops {
 #endif
 };
 
+#ifdef CONFIG_ARCH_TEGRA
 extern struct tegra_cpu_car_ops *tegra_cpu_car_ops;
+#else
+static struct tegra_cpu_car_ops *tegra_cpu_car_ops __maybe_unused;
+#endif

Ugh... this one seems a bit over the top, to be honest. The only place where this seems to be used is in arch/arm/mach-tegra/pm.c, but that already uses one of the stubs from include/linux/clk/tegra.h, so I'm wondering if we can't define that latter stub in a way to make it unnecessary to declare this bogus pointer.

I'll play around with this a little bit.

Thierry

#6 lore raw

Re: [PATCH v1 4/6] clk: tegra: Add stubs needed for compile testing

Quoted 28 lines · Thierry Reding
Quoted 12 lines · Dmitry Osipenko

Add stubs needed for compile-testing of tegra-cpuidle driver.

---
 include/linux/clk/tegra.h | 4 ++++
 1 file changed, 4 insertions(+)
include/linux/clk/tegra.h +0 -0
index d128ad1570aa..9bd06d8a5436 100644

include/linux/clk/tegra.h struct tegra_cpu_car_ops { · line 49

Quoted 8 lines
include/linux/clk/tegra.h +4 -0
@@ -42,7 +42,11 @@ struct tegra_cpu_car_ops {
 #endif
 };
 
+#ifdef CONFIG_ARCH_TEGRA
 extern struct tegra_cpu_car_ops *tegra_cpu_car_ops;
+#else
+static struct tegra_cpu_car_ops *tegra_cpu_car_ops __maybe_unused;
+#endif

Ugh... this one seems a bit over the top, to be honest. The only place where this seems to be used is in arch/arm/mach-tegra/pm.c, but that already uses one of the stubs from include/linux/clk/tegra.h, so I'm wondering if we can't define that latter stub in a way to make it unnecessary to declare this bogus pointer.

Quoted 1 line

I'll play around with this a little bit.

The below does the trick for me as well:

--- >8 ---

include/linux/clk/tegra.h +22 -0
index d128ad1570aa..d261db7e6060 100644
@@ -42,6 +42,7 @@ struct tegra_cpu_car_ops {
 #endif
 };
 
+#ifdef CONFIG_ARCH_TEGRA
 extern struct tegra_cpu_car_ops *tegra_cpu_car_ops;
 
 static inline void tegra_wait_cpu_in_reset(u32 cpu)
@@ -83,6 +84,27 @@ static inline void tegra_disable_cpu_clock(u32 cpu)
 
 	tegra_cpu_car_ops->disable_clock(cpu);
 }
+#else
+static inline void tegra_wait_cpu_in_reset(u32 cpu)
+{
+}
+
+static inline void tegra_put_cpu_in_reset(u32 cpu)
+{
+}
+
+static inline void tegra_cpu_out_of_reset(u32 cpu)
+{
+}
+
+static inline void tegra_enable_cpu_clock(u32 cpu)
+{
+}
+
+static inline void tegra_disable_cpu_clock(u32 cpu)
+{
+}
+#endif
 
 #ifdef CONFIG_PM_SLEEP
 static inline bool tegra_cpu_rail_off_ready(void)

--- >8 ---

Do you mind if I replace your version with that? I think that's a little bit cleaner because it should be easier for the compiler to completely compile it out.

Thierry

#7 lore raw

Re: [PATCH v1 4/6] clk: tegra: Add stubs needed for compile testing

04.10.2021 23:02, Thierry Reding пишет:

Quoted 76 lines · Thierry Reding
Quoted 28 lines · Dmitry Osipenko
Quoted 12 lines

Add stubs needed for compile-testing of tegra-cpuidle driver.

---
 include/linux/clk/tegra.h | 4 ++++
 1 file changed, 4 insertions(+)
include/linux/clk/tegra.h +0 -0
index d128ad1570aa..9bd06d8a5436 100644

include/linux/clk/tegra.h struct tegra_cpu_car_ops { · line 49

Quoted 8 lines
include/linux/clk/tegra.h +4 -0
@@ -42,7 +42,11 @@ struct tegra_cpu_car_ops {
 #endif
 };
 
+#ifdef CONFIG_ARCH_TEGRA
 extern struct tegra_cpu_car_ops *tegra_cpu_car_ops;
+#else
+static struct tegra_cpu_car_ops *tegra_cpu_car_ops __maybe_unused;
+#endif

Ugh... this one seems a bit over the top, to be honest. The only place where this seems to be used is in arch/arm/mach-tegra/pm.c, but that already uses one of the stubs from include/linux/clk/tegra.h, so I'm wondering if we can't define that latter stub in a way to make it unnecessary to declare this bogus pointer.

Quoted 1 line

I'll play around with this a little bit.

The below does the trick for me as well:

--- >8 ---

include/linux/clk/tegra.h +22 -1
index d128ad1570aa..d261db7e6060 100644
@@ -42,6 +42,7 @@ struct tegra_cpu_car_ops {
 #endif
 };
 
+#ifdef CONFIG_ARCH_TEGRA
 extern struct tegra_cpu_car_ops *tegra_cpu_car_ops;
 
 static inline void tegra_wait_cpu_in_reset(u32 cpu)
@@ -83,6 +84,27 @@ static inline void tegra_disable_cpu_clock(u32 cpu)
 
 	tegra_cpu_car_ops->disable_clock(cpu);
 }
+#else
+static inline void tegra_wait_cpu_in_reset(u32 cpu)
+{
+}
+
+static inline void tegra_put_cpu_in_reset(u32 cpu)
+{
+}
+
+static inline void tegra_cpu_out_of_reset(u32 cpu)
+{
+}
+
+static inline void tegra_enable_cpu_clock(u32 cpu)
+{
+}
+
+static inline void tegra_disable_cpu_clock(u32 cpu)
+{
+}
+#endif
 
 #ifdef CONFIG_PM_SLEEP
 static inline bool tegra_cpu_rail_off_ready(void)
--- >8 ---
Quoted 3 lines

Do you mind if I replace your version with that? I think that's a little bit cleaner because it should be easier for the compiler to completely compile it out.

I don't mind, please choose what you prefer more. This was one of the first variants of this patch, it adds more lines and it won't be compiled with ARCH_TEGRA=n and COMPILE_TEST=n anyways.

#8 lore raw

PATCH 5/6 cpuidle: tegra: Enable compile testing

In reply to Dmitry Osipenko

Enable compile testing of tegra-cpuidle driver.

---
 drivers/cpuidle/Kconfig.arm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/cpuidle/Kconfig.arm +1 -1
index 334f83e56120..599286fc0b08 100644
@@ -99,7 +99,7 @@ config ARM_MVEBU_V7_CPUIDLE
 
 config ARM_TEGRA_CPUIDLE
 	bool "CPU Idle Driver for NVIDIA Tegra SoCs"
-	depends on ARCH_TEGRA && !ARM64
+	depends on (ARCH_TEGRA || COMPILE_TEST) && !ARM64 && MMU
 	select ARCH_NEEDS_CPU_IDLE_COUPLED if SMP
 	select ARM_CPU_SUSPEND
 	help
Signature · 1 line
2.32.0
#9 lore raw

Re: [PATCH v1 5/6] cpuidle: tegra: Enable compile testing

12.09.2021 23:29, Dmitry Osipenko пишет:

Quoted 13 lines

Enable compile testing of tegra-cpuidle driver.

---
 drivers/cpuidle/Kconfig.arm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/cpuidle/Kconfig.arm +0 -0
index 334f83e56120..599286fc0b08 100644

drivers/cpuidle/Kconfig.arm config ARM_MVEBU_V7_CPUIDLE · line 105

Quoted 8 lines
drivers/cpuidle/Kconfig.arm +1 -1
@@ -99,7 +99,7 @@ config ARM_MVEBU_V7_CPUIDLE
 
 config ARM_TEGRA_CPUIDLE
 	bool "CPU Idle Driver for NVIDIA Tegra SoCs"
-	depends on ARCH_TEGRA && !ARM64
+	depends on (ARCH_TEGRA || COMPILE_TEST) && !ARM64 && MMU
 	select ARCH_NEEDS_CPU_IDLE_COUPLED if SMP
 	select ARM_CPU_SUSPEND
 	help

Daniel / Rafael, could you please ack this patch to allow Thierry to take this whole series via the Tegra tree?

#10 lore raw

Re: [PATCH v1 5/6] cpuidle: tegra: Enable compile testing

Quoted 14 lines · Dmitry Osipenko

12.09.2021 23:29, Dmitry Osipenko пишет:

Quoted 13 lines

Enable compile testing of tegra-cpuidle driver.

---
 drivers/cpuidle/Kconfig.arm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/cpuidle/Kconfig.arm +0 -0
index 334f83e56120..599286fc0b08 100644
@@ -99,7 +99,7 @@ config ARM_MVEBU_V7_CPUIDLE
 
Quoted 12 lines

drivers/cpuidle/Kconfig.arm config ARM_MVEBU_V7_CPUIDLE · line 105

Quoted 8 lines
drivers/cpuidle/Kconfig.arm +1 -1
 config ARM_TEGRA_CPUIDLE
 	bool "CPU Idle Driver for NVIDIA Tegra SoCs"
-	depends on ARCH_TEGRA && !ARM64
+	depends on (ARCH_TEGRA || COMPILE_TEST) && !ARM64 && MMU
 	select ARCH_NEEDS_CPU_IDLE_COUPLED if SMP
 	select ARM_CPU_SUSPEND
 	help

Daniel / Rafael, could you please ack this patch to allow Thierry to take this whole series via the Tegra tree?

Signature · 5 lines
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

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

PATCH 6/6 cpuidle: tegra: Check whether PMC is ready

In reply to Dmitry Osipenko

Check whether PMC is ready before proceeding with the cpuidle registration. This fixes racing with the PMC driver probe order, which results in a disabled deepest CC6 idling state if cpuidle driver is probed before the PMC.

---
 drivers/cpuidle/cpuidle-tegra.c | 3 +++
 1 file changed, 3 insertions(+)
drivers/cpuidle/cpuidle-tegra.c +3 -0
index 508bd9f23792..9845629aeb6d 100644
@@ -337,6 +337,9 @@ static void tegra_cpuidle_setup_tegra114_c7_state(void)
 
 static int tegra_cpuidle_probe(struct platform_device *pdev)
 {
+	if (tegra_pmc_get_suspend_mode() == TEGRA_SUSPEND_NOT_READY)
+		return -EPROBE_DEFER;
+
 	/* LP2 could be disabled in device-tree */
 	if (tegra_pmc_get_suspend_mode() < TEGRA_SUSPEND_LP2)
 		tegra_cpuidle_disable_state(TEGRA_CC6);
Signature · 1 line
2.32.0