2017-04-26 16:06:38

by Alexandre Belloni

[permalink] [raw]
Subject: [PATCH 1/3] ARM: at91: pm: Add sama5d2 backup mode

The sama5d2 has a mode were it is possible to cut power to the SoC while
keeping the RAM in self refresh.
Resuming from that mode needs support in the firmware/bootloader.

Signed-off-by: Alexandre Belloni <[email protected]>
---
arch/arm/mach-at91/Makefile | 4 ++
arch/arm/mach-at91/generic.h | 2 +
arch/arm/mach-at91/pm.c | 103 ++++++++++++++++++++++++++++++++++-
arch/arm/mach-at91/pm.h | 4 ++
arch/arm/mach-at91/pm_data-offsets.c | 3 +
arch/arm/mach-at91/pm_suspend.S | 86 ++++++++++++++++++++++-------
arch/arm/mach-at91/sama5.c | 19 ++++++-
7 files changed, 198 insertions(+), 23 deletions(-)

diff --git a/arch/arm/mach-at91/Makefile b/arch/arm/mach-at91/Makefile
index cfd8f60a9268..87fe17dbdb56 100644
--- a/arch/arm/mach-at91/Makefile
+++ b/arch/arm/mach-at91/Makefile
@@ -14,6 +14,10 @@ obj-$(CONFIG_PM) += pm_suspend.o
ifeq ($(CONFIG_CPU_V7),y)
AFLAGS_pm_suspend.o := -march=armv7-a
endif
+# Backup mode will not compile for ARMv5 because of movt
+ifeq ($(CONFIG_SOC_SAMA5D2),y)
+AFLAGS_pm_suspend.o += -DBACKUP_MODE
+endif
ifeq ($(CONFIG_PM_DEBUG),y)
CFLAGS_pm.o += -DDEBUG
endif
diff --git a/arch/arm/mach-at91/generic.h b/arch/arm/mach-at91/generic.h
index f1ead0f13c19..e2bd17237964 100644
--- a/arch/arm/mach-at91/generic.h
+++ b/arch/arm/mach-at91/generic.h
@@ -15,10 +15,12 @@
extern void __init at91rm9200_pm_init(void);
extern void __init at91sam9_pm_init(void);
extern void __init sama5_pm_init(void);
+extern void __init sama5d2_pm_init(void);
#else
static inline void __init at91rm9200_pm_init(void) { }
static inline void __init at91sam9_pm_init(void) { }
static inline void __init sama5_pm_init(void) { }
+static inline void __init sama5d2_pm_init(void) { }
#endif

#endif /* _AT91_GENERIC_H */
diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
index 2cd27c830ab6..1e03f1277f14 100644
--- a/arch/arm/mach-at91/pm.c
+++ b/arch/arm/mach-at91/pm.c
@@ -22,6 +22,7 @@
#include <asm/cacheflush.h>
#include <asm/fncpy.h>
#include <asm/system_misc.h>
+#include <asm/suspend.h>

#include "generic.h"
#include "pm.h"
@@ -58,6 +59,14 @@ static int at91_pm_valid_state(suspend_state_t state)
}
}

+static int canary = 0xA5A5A5A5;
+
+static struct at91_pm_bu {
+ int suspended;
+ unsigned long reserved;
+ phys_addr_t canary;
+ phys_addr_t resume;
+} *pm_bu;

static suspend_state_t target_state;

@@ -123,15 +132,39 @@ static void (*at91_suspend_sram_fn)(struct at91_pm_data *);
extern void at91_pm_suspend_in_sram(struct at91_pm_data *pm_data);
extern u32 at91_pm_suspend_in_sram_sz;

-static void at91_pm_suspend(suspend_state_t state)
+static int at91_suspend_finish(unsigned long val)
{
- pm_data.mode = (state == PM_SUSPEND_MEM) ? AT91_PM_SLOW_CLOCK : 0;
-
flush_cache_all();
outer_disable();

at91_suspend_sram_fn(&pm_data);

+ return 0;
+}
+
+static void at91_pm_suspend(suspend_state_t state)
+{
+ if (pm_data.deepest_state == AT91_PM_BACKUP)
+ if (state == PM_SUSPEND_MEM)
+ pm_data.mode = AT91_PM_BACKUP;
+ else
+ pm_data.mode = AT91_PM_SLOW_CLOCK;
+ else
+ pm_data.mode = (state == PM_SUSPEND_MEM) ? AT91_PM_SLOW_CLOCK : 0;
+
+ if (pm_data.mode == AT91_PM_BACKUP) {
+ pm_bu->suspended = 1;
+
+ cpu_suspend(0, at91_suspend_finish);
+
+ /* The SRAM is lost between suspend cycles */
+ at91_suspend_sram_fn = fncpy(at91_suspend_sram_fn,
+ &at91_pm_suspend_in_sram,
+ at91_pm_suspend_in_sram_sz);
+ } else {
+ at91_suspend_finish(0);
+ }
+
outer_resume();
}

@@ -375,6 +408,25 @@ static __init void at91_dt_ramc(void)
at91_cpuidle_device.dev.platform_data = standby;
}

