Re: [PATCH v7 2/5] ARM: add AFTR mode support to firmware do_idle method

[Thierry Reding]

On Thu, Sep 25, 2014 at 05:23:32PM +0900, Kukjin Kim wrote:
> On 09/24/14 21:24, Bartlomiej Zolnierkiewicz wrote:
> >On some platforms (i.e. EXYNOS ones) more than one idle mode is
> >available and we need to distinguish them in firmware do_idle method.
> >
> >Add mode parameter to do_idle firmware method and AFTR mode support
> >to EXYNOS do_idle implementation.
> >
> >This change is a preparation for adding secure firmware support to
> >EXYNOS cpuidle driver.
> >
> >This patch shouldn't cause any functionality changes.
> >
> >Signed-off-by: Bartlomiej Zolnierkiewicz<b.zolnierkie@xxxxxxxxxxx>
> >Acked-by: Kyungmin Park<kyungmin.park@xxxxxxxxxxx>
> >---
> >v7:
> >- fixed build on Tegra
> >- updated summary line
> >
> >  arch/arm/include/asm/firmware.h        |  2 +-
> >  arch/arm/mach-exynos/common.h          |  5 +++++
> >  arch/arm/mach-exynos/firmware.c        | 10 ++++++++--
> >  arch/arm/mach-tegra/cpuidle-tegra114.c |  2 +-
> >  4 files changed, 15 insertions(+), 4 deletions(-)
> >
> >diff --git a/arch/arm/include/asm/firmware.h b/arch/arm/include/asm/firmware.h
> >index 5904f59..89aefe1 100644
> >--- a/arch/arm/include/asm/firmware.h
> >+++ b/arch/arm/include/asm/firmware.h
> >@@ -28,7 +28,7 @@ struct firmware_ops {
> >  	/*
> >  	 * Enters CPU idle mode
> >  	 */
> >-	int (*do_idle)(void);
> >+	int (*do_idle)(unsigned long mode);
> >  	/*
> >  	 * Sets boot address of specified physical CPU
> >  	 */
> >diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
> >index c218200..2d830df 100644
> >--- a/arch/arm/mach-exynos/common.h
> >+++ b/arch/arm/mach-exynos/common.h
> >@@ -119,6 +119,11 @@ extern void __iomem *sysram_base_addr;
> >  extern void __iomem *pmu_base_addr;
> >  void exynos_sysram_init(void);
> >
> >+enum {
> >+	FW_DO_IDLE_SLEEP,
> >+	FW_DO_IDLE_AFTR,
> >+};
> >+
> >  void exynos_firmware_init(void);
> >
> >  extern u32 exynos_get_eint_wake_mask(void);
> >diff --git a/arch/arm/mach-exynos/firmware.c b/arch/arm/mach-exynos/firmware.c
> >index f5e626d..e57b7c3 100644
> >--- a/arch/arm/mach-exynos/firmware.c
> >+++ b/arch/arm/mach-exynos/firmware.c
> >@@ -28,9 +28,15 @@
> >  #define EXYNOS_BOOT_ADDR	0x8
> >  #define EXYNOS_BOOT_FLAG	0xc
> >
> >-static int exynos_do_idle(void)
> >+static int exynos_do_idle(unsigned long mode)
> >  {
> >-	exynos_smc(SMC_CMD_SLEEP, 0, 0, 0);
> >+	switch (mode) {
> >+	case FW_DO_IDLE_AFTR:
> >+		exynos_smc(SMC_CMD_CPU0AFTR, 0, 0, 0);
> >+		break;
> >+	case FW_DO_IDLE_SLEEP:
> >+		exynos_smc(SMC_CMD_SLEEP, 0, 0, 0);
> >+	}
> >  	return 0;
> >  }
> >
> >diff --git a/arch/arm/mach-tegra/cpuidle-tegra114.c b/arch/arm/mach-tegra/cpuidle-tegra114.c
> >index e3ebdce..425b6c8 100644
> >--- a/arch/arm/mach-tegra/cpuidle-tegra114.c
> >+++ b/arch/arm/mach-tegra/cpuidle-tegra114.c
> >@@ -49,7 +49,7 @@ static int tegra114_idle_power_down(struct cpuidle_device *dev,
> >  	call_firmware_op(prepare_idle);
> >
> >  	/* Do suspend by ourselves if the firmware does not implement it */
> >-	if (call_firmware_op(do_idle) == -ENOSYS)
> >+	if (call_firmware_op(do_idle, 0) == -ENOSYS)
> >  		cpu_suspend(0, tegra30_sleep_cpu_secondary_finish);
> >
> >  	clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT,&dev->cpu);
> 
> + Thierry(using nvidia.com e-mail address)
> 
> Hi Thierry,
> 
> For supporting AFTR mode in call_firmware_op(do_idle), need to modify tegra
> stuff as you can see. The functionality will be same with before. Is it OK
> to you? If you have no objection, I'd like to take this series into samsung
> tree.

I'm only aware of any devices using TrustedFirmware on Tegra, which is
what the above code handles. The implementation for TrustedFirmware does
not implement do_idle() (yet?) so I don't think this patch would cause
any damage.

Acked-by: Thierry Reding <treding@xxxxxxxxxx>

Attachment: pgpqrD1D3tSXY.pgp
Description: PGP signature