LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Sylvain Rochet <sylvain.rochet@finsecur.com>
To: Wenyou Yang <wenyou.yang@atmel.com>
Cc: nicolas.ferre@atmel.com, linux@arm.linux.org.uk,
	linux-kernel@vger.kernel.org,
	alexandre.belloni@free-electrons.com, peda@axentia.se,
	linux-arm-kernel@lists.infradead.org
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 18:32:34 +0100	[thread overview]
Message-ID: <20150123173234.GA26382@gradator.net> (raw)
In-Reply-To: <1421741825-18226-8-git-send-email-wenyou.yang@atmel.com>

Hello Wenyou,

On Tue, Jan 20, 2015 at 04:17:00PM +0800, Wenyou Yang wrote:
> 
> 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
  (...)
>  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:
   (...)
> -		case PM_SUSPEND_MEM:
> -			/*
> -			 * Ensure that clocks are in a valid state.
> -			 */
> -			if (!at91_pm_verify_clocks())
> -				goto error;
   (...)
> +		if (!at91_pm_verify_clocks())
> +			goto error;
>  
   (...)
> -		case PM_SUSPEND_STANDBY:
> -			/*
> -			 * NOTE: the Wait-for-Interrupt instruction needs to be

By doing that at91_pm_verify_clocks() is now called for both MEM and 
STANDBY targets.

In my opinion this function is misnamed and should be called 
at91_pm_verify_clocks_for_slow_clock_mode(). This function actually 
checks if we can safely switch to slow clock mode, if some peripherals 
are still using the master clock, we abort the suspend because we can't 
suspend in good condition. Hard unclocking peripherals which ask for a 
soft stop, like USB controllers, is something we should avoid doing.

This function checks if USB PLL and PLL B are stopped, if PCK0..PCK3 are 
stopped too (or just using the 32k clock). If all drivers suspended 
correctly this is the state we expect and we can suspend in a deep 
state.

Not this is currently not the case in linux-next, suspend/resume support 
to all Atmel USB drivers (ehci-atmel,ohci-at91,atmel_usba,at91_udc) are 
in my series:
 [PATCHv7 0/6] USB: host: Atmel OHCI and EHCI drivers improvements
   <1421761144-11767-1-git-send-email-sylvain.rochet@finsecur.com>
 [PATCHv6 0/5] USB: gadget: atmel_usba_udc: Driver improvements 
   <1421945805-31129-1-git-send-email-sylvain.rochet@finsecur.com>

We are not going to change any clock for STANDBY target, there is no 
clock to check, so we don't need to call at91_pm_verify_clocks() for 
this target.

Sylvain

  parent reply	other threads:[~2015-01-23 17:32 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
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 [this message]
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=20150123173234.GA26382@gradator.net \
    --to=sylvain.rochet@finsecur.com \
    --cc=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=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).