All of lore.kernel.org
 help / color / mirror / Atom feed
From: jason.chen@freescale.com (Jason Chen)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] arm: imx6q: bypass anatop regulator during suspend
Date: Tue, 10 Jan 2012 10:03:14 +0800	[thread overview]
Message-ID: <20120110020313.GA3204@weitway> (raw)
In-Reply-To: <20120108082650.GA20216@S2101-09.ap.freescale.net>

On Sun, Jan 08, 2012 at 04:27:23PM +0800, Shawn Guo wrote:
Ok, then we wait for anatop regulator first.
Thanks
> For subject, please make it be like 'ARM: imx6q: ...'
> 
> On Thu, Jan 05, 2012 at 01:20:42PM +0800, Jason Chen wrote:
> > enable bit CCM_CLPCR_wb_per_at_lpm to decrease leakage during suspend.
> 
> It looks odd to have capital and lower case mixed in name
> 'CCM_CLPCR_wb_per_at_lpm'.
> 
> > enable bit CCM_CCR_RBC_EN to disable/bypass anatop regulator during suspend.
> > 
> > Signed-off-by: Jason Chen <jason.chen@linaro.org>
> > Signed-off-by: Jason Chen <jason.chen@freescale.com>
> > ---
> >  arch/arm/mach-imx/clock-imx6q.c |   31 +++++++++++++++---------
> >  arch/arm/mach-imx/pm-imx6q.c    |   48 +++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 67 insertions(+), 12 deletions(-)
> > 
> > diff --git a/arch/arm/mach-imx/clock-imx6q.c b/arch/arm/mach-imx/clock-imx6q.c
> > index 56a7a3f..11d2453 100644
> > --- a/arch/arm/mach-imx/clock-imx6q.c
> > +++ b/arch/arm/mach-imx/clock-imx6q.c
> > @@ -115,6 +115,8 @@
> >  #define CG14		28
> >  #define CG15		30
> >  
> > +#define BM_CCR_RBC_EN			(0x1 << 27)
> > +
> >  #define BM_CCSR_PLL1_SW_SEL		(0x1 << 2)
> >  #define BM_CCSR_STEP_SEL		(0x1 << 8)
> >  
> > @@ -1916,33 +1918,38 @@ static struct clk_lookup lookups[] = {
> >  
> >  int imx6q_set_lpm(enum mxc_cpu_pwr_mode mode)
> >  {
> > -	u32 val = readl_relaxed(CLPCR);
> > +	u32 clpcr = readl_relaxed(CLPCR);
> > +	u32 ccr = readl_relaxed(CCR);
> >  
> > -	val &= ~BM_CLPCR_LPM;
> > +	clpcr &= ~(BM_CLPCR_LPM | BM_CLPCR_VSTBY | BM_CLPCR_SBYOS
> > +			| BM_CLPCR_STBY_COUNT | BM_CLPCR_WB_PER_AT_LPM);
> > +	ccr &= ~(BM_CCR_RBC_EN);
> 
> Unneeded parentheses.
> 
> >  	switch (mode) {
> >  	case WAIT_CLOCKED:
> >  		break;
> >  	case WAIT_UNCLOCKED:
> > -		val |= 0x1 << BP_CLPCR_LPM;
> > +		clpcr |= 0x1 << BP_CLPCR_LPM;
> >  		break;
> >  	case STOP_POWER_ON:
> > -		val |= 0x2 << BP_CLPCR_LPM;
> > +		clpcr |= 0x2 << BP_CLPCR_LPM;
> >  		break;
> >  	case WAIT_UNCLOCKED_POWER_OFF:
> > -		val |= 0x1 << BP_CLPCR_LPM;
> > -		val &= ~BM_CLPCR_VSTBY;
> > -		val &= ~BM_CLPCR_SBYOS;
> > +		clpcr |= 0x1 << BP_CLPCR_LPM;
> >  		break;
> >  	case STOP_POWER_OFF:
> > -		val |= 0x2 << BP_CLPCR_LPM;
> > -		val |= 0x3 << BP_CLPCR_STBY_COUNT;
> > -		val |= BM_CLPCR_VSTBY;
> > -		val |= BM_CLPCR_SBYOS;
> > +		clpcr |= 0x2 << BP_CLPCR_LPM;
> > +		clpcr |= 0x3 << BP_CLPCR_STBY_COUNT;
> > +		clpcr |= BM_CLPCR_VSTBY;
> > +		clpcr |= BM_CLPCR_SBYOS;
> > +		clpcr |= BM_CLPCR_WB_PER_AT_LPM;
> > +		/* assert anatop_reg_bypass signal */
> 
> We do not have to put the comment here, as it does not help too much.
> 
> > +		ccr |= BM_CCR_RBC_EN;
> >  		break;
> >  	default:
> >  		return -EINVAL;
> >  	}
> > -	writel_relaxed(val, CLPCR);
> > +	writel_relaxed(clpcr, CLPCR);
> > +	writel_relaxed(ccr, CCR);
> >  
> >  	return 0;
> >  }
> > diff --git a/arch/arm/mach-imx/pm-imx6q.c b/arch/arm/mach-imx/pm-imx6q.c
> > index f20f191..72a1399 100644
> > --- a/arch/arm/mach-imx/pm-imx6q.c
> > +++ b/arch/arm/mach-imx/pm-imx6q.c
> > @@ -13,6 +13,7 @@
> >  #include <linux/init.h>
> >  #include <linux/io.h>
> >  #include <linux/of.h>
> > +#include <linux/of_address.h>
> >  #include <linux/suspend.h>
> >  #include <asm/cacheflush.h>
> >  #include <asm/proc-fns.h>
> > @@ -22,6 +23,41 @@
> >  #include <mach/hardware.h>
> >  
> >  extern unsigned long phys_l2x0_saved_regs;
> > +static void __iomem *anatop_base;
> > +
> > +#define ANATOP_REG_2P5			0x130
> > +#define BM_ANATOP_REG_2P5_WEAK_EN	(0x1 << 18)
> > +
> > +static int imx6q_anatop_reg_pre_suspend(void)
> > +{
> > +	/*
> > +	 * enable anatop weak 2p5 regulator, which should use
> > +	 * regulator API after anatop regulator implementation.
> > +	 */
> > +	if (anatop_base) {
> > +		unsigned int reg_2p5;
> > +		reg_2p5 = readl(anatop_base + ANATOP_REG_2P5);
> > +		reg_2p5 |= BM_ANATOP_REG_2P5_WEAK_EN;
> > +		writel(reg_2p5, anatop_base + ANATOP_REG_2P5);
> > +	} else
> > +		return -EINVAL;
> > +
> > +	return 0;
> > +}
> > +
> > +static void imx6q_anatop_reg_post_resume(void)
> > +{
> > +	/*
> > +	 * disable anatop weak 2p5 regulator, which should use
> > +	 * regulator API after anatop regulator implementation.
> > +	 */
> > +	if (anatop_base) {
> > +		unsigned int reg_2p5;
> > +		reg_2p5 = readl(anatop_base + ANATOP_REG_2P5);
> > +		reg_2p5 &= ~BM_ANATOP_REG_2P5_WEAK_EN;
> > +		writel(reg_2p5, anatop_base + ANATOP_REG_2P5);
> > +	}
> > +}
> >  
> I do not like to have these anatop functions implemented in pm-imx6q.c.
> 
> >  static int imx6q_suspend_finish(unsigned long val)
> >  {
> > @@ -33,6 +69,8 @@ static int imx6q_pm_enter(suspend_state_t state)
> >  {
> >  	switch (state) {
> >  	case PM_SUSPEND_MEM:
> > +		if (imx6q_anatop_reg_pre_suspend() < 0)
> > +			return -EINVAL;
> >  		imx6q_set_lpm(STOP_POWER_OFF);
> >  		imx_gpc_pre_suspend();
> >  		imx_set_cpu_jump(0, v7_cpu_resume);
> > @@ -40,6 +78,7 @@ static int imx6q_pm_enter(suspend_state_t state)
> >  		cpu_suspend(0, imx6q_suspend_finish);
> >  		imx_smp_prepare();
> >  		imx_gpc_post_resume();
> > +		imx6q_anatop_reg_post_resume();
> >  		break;
> >  	default:
> >  		return -EINVAL;
> > @@ -55,6 +94,15 @@ static const struct platform_suspend_ops imx6q_pm_ops = {
> >  
> >  void __init imx6q_pm_init(void)
> >  {
> > +	struct device_node *np;
> > +
> > +	/*
> > +	 * remap anatop to adjust anatop regulator, which should
> > +	 * remove after anatop regulator driver implementation.
> > +	 */
> > +	np = of_find_compatible_node(NULL, NULL, "fsl,imx6q-anatop");
> > +	anatop_base = of_iomap(np, 0);
> > +
> 
> I would wait for anatop regulator driver being ready before we add
> this bypass support, so that we can save all this unnecessary churn
> in the pm-imx6q.c.
> 
> Regards,
> Shawn
> 
> >  	/*
> >  	 * The l2x0 core code provides an infrastucture to save and restore
> >  	 * l2x0 registers across suspend/resume cycle.  But because imx6q
> > -- 
> > 1.7.4.1
> > 
> > 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

      reply	other threads:[~2012-01-10  2:03 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-01-05  5:20 [PATCH] arm: imx6q: bypass anatop regulator during suspend Jason Chen
2012-01-08  8:27 ` Shawn Guo
2012-01-10  2:03   ` Jason Chen [this message]

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=20120110020313.GA3204@weitway \
    --to=jason.chen@freescale.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.