public inbox for linuxppc-dev@ozlabs.org 
 help / color / mirror / Atom feed
From: Scott Wood <scottwood@freescale•com>
To: Zhao Chenhui <chenhui.zhao@freescale•com>
Cc: linuxppc-dev@lists•ozlabs.org
Subject: Re: [PATCH 3/7] powerpc/85xx: add sleep and deep sleep support
Date: Fri, 4 Nov 2011 13:45:21 -0500	[thread overview]
Message-ID: <4EB432C1.1000601@freescale.com> (raw)
In-Reply-To: <1320410014-14453-1-git-send-email-chenhui.zhao@freescale.com>

On 11/04/2011 07:33 AM, Zhao Chenhui wrote:
> +/* Cast the ccsrbar to 64-bit parameter so that the assembly
> + * code can be compatible with both 32-bit & 36-bit */
> +extern void mpc85xx_enter_deep_sleep(u64 ccsrbar, u32 powmgtreq);

/*
 * Please use proper
 * Linux multi-line comment format.
 */

>  static int pmc_suspend_enter(suspend_state_t state)
>  {
>  	int ret;
> +	u32 powmgtreq = 0x00500000;

Where does this 0x00500000 come from?  Please symbolically define
individual bits.

The comment in the asm code says it should be 0x00100000, BTW.

> +
> +	switch (state) {
> +	case PM_SUSPEND_MEM:
> +#ifdef CONFIG_SPE
> +		enable_kernel_spe();
> +#endif

Should comment that currently only e500v2 hardware supports deep sleep
-- else we'd need to save normal FP here.

> +		pr_debug("Entering deep sleep\n");
> +
> +		local_irq_disable();
> +		mpc85xx_enter_deep_sleep(get_immrbase(),
> +				powmgtreq);
> +		pr_debug("Resumed from deep sleep\n");
> +
> +		return 0;
> +
> +	/* else fall-through */
> +	case PM_SUSPEND_STANDBY:

What fall-through?  You just returned.

> +	}
>  
> -	/* Upon resume, wait for SLP bit to be clear. */
> -	ret = spin_event_timeout((in_be32(&pmc_regs->pmcsr) & PMCSR_SLP) == 0,
> -				 10000, 10) ? 0 : -ETIMEDOUT;
> -	if (ret)
> -		dev_err(pmc_dev, "tired waiting for SLP bit to clear\n");
> -	return ret;
>  }

Remove that blank line as well.

> @@ -58,13 +101,23 @@ static const struct platform_suspend_ops pmc_suspend_ops = {
>  	.enter = pmc_suspend_enter,
>  };
>  
> -static int pmc_probe(struct platform_device *ofdev)
> +static int pmc_probe(struct platform_device *pdev)
>  {
> -	pmc_regs = of_iomap(ofdev->dev.of_node, 0);
> +	struct device_node *np = pdev->dev.of_node;
> +
> +	pmc_regs = of_iomap(pdev->dev.of_node, 0);
>  	if (!pmc_regs)
>  		return -ENOMEM;
>  
> -	pmc_dev = &ofdev->dev;
> +	has_deep_sleep = 0;
> +	if (of_device_is_compatible(np, "fsl,mpc8536-pmc"))
> +		has_deep_sleep = 1;
> +
> +	has_lossless = 0;
> +	if (of_device_is_compatible(np, "fsl,p1022-pmc"))
> +		has_lossless = 1;
> +

You never use has_lossless.

-Scott

  parent reply	other threads:[~2011-11-04 18:45 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-11-04 12:33 [PATCH 3/7] powerpc/85xx: add sleep and deep sleep support Zhao Chenhui
2011-11-04 17:36 ` Felix Radensky
2011-11-08 10:27   ` Li Yang-R58472
2011-11-04 18:45 ` Scott Wood [this message]
2011-11-08 10:32   ` Li Yang-R58472
2011-11-08 17:31     ` Scott Wood
2011-11-04 22:43 ` Felix Radensky

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=4EB432C1.1000601@freescale.com \
    --to=scottwood@freescale$(echo .)com \
    --cc=chenhui.zhao@freescale$(echo .)com \
    --cc=linuxppc-dev@lists$(echo .)ozlabs.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox