[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <514c1cd6-2180-b55f-dacc-f0e08d9a366f@gmail.com>
Date: Thu, 11 Jul 2019 20:25:25 +0300
From: Dmitry Osipenko <digetx@...il.com>
To: Jon Hunter <jonathanh@...dia.com>,
Thierry Reding <thierry.reding@...il.com>,
Peter De Schrijver <pdeschrijver@...dia.com>,
"Rafael J. Wysocki" <rjw@...ysocki.net>,
Daniel Lezcano <daniel.lezcano@...aro.org>
Cc: linux-pm@...r.kernel.org, linux-tegra@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/6] ARM: tegra: Expose functions required for cpuidle
driver
11.07.2019 15:42, Jon Hunter пишет:
>
> On 11/07/2019 04:13, Dmitry Osipenko wrote:
>> 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 <digetx@...il.com>
>> ---
>> 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(-)
>>
>> diff --git a/arch/arm/mach-tegra/Makefile b/arch/arm/mach-tegra/Makefile
>> index 5d93a0b36866..27bd5d9865e3 100644
>> --- a/arch/arm/mach-tegra/Makefile
>> +++ b/arch/arm/mach-tegra/Makefile
>> @@ -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
>> diff --git a/arch/arm/mach-tegra/platsmp.c b/arch/arm/mach-tegra/platsmp.c
>> index e6911a14c096..c8a63719a143 100644
>> --- a/arch/arm/mach-tegra/platsmp.c
>> +++ b/arch/arm/mach-tegra/platsmp.c
>> @@ -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
>> };
>> diff --git a/arch/arm/mach-tegra/pm.c b/arch/arm/mach-tegra/pm.c
>> index 6aaacb5757e1..f9c9bce9e15d 100644
>> --- a/arch/arm/mach-tegra/pm.c
>> +++ b/arch/arm/mach-tegra/pm.c
>> @@ -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);
>>
>> - 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.
Indeed, it could be not very obvious what's going on here without a
thorough review. I'll factor out all these minor changes into separate
commits.
In particular there is no need to know whether CPU is the "last_cpu" for
the new driver because CPU0 is always the "last" since it awaits for the
secondaries in the coupled state.
>> 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(
>> diff --git a/arch/arm/mach-tegra/pm.h b/arch/arm/mach-tegra/pm.h
>> index 1e51a9b636eb..81525f5f4a44 100644
>> --- a/arch/arm/mach-tegra/pm.h
>> +++ b/arch/arm/mach-tegra/pm.h
>> @@ -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
>> diff --git a/arch/arm/mach-tegra/sleep.h b/arch/arm/mach-tegra/sleep.h
>> index d219872b7546..0d9956e9a8ea 100644
>> --- a/arch/arm/mach-tegra/sleep.h
>> +++ b/arch/arm/mach-tegra/sleep.h
>> @@ -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
>> diff --git a/include/linux/clk/tegra.h b/include/linux/clk/tegra.h
>> index b8aef62cc3f5..cf0f2cb5e109 100644
>> --- a/include/linux/clk/tegra.h
>> +++ b/include/linux/clk/tegra.h
>> @@ -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);
>> diff --git a/include/soc/tegra/pm.h b/include/soc/tegra/pm.h
>> index 951fcd738d55..fa18c2df5028 100644
>> --- a/include/soc/tegra/pm.h
>> +++ b/include/soc/tegra/pm.h
>> @@ -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)
>> {
>> }
>> +
>> +static inline int tegra30_sleep_cpu_secondary_finish(unsigned long arg)
>> +{
>> + return -1;
>> +}
>
> -ENOTSUPP?
Good point, thanks!
Powered by blists - more mailing lists