LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Alexandre Belloni <alexandre.belloni@free-electrons.com>
To: Wenyou Yang <wenyou.yang@atmel.com>
Cc: nicolas.ferre@atmel.com, linux@arm.linux.org.uk,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, sylvain.rochet@finsecur.com,
	peda@axentia.se
Subject: Re: [PATCH 07/12] pm: at91: the standby mode uses the same sram function as the suspend to memory mode
Date: Fri, 23 Jan 2015 11:30:06 +0100	[thread overview]
Message-ID: <20150123103006.GH19922@piout.net> (raw)
In-Reply-To: <1421741825-18226-8-git-send-email-wenyou.yang@atmel.com>

On 20/01/2015 at 16:17:00 +0800, Wenyou Yang wrote :
> 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.yang@atmel.com>

That looks quite better, thanks.
Acked-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>

> ---
>  arch/arm/mach-at91/pm.c           |   81 ++++++++++++++++---------------------
>  arch/arm/mach-at91/pm.h           |    6 +++
>  arch/arm/mach-at91/pm_slowclock.S |   18 +++++++++
>  3 files changed, 59 insertions(+), 46 deletions(-)
> 
> diff --git a/arch/arm/mach-at91/pm.c b/arch/arm/mach-at91/pm.c
> index 691e6db..a1010f0 100644
> --- a/arch/arm/mach-at91/pm.c
> +++ b/arch/arm/mach-at91/pm.c
> @@ -145,62 +145,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_SLOW_CLOCK << AT91_PM_MODE_OFFSET) : 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())
> +			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;
>  
> -		default:
> -			pr_debug("AT91: PM - bogus suspend state %d\n", state);
> -			goto error;
> +	case PM_SUSPEND_ON:
> +		cpu_do_idle();
> +		break;
> +
> +	default:
> +		pr_debug("AT91: PM - bogus suspend state %d\n", state);
> +		goto error;
>  	}
>  
>  error:
> diff --git a/arch/arm/mach-at91/pm.h b/arch/arm/mach-at91/pm.h
> index 7664d39..fbfea42 100644
> --- a/arch/arm/mach-at91/pm.h
> +++ b/arch/arm/mach-at91/pm.h
> @@ -15,6 +15,12 @@
>  
>  #include <mach/at91_ramc.h>
>  
> +#define AT91_PM_MEMCTRL_MASK	0x0f
> +#define AT91_PM_MODE_OFFSET	4
> +#define AT91_PM_MODE_MASK	0x0f
> +
> +#define	AT91_PM_SLOW_CLOCK	0x01
> +
>  /*
>   * On all at91 except rm9200 and x40 have the System Controller starts
>   * at address 0xffffc000 and has a size of 16KiB.
> diff --git a/arch/arm/mach-at91/pm_slowclock.S b/arch/arm/mach-at91/pm_slowclock.S
> index 634d819..92d7e63 100644
> --- a/arch/arm/mach-at91/pm_slowclock.S
> +++ b/arch/arm/mach-at91/pm_slowclock.S
> @@ -15,12 +15,15 @@
>  #include <linux/clk/at91_pmc.h>
>  #include <mach/at91_ramc.h>
>  
> +#include "pm.h"
> +
>  pmc	.req	r0
>  sdramc	.req	r1
>  ramc1	.req	r2
>  memctrl	.req	r3
>  tmp1	.req	r4
>  tmp2	.req	r5
> +mode	.req	r6
>  
>  /*
>   * Wait until master clock is ready (after switching master clock source)
> @@ -72,6 +75,13 @@ ENTRY(at91_slow_clock)
>  	mov	tmp1, #0
>  	mcr	p15, 0, tmp1, c7, c10, 4
>  
> +	mov	tmp1, memctrl
> +	mov	tmp2, tmp1, lsr#AT91_PM_MODE_OFFSET
> +	and	mode, tmp2, #AT91_PM_MODE_MASK
> +
> +	mov	tmp1, memctrl
> +	and	memctrl, tmp1, #AT91_PM_MEMCTRL_MASK
> +
>  	cmp	memctrl, #AT91_MEMCTRL_MC
>  	bne	ddr_sr_enable
>  
> @@ -144,6 +154,9 @@ sdr_sr_enable:
>  	str	tmp1, [sdramc, #AT91_SDRAMC_LPR]
>  
>  sdr_sr_done:
> +	tst	mode, #AT91_PM_SLOW_CLOCK
> +	beq	skip_disable_clock
> +
>  	/* Save Master clock setting */
>  	ldr	tmp1, [pmc, #AT91_PMC_MCKR]
>  	str	tmp1, .saved_mckr
> @@ -169,9 +182,13 @@ sdr_sr_done:
>  	bic	tmp1, tmp1, #AT91_PMC_MOSCEN
>  	str	tmp1, [pmc, #AT91_CKGR_MOR]
>  
> +skip_disable_clock:
>  	/* Wait for interrupt */
>  	mcr	p15, 0, tmp1, c7, c0, 4
>  
> +	tst	mode, #AT91_PM_SLOW_CLOCK
> +	beq	skip_enable_clock
> +
>  	/* Turn on the main oscillator */
>  	ldr	tmp1, [pmc, #AT91_CKGR_MOR]
>  	orr	tmp1, tmp1, #AT91_PMC_MOSCEN
> @@ -199,6 +216,7 @@ sdr_sr_done:
>  
>  	wait_mckrdy
>  
> +skip_enable_clock:
>  	/*
>  	 * at91rm9200 Memory controller
>  	 * Do nothing - self-refresh is automatically disabled.
> -- 
> 1.7.9.5
> 

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

  reply	other threads:[~2015-01-23 10:30 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-20  8:16 [PATCH 00/12] AT91 pm cleanup for 3.20 Wenyou Yang
2015-01-20  8:16 ` [PATCH 01/12] pm: at91: pm_slowclock: improve reliability of suspend/resume Wenyou Yang
2015-01-20  8:16 ` [PATCH 02/12] pm: at91: Workaround DDRSDRC self-refresh bug with LPDDR1 memories Wenyou Yang
2015-01-20  8:16 ` [PATCH 03/12] pm: at91: pm_slowclock: remove the unused code related with SLOWDOWN_MASTER_CLOCK Wenyou Yang
2015-01-20  8:16 ` [PATCH 04/12] pm: at91: move the copying the sram function to the sram initializationi phase Wenyou Yang
2015-01-20  8:16 ` [PATCH 05/12] ARM: at91: move select SRAM to ARCH_AT91 Wenyou Yang
2015-01-23 10:24   ` Alexandre Belloni
2015-01-26  1:10     ` Yang, Wenyou
2015-01-20  8:16 ` [PATCH 06/12] pm: at91: remove the config item CONFIG_AT91_SLOW_CLOCK Wenyou Yang
2015-01-20  8:17 ` [PATCH 07/12] pm: at91: the standby mode uses the same sram function as the suspend to memory mode Wenyou Yang
2015-01-23 10:30   ` Alexandre Belloni [this message]
2015-01-23 16:50   ` Sylvain Rochet
2015-01-23 23:13     ` Alexandre Belloni
2015-01-27  3:08       ` Yang, Wenyou
2015-01-23 17:32   ` Sylvain Rochet
2015-01-23 23:02     ` Alexandre Belloni
2015-01-26  3:08       ` Yang, Wenyou
2015-01-26  3:06     ` Yang, Wenyou
2015-01-20  8:17 ` [PATCH 08/12] pm: at91: rename file name: pm_slowclock.S -->pm_suspend.S Wenyou Yang
2015-01-23 19:17   ` Sylvain Rochet
2015-01-23 23:17     ` Alexandre Belloni
2015-01-25 13:30       ` Sylvain Rochet
2015-01-26  1:25     ` Yang, Wenyou
2015-01-20  8:17 ` [PATCH 09/12] pm: at91: rename function name: at91_slow_clock()-->at91_pm_suspend_sram_fn Wenyou Yang
2015-01-20  8:24 ` Wenyou Yang
2015-01-20  8:24 ` [PATCH 10/12] pm: at91: remove the at91_xxx_standby() function definitions in the pm.h Wenyou Yang
2015-01-20  8:25 ` [PATCH 11/12] pm: at91: remove the struct ramc_ids .data at91_xxx_standby members Wenyou Yang
2015-01-20  8:26 ` [PATCH 12/12] pm: at91: amend the pm_suspend entry for at91_cpuidle_device Wenyou Yang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150123103006.GH19922@piout.net \
    --to=alexandre.belloni@free-electrons.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@arm.linux.org.uk \
    --cc=nicolas.ferre@atmel.com \
    --cc=peda@axentia.se \
    --cc=sylvain.rochet@finsecur.com \
    --cc=wenyou.yang@atmel.com \
    --subject='Re: [PATCH 07/12] pm: at91: the standby mode uses the same sram function as the suspend to memory mode' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).