Consolidate and improve NVIDIA Tegra CPUIDLE driver(s)

Remove the old drivers to replace them cleanly with a new one later on.

Signed-off-by Dmitry Osipenko <[email protected]>

---
 arch/arm/mach-tegra/Makefile           |  13 --
 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              |  18 ---
 arch/arm/mach-tegra/irq.h              |  11 --
 arch/arm/mach-tegra/pm.c               |   7 -
 arch/arm/mach-tegra/pm.h               |   1 -
 arch/arm/mach-tegra/reset-handler.S    |  11 --
 arch/arm/mach-tegra/reset.h            |   9 +-
 arch/arm/mach-tegra/sleep-tegra20.S    | 190 +---------------------
 arch/arm/mach-tegra/sleep.h            |  12 --
 arch/arm/mach-tegra/tegra.c            |   3 -
 drivers/soc/tegra/Kconfig              |   1 -

 include/soc/tegra/cpuidle.h            |   4 -
 17 files changed, 5 insertions(+), 779 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
 delete mode 100644 arch/arm/mach-tegra/irq.h

The upcoming unified CPUIDLE driver will be added to the drivers/cpuidle/ directory and it will require all these Tegra PM-core functions.

Signed-off-by Dmitry Osipenko <[email protected]>

---
 arch/arm/mach-tegra/Makefile  |  2 +-
 arch/arm/mach-tegra/platsmp.c |  2 --
 arch/arm/mach-tegra/pm.c      | 16 +++++++---------
 arch/arm/mach-tegra/pm.h      |  3 ---
 arch/arm/mach-tegra/sleep.h   |  1 -
 include/linux/clk/tegra.h     | 13 +++++++++++++
 include/soc/tegra/pm.h        | 28 ++++++++++++++++++++++++++++
 7 files changed, 49 insertions(+), 16 deletions(-)

arch/arm/mach-tegra/Makefile +1 -1

index 5d93a0b36866..27bd5d9865e3 100644
@@ -13,7 +13,7 @@ obj-$(CONFIG_ARCH_TEGRA_2x_SOC)		+= pm-tegra20.o
 obj-$(CONFIG_ARCH_TEGRA_3x_SOC)		+= sleep-tegra30.o
 obj-$(CONFIG_ARCH_TEGRA_3x_SOC)		+= pm-tegra30.o
 obj-$(CONFIG_SMP)			+= platsmp.o
-obj-$(CONFIG_HOTPLUG_CPU)               += hotplug.o
+obj-y					+= hotplug.o
 
 obj-$(CONFIG_ARCH_TEGRA_114_SOC)	+= sleep-tegra30.o
 obj-$(CONFIG_ARCH_TEGRA_114_SOC)	+= pm-tegra30.o

arch/arm/mach-tegra/platsmp.c +0 -2

index e6911a14c096..c8a63719a143 100644
@@ -183,8 +183,6 @@ const struct smp_operations tegra_smp_ops __initconst = {
 	.smp_prepare_cpus	= tegra_smp_prepare_cpus,
 	.smp_secondary_init	= tegra_secondary_init,
 	.smp_boot_secondary	= tegra_boot_secondary,
-#ifdef CONFIG_HOTPLUG_CPU
 	.cpu_kill		= tegra_cpu_kill,
 	.cpu_die		= tegra_cpu_die,
-#endif
 };

arch/arm/mach-tegra/pm.c +1 -3

index 6aaacb5757e1..f9c9bce9e15d 100644
@@ -123,11 +123,9 @@ void tegra_clear_cpu_in_lp2(void)
 	spin_unlock(&tegra_lp2_lock);
 }
 
-bool tegra_set_cpu_in_lp2(void)
+void tegra_set_cpu_in_lp2(void)
 {
 	int phy_cpu_id = cpu_logical_map(smp_processor_id());
-	bool last_cpu = false;
-	cpumask_t *cpu_lp2_mask = tegra_cpu_lp2_mask;
 	u32 *cpu_in_lp2 = tegra_cpu_lp2_mask;
 
 	spin_lock(&tegra_lp2_lock);

arch/arm/mach-tegra/pm.c +6 -2

 static int tegra_sleep_cpu(unsigned long v2p)

@@ -195,14 +189,16 @@ static void tegra_pm_set(enum tegra_suspend_mode mode)
 	tegra_pmc_enter_suspend_mode(mode);
 }
 
