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 <[email protected]> > --- > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
