This series inteds to fix various issues spotted while testing v3.16-rc2.
The patches should be reasonably independent from each other and so could be
picked to respective trees. See descriptions of individual patches for more
information.
Tested on Exynos4412-based Trats2 and Exynos4210-based Trats boards.
AFAIK there is no need to backport those fixes to stable.
Tomasz Figa (6):
mmc: sdhci-s3c: Fix local I/O clock gating
ARM: EXYNOS: Fix core ID used by platsmp and hotplug code
clk: samsung: exynos4: Remove SRC_MASK_ISP gates
ARM: SAMSUNG: Restore Samsung PM Debug functionality
ARM: EXYNOS: Fix suspend/resume sequencies
ARM: EXYNOS: Register cpuidle device only on Exynos4210 and 5250
arch/arm/mach-exynos/exynos.c | 6 ++----
arch/arm/mach-exynos/hotplug.c | 10 ++++++----
arch/arm/mach-exynos/platsmp.c | 34 +++++++++++++++++++---------------
arch/arm/mach-exynos/pm.c | 20 ++++----------------
arch/arm/plat-samsung/Kconfig | 8 +++++++-
arch/arm/plat-samsung/pm-debug.c | 1 +
drivers/clk/samsung/clk-exynos4.c | 16 ++++------------
drivers/mmc/host/sdhci-s3c.c | 17 ++++++++++-------
8 files changed, 53 insertions(+), 59 deletions(-)
--
1.9.3
For internal card detection mechanism it is required that the local I/O
clock is always running. However while current implementation accounts
for this, it does so incorrectly leading to race conditions and warnings
about unbalanced clock disables.
This patch fixes it by inverting the logic, which now increases local
I/O clock enable count when internal card detect is used, instead of
decreasing it otherwise.
Signed-off-by: Tomasz Figa <[email protected]>
Cc: Ben Dooks <[email protected]>
Cc: Chris Ball <[email protected]>
Cc: Ulf Hansson <[email protected]>
Cc: [email protected]
---
drivers/mmc/host/sdhci-s3c.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
index fa5954a..1795e1f 100644
--- a/drivers/mmc/host/sdhci-s3c.c
+++ b/drivers/mmc/host/sdhci-s3c.c
@@ -487,8 +487,13 @@ static int sdhci_s3c_probe(struct platform_device *pdev)
goto err_pdata_io_clk;
}
- /* enable the local io clock and keep it running for the moment. */
- clk_prepare_enable(sc->clk_io);
+ /*
+ * Keep local I/O clock enabled for internal card detect pin
+ * or runtime PM is disabled.
+ */
+ if (pdata->cd_type == S3C_SDHCI_CD_INTERNAL
+ || !IS_ENABLED(CONFIG_PM_RUNTIME))
+ clk_prepare_enable(sc->clk_io);
for (clks = 0, ptr = 0; ptr < MAX_BUS_CLK; ptr++) {
char name[14];
@@ -611,15 +616,13 @@ static int sdhci_s3c_probe(struct platform_device *pdev)
goto err_req_regs;
}
-#ifdef CONFIG_PM_RUNTIME
- if (pdata->cd_type != S3C_SDHCI_CD_INTERNAL)
- clk_disable_unprepare(sc->clk_io);
-#endif
return 0;
err_req_regs:
err_no_busclks:
- clk_disable_unprepare(sc->clk_io);
+ if (pdata->cd_type == S3C_SDHCI_CD_INTERNAL
+ || !IS_ENABLED(CONFIG_PM_RUNTIME))
+ clk_disable_unprepare(sc->clk_io);
err_pdata_io_clk:
sdhci_free_host(host);
--
1.9.3
Currently, the Exynos cpuidle driver works correctly only on Exynos4210
and 5250. Trying to use it with just one CPU online on any other Exynos
SoC will lead to system failure, due to unsupported AFTR mode on other
SoCs. This patch fixes the problem by registering the driver only on
supported SoCs and letting others simply use default WFI mode until
support for them is added.
Signed-off-by: Tomasz Figa <[email protected]>
---
arch/arm/mach-exynos/exynos.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c
index f38cf7c..176bbf5 100644
--- a/arch/arm/mach-exynos/exynos.c
+++ b/arch/arm/mach-exynos/exynos.c
@@ -173,10 +173,8 @@ static struct platform_device exynos_cpuidle = {
void __init exynos_cpuidle_init(void)
{
- if (soc_is_exynos5440())
- return;
-
- platform_device_register(&exynos_cpuidle);
+ if (soc_is_exynos4210() || soc_is_exynos5250())
+ platform_device_register(&exynos_cpuidle);
}
void __init exynos_cpufreq_init(void)
--
1.9.3
Due to recently merged patches and previous merge conflicts, the Samsung
PM Debug functionality no longer can be enabled. This patch fixes
incorrect dependency of SAMSUNG_PM_DEBUG on an integer symbol and adds
missing header inclusion.
Signed-off-by: Tomasz Figa <[email protected]>
---
arch/arm/plat-samsung/Kconfig | 8 +++++++-
arch/arm/plat-samsung/pm-debug.c | 1 +
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/arch/arm/plat-samsung/Kconfig b/arch/arm/plat-samsung/Kconfig
index 301b892..483c959 100644
--- a/arch/arm/plat-samsung/Kconfig
+++ b/arch/arm/plat-samsung/Kconfig
@@ -412,9 +412,14 @@ config S5P_DEV_MFC
comment "Power management"
+config HAVE_SAMSUNG_PM_DEBUG
+ bool
+ help
+ Allow compilation of Samsung PM debugging code.
+
config SAMSUNG_PM_DEBUG
bool "S3C2410 PM Suspend debug"
- depends on PM && DEBUG_KERNEL && DEBUG_S3C_UART
+ depends on PM && DEBUG_KERNEL && HAVE_SAMSUNG_PM_DEBUG
help
Say Y here if you want verbose debugging from the PM Suspend and
Resume code. See <file:Documentation/arm/Samsung-S3C24XX/Suspend.txt>
@@ -484,6 +489,7 @@ config S5P_SLEEP
config DEBUG_S3C_UART
depends on PLAT_SAMSUNG
+ select HAVE_SAMSUNG_PM_DEBUG
int
default "0" if DEBUG_S3C_UART0
default "1" if DEBUG_S3C_UART1
diff --git a/arch/arm/plat-samsung/pm-debug.c b/arch/arm/plat-samsung/pm-debug.c
index 8f19f66..3960960 100644
--- a/arch/arm/plat-samsung/pm-debug.c
+++ b/arch/arm/plat-samsung/pm-debug.c
@@ -14,6 +14,7 @@
*/
#include <linux/serial_core.h>
+#include <linux/serial_s3c.h>
#include <linux/io.h>
#include <asm/mach/map.h>
--
1.9.3
Due to recent consolidation of Exynos suspend and cpuidle code, some
parts of suspend and resume sequences are executed two times, once from
exynos_pm_syscore_ops and then from exynos_cpu_pm_notifier() and thus it
breaks suspend, at least on Exynos4-based boards.
This patch fixes the issue by removing exynos_pm_syscore_ops completely
and making the code rely only on CPU PM notifier.
Tested on Exynos4210-based Trats board.
Signed-off-by: Tomasz Figa <[email protected]>
---
arch/arm/mach-exynos/pm.c | 20 ++++----------------
1 file changed, 4 insertions(+), 16 deletions(-)
diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c
index 87c0d34..98d4926 100644
--- a/arch/arm/mach-exynos/pm.c
+++ b/arch/arm/mach-exynos/pm.c
@@ -364,11 +364,6 @@ early_wakeup:
return;
}
-static struct syscore_ops exynos_pm_syscore_ops = {
- .suspend = exynos_pm_suspend,
- .resume = exynos_pm_resume,
-};
-
/*
* Suspend Ops
*/
@@ -438,19 +433,13 @@ static int exynos_cpu_pm_notifier(struct notifier_block *self,
switch (cmd) {
case CPU_PM_ENTER:
- if (cpu == 0) {
- exynos_pm_central_suspend();
- exynos_cpu_save_register();
- }
+ if (cpu == 0)
+ exynos_pm_suspend();
break;
case CPU_PM_EXIT:
- if (cpu == 0) {
- if (!soc_is_exynos5250())
- scu_enable(S5P_VA_SCU);
- exynos_cpu_restore_register();
- exynos_pm_central_resume();
- }
+ if (cpu == 0)
+ exynos_pm_resume();
break;
}
@@ -475,6 +464,5 @@ void __init exynos_pm_init(void)
tmp |= ((0xFF << 8) | (0x1F << 1));
__raw_writel(tmp, S5P_WAKEUP_MASK);
- register_syscore_ops(&exynos_pm_syscore_ops);
suspend_set_ops(&exynos_suspend_ops);
}
--
1.9.3
When CPU topology is specified in device tree, cpu_logical_map() does
not return core ID anymore, but rather full MPIDR value. This breaks
existing calculation of PMU register offsets on Exynos SoCs.
This patch fixes the problem by adjusting the code to use only core ID
bits of the value returned by cpu_logical_map() to allow CPU topology to
be specified in device tree on Exynos SoCs.
Signed-off-by: Tomasz Figa <[email protected]>
---
arch/arm/mach-exynos/hotplug.c | 10 ++++++----
arch/arm/mach-exynos/platsmp.c | 34 +++++++++++++++++++---------------
2 files changed, 25 insertions(+), 19 deletions(-)
diff --git a/arch/arm/mach-exynos/hotplug.c b/arch/arm/mach-exynos/hotplug.c
index 69fa483..5644dac 100644
--- a/arch/arm/mach-exynos/hotplug.c
+++ b/arch/arm/mach-exynos/hotplug.c
@@ -40,11 +40,13 @@ static inline void cpu_leave_lowpower(void)
static inline void platform_do_lowpower(unsigned int cpu, int *spurious)
{
+ u32 mpidr = cpu_logical_map(cpu);
+ u32 core_id = MPIDR_AFFINITY_LEVEL(mpidr, 0);
+
for (;;) {
- /* make cpu1 to be turned off at next WFI command */
- if (cpu == 1)
- exynos_cpu_power_down(cpu);
+ /* Turn the CPU off on next WFI instruction. */
+ exynos_cpu_power_down(core_id);
/*
* here's the WFI
@@ -54,7 +56,7 @@ static inline void platform_do_lowpower(unsigned int cpu, int *spurious)
:
: "memory", "cc");
- if (pen_release == cpu_logical_map(cpu)) {
+ if (pen_release == core_id) {
/*
* OK, proper wakeup, we're done
*/
diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c
index 1c8d31e..50b9aad 100644
--- a/arch/arm/mach-exynos/platsmp.c
+++ b/arch/arm/mach-exynos/platsmp.c
@@ -90,7 +90,8 @@ static void exynos_secondary_init(unsigned int cpu)
static int exynos_boot_secondary(unsigned int cpu, struct task_struct *idle)
{
unsigned long timeout;
- unsigned long phys_cpu = cpu_logical_map(cpu);
+ u32 mpidr = cpu_logical_map(cpu);
+ u32 core_id = MPIDR_AFFINITY_LEVEL(mpidr, 0);
int ret = -ENOSYS;
/*
@@ -104,17 +105,18 @@ static int exynos_boot_secondary(unsigned int cpu, struct task_struct *idle)
* the holding pen - release it, then wait for it to flag
* that it has been released by resetting pen_release.
*
- * Note that "pen_release" is the hardware CPU ID, whereas
+ * Note that "pen_release" is the hardware CPU core ID, whereas
* "cpu" is Linux's internal ID.
*/
- write_pen_release(phys_cpu);
+ write_pen_release(core_id);
- if (!exynos_cpu_power_state(cpu)) {
- exynos_cpu_power_up(cpu);
+ if (!exynos_cpu_power_state(core_id)) {
+ exynos_cpu_power_up(core_id);
timeout = 10;
/* wait max 10 ms until cpu1 is on */
- while (exynos_cpu_power_state(cpu) != S5P_CORE_LOCAL_PWR_EN) {
+ while (exynos_cpu_power_state(core_id)
+ != S5P_CORE_LOCAL_PWR_EN) {
if (timeout-- == 0)
break;
@@ -145,20 +147,20 @@ static int exynos_boot_secondary(unsigned int cpu, struct task_struct *idle)
* Try to set boot address using firmware first
* and fall back to boot register if it fails.
*/
- ret = call_firmware_op(set_cpu_boot_addr, phys_cpu, boot_addr);
+ ret = call_firmware_op(set_cpu_boot_addr, core_id, boot_addr);
if (ret && ret != -ENOSYS)
goto fail;
if (ret == -ENOSYS) {
- void __iomem *boot_reg = cpu_boot_reg(phys_cpu);
+ void __iomem *boot_reg = cpu_boot_reg(core_id);
if (IS_ERR(boot_reg)) {
ret = PTR_ERR(boot_reg);
goto fail;
}
- __raw_writel(boot_addr, cpu_boot_reg(phys_cpu));
+ __raw_writel(boot_addr, cpu_boot_reg(core_id));
}
- call_firmware_op(cpu_boot, phys_cpu);
+ call_firmware_op(cpu_boot, core_id);
arch_send_wakeup_ipi_mask(cpumask_of(cpu));
@@ -227,22 +229,24 @@ static void __init exynos_smp_prepare_cpus(unsigned int max_cpus)
* boot register if it fails.
*/
for (i = 1; i < max_cpus; ++i) {
- unsigned long phys_cpu;
unsigned long boot_addr;
+ u32 mpidr;
+ u32 core_id;
int ret;
- phys_cpu = cpu_logical_map(i);
+ mpidr = cpu_logical_map(i);
+ core_id = MPIDR_AFFINITY_LEVEL(mpidr, 0);
boot_addr = virt_to_phys(exynos4_secondary_startup);
- ret = call_firmware_op(set_cpu_boot_addr, phys_cpu, boot_addr);
+ ret = call_firmware_op(set_cpu_boot_addr, core_id, boot_addr);
if (ret && ret != -ENOSYS)
break;
if (ret == -ENOSYS) {
- void __iomem *boot_reg = cpu_boot_reg(phys_cpu);
+ void __iomem *boot_reg = cpu_boot_reg(core_id);
if (IS_ERR(boot_reg))
break;
- __raw_writel(boot_addr, cpu_boot_reg(phys_cpu));
+ __raw_writel(boot_addr, cpu_boot_reg(core_id));
}
}
}
--
1.9.3
ISP special clocks have dedicated gating registers and so MUX SRC_MASK
register should not be used. This patch fixes the problem of
Exynos4x12-based boards freezing on system suspend, because those
mux outputs need not to be masked while suspending.
Signed-off-by: Tomasz Figa <[email protected]>
Cc: Mike Turquette <[email protected]>
---
drivers/clk/samsung/clk-exynos4.c | 16 ++++------------
1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/drivers/clk/samsung/clk-exynos4.c b/drivers/clk/samsung/clk-exynos4.c
index 4f150c9..7f4a473 100644
--- a/drivers/clk/samsung/clk-exynos4.c
+++ b/drivers/clk/samsung/clk-exynos4.c
@@ -925,21 +925,13 @@ static struct samsung_gate_clock exynos4x12_gate_clks[] __initdata = {
GATE(CLK_RTC, "rtc", "aclk100", E4X12_GATE_IP_PERIR, 15,
0, 0),
GATE(CLK_KEYIF, "keyif", "aclk100", E4X12_GATE_IP_PERIR, 16, 0, 0),
- GATE(CLK_SCLK_PWM_ISP, "sclk_pwm_isp", "div_pwm_isp",
- E4X12_SRC_MASK_ISP, 0, CLK_SET_RATE_PARENT, 0),
- GATE(CLK_SCLK_SPI0_ISP, "sclk_spi0_isp", "div_spi0_isp_pre",
- E4X12_SRC_MASK_ISP, 4, CLK_SET_RATE_PARENT, 0),
- GATE(CLK_SCLK_SPI1_ISP, "sclk_spi1_isp", "div_spi1_isp_pre",
- E4X12_SRC_MASK_ISP, 8, CLK_SET_RATE_PARENT, 0),
- GATE(CLK_SCLK_UART_ISP, "sclk_uart_isp", "div_uart_isp",
- E4X12_SRC_MASK_ISP, 12, CLK_SET_RATE_PARENT, 0),
- GATE(CLK_PWM_ISP_SCLK, "pwm_isp_sclk", "sclk_pwm_isp",
+ GATE(CLK_PWM_ISP_SCLK, "pwm_isp_sclk", "div_pwm_isp",
E4X12_GATE_IP_ISP, 0, 0, 0),
- GATE(CLK_SPI0_ISP_SCLK, "spi0_isp_sclk", "sclk_spi0_isp",
+ GATE(CLK_SPI0_ISP_SCLK, "spi0_isp_sclk", "div_spi0_isp_pre",
E4X12_GATE_IP_ISP, 1, 0, 0),
- GATE(CLK_SPI1_ISP_SCLK, "spi1_isp_sclk", "sclk_spi1_isp",
+ GATE(CLK_SPI1_ISP_SCLK, "spi1_isp_sclk", "div_spi1_isp_pre",
E4X12_GATE_IP_ISP, 2, 0, 0),
- GATE(CLK_UART_ISP_SCLK, "uart_isp_sclk", "sclk_uart_isp",
+ GATE(CLK_UART_ISP_SCLK, "uart_isp_sclk", "div_uart_isp",
E4X12_GATE_IP_ISP, 3, 0, 0),
GATE(CLK_WDT, "watchdog", "aclk100", E4X12_GATE_IP_PERIR, 14, 0, 0),
GATE(CLK_PCM0, "pcm0", "aclk100", E4X12_GATE_IP_MAUDIO, 2,
--
1.9.3
Hi Tomasz,
On Tue, Jun 24, 2014 at 7:27 PM, Tomasz Figa <[email protected]> wrote:
> Due to recent consolidation of Exynos suspend and cpuidle code, some
> parts of suspend and resume sequences are executed two times, once from
> exynos_pm_syscore_ops and then from exynos_cpu_pm_notifier() and thus it
> breaks suspend, at least on Exynos4-based boards.
>
> This patch fixes the issue by removing exynos_pm_syscore_ops completely
> and making the code rely only on CPU PM notifier.
>
> Tested on Exynos4210-based Trats board.
>
> Signed-off-by: Tomasz Figa <[email protected]>
> ---
> arch/arm/mach-exynos/pm.c | 20 ++++----------------
> 1 file changed, 4 insertions(+), 16 deletions(-)
>
A patch from me has recently gone in which converts the soc_is checks
for A9 specific registers to the A9 part number. This patch will
probably have to be rebased atop it.
Regards,
Abhilash
> diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c
> index 87c0d34..98d4926 100644
> --- a/arch/arm/mach-exynos/pm.c
> +++ b/arch/arm/mach-exynos/pm.c
> @@ -364,11 +364,6 @@ early_wakeup:
> return;
> }
>
> -static struct syscore_ops exynos_pm_syscore_ops = {
> - .suspend = exynos_pm_suspend,
> - .resume = exynos_pm_resume,
> -};
> -
> /*
> * Suspend Ops
> */
> @@ -438,19 +433,13 @@ static int exynos_cpu_pm_notifier(struct notifier_block *self,
>
> switch (cmd) {
> case CPU_PM_ENTER:
> - if (cpu == 0) {
> - exynos_pm_central_suspend();
> - exynos_cpu_save_register();
> - }
> + if (cpu == 0)
> + exynos_pm_suspend();
> break;
>
> case CPU_PM_EXIT:
> - if (cpu == 0) {
> - if (!soc_is_exynos5250())
> - scu_enable(S5P_VA_SCU);
> - exynos_cpu_restore_register();
> - exynos_pm_central_resume();
> - }
> + if (cpu == 0)
> + exynos_pm_resume();
> break;
> }
>
> @@ -475,6 +464,5 @@ void __init exynos_pm_init(void)
> tmp |= ((0xFF << 8) | (0x1F << 1));
> __raw_writel(tmp, S5P_WAKEUP_MASK);
>
> - register_syscore_ops(&exynos_pm_syscore_ops);
> suspend_set_ops(&exynos_suspend_ops);
> }
> --
> 1.9.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
Hi Tomasz,
On Tue, Jun 24, 2014 at 2:57 PM, Tomasz Figa <[email protected]> wrote:
> ISP special clocks have dedicated gating registers and so MUX SRC_MASK
> register should not be used. This patch fixes the problem of
> Exynos4x12-based boards freezing on system suspend, because those
> mux outputs need not to be masked while suspending.
Not sure if you will be interested in this, as your plate must be
pretty full already, and I am probably the first person to try
suspend/resume on ODROID, but:
ODROID-U2 fails to suspend/resume. I am testing with rtcwake, trying
to raise a wakeup alarm on the internal Exynos4412 RTC. For this,
CONFIG_COMMON_CLK_MAX77686 must be disabled (otherwise it disables the
upstream 32KHz clock source for the RTC), I also have
CONFIG_RTC_DRV_MAX77686 disabled so that there is only one RTC to
worry about.
Then:
rtcwake --utc -m mem -s 10 -v
Before this patch, it would totally hang after calling cpu_suspend()
(checked with S3C_PMDBG) - not sure if it hangs before sleeping, or if
it sleeps but simply fails to wake up.
With this patch, now it seems like the RTC alarm does wake up the
system after the desired time, however it immediately goes back into
uboot rather than resuming into Linux. So this patch does make some
progress at least.
The power light is on at all times during these tests (not sure if
that means anything, but I was wondering if it should go out when the
system suspends).
Thanks
Daniel
Hi Tomasz,
On Tue, Jun 24, 2014 at 2:57 PM, Tomasz Figa <[email protected]> wrote:
> Due to recently merged patches and previous merge conflicts, the Samsung
> PM Debug functionality no longer can be enabled. This patch fixes
> incorrect dependency of SAMSUNG_PM_DEBUG on an integer symbol and adds
> missing header inclusion.
Testing against 3.16-rc2, this doesn't quite work for me.
SAMSUNG_PM_DEBUG still doesn't appear in menuconfig.
HAVE_SAMSUNG_PM_DEBUG is 'n'. Looks like the select from
DEBUG_S3C_UART is not working, but I'm not sure why.
Daniel
Hi Daniel,
On 25.06.2014 11:58, Daniel Drake wrote:
> Hi Tomasz,
>
> On Tue, Jun 24, 2014 at 2:57 PM, Tomasz Figa <[email protected]> wrote:
>> ISP special clocks have dedicated gating registers and so MUX SRC_MASK
>> register should not be used. This patch fixes the problem of
>> Exynos4x12-based boards freezing on system suspend, because those
>> mux outputs need not to be masked while suspending.
>
> Not sure if you will be interested in this, as your plate must be
> pretty full already, and I am probably the first person to try
> suspend/resume on ODROID, but:
>
> ODROID-U2 fails to suspend/resume. I am testing with rtcwake, trying
> to raise a wakeup alarm on the internal Exynos4412 RTC. For this,
> CONFIG_COMMON_CLK_MAX77686 must be disabled (otherwise it disables the
> upstream 32KHz clock source for the RTC), I also have
> CONFIG_RTC_DRV_MAX77686 disabled so that there is only one RTC to
> worry about.
>
> Then:
> rtcwake --utc -m mem -s 10 -v
>
> Before this patch, it would totally hang after calling cpu_suspend()
> (checked with S3C_PMDBG) - not sure if it hangs before sleeping, or if
> it sleeps but simply fails to wake up.
>
> With this patch, now it seems like the RTC alarm does wake up the
> system after the desired time, however it immediately goes back into
> uboot rather than resuming into Linux. So this patch does make some
> progress at least.
>
> The power light is on at all times during these tests (not sure if
> that means anything, but I was wondering if it should go out when the
> system suspends).
As far as I'm aware of, all Exynos4412-based ODROIDs run secure
firmware, which needs special handling of suspend/resume. I already have
a series to address this, but there is one more issue that I'd like to
fix, until I send it. The patches should hit the ML this week, though.
Best regards,
Tomasz
On 25.06.2014 12:09, Daniel Drake wrote:
> Hi Tomasz,
>
> On Tue, Jun 24, 2014 at 2:57 PM, Tomasz Figa <[email protected]> wrote:
>> Due to recently merged patches and previous merge conflicts, the Samsung
>> PM Debug functionality no longer can be enabled. This patch fixes
>> incorrect dependency of SAMSUNG_PM_DEBUG on an integer symbol and adds
>> missing header inclusion.
>
> Testing against 3.16-rc2, this doesn't quite work for me.
> SAMSUNG_PM_DEBUG still doesn't appear in menuconfig.
>
> HAVE_SAMSUNG_PM_DEBUG is 'n'. Looks like the select from
> DEBUG_S3C_UART is not working, but I'm not sure why.
I hastily changed this to something simpler in last minute before
sending without proper testing. Let me send something that works.
Best regards,
Tomasz
Due to recently merged patches and previous merge conflicts, the Samsung
PM Debug functionality no longer can be enabled. This patch fixes
incorrect dependency of SAMSUNG_PM_DEBUG on an integer symbol and adds
missing header inclusion.
Signed-off-by: Tomasz Figa <[email protected]>
---
arch/arm/plat-samsung/Kconfig | 5 +++--
arch/arm/plat-samsung/pm-debug.c | 1 +
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/arm/plat-samsung/Kconfig b/arch/arm/plat-samsung/Kconfig
index 301b892..ad9515f 100644
--- a/arch/arm/plat-samsung/Kconfig
+++ b/arch/arm/plat-samsung/Kconfig
@@ -413,8 +413,9 @@ config S5P_DEV_MFC
comment "Power management"
config SAMSUNG_PM_DEBUG
- bool "S3C2410 PM Suspend debug"
- depends on PM && DEBUG_KERNEL && DEBUG_S3C_UART
+ bool "Samsung PM Suspend debug"
+ depends on PM && DEBUG_KERNEL
+ depends on DEBUG_EXYNOS_UART || DEBUG_S3C24XX_UART || DEBUG_S3C2410_UART
help
Say Y here if you want verbose debugging from the PM Suspend and
Resume code. See <file:Documentation/arm/Samsung-S3C24XX/Suspend.txt>
diff --git a/arch/arm/plat-samsung/pm-debug.c b/arch/arm/plat-samsung/pm-debug.c
index 8f19f66..3960960 100644
--- a/arch/arm/plat-samsung/pm-debug.c
+++ b/arch/arm/plat-samsung/pm-debug.c
@@ -14,6 +14,7 @@
*/
#include <linux/serial_core.h>
+#include <linux/serial_s3c.h>
#include <linux/io.h>
#include <asm/mach/map.h>
--
1.9.3
Due to recent consolidation of Exynos suspend and cpuidle code, some
parts of suspend and resume sequences are executed two times, once from
exynos_pm_syscore_ops and then from exynos_cpu_pm_notifier() and thus it
breaks suspend, at least on Exynos4-based boards.
This patch fixes the issue by removing exynos_pm_syscore_ops completely
and making the code rely only on CPU PM notifier.
Tested on Exynos4210-based Trats board.
Signed-off-by: Tomasz Figa <[email protected]>
---
arch/arm/mach-exynos/pm.c | 23 ++++-------------------
1 file changed, 4 insertions(+), 19 deletions(-)
Changes since v1:
- rebased onto Kukjin's fixes branch.
diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c
index 202ca73..f23cc77 100644
--- a/arch/arm/mach-exynos/pm.c
+++ b/arch/arm/mach-exynos/pm.c
@@ -364,11 +364,6 @@ early_wakeup:
return;
}
-static struct syscore_ops exynos_pm_syscore_ops = {
- .suspend = exynos_pm_suspend,
- .resume = exynos_pm_resume,
-};
-
/*
* Suspend Ops
*/
@@ -438,22 +433,13 @@ static int exynos_cpu_pm_notifier(struct notifier_block *self,
switch (cmd) {
case CPU_PM_ENTER:
- if (cpu == 0) {
- exynos_pm_central_suspend();
- if (read_cpuid_part_number() == ARM_CPU_PART_CORTEX_A9)
- exynos_cpu_save_register();
- }
+ if (cpu == 0)
+ exynos_pm_suspend();
break;
case CPU_PM_EXIT:
- if (cpu == 0) {
- if (read_cpuid_part_number() ==
- ARM_CPU_PART_CORTEX_A9) {
- scu_enable(S5P_VA_SCU);
- exynos_cpu_restore_register();
- }
- exynos_pm_central_resume();
- }
+ if (cpu == 0)
+ exynos_pm_resume();
break;
}
@@ -478,6 +464,5 @@ void __init exynos_pm_init(void)
tmp |= ((0xFF << 8) | (0x1F << 1));
__raw_writel(tmp, S5P_WAKEUP_MASK);
- register_syscore_ops(&exynos_pm_syscore_ops);
suspend_set_ops(&exynos_suspend_ops);
}
--
1.9.3
On Wed, Jun 25, 2014 at 12:43 PM, Tomasz Figa <[email protected]> wrote:
> Due to recently merged patches and previous merge conflicts, the Samsung
> PM Debug functionality no longer can be enabled. This patch fixes
> incorrect dependency of SAMSUNG_PM_DEBUG on an integer symbol and adds
> missing header inclusion.
I replaced the original patch with this updated version and can
confirm that the option now appears in the menu again, and compile
succeeds when set to yes.
Thanks
Daniel
On 24.06.2014 15:57, Tomasz Figa wrote:
> ISP special clocks have dedicated gating registers and so MUX SRC_MASK
> register should not be used. This patch fixes the problem of
> Exynos4x12-based boards freezing on system suspend, because those
> mux outputs need not to be masked while suspending.
>
> Signed-off-by: Tomasz Figa <[email protected]>
> Cc: Mike Turquette <[email protected]>
> ---
> drivers/clk/samsung/clk-exynos4.c | 16 ++++------------
> 1 file changed, 4 insertions(+), 12 deletions(-)
Applied as a fix for 3.16.
Best regards,
Tomasz
Hi,
On 24.06.2014 15:57, Tomasz Figa wrote:
> This series inteds to fix various issues spotted while testing v3.16-rc2.
> The patches should be reasonably independent from each other and so could be
> picked to respective trees. See descriptions of individual patches for more
> information.
>
> Tested on Exynos4412-based Trats2 and Exynos4210-based Trats boards.
>
> AFAIK there is no need to backport those fixes to stable.
>
> Tomasz Figa (6):
> mmc: sdhci-s3c: Fix local I/O clock gating
> ARM: EXYNOS: Fix core ID used by platsmp and hotplug code
> clk: samsung: exynos4: Remove SRC_MASK_ISP gates
> ARM: SAMSUNG: Restore Samsung PM Debug functionality
> ARM: EXYNOS: Fix suspend/resume sequencies
> ARM: EXYNOS: Register cpuidle device only on Exynos4210 and 5250
>
> arch/arm/mach-exynos/exynos.c | 6 ++----
> arch/arm/mach-exynos/hotplug.c | 10 ++++++----
> arch/arm/mach-exynos/platsmp.c | 34 +++++++++++++++++++---------------
> arch/arm/mach-exynos/pm.c | 20 ++++----------------
> arch/arm/plat-samsung/Kconfig | 8 +++++++-
> arch/arm/plat-samsung/pm-debug.c | 1 +
> drivers/clk/samsung/clk-exynos4.c | 16 ++++------------
> drivers/mmc/host/sdhci-s3c.c | 17 ++++++++++-------
> 8 files changed, 53 insertions(+), 59 deletions(-)
>
Gentle ping.
Best regards,
Tomasz
On 07/01/14 22:54, Tomasz Figa wrote:
> Hi,
>
> On 24.06.2014 15:57, Tomasz Figa wrote:
>> This series inteds to fix various issues spotted while testing v3.16-rc2.
>> The patches should be reasonably independent from each other and so could be
>> picked to respective trees. See descriptions of individual patches for more
>> information.
>>
>> Tested on Exynos4412-based Trats2 and Exynos4210-based Trats boards.
>>
>> AFAIK there is no need to backport those fixes to stable.
>>
>> Tomasz Figa (6):
>> mmc: sdhci-s3c: Fix local I/O clock gating
>> ARM: EXYNOS: Fix core ID used by platsmp and hotplug code
>> clk: samsung: exynos4: Remove SRC_MASK_ISP gates
>> ARM: SAMSUNG: Restore Samsung PM Debug functionality
>> ARM: EXYNOS: Fix suspend/resume sequencies
>> ARM: EXYNOS: Register cpuidle device only on Exynos4210 and 5250
>>
>> arch/arm/mach-exynos/exynos.c | 6 ++----
>> arch/arm/mach-exynos/hotplug.c | 10 ++++++----
>> arch/arm/mach-exynos/platsmp.c | 34 +++++++++++++++++++---------------
>> arch/arm/mach-exynos/pm.c | 20 ++++----------------
>> arch/arm/plat-samsung/Kconfig | 8 +++++++-
>> arch/arm/plat-samsung/pm-debug.c | 1 +
>> drivers/clk/samsung/clk-exynos4.c | 16 ++++------------
>> drivers/mmc/host/sdhci-s3c.c | 17 ++++++++++-------
>> 8 files changed, 53 insertions(+), 59 deletions(-)
>>
>
> Gentle ping.
>
Oops, I missed this fixes from 2nd samsung fixes for 3.16...let me have
a look this series today then will send again soon...
Thanks,
Kukjin
Tomasz Figa wrote:
>
> When CPU topology is specified in device tree, cpu_logical_map() does
> not return core ID anymore, but rather full MPIDR value. This breaks
> existing calculation of PMU register offsets on Exynos SoCs.
>
> This patch fixes the problem by adjusting the code to use only core ID
> bits of the value returned by cpu_logical_map() to allow CPU topology to
> be specified in device tree on Exynos SoCs.
>
> Signed-off-by: Tomasz Figa <[email protected]>
Looks good to me, but I think we don't need this fix in 3.16 because the CPU
topology is not specified in DT yet...if I'm wrong, please correct me.
Will apply for 3.17.
Thanks,
Kukjin
> ---
> arch/arm/mach-exynos/hotplug.c | 10 ++++++----
> arch/arm/mach-exynos/platsmp.c | 34 +++++++++++++++++++---------------
> 2 files changed, 25 insertions(+), 19 deletions(-)
>
> diff --git a/arch/arm/mach-exynos/hotplug.c b/arch/arm/mach-exynos/hotplug.c
> index 69fa483..5644dac 100644
> --- a/arch/arm/mach-exynos/hotplug.c
> +++ b/arch/arm/mach-exynos/hotplug.c
> @@ -40,11 +40,13 @@ static inline void cpu_leave_lowpower(void)
>
> static inline void platform_do_lowpower(unsigned int cpu, int *spurious)
> {
> + u32 mpidr = cpu_logical_map(cpu);
> + u32 core_id = MPIDR_AFFINITY_LEVEL(mpidr, 0);
> +
> for (;;) {
>
> - /* make cpu1 to be turned off at next WFI command */
> - if (cpu == 1)
> - exynos_cpu_power_down(cpu);
> + /* Turn the CPU off on next WFI instruction. */
> + exynos_cpu_power_down(core_id);
>
> /*
> * here's the WFI
> @@ -54,7 +56,7 @@ static inline void platform_do_lowpower(unsigned int cpu, int *spurious)
> :
> : "memory", "cc");
>
> - if (pen_release == cpu_logical_map(cpu)) {
> + if (pen_release == core_id) {
> /*
> * OK, proper wakeup, we're done
> */
> diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c
> index 1c8d31e..50b9aad 100644
> --- a/arch/arm/mach-exynos/platsmp.c
> +++ b/arch/arm/mach-exynos/platsmp.c
> @@ -90,7 +90,8 @@ static void exynos_secondary_init(unsigned int cpu)
> static int exynos_boot_secondary(unsigned int cpu, struct task_struct *idle)
> {
> unsigned long timeout;
> - unsigned long phys_cpu = cpu_logical_map(cpu);
> + u32 mpidr = cpu_logical_map(cpu);
> + u32 core_id = MPIDR_AFFINITY_LEVEL(mpidr, 0);
> int ret = -ENOSYS;
>
> /*
> @@ -104,17 +105,18 @@ static int exynos_boot_secondary(unsigned int cpu, struct task_struct *idle)
> * the holding pen - release it, then wait for it to flag
> * that it has been released by resetting pen_release.
> *
> - * Note that "pen_release" is the hardware CPU ID, whereas
> + * Note that "pen_release" is the hardware CPU core ID, whereas
> * "cpu" is Linux's internal ID.
> */
> - write_pen_release(phys_cpu);
> + write_pen_release(core_id);
>
> - if (!exynos_cpu_power_state(cpu)) {
> - exynos_cpu_power_up(cpu);
> + if (!exynos_cpu_power_state(core_id)) {
> + exynos_cpu_power_up(core_id);
> timeout = 10;
>
> /* wait max 10 ms until cpu1 is on */
> - while (exynos_cpu_power_state(cpu) != S5P_CORE_LOCAL_PWR_EN) {
> + while (exynos_cpu_power_state(core_id)
> + != S5P_CORE_LOCAL_PWR_EN) {
> if (timeout-- == 0)
> break;
>
> @@ -145,20 +147,20 @@ static int exynos_boot_secondary(unsigned int cpu, struct task_struct *idle)
> * Try to set boot address using firmware first
> * and fall back to boot register if it fails.
> */
> - ret = call_firmware_op(set_cpu_boot_addr, phys_cpu, boot_addr);
> + ret = call_firmware_op(set_cpu_boot_addr, core_id, boot_addr);
> if (ret && ret != -ENOSYS)
> goto fail;
> if (ret == -ENOSYS) {
> - void __iomem *boot_reg = cpu_boot_reg(phys_cpu);
> + void __iomem *boot_reg = cpu_boot_reg(core_id);
>
> if (IS_ERR(boot_reg)) {
> ret = PTR_ERR(boot_reg);
> goto fail;
> }
> - __raw_writel(boot_addr, cpu_boot_reg(phys_cpu));
> + __raw_writel(boot_addr, cpu_boot_reg(core_id));
> }
>
> - call_firmware_op(cpu_boot, phys_cpu);
> + call_firmware_op(cpu_boot, core_id);
>
> arch_send_wakeup_ipi_mask(cpumask_of(cpu));
>
> @@ -227,22 +229,24 @@ static void __init exynos_smp_prepare_cpus(unsigned int max_cpus)
> * boot register if it fails.
> */
> for (i = 1; i < max_cpus; ++i) {
> - unsigned long phys_cpu;
> unsigned long boot_addr;
> + u32 mpidr;
> + u32 core_id;
> int ret;
>
> - phys_cpu = cpu_logical_map(i);
> + mpidr = cpu_logical_map(i);
> + core_id = MPIDR_AFFINITY_LEVEL(mpidr, 0);
> boot_addr = virt_to_phys(exynos4_secondary_startup);
>
> - ret = call_firmware_op(set_cpu_boot_addr, phys_cpu, boot_addr);
> + ret = call_firmware_op(set_cpu_boot_addr, core_id, boot_addr);
> if (ret && ret != -ENOSYS)
> break;
> if (ret == -ENOSYS) {
> - void __iomem *boot_reg = cpu_boot_reg(phys_cpu);
> + void __iomem *boot_reg = cpu_boot_reg(core_id);
>
> if (IS_ERR(boot_reg))
> break;
> - __raw_writel(boot_addr, cpu_boot_reg(phys_cpu));
> + __raw_writel(boot_addr, cpu_boot_reg(core_id));
> }
> }
> }
> --
> 1.9.3
Tomasz Figa wrote:
>
> Due to recently merged patches and previous merge conflicts, the Samsung
> PM Debug functionality no longer can be enabled. This patch fixes
> incorrect dependency of SAMSUNG_PM_DEBUG on an integer symbol and adds
> missing header inclusion.
>
Yes, you're right and it should be fixed...but I have comments...
> Signed-off-by: Tomasz Figa <[email protected]>
> ---
> arch/arm/plat-samsung/Kconfig | 8 +++++++-
> arch/arm/plat-samsung/pm-debug.c | 1 +
> 2 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/plat-samsung/Kconfig b/arch/arm/plat-samsung/Kconfig
> index 301b892..483c959 100644
> --- a/arch/arm/plat-samsung/Kconfig
> +++ b/arch/arm/plat-samsung/Kconfig
> @@ -412,9 +412,14 @@ config S5P_DEV_MFC
>
> comment "Power management"
>
> +config HAVE_SAMSUNG_PM_DEBUG
> + bool
> + help
> + Allow compilation of Samsung PM debugging code.
> +
> config SAMSUNG_PM_DEBUG
> bool "S3C2410 PM Suspend debug"
> - depends on PM && DEBUG_KERNEL && DEBUG_S3C_UART
> + depends on PM && DEBUG_KERNEL && HAVE_SAMSUNG_PM_DEBUG
> help
> Say Y here if you want verbose debugging from the PM Suspend and
> Resume code. See <file:Documentation/arm/Samsung-S3C24XX/Suspend.txt>
> @@ -484,6 +489,7 @@ config S5P_SLEEP
>
> config DEBUG_S3C_UART
> depends on PLAT_SAMSUNG
> + select HAVE_SAMSUNG_PM_DEBUG
Hmm...
The DEBUG_S3C_UART will be '0' when we select DEBUG_S3C_UART0, then the
HAVE_SAMSUNG_PM_DEBUG will not be selected so SAMSUNG_PM_DEBUG is also...
> int
> default "0" if DEBUG_S3C_UART0
> default "1" if DEBUG_S3C_UART1
> diff --git a/arch/arm/plat-samsung/pm-debug.c b/arch/arm/plat-samsung/pm-debug.c
> index 8f19f66..3960960 100644
> --- a/arch/arm/plat-samsung/pm-debug.c
> +++ b/arch/arm/plat-samsung/pm-debug.c
> @@ -14,6 +14,7 @@
> */
>
> #include <linux/serial_core.h>
> +#include <linux/serial_s3c.h>
Yeah, this is required then we don't need inclusion <linux/serial_core.h> here.
> #include <linux/io.h>
>
> #include <asm/mach/map.h>
> --
> 1.9.3
On 08.07.2014 15:48, Kukjin Kim wrote:
> Tomasz Figa wrote:
>>
>> Due to recently merged patches and previous merge conflicts, the Samsung
>> PM Debug functionality no longer can be enabled. This patch fixes
>> incorrect dependency of SAMSUNG_PM_DEBUG on an integer symbol and adds
>> missing header inclusion.
>>
> Yes, you're right and it should be fixed...but I have comments...
>
>> Signed-off-by: Tomasz Figa <[email protected]>
>> ---
>> arch/arm/plat-samsung/Kconfig | 8 +++++++-
>> arch/arm/plat-samsung/pm-debug.c | 1 +
>> 2 files changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/plat-samsung/Kconfig b/arch/arm/plat-samsung/Kconfig
>> index 301b892..483c959 100644
>> --- a/arch/arm/plat-samsung/Kconfig
>> +++ b/arch/arm/plat-samsung/Kconfig
>> @@ -412,9 +412,14 @@ config S5P_DEV_MFC
>>
>> comment "Power management"
>>
>> +config HAVE_SAMSUNG_PM_DEBUG
>> + bool
>> + help
>> + Allow compilation of Samsung PM debugging code.
>> +
>> config SAMSUNG_PM_DEBUG
>> bool "S3C2410 PM Suspend debug"
>> - depends on PM && DEBUG_KERNEL && DEBUG_S3C_UART
>> + depends on PM && DEBUG_KERNEL && HAVE_SAMSUNG_PM_DEBUG
>> help
>> Say Y here if you want verbose debugging from the PM Suspend and
>> Resume code. See <file:Documentation/arm/Samsung-S3C24XX/Suspend.txt>
>> @@ -484,6 +489,7 @@ config S5P_SLEEP
>>
>> config DEBUG_S3C_UART
>> depends on PLAT_SAMSUNG
>> + select HAVE_SAMSUNG_PM_DEBUG
>
> Hmm...
>
> The DEBUG_S3C_UART will be '0' when we select DEBUG_S3C_UART0, then the
> HAVE_SAMSUNG_PM_DEBUG will not be selected so SAMSUNG_PM_DEBUG is also...
Yes, that's right. I posted v2 after a while in another reply to this
thread.
Best regards,
Tomasz
Tomasz Figa wrote:
>
> Due to recent consolidation of Exynos suspend and cpuidle code, some
> parts of suspend and resume sequences are executed two times, once from
> exynos_pm_syscore_ops and then from exynos_cpu_pm_notifier() and thus it
> breaks suspend, at least on Exynos4-based boards.
>
+ Jonghwan Choi
Yes, right. We moved the functionality into cpu_pm notifier in the commit ID
0ebc13e2 ("ARM: EXYNOS: Move the power sequence call in the cpu_pm notifier"),
so this should be fixed as you pointed out. But I talked to my colleague
Jonghwan about this and we need more time to have a look in detail for every
exynos SoCs. Will be back soon.
Thanks,
Kukjin
> This patch fixes the issue by removing exynos_pm_syscore_ops completely
> and making the code rely only on CPU PM notifier.
>
> Tested on Exynos4210-based Trats board.
>
> Signed-off-by: Tomasz Figa <[email protected]>
> ---
> arch/arm/mach-exynos/pm.c | 20 ++++----------------
> 1 file changed, 4 insertions(+), 16 deletions(-)
>
> diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c
> index 87c0d34..98d4926 100644
> --- a/arch/arm/mach-exynos/pm.c
> +++ b/arch/arm/mach-exynos/pm.c
> @@ -364,11 +364,6 @@ early_wakeup:
> return;
> }
>
> -static struct syscore_ops exynos_pm_syscore_ops = {
> - .suspend = exynos_pm_suspend,
> - .resume = exynos_pm_resume,
> -};
> -
> /*
> * Suspend Ops
> */
> @@ -438,19 +433,13 @@ static int exynos_cpu_pm_notifier(struct notifier_block *self,
>
> switch (cmd) {
> case CPU_PM_ENTER:
> - if (cpu == 0) {
> - exynos_pm_central_suspend();
> - exynos_cpu_save_register();
> - }
> + if (cpu == 0)
> + exynos_pm_suspend();
> break;
>
> case CPU_PM_EXIT:
> - if (cpu == 0) {
> - if (!soc_is_exynos5250())
> - scu_enable(S5P_VA_SCU);
> - exynos_cpu_restore_register();
> - exynos_pm_central_resume();
> - }
> + if (cpu == 0)
> + exynos_pm_resume();
> break;
> }
>
> @@ -475,6 +464,5 @@ void __init exynos_pm_init(void)
> tmp |= ((0xFF << 8) | (0x1F << 1));
> __raw_writel(tmp, S5P_WAKEUP_MASK);
>
> - register_syscore_ops(&exynos_pm_syscore_ops);
> suspend_set_ops(&exynos_suspend_ops);
> }
> --
> 1.9.3
Tomasz Figa wrote:
>
> Currently, the Exynos cpuidle driver works correctly only on Exynos4210
> and 5250. Trying to use it with just one CPU online on any other Exynos
> SoC will lead to system failure, due to unsupported AFTR mode on other
> SoCs. This patch fixes the problem by registering the driver only on
> supported SoCs and letting others simply use default WFI mode until
> support for them is added.
>
Hmm...I thought other SoCs have no problem on cpuidle except exynos5420 and
exynos5440....something like this would be helpful to avoid system failure.
But unfortunately this conflicts with Pankaj's cleanup cpufreq_init() and
cpuidle_init() patch you've reviewed and I've applied in my local...
I'm going to check which exynos is ok on cpuidle and then sort them out.
Thanks,
Kukjin
> Signed-off-by: Tomasz Figa <[email protected]>
> ---
> arch/arm/mach-exynos/exynos.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c
> index f38cf7c..176bbf5 100644
> --- a/arch/arm/mach-exynos/exynos.c
> +++ b/arch/arm/mach-exynos/exynos.c
> @@ -173,10 +173,8 @@ static struct platform_device exynos_cpuidle = {
>
> void __init exynos_cpuidle_init(void)
> {
> - if (soc_is_exynos5440())
> - return;
> -
> - platform_device_register(&exynos_cpuidle);
> + if (soc_is_exynos4210() || soc_is_exynos5250())
> + platform_device_register(&exynos_cpuidle);
> }
>
> void __init exynos_cpufreq_init(void)
> --
> 1.9.3
Hi,
On Tuesday, July 08, 2014 11:15:21 PM Kukjin Kim wrote:
> Tomasz Figa wrote:
> >
> > Currently, the Exynos cpuidle driver works correctly only on Exynos4210
> > and 5250. Trying to use it with just one CPU online on any other Exynos
> > SoC will lead to system failure, due to unsupported AFTR mode on other
> > SoCs. This patch fixes the problem by registering the driver only on
> > supported SoCs and letting others simply use default WFI mode until
> > support for them is added.
> >
> Hmm...I thought other SoCs have no problem on cpuidle except exynos5420 and
> exynos5440....something like this would be helpful to avoid system failure.
> But unfortunately this conflicts with Pankaj's cleanup cpufreq_init() and
> cpuidle_init() patch you've reviewed and I've applied in my local...
>
> I'm going to check which exynos is ok on cpuidle and then sort them out.
Tomasz's patch is correct - currently only Exynos4210 and Exynos5250 work
fine with AFTR mode and upstream kernel (AFTR gets triggered by offlining
CPUs other than CPU0).
[ Exynos4x12 and Exynos3250 need secure firmware support (patches for this
need to be reworked on top of recent PM/firmware/cpuidle changes).
Exynos5410 should use big_little cpuidle driver. ]
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
Bartlomiej Zolnierkiewicz wrote:
>
> Hi,
>
Hi Bart,
> On Tuesday, July 08, 2014 11:15:21 PM Kukjin Kim wrote:
> > Tomasz Figa wrote:
> > >
> > > Currently, the Exynos cpuidle driver works correctly only on Exynos4210
> > > and 5250. Trying to use it with just one CPU online on any other Exynos
> > > SoC will lead to system failure, due to unsupported AFTR mode on other
> > > SoCs. This patch fixes the problem by registering the driver only on
> > > supported SoCs and letting others simply use default WFI mode until
> > > support for them is added.
> > >
> > Hmm...I thought other SoCs have no problem on cpuidle except exynos5420 and
> > exynos5440....something like this would be helpful to avoid system failure.
> > But unfortunately this conflicts with Pankaj's cleanup cpufreq_init() and
> > cpuidle_init() patch you've reviewed and I've applied in my local...
> >
> > I'm going to check which exynos is ok on cpuidle and then sort them out.
>
> Tomasz's patch is correct - currently only Exynos4210 and Exynos5250 work
> fine with AFTR mode and upstream kernel (AFTR gets triggered by offlining
> CPUs other than CPU0).
>
> [ Exynos4x12 and Exynos3250 need secure firmware support (patches for this
> need to be reworked on top of recent PM/firmware/cpuidle changes).
>
> Exynos5410 should use big_little cpuidle driver. ]
>
Agreed that this is required at this moment.
I will take this into fixes for 3.16.
Thanks,
Kukjin
Looks good to me.
Acked-by: Jaehoon Chung <[email protected]>
On 06/24/2014 10:57 PM, Tomasz Figa wrote:
> For internal card detection mechanism it is required that the local I/O
> clock is always running. However while current implementation accounts
> for this, it does so incorrectly leading to race conditions and warnings
> about unbalanced clock disables.
>
> This patch fixes it by inverting the logic, which now increases local
> I/O clock enable count when internal card detect is used, instead of
> decreasing it otherwise.
>
> Signed-off-by: Tomasz Figa <[email protected]>
> Cc: Ben Dooks <[email protected]>
> Cc: Chris Ball <[email protected]>
> Cc: Ulf Hansson <[email protected]>
> Cc: [email protected]
> ---
> drivers/mmc/host/sdhci-s3c.c | 17 ++++++++++-------
> 1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
> index fa5954a..1795e1f 100644
> --- a/drivers/mmc/host/sdhci-s3c.c
> +++ b/drivers/mmc/host/sdhci-s3c.c
> @@ -487,8 +487,13 @@ static int sdhci_s3c_probe(struct platform_device *pdev)
> goto err_pdata_io_clk;
> }
>
> - /* enable the local io clock and keep it running for the moment. */
> - clk_prepare_enable(sc->clk_io);
> + /*
> + * Keep local I/O clock enabled for internal card detect pin
> + * or runtime PM is disabled.
> + */
> + if (pdata->cd_type == S3C_SDHCI_CD_INTERNAL
> + || !IS_ENABLED(CONFIG_PM_RUNTIME))
> + clk_prepare_enable(sc->clk_io);
>
> for (clks = 0, ptr = 0; ptr < MAX_BUS_CLK; ptr++) {
> char name[14];
> @@ -611,15 +616,13 @@ static int sdhci_s3c_probe(struct platform_device *pdev)
> goto err_req_regs;
> }
>
> -#ifdef CONFIG_PM_RUNTIME
> - if (pdata->cd_type != S3C_SDHCI_CD_INTERNAL)
> - clk_disable_unprepare(sc->clk_io);
> -#endif
> return 0;
>
> err_req_regs:
> err_no_busclks:
> - clk_disable_unprepare(sc->clk_io);
> + if (pdata->cd_type == S3C_SDHCI_CD_INTERNAL
> + || !IS_ENABLED(CONFIG_PM_RUNTIME))
> + clk_disable_unprepare(sc->clk_io);
>
> err_pdata_io_clk:
> sdhci_free_host(host);
>
Hi Kukjin,
On 08.07.2014 15:21, Kukjin Kim wrote:
> Tomasz Figa wrote:
>>
>> When CPU topology is specified in device tree, cpu_logical_map() does
>> not return core ID anymore, but rather full MPIDR value. This breaks
>> existing calculation of PMU register offsets on Exynos SoCs.
>>
>> This patch fixes the problem by adjusting the code to use only core ID
>> bits of the value returned by cpu_logical_map() to allow CPU topology to
>> be specified in device tree on Exynos SoCs.
>>
>> Signed-off-by: Tomasz Figa <[email protected]>
>
> Looks good to me, but I think we don't need this fix in 3.16 because the CPU
> topology is not specified in DT yet...if I'm wrong, please correct me.
CPU topology is already specified in DT for Exynos3250, 5250, 5260, 5410
and 5420/5800.
This patch also fixes CPU hotplug on SoCs with more than 2 cores,
because it removes incorrect condition check in platform_do_lowpower().
Please take this fix for next -rc release.
Best regards,
Tomasz
Hi Kukjin,
On 25.06.2014 13:43, Tomasz Figa wrote:
> Due to recently merged patches and previous merge conflicts, the Samsung
> PM Debug functionality no longer can be enabled. This patch fixes
> incorrect dependency of SAMSUNG_PM_DEBUG on an integer symbol and adds
> missing header inclusion.
>
> Signed-off-by: Tomasz Figa <[email protected]>
> ---
> arch/arm/plat-samsung/Kconfig | 5 +++--
> arch/arm/plat-samsung/pm-debug.c | 1 +
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/plat-samsung/Kconfig b/arch/arm/plat-samsung/Kconfig
> index 301b892..ad9515f 100644
> --- a/arch/arm/plat-samsung/Kconfig
> +++ b/arch/arm/plat-samsung/Kconfig
> @@ -413,8 +413,9 @@ config S5P_DEV_MFC
> comment "Power management"
>
> config SAMSUNG_PM_DEBUG
> - bool "S3C2410 PM Suspend debug"
> - depends on PM && DEBUG_KERNEL && DEBUG_S3C_UART
> + bool "Samsung PM Suspend debug"
> + depends on PM && DEBUG_KERNEL
> + depends on DEBUG_EXYNOS_UART || DEBUG_S3C24XX_UART || DEBUG_S3C2410_UART
> help
> Say Y here if you want verbose debugging from the PM Suspend and
> Resume code. See <file:Documentation/arm/Samsung-S3C24XX/Suspend.txt>
> diff --git a/arch/arm/plat-samsung/pm-debug.c b/arch/arm/plat-samsung/pm-debug.c
> index 8f19f66..3960960 100644
> --- a/arch/arm/plat-samsung/pm-debug.c
> +++ b/arch/arm/plat-samsung/pm-debug.c
> @@ -14,6 +14,7 @@
> */
>
> #include <linux/serial_core.h>
> +#include <linux/serial_s3c.h>
> #include <linux/io.h>
>
> #include <asm/mach/map.h>
>
Please consider this patch for next fixes pull request.
Best regards,
Tomasz
Hi Kukjin,
On 25.06.2014 13:52, Tomasz Figa wrote:
> Due to recent consolidation of Exynos suspend and cpuidle code, some
> parts of suspend and resume sequences are executed two times, once from
> exynos_pm_syscore_ops and then from exynos_cpu_pm_notifier() and thus it
> breaks suspend, at least on Exynos4-based boards.
>
> This patch fixes the issue by removing exynos_pm_syscore_ops completely
> and making the code rely only on CPU PM notifier.
>
> Tested on Exynos4210-based Trats board.
>
> Signed-off-by: Tomasz Figa <[email protected]>
> ---
> arch/arm/mach-exynos/pm.c | 23 ++++-------------------
> 1 file changed, 4 insertions(+), 19 deletions(-)
>
> Changes since v1:
> - rebased onto Kukjin's fixes branch.
>
> diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c
> index 202ca73..f23cc77 100644
> --- a/arch/arm/mach-exynos/pm.c
> +++ b/arch/arm/mach-exynos/pm.c
> @@ -364,11 +364,6 @@ early_wakeup:
> return;
> }
>
> -static struct syscore_ops exynos_pm_syscore_ops = {
> - .suspend = exynos_pm_suspend,
> - .resume = exynos_pm_resume,
> -};
> -
> /*
> * Suspend Ops
> */
> @@ -438,22 +433,13 @@ static int exynos_cpu_pm_notifier(struct notifier_block *self,
>
> switch (cmd) {
> case CPU_PM_ENTER:
> - if (cpu == 0) {
> - exynos_pm_central_suspend();
> - if (read_cpuid_part_number() == ARM_CPU_PART_CORTEX_A9)
> - exynos_cpu_save_register();
> - }
> + if (cpu == 0)
> + exynos_pm_suspend();
> break;
>
> case CPU_PM_EXIT:
> - if (cpu == 0) {
> - if (read_cpuid_part_number() ==
> - ARM_CPU_PART_CORTEX_A9) {
> - scu_enable(S5P_VA_SCU);
> - exynos_cpu_restore_register();
> - }
> - exynos_pm_central_resume();
> - }
> + if (cpu == 0)
> + exynos_pm_resume();
> break;
> }
>
> @@ -478,6 +464,5 @@ void __init exynos_pm_init(void)
> tmp |= ((0xFF << 8) | (0x1F << 1));
> __raw_writel(tmp, S5P_WAKEUP_MASK);
>
> - register_syscore_ops(&exynos_pm_syscore_ops);
> suspend_set_ops(&exynos_suspend_ops);
> }
>
Please consider this patch for next fixes pull request. Without it
suspend/resume is broken for Exynos4 and probably other SoCs. This patch
just restores the sequence from before the patch moving things to PM
notifier, so I don't think it should need any special treatment.
Best regards,
Tomasz
Hi,
On Monday, July 14, 2014 11:54:48 AM Tomasz Figa wrote:
> Hi Kukjin,
>
> On 25.06.2014 13:52, Tomasz Figa wrote:
> > Due to recent consolidation of Exynos suspend and cpuidle code, some
> > parts of suspend and resume sequences are executed two times, once from
> > exynos_pm_syscore_ops and then from exynos_cpu_pm_notifier() and thus it
> > breaks suspend, at least on Exynos4-based boards.
> >
> > This patch fixes the issue by removing exynos_pm_syscore_ops completely
> > and making the code rely only on CPU PM notifier.
> >
> > Tested on Exynos4210-based Trats board.
> >
> > Signed-off-by: Tomasz Figa <[email protected]>
> > ---
> > arch/arm/mach-exynos/pm.c | 23 ++++-------------------
> > 1 file changed, 4 insertions(+), 19 deletions(-)
> >
> > Changes since v1:
> > - rebased onto Kukjin's fixes branch.
> >
> > diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c
> > index 202ca73..f23cc77 100644
> > --- a/arch/arm/mach-exynos/pm.c
> > +++ b/arch/arm/mach-exynos/pm.c
> > @@ -364,11 +364,6 @@ early_wakeup:
> > return;
> > }
> >
> > -static struct syscore_ops exynos_pm_syscore_ops = {
> > - .suspend = exynos_pm_suspend,
> > - .resume = exynos_pm_resume,
> > -};
> > -
> > /*
> > * Suspend Ops
> > */
> > @@ -438,22 +433,13 @@ static int exynos_cpu_pm_notifier(struct notifier_block *self,
> >
> > switch (cmd) {
> > case CPU_PM_ENTER:
> > - if (cpu == 0) {
> > - exynos_pm_central_suspend();
> > - if (read_cpuid_part_number() == ARM_CPU_PART_CORTEX_A9)
> > - exynos_cpu_save_register();
> > - }
> > + if (cpu == 0)
> > + exynos_pm_suspend();
> > break;
> >
> > case CPU_PM_EXIT:
> > - if (cpu == 0) {
> > - if (read_cpuid_part_number() ==
> > - ARM_CPU_PART_CORTEX_A9) {
> > - scu_enable(S5P_VA_SCU);
> > - exynos_cpu_restore_register();
> > - }
> > - exynos_pm_central_resume();
> > - }
> > + if (cpu == 0)
> > + exynos_pm_resume();
> > break;
> > }
> >
> > @@ -478,6 +464,5 @@ void __init exynos_pm_init(void)
> > tmp |= ((0xFF << 8) | (0x1F << 1));
> > __raw_writel(tmp, S5P_WAKEUP_MASK);
> >
> > - register_syscore_ops(&exynos_pm_syscore_ops);
> > suspend_set_ops(&exynos_suspend_ops);
> > }
> >
>
> Please consider this patch for next fixes pull request. Without it
> suspend/resume is broken for Exynos4 and probably other SoCs. This patch
> just restores the sequence from before the patch moving things to PM
> notifier, so I don't think it should need any special treatment.
Your patch fixes the regression and is a step in the good direction but it
seems that it needs a bit more work:
Your patch adds to cpuidle AFTR code path restoring of exynos_core_save and
exynos5_sys_save registers without saving them first (restoring is done
through exynos_pm_resume() which is used by both suspend and cpuidle while
saving is done through exynos_pm_prepare() which is used only by suspend).
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
On 15.07.2014 13:19, Bartlomiej Zolnierkiewicz wrote:
>
> Hi,
>
> On Monday, July 14, 2014 11:54:48 AM Tomasz Figa wrote:
>> Hi Kukjin,
>>
>> On 25.06.2014 13:52, Tomasz Figa wrote:
>>> Due to recent consolidation of Exynos suspend and cpuidle code, some
>>> parts of suspend and resume sequences are executed two times, once from
>>> exynos_pm_syscore_ops and then from exynos_cpu_pm_notifier() and thus it
>>> breaks suspend, at least on Exynos4-based boards.
>>>
>>> This patch fixes the issue by removing exynos_pm_syscore_ops completely
>>> and making the code rely only on CPU PM notifier.
>>>
>>> Tested on Exynos4210-based Trats board.
>>>
>>> Signed-off-by: Tomasz Figa <[email protected]>
>>> ---
>>> arch/arm/mach-exynos/pm.c | 23 ++++-------------------
>>> 1 file changed, 4 insertions(+), 19 deletions(-)
>>>
>>> Changes since v1:
>>> - rebased onto Kukjin's fixes branch.
>>>
>>> diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c
>>> index 202ca73..f23cc77 100644
>>> --- a/arch/arm/mach-exynos/pm.c
>>> +++ b/arch/arm/mach-exynos/pm.c
>>> @@ -364,11 +364,6 @@ early_wakeup:
>>> return;
>>> }
>>>
>>> -static struct syscore_ops exynos_pm_syscore_ops = {
>>> - .suspend = exynos_pm_suspend,
>>> - .resume = exynos_pm_resume,
>>> -};
>>> -
>>> /*
>>> * Suspend Ops
>>> */
>>> @@ -438,22 +433,13 @@ static int exynos_cpu_pm_notifier(struct notifier_block *self,
>>>
>>> switch (cmd) {
>>> case CPU_PM_ENTER:
>>> - if (cpu == 0) {
>>> - exynos_pm_central_suspend();
>>> - if (read_cpuid_part_number() == ARM_CPU_PART_CORTEX_A9)
>>> - exynos_cpu_save_register();
>>> - }
>>> + if (cpu == 0)
>>> + exynos_pm_suspend();
>>> break;
>>>
>>> case CPU_PM_EXIT:
>>> - if (cpu == 0) {
>>> - if (read_cpuid_part_number() ==
>>> - ARM_CPU_PART_CORTEX_A9) {
>>> - scu_enable(S5P_VA_SCU);
>>> - exynos_cpu_restore_register();
>>> - }
>>> - exynos_pm_central_resume();
>>> - }
>>> + if (cpu == 0)
>>> + exynos_pm_resume();
>>> break;
>>> }
>>>
>>> @@ -478,6 +464,5 @@ void __init exynos_pm_init(void)
>>> tmp |= ((0xFF << 8) | (0x1F << 1));
>>> __raw_writel(tmp, S5P_WAKEUP_MASK);
>>>
>>> - register_syscore_ops(&exynos_pm_syscore_ops);
>>> suspend_set_ops(&exynos_suspend_ops);
>>> }
>>>
>>
>> Please consider this patch for next fixes pull request. Without it
>> suspend/resume is broken for Exynos4 and probably other SoCs. This patch
>> just restores the sequence from before the patch moving things to PM
>> notifier, so I don't think it should need any special treatment.
>
> Your patch fixes the regression and is a step in the good direction but it
> seems that it needs a bit more work:
>
> Your patch adds to cpuidle AFTR code path restoring of exynos_core_save and
> exynos5_sys_save registers without saving them first (restoring is done
> through exynos_pm_resume() which is used by both suspend and cpuidle while
> saving is done through exynos_pm_prepare() which is used only by suspend).
That's right, unfortunately. Interestingly enough AFTR worked fine on
Exynos4210 with this patch, but still this needs to be fixed.
Now, in fact, I recall that Chander had some objections to this patch as
well and we decided that he will send another patch to replace mine [1],
but I haven't heard from him since that.
Chander, do you have any progress with this? Keep in mind that we need
this as an rc fix and we already have rc5, so not much time left. If you
don't have time to work on this I can take care of this, proceeding as
we discussed in [1].
[1] http://thread.gmane.org/gmane.linux.kernel.samsung-soc/29085/focus=33975
Best regards,
Tomasz
Due to recent consolidation of Exynos suspend and cpuidle code, some
parts of suspend and resume sequences are executed two times, once from
exynos_pm_syscore_ops and then from exynos_cpu_pm_notifier() and thus it
breaks suspend, at least on Exynos4-based boards. In addition, simple
core power down from a cpuidle driver could, in case of CPU 0 could
result in calling functions that are specific to suspend and deeper idle
states.
This patch fixes the issue by moving those operations outside the CPU PM
notifier into suspend and AFTR code paths. This leads to a bit of code
duplication, but allows additional code simplification, so in the end
more code is removed than added.
Signed-off-by: Tomasz Figa <[email protected]>
---
arch/arm/mach-exynos/pm.c | 164 ++++++++++++++++++---------------------
drivers/cpuidle/cpuidle-exynos.c | 25 +-----
2 files changed, 80 insertions(+), 109 deletions(-)
diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c
index 202ca73..e3b04b4 100644
--- a/arch/arm/mach-exynos/pm.c
+++ b/arch/arm/mach-exynos/pm.c
@@ -176,26 +176,6 @@ int exynos_cluster_power_state(int cluster)
#define S5P_CHECK_AFTR 0xFCBA0D10
#define S5P_CHECK_SLEEP 0x00000BAD
-/* Ext-GIC nIRQ/nFIQ is the only wakeup source in AFTR */
-static void exynos_set_wakeupmask(long mask)
-{
- __raw_writel(mask, S5P_WAKEUP_MASK);
-}
-
-static void exynos_cpu_set_boot_vector(long flags)
-{
- __raw_writel(virt_to_phys(exynos_cpu_resume), EXYNOS_BOOT_VECTOR_ADDR);
- __raw_writel(flags, EXYNOS_BOOT_VECTOR_FLAG);
-}
-
-void exynos_enter_aftr(void)
-{
- exynos_set_wakeupmask(0x0000ff3e);
- exynos_cpu_set_boot_vector(S5P_CHECK_AFTR);
- /* Set value of power down register for aftr mode */
- exynos_sys_powerdown_conf(SYS_AFTR);
-}
-
/* For Cortex-A9 Diagnostic and Power control register */
static unsigned int save_arm_register[2];
@@ -235,6 +215,82 @@ static void exynos_cpu_restore_register(void)
: "cc");
}
+static void exynos_pm_central_suspend(void)
+{
+ unsigned long tmp;
+
+ /* Setting Central Sequence Register for power down mode */
+ tmp = __raw_readl(S5P_CENTRAL_SEQ_CONFIGURATION);
+ tmp &= ~S5P_CENTRAL_LOWPWR_CFG;
+ __raw_writel(tmp, S5P_CENTRAL_SEQ_CONFIGURATION);
+}
+
+static int exynos_pm_central_resume(void)
+{
+ unsigned long tmp;
+
+ /*
+ * If PMU failed while entering sleep mode, WFI will be
+ * ignored by PMU and then exiting cpu_do_idle().
+ * S5P_CENTRAL_LOWPWR_CFG bit will not be set automatically
+ * in this situation.
+ */
+ tmp = __raw_readl(S5P_CENTRAL_SEQ_CONFIGURATION);
+ if (!(tmp & S5P_CENTRAL_LOWPWR_CFG)) {
+ tmp |= S5P_CENTRAL_LOWPWR_CFG;
+ __raw_writel(tmp, S5P_CENTRAL_SEQ_CONFIGURATION);
+ /* clear the wakeup state register */
+ __raw_writel(0x0, S5P_WAKEUP_STAT);
+ /* No need to perform below restore code */
+ return -1;
+ }
+
+ return 0;
+}
+
+/* Ext-GIC nIRQ/nFIQ is the only wakeup source in AFTR */
+static void exynos_set_wakeupmask(long mask)
+{
+ __raw_writel(mask, S5P_WAKEUP_MASK);
+}
+
+static void exynos_cpu_set_boot_vector(long flags)
+{
+ __raw_writel(virt_to_phys(exynos_cpu_resume), EXYNOS_BOOT_VECTOR_ADDR);
+ __raw_writel(flags, EXYNOS_BOOT_VECTOR_FLAG);
+}
+
+static int exynos_aftr_finisher(unsigned long flags)
+{
+ exynos_set_wakeupmask(0x0000ff3e);
+ exynos_cpu_set_boot_vector(S5P_CHECK_AFTR);
+ /* Set value of power down register for aftr mode */
+ exynos_sys_powerdown_conf(SYS_AFTR);
+ cpu_do_idle();
+
+ return 0;
+}
+
+void exynos_enter_aftr(void)
+{
+ cpu_pm_enter();
+
+ exynos_pm_central_suspend();
+ if (read_cpuid_part_number() == ARM_CPU_PART_CORTEX_A9)
+ exynos_cpu_save_register();
+
+ cpu_suspend(0, exynos_aftr_finisher);
+
+ if (read_cpuid_part_number() == ARM_CPU_PART_CORTEX_A9) {
+ scu_enable(S5P_VA_SCU);
+ exynos_cpu_restore_register();
+ }
+
+ exynos_pm_central_resume();
+
+ cpu_pm_exit();
+}
+
static int exynos_cpu_suspend(unsigned long arg)
{
#ifdef CONFIG_CACHE_L2X0
@@ -279,16 +335,6 @@ static void exynos_pm_prepare(void)
__raw_writel(virt_to_phys(exynos_cpu_resume), S5P_INFORM0);
}
-static void exynos_pm_central_suspend(void)
-{
- unsigned long tmp;
-
- /* Setting Central Sequence Register for power down mode */
- tmp = __raw_readl(S5P_CENTRAL_SEQ_CONFIGURATION);
- tmp &= ~S5P_CENTRAL_LOWPWR_CFG;
- __raw_writel(tmp, S5P_CENTRAL_SEQ_CONFIGURATION);
-}
-
static int exynos_pm_suspend(void)
{
unsigned long tmp;
@@ -306,29 +352,6 @@ static int exynos_pm_suspend(void)
return 0;
}
-static int exynos_pm_central_resume(void)
-{
- unsigned long tmp;
-
- /*
- * If PMU failed while entering sleep mode, WFI will be
- * ignored by PMU and then exiting cpu_do_idle().
- * S5P_CENTRAL_LOWPWR_CFG bit will not be set automatically
- * in this situation.
- */
- tmp = __raw_readl(S5P_CENTRAL_SEQ_CONFIGURATION);
- if (!(tmp & S5P_CENTRAL_LOWPWR_CFG)) {
- tmp |= S5P_CENTRAL_LOWPWR_CFG;
- __raw_writel(tmp, S5P_CENTRAL_SEQ_CONFIGURATION);
- /* clear the wakeup state register */
- __raw_writel(0x0, S5P_WAKEUP_STAT);
- /* No need to perform below restore code */
- return -1;
- }
-
- return 0;
-}
-
static void exynos_pm_resume(void)
{
if (exynos_pm_central_resume())
@@ -431,45 +454,10 @@ static const struct platform_suspend_ops exynos_suspend_ops = {
.valid = suspend_valid_only_mem,
};
-static int exynos_cpu_pm_notifier(struct notifier_block *self,
- unsigned long cmd, void *v)
-{
- int cpu = smp_processor_id();
-
- switch (cmd) {
- case CPU_PM_ENTER:
- if (cpu == 0) {
- exynos_pm_central_suspend();
- if (read_cpuid_part_number() == ARM_CPU_PART_CORTEX_A9)
- exynos_cpu_save_register();
- }
- break;
-
- case CPU_PM_EXIT:
- if (cpu == 0) {
- if (read_cpuid_part_number() ==
- ARM_CPU_PART_CORTEX_A9) {
- scu_enable(S5P_VA_SCU);
- exynos_cpu_restore_register();
- }
- exynos_pm_central_resume();
- }
- break;
- }
-
- return NOTIFY_OK;
-}
-
-static struct notifier_block exynos_cpu_pm_notifier_block = {
- .notifier_call = exynos_cpu_pm_notifier,
-};
-
void __init exynos_pm_init(void)
{
u32 tmp;
- cpu_pm_register_notifier(&exynos_cpu_pm_notifier_block);
-
/* Platform-specific GIC callback */
gic_arch_extn.irq_set_wake = exynos_irq_set_wake;
diff --git a/drivers/cpuidle/cpuidle-exynos.c b/drivers/cpuidle/cpuidle-exynos.c
index 7c01512..ba9b34b 100644
--- a/drivers/cpuidle/cpuidle-exynos.c
+++ b/drivers/cpuidle/cpuidle-exynos.c
@@ -20,25 +20,6 @@
static void (*exynos_enter_aftr)(void);
-static int idle_finisher(unsigned long flags)
-{
- exynos_enter_aftr();
- cpu_do_idle();
-
- return 1;
-}
-
-static int exynos_enter_core0_aftr(struct cpuidle_device *dev,
- struct cpuidle_driver *drv,
- int index)
-{
- cpu_pm_enter();
- cpu_suspend(0, idle_finisher);
- cpu_pm_exit();
-
- return index;
-}
-
static int exynos_enter_lowpower(struct cpuidle_device *dev,
struct cpuidle_driver *drv,
int index)
@@ -51,8 +32,10 @@ static int exynos_enter_lowpower(struct cpuidle_device *dev,
if (new_index == 0)
return arm_cpuidle_simple_enter(dev, drv, new_index);
- else
- return exynos_enter_core0_aftr(dev, drv, new_index);
+
+ exynos_enter_aftr();
+
+ return new_index;
}
static struct cpuidle_driver exynos_idle_driver = {
--
1.9.3
Forgot to CC Daniel and linux-pm. Sorry for the noise.
On 15.07.2014 16:24, Tomasz Figa wrote:
> Due to recent consolidation of Exynos suspend and cpuidle code, some
> parts of suspend and resume sequences are executed two times, once from
> exynos_pm_syscore_ops and then from exynos_cpu_pm_notifier() and thus it
> breaks suspend, at least on Exynos4-based boards. In addition, simple
> core power down from a cpuidle driver could, in case of CPU 0 could
> result in calling functions that are specific to suspend and deeper idle
> states.
>
> This patch fixes the issue by moving those operations outside the CPU PM
> notifier into suspend and AFTR code paths. This leads to a bit of code
> duplication, but allows additional code simplification, so in the end
> more code is removed than added.
>
> Signed-off-by: Tomasz Figa <[email protected]>
> ---
> arch/arm/mach-exynos/pm.c | 164 ++++++++++++++++++---------------------
> drivers/cpuidle/cpuidle-exynos.c | 25 +-----
> 2 files changed, 80 insertions(+), 109 deletions(-)
>
> diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c
> index 202ca73..e3b04b4 100644
> --- a/arch/arm/mach-exynos/pm.c
> +++ b/arch/arm/mach-exynos/pm.c
> @@ -176,26 +176,6 @@ int exynos_cluster_power_state(int cluster)
> #define S5P_CHECK_AFTR 0xFCBA0D10
> #define S5P_CHECK_SLEEP 0x00000BAD
>
> -/* Ext-GIC nIRQ/nFIQ is the only wakeup source in AFTR */
> -static void exynos_set_wakeupmask(long mask)
> -{
> - __raw_writel(mask, S5P_WAKEUP_MASK);
> -}
> -
> -static void exynos_cpu_set_boot_vector(long flags)
> -{
> - __raw_writel(virt_to_phys(exynos_cpu_resume), EXYNOS_BOOT_VECTOR_ADDR);
> - __raw_writel(flags, EXYNOS_BOOT_VECTOR_FLAG);
> -}
> -
> -void exynos_enter_aftr(void)
> -{
> - exynos_set_wakeupmask(0x0000ff3e);
> - exynos_cpu_set_boot_vector(S5P_CHECK_AFTR);
> - /* Set value of power down register for aftr mode */
> - exynos_sys_powerdown_conf(SYS_AFTR);
> -}
> -
> /* For Cortex-A9 Diagnostic and Power control register */
> static unsigned int save_arm_register[2];
>
> @@ -235,6 +215,82 @@ static void exynos_cpu_restore_register(void)
> : "cc");
> }
>
> +static void exynos_pm_central_suspend(void)
> +{
> + unsigned long tmp;
> +
> + /* Setting Central Sequence Register for power down mode */
> + tmp = __raw_readl(S5P_CENTRAL_SEQ_CONFIGURATION);
> + tmp &= ~S5P_CENTRAL_LOWPWR_CFG;
> + __raw_writel(tmp, S5P_CENTRAL_SEQ_CONFIGURATION);
> +}
> +
> +static int exynos_pm_central_resume(void)
> +{
> + unsigned long tmp;
> +
> + /*
> + * If PMU failed while entering sleep mode, WFI will be
> + * ignored by PMU and then exiting cpu_do_idle().
> + * S5P_CENTRAL_LOWPWR_CFG bit will not be set automatically
> + * in this situation.
> + */
> + tmp = __raw_readl(S5P_CENTRAL_SEQ_CONFIGURATION);
> + if (!(tmp & S5P_CENTRAL_LOWPWR_CFG)) {
> + tmp |= S5P_CENTRAL_LOWPWR_CFG;
> + __raw_writel(tmp, S5P_CENTRAL_SEQ_CONFIGURATION);
> + /* clear the wakeup state register */
> + __raw_writel(0x0, S5P_WAKEUP_STAT);
> + /* No need to perform below restore code */
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +/* Ext-GIC nIRQ/nFIQ is the only wakeup source in AFTR */
> +static void exynos_set_wakeupmask(long mask)
> +{
> + __raw_writel(mask, S5P_WAKEUP_MASK);
> +}
> +
> +static void exynos_cpu_set_boot_vector(long flags)
> +{
> + __raw_writel(virt_to_phys(exynos_cpu_resume), EXYNOS_BOOT_VECTOR_ADDR);
> + __raw_writel(flags, EXYNOS_BOOT_VECTOR_FLAG);
> +}
> +
> +static int exynos_aftr_finisher(unsigned long flags)
> +{
> + exynos_set_wakeupmask(0x0000ff3e);
> + exynos_cpu_set_boot_vector(S5P_CHECK_AFTR);
> + /* Set value of power down register for aftr mode */
> + exynos_sys_powerdown_conf(SYS_AFTR);
> + cpu_do_idle();
> +
> + return 0;
> +}
> +
> +void exynos_enter_aftr(void)
> +{
> + cpu_pm_enter();
> +
> + exynos_pm_central_suspend();
> + if (read_cpuid_part_number() == ARM_CPU_PART_CORTEX_A9)
> + exynos_cpu_save_register();
> +
> + cpu_suspend(0, exynos_aftr_finisher);
> +
> + if (read_cpuid_part_number() == ARM_CPU_PART_CORTEX_A9) {
> + scu_enable(S5P_VA_SCU);
> + exynos_cpu_restore_register();
> + }
> +
> + exynos_pm_central_resume();
> +
> + cpu_pm_exit();
> +}
> +
> static int exynos_cpu_suspend(unsigned long arg)
> {
> #ifdef CONFIG_CACHE_L2X0
> @@ -279,16 +335,6 @@ static void exynos_pm_prepare(void)
> __raw_writel(virt_to_phys(exynos_cpu_resume), S5P_INFORM0);
> }
>
> -static void exynos_pm_central_suspend(void)
> -{
> - unsigned long tmp;
> -
> - /* Setting Central Sequence Register for power down mode */
> - tmp = __raw_readl(S5P_CENTRAL_SEQ_CONFIGURATION);
> - tmp &= ~S5P_CENTRAL_LOWPWR_CFG;
> - __raw_writel(tmp, S5P_CENTRAL_SEQ_CONFIGURATION);
> -}
> -
> static int exynos_pm_suspend(void)
> {
> unsigned long tmp;
> @@ -306,29 +352,6 @@ static int exynos_pm_suspend(void)
> return 0;
> }
>
> -static int exynos_pm_central_resume(void)
> -{
> - unsigned long tmp;
> -
> - /*
> - * If PMU failed while entering sleep mode, WFI will be
> - * ignored by PMU and then exiting cpu_do_idle().
> - * S5P_CENTRAL_LOWPWR_CFG bit will not be set automatically
> - * in this situation.
> - */
> - tmp = __raw_readl(S5P_CENTRAL_SEQ_CONFIGURATION);
> - if (!(tmp & S5P_CENTRAL_LOWPWR_CFG)) {
> - tmp |= S5P_CENTRAL_LOWPWR_CFG;
> - __raw_writel(tmp, S5P_CENTRAL_SEQ_CONFIGURATION);
> - /* clear the wakeup state register */
> - __raw_writel(0x0, S5P_WAKEUP_STAT);
> - /* No need to perform below restore code */
> - return -1;
> - }
> -
> - return 0;
> -}
> -
> static void exynos_pm_resume(void)
> {
> if (exynos_pm_central_resume())
> @@ -431,45 +454,10 @@ static const struct platform_suspend_ops exynos_suspend_ops = {
> .valid = suspend_valid_only_mem,
> };
>
> -static int exynos_cpu_pm_notifier(struct notifier_block *self,
> - unsigned long cmd, void *v)
> -{
> - int cpu = smp_processor_id();
> -
> - switch (cmd) {
> - case CPU_PM_ENTER:
> - if (cpu == 0) {
> - exynos_pm_central_suspend();
> - if (read_cpuid_part_number() == ARM_CPU_PART_CORTEX_A9)
> - exynos_cpu_save_register();
> - }
> - break;
> -
> - case CPU_PM_EXIT:
> - if (cpu == 0) {
> - if (read_cpuid_part_number() ==
> - ARM_CPU_PART_CORTEX_A9) {
> - scu_enable(S5P_VA_SCU);
> - exynos_cpu_restore_register();
> - }
> - exynos_pm_central_resume();
> - }
> - break;
> - }
> -
> - return NOTIFY_OK;
> -}
> -
> -static struct notifier_block exynos_cpu_pm_notifier_block = {
> - .notifier_call = exynos_cpu_pm_notifier,
> -};
> -
> void __init exynos_pm_init(void)
> {
> u32 tmp;
>
> - cpu_pm_register_notifier(&exynos_cpu_pm_notifier_block);
> -
> /* Platform-specific GIC callback */
> gic_arch_extn.irq_set_wake = exynos_irq_set_wake;
>
> diff --git a/drivers/cpuidle/cpuidle-exynos.c b/drivers/cpuidle/cpuidle-exynos.c
> index 7c01512..ba9b34b 100644
> --- a/drivers/cpuidle/cpuidle-exynos.c
> +++ b/drivers/cpuidle/cpuidle-exynos.c
> @@ -20,25 +20,6 @@
>
> static void (*exynos_enter_aftr)(void);
>
> -static int idle_finisher(unsigned long flags)
> -{
> - exynos_enter_aftr();
> - cpu_do_idle();
> -
> - return 1;
> -}
> -
> -static int exynos_enter_core0_aftr(struct cpuidle_device *dev,
> - struct cpuidle_driver *drv,
> - int index)
> -{
> - cpu_pm_enter();
> - cpu_suspend(0, idle_finisher);
> - cpu_pm_exit();
> -
> - return index;
> -}
> -
> static int exynos_enter_lowpower(struct cpuidle_device *dev,
> struct cpuidle_driver *drv,
> int index)
> @@ -51,8 +32,10 @@ static int exynos_enter_lowpower(struct cpuidle_device *dev,
>
> if (new_index == 0)
> return arm_cpuidle_simple_enter(dev, drv, new_index);
> - else
> - return exynos_enter_core0_aftr(dev, drv, new_index);
> +
> + exynos_enter_aftr();
> +
> + return new_index;
> }
>
> static struct cpuidle_driver exynos_idle_driver = {
>
On Tue, Jul 15, 2014 at 5:45 PM, Tomasz Figa <[email protected]> wrote:
> On 15.07.2014 13:19, Bartlomiej Zolnierkiewicz wrote:
>>
>> Hi,
>>
>> On Monday, July 14, 2014 11:54:48 AM Tomasz Figa wrote:
>>> Hi Kukjin,
>>>
>>> On 25.06.2014 13:52, Tomasz Figa wrote:
>>>> Due to recent consolidation of Exynos suspend and cpuidle code, some
>>>> parts of suspend and resume sequences are executed two times, once from
>>>> exynos_pm_syscore_ops and then from exynos_cpu_pm_notifier() and thus it
>>>> breaks suspend, at least on Exynos4-based boards.
>>>>
>>>> This patch fixes the issue by removing exynos_pm_syscore_ops completely
>>>> and making the code rely only on CPU PM notifier.
>>>>
>>>> Tested on Exynos4210-based Trats board.
>>>>
>>>> Signed-off-by: Tomasz Figa <[email protected]>
>>>> ---
>>>> arch/arm/mach-exynos/pm.c | 23 ++++-------------------
>>>> 1 file changed, 4 insertions(+), 19 deletions(-)
>>>>
>>>> Changes since v1:
>>>> - rebased onto Kukjin's fixes branch.
>>>>
>>>> diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c
>>>> index 202ca73..f23cc77 100644
>>>> --- a/arch/arm/mach-exynos/pm.c
>>>> +++ b/arch/arm/mach-exynos/pm.c
>>>> @@ -364,11 +364,6 @@ early_wakeup:
>>>> return;
>>>> }
>>>>
>>>> -static struct syscore_ops exynos_pm_syscore_ops = {
>>>> - .suspend = exynos_pm_suspend,
>>>> - .resume = exynos_pm_resume,
>>>> -};
>>>> -
>>>> /*
>>>> * Suspend Ops
>>>> */
>>>> @@ -438,22 +433,13 @@ static int exynos_cpu_pm_notifier(struct notifier_block *self,
>>>>
>>>> switch (cmd) {
>>>> case CPU_PM_ENTER:
>>>> - if (cpu == 0) {
>>>> - exynos_pm_central_suspend();
>>>> - if (read_cpuid_part_number() == ARM_CPU_PART_CORTEX_A9)
>>>> - exynos_cpu_save_register();
>>>> - }
>>>> + if (cpu == 0)
>>>> + exynos_pm_suspend();
>>>> break;
>>>>
>>>> case CPU_PM_EXIT:
>>>> - if (cpu == 0) {
>>>> - if (read_cpuid_part_number() ==
>>>> - ARM_CPU_PART_CORTEX_A9) {
>>>> - scu_enable(S5P_VA_SCU);
>>>> - exynos_cpu_restore_register();
>>>> - }
>>>> - exynos_pm_central_resume();
>>>> - }
>>>> + if (cpu == 0)
>>>> + exynos_pm_resume();
>>>> break;
>>>> }
>>>>
>>>> @@ -478,6 +464,5 @@ void __init exynos_pm_init(void)
>>>> tmp |= ((0xFF << 8) | (0x1F << 1));
>>>> __raw_writel(tmp, S5P_WAKEUP_MASK);
>>>>
>>>> - register_syscore_ops(&exynos_pm_syscore_ops);
>>>> suspend_set_ops(&exynos_suspend_ops);
>>>> }
>>>>
>>>
>>> Please consider this patch for next fixes pull request. Without it
>>> suspend/resume is broken for Exynos4 and probably other SoCs. This patch
>>> just restores the sequence from before the patch moving things to PM
>>> notifier, so I don't think it should need any special treatment.
>>
>> Your patch fixes the regression and is a step in the good direction but it
>> seems that it needs a bit more work:
>>
>> Your patch adds to cpuidle AFTR code path restoring of exynos_core_save and
>> exynos5_sys_save registers without saving them first (restoring is done
>> through exynos_pm_resume() which is used by both suspend and cpuidle while
>> saving is done through exynos_pm_prepare() which is used only by suspend).
>
> That's right, unfortunately. Interestingly enough AFTR worked fine on
> Exynos4210 with this patch, but still this needs to be fixed.
>
> Now, in fact, I recall that Chander had some objections to this patch as
> well and we decided that he will send another patch to replace mine [1],
> but I haven't heard from him since that.
Patch for the same has been already posted.
Below is the link for the same
http://www.spinics.net/lists/arm-kernel/msg343402.html
This patch has conflict with Russel's patch, so i will rebase on his
tree and resend
This patch can be ignored in that case.
>
> Chander, do you have any progress with this? Keep in mind that we need
> this as an rc fix and we already have rc5, so not much time left. If you
> don't have time to work on this I can take care of this, proceeding as
> we discussed in [1].
>
> [1] http://thread.gmane.org/gmane.linux.kernel.samsung-soc/29085/focus=33975
>
> 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
On 07/14/14 18:50, Tomasz Figa wrote:
> Hi Kukjin,
>
Hi,
> On 08.07.2014 15:21, Kukjin Kim wrote:
>> Tomasz Figa wrote:
>>>
>>> When CPU topology is specified in device tree, cpu_logical_map() does
>>> not return core ID anymore, but rather full MPIDR value. This breaks
>>> existing calculation of PMU register offsets on Exynos SoCs.
>>>
>>> This patch fixes the problem by adjusting the code to use only core ID
>>> bits of the value returned by cpu_logical_map() to allow CPU topology to
>>> be specified in device tree on Exynos SoCs.
>>>
>>> Signed-off-by: Tomasz Figa<[email protected]>
>>
>> Looks good to me, but I think we don't need this fix in 3.16 because the CPU
>> topology is not specified in DT yet...if I'm wrong, please correct me.
>
> CPU topology is already specified in DT for Exynos3250, 5250, 5260, 5410
> and 5420/5800.
>
> This patch also fixes CPU hotplug on SoCs with more than 2 cores,
> because it removes incorrect condition check in platform_do_lowpower().
>
> Please take this fix for next -rc release.
>
Agreed and I've applied this into fixes for 3.16...
Thanks,
Kukjin
On 07/08/14 22:54, Tomasz Figa wrote:
> On 08.07.2014 15:48, Kukjin Kim wrote:
>> Tomasz Figa wrote:
>>>
>>> Due to recently merged patches and previous merge conflicts, the Samsung
>>> PM Debug functionality no longer can be enabled. This patch fixes
>>> incorrect dependency of SAMSUNG_PM_DEBUG on an integer symbol and adds
>>> missing header inclusion.
>>>
>> Yes, you're right and it should be fixed...but I have comments...
>>
>>> Signed-off-by: Tomasz Figa<[email protected]>
>>> ---
>>> arch/arm/plat-samsung/Kconfig | 8 +++++++-
>>> arch/arm/plat-samsung/pm-debug.c | 1 +
>>> 2 files changed, 8 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm/plat-samsung/Kconfig b/arch/arm/plat-samsung/Kconfig
>>> index 301b892..483c959 100644
>>> --- a/arch/arm/plat-samsung/Kconfig
>>> +++ b/arch/arm/plat-samsung/Kconfig
>>> @@ -412,9 +412,14 @@ config S5P_DEV_MFC
>>>
>>> comment "Power management"
>>>
>>> +config HAVE_SAMSUNG_PM_DEBUG
>>> + bool
>>> + help
>>> + Allow compilation of Samsung PM debugging code.
>>> +
>>> config SAMSUNG_PM_DEBUG
>>> bool "S3C2410 PM Suspend debug"
>>> - depends on PM&& DEBUG_KERNEL&& DEBUG_S3C_UART
>>> + depends on PM&& DEBUG_KERNEL&& HAVE_SAMSUNG_PM_DEBUG
>>> help
>>> Say Y here if you want verbose debugging from the PM Suspend and
>>> Resume code. See<file:Documentation/arm/Samsung-S3C24XX/Suspend.txt>
>>> @@ -484,6 +489,7 @@ config S5P_SLEEP
>>>
>>> config DEBUG_S3C_UART
>>> depends on PLAT_SAMSUNG
>>> + select HAVE_SAMSUNG_PM_DEBUG
>>
>> Hmm...
>>
>> The DEBUG_S3C_UART will be '0' when we select DEBUG_S3C_UART0, then the
>> HAVE_SAMSUNG_PM_DEBUG will not be selected so SAMSUNG_PM_DEBUG is also...
>
> Yes, that's right. I posted v2 after a while in another reply to this
> thread.
>
Any updates? Or I missed your updated?
- Kukjin
On 15.07.2014 20:02, Kukjin Kim wrote:
> On 07/08/14 22:54, Tomasz Figa wrote:
>> On 08.07.2014 15:48, Kukjin Kim wrote:
>>> Tomasz Figa wrote:
>>>>
>>>> Due to recently merged patches and previous merge conflicts, the
>>>> Samsung
>>>> PM Debug functionality no longer can be enabled. This patch fixes
>>>> incorrect dependency of SAMSUNG_PM_DEBUG on an integer symbol and adds
>>>> missing header inclusion.
>>>>
>>> Yes, you're right and it should be fixed...but I have comments...
>>>
>>>> Signed-off-by: Tomasz Figa<[email protected]>
>>>> ---
>>>> arch/arm/plat-samsung/Kconfig | 8 +++++++-
>>>> arch/arm/plat-samsung/pm-debug.c | 1 +
>>>> 2 files changed, 8 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/arm/plat-samsung/Kconfig
>>>> b/arch/arm/plat-samsung/Kconfig
>>>> index 301b892..483c959 100644
>>>> --- a/arch/arm/plat-samsung/Kconfig
>>>> +++ b/arch/arm/plat-samsung/Kconfig
>>>> @@ -412,9 +412,14 @@ config S5P_DEV_MFC
>>>>
>>>> comment "Power management"
>>>>
>>>> +config HAVE_SAMSUNG_PM_DEBUG
>>>> + bool
>>>> + help
>>>> + Allow compilation of Samsung PM debugging code.
>>>> +
>>>> config SAMSUNG_PM_DEBUG
>>>> bool "S3C2410 PM Suspend debug"
>>>> - depends on PM&& DEBUG_KERNEL&& DEBUG_S3C_UART
>>>> + depends on PM&& DEBUG_KERNEL&& HAVE_SAMSUNG_PM_DEBUG
>>>> help
>>>> Say Y here if you want verbose debugging from the PM Suspend
>>>> and
>>>> Resume code.
>>>> See<file:Documentation/arm/Samsung-S3C24XX/Suspend.txt>
>>>> @@ -484,6 +489,7 @@ config S5P_SLEEP
>>>>
>>>> config DEBUG_S3C_UART
>>>> depends on PLAT_SAMSUNG
>>>> + select HAVE_SAMSUNG_PM_DEBUG
>>>
>>> Hmm...
>>>
>>> The DEBUG_S3C_UART will be '0' when we select DEBUG_S3C_UART0, then the
>>> HAVE_SAMSUNG_PM_DEBUG will not be selected so SAMSUNG_PM_DEBUG is
>>> also...
>>
>> Yes, that's right. I posted v2 after a while in another reply to this
>> thread.
>>
> Any updates? Or I missed your updated?
I'm afraid you missed. V2 has been there since a long time posted as a
reply to original patch. You can find it here:
https://lkml.org/lkml/2014/6/25/301
Best regards,
Tomasz
Hi Kukjin,
On 15.07.2014 16:26, Tomasz Figa wrote:
> Forgot to CC Daniel and linux-pm. Sorry for the noise.
>
> On 15.07.2014 16:24, Tomasz Figa wrote:
>> Due to recent consolidation of Exynos suspend and cpuidle code, some
>> parts of suspend and resume sequences are executed two times, once from
>> exynos_pm_syscore_ops and then from exynos_cpu_pm_notifier() and thus it
>> breaks suspend, at least on Exynos4-based boards. In addition, simple
>> core power down from a cpuidle driver could, in case of CPU 0 could
>> result in calling functions that are specific to suspend and deeper idle
>> states.
>>
>> This patch fixes the issue by moving those operations outside the CPU PM
>> notifier into suspend and AFTR code paths. This leads to a bit of code
>> duplication, but allows additional code simplification, so in the end
>> more code is removed than added.
>>
>> Signed-off-by: Tomasz Figa <[email protected]>
>> ---
>> arch/arm/mach-exynos/pm.c | 164 ++++++++++++++++++---------------------
>> drivers/cpuidle/cpuidle-exynos.c | 25 +-----
>> 2 files changed, 80 insertions(+), 109 deletions(-)
>>
Please consider this patch for next pull request with rc fixes. It
replaces following patches posted in this thread:
[PATCH 5/6] ARM: EXYNOS: Fix suspend/resume sequencies
[PATCH v2 5/6] ARM: EXYNOS: Fix suspend/resume sequences
and also similar patch by Chander:
[PATCH 2/2] cpuidle: Exynos: fix cpuidle for all states
Best regards,
Tomasz
Hi,
On 15.07.2014 16:26, Tomasz Figa wrote:
> Forgot to CC Daniel and linux-pm. Sorry for the noise.
>
> On 15.07.2014 16:24, Tomasz Figa wrote:
>> Due to recent consolidation of Exynos suspend and cpuidle code, some
>> parts of suspend and resume sequences are executed two times, once from
>> exynos_pm_syscore_ops and then from exynos_cpu_pm_notifier() and thus it
>> breaks suspend, at least on Exynos4-based boards. In addition, simple
>> core power down from a cpuidle driver could, in case of CPU 0 could
>> result in calling functions that are specific to suspend and deeper idle
>> states.
>>
>> This patch fixes the issue by moving those operations outside the CPU PM
>> notifier into suspend and AFTR code paths. This leads to a bit of code
>> duplication, but allows additional code simplification, so in the end
>> more code is removed than added.
>>
>> Signed-off-by: Tomasz Figa <[email protected]>
>> ---
>> arch/arm/mach-exynos/pm.c | 164 ++++++++++++++++++---------------------
>> drivers/cpuidle/cpuidle-exynos.c | 25 +-----
>> 2 files changed, 80 insertions(+), 109 deletions(-)
>>
This is an important regression fix for 3.16 and today 3.16-rc6 was
already released...
Kukjin, could you pick up this patch and few others you missed and make
sure that it is included in 3.16-rc7? If you are busy, maybe Arnd or
Olof could take them directly?
Best regards,
Tomasz
On 16.07.2014 01:59, Tomasz Figa wrote:
> On 15.07.2014 20:02, Kukjin Kim wrote:
>> On 07/08/14 22:54, Tomasz Figa wrote:
>>> On 08.07.2014 15:48, Kukjin Kim wrote:
>>>> Tomasz Figa wrote:
>>>>>
>>>>> Due to recently merged patches and previous merge conflicts, the
>>>>> Samsung
>>>>> PM Debug functionality no longer can be enabled. This patch fixes
>>>>> incorrect dependency of SAMSUNG_PM_DEBUG on an integer symbol and adds
>>>>> missing header inclusion.
>>>>>
>>>> Yes, you're right and it should be fixed...but I have comments...
>>>>
>>>>> Signed-off-by: Tomasz Figa<[email protected]>
>>>>> ---
>>>>> arch/arm/plat-samsung/Kconfig | 8 +++++++-
>>>>> arch/arm/plat-samsung/pm-debug.c | 1 +
>>>>> 2 files changed, 8 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/arch/arm/plat-samsung/Kconfig
>>>>> b/arch/arm/plat-samsung/Kconfig
>>>>> index 301b892..483c959 100644
>>>>> --- a/arch/arm/plat-samsung/Kconfig
>>>>> +++ b/arch/arm/plat-samsung/Kconfig
>>>>> @@ -412,9 +412,14 @@ config S5P_DEV_MFC
>>>>>
>>>>> comment "Power management"
>>>>>
>>>>> +config HAVE_SAMSUNG_PM_DEBUG
>>>>> + bool
>>>>> + help
>>>>> + Allow compilation of Samsung PM debugging code.
>>>>> +
>>>>> config SAMSUNG_PM_DEBUG
>>>>> bool "S3C2410 PM Suspend debug"
>>>>> - depends on PM&& DEBUG_KERNEL&& DEBUG_S3C_UART
>>>>> + depends on PM&& DEBUG_KERNEL&& HAVE_SAMSUNG_PM_DEBUG
>>>>> help
>>>>> Say Y here if you want verbose debugging from the PM Suspend
>>>>> and
>>>>> Resume code.
>>>>> See<file:Documentation/arm/Samsung-S3C24XX/Suspend.txt>
>>>>> @@ -484,6 +489,7 @@ config S5P_SLEEP
>>>>>
>>>>> config DEBUG_S3C_UART
>>>>> depends on PLAT_SAMSUNG
>>>>> + select HAVE_SAMSUNG_PM_DEBUG
>>>>
>>>> Hmm...
>>>>
>>>> The DEBUG_S3C_UART will be '0' when we select DEBUG_S3C_UART0, then the
>>>> HAVE_SAMSUNG_PM_DEBUG will not be selected so SAMSUNG_PM_DEBUG is
>>>> also...
>>>
>>> Yes, that's right. I posted v2 after a while in another reply to this
>>> thread.
>>>
>> Any updates? Or I missed your updated?
>
> I'm afraid you missed. V2 has been there since a long time posted as a
> reply to original patch. You can find it here:
>
> https://lkml.org/lkml/2014/6/25/301
Ping.
Best regards,
Tomasz