+static __init void at91_dt_shdwc(void)
+{
+ struct device_node *np;
+
+ np = of_find_compatible_node(NULL, NULL, "atmel,sama5d2-shdwc");
+ if (!np)
+ return;
+
+ pm_data.shdwc = of_iomap(np, 0);
+ of_node_put(np);
+
+ np = of_find_compatible_node(NULL, NULL, "atmel,sama5d2-sfrbu");
+ if (!np)
+ return;
+
+ pm_data.sfrbu = of_iomap(np, 0);
+ of_node_put(np);
+}
+
static void at91rm9200_idle(void)
{
/*
@@ -436,6 +488,44 @@ static void __init at91_pm_sram_init(void)
&at91_pm_suspend_in_sram, at91_pm_suspend_in_sram_sz);
}

+static void __init at91_pm_bu_sram_init(void)
+{
+ struct gen_pool *sram_pool;
+ struct device_node *node;
+ struct platform_device *pdev = NULL;
+
+ pm_bu = NULL;
+
+ for_each_compatible_node(node, NULL, "atmel,sama5d2-securam") {
+ pdev = of_find_device_by_node(node);
+ if (pdev) {
+ of_node_put(node);
+ break;
+ }
+ }
+
+ if (!pdev) {
+ pr_warn("%s: failed to find securam device!\n", __func__);
+ return;
+ }
+
+ sram_pool = gen_pool_get(&pdev->dev, NULL);
+ if (!sram_pool) {
+ pr_warn("%s: securam pool unavailable!\n", __func__);
+ return;
+ }
+
+ pm_bu = (void *)gen_pool_alloc(sram_pool, sizeof(struct at91_pm_bu));
+ if (!pm_bu) {
+ pr_warn("%s: unable to alloc securam!\n", __func__);
+ return;
+ }
+
+ pm_bu->suspended = 0;
+ pm_bu->canary = virt_to_phys(&canary);
+ pm_bu->resume = virt_to_phys(cpu_resume);
+}
+
struct pmc_info {
unsigned long uhp_udp_mask;
};
@@ -510,3 +600,10 @@ void __init sama5_pm_init(void)
at91_dt_ramc();
at91_pm_init(NULL);
}
+
+void __init sama5d2_pm_init(void)
+{
+ at91_dt_shdwc();
+ at91_pm_bu_sram_init();
+ sama5_pm_init();
+}
diff --git a/arch/arm/mach-at91/pm.h b/arch/arm/mach-at91/pm.h
index fc0f7d048187..d9c6612ef62f 100644
--- a/arch/arm/mach-at91/pm.h
+++ b/arch/arm/mach-at91/pm.h
@@ -22,6 +22,7 @@
#define AT91_MEMCTRL_DDRSDR 2

#define AT91_PM_SLOW_CLOCK 0x01
+#define AT91_PM_BACKUP 0x02

#ifndef __ASSEMBLY__
struct at91_pm_data {
@@ -30,6 +31,9 @@ struct at91_pm_data {
unsigned long uhp_udp_mask;
unsigned int memctrl;
unsigned int mode;
+ void __iomem *shdwc;
+ void __iomem *sfrbu;
+ unsigned int deepest_state;
};
#endif

diff --git a/arch/arm/mach-at91/pm_data-offsets.c b/arch/arm/mach-at91/pm_data-offsets.c
index 30302cb16df0..c0a73e62b725 100644
--- a/arch/arm/mach-at91/pm_data-offsets.c
+++ b/arch/arm/mach-at91/pm_data-offsets.c
@@ -9,5 +9,8 @@ int main(void)
DEFINE(PM_DATA_RAMC1, offsetof(struct at91_pm_data, ramc[1]));
DEFINE(PM_DATA_MEMCTRL, offsetof(struct at91_pm_data, memctrl));
DEFINE(PM_DATA_MODE, offsetof(struct at91_pm_data, mode));
+ DEFINE(PM_DATA_SHDWC, offsetof(struct at91_pm_data, shdwc));
+ DEFINE(PM_DATA_SFRBU, offsetof(struct at91_pm_data, sfrbu));
+
return 0;
}
diff --git a/arch/arm/mach-at91/pm_suspend.S b/arch/arm/mach-at91/pm_suspend.S
index 96781daa671a..b5ffa8e1f203 100644
--- a/arch/arm/mach-at91/pm_suspend.S
+++ b/arch/arm/mach-at91/pm_suspend.S
@@ -97,15 +97,74 @@ ENTRY(at91_pm_suspend_in_sram)
str tmp1, .memtype
ldr tmp1, [r0, #PM_DATA_MODE]
str tmp1, .pm_mode
+ ldr tmp1, [r0, #PM_DATA_SHDWC]
+#if defined(BACKUP_MODE)
+ str tmp1, .shdwc
+ cmp tmp1, #0
+ ldrne tmp2, [tmp1, #0]
+ ldr tmp1, [r0, #PM_DATA_SFRBU]
+ str tmp1, .sfr
+ cmp tmp1, #0
+ ldrne tmp2, [tmp1, #0x10]
+#endif

/* Active the self-refresh mode */
mov r0, #SRAMC_SELF_FRESH_ACTIVE
bl at91_sramc_self_refresh

ldr r0, .pm_mode
- tst r0, #AT91_PM_SLOW_CLOCK
- beq skip_disable_main_clock
+ cmp r0, #AT91_PM_SLOW_CLOCK
+ beq slow_clock
+#if defined(BACKUP_MODE)
+ cmp r0, #AT91_PM_BACKUP
+ beq backup_mode
+#endif

+ /* Wait for interrupt */
+ ldr pmc, .pmc_base
+ at91_cpu_idle
+ b exit_suspend
+
+slow_clock:
+ bl at91_slowck_mode
+ b exit_suspend
+#if defined(BACKUP_MODE)
+backup_mode:
+ bl at91_backup_mode
+ b exit_suspend
+#endif
+
+exit_suspend:
+ /* Exit the self-refresh mode */
+ mov r0, #SRAMC_SELF_FRESH_EXIT
+ bl at91_sramc_self_refresh
+
+ /* Restore registers, and return */
+ ldmfd sp!, {r4 - r12, pc}
+ENDPROC(at91_pm_suspend_in_sram)
+
+#if defined(BACKUP_MODE)
+ENTRY(at91_backup_mode)
+ #if 0
+ /* Read LPR */
+ ldr r2, .sramc_base
+ ldr r3, [r2, #AT91_DDRSDRC_LPR]
+ #endif
+
+ /*BUMEN*/
+ ldr r0, .sfr
+ mov tmp1, #(0x1)
+ str tmp1, [r0, #0x10]
+
+ /* Shutdown */
+ ldr r0, .shdwc
+ movw tmp1, #0x1
+ movt tmp1, #0xA500
+ str tmp1, [r0, #0]
+ENDPROC(at91_backup_mode)
+#endif
+
+ENTRY(at91_slowck_mode)
ldr pmc, .pmc_base

/* Save Master clock setting */
@@ -134,18 +193,9 @@ ENTRY(at91_pm_suspend_in_sram)
orr tmp1, tmp1, #AT91_PMC_KEY
str tmp1, [pmc, #AT91_CKGR_MOR]

-skip_disable_main_clock:
- ldr pmc, .pmc_base
-
/* Wait for interrupt */
at91_cpu_idle

- ldr r0, .pm_mode
- tst r0, #AT91_PM_SLOW_CLOCK
- beq skip_enable_main_clock
-
- ldr pmc, .pmc_base
-
/* Turn on the main oscillator */
ldr tmp1, [pmc, #AT91_CKGR_MOR]
orr tmp1, tmp1, #AT91_PMC_MOSCEN
@@ -174,14 +224,8 @@ skip_disable_main_clock:

wait_mckrdy

-skip_enable_main_clock:
- /* Exit the self-refresh mode */
- mov r0, #SRAMC_SELF_FRESH_EXIT
- bl at91_sramc_self_refresh
-
- /* Restore registers, and return */
- ldmfd sp!, {r4 - r12, pc}
-ENDPROC(at91_pm_suspend_in_sram)
+ mov pc, lr
+ENDPROC(at91_slowck_mode)

/*
* void at91_sramc_self_refresh(unsigned int is_active)
@@ -314,6 +358,10 @@ ENDPROC(at91_sramc_self_refresh)
.word 0
.sramc1_base:
.word 0
+.shdwc:
+ .word 0
+.sfr:
+ .word 0
.memtype:
.word 0
.pm_mode:
diff --git a/arch/arm/mach-at91/sama5.c b/arch/arm/mach-at91/sama5.c
index 6d157d0ead8e..3d0bf95a56ae 100644
--- a/arch/arm/mach-at91/sama5.c
+++ b/arch/arm/mach-at91/sama5.c
@@ -34,7 +34,6 @@ DT_MACHINE_START(sama5_dt, "Atmel SAMA5")
MACHINE_END

static const char *const sama5_alt_dt_board_compat[] __initconst = {
- "atmel,sama5d2",
"atmel,sama5d4",
NULL
};
@@ -45,3 +44,21 @@ DT_MACHINE_START(sama5_alt_dt, "Atmel SAMA5")
.dt_compat = sama5_alt_dt_board_compat,
.l2c_aux_mask = ~0UL,
MACHINE_END
+
+static void __init sama5d2_init(void)
+{
+ of_platform_default_populate(NULL, NULL, NULL);
+ sama5d2_pm_init();
+}
+
+static const char *const sama5d2_compat[] __initconst = {
+ "atmel,sama5d2",
+ NULL
+};
+
+DT_MACHINE_START(sama5d2, "Atmel SAMA5")
+ /* Maintainer: Atmel */
+ .init_machine = sama5d2_init,
+ .dt_compat = sama5d2_compat,
+ .l2c_aux_mask = ~0UL,
+MACHINE_END
--
2.11.0


2017-04-26 16:04:55

by Alexandre Belloni

[permalink] [raw]
Subject: [PATCH 2/3] ARM: at91: pm: allow selecting standby and suspend modes

While we can only select between "standby" and "mem" states for power
management, the atmel platforms can actually support more modes.

For both standby and mem, allow selecting which mode will be used using the
atmel.pm_modes kernel parameter.
By default, keep the current modes.

Signed-off-by: Alexandre Belloni <[email protected]>
---
arch/arm/mach-at91/pm.c | 110 +++++++++++++++++++++++++++++++++---------------
arch/arm/mach-at91/pm.h | 3 +-
2 files changed, 78 insertions(+), 35 deletions(-)

diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
index 1e03f1277f14..d08f032f9d94 100644
--- a/arch/arm/mach-at91/pm.c
+++ b/arch/arm/mach-at91/pm.c
@@ -15,6 +15,7 @@
#include <linux/of_address.h>
#include <linux/of.h>
#include <linux/of_platform.h>
+#include <linux/parser.h>
#include <linux/suspend.h>

#include <linux/clk/at91_pmc.h>
@@ -38,7 +39,17 @@ extern void at91_pinctrl_gpio_suspend(void);
extern void at91_pinctrl_gpio_resume(void);
#endif

-static struct at91_pm_data pm_data;
+static const match_table_t pm_modes __initconst = {
+ { 0, "standby" },
+ { AT91_PM_SLOW_CLOCK, "ulp0" },
+ { AT91_PM_BACKUP, "backup" },
+ { -1, NULL },
+};
+
+static struct at91_pm_data pm_data = {
+ .standby_mode = 0,
+ .suspend_mode = AT91_PM_SLOW_CLOCK,
+};

#define at91_ramc_read(id, field) \
__raw_readl(pm_data.ramc[id] + field)
@@ -68,14 +79,24 @@ static struct at91_pm_bu {
phys_addr_t resume;
} *pm_bu;

-static suspend_state_t target_state;
-
/*
* Called after processes are frozen, but before we shutdown devices.
*/
static int at91_pm_begin(suspend_state_t state)
{
- target_state = state;
+ switch (state) {
+ case PM_SUSPEND_MEM:
+ pm_data.mode = pm_data.suspend_mode;
+ break;
+
+ case PM_SUSPEND_STANDBY:
+ pm_data.mode = pm_data.standby_mode;
+ break;
+
+ default:
+ pm_data.mode = -1;
+ }
+
return 0;
}

@@ -124,7 +145,7 @@ static int at91_pm_verify_clocks(void)
*/
int at91_suspend_entering_slow_clock(void)
{
- return (target_state == PM_SUSPEND_MEM);
+ return (pm_data.mode >= AT91_PM_SLOW_CLOCK);
}
EXPORT_SYMBOL(at91_suspend_entering_slow_clock);

@@ -144,14 +165,6 @@ static int at91_suspend_finish(unsigned long val)

static void at91_pm_suspend(suspend_state_t state)
{
- if (pm_data.deepest_state == AT91_PM_BACKUP)
- if (state == PM_SUSPEND_MEM)
- pm_data.mode = AT91_PM_BACKUP;
- else
- pm_data.mode = AT91_PM_SLOW_CLOCK;
- else
- pm_data.mode = (state == PM_SUSPEND_MEM) ? AT91_PM_SLOW_CLOCK : 0;
-
if (pm_data.mode == AT91_PM_BACKUP) {
pm_bu->suspended = 1;

@@ -168,38 +181,37 @@ static void at91_pm_suspend(suspend_state_t state)
outer_resume();
}

+/*
+ * STANDBY mode has *all* drivers suspended; ignores irqs not marked as 'wakeup'
+ * event sources; and reduces DRAM power. But otherwise it's identical to
+ * PM_SUSPEND_ON: cpu idle, and nothing fancy done with main or cpu clocks.
+ *
+ * AT91_PM_SLOW_CLOCK is like STANDBY plus slow clock mode, so drivers must
+ * suspend more deeply, the master clock switches to the clk32k and turns off
+ * the main oscillator
+ *
+ * AT91_PM_BACKUP turns off the whole SoC after placing the DDR in self refresh
+ */
static int at91_pm_enter(suspend_state_t state)
{
#ifdef CONFIG_PINCTRL_AT91
at91_pinctrl_gpio_suspend();
#endif
+
switch (state) {
- /*
- * Suspend-to-RAM is like STANDBY plus slow clock mode, so
- * drivers must suspend more deeply, the master clock switches
- * to the clk32k and turns off the main oscillator
- */
case PM_SUSPEND_MEM:
+ case PM_SUSPEND_STANDBY:
/*
* Ensure that clocks are in a valid state.
*/
- if (!at91_pm_verify_clocks())
+ if ((pm_data.mode >= AT91_PM_SLOW_CLOCK) &&
+ !at91_pm_verify_clocks())
goto error;

at91_pm_suspend(state);

break;

- /*
- * STANDBY mode has *all* drivers suspended; ignores irqs not
- * marked as 'wakeup' event sources; and reduces DRAM power.
- * But otherwise it's identical to PM_SUSPEND_ON: cpu idle, and
- * nothing fancy done with main or cpu clocks.
- */
- case PM_SUSPEND_STANDBY:
- at91_pm_suspend(state);
- break;
-
case PM_SUSPEND_ON:
cpu_do_idle();
break;
@@ -210,8 +222,6 @@ static int at91_pm_enter(suspend_state_t state)
}

error:
- target_state = PM_SUSPEND_ON;
-
#ifdef CONFIG_PINCTRL_AT91
at91_pinctrl_gpio_resume();
#endif
@@ -223,7 +233,6 @@ static int at91_pm_enter(suspend_state_t state)
*/
static void at91_pm_end(void)
{
- target_state = PM_SUSPEND_ON;
}


@@ -494,6 +503,10 @@ static void __init at91_pm_bu_sram_init(void)
struct device_node *node;
struct platform_device *pdev = NULL;

+ if ((pm_data.standby_mode != AT91_PM_BACKUP) &&
+ (pm_data.suspend_mode != AT91_PM_BACKUP))
+ return;
+
pm_bu = NULL;

for_each_compatible_node(node, NULL, "atmel,sama5d2-securam") {
@@ -571,10 +584,14 @@ static void __init at91_pm_init(void (*pm_idle)(void))

at91_pm_sram_init();

- if (at91_suspend_sram_fn)
+ if (at91_suspend_sram_fn) {
suspend_set_ops(&at91_pm_ops);
- else
+ pr_info("AT91: PM: standby: %s, suspend: %s\n",
+ pm_modes[pm_data.standby_mode].pattern,
+ pm_modes[pm_data.suspend_mode].pattern);
+ } else {
pr_info("AT91: PM not supported, due to no SRAM allocated\n");
+ }
}

void __init at91rm9200_pm_init(void)
@@ -607,3 +624,28 @@ void __init sama5d2_pm_init(void)
at91_pm_bu_sram_init();
sama5_pm_init();
}
+
+static int __init at91_pm_modes_select(char *str)
+{
+ char *s;
+ substring_t args[MAX_OPT_ARGS];
+ int standby, suspend;
+
+ if (!str)
+ return 0;
+
+ s = strsep(&str, ",");
+ standby = match_token(s, pm_modes, args);
+ if (standby < 0)
+ return 0;
+
+ suspend = match_token(str, pm_modes, args);
+ if (suspend < 0)
+ return 0;
+
+ pm_data.standby_mode = standby;
+ pm_data.suspend_mode = suspend;
+
+ return 0;
+}
+early_param("atmel.pm_modes", at91_pm_modes_select);
diff --git a/arch/arm/mach-at91/pm.h b/arch/arm/mach-at91/pm.h
index d9c6612ef62f..f95d31496f08 100644
--- a/arch/arm/mach-at91/pm.h
+++ b/arch/arm/mach-at91/pm.h
@@ -33,7 +33,8 @@ struct at91_pm_data {
unsigned int mode;
void __iomem *shdwc;
void __iomem *sfrbu;
- unsigned int deepest_state;
+ unsigned int standby_mode;
+ unsigned int suspend_mode;
};
#endif

--
2.11.0

2017-04-26 16:05:05

by Alexandre Belloni

[permalink] [raw]
Subject: [PATCH 3/3] ARM: at91: pm: fallback to slowclock when backup mode fails

If the backup sram allocation fails, ensure we can suspend by falling back
to the usual slow clock mode.

Signed-off-by: Alexandre Belloni <[email protected]>
---
arch/arm/mach-at91/pm.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
index d08f032f9d94..02823d8f3ada 100644
--- a/arch/arm/mach-at91/pm.c
+++ b/arch/arm/mach-at91/pm.c
@@ -519,24 +519,30 @@ static void __init at91_pm_bu_sram_init(void)

if (!pdev) {
pr_warn("%s: failed to find securam device!\n", __func__);
- return;
+ goto fallback;
}

sram_pool = gen_pool_get(&pdev->dev, NULL);
if (!sram_pool) {
pr_warn("%s: securam pool unavailable!\n", __func__);
- return;
+ goto fallback;
}

pm_bu = (void *)gen_pool_alloc(sram_pool, sizeof(struct at91_pm_bu));
if (!pm_bu) {
pr_warn("%s: unable to alloc securam!\n", __func__);
- return;
+ goto fallback;
}

pm_bu->suspended = 0;
pm_bu->canary = virt_to_phys(&canary);
pm_bu->resume = virt_to_phys(cpu_resume);
+
+fallback:
+ if (pm_data.standby_mode == AT91_PM_BACKUP)
+ pm_data.standby_mode = AT91_PM_SLOW_CLOCK;
+ if (pm_data.suspend_mode != AT91_PM_BACKUP)
+ pm_data.suspend_mode = AT91_PM_SLOW_CLOCK;
}

struct pmc_info {
--
2.11.0

2017-04-27 13:34:21

by Romain Izard

[permalink] [raw]
Subject: Re: [PATCH 1/3] ARM: at91: pm: Add sama5d2 backup mode

Hello Alexandre,

This series might also be of interest for the linux-pm mailing list.

2017-04-26 18:04 GMT+02:00 Alexandre Belloni
<[email protected]>:
> The sama5d2 has a mode were it is possible to cut power to the SoC while
> keeping the RAM in self refresh.
> Resuming from that mode needs support in the firmware/bootloader.
>
> Signed-off-by: Alexandre Belloni <[email protected]>
> ---
> arch/arm/mach-at91/Makefile | 4 ++
> arch/arm/mach-at91/generic.h | 2 +
> arch/arm/mach-at91/pm.c | 103 ++++++++++++++++++++++++++++++++++-
> arch/arm/mach-at91/pm.h | 4 ++
> arch/arm/mach-at91/pm_data-offsets.c | 3 +
> arch/arm/mach-at91/pm_suspend.S | 86 ++++++++++++++++++++++-------
> arch/arm/mach-at91/sama5.c | 19 ++++++-
> 7 files changed, 198 insertions(+), 23 deletions(-)
>
> diff --git a/arch/arm/mach-at91/Makefile b/arch/arm/mach-at91/Makefile
> index cfd8f60a9268..87fe17dbdb56 100644
> --- a/arch/arm/mach-at91/Makefile
> +++ b/arch/arm/mach-at91/Makefile
> @@ -14,6 +14,10 @@ obj-$(CONFIG_PM) += pm_suspend.o
> ifeq ($(CONFIG_CPU_V7),y)
> AFLAGS_pm_suspend.o := -march=armv7-a
> endif
> +# Backup mode will not compile for ARMv5 because of movt
> +ifeq ($(CONFIG_SOC_SAMA5D2),y)
> +AFLAGS_pm_suspend.o += -DBACKUP_MODE
> +endif
> ifeq ($(CONFIG_PM_DEBUG),y)
> CFLAGS_pm.o += -DDEBUG
> endif

We can rewrite the assembly to avoid using movt, and remove some ifdefs
from the code.


> diff --git a/arch/arm/mach-at91/generic.h b/arch/arm/mach-at91/generic.h
> index f1ead0f13c19..e2bd17237964 100644
> --- a/arch/arm/mach-at91/generic.h
> +++ b/arch/arm/mach-at91/generic.h
> @@ -15,10 +15,12 @@
> extern void __init at91rm9200_pm_init(void);
> extern void __init at91sam9_pm_init(void);
> extern void __init sama5_pm_init(void);
> +extern void __init sama5d2_pm_init(void);
> #else
> static inline void __init at91rm9200_pm_init(void) { }
> static inline void __init at91sam9_pm_init(void) { }
> static inline void __init sama5_pm_init(void) { }
> +static inline void __init sama5d2_pm_init(void) { }
> #endif
>
> #endif /* _AT91_GENERIC_H */
> diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
> index 2cd27c830ab6..1e03f1277f14 100644
> --- a/arch/arm/mach-at91/pm.c
> +++ b/arch/arm/mach-at91/pm.c
> @@ -22,6 +22,7 @@
> #include <asm/cacheflush.h>
> #include <asm/fncpy.h>
> #include <asm/system_misc.h>
> +#include <asm/suspend.h>
>
> #include "generic.h"
> #include "pm.h"
> @@ -58,6 +59,14 @@ static int at91_pm_valid_state(suspend_state_t state)
> }
> }
>
> +static int canary = 0xA5A5A5A5;
> +
> +static struct at91_pm_bu {
> + int suspended;
> + unsigned long reserved;
> + phys_addr_t canary;
> + phys_addr_t resume;
> +} *pm_bu;
>
> static suspend_state_t target_state;
>
> @@ -123,15 +132,39 @@ static void (*at91_suspend_sram_fn)(struct at91_pm_data *);
> extern void at91_pm_suspend_in_sram(struct at91_pm_data *pm_data);
> extern u32 at91_pm_suspend_in_sram_sz;
>
> -static void at91_pm_suspend(suspend_state_t state)
> +static int at91_suspend_finish(unsigned long val)
> {
> - pm_data.mode = (state == PM_SUSPEND_MEM) ? AT91_PM_SLOW_CLOCK : 0;
> -
> flush_cache_all();
> outer_disable();
>
> at91_suspend_sram_fn(&pm_data);
>
> + return 0;
> +}
> +
> +static void at91_pm_suspend(suspend_state_t state)
> +{
> + if (pm_data.deepest_state == AT91_PM_BACKUP)
> + if (state == PM_SUSPEND_MEM)
> + pm_data.mode = AT91_PM_BACKUP;
> + else
> + pm_data.mode = AT91_PM_SLOW_CLOCK;
> + else
> + pm_data.mode = (state == PM_SUSPEND_MEM) ? AT91_PM_SLOW_CLOCK : 0;
> +
> + if (pm_data.mode == AT91_PM_BACKUP) {
> + pm_bu->suspended = 1;
> +
> + cpu_suspend(0, at91_suspend_finish);
> +
> + /* The SRAM is lost between suspend cycles */
> + at91_suspend_sram_fn = fncpy(at91_suspend_sram_fn,
> + &at91_pm_suspend_in_sram,
> + at91_pm_suspend_in_sram_sz);
> + } else {
> + at91_suspend_finish(0);
> + }
> +
> outer_resume();
> }
>
> @@ -375,6 +408,25 @@ static __init void at91_dt_ramc(void)
> at91_cpuidle_device.dev.platform_data = standby;
> }
>
> +static __init void at91_dt_shdwc(void)
> +{
> + struct device_node *np;
> +
> + np = of_find_compatible_node(NULL, NULL, "atmel,sama5d2-shdwc");
> + if (!np)
> + return;
> +
> + pm_data.shdwc = of_iomap(np, 0);
> + of_node_put(np);
> +
> + np = of_find_compatible_node(NULL, NULL, "atmel,sama5d2-sfrbu");
> + if (!np)
> + return;
> +
> + pm_data.sfrbu = of_iomap(np, 0);
> + of_node_put(np);
> +}
> +
> static void at91rm9200_idle(void)
> {
> /*
> @@ -436,6 +488,44 @@ static void __init at91_pm_sram_init(void)
> &at91_pm_suspend_in_sram, at91_pm_suspend_in_sram_sz);
> }
>
> +static void __init at91_pm_bu_sram_init(void)
> +{
> + struct gen_pool *sram_pool;
> + struct device_node *node;
> + struct platform_device *pdev = NULL;
> +
> + pm_bu = NULL;
> +
> + for_each_compatible_node(node, NULL, "atmel,sama5d2-securam") {
> + pdev = of_find_device_by_node(node);
> + if (pdev) {
> + of_node_put(node);
> + break;
> + }
> + }
> +

Do we really need to iterate over compatible nodes ?

> + if (!pdev) {
> + pr_warn("%s: failed to find securam device!\n", __func__);
> + return;
> + }
> +
> + sram_pool = gen_pool_get(&pdev->dev, NULL);
> + if (!sram_pool) {
> + pr_warn("%s: securam pool unavailable!\n", __func__);
> + return;
> + }
> +
> + pm_bu = (void *)gen_pool_alloc(sram_pool, sizeof(struct at91_pm_bu));
> + if (!pm_bu) {
> + pr_warn("%s: unable to alloc securam!\n", __func__);
> + return;
> + }
> +
> + pm_bu->suspended = 0;
> + pm_bu->canary = virt_to_phys(&canary);
> + pm_bu->resume = virt_to_phys(cpu_resume);
> +}
> +

at91_pm_bu_sram_init and at91_dt_shdwc are necessary to use backup mode.
But those functions do not return error codes, and do no cleanup in case
of error. I believe that it would be simpler if we only had a single
function.

> struct pmc_info {
> unsigned long uhp_udp_mask;
> };
> @@ -510,3 +600,10 @@ void __init sama5_pm_init(void)
> at91_dt_ramc();
> at91_pm_init(NULL);
> }
> +
> +void __init sama5d2_pm_init(void)
> +{
> + at91_dt_shdwc();
> + at91_pm_bu_sram_init();
> + sama5_pm_init();
> +}
> diff --git a/arch/arm/mach-at91/pm.h b/arch/arm/mach-at91/pm.h
> index fc0f7d048187..d9c6612ef62f 100644
> --- a/arch/arm/mach-at91/pm.h
> +++ b/arch/arm/mach-at91/pm.h
> @@ -22,6 +22,7 @@
> #define AT91_MEMCTRL_DDRSDR 2
>
> #define AT91_PM_SLOW_CLOCK 0x01
> +#define AT91_PM_BACKUP 0x02
>
> #ifndef __ASSEMBLY__
> struct at91_pm_data {
> @@ -30,6 +31,9 @@ struct at91_pm_data {
> unsigned long uhp_udp_mask;
> unsigned int memctrl;
> unsigned int mode;
> + void __iomem *shdwc;
> + void __iomem *sfrbu;
> + unsigned int deepest_state;
> };
> #endif
>
> diff --git a/arch/arm/mach-at91/pm_data-offsets.c b/arch/arm/mach-at91/pm_data-offsets.c
> index 30302cb16df0..c0a73e62b725 100644
> --- a/arch/arm/mach-at91/pm_data-offsets.c
> +++ b/arch/arm/mach-at91/pm_data-offsets.c
> @@ -9,5 +9,8 @@ int main(void)
> DEFINE(PM_DATA_RAMC1, offsetof(struct at91_pm_data, ramc[1]));
> DEFINE(PM_DATA_MEMCTRL, offsetof(struct at91_pm_data, memctrl));
> DEFINE(PM_DATA_MODE, offsetof(struct at91_pm_data, mode));
> + DEFINE(PM_DATA_SHDWC, offsetof(struct at91_pm_data, shdwc));
> + DEFINE(PM_DATA_SFRBU, offsetof(struct at91_pm_data, sfrbu));
> +
> return 0;
> }
> diff --git a/arch/arm/mach-at91/pm_suspend.S b/arch/arm/mach-at91/pm_suspend.S
> index 96781daa671a..b5ffa8e1f203 100644
> --- a/arch/arm/mach-at91/pm_suspend.S
> +++ b/arch/arm/mach-at91/pm_suspend.S
> @@ -97,15 +97,74 @@ ENTRY(at91_pm_suspend_in_sram)
> str tmp1, .memtype
> ldr tmp1, [r0, #PM_DATA_MODE]
> str tmp1, .pm_mode
> + ldr tmp1, [r0, #PM_DATA_SHDWC]
> +#if defined(BACKUP_MODE)
> + str tmp1, .shdwc
> + cmp tmp1, #0
> + ldrne tmp2, [tmp1, #0]
> + ldr tmp1, [r0, #PM_DATA_SFRBU]
> + str tmp1, .sfr
> + cmp tmp1, #0
> + ldrne tmp2, [tmp1, #0x10]
> +#endif

If I understand this well, we are doing this to fill the TLB in advance
before the external RAM is put in self-refresh. It might be worthy of a
comment. Moreover, .pm_mode and .memtype do not need to be protected as
they are accessed during the at91_sramc_self_refresh, but .pmc_base
may need to be loaded in the TLB as well.

>
> /* Active the self-refresh mode */
> mov r0, #SRAMC_SELF_FRESH_ACTIVE
> bl at91_sramc_self_refresh
>
> ldr r0, .pm_mode
> - tst r0, #AT91_PM_SLOW_CLOCK
> - beq skip_disable_main_clock
> + cmp r0, #AT91_PM_SLOW_CLOCK
> + beq slow_clock
> +#if defined(BACKUP_MODE)
> + cmp r0, #AT91_PM_BACKUP
> + beq backup_mode
> +#endif
>
> + /* Wait for interrupt */
> + ldr pmc, .pmc_base
> + at91_cpu_idle
> + b exit_suspend
> +
> +slow_clock:
> + bl at91_slowck_mode
> + b exit_suspend
> +#if defined(BACKUP_MODE)
> +backup_mode:
> + bl at91_backup_mode
> + b exit_suspend
> +#endif
> +
> +exit_suspend:
> + /* Exit the self-refresh mode */
> + mov r0, #SRAMC_SELF_FRESH_EXIT
> + bl at91_sramc_self_refresh
> +
> + /* Restore registers, and return */
> + ldmfd sp!, {r4 - r12, pc}
> +ENDPROC(at91_pm_suspend_in_sram)
> +
> +#if defined(BACKUP_MODE)
> +ENTRY(at91_backup_mode)
> + #if 0
> + /* Read LPR */
> + ldr r2, .sramc_base
> + ldr r3, [r2, #AT91_DDRSDRC_LPR]
> + #endif
> +

Do we need to keep this commented code ?

> + /*BUMEN*/
> + ldr r0, .sfr
> + mov tmp1, #(0x1)

We don't need any parenthesis here

> + str tmp1, [r0, #0x10]
> +
> + /* Shutdown */
> + ldr r0, .shdwc
> + movw tmp1, #0x1
> + movt tmp1, #0xA500

I believe the following assembly should do the same thing
without using v6+ instructions.

mov tmp1, #0xA5000000
add tmp1, tmp1, #0x1

> + str tmp1, [r0, #0]
> +ENDPROC(at91_backup_mode)
> +#endif
> +
> +ENTRY(at91_slowck_mode)
> ldr pmc, .pmc_base
>
> /* Save Master clock setting */
> @@ -134,18 +193,9 @@ ENTRY(at91_pm_suspend_in_sram)
> orr tmp1, tmp1, #AT91_PMC_KEY
> str tmp1, [pmc, #AT91_CKGR_MOR]
>
> -skip_disable_main_clock:
> - ldr pmc, .pmc_base
> -
> /* Wait for interrupt */
> at91_cpu_idle
>
> - ldr r0, .pm_mode
> - tst r0, #AT91_PM_SLOW_CLOCK
> - beq skip_enable_main_clock
> -
> - ldr pmc, .pmc_base
> -
> /* Turn on the main oscillator */
> ldr tmp1, [pmc, #AT91_CKGR_MOR]
> orr tmp1, tmp1, #AT91_PMC_MOSCEN
> @@ -174,14 +224,8 @@ skip_disable_main_clock:
>
> wait_mckrdy
>
> -skip_enable_main_clock:
> - /* Exit the self-refresh mode */
> - mov r0, #SRAMC_SELF_FRESH_EXIT
> - bl at91_sramc_self_refresh
> -
> - /* Restore registers, and return */
> - ldmfd sp!, {r4 - r12, pc}
> -ENDPROC(at91_pm_suspend_in_sram)
> + mov pc, lr
> +ENDPROC(at91_slowck_mode)
>
> /*
> * void at91_sramc_self_refresh(unsigned int is_active)
> @@ -314,6 +358,10 @@ ENDPROC(at91_sramc_self_refresh)
> .word 0
> .sramc1_base:
> .word 0
> +.shdwc:
> + .word 0
> +.sfr:
> + .word 0
> .memtype:
> .word 0
> .pm_mode:
> diff --git a/arch/arm/mach-at91/sama5.c b/arch/arm/mach-at91/sama5.c
> index 6d157d0ead8e..3d0bf95a56ae 100644
> --- a/arch/arm/mach-at91/sama5.c
> +++ b/arch/arm/mach-at91/sama5.c
> @@ -34,7 +34,6 @@ DT_MACHINE_START(sama5_dt, "Atmel SAMA5")
> MACHINE_END
>
> static const char *const sama5_alt_dt_board_compat[] __initconst = {
> - "atmel,sama5d2",
> "atmel,sama5d4",
> NULL
> };
> @@ -45,3 +44,21 @@ DT_MACHINE_START(sama5_alt_dt, "Atmel SAMA5")
> .dt_compat = sama5_alt_dt_board_compat,
> .l2c_aux_mask = ~0UL,
> MACHINE_END
> +
> +static void __init sama5d2_init(void)
> +{
> + of_platform_default_populate(NULL, NULL, NULL);
> + sama5d2_pm_init();
> +}
> +
> +static const char *const sama5d2_compat[] __initconst = {
> + "atmel,sama5d2",
> + NULL
> +};
> +
> +DT_MACHINE_START(sama5d2, "Atmel SAMA5")
> + /* Maintainer: Atmel */
> + .init_machine = sama5d2_init,
> + .dt_compat = sama5d2_compat,
> + .l2c_aux_mask = ~0UL,
> +MACHINE_END

Best regards,
--
Romain Izard

2017-04-27 14:42:13

by Alexandre Belloni

[permalink] [raw]
Subject: Re: [PATCH 1/3] ARM: at91: pm: Add sama5d2 backup mode

On 27/04/2017 at 15:34:07 +0200, Romain Izard wrote:
> Hello Alexandre,
>
> This series might also be of interest for the linux-pm mailing list.
>

I don't think they care enough to review that.

> 2017-04-26 18:04 GMT+02:00 Alexandre Belloni
> > diff --git a/arch/arm/mach-at91/Makefile b/arch/arm/mach-at91/Makefile
> > index cfd8f60a9268..87fe17dbdb56 100644
> > --- a/arch/arm/mach-at91/Makefile
> > +++ b/arch/arm/mach-at91/Makefile
> > @@ -14,6 +14,10 @@ obj-$(CONFIG_PM) += pm_suspend.o
> > ifeq ($(CONFIG_CPU_V7),y)
> > AFLAGS_pm_suspend.o := -march=armv7-a
> > endif
> > +# Backup mode will not compile for ARMv5 because of movt
> > +ifeq ($(CONFIG_SOC_SAMA5D2),y)
> > +AFLAGS_pm_suspend.o += -DBACKUP_MODE
> > +endif
> > ifeq ($(CONFIG_PM_DEBUG),y)
> > CFLAGS_pm.o += -DDEBUG
> > endif
>
> We can rewrite the assembly to avoid using movt, and remove some ifdefs
> from the code.
>

I'm kind of balanced there because I'm wondering whether we should
better separate what is in that assembly file because there are part of
it that have no chance to run on some platforms anyway.

But your solution is correct, I'll remove that.

> > +static void __init at91_pm_bu_sram_init(void)
> > +{
> > + struct gen_pool *sram_pool;
> > + struct device_node *node;
> > + struct platform_device *pdev = NULL;
> > +
> > + pm_bu = NULL;
> > +
> > + for_each_compatible_node(node, NULL, "atmel,sama5d2-securam") {
> > + pdev = of_find_device_by_node(node);
> > + if (pdev) {
> > + of_node_put(node);
> > + break;
> > + }
> > + }
> > +
>
> Do we really need to iterate over compatible nodes ?
>

You're right, this can probably be avoided.

> > + if (!pdev) {
> > + pr_warn("%s: failed to find securam device!\n", __func__);
> > + return;
> > + }
> > +
> > + sram_pool = gen_pool_get(&pdev->dev, NULL);
> > + if (!sram_pool) {
> > + pr_warn("%s: securam pool unavailable!\n", __func__);
> > + return;
> > + }
> > +
> > + pm_bu = (void *)gen_pool_alloc(sram_pool, sizeof(struct at91_pm_bu));
> > + if (!pm_bu) {
> > + pr_warn("%s: unable to alloc securam!\n", __func__);
> > + return;
> > + }
> > +
> > + pm_bu->suspended = 0;
> > + pm_bu->canary = virt_to_phys(&canary);
> > + pm_bu->resume = virt_to_phys(cpu_resume);
> > +}
> > +
>
> at91_pm_bu_sram_init and at91_dt_shdwc are necessary to use backup mode.
> But those functions do not return error codes, and do no cleanup in case
> of error. I believe that it would be simpler if we only had a single
> function.
>

Yeah, this is kind of solved by adding the fallback in a latter patch
but I agree it can be done better.

> > diff --git a/arch/arm/mach-at91/pm_suspend.S b/arch/arm/mach-at91/pm_suspend.S
> > index 96781daa671a..b5ffa8e1f203 100644
> > --- a/arch/arm/mach-at91/pm_suspend.S
> > +++ b/arch/arm/mach-at91/pm_suspend.S
> > @@ -97,15 +97,74 @@ ENTRY(at91_pm_suspend_in_sram)
> > str tmp1, .memtype
> > ldr tmp1, [r0, #PM_DATA_MODE]
> > str tmp1, .pm_mode
> > + ldr tmp1, [r0, #PM_DATA_SHDWC]
> > +#if defined(BACKUP_MODE)
> > + str tmp1, .shdwc
> > + cmp tmp1, #0
> > + ldrne tmp2, [tmp1, #0]
> > + ldr tmp1, [r0, #PM_DATA_SFRBU]
> > + str tmp1, .sfr
> > + cmp tmp1, #0
> > + ldrne tmp2, [tmp1, #0x10]
> > +#endif
>
> If I understand this well, we are doing this to fill the TLB in advance
> before the external RAM is put in self-refresh. It might be worthy of a
> comment. Moreover, .pm_mode and .memtype do not need to be protected as
> they are accessed during the at91_sramc_self_refresh, but .pmc_base
> may need to be loaded in the TLB as well.

We never had issue with .pmc_base because it is used in the C part of
the code, right before calling the assembly.
I'll add a comment.

>
> > +#if defined(BACKUP_MODE)
> > +ENTRY(at91_backup_mode)
> > + #if 0
> > + /* Read LPR */
> > + ldr r2, .sramc_base
> > + ldr r3, [r2, #AT91_DDRSDRC_LPR]
> > + #endif
> > +
>
> Do we need to keep this commented code ?
>

Nope, leftover from development


--
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

2017-04-28 01:25:25

by Wenyou Yang

[permalink] [raw]
Subject: Re: [PATCH 1/3] ARM: at91: pm: Add sama5d2 backup mode



On 2017/4/27 0:04, Alexandre Belloni wrote:
> The sama5d2 has a mode were it is possible to cut power to the SoC while
> keeping the RAM in self refresh.
> Resuming from that mode needs support in the firmware/bootloader.
>
> Signed-off-by: Alexandre Belloni <[email protected]>

Acked-by: Wenyou Yang <[email protected]>
> ---
> arch/arm/mach-at91/Makefile | 4 ++
> arch/arm/mach-at91/generic.h | 2 +
> arch/arm/mach-at91/pm.c | 103 ++++++++++++++++++++++++++++++++++-
> arch/arm/mach-at91/pm.h | 4 ++
> arch/arm/mach-at91/pm_data-offsets.c | 3 +
> arch/arm/mach-at91/pm_suspend.S | 86 ++++++++++++++++++++++-------
> arch/arm/mach-at91/sama5.c | 19 ++++++-
> 7 files changed, 198 insertions(+), 23 deletions(-)
>
> diff --git a/arch/arm/mach-at91/Makefile b/arch/arm/mach-at91/Makefile
> index cfd8f60a9268..87fe17dbdb56 100644
> --- a/arch/arm/mach-at91/Makefile
> +++ b/arch/arm/mach-at91/Makefile
> @@ -14,6 +14,10 @@ obj-$(CONFIG_PM) += pm_suspend.o
> ifeq ($(CONFIG_CPU_V7),y)
> AFLAGS_pm_suspend.o := -march=armv7-a
> endif
> +# Backup mode will not compile for ARMv5 because of movt
> +ifeq ($(CONFIG_SOC_SAMA5D2),y)
> +AFLAGS_pm_suspend.o += -DBACKUP_MODE
> +endif
> ifeq ($(CONFIG_PM_DEBUG),y)
> CFLAGS_pm.o += -DDEBUG
> endif
> diff --git a/arch/arm/mach-at91/generic.h b/arch/arm/mach-at91/generic.h
> index f1ead0f13c19..e2bd17237964 100644
> --- a/arch/arm/mach-at91/generic.h
> +++ b/arch/arm/mach-at91/generic.h
> @@ -15,10 +15,12 @@
> extern void __init at91rm9200_pm_init(void);
> extern void __init at91sam9_pm_init(void);
> extern void __init sama5_pm_init(void);
> +extern void __init sama5d2_pm_init(void);
> #else
> static inline void __init at91rm9200_pm_init(void) { }
> static inline void __init at91sam9_pm_init(void) { }
> static inline void __init sama5_pm_init(void) { }
> +static inline void __init sama5d2_pm_init(void) { }
> #endif
>
> #endif /* _AT91_GENERIC_H */
> diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
> index 2cd27c830ab6..1e03f1277f14 100644
> --- a/arch/arm/mach-at91/pm.c
> +++ b/arch/arm/mach-at91/pm.c
> @@ -22,6 +22,7 @@
> #include <asm/cacheflush.h>
> #include <asm/fncpy.h>
> #include <asm/system_misc.h>
> +#include <asm/suspend.h>
>
> #include "generic.h"
> #include "pm.h"
> @@ -58,6 +59,14 @@ static int at91_pm_valid_state(suspend_state_t state)
> }
> }
>
> +static int canary = 0xA5A5A5A5;
> +
> +static struct at91_pm_bu {
> + int suspended;
> + unsigned long reserved;
> + phys_addr_t canary;
> + phys_addr_t resume;
> +} *pm_bu;
>
> static suspend_state_t target_state;
>
> @@ -123,15 +132,39 @@ static void (*at91_suspend_sram_fn)(struct at91_pm_data *);
> extern void at91_pm_suspend_in_sram(struct at91_pm_data *pm_data);
> extern u32 at91_pm_suspend_in_sram_sz;
>
> -static void at91_pm_suspend(suspend_state_t state)
> +static int at91_suspend_finish(unsigned long val)
> {
> - pm_data.mode = (state == PM_SUSPEND_MEM) ? AT91_PM_SLOW_CLOCK : 0;
> -
> flush_cache_all();
> outer_disable();
>
> at91_suspend_sram_fn(&pm_data);
>
> + return 0;
> +}
> +
> +static void at91_pm_suspend(suspend_state_t state)
> +{
> + if (pm_data.deepest_state == AT91_PM_BACKUP)
> + if (state == PM_SUSPEND_MEM)
> + pm_data.mode = AT91_PM_BACKUP;
> + else
> + pm_data.mode = AT91_PM_SLOW_CLOCK;
> + else
> + pm_data.mode = (state == PM_SUSPEND_MEM) ? AT91_PM_SLOW_CLOCK : 0;
> +
> + if (pm_data.mode == AT91_PM_BACKUP) {
> + pm_bu->suspended = 1;
> +
> + cpu_suspend(0, at91_suspend_finish);
> +
> + /* The SRAM is lost between suspend cycles */
> + at91_suspend_sram_fn = fncpy(at91_suspend_sram_fn,
> + &at91_pm_suspend_in_sram,
> + at91_pm_suspend_in_sram_sz);
> + } else {
> + at91_suspend_finish(0);
> + }
> +
> outer_resume();
> }
>
> @@ -375,6 +408,25 @@ static __init void at91_dt_ramc(void)
> at91_cpuidle_device.dev.platform_data = standby;
> }
>
> +static __init void at91_dt_shdwc(void)
> +{
> + struct device_node *np;
> +
> + np = of_find_compatible_node(NULL, NULL, "atmel,sama5d2-shdwc");
> + if (!np)
> + return;
> +
> + pm_data.shdwc = of_iomap(np, 0);
> + of_node_put(np);
> +
> + np = of_find_compatible_node(NULL, NULL, "atmel,sama5d2-sfrbu");
> + if (!np)
> + return;
> +
> + pm_data.sfrbu = of_iomap(np, 0);
> + of_node_put(np);
> +}
> +
> static void at91rm9200_idle(void)
> {
> /*
> @@ -436,6 +488,44 @@ static void __init at91_pm_sram_init(void)
> &at91_pm_suspend_in_sram, at91_pm_suspend_in_sram_sz);
> }
>
> +static void __init at91_pm_bu_sram_init(void)
> +{
> + struct gen_pool *sram_pool;
> + struct device_node *node;
> + struct platform_device *pdev = NULL;
> +
> + pm_bu = NULL;
> +
> + for_each_compatible_node(node, NULL, "atmel,sama5d2-securam") {
> + pdev = of_find_device_by_node(node);
> + if (pdev) {
> + of_node_put(node);
> + break;
> + }
> + }
> +
> + if (!pdev) {
> + pr_warn("%s: failed to find securam device!\n", __func__);
> + return;
> + }
> +
> + sram_pool = gen_pool_get(&pdev->dev, NULL);
> + if (!sram_pool) {
> + pr_warn("%s: securam pool unavailable!\n", __func__);
> + return;
> + }
> +
> + pm_bu = (void *)gen_pool_alloc(sram_pool, sizeof(struct at91_pm_bu));
> + if (!pm_bu) {
> + pr_warn("%s: unable to alloc securam!\n", __func__);
> + return;
> + }
> +
> + pm_bu->suspended = 0;
> + pm_bu->canary = virt_to_phys(&canary);
> + pm_bu->resume = virt_to_phys(cpu_resume);
> +}
> +
> struct pmc_info {
> unsigned long uhp_udp_mask;
> };
> @@ -510,3 +600,10 @@ void __init sama5_pm_init(void)
> at91_dt_ramc();
> at91_pm_init(NULL);
> }
> +
> +void __init sama5d2_pm_init(void)
> +{
> + at91_dt_shdwc();
> + at91_pm_bu_sram_init();
> + sama5_pm_init();
> +}
> diff --git a/arch/arm/mach-at91/pm.h b/arch/arm/mach-at91/pm.h
> index fc0f7d048187..d9c6612ef62f 100644
> --- a/arch/arm/mach-at91/pm.h
> +++ b/arch/arm/mach-at91/pm.h
> @@ -22,6 +22,7 @@
> #define AT91_MEMCTRL_DDRSDR 2
>
> #define AT91_PM_SLOW_CLOCK 0x01
> +#define AT91_PM_BACKUP 0x02
>
> #ifndef __ASSEMBLY__
> struct at91_pm_data {
> @@ -30,6 +31,9 @@ struct at91_pm_data {
> unsigned long uhp_udp_mask;
> unsigned int memctrl;
> unsigned int mode;
> + void __iomem *shdwc;
> + void __iomem *sfrbu;
> + unsigned int deepest_state;
> };
> #endif
>
> diff --git a/arch/arm/mach-at91/pm_data-offsets.c b/arch/arm/mach-at91/pm_data-offsets.c
> index 30302cb16df0..c0a73e62b725 100644
> --- a/arch/arm/mach-at91/pm_data-offsets.c
> +++ b/arch/arm/mach-at91/pm_data-offsets.c
> @@ -9,5 +9,8 @@ int main(void)
> DEFINE(PM_DATA_RAMC1, offsetof(struct at91_pm_data, ramc[1]));
> DEFINE(PM_DATA_MEMCTRL, offsetof(struct at91_pm_data, memctrl));
> DEFINE(PM_DATA_MODE, offsetof(struct at91_pm_data, mode));
> + DEFINE(PM_DATA_SHDWC, offsetof(struct at91_pm_data, shdwc));
> + DEFINE(PM_DATA_SFRBU, offsetof(struct at91_pm_data, sfrbu));
> +
> return 0;
> }
> diff --git a/arch/arm/mach-at91/pm_suspend.S b/arch/arm/mach-at91/pm_suspend.S
> index 96781daa671a..b5ffa8e1f203 100644
> --- a/arch/arm/mach-at91/pm_suspend.S
> +++ b/arch/arm/mach-at91/pm_suspend.S
> @@ -97,15 +97,74 @@ ENTRY(at91_pm_suspend_in_sram)
> str tmp1, .memtype
> ldr tmp1, [r0, #PM_DATA_MODE]
> str tmp1, .pm_mode
> + ldr tmp1, [r0, #PM_DATA_SHDWC]
> +#if defined(BACKUP_MODE)
> + str tmp1, .shdwc
> + cmp tmp1, #0
> + ldrne tmp2, [tmp1, #0]
> + ldr tmp1, [r0, #PM_DATA_SFRBU]
> + str tmp1, .sfr
> + cmp tmp1, #0
> + ldrne tmp2, [tmp1, #0x10]
> +#endif
>
> /* Active the self-refresh mode */
> mov r0, #SRAMC_SELF_FRESH_ACTIVE
> bl at91_sramc_self_refresh
>
> ldr r0, .pm_mode
> - tst r0, #AT91_PM_SLOW_CLOCK
> - beq skip_disable_main_clock
> + cmp r0, #AT91_PM_SLOW_CLOCK
> + beq slow_clock
> +#if defined(BACKUP_MODE)
> + cmp r0, #AT91_PM_BACKUP
> + beq backup_mode
> +#endif
>
> + /* Wait for interrupt */
> + ldr pmc, .pmc_base
> + at91_cpu_idle
> + b exit_suspend
> +
> +slow_clock:
> + bl at91_slowck_mode
> + b exit_suspend
> +#if defined(BACKUP_MODE)
> +backup_mode:
> + bl at91_backup_mode
> + b exit_suspend
> +#endif
> +
> +exit_suspend:
> + /* Exit the self-refresh mode */
> + mov r0, #SRAMC_SELF_FRESH_EXIT
> + bl at91_sramc_self_refresh
> +
> + /* Restore registers, and return */
> + ldmfd sp!, {r4 - r12, pc}
> +ENDPROC(at91_pm_suspend_in_sram)
> +
> +#if defined(BACKUP_MODE)
> +ENTRY(at91_backup_mode)
> + #if 0
> + /* Read LPR */
> + ldr r2, .sramc_base
> + ldr r3, [r2, #AT91_DDRSDRC_LPR]
> + #endif
> +
> + /*BUMEN*/
> + ldr r0, .sfr
> + mov tmp1, #(0x1)
> + str tmp1, [r0, #0x10]
> +
> + /* Shutdown */
> + ldr r0, .shdwc
> + movw tmp1, #0x1
> + movt tmp1, #0xA500
> + str tmp1, [r0, #0]
> +ENDPROC(at91_backup_mode)
> +#endif
> +
> +ENTRY(at91_slowck_mode)
> ldr pmc, .pmc_base
>
> /* Save Master clock setting */
> @@ -134,18 +193,9 @@ ENTRY(at91_pm_suspend_in_sram)
> orr tmp1, tmp1, #AT91_PMC_KEY
> str tmp1, [pmc, #AT91_CKGR_MOR]
>
> -skip_disable_main_clock:
> - ldr pmc, .pmc_base
> -
> /* Wait for interrupt */
> at91_cpu_idle
>
> - ldr r0, .pm_mode
> - tst r0, #AT91_PM_SLOW_CLOCK
> - beq skip_enable_main_clock
> -
> - ldr pmc, .pmc_base
> -
> /* Turn on the main oscillator */
> ldr tmp1, [pmc, #AT91_CKGR_MOR]
> orr tmp1, tmp1, #AT91_PMC_MOSCEN
> @@ -174,14 +224,8 @@ skip_disable_main_clock:
>
> wait_mckrdy
>
> -skip_enable_main_clock:
> - /* Exit the self-refresh mode */
> - mov r0, #SRAMC_SELF_FRESH_EXIT
> - bl at91_sramc_self_refresh
> -
> - /* Restore registers, and return */
> - ldmfd sp!, {r4 - r12, pc}
> -ENDPROC(at91_pm_suspend_in_sram)
> + mov pc, lr
> +ENDPROC(at91_slowck_mode)
>
> /*
> * void at91_sramc_self_refresh(unsigned int is_active)
> @@ -314,6 +358,10 @@ ENDPROC(at91_sramc_self_refresh)
> .word 0
> .sramc1_base:
> .word 0
> +.shdwc:
> + .word 0
> +.sfr:
> + .word 0
> .memtype:
> .word 0
> .pm_mode:
> diff --git a/arch/arm/mach-at91/sama5.c b/arch/arm/mach-at91/sama5.c
> index 6d157d0ead8e..3d0bf95a56ae 100644
> --- a/arch/arm/mach-at91/sama5.c
> +++ b/arch/arm/mach-at91/sama5.c
> @@ -34,7 +34,6 @@ DT_MACHINE_START(sama5_dt, "Atmel SAMA5")
> MACHINE_END
>
> static const char *const sama5_alt_dt_board_compat[] __initconst = {
> - "atmel,sama5d2",
> "atmel,sama5d4",
> NULL
> };
> @@ -45,3 +44,21 @@ DT_MACHINE_START(sama5_alt_dt, "Atmel SAMA5")
> .dt_compat = sama5_alt_dt_board_compat,
> .l2c_aux_mask = ~0UL,
> MACHINE_END
> +
> +static void __init sama5d2_init(void)
> +{
> + of_platform_default_populate(NULL, NULL, NULL);
> + sama5d2_pm_init();
> +}
> +
> +static const char *const sama5d2_compat[] __initconst = {
> + "atmel,sama5d2",
> + NULL
> +};
> +
> +DT_MACHINE_START(sama5d2, "Atmel SAMA5")
> + /* Maintainer: Atmel */
> + .init_machine = sama5d2_init,
> + .dt_compat = sama5d2_compat,
> + .l2c_aux_mask = ~0UL,
> +MACHINE_END

2017-04-28 01:33:17

by Wenyou Yang

[permalink] [raw]
Subject: Re: [PATCH 2/3] ARM: at91: pm: allow selecting standby and suspend modes



On 2017/4/27 0:04, Alexandre Belloni wrote:
> While we can only select between "standby" and "mem" states for power
> management, the atmel platforms can actually support more modes.
>
> For both standby and mem, allow selecting which mode will be used using the
> atmel.pm_modes kernel parameter.
> By default, keep the current modes.
>
> Signed-off-by: Alexandre Belloni <[email protected]>

Acked-by: Wenyou Yang <[email protected]>
> ---
> arch/arm/mach-at91/pm.c | 110 +++++++++++++++++++++++++++++++++---------------
> arch/arm/mach-at91/pm.h | 3 +-
> 2 files changed, 78 insertions(+), 35 deletions(-)
>
> diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
> index 1e03f1277f14..d08f032f9d94 100644
> --- a/arch/arm/mach-at91/pm.c
> +++ b/arch/arm/mach-at91/pm.c
> @@ -15,6 +15,7 @@
> #include <linux/of_address.h>
> #include <linux/of.h>
> #include <linux/of_platform.h>
> +#include <linux/parser.h>
> #include <linux/suspend.h>
>
> #include <linux/clk/at91_pmc.h>
> @@ -38,7 +39,17 @@ extern void at91_pinctrl_gpio_suspend(void);
> extern void at91_pinctrl_gpio_resume(void);
> #endif
>
> -static struct at91_pm_data pm_data;
> +static const match_table_t pm_modes __initconst = {
> + { 0, "standby" },
> + { AT91_PM_SLOW_CLOCK, "ulp0" },
> + { AT91_PM_BACKUP, "backup" },
> + { -1, NULL },
> +};
> +
> +static struct at91_pm_data pm_data = {
> + .standby_mode = 0,
> + .suspend_mode = AT91_PM_SLOW_CLOCK,
> +};
>
> #define at91_ramc_read(id, field) \
> __raw_readl(pm_data.ramc[id] + field)
> @@ -68,14 +79,24 @@ static struct at91_pm_bu {
> phys_addr_t resume;
> } *pm_bu;
>
> -static suspend_state_t target_state;
> -
> /*
> * Called after processes are frozen, but before we shutdown devices.
> */
> static int at91_pm_begin(suspend_state_t state)
> {
> - target_state = state;
> + switch (state) {
> + case PM_SUSPEND_MEM:
> + pm_data.mode = pm_data.suspend_mode;
> + break;
> +
> + case PM_SUSPEND_STANDBY:
> + pm_data.mode = pm_data.standby_mode;
> + break;
> +
> + default:
> + pm_data.mode = -1;
> + }
> +
> return 0;
> }
>
> @@ -124,7 +145,7 @@ static int at91_pm_verify_clocks(void)
> */
> int at91_suspend_entering_slow_clock(void)
> {
> - return (target_state == PM_SUSPEND_MEM);
> + return (pm_data.mode >= AT91_PM_SLOW_CLOCK);
> }
> EXPORT_SYMBOL(at91_suspend_entering_slow_clock);
>
> @@ -144,14 +165,6 @@ static int at91_suspend_finish(unsigned long val)
>
> static void at91_pm_suspend(suspend_state_t state)
> {
> - if (pm_data.deepest_state == AT91_PM_BACKUP)
> - if (state == PM_SUSPEND_MEM)
> - pm_data.mode = AT91_PM_BACKUP;
> - else
> - pm_data.mode = AT91_PM_SLOW_CLOCK;
> - else
> - pm_data.mode = (state == PM_SUSPEND_MEM) ? AT91_PM_SLOW_CLOCK : 0;
> -
> if (pm_data.mode == AT91_PM_BACKUP) {
> pm_bu->suspended = 1;
>
> @@ -168,38 +181,37 @@ static void at91_pm_suspend(suspend_state_t state)
> outer_resume();
> }
>
> +/*
> + * STANDBY mode has *all* drivers suspended; ignores irqs not marked as 'wakeup'
> + * event sources; and reduces DRAM power. But otherwise it's identical to
> + * PM_SUSPEND_ON: cpu idle, and nothing fancy done with main or cpu clocks.
> + *
> + * AT91_PM_SLOW_CLOCK is like STANDBY plus slow clock mode, so drivers must
> + * suspend more deeply, the master clock switches to the clk32k and turns off
> + * the main oscillator
> + *
> + * AT91_PM_BACKUP turns off the whole SoC after placing the DDR in self refresh
> + */
> static int at91_pm_enter(suspend_state_t state)
> {
> #ifdef CONFIG_PINCTRL_AT91
> at91_pinctrl_gpio_suspend();
> #endif
> +
> switch (state) {
> - /*
> - * Suspend-to-RAM is like STANDBY plus slow clock mode, so
> - * drivers must suspend more deeply, the master clock switches
> - * to the clk32k and turns off the main oscillator
> - */
> case PM_SUSPEND_MEM:
> + case PM_SUSPEND_STANDBY:
> /*
> * Ensure that clocks are in a valid state.
> */
> - if (!at91_pm_verify_clocks())
> + if ((pm_data.mode >= AT91_PM_SLOW_CLOCK) &&
> + !at91_pm_verify_clocks())
> goto error;
>
> at91_pm_suspend(state);
>
> break;
>
> - /*
> - * STANDBY mode has *all* drivers suspended; ignores irqs not
> - * marked as 'wakeup' event sources; and reduces DRAM power.
> - * But otherwise it's identical to PM_SUSPEND_ON: cpu idle, and
> - * nothing fancy done with main or cpu clocks.
> - */
> - case PM_SUSPEND_STANDBY:
> - at91_pm_suspend(state);
> - break;
> -
> case PM_SUSPEND_ON:
> cpu_do_idle();
> break;
> @@ -210,8 +222,6 @@ static int at91_pm_enter(suspend_state_t state)
> }
>
> error:
> - target_state = PM_SUSPEND_ON;
> -
> #ifdef CONFIG_PINCTRL_AT91
> at91_pinctrl_gpio_resume();
> #endif
> @@ -223,7 +233,6 @@ static int at91_pm_enter(suspend_state_t state)
> */
> static void at91_pm_end(void)
> {
> - target_state = PM_SUSPEND_ON;
> }
>
>
> @@ -494,6 +503,10 @@ static void __init at91_pm_bu_sram_init(void)
> struct device_node *node;
> struct platform_device *pdev = NULL;
>
> + if ((pm_data.standby_mode != AT91_PM_BACKUP) &&
> + (pm_data.suspend_mode != AT91_PM_BACKUP))
> + return;
> +
> pm_bu = NULL;
>
> for_each_compatible_node(node, NULL, "atmel,sama5d2-securam") {
> @@ -571,10 +584,14 @@ static void __init at91_pm_init(void (*pm_idle)(void))
>
> at91_pm_sram_init();
>
> - if (at91_suspend_sram_fn)
> + if (at91_suspend_sram_fn) {
> suspend_set_ops(&at91_pm_ops);
> - else
> + pr_info("AT91: PM: standby: %s, suspend: %s\n",
> + pm_modes[pm_data.standby_mode].pattern,
> + pm_modes[pm_data.suspend_mode].pattern);
> + } else {
> pr_info("AT91: PM not supported, due to no SRAM allocated\n");
> + }
> }
>
> void __init at91rm9200_pm_init(void)
> @@ -607,3 +624,28 @@ void __init sama5d2_pm_init(void)
> at91_pm_bu_sram_init();
> sama5_pm_init();
> }
> +
> +static int __init at91_pm_modes_select(char *str)
> +{
> + char *s;
> + substring_t args[MAX_OPT_ARGS];
> + int standby, suspend;
> +
> + if (!str)
> + return 0;
> +
> + s = strsep(&str, ",");
> + standby = match_token(s, pm_modes, args);
> + if (standby < 0)
> + return 0;
> +
> + suspend = match_token(str, pm_modes, args);
> + if (suspend < 0)
> + return 0;
> +
> + pm_data.standby_mode = standby;
> + pm_data.suspend_mode = suspend;
> +
> + return 0;
> +}
> +early_param("atmel.pm_modes", at91_pm_modes_select);
> diff --git a/arch/arm/mach-at91/pm.h b/arch/arm/mach-at91/pm.h
> index d9c6612ef62f..f95d31496f08 100644
> --- a/arch/arm/mach-at91/pm.h
> +++ b/arch/arm/mach-at91/pm.h
> @@ -33,7 +33,8 @@ struct at91_pm_data {
> unsigned int mode;
> void __iomem *shdwc;
> void __iomem *sfrbu;
> - unsigned int deepest_state;
> + unsigned int standby_mode;
> + unsigned int suspend_mode;
> };
> #endif
>
Best Regards,
Wenyou Yang

2017-04-28 01:34:28

by Wenyou Yang

[permalink] [raw]
Subject: Re: [PATCH 3/3] ARM: at91: pm: fallback to slowclock when backup mode fails



On 2017/4/27 0:04, Alexandre Belloni wrote:
> If the backup sram allocation fails, ensure we can suspend by falling back
> to the usual slow clock mode.
>
> Signed-off-by: Alexandre Belloni <[email protected]>
Acked-by: Wenyou Yang <[email protected]>

> ---
> arch/arm/mach-at91/pm.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
> index d08f032f9d94..02823d8f3ada 100644
> --- a/arch/arm/mach-at91/pm.c
> +++ b/arch/arm/mach-at91/pm.c
> @@ -519,24 +519,30 @@ static void __init at91_pm_bu_sram_init(void)
>
> if (!pdev) {
> pr_warn("%s: failed to find securam device!\n", __func__);
> - return;
> + goto fallback;
> }
>
> sram_pool = gen_pool_get(&pdev->dev, NULL);
> if (!sram_pool) {
> pr_warn("%s: securam pool unavailable!\n", __func__);
> - return;
> + goto fallback;
> }
>
> pm_bu = (void *)gen_pool_alloc(sram_pool, sizeof(struct at91_pm_bu));
> if (!pm_bu) {
> pr_warn("%s: unable to alloc securam!\n", __func__);
> - return;
> + goto fallback;
> }
>
> pm_bu->suspended = 0;
> pm_bu->canary = virt_to_phys(&canary);
> pm_bu->resume = virt_to_phys(cpu_resume);
> +
> +fallback:
> + if (pm_data.standby_mode == AT91_PM_BACKUP)
> + pm_data.standby_mode = AT91_PM_SLOW_CLOCK;
> + if (pm_data.suspend_mode != AT91_PM_BACKUP)
> + pm_data.suspend_mode = AT91_PM_SLOW_CLOCK;
> }
>
> struct pmc_info {
Best Regards,
Wenyou Yang