RE: [PATCH v2 07/12] pm: at91: the standby mode uses the same sram function as the suspend to memory mode
Hi Sylvain,
> -Original Message-
> From: Sylvain Rochet [mailto:sylvain.roc...@finsecur.com]
> Sent: Monday, January 26, 2015 6:10 PM
> To: Yang, Wenyou
> Cc: Ferre, Nicolas; li...@arm.linux.org.uk;
> linux-arm-ker...@lists.infradead.org;
> linux-kernel@vger.kernel.org; alexandre.bell...@free-electrons.com;
> p...@axentia.se
> Subject: Re: [PATCH v2 07/12] pm: at91: the standby mode uses the same sram
> function as the suspend to memory mode
>
> Hello Wenyou,
>
>
> On Mon, Jan 26, 2015 at 05:42:11PM +0800, Wenyou Yang wrote:
> > +static int at91_pm_verify_clocks(suspend_state_t state)
> > {
> > unsigned long scsr;
> > int i;
> >
> > + /* For PM_SUSPEND_STANDBY, skip verifying the clock */
> > + if (state == PM_SUSPEND_STANDBY)
> > + return 1;
> > +
>
> In my opinion we should use the select() already in place in
> at91_pm_enter() to do that:
Accepted. Thanks
>
> > static int at91_pm_enter(suspend_state_t state) {
> > at91_pinctrl_gpio_suspend();
> >
> > switch (state) {
> (...)
> > + case PM_SUSPEND_MEM:
>
> /*
>* Ensure that clocks are in a valid state.
>*/
> if (!at91_pm_verify_clocks())
> goto error;
> /* FALLTHROUGH */
>
> > + case PM_SUSPEND_STANDBY:
> > /*
> > -* Suspend-to-RAM is like STANDBY plus slow clock mode, so
>
>
> Sylvain
Best Regards,
Wenyou Yang
Re: [PATCH v2 07/12] pm: at91: the standby mode uses the same sram function as the suspend to memory mode
Hello Wenyou,
On Mon, Jan 26, 2015 at 05:42:11PM +0800, Wenyou Yang wrote:
> +static int at91_pm_verify_clocks(suspend_state_t state)
> {
> unsigned long scsr;
> int i;
>
> + /* For PM_SUSPEND_STANDBY, skip verifying the clock */
> + if (state == PM_SUSPEND_STANDBY)
> + return 1;
> +
In my opinion we should use the select() already in place in
at91_pm_enter() to do that:
> static int at91_pm_enter(suspend_state_t state)
> {
> at91_pinctrl_gpio_suspend();
>
> switch (state) {
(...)
> + case PM_SUSPEND_MEM:
/*
* Ensure that clocks are in a valid state.
*/
if (!at91_pm_verify_clocks())
goto error;
/* FALLTHROUGH */
> + case PM_SUSPEND_STANDBY:
> /*
> - * Suspend-to-RAM is like STANDBY plus slow clock mode, so
Sylvain
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 07/12] pm: at91: the standby mode uses the same sram function as the suspend to memory mode
To simply the PM code, the suspend to standby mode uses the same sram function
as the suspend to memory mode, running in the internal SRAM,
instead of the respective code for each mode.
But for the suspend to standby mode, the master clock doesn't
switch to the slow clock, and the main oscillator doesn't
turn off as well.
Signed-off-by: Wenyou Yang
---
arch/arm/mach-at91/pm.c | 87 +
arch/arm/mach-at91/pm.h |7 +++
arch/arm/mach-at91/pm_slowclock.S | 18
3 files changed, 65 insertions(+), 47 deletions(-)
diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
index dc2541b..509ac9d 100644
--- a/arch/arm/mach-at91/pm.c
+++ b/arch/arm/mach-at91/pm.c
@@ -71,11 +71,15 @@ static int at91_pm_begin(suspend_state_t state)
* Verify that all the clocks are correct before entering
* slow-clock mode.
*/
-static int at91_pm_verify_clocks(void)
+static int at91_pm_verify_clocks(suspend_state_t state)
{
unsigned long scsr;
int i;
+ /* For PM_SUSPEND_STANDBY, skip verifying the clock */
+ if (state == PM_SUSPEND_STANDBY)
+ return 1;
+
scsr = at91_pmc_read(AT91_PMC_SCSR);
/* USB must not be using PLLB */
@@ -137,62 +141,51 @@ extern void at91_slow_clock(void __iomem *pmc, void
__iomem *ramc0,
void __iomem *ramc1, int memctrl);
extern u32 at91_slow_clock_sz;
+static void at91_pm_suspend(suspend_state_t state)
+{
+ unsigned int pm_data = at91_pm_data.memctrl;
+
+ pm_data |= (state == PM_SUSPEND_MEM) ?
+ AT91_PM_MODE(AT91_PM_SLOW_CLOCK) : 0;
+
+ slow_clock(at91_pmc_base, at91_ramc_base[0],
+ at91_ramc_base[1], pm_data);
+}
+
static int at91_pm_enter(suspend_state_t state)
{
at91_pinctrl_gpio_suspend();
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
+*
+* 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_MEM:
+ case PM_SUSPEND_STANDBY:
/*
-* Suspend-to-RAM is like STANDBY plus slow clock mode, so
-* drivers must suspend more deeply: only the master clock
-* controller may be using the main oscillator.
+* Ensure that clocks are in a valid state.
*/
- case PM_SUSPEND_MEM:
- /*
-* Ensure that clocks are in a valid state.
-*/
- if (!at91_pm_verify_clocks())
- goto error;
-
- /*
-* Enter slow clock mode by switching over to clk32k and
-* turning off the main oscillator; reverse on wakeup.
-*/
- if (slow_clock) {
- slow_clock(at91_pmc_base, at91_ramc_base[0],
- at91_ramc_base[1],
- at91_pm_data.memctrl);
- break;
- } else {
- pr_info("AT91: PM - no slow clock mode enabled
...\n");
- /* FALLTHROUGH leaving master clock alone */
- }
+ if (!at91_pm_verify_clocks(state))
+ goto error;
- /*
-* 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:
- /*
-* NOTE: the Wait-for-Interrupt instruction needs to be
-* in icache so no SDRAM accesses are needed until the
-* wakeup IRQ occurs and self-refresh is terminated.
-* For ARM 926 based chips, this requirement is weaker
-* as at91sam9 can access a RAM in self-refresh mode.
-*/
- if (at91_pm_standby)
- at91_pm_standby();
- break;
+ at91_pm_suspend(state);
- case PM_SUSPEND_ON:
- cpu_do_idle();
- break;
+ break;
-
RE: [PATCH v2 07/12] pm: at91: the standby mode uses the same sram function as the suspend to memory mode
Hi Sylvain,
-Original Message-
From: Sylvain Rochet [mailto:sylvain.roc...@finsecur.com]
Sent: Monday, January 26, 2015 6:10 PM
To: Yang, Wenyou
Cc: Ferre, Nicolas; li...@arm.linux.org.uk;
linux-arm-ker...@lists.infradead.org;
linux-kernel@vger.kernel.org; alexandre.bell...@free-electrons.com;
p...@axentia.se
Subject: Re: [PATCH v2 07/12] pm: at91: the standby mode uses the same sram
function as the suspend to memory mode
Hello Wenyou,
On Mon, Jan 26, 2015 at 05:42:11PM +0800, Wenyou Yang wrote:
+static int at91_pm_verify_clocks(suspend_state_t state)
{
unsigned long scsr;
int i;
+ /* For PM_SUSPEND_STANDBY, skip verifying the clock */
+ if (state == PM_SUSPEND_STANDBY)
+ return 1;
+
In my opinion we should use the select() already in place in
at91_pm_enter() to do that:
Accepted. Thanks
static int at91_pm_enter(suspend_state_t state) {
at91_pinctrl_gpio_suspend();
switch (state) {
(...)
+ case PM_SUSPEND_MEM:
/*
* Ensure that clocks are in a valid state.
*/
if (!at91_pm_verify_clocks())
goto error;
/* FALLTHROUGH */
+ case PM_SUSPEND_STANDBY:
/*
-* Suspend-to-RAM is like STANDBY plus slow clock mode, so
Sylvain
Best Regards,
Wenyou Yang
Re: [PATCH v2 07/12] pm: at91: the standby mode uses the same sram function as the suspend to memory mode
Hello Wenyou,
On Mon, Jan 26, 2015 at 05:42:11PM +0800, Wenyou Yang wrote:
+static int at91_pm_verify_clocks(suspend_state_t state)
{
unsigned long scsr;
int i;
+ /* For PM_SUSPEND_STANDBY, skip verifying the clock */
+ if (state == PM_SUSPEND_STANDBY)
+ return 1;
+
In my opinion we should use the select() already in place in
at91_pm_enter() to do that:
static int at91_pm_enter(suspend_state_t state)
{
at91_pinctrl_gpio_suspend();
switch (state) {
(...)
+ case PM_SUSPEND_MEM:
/*
* Ensure that clocks are in a valid state.
*/
if (!at91_pm_verify_clocks())
goto error;
/* FALLTHROUGH */
+ case PM_SUSPEND_STANDBY:
/*
- * Suspend-to-RAM is like STANDBY plus slow clock mode, so
Sylvain
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 07/12] pm: at91: the standby mode uses the same sram function as the suspend to memory mode
To simply the PM code, the suspend to standby mode uses the same sram function
as the suspend to memory mode, running in the internal SRAM,
instead of the respective code for each mode.
But for the suspend to standby mode, the master clock doesn't
switch to the slow clock, and the main oscillator doesn't
turn off as well.
Signed-off-by: Wenyou Yang wenyou.y...@atmel.com
---
arch/arm/mach-at91/pm.c | 87 +
arch/arm/mach-at91/pm.h |7 +++
arch/arm/mach-at91/pm_slowclock.S | 18
3 files changed, 65 insertions(+), 47 deletions(-)
diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
index dc2541b..509ac9d 100644
--- a/arch/arm/mach-at91/pm.c
+++ b/arch/arm/mach-at91/pm.c
@@ -71,11 +71,15 @@ static int at91_pm_begin(suspend_state_t state)
* Verify that all the clocks are correct before entering
* slow-clock mode.
*/
-static int at91_pm_verify_clocks(void)
+static int at91_pm_verify_clocks(suspend_state_t state)
{
unsigned long scsr;
int i;
+ /* For PM_SUSPEND_STANDBY, skip verifying the clock */
+ if (state == PM_SUSPEND_STANDBY)
+ return 1;
+
scsr = at91_pmc_read(AT91_PMC_SCSR);
/* USB must not be using PLLB */
@@ -137,62 +141,51 @@ extern void at91_slow_clock(void __iomem *pmc, void
__iomem *ramc0,
void __iomem *ramc1, int memctrl);
extern u32 at91_slow_clock_sz;
+static void at91_pm_suspend(suspend_state_t state)
+{
+ unsigned int pm_data = at91_pm_data.memctrl;
+
+ pm_data |= (state == PM_SUSPEND_MEM) ?
+ AT91_PM_MODE(AT91_PM_SLOW_CLOCK) : 0;
+
+ slow_clock(at91_pmc_base, at91_ramc_base[0],
+ at91_ramc_base[1], pm_data);
+}
+
static int at91_pm_enter(suspend_state_t state)
{
at91_pinctrl_gpio_suspend();
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
+*
+* 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_MEM:
+ case PM_SUSPEND_STANDBY:
/*
-* Suspend-to-RAM is like STANDBY plus slow clock mode, so
-* drivers must suspend more deeply: only the master clock
-* controller may be using the main oscillator.
+* Ensure that clocks are in a valid state.
*/
- case PM_SUSPEND_MEM:
- /*
-* Ensure that clocks are in a valid state.
-*/
- if (!at91_pm_verify_clocks())
- goto error;
-
- /*
-* Enter slow clock mode by switching over to clk32k and
-* turning off the main oscillator; reverse on wakeup.
-*/
- if (slow_clock) {
- slow_clock(at91_pmc_base, at91_ramc_base[0],
- at91_ramc_base[1],
- at91_pm_data.memctrl);
- break;
- } else {
- pr_info(AT91: PM - no slow clock mode enabled
...\n);
- /* FALLTHROUGH leaving master clock alone */
- }
+ if (!at91_pm_verify_clocks(state))
+ goto error;
- /*
-* 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:
- /*
-* NOTE: the Wait-for-Interrupt instruction needs to be
-* in icache so no SDRAM accesses are needed until the
-* wakeup IRQ occurs and self-refresh is terminated.
-* For ARM 926 based chips, this requirement is weaker
-* as at91sam9 can access a RAM in self-refresh mode.
-*/
- if (at91_pm_standby)
- at91_pm_standby();
- break;
+ at91_pm_suspend(state);
- case PM_SUSPEND_ON:
- cpu_do_idle();
- break;
+ break;
