Message ID | 1412089142-12056-1-git-send-email-a.kesavan@samsung.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Hi Javier, [...] > diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c > index adb36a8..222aa3c 100644 > --- a/arch/arm/mach-exynos/platsmp.c > +++ b/arch/arm/mach-exynos/platsmp.c > @@ -137,6 +137,18 @@ void exynos_cpu_power_down(int cpu) > */ > void exynos_cpu_power_up(int cpu) > { > + if (cpu == 0 && (of_machine_is_compatible("samsung,exynos5420") || > + of_machine_is_compatible("samsung,exynos5800"))) { > + /* > + * Bypass power down for CPU0 during suspend. Check for > + * the SYS_PWR_REG value to decide if we are suspending > + * the system. > + */ > + int val = __raw_readl(pmu_base_addr + > + EXYNOS5_ARM_CORE0_SYS_PWR_REG); > + if (!(val & S5P_CORE_LOCAL_PWR_EN)) > + return; > + } > pmu_raw_writel(S5P_CORE_LOCAL_PWR_EN, > EXYNOS_ARM_CORE_CONFIGURATION(cpu)); > } The above block of code should be in exynos_cpu_power_down(). Can you re-test s2r after modifying it ? I have to convert a __raw_readl to pmu_raw_readl in this patch as well. Once Vikas re-works the base s2r patch I will re-post a v9. Regards, Abhilash
Hello Abhilash, On Mon, Oct 6, 2014 at 5:40 AM, Abhilash Kesavan <kesavan.abhilash@gmail.com> wrote: > Hi Javier, > > [...] > >> diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c >> index adb36a8..222aa3c 100644 >> --- a/arch/arm/mach-exynos/platsmp.c >> +++ b/arch/arm/mach-exynos/platsmp.c >> @@ -137,6 +137,18 @@ void exynos_cpu_power_down(int cpu) >> */ >> void exynos_cpu_power_up(int cpu) >> { >> + if (cpu == 0 && (of_machine_is_compatible("samsung,exynos5420") || >> + of_machine_is_compatible("samsung,exynos5800"))) { >> + /* >> + * Bypass power down for CPU0 during suspend. Check for >> + * the SYS_PWR_REG value to decide if we are suspending >> + * the system. >> + */ >> + int val = __raw_readl(pmu_base_addr + >> + EXYNOS5_ARM_CORE0_SYS_PWR_REG); >> + if (!(val & S5P_CORE_LOCAL_PWR_EN)) >> + return; >> + } >> pmu_raw_writel(S5P_CORE_LOCAL_PWR_EN, >> EXYNOS_ARM_CORE_CONFIGURATION(cpu)); >> } > > The above block of code should be in exynos_cpu_power_down(). Can you > re-test s2r after modifying it ? I have to convert a __raw_readl to > pmu_raw_readl in this patch as well. Once Vikas re-works the base s2r > patch I will re-post a v9. > Thanks, for the information but unfortunately even after this change I still have the issue reported in [0]. I also removed the lines from exynos5420_pm_resume() that you asked Vikash [1]. Which __raw_readl() you had to convert? That is only for consistency right? Since I see that the __raw_readl() calls are using pmu_base_addr + offset so it seems to be correct. I'll test again once you both re-spin your patches then. > Regards, > Abhilash Best regards, Javier [0]: http://www.spinics.net/lists/arm-kernel/msg367615.html [1]: http://www.spinics.net/lists/linux-samsung-soc/msg37561.html
Hi Javier, On Mon, Oct 6, 2014 at 1:45 PM, Javier Martinez Canillas <javier@dowhile0.org> wrote: > Hello Abhilash, > > On Mon, Oct 6, 2014 at 5:40 AM, Abhilash Kesavan > <kesavan.abhilash@gmail.com> wrote: >> Hi Javier, >> >> [...] >> >>> diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c >>> index adb36a8..222aa3c 100644 >>> --- a/arch/arm/mach-exynos/platsmp.c >>> +++ b/arch/arm/mach-exynos/platsmp.c >>> @@ -137,6 +137,18 @@ void exynos_cpu_power_down(int cpu) >>> */ >>> void exynos_cpu_power_up(int cpu) >>> { >>> + if (cpu == 0 && (of_machine_is_compatible("samsung,exynos5420") || >>> + of_machine_is_compatible("samsung,exynos5800"))) { >>> + /* >>> + * Bypass power down for CPU0 during suspend. Check for >>> + * the SYS_PWR_REG value to decide if we are suspending >>> + * the system. >>> + */ >>> + int val = __raw_readl(pmu_base_addr + >>> + EXYNOS5_ARM_CORE0_SYS_PWR_REG); >>> + if (!(val & S5P_CORE_LOCAL_PWR_EN)) >>> + return; >>> + } >>> pmu_raw_writel(S5P_CORE_LOCAL_PWR_EN, >>> EXYNOS_ARM_CORE_CONFIGURATION(cpu)); >>> } >> >> The above block of code should be in exynos_cpu_power_down(). Can you >> re-test s2r after modifying it ? I have to convert a __raw_readl to >> pmu_raw_readl in this patch as well. Once Vikas re-works the base s2r >> patch I will re-post a v9. >> > > Thanks, for the information but unfortunately even after this change I > still have the issue reported in [0]. I also removed the lines from > exynos5420_pm_resume() that you asked Vikash [1]. I have just tested this on the 5420 that I have and it resumes fine. The difference could be in the bootloader we are using. I'll send out details of my current setup so that you can cross-check if everything is the same. > > Which __raw_readl() you had to convert? That is only for consistency > right? Since I see that the __raw_readl() calls are using > pmu_base_addr + offset so it seems to be correct. Yes, it is only for consistency. It should not cause any issues if you leave it as it is. > > I'll test again once you both re-spin your patches then. OK. Regards, Abhilash > >> Regards, >> Abhilash > > Best regards, > Javier > > [0]: http://www.spinics.net/lists/arm-kernel/msg367615.html > [1]: http://www.spinics.net/lists/linux-samsung-soc/msg37561.html
Hello Abhilash, On Mon, Oct 6, 2014 at 11:08 AM, Abhilash Kesavan <kesavan.abhilash@gmail.com> wrote: >>>> diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c >>>> index adb36a8..222aa3c 100644 >>>> --- a/arch/arm/mach-exynos/platsmp.c >>>> +++ b/arch/arm/mach-exynos/platsmp.c >>>> @@ -137,6 +137,18 @@ void exynos_cpu_power_down(int cpu) >>>> */ >>>> void exynos_cpu_power_up(int cpu) >>>> { >>>> + if (cpu == 0 && (of_machine_is_compatible("samsung,exynos5420") || >>>> + of_machine_is_compatible("samsung,exynos5800"))) { >>>> + /* >>>> + * Bypass power down for CPU0 during suspend. Check for >>>> + * the SYS_PWR_REG value to decide if we are suspending >>>> + * the system. >>>> + */ >>>> + int val = __raw_readl(pmu_base_addr + >>>> + EXYNOS5_ARM_CORE0_SYS_PWR_REG); >>>> + if (!(val & S5P_CORE_LOCAL_PWR_EN)) >>>> + return; >>>> + } >>>> pmu_raw_writel(S5P_CORE_LOCAL_PWR_EN, >>>> EXYNOS_ARM_CORE_CONFIGURATION(cpu)); >>>> } >>> >>> The above block of code should be in exynos_cpu_power_down(). Can you >>> re-test s2r after modifying it ? I have to convert a __raw_readl to >>> pmu_raw_readl in this patch as well. Once Vikas re-works the base s2r >>> patch I will re-post a v9. >>> >> >> Thanks, for the information but unfortunately even after this change I >> still have the issue reported in [0]. I also removed the lines from >> exynos5420_pm_resume() that you asked Vikash [1]. > > I have just tested this on the 5420 that I have and it resumes fine. > The difference could be in the bootloader we are using. I'll send out > details of my current setup so that you can cross-check if everything > is the same. Thanks a lot for sending me the patches you are using to test. But after looking at the differences I noticed that your patches are not removing the lines that set the CPU0 low power states at the start of exynos5420_pm_resume() as you asked Vikas [1] but instead your version of this patch does not have the code that does the same operation after the early_wakeup label like is made in $subject. IOW, this [2] is what I had to change on top of $subject to have s2r working. After that change, both suspending to ram and resuming due the RTC IRQ alarm being triggered and the core pm_test case works well on my Exynos5420 Peach Pit. > > Regards, > Abhilash Best regards, Javier [0]: http://www.spinics.net/lists/arm-kernel/msg367615.html [1]: http://www.spinics.net/lists/linux-samsung-soc/msg37561.html [2]: http://pastebin.com/raw.php?i=XSxJH3Ys
Hi Javier, On Mon, Oct 6, 2014 at 5:29 PM, Javier Martinez Canillas <javier@dowhile0.org> wrote: > Hello Abhilash, > > On Mon, Oct 6, 2014 at 11:08 AM, Abhilash Kesavan > <kesavan.abhilash@gmail.com> wrote: >>>>> diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c >>>>> index adb36a8..222aa3c 100644 >>>>> --- a/arch/arm/mach-exynos/platsmp.c >>>>> +++ b/arch/arm/mach-exynos/platsmp.c >>>>> @@ -137,6 +137,18 @@ void exynos_cpu_power_down(int cpu) >>>>> */ >>>>> void exynos_cpu_power_up(int cpu) >>>>> { >>>>> + if (cpu == 0 && (of_machine_is_compatible("samsung,exynos5420") || >>>>> + of_machine_is_compatible("samsung,exynos5800"))) { >>>>> + /* >>>>> + * Bypass power down for CPU0 during suspend. Check for >>>>> + * the SYS_PWR_REG value to decide if we are suspending >>>>> + * the system. >>>>> + */ >>>>> + int val = __raw_readl(pmu_base_addr + >>>>> + EXYNOS5_ARM_CORE0_SYS_PWR_REG); >>>>> + if (!(val & S5P_CORE_LOCAL_PWR_EN)) >>>>> + return; >>>>> + } >>>>> pmu_raw_writel(S5P_CORE_LOCAL_PWR_EN, >>>>> EXYNOS_ARM_CORE_CONFIGURATION(cpu)); >>>>> } >>>> >>>> The above block of code should be in exynos_cpu_power_down(). Can you >>>> re-test s2r after modifying it ? I have to convert a __raw_readl to >>>> pmu_raw_readl in this patch as well. Once Vikas re-works the base s2r >>>> patch I will re-post a v9. >>>> >>> >>> Thanks, for the information but unfortunately even after this change I >>> still have the issue reported in [0]. I also removed the lines from >>> exynos5420_pm_resume() that you asked Vikash [1]. >> >> I have just tested this on the 5420 that I have and it resumes fine. >> The difference could be in the bootloader we are using. I'll send out >> details of my current setup so that you can cross-check if everything >> is the same. > > Thanks a lot for sending me the patches you are using to test. > > But after looking at the differences I noticed that your patches are > not removing the lines that set the CPU0 low power states at the start > of exynos5420_pm_resume() as you asked Vikas [1] but instead your > version of this patch does not have the code that does the same > operation after the early_wakeup label like is made in $subject. The reason for my asking Vikas to remove the lines that restore the core low power state register from the base s2r patch was because it should be part of the mcpm s2r patch where the actual setting of the register takes place. Regarding the location of those lines we should be restoring the register as early as possible, so my next version would have it at the beginning of exynos5420_pm_resume. > > IOW, this [2] is what I had to change on top of $subject to have s2r > working. After that change, both suspending to ram and resuming due > the RTC IRQ alarm being triggered and the core pm_test case works well > on my Exynos5420 Peach Pit. That's good. Thanks for testing ! Regards, Abhilash > >> >> Regards, >> Abhilash > > Best regards, > Javier > > [0]: http://www.spinics.net/lists/arm-kernel/msg367615.html > [1]: http://www.spinics.net/lists/linux-samsung-soc/msg37561.html > [2]: http://pastebin.com/raw.php?i=XSxJH3Ys
diff --git a/arch/arm/mach-exynos/mcpm-exynos.c b/arch/arm/mach-exynos/mcpm-exynos.c index dc9a764..b0d3c2e 100644 --- a/arch/arm/mach-exynos/mcpm-exynos.c +++ b/arch/arm/mach-exynos/mcpm-exynos.c @@ -15,6 +15,7 @@ #include <linux/delay.h> #include <linux/io.h> #include <linux/of_address.h> +#include <linux/syscore_ops.h> #include <asm/cputype.h> #include <asm/cp15.h> @@ -30,6 +31,8 @@ #define EXYNOS5420_USE_ARM_CORE_DOWN_STATE BIT(29) #define EXYNOS5420_USE_L2_COMMON_UP_STATE BIT(30) +static void __iomem *ns_sram_base_addr; + /* * The common v7_exit_coherency_flush API could not be used because of the * Erratum 799270 workaround. This macro is the same as the common one (in @@ -318,10 +321,26 @@ static const struct of_device_id exynos_dt_mcpm_match[] = { {}, }; +static void exynos_mcpm_setup_entry_point(void) +{ + /* + * U-Boot SPL is hardcoded to jump to the start of ns_sram_base_addr + * as part of secondary_cpu_start(). Let's redirect it to the + * mcpm_entry_point(). This is done during both secondary boot-up as + * well as system resume. + */ + __raw_writel(0xe59f0000, ns_sram_base_addr); /* ldr r0, [pc, #0] */ + __raw_writel(0xe12fff10, ns_sram_base_addr + 4); /* bx r0 */ + __raw_writel(virt_to_phys(mcpm_entry_point), ns_sram_base_addr + 8); +} + +static struct syscore_ops exynos_mcpm_syscore_ops = { + .resume = exynos_mcpm_setup_entry_point, +}; + static int __init exynos_mcpm_init(void) { struct device_node *node; - void __iomem *ns_sram_base_addr; unsigned int value, i; int ret; @@ -387,16 +406,9 @@ static int __init exynos_mcpm_init(void) pmu_raw_writel(value, EXYNOS_COMMON_OPTION(i)); } - /* - * U-Boot SPL is hardcoded to jump to the start of ns_sram_base_addr - * as part of secondary_cpu_start(). Let's redirect it to the - * mcpm_entry_point(). - */ - __raw_writel(0xe59f0000, ns_sram_base_addr); /* ldr r0, [pc, #0] */ - __raw_writel(0xe12fff10, ns_sram_base_addr + 4); /* bx r0 */ - __raw_writel(virt_to_phys(mcpm_entry_point), ns_sram_base_addr + 8); + exynos_mcpm_setup_entry_point(); - iounmap(ns_sram_base_addr); + register_syscore_ops(&exynos_mcpm_syscore_ops); return ret; } diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c index adb36a8..222aa3c 100644 --- a/arch/arm/mach-exynos/platsmp.c +++ b/arch/arm/mach-exynos/platsmp.c @@ -137,6 +137,18 @@ void exynos_cpu_power_down(int cpu) */ void exynos_cpu_power_up(int cpu) { + if (cpu == 0 && (of_machine_is_compatible("samsung,exynos5420") || + of_machine_is_compatible("samsung,exynos5800"))) { + /* + * Bypass power down for CPU0 during suspend. Check for + * the SYS_PWR_REG value to decide if we are suspending + * the system. + */ + int val = __raw_readl(pmu_base_addr + + EXYNOS5_ARM_CORE0_SYS_PWR_REG); + if (!(val & S5P_CORE_LOCAL_PWR_EN)) + return; + } pmu_raw_writel(S5P_CORE_LOCAL_PWR_EN, EXYNOS_ARM_CORE_CONFIGURATION(cpu)); } diff --git a/arch/arm/mach-exynos/suspend.c b/arch/arm/mach-exynos/suspend.c index 773140c..e32a42c 100644 --- a/arch/arm/mach-exynos/suspend.c +++ b/arch/arm/mach-exynos/suspend.c @@ -24,6 +24,7 @@ #include <asm/cacheflush.h> #include <asm/hardware/cache-l2x0.h> #include <asm/firmware.h> +#include <asm/mcpm.h> #include <asm/smp_scu.h> #include <asm/suspend.h> @@ -72,6 +73,7 @@ struct exynos_pm_data { unsigned int *release_ret_regs; void (*pm_prepare)(void); + void (*pm_resume_prepare)(void); void (*pm_resume)(void); int (*pm_suspend)(void); int (*cpu_suspend)(unsigned long); @@ -172,9 +174,28 @@ static int exynos_cpu_suspend(unsigned long arg) static int exynos5420_cpu_suspend(unsigned long arg) { - exynos_flush_cache_all(); + /* MCPM works with HW CPU identifiers */ + unsigned int mpidr = read_cpuid_mpidr(); + unsigned int cluster = MPIDR_AFFINITY_LEVEL(mpidr, 1); + unsigned int cpu = MPIDR_AFFINITY_LEVEL(mpidr, 0); + __raw_writel(0x0, sysram_base_addr + EXYNOS5420_CPU_STATE); - return exynos_cpu_do_idle(); + + if (IS_ENABLED(CONFIG_EXYNOS5420_MCPM)) { + mcpm_set_entry_vector(cpu, cluster, exynos_cpu_resume); + + /* + * Residency value passed to mcpm_cpu_suspend back-end + * has to be given clear semantics. Set to 0 as a + * temporary value. + */ + mcpm_cpu_suspend(0); + } + + pr_info("Failed to suspend the system\n"); + + /* return value != 0 means failure */ + return 1; } static void exynos_pm_set_wakeup_mask(void) @@ -189,9 +210,6 @@ static void exynos_pm_enter_sleep_mode(void) /* Set value of power down register for sleep mode */ exynos_sys_powerdown_conf(SYS_SLEEP); pmu_raw_writel(S5P_CHECK_SLEEP, S5P_INFORM1); - - /* ensure at least INFORM0 has the resume address */ - pmu_raw_writel(virt_to_phys(exynos_cpu_resume), S5P_INFORM0); } static void exynos_pm_prepare(void) @@ -206,6 +224,9 @@ static void exynos_pm_prepare(void) pm_data->num_extra_save); exynos_pm_enter_sleep_mode(); + + /* ensure at least INFORM0 has the resume address */ + pmu_raw_writel(virt_to_phys(exynos_cpu_resume), S5P_INFORM0); } static void exynos5420_pm_prepare(void) @@ -230,6 +251,10 @@ static void exynos5420_pm_prepare(void) exynos_pm_enter_sleep_mode(); + /* ensure at least INFORM0 has the resume address */ + if (IS_ENABLED(CONFIG_EXYNOS5420_MCPM)) + pmu_raw_writel(virt_to_phys(mcpm_entry_point), S5P_INFORM0); + tmp = pmu_raw_readl(EXYNOS5_ARM_L2_OPTION); tmp &= ~EXYNOS5_USE_RETENTION; pmu_raw_writel(tmp, EXYNOS5_ARM_L2_OPTION); @@ -318,6 +343,12 @@ early_wakeup: pmu_raw_writel(0x0, S5P_INFORM1); } +static void exynos5420_prepare_pm_resume(void) +{ + if (IS_ENABLED(CONFIG_EXYNOS5420_MCPM)) + WARN_ON(mcpm_cpu_powered_up()); +} + static void exynos5420_pm_resume(void) { unsigned long tmp; @@ -346,6 +377,12 @@ static void exynos5420_pm_resume(void) early_wakeup: + /* Restore the CPU0 low power state register */ + tmp = __raw_readl(pmu_base_addr + + EXYNOS5_ARM_CORE0_SYS_PWR_REG); + pmu_raw_writel(tmp | S5P_CORE_LOCAL_PWR_EN, + EXYNOS5_ARM_CORE0_SYS_PWR_REG); + tmp = pmu_raw_readl(EXYNOS5420_SFR_AXI_CGDIS1); tmp &= ~EXYNOS5420_UFS; pmu_raw_writel(tmp, EXYNOS5420_SFR_AXI_CGDIS1); @@ -396,6 +433,8 @@ static int exynos_suspend_enter(suspend_state_t state) if (ret) return ret; + if (pm_data->pm_resume_prepare) + pm_data->pm_resume_prepare(); s3c_pm_restore_uarts(); S3C_PMDBG("%s: wakeup stat: %08x\n", __func__, @@ -453,6 +492,7 @@ static struct exynos_pm_data exynos5420_pm_data = { .wkup_irq = exynos5250_wkup_irq, .wake_disable_mask = (0x7F << 7) | (0x1F << 1), .release_ret_regs = exynos5420_release_ret_regs, + .pm_resume_prepare = exynos5420_prepare_pm_resume, .pm_resume = exynos5420_pm_resume, .pm_suspend = exynos5420_pm_suspend, .pm_prepare = exynos5420_pm_prepare,