-void tegra_idle_lp2_last(void)
+int tegra_idle_lp2_last(void)
 {
+	int err;
+
 	tegra_pm_set(TEGRA_SUSPEND_LP2);
 
 	cpu_cluster_pm_enter();
 	suspend_cpu_complex();
 
-	cpu_suspend(PHYS_OFFSET - PAGE_OFFSET, &tegra_sleep_cpu);
+	err = cpu_suspend(PHYS_OFFSET - PAGE_OFFSET, &tegra_sleep_cpu);
 
 	/*
 	 * Resume L2 cache if it wasn't re-enabled early during resume,

@@ -214,6 +210,8 @@ void tegra_idle_lp2_last(void)
 
 	restore_cpu_complex();
 	cpu_cluster_pm_exit();
+
+	return err;
 }
 
 enum tegra_suspend_mode tegra_pm_validate_suspend_mode(

arch/arm/mach-tegra/pm.h +0 -3

index 1e51a9b636eb..81525f5f4a44 100644
@@ -23,9 +23,6 @@ void tegra20_sleep_core_init(void);
 void tegra30_lp1_iram_hook(void);
 void tegra30_sleep_core_init(void);
 
-void tegra_clear_cpu_in_lp2(void);
-bool tegra_set_cpu_in_lp2(void);
-void tegra_idle_lp2_last(void);
 extern void (*tegra_tear_down_cpu)(void);
 
 #ifdef CONFIG_PM_SLEEP

arch/arm/mach-tegra/sleep.h +0 -1

index d219872b7546..0d9956e9a8ea 100644
@@ -124,7 +124,6 @@ void tegra30_hotplug_shutdown(void);
 #endif
 
 void tegra20_tear_down_cpu(void);
-int tegra30_sleep_cpu_secondary_finish(unsigned long);
 void tegra30_tear_down_cpu(void);
 
 #endif

include/linux/clk/tegra.h +13 -0

index b8aef62cc3f5..cf0f2cb5e109 100644
@@ -108,6 +108,19 @@ static inline void tegra_cpu_clock_resume(void)
 
 	tegra_cpu_car_ops->resume();
 }
+#else
+static inline bool tegra_cpu_rail_off_ready(void)
+{
+	return false;
+}
+
+static inline void tegra_cpu_clock_suspend(void)
+{
+}
+
+static inline void tegra_cpu_clock_resume(void)
+{
+}
 #endif
 
 extern void tegra210_xusb_pll_hw_control_enable(void);

include/soc/tegra/pm.h +6 -0

index 951fcd738d55..fa18c2df5028 100644
@@ -20,6 +20,12 @@ tegra_pm_validate_suspend_mode(enum tegra_suspend_mode mode);
 
 /* low-level resume entry point */
 void tegra_resume(void);
+
+int tegra30_sleep_cpu_secondary_finish(unsigned long arg);
+void tegra_clear_cpu_in_lp2(void);
+void tegra_set_cpu_in_lp2(void);
+int tegra_idle_lp2_last(void);
+void tegra_cpu_die(unsigned int cpu);
 #else
 static inline enum tegra_suspend_mode
 tegra_pm_validate_suspend_mode(enum tegra_suspend_mode mode)

Quoted 20 lines

Remove the old drivers to replace them cleanly with a new one later on.

Signed-off-by Dmitry Osipenko <[email protected]>

---
 arch/arm/mach-tegra/Makefile           |  13 --
 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              |  18 ---
 arch/arm/mach-tegra/irq.h              |  11 --
 arch/arm/mach-tegra/pm.c               |   7 -
 arch/arm/mach-tegra/pm.h               |   1 -
 arch/arm/mach-tegra/reset-handler.S    |  11 --
 arch/arm/mach-tegra/reset.h            |   9 +-
 arch/arm/mach-tegra/sleep-tegra20.S    | 190 +---------------------
 arch/arm/mach-tegra/sleep.h            |  12 --
 arch/arm/mach-tegra/tegra.c            |   3 -
 drivers/soc/tegra/Kconfig              |   1 -

Quoted 8 lines

 include/soc/tegra/cpuidle.h            |   4 -
 17 files changed, 5 insertions(+), 779 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
 delete mode 100644 arch/arm/mach-tegra/irq.h

By removing all the above, it is really hard to review the diff. Is there any way you could first consolidate the cpuidle drivers into say the existing arch/arm/mach-tegra/cpuidle-tegra20.c and then move to drivers/cpuidle?

Quoted 61 lines

The upcoming unified CPUIDLE driver will be added to the drivers/cpuidle/ directory and it will require all these Tegra PM-core functions.

Signed-off-by Dmitry Osipenko <[email protected]>

---
 arch/arm/mach-tegra/Makefile  |  2 +-
 arch/arm/mach-tegra/platsmp.c |  2 --
 arch/arm/mach-tegra/pm.c      | 16 +++++++---------
 arch/arm/mach-tegra/pm.h      |  3 ---
 arch/arm/mach-tegra/sleep.h   |  1 -
 include/linux/clk/tegra.h     | 13 +++++++++++++
 include/soc/tegra/pm.h        | 28 ++++++++++++++++++++++++++++
 7 files changed, 49 insertions(+), 16 deletions(-)

arch/arm/mach-tegra/Makefile +1 -1

index 5d93a0b36866..27bd5d9865e3 100644
@@ -13,7 +13,7 @@ obj-$(CONFIG_ARCH_TEGRA_2x_SOC)		+= pm-tegra20.o
 obj-$(CONFIG_ARCH_TEGRA_3x_SOC)		+= sleep-tegra30.o
 obj-$(CONFIG_ARCH_TEGRA_3x_SOC)		+= pm-tegra30.o
 obj-$(CONFIG_SMP)			+= platsmp.o
-obj-$(CONFIG_HOTPLUG_CPU)               += hotplug.o
+obj-y					+= hotplug.o
 
 obj-$(CONFIG_ARCH_TEGRA_114_SOC)	+= sleep-tegra30.o
 obj-$(CONFIG_ARCH_TEGRA_114_SOC)	+= pm-tegra30.o

arch/arm/mach-tegra/platsmp.c +0 -2

index e6911a14c096..c8a63719a143 100644
@@ -183,8 +183,6 @@ const struct smp_operations tegra_smp_ops __initconst = {
 	.smp_prepare_cpus	= tegra_smp_prepare_cpus,
 	.smp_secondary_init	= tegra_secondary_init,
 	.smp_boot_secondary	= tegra_boot_secondary,
-#ifdef CONFIG_HOTPLUG_CPU
 	.cpu_kill		= tegra_cpu_kill,
 	.cpu_die		= tegra_cpu_die,
-#endif
 };

arch/arm/mach-tegra/pm.c +1 -3

index 6aaacb5757e1..f9c9bce9e15d 100644
@@ -123,11 +123,9 @@ void tegra_clear_cpu_in_lp2(void)
 	spin_unlock(&tegra_lp2_lock);
 }
 
