[PATCH 1/2] ARM: EXYNOS: Support Suspend-to-RAM on EXYNOS5420
Tomasz Figa
tomasz.figa at gmail.com
Fri Dec 20 16:37:31 EST 2013
Hi Abhilash,
Please see my comments inline.
On Monday 16 of December 2013 17:31:10 Abhilash Kesavan wrote:
> Add PMU configuration table for various low power modes - AFTR/LPA/SLEEP.
> Also, add core s2r support for Exynos5420.
>
> Signed-off-by: Abhilash Kesavan <a.kesavan at samsung.com>
> ---
> This patch depends on "ARM: EXYNOS5: Add PMU settings for exynos5420"
> http://www.spinics.net/lists/linux-samsung-soc/msg24902.html
>
> arch/arm/mach-exynos/include/mach/regs-clock.h | 15 +++
> arch/arm/mach-exynos/include/mach/regs-pmu.h | 84 ++++++++++++
> arch/arm/mach-exynos/pm.c | 151 +++++++++++++++++++---
> arch/arm/mach-exynos/pmu.c | 165 ++++++++++++++++++++++++
> 4 files changed, 396 insertions(+), 19 deletions(-)
>
> diff --git a/arch/arm/mach-exynos/include/mach/regs-clock.h b/arch/arm/mach-exynos/include/mach/regs-clock.h
> index d36ad76..20008f6 100644
> --- a/arch/arm/mach-exynos/include/mach/regs-clock.h
> +++ b/arch/arm/mach-exynos/include/mach/regs-clock.h
> @@ -363,6 +363,21 @@
> #define PWR_CTRL2_CORE2_UP_RATIO (1 << 4)
> #define PWR_CTRL2_CORE1_UP_RATIO (1 << 0)
>
> +/* For EXYNOS5420 */
> +#define EXYNOS5420_CLKSRC_MASK_CPERI EXYNOS_CLKREG(0x04300)
> +#define EXYNOS5420_CLKSRC_MASK_TOP0 EXYNOS_CLKREG(0x10300)
> +#define EXYNOS5420_CLKSRC_MASK_TOP1 EXYNOS_CLKREG(0x10304)
> +#define EXYNOS5420_CLKSRC_MASK_TOP2 EXYNOS_CLKREG(0x10308)
> +#define EXYNOS5420_CLKSRC_MASK_TOP7 EXYNOS_CLKREG(0x1031C)
> +#define EXYNOS5420_CLKSRC_MASK_DISP10 EXYNOS_CLKREG(0x1032C)
> +#define EXYNOS5420_CLKSRC_MASK_MAU EXYNOS_CLKREG(0x10334)
> +#define EXYNOS5420_CLKSRC_MASK_FSYS EXYNOS_CLKREG(0x10340)
> +#define EXYNOS5420_CLKSRC_MASK_PERIC0 EXYNOS_CLKREG(0x10350)
> +#define EXYNOS5420_CLKSRC_MASK_PERIC1 EXYNOS_CLKREG(0x10354)
> +#define EXYNOS5420_CLKSRC_MASK_ISP EXYNOS_CLKREG(0x10370)
> +#define EXYNOS5420_CLKGATE_BUS_DISP1 EXYNOS_CLKREG(0x10728)
> +#define EXYNOS5420_CLKGATE_IP_PERIC EXYNOS_CLKREG(0x10950)
This looks suspicious. Let me see below if this is really needed for what
I think it is.
> +
> /* Compatibility defines and inclusion */
>
> #include <mach/regs-pmu.h>
> diff --git a/arch/arm/mach-exynos/include/mach/regs-pmu.h b/arch/arm/mach-exynos/include/mach/regs-pmu.h
> index d5d5386..ad316f3 100644
> --- a/arch/arm/mach-exynos/include/mach/regs-pmu.h
> +++ b/arch/arm/mach-exynos/include/mach/regs-pmu.h
> @@ -229,6 +229,7 @@
>
> /* For EXYNOS5 */
>
> +#define EXYNOS5_SYS_DISP1_BLK_CFG S5P_SYSREG(0x0214)
Is it really a definition common for all Exynos5 SoCs?
> #define EXYNOS5_SYS_I2C_CFG S5P_SYSREG(0x0234)
>
> #define EXYNOS5_AUTO_WDTRESET_DISABLE S5P_PMUREG(0x0408)
> @@ -360,6 +361,7 @@
> #define EXYNOS5_USE_SC_COUNTER (1 << 0)
>
> #define EXYNOS5_MANUAL_L2RSTDISABLE_CONTROL (1 << 2)
> +#define EXYNOS5_L2RSTDISABLE_VALUE (1 << 3)
Ditto.
> #define EXYNOS5_SKIP_DEACTIVATE_ACEACP_IN_PWDN (1 << 7)
>
> #define EXYNOS5_OPTION_USE_STANDBYWFE (1 << 24)
[snip]
> diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c
> index 7fb0f13..3ad103f 100644
> --- a/arch/arm/mach-exynos/pm.c
> +++ b/arch/arm/mach-exynos/pm.c
> @@ -52,6 +52,22 @@ static const struct sleep_save exynos4210_set_clksrc[] = {
> { .reg = EXYNOS4210_CLKSRC_MASK_LCD1 , .val = 0x00001111, },
> };
>
> +static struct sleep_save exynos5420_set_clksrc[] = {
> + { .reg = EXYNOS5420_CLKSRC_MASK_CPERI, .val = 0xffffffff, },
> + { .reg = EXYNOS5420_CLKSRC_MASK_TOP0, .val = 0x11111111, },
> + { .reg = EXYNOS5420_CLKSRC_MASK_TOP1, .val = 0x11101111, },
> + { .reg = EXYNOS5420_CLKSRC_MASK_TOP2, .val = 0x11111110, },
> + { .reg = EXYNOS5420_CLKSRC_MASK_TOP7, .val = 0x00111100, },
> + { .reg = EXYNOS5420_CLKSRC_MASK_DISP10, .val = 0x11111110, },
> + { .reg = EXYNOS5420_CLKSRC_MASK_MAU, .val = 0x10000000, },
> + { .reg = EXYNOS5420_CLKSRC_MASK_FSYS, .val = 0x11111110, },
> + { .reg = EXYNOS5420_CLKSRC_MASK_PERIC0, .val = 0x11111110, },
> + { .reg = EXYNOS5420_CLKSRC_MASK_PERIC1, .val = 0x11111100, },
> + { .reg = EXYNOS5420_CLKSRC_MASK_ISP, .val = 0x11111000, },
> + { .reg = EXYNOS5420_CLKGATE_BUS_DISP1, .val = 0xffffffff, },
> + { .reg = EXYNOS5420_CLKGATE_IP_PERIC, .val = 0xffffffff, },
> +};
> +
OK, just as I thought...
NAK. Clock registers should be accessed only inside clock driver. Please
see my patch implementing similar thing for Exynos 4:
http://thread.gmane.org/gmane.linux.kernel.samsung-soc/24078/focus=24087
> static struct sleep_save exynos4_epll_save[] = {
> SAVE_ITEM(EXYNOS4_EPLL_CON0),
> SAVE_ITEM(EXYNOS4_EPLL_CON1),
> @@ -66,6 +82,14 @@ static struct sleep_save exynos5_sys_save[] = {
> SAVE_ITEM(EXYNOS5_SYS_I2C_CFG),
> };
>
> +static struct sleep_save exynos5420_sys_save[] = {
> + SAVE_ITEM(EXYNOS5_SYS_DISP1_BLK_CFG),
> +};
> +
> +static struct sleep_save exynos5420_cpustate_save[] = {
> + SAVE_ITEM(S5P_VA_SYSRAM + 0x28),
> +};
> +
> static struct sleep_save exynos_core_save[] = {
> /* SROM side */
> SAVE_ITEM(S5P_SROM_BW),
> @@ -84,8 +108,15 @@ static int exynos_cpu_suspend(unsigned long arg)
> #ifdef CONFIG_CACHE_L2X0
> outer_flush_all();
> #endif
> + /*
> + * Clear the IRAM register that holds a low power flag. The presence
> + * of this flag decides if the primary cpu starts executing the low
> + * power function at wake-up or not.
> + */
> + if (soc_is_exynos5420())
> + __raw_writel(0x0, S5P_VA_SYSRAM + 0x28);
>
> - if (soc_is_exynos5250())
> + if (soc_is_exynos5250() || soc_is_exynos5420())
> flush_cache_all();
>
> /* issue the standby signal into the pm unit. */
> @@ -101,9 +132,14 @@ static void exynos_pm_prepare(void)
>
> s3c_pm_do_save(exynos_core_save, ARRAY_SIZE(exynos_core_save));
>
> - if (!soc_is_exynos5250()) {
> + if (!(soc_is_exynos5250() || soc_is_exynos5420())) {
> s3c_pm_do_save(exynos4_epll_save, ARRAY_SIZE(exynos4_epll_save));
> s3c_pm_do_save(exynos4_vpll_save, ARRAY_SIZE(exynos4_vpll_save));
> + } else if (soc_is_exynos5420()) {
> + s3c_pm_do_save(exynos5420_sys_save,
> + ARRAY_SIZE(exynos5420_sys_save));
> + s3c_pm_do_save(exynos5420_cpustate_save,
> + ARRAY_SIZE(exynos5420_cpustate_save));
> } else {
> s3c_pm_do_save(exynos5_sys_save, ARRAY_SIZE(exynos5_sys_save));
> /* Disable USE_RETENTION of JPEG_MEM_OPTION */
What about rewriting this code as follows:
if (soc_is_exynos5420()) {
// exynos5420 specific code
} else if (soc_is_exynos5250()) {
// exynos5250 specific code
} else {
// exynos4 specific code
}
Still, I'd prefer this to be based on top of my series I mentioned above
which removes any clock handling from this file and so makes the exynos4
branch above disappear.
> @@ -123,12 +159,32 @@ static void exynos_pm_prepare(void)
>
> /* Before enter central sequence mode, clock src register have to set */
>
> - if (!soc_is_exynos5250())
> + if (!(soc_is_exynos5250() || soc_is_exynos5420()))
> s3c_pm_do_restore_core(exynos4_set_clksrc, ARRAY_SIZE(exynos4_set_clksrc));
>
> if (soc_is_exynos4210())
> s3c_pm_do_restore_core(exynos4210_set_clksrc, ARRAY_SIZE(exynos4210_set_clksrc));
>
> + if (soc_is_exynos5420()) {
> + s3c_pm_do_restore_core(exynos5420_set_clksrc,
> + ARRAY_SIZE(exynos5420_set_clksrc));
> +
> + tmp = __raw_readl(EXYNOS5420_SFR_AXI_CGDIS1);
> + tmp |= EXYNOS5420_UFS;
> + __raw_writel(tmp, EXYNOS5420_SFR_AXI_CGDIS1);
> +
> + tmp = __raw_readl(EXYNOS5420_ARM_COMMON_OPTION);
> + tmp &= ~EXYNOS5_L2RSTDISABLE_VALUE;
> + __raw_writel(tmp, EXYNOS5420_ARM_COMMON_OPTION);
> +
> + tmp = __raw_readl(EXYNOS5420_FSYS2_OPTION);
> + tmp |= EXYNOS5420_EMULATION;
> + __raw_writel(tmp, EXYNOS5420_FSYS2_OPTION);
> +
> + tmp = __raw_readl(EXYNOS5420_PSGEN_OPTION);
> + tmp |= EXYNOS5420_EMULATION;
> + __raw_writel(tmp, EXYNOS5420_PSGEN_OPTION);
> + }
> }
>
> static int exynos_pm_add(struct device *dev, struct subsys_interface *sif)
> @@ -224,11 +280,17 @@ static __init int exynos_pm_drvinit(void)
>
> /* All wakeup disable */
>
> - tmp = __raw_readl(S5P_WAKEUP_MASK);
> - tmp |= ((0xFF << 8) | (0x1F << 1));
> - __raw_writel(tmp, S5P_WAKEUP_MASK);
> + if (soc_is_exynos5420()) {
> + tmp = __raw_readl(S5P_WAKEUP_MASK);
> + tmp |= ((0x7F << 7) | (0x1F << 1));
> + __raw_writel(tmp, S5P_WAKEUP_MASK);
> + } else {
> + tmp = __raw_readl(S5P_WAKEUP_MASK);
> + tmp |= ((0xFF << 8) | (0x1F << 1));
> + __raw_writel(tmp, S5P_WAKEUP_MASK);
> + }
>
> - if (!soc_is_exynos5250()) {
> + if (!(soc_is_exynos5250() || soc_is_exynos5420())) {
> pll_base = clk_get(NULL, "xtal");
This code is also removed by my series.
>
> if (!IS_ERR(pll_base)) {
> @@ -244,6 +306,7 @@ arch_initcall(exynos_pm_drvinit);
> static int exynos_pm_suspend(void)
> {
> unsigned long tmp;
> + unsigned int cluster_id;
>
> /* Setting Central Sequence Register for power down mode */
>
> @@ -253,10 +316,20 @@ static int exynos_pm_suspend(void)
>
> /* Setting SEQ_OPTION register */
>
> - tmp = (S5P_USE_STANDBY_WFI0 | S5P_USE_STANDBY_WFE0);
> - __raw_writel(tmp, S5P_CENTRAL_SEQ_OPTION);
> + if (soc_is_exynos5420()) {
> + cluster_id = (read_cpuid(CPUID_MPIDR) >> 8) & 0xf;
> + if (!cluster_id)
> + __raw_writel(EXYNOS5420_ARM_USE_STANDBY_WFI0,
> + S5P_CENTRAL_SEQ_OPTION);
> + else
> + __raw_writel(EXYNOS5420_KFC_USE_STANDBY_WFI0,
> + S5P_CENTRAL_SEQ_OPTION);
> + } else if (soc_is_exynos5250()) {
This code should be also called on other Exynos SoCs, not just 5250.
> + tmp = (S5P_USE_STANDBY_WFI0 | S5P_USE_STANDBY_WFE0);
> + __raw_writel(tmp, S5P_CENTRAL_SEQ_OPTION);
> + }
>
> - if (!soc_is_exynos5250()) {
> + if (!(soc_is_exynos5250() || soc_is_exynos5420())) {
> /* Save Power control register */
> asm ("mrc p15, 0, %0, c15, c0, 0"
> : "=r" (tmp) : : "cc");
> @@ -275,6 +348,15 @@ static void exynos_pm_resume(void)
> {
> unsigned long tmp;
>
> + if (soc_is_exynos5420()) {
> + /* Restore the low power flag */
> + s3c_pm_do_restore(exynos5420_cpustate_save,
> + ARRAY_SIZE(exynos5420_cpustate_save));
> +
> + __raw_writel(EXYNOS5420_USE_STANDBY_WFI_ALL,
> + S5P_CENTRAL_SEQ_OPTION);
> + }
> +
> /*
> * If PMU failed while entering sleep mode, WFI will be
> * ignored by PMU and then exiting cpu_do_idle().
> @@ -290,7 +372,7 @@ static void exynos_pm_resume(void)
> /* No need to perform below restore code */
> goto early_wakeup;
> }
> - if (!soc_is_exynos5250()) {
> + if (!(soc_is_exynos5250() || soc_is_exynos5420())) {
> /* Restore Power control register */
> tmp = save_arm_register[0];
> asm volatile ("mcr p15, 0, %0, c15, c0, 0"
> @@ -306,21 +388,40 @@ static void exynos_pm_resume(void)
>
> /* For release retention */
>
> - __raw_writel((1 << 28), S5P_PAD_RET_MAUDIO_OPTION);
> - __raw_writel((1 << 28), S5P_PAD_RET_GPIO_OPTION);
> - __raw_writel((1 << 28), S5P_PAD_RET_UART_OPTION);
> - __raw_writel((1 << 28), S5P_PAD_RET_MMCA_OPTION);
> - __raw_writel((1 << 28), S5P_PAD_RET_MMCB_OPTION);
> - __raw_writel((1 << 28), S5P_PAD_RET_EBIA_OPTION);
> - __raw_writel((1 << 28), S5P_PAD_RET_EBIB_OPTION);
> + if (soc_is_exynos5420()) {
Indentation issue.
> + __raw_writel((1 << 28), EXYNOS5420_PAD_RET_DRAM_OPTION);
> + __raw_writel((1 << 28), EXYNOS5420_PAD_RET_MAUDIO_OPTION);
> + __raw_writel((1 << 28), EXYNOS5420_PAD_RET_JTAG_OPTION);
> + __raw_writel((1 << 28), EXYNOS5420_PAD_RET_GPIO_OPTION);
> + __raw_writel((1 << 28), EXYNOS5420_PAD_RET_UART_OPTION);
> + __raw_writel((1 << 28), EXYNOS5420_PAD_RET_MMCA_OPTION);
> + __raw_writel((1 << 28), EXYNOS5420_PAD_RET_MMCB_OPTION);
> + __raw_writel((1 << 28), EXYNOS5420_PAD_RET_MMCC_OPTION);
> + __raw_writel((1 << 28), EXYNOS5420_PAD_RET_HSI_OPTION);
> + __raw_writel((1 << 28), EXYNOS5420_PAD_RET_EBIA_OPTION);
> + __raw_writel((1 << 28), EXYNOS5420_PAD_RET_EBIB_OPTION);
> + __raw_writel((1 << 28), EXYNOS5420_PAD_RET_SPI_OPTION);
> + __raw_writel((1 << 28), EXYNOS5420_PAD_RET_DRAM_COREBLK_OPTION);
> + } else {
> + __raw_writel((1 << 28), S5P_PAD_RET_MAUDIO_OPTION);
> + __raw_writel((1 << 28), S5P_PAD_RET_GPIO_OPTION);
> + __raw_writel((1 << 28), S5P_PAD_RET_UART_OPTION);
> + __raw_writel((1 << 28), S5P_PAD_RET_MMCA_OPTION);
> + __raw_writel((1 << 28), S5P_PAD_RET_MMCB_OPTION);
> + __raw_writel((1 << 28), S5P_PAD_RET_EBIA_OPTION);
> + __raw_writel((1 << 28), S5P_PAD_RET_EBIB_OPTION);
> + }
>
> if (soc_is_exynos5250())
> s3c_pm_do_restore(exynos5_sys_save,
> ARRAY_SIZE(exynos5_sys_save));
> + else if (soc_is_exynos5420())
> + s3c_pm_do_restore(exynos5420_sys_save,
> + ARRAY_SIZE(exynos5420_sys_save));
>
> s3c_pm_do_restore_core(exynos_core_save, ARRAY_SIZE(exynos_core_save));
>
> - if (!soc_is_exynos5250()) {
> + if (!(soc_is_exynos5250() || soc_is_exynos5420())) {
> exynos4_restore_pll();
>
> #ifdef CONFIG_SMP
> @@ -330,6 +431,18 @@ static void exynos_pm_resume(void)
>
> early_wakeup:
>
> + if (soc_is_exynos5420()) {
> + tmp = __raw_readl(EXYNOS5420_SFR_AXI_CGDIS1);
> + tmp &= ~EXYNOS5420_UFS;
> + __raw_writel(tmp, EXYNOS5420_SFR_AXI_CGDIS1);
> + tmp = __raw_readl(EXYNOS5420_FSYS2_OPTION);
> + tmp &= ~EXYNOS5420_EMULATION;
> + __raw_writel(tmp, EXYNOS5420_FSYS2_OPTION);
> + tmp = __raw_readl(EXYNOS5420_PSGEN_OPTION);
> + tmp &= ~EXYNOS5420_EMULATION;
> + __raw_writel(tmp, EXYNOS5420_PSGEN_OPTION);
> + }
> +
> /* Clear SLEEP mode set in INFORM1 */
> __raw_writel(0x0, S5P_INFORM1);
>
> diff --git a/arch/arm/mach-exynos/pmu.c b/arch/arm/mach-exynos/pmu.c
> index e39cc75..1449404 100644
> --- a/arch/arm/mach-exynos/pmu.c
> +++ b/arch/arm/mach-exynos/pmu.c
> @@ -16,6 +16,8 @@
> #include <mach/regs-clock.h>
> #include <mach/regs-pmu.h>
>
> +#include <asm/cputype.h>
> +
> #include "common.h"
>
> static const struct exynos_pmu_conf *exynos_pmu_config;
> @@ -318,6 +320,151 @@ static const struct exynos_pmu_conf exynos5250_pmu_config[] = {
> { PMU_TABLE_END,},
> };
>
> +static struct exynos_pmu_conf exynos5420_pmu_config[] = {
static const?
Best regards,
Tomasz
More information about the linux-arm-kernel
mailing list