-bool tegra_set_cpu_in_lp2(void)
+void tegra_set_cpu_in_lp2(void)
 {
 	int phy_cpu_id = cpu_logical_map(smp_processor_id());
-	bool last_cpu = false;
-	cpumask_t *cpu_lp2_mask = tegra_cpu_lp2_mask;
 	u32 *cpu_in_lp2 = tegra_cpu_lp2_mask;
 
 	spin_lock(&tegra_lp2_lock);

@@ -135,11 +133,7 @@ bool tegra_set_cpu_in_lp2(void)
 	BUG_ON((*cpu_in_lp2 & BIT(phy_cpu_id)));
 	*cpu_in_lp2 |= BIT(phy_cpu_id);
 

arch/arm/mach-tegra/pm.c bool tegra_set_cpu_in_lp2(void) · line 137

Quoted 6 lines

arch/arm/mach-tegra/pm.c +0 -4

-	if ((phy_cpu_id == 0) && cpumask_equal(cpu_lp2_mask, cpu_online_mask))
-		last_cpu = true;
-
 	spin_unlock(&tegra_lp2_lock);
-	return last_cpu;
 }

I think that the commit message should describe what is going on here or this should be a separate change.

Quoted 99 lines

arch/arm/mach-tegra/pm.c +6 -2

 static int tegra_sleep_cpu(unsigned long v2p)

@@ -195,14 +189,16 @@ static void tegra_pm_set(enum tegra_suspend_mode mode)
 	tegra_pmc_enter_suspend_mode(mode);
 }
 
-void tegra_idle_lp2_last(void)
+int tegra_idle_lp2_last(void)
 {
+	int err;
+
 	tegra_pm_set(TEGRA_SUSPEND_LP2);
 
 	cpu_cluster_pm_enter();
 	suspend_cpu_complex();
 
-	cpu_suspend(PHYS_OFFSET - PAGE_OFFSET, &tegra_sleep_cpu);
+	err = cpu_suspend(PHYS_OFFSET - PAGE_OFFSET, &tegra_sleep_cpu);
 
 	/*
 	 * Resume L2 cache if it wasn't re-enabled early during resume,

@@ -214,6 +210,8 @@ void tegra_idle_lp2_last(void)
 
 	restore_cpu_complex();
 	cpu_cluster_pm_exit();
+
+	return err;
 }
 
 enum tegra_suspend_mode tegra_pm_validate_suspend_mode(

arch/arm/mach-tegra/pm.h +0 -3

index 1e51a9b636eb..81525f5f4a44 100644
@@ -23,9 +23,6 @@ void tegra20_sleep_core_init(void);
 void tegra30_lp1_iram_hook(void);
 void tegra30_sleep_core_init(void);
 
-void tegra_clear_cpu_in_lp2(void);
-bool tegra_set_cpu_in_lp2(void);
-void tegra_idle_lp2_last(void);
 extern void (*tegra_tear_down_cpu)(void);
 
 #ifdef CONFIG_PM_SLEEP

arch/arm/mach-tegra/sleep.h +0 -1

index d219872b7546..0d9956e9a8ea 100644
@@ -124,7 +124,6 @@ void tegra30_hotplug_shutdown(void);
 #endif
 
 void tegra20_tear_down_cpu(void);
-int tegra30_sleep_cpu_secondary_finish(unsigned long);
 void tegra30_tear_down_cpu(void);
 
 #endif

include/linux/clk/tegra.h +13 -0

index b8aef62cc3f5..cf0f2cb5e109 100644
@@ -108,6 +108,19 @@ static inline void tegra_cpu_clock_resume(void)
 
 	tegra_cpu_car_ops->resume();
 }
+#else
+static inline bool tegra_cpu_rail_off_ready(void)
+{
+	return false;
+}
+
+static inline void tegra_cpu_clock_suspend(void)
+{
+}
+
+static inline void tegra_cpu_clock_resume(void)
+{
+}
 #endif
 
 extern void tegra210_xusb_pll_hw_control_enable(void);

include/soc/tegra/pm.h +6 -0

index 951fcd738d55..fa18c2df5028 100644
@@ -20,6 +20,12 @@ tegra_pm_validate_suspend_mode(enum tegra_suspend_mode mode);
 
 /* low-level resume entry point */
 void tegra_resume(void);
+
+int tegra30_sleep_cpu_secondary_finish(unsigned long arg);
+void tegra_clear_cpu_in_lp2(void);
+void tegra_set_cpu_in_lp2(void);
+int tegra_idle_lp2_last(void);
+void tegra_cpu_die(unsigned int cpu);
 #else
 static inline enum tegra_suspend_mode
 tegra_pm_validate_suspend_mode(enum tegra_suspend_mode mode)

@@ -30,6 +36,28 @@ tegra_pm_validate_suspend_mode(enum tegra_suspend_mode mode)
 static inline void tegra_resume(void)
 {

include/soc/tegra/pm.h tegra_pm_validate_suspend_mode(enum tegra_suspend_mode mode) · line 43

Quoted 6 lines

include/soc/tegra/pm.h +5 -0

 }
+
+static inline int tegra30_sleep_cpu_secondary_finish(unsigned long arg)
+{
+	return -1;
+}

-ENOTSUPP?

The new driver is based on the old CPU Idle drivers that are removed now from arm/arch/mach-tegra/ directory. Those removed drivers were reworked and squashed into a single unified driver that covers multiple hardware generations, starting from Tegra20 and ending with Tegra124.

Signed-off-by Dmitry Osipenko <[email protected]>

---
 arch/arm/mach-tegra/tegra.c     |   4 +
 drivers/cpuidle/Kconfig.arm     |   8 +
 drivers/cpuidle/Makefile        |   1 +
 drivers/cpuidle/cpuidle-tegra.c | 304 ++++++++++++++++++++++++++++++++
 include/soc/tegra/cpuidle.h     |   4 +
 5 files changed, 321 insertions(+)
 create mode 100644 drivers/cpuidle/cpuidle-tegra.c

arch/arm/mach-tegra/tegra.c +4 -0

index d9237769a37c..f1ce2857a251 100644
@@ -36,6 +36,7 @@
 #include <asm/mach/arch.h>
 #include <asm/mach/time.h>
 #include <asm/mach-types.h>
+#include <asm/psci.h>
 #include <asm/setup.h>
 
 #include "board.h"

@@ -92,6 +93,9 @@ static void __init tegra_dt_init_late(void)
 	if (IS_ENABLED(CONFIG_ARCH_TEGRA_2x_SOC) &&
 	    of_machine_is_compatible("nvidia,tegra20"))
 		platform_device_register_simple("tegra20-cpufreq", -1, NULL, 0);
+
+	if (IS_ENABLED(CONFIG_ARM_TEGRA_CPUIDLE) && !psci_smp_available())
+		platform_device_register_simple("tegra-cpuidle", -1, NULL, 0);
 }
 
 static const char * const tegra_dt_board_compat[] = {

drivers/cpuidle/Kconfig.arm +8 -0

index 48cb3d4bb7d1..d90861361f1d 100644
@@ -76,3 +76,11 @@ config ARM_MVEBU_V7_CPUIDLE
 	depends on ARCH_MVEBU && !ARM64
 	help
 	  Select this to enable cpuidle on Armada 370, 38x and XP processors.
+
+config ARM_TEGRA_CPUIDLE
+	bool "CPU Idle Driver for NVIDIA Tegra SoCs"
+	depends on ARCH_TEGRA && !ARM64
+	select ARCH_NEEDS_CPU_IDLE_COUPLED if SMP
+	select ARM_CPU_SUSPEND
+	help
+	  Select this to enable cpuidle for NVIDIA Tegra20/30/114/124 SoCs.

drivers/cpuidle/Makefile +1 -0

index 9d7176cee3d3..470d17fa8746 100644
@@ -20,6 +20,7 @@ obj-$(CONFIG_ARM_U8500_CPUIDLE)         += cpuidle-ux500.o
 obj-$(CONFIG_ARM_AT91_CPUIDLE)          += cpuidle-at91.o
 obj-$(CONFIG_ARM_EXYNOS_CPUIDLE)        += cpuidle-exynos.o
 obj-$(CONFIG_ARM_CPUIDLE)		+= cpuidle-arm.o
+obj-$(CONFIG_ARM_TEGRA_CPUIDLE)		+= cpuidle-tegra.o
 
 ###############################################################################
 # MIPS drivers

drivers/cpuidle/cpuidle-tegra.c new file +120 -0

new file mode 100644
index 000000000000..54974cd2255f
@@ -0,0 +1,304 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * CPU idle driver for Tegra CPUs
+ *
+ * Copyright (c) 2010-2013, NVIDIA Corporation.
+ * Copyright (c) 2011 Google, Inc.
+ * Author: Colin Cross <[email protected]>
+ *         Gary King <[email protected]>
+ *
+ * Rework for 3.3 by Peter De Schrijver <[email protected]>
+ *
+ * Tegra20/124 driver unification by Dmitry Osipenko <[email protected]>
+ */
+
+#include <linux/cpuidle.h>
+#include <linux/cpumask.h>
+#include <linux/cpu_pm.h>
+#include <linux/errno.h>
+#include <linux/ktime.h>
+#include <linux/platform_device.h>
+#include <linux/types.h>
+
+#include <linux/clk/tegra.h>
+#include <linux/firmware/trusted_foundations.h>
+
+#include <soc/tegra/cpuidle.h>
+#include <soc/tegra/flowctrl.h>
+#include <soc/tegra/fuse.h>
+#include <soc/tegra/pm.h>
+
+#include <asm/cpuidle.h>
+#include <asm/firmware.h>
+#include <asm/smp_plat.h>
+#include <asm/suspend.h>
+
+#define TEGRA_C1		0
+#define TEGRA_C7		1
+#define TEGRA_CC6		2
+
+static atomic_t tegra_idle_barrier;
+
+static inline bool tegra_cpuidle_using_firmware(void)
+{
+	return firmware_ops->prepare_idle && firmware_ops->do_idle;
+}
+
+static int tegra_shutdown_secondary_cpu(unsigned long cpu)
+{
+	tegra_cpu_die(cpu);
+
+	return -EINVAL;
+}
+
+static int tegra_await_secondary_cpus_shutdown(void)
+{
+	ktime_t timeout = ktime_add_ms(ktime_get(), 1);
+
+	/*
+	 * The boot CPU shall await for the secondaries shutdown in order
+	 * to power-off CPU's cluster safely.
+	 */
+	do {
+		if (tegra_cpu_rail_off_ready())
+			return 0;
+
+	} while (ktime_compare(ktime_get(), timeout) < 0);
+
+	return -ETIMEDOUT;
+}
+
+static void tegra_wake_up_secondary_cpus(void)
+{
+	unsigned int cpu, lcpu;
+
+	for_each_cpu(lcpu, cpu_online_mask) {
+		cpu = cpu_logical_map(lcpu);
+
+		if (cpu > 0) {
+			tegra_enable_cpu_clock(cpu);
+			tegra_cpu_out_of_reset(cpu);
+			flowctrl_write_cpu_halt(cpu, 0);
+		}
+	}
+}
+
+static int tegra_cpuidle_cc6_enter(void)
+{
+	int err;
+
+	err = tegra_await_secondary_cpus_shutdown();
+	if (err)
+		return err;
+
+	err = tegra_idle_lp2_last();
+
+	tegra_wake_up_secondary_cpus();
+
+	return err;
+}
+
+static int tegra_cpuidle_c7_enter(void)
+{
+	int err;
+
+	if (tegra_cpuidle_using_firmware()) {
+		err = call_firmware_op(prepare_idle, TF_PM_MODE_LP2_NOFLUSH_L2);
+		if (err)
+			return err;
+
+		return call_firmware_op(do_idle, 0);
+	}
+
+	return cpu_suspend(0, tegra30_sleep_cpu_secondary_finish);
+}
+
+static int tegra_cpuidle_enter(struct cpuidle_device *dev,
+			       int index, unsigned int cpu)
+{
+	int err;
+

11.07.2019 12:26, Jon Hunter пишет:

Quoted 31 lines · Dmitry Osipenko

Quoted 20 lines

Remove the old drivers to replace them cleanly with a new one later on.

Signed-off-by Dmitry Osipenko <[email protected]>

---
 arch/arm/mach-tegra/Makefile           |  13 --
 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              |  18 ---
 arch/arm/mach-tegra/irq.h              |  11 --
 arch/arm/mach-tegra/pm.c               |   7 -
 arch/arm/mach-tegra/pm.h               |   1 -
 arch/arm/mach-tegra/reset-handler.S    |  11 --
 arch/arm/mach-tegra/reset.h            |   9 +-
 arch/arm/mach-tegra/sleep-tegra20.S    | 190 +---------------------
 arch/arm/mach-tegra/sleep.h            |  12 --
 arch/arm/mach-tegra/tegra.c            |   3 -
 drivers/soc/tegra/Kconfig              |   1 -

Quoted 8 lines

 include/soc/tegra/cpuidle.h            |   4 -
 17 files changed, 5 insertions(+), 779 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
 delete mode 100644 arch/arm/mach-tegra/irq.h

Quoted 4 lines

By removing all the above, it is really hard to review the diff. Is there any way you could first consolidate the cpuidle drivers into say the existing arch/arm/mach-tegra/cpuidle-tegra20.c and then move to drivers/cpuidle?

I'm afraid that it will make reviewing even more difficult because everything that is removed here is not returned in the further patches. The new driver is based on the older ones, but I wrote it from scratch and it's not only looks different, but also works a bit different as you may see.

Could you please clarify what exactly makes it hard to review? The diff looks pretty clean to me, while squashing everything into existing driver should be quite a mess.

Quoted 43 lines

11.07.2019 12:26, Jon Hunter пишет:

Quoted 31 lines · Dmitry Osipenko

Quoted 20 lines

Remove the old drivers to replace them cleanly with a new one later on.

Signed-off-by Dmitry Osipenko <[email protected]>

---
 arch/arm/mach-tegra/Makefile           |  13 --
 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              |  18 ---
 arch/arm/mach-tegra/irq.h              |  11 --
 arch/arm/mach-tegra/pm.c               |   7 -
 arch/arm/mach-tegra/pm.h               |   1 -
 arch/arm/mach-tegra/reset-handler.S    |  11 --
 arch/arm/mach-tegra/reset.h            |   9 +-
 arch/arm/mach-tegra/sleep-tegra20.S    | 190 +---------------------
 arch/arm/mach-tegra/sleep.h            |  12 --
 arch/arm/mach-tegra/tegra.c            |   3 -
 drivers/soc/tegra/Kconfig              |   1 -

Quoted 8 lines

 include/soc/tegra/cpuidle.h            |   4 -
 17 files changed, 5 insertions(+), 779 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
 delete mode 100644 arch/arm/mach-tegra/irq.h

Quoted 4 lines

By removing all the above, it is really hard to review the diff. Is there any way you could first consolidate the cpuidle drivers into say the existing arch/arm/mach-tegra/cpuidle-tegra20.c and then move to drivers/cpuidle?

I'm afraid that it will make reviewing even more difficult because everything that is removed here is not returned in the further patches. The new driver is based on the older ones, but I wrote it from scratch and it's not only looks different, but also works a bit different as you may see.

Quoted 3 lines

Could you please clarify what exactly makes it hard to review? The diff looks pretty clean to me, while squashing everything into existing driver should be quite a mess.

Ideally a patch should standalone and can be reviewed by itself. However, to review this, we need to review patches 1, 2 and 3 at the same time. So IMO it is not that convenient from a reviewers perspective. Furthermore, patches 1 and 3 are large and so easy to miss something.

Is there really no way to have a patch to combined the existing drivers, then a patch to convert them into the newer rewritten version you have implemented, then move the driver?

11.07.2019 6:13, Dmitry Osipenko пишет:

Quoted 198 lines

The new driver is based on the old CPU Idle drivers that are removed now from arm/arch/mach-tegra/ directory. Those removed drivers were reworked and squashed into a single unified driver that covers multiple hardware generations, starting from Tegra20 and ending with Tegra124.

Signed-off-by Dmitry Osipenko <[email protected]>

---
 arch/arm/mach-tegra/tegra.c     |   4 +
 drivers/cpuidle/Kconfig.arm     |   8 +
 drivers/cpuidle/Makefile        |   1 +
 drivers/cpuidle/cpuidle-tegra.c | 304 ++++++++++++++++++++++++++++++++
 include/soc/tegra/cpuidle.h     |   4 +
 5 files changed, 321 insertions(+)
 create mode 100644 drivers/cpuidle/cpuidle-tegra.c

arch/arm/mach-tegra/tegra.c +4 -0

index d9237769a37c..f1ce2857a251 100644
@@ -36,6 +36,7 @@
 #include <asm/mach/arch.h>
 #include <asm/mach/time.h>
 #include <asm/mach-types.h>
+#include <asm/psci.h>
 #include <asm/setup.h>
 
 #include "board.h"

@@ -92,6 +93,9 @@ static void __init tegra_dt_init_late(void)
 	if (IS_ENABLED(CONFIG_ARCH_TEGRA_2x_SOC) &&
 	    of_machine_is_compatible("nvidia,tegra20"))
 		platform_device_register_simple("tegra20-cpufreq", -1, NULL, 0);
+
+	if (IS_ENABLED(CONFIG_ARM_TEGRA_CPUIDLE) && !psci_smp_available())
+		platform_device_register_simple("tegra-cpuidle", -1, NULL, 0);
 }
 
 static const char * const tegra_dt_board_compat[] = {

drivers/cpuidle/Kconfig.arm +8 -0

index 48cb3d4bb7d1..d90861361f1d 100644
@@ -76,3 +76,11 @@ config ARM_MVEBU_V7_CPUIDLE
 	depends on ARCH_MVEBU && !ARM64
 	help
 	  Select this to enable cpuidle on Armada 370, 38x and XP processors.
+
+config ARM_TEGRA_CPUIDLE
+	bool "CPU Idle Driver for NVIDIA Tegra SoCs"
+	depends on ARCH_TEGRA && !ARM64
+	select ARCH_NEEDS_CPU_IDLE_COUPLED if SMP
+	select ARM_CPU_SUSPEND
+	help
+	  Select this to enable cpuidle for NVIDIA Tegra20/30/114/124 SoCs.

drivers/cpuidle/Makefile +1 -0

index 9d7176cee3d3..470d17fa8746 100644
@@ -20,6 +20,7 @@ obj-$(CONFIG_ARM_U8500_CPUIDLE)         += cpuidle-ux500.o
 obj-$(CONFIG_ARM_AT91_CPUIDLE)          += cpuidle-at91.o
 obj-$(CONFIG_ARM_EXYNOS_CPUIDLE)        += cpuidle-exynos.o
 obj-$(CONFIG_ARM_CPUIDLE)		+= cpuidle-arm.o
+obj-$(CONFIG_ARM_TEGRA_CPUIDLE)		+= cpuidle-tegra.o
 
 ###############################################################################
 # MIPS drivers

drivers/cpuidle/cpuidle-tegra.c new file +127 -0

new file mode 100644
index 000000000000..54974cd2255f
@@ -0,0 +1,304 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * CPU idle driver for Tegra CPUs
+ *
+ * Copyright (c) 2010-2013, NVIDIA Corporation.
+ * Copyright (c) 2011 Google, Inc.
+ * Author: Colin Cross <[email protected]>
+ *         Gary King <[email protected]>
+ *
+ * Rework for 3.3 by Peter De Schrijver <[email protected]>
+ *
+ * Tegra20/124 driver unification by Dmitry Osipenko <[email protected]>
+ */
+
+#include <linux/cpuidle.h>
+#include <linux/cpumask.h>
+#include <linux/cpu_pm.h>
+#include <linux/errno.h>
+#include <linux/ktime.h>
+#include <linux/platform_device.h>
+#include <linux/types.h>
+
+#include <linux/clk/tegra.h>
+#include <linux/firmware/trusted_foundations.h>
+
+#include <soc/tegra/cpuidle.h>
+#include <soc/tegra/flowctrl.h>
+#include <soc/tegra/fuse.h>
+#include <soc/tegra/pm.h>
+
+#include <asm/cpuidle.h>
+#include <asm/firmware.h>
+#include <asm/smp_plat.h>
+#include <asm/suspend.h>
+
+#define TEGRA_C1		0
+#define TEGRA_C7		1
+#define TEGRA_CC6		2
+
+static atomic_t tegra_idle_barrier;
+
+static inline bool tegra_cpuidle_using_firmware(void)
+{
+	return firmware_ops->prepare_idle && firmware_ops->do_idle;
+}
+
+static int tegra_shutdown_secondary_cpu(unsigned long cpu)
+{
+	tegra_cpu_die(cpu);
+
+	return -EINVAL;
+}
+
+static int tegra_await_secondary_cpus_shutdown(void)
+{
+	ktime_t timeout = ktime_add_ms(ktime_get(), 1);
+
+	/*
+	 * The boot CPU shall await for the secondaries shutdown in order
+	 * to power-off CPU's cluster safely.
+	 */
+	do {
+		if (tegra_cpu_rail_off_ready())
+			return 0;
+
+	} while (ktime_compare(ktime_get(), timeout) < 0);
+
+	return -ETIMEDOUT;
+}
+
+static void tegra_wake_up_secondary_cpus(void)
+{
+	unsigned int cpu, lcpu;
+
+	for_each_cpu(lcpu, cpu_online_mask) {
+		cpu = cpu_logical_map(lcpu);
+
+		if (cpu > 0) {
+			tegra_enable_cpu_clock(cpu);
+			tegra_cpu_out_of_reset(cpu);
+			flowctrl_write_cpu_halt(cpu, 0);
+		}
+	}
+}
+
+static int tegra_cpuidle_cc6_enter(void)
+{
+	int err;
+
+	err = tegra_await_secondary_cpus_shutdown();
+	if (err)
+		return err;
+
+	err = tegra_idle_lp2_last();
+
+	tegra_wake_up_secondary_cpus();
+
+	return err;
+}
+
+static int tegra_cpuidle_c7_enter(void)
+{
+	int err;
+
+	if (tegra_cpuidle_using_firmware()) {
+		err = call_firmware_op(prepare_idle, TF_PM_MODE_LP2_NOFLUSH_L2);
+		if (err)
+			return err;
+
+		return call_firmware_op(do_idle, 0);
+	}
+
+	return cpu_suspend(0, tegra30_sleep_cpu_secondary_finish);
+}
+
+static int tegra_cpuidle_enter(struct cpuidle_device *dev,
+			       int index, unsigned int cpu)
+{
+	int err;
+
+	local_fiq_disable();
+	tegra_set_cpu_in_lp2();
+	cpu_pm_enter();
+
+	switch (index) {
+	case TEGRA_C7:
+		err = tegra_cpuidle_c7_enter();

drivers/cpuidle/cpuidle-tegra.c line 130

Quoted 3 lines

drivers/cpuidle/cpuidle-tegra.c new file +3 -0

+		break;
+	case TEGRA_CC6:
+		cpuidle_coupled_parallel_barrier(dev, &tegra_idle_barrier);

I realized that this is not very correct. We still need to do a proper barrier with SGI checking in order to bail out if other CPU sent IPI during of the awaiting for a coupled barrier to avoid the overhead of unnecessary power-gating. Will correct that in the next revision.

12.07.2019 12:39, Jon Hunter пишет:

Quoted 55 lines · Dmitry Osipenko

Quoted 43 lines

11.07.2019 12:26, Jon Hunter пишет:

Quoted 31 lines · Dmitry Osipenko

Quoted 20 lines

Remove the old drivers to replace them cleanly with a new one later on.

Signed-off-by Dmitry Osipenko <[email protected]>

---
 arch/arm/mach-tegra/Makefile           |  13 --
 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              |  18 ---
 arch/arm/mach-tegra/irq.h              |  11 --
 arch/arm/mach-tegra/pm.c               |   7 -
 arch/arm/mach-tegra/pm.h               |   1 -
 arch/arm/mach-tegra/reset-handler.S    |  11 --
 arch/arm/mach-tegra/reset.h            |   9 +-
 arch/arm/mach-tegra/sleep-tegra20.S    | 190 +---------------------
 arch/arm/mach-tegra/sleep.h            |  12 --
 arch/arm/mach-tegra/tegra.c            |   3 -
 drivers/soc/tegra/Kconfig              |   1 -

Quoted 8 lines

 include/soc/tegra/cpuidle.h            |   4 -
 17 files changed, 5 insertions(+), 779 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
 delete mode 100644 arch/arm/mach-tegra/irq.h

Quoted 4 lines

By removing all the above, it is really hard to review the diff. Is there any way you could first consolidate the cpuidle drivers into say the existing arch/arm/mach-tegra/cpuidle-tegra20.c and then move to drivers/cpuidle?

I'm afraid that it will make reviewing even more difficult because everything that is removed here is not returned in the further patches. The new driver is based on the older ones, but I wrote it from scratch and it's not only looks different, but also works a bit different as you may see.

Quoted 3 lines

Could you please clarify what exactly makes it hard to review? The diff looks pretty clean to me, while squashing everything into existing driver should be quite a mess.

Ideally a patch should standalone and can be reviewed by itself. However, to review this, we need to review patches 1, 2 and 3 at the same time. So IMO it is not that convenient from a reviewers perspective. Furthermore, patches 1 and 3 are large and so easy to miss something.

Quoted 3 lines

Is there really no way to have a patch to combined the existing drivers, then a patch to convert them into the newer rewritten version you have implemented, then move the driver?

Probably I spent a bit too much time with that code, so now yours suggestion looks to me like an unnecessary step. But I will try and see how it goes, at least it should be possible to break down the patch 1 a bit more, hopefully it will help to better understand what's going on in the further patches if you're not familiar or don't remember how it all works.