| Message ID | 1363801180-8284-3-git-send-email-b20788@freescale.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Mar 20, 2013 at 01:39:40PM -0400, Anson Huang wrote: > RBC is to control whether some ANATOP sub modules > can enter lpm mode when SOC is into STOP mode, if > RBC is enabled and PMIC_VSTBY_REQ is set, ANATOP > will have below behaviors: > > 1. Digital LDOs(CORE, SOC and PU) are bypassed; > 2. Analog LDOs(1P1, 2P5, 3P0) are disabled; > > As the 2P5 is necessary for DRAM IO pre-drive in > STOP mode, so we need to enable weak 2P5 in STOP > mode when 2P5 LDO is disabled. > > For RBC settings, there are some rules as below > due to hardware designe: s/designe/design > > 1. All interrupts must be masked during operating > RBC registers; > 2. At least 2 CKIL(32K) cycles is needed after the > RBC setting is changed. > > Signed-off-by: Anson Huang <b20788@freescale.com> > --- > arch/arm/mach-imx/anatop.c | 19 +++++++++++++++++++ > arch/arm/mach-imx/clk-imx6q.c | 35 +++++++++++++++++++++++++++++++++++ > arch/arm/mach-imx/common.h | 3 +++ > arch/arm/mach-imx/gpc.c | 21 ++++++++++++++++++++- > arch/arm/mach-imx/pm-imx6q.c | 2 ++ > 5 files changed, 79 insertions(+), 1 deletion(-) > > diff --git a/arch/arm/mach-imx/anatop.c b/arch/arm/mach-imx/anatop.c > index 8f6ab27..38b4f44 100644 > --- a/arch/arm/mach-imx/anatop.c > +++ b/arch/arm/mach-imx/anatop.c > @@ -18,12 +18,29 @@ > > #define REG_SET 0x4 > #define REG_CLR 0x8 > +#define ANA_MISC0 0x150 > #define ANA_REG_CORE 0x140 > +#define ANA_REG_2P5 0x130 Please put these registers in acceding offset. > > +#define BM_ANADIG_REG_2P5_ENABLE_WEAK_LINREG 0x40000 > +#define BM_ANADIG_ANA_MISC0_STOP_MODE_CONFIG 0x1000 > #define BM_ANADIG_REG_CORE_FET_ODRIVE 0x20000000 > > static struct regmap *anatop; > > +void imx_anatop_enable_weak2p5(bool enable) static > +{ > + u32 val; > + > + regmap_read(anatop, ANA_MISC0, &val); > + > + /* can only be enabled when stop_mode_config is clear. */ > + regmap_write(anatop, ANA_REG_2P5 + ((enable && > + ((val & BM_ANADIG_ANA_MISC0_STOP_MODE_CONFIG) > + == 0)) ? REG_SET : REG_CLR), > + BM_ANADIG_REG_2P5_ENABLE_WEAK_LINREG); It's a little difficult to parse the expression even it's on the same line, not mentioning with indentation. There are so many levels of parentheses. ((enable && ((val & BM_ANADIG_ANA_MISC0_STOP_MODE_CONFIG) == 0)) ? REG_SET : REG_CLR) What about rewriting it as below to make it easy for people to read? reg = ANA_REG_2P5; reg += (enable && (val & BM_ANADIG_ANA_MISC0_STOP_MODE_CONFIG) == 0) ? REG_SET : REG_CLR; regmap_write(anatop, reg, BM_ANADIG_REG_2P5_ENABLE_WEAK_LINREG); > +} > + > void imx_anatop_enable_fet_odrive(bool enable) > { > regmap_write(anatop, ANA_REG_CORE + (enable ? > @@ -32,12 +49,14 @@ void imx_anatop_enable_fet_odrive(bool enable) > > void imx_anatop_pre_suspend(void) > { > + imx_anatop_enable_weak2p5(true); > imx_anatop_enable_fet_odrive(true); > } > > void imx_anatop_post_resume(void) > { > imx_anatop_enable_fet_odrive(false); > + imx_anatop_enable_weak2p5(false); > } > > void __init imx_anatop_init(void) > diff --git a/arch/arm/mach-imx/clk-imx6q.c b/arch/arm/mach-imx/clk-imx6q.c > index b365efc..646ce12 100644 > --- a/arch/arm/mach-imx/clk-imx6q.c > +++ b/arch/arm/mach-imx/clk-imx6q.c > @@ -14,6 +14,7 @@ > #include <linux/types.h> > #include <linux/clk.h> > #include <linux/clkdev.h> > +#include <linux/delay.h> > #include <linux/err.h> > #include <linux/io.h> > #include <linux/of.h> > @@ -25,6 +26,8 @@ > > #define CCR 0x0 > #define BM_CCR_WB_COUNT (0x7 << 16) > +#define BM_CCR_RBC_BYPASS_COUNT (0x3f << 21) > +#define BM_CCR_RBC_EN (0x1 << 27) > > #define CCGR0 0x68 > #define CCGR1 0x6c > @@ -70,6 +73,38 @@ void imx6q_set_chicken_bit(void) > writel_relaxed(val, ccm_base + CGPR); > } > > +void imx6q_set_rbc(bool enable) Same question as imx6q_set_wb, can we manage to call it in imx6q_set_lpm()? The intension behind this is we should try to a centralized place to configure CCM registers/bits for suspend instead of exporting so many functions to suspend routine to call individually. > +{ > + u32 val; > + > + /* > + * need to mask all interrupts in GPC before > + * operating RBC configurations > + */ > + imx_gpc_mask_all(); > + > + /* configurate RBC enable bit */ s/configurate/configure > + val = readl_relaxed(ccm_base + CCR); > + val &= ~BM_CCR_RBC_EN; > + val |= enable ? BM_CCR_RBC_EN : 0; > + writel_relaxed(val, ccm_base + CCR); > + > + /* configurate RBC count */ Ditto > + val = readl_relaxed(ccm_base + CCR); > + val &= ~BM_CCR_RBC_BYPASS_COUNT; > + val |= enable ? BM_CCR_RBC_BYPASS_COUNT : 0; > + writel(val, ccm_base + CCR); > + > + /* > + * need to delay at least 2 cycles of CKIL(32K) > + * due to hardware design requirement, which is > + * ~61us, here we use 65us for safe > + */ > + udelay(65); Nit, have a new line here. > + /* restore GPC interrupt mask settings */ > + imx_gpc_restore_all(); > +} > + > void imx6q_set_wb(bool enable) > { > u32 val; > diff --git a/arch/arm/mach-imx/common.h b/arch/arm/mach-imx/common.h > index b9125cf..66fe41c 100644 > --- a/arch/arm/mach-imx/common.h > +++ b/arch/arm/mach-imx/common.h > @@ -129,11 +129,14 @@ extern void imx_src_prepare_restart(void); > extern void imx_gpc_init(void); > extern void imx_gpc_pre_suspend(void); > extern void imx_gpc_post_resume(void); > +extern void imx_gpc_mask_all(void); > +extern void imx_gpc_restore_all(void); > extern void imx_anatop_init(void); > extern void imx_anatop_pre_suspend(void); > extern void imx_anatop_post_resume(void); > extern int imx6q_set_lpm(enum mxc_cpu_pwr_mode mode); > extern void imx6q_set_chicken_bit(void); > +extern void imx6q_set_rbc(bool enable); > extern void imx6q_set_wb(bool enable); Since they are taking "enable" parameter, can we rename them as below? void imx6q_enable_rbc(bool enable); void imx6q_enable_wb(bool enable); > extern void imx_cpu_die(unsigned int cpu); > extern int imx_cpu_kill(unsigned int cpu); > diff --git a/arch/arm/mach-imx/gpc.c b/arch/arm/mach-imx/gpc.c > index a96ccc7..504049e 100644 > --- a/arch/arm/mach-imx/gpc.c > +++ b/arch/arm/mach-imx/gpc.c > @@ -1,5 +1,5 @@ > /* > - * Copyright 2011 Freescale Semiconductor, Inc. > + * Copyright 2011-2013 Freescale Semiconductor, Inc. > * Copyright 2011 Linaro Ltd. > * > * The code contained herein is licensed under the GNU General Public > @@ -68,6 +68,25 @@ static int imx_gpc_irq_set_wake(struct irq_data *d, unsigned int on) > return 0; > } > > +void imx_gpc_mask_all(void) > +{ > + void __iomem *reg_imr1 = gpc_base + GPC_IMR1; > + int i; > + > + for (i = 0; i < IMR_NUM; i++) > + writel_relaxed(0xffffffff, reg_imr1 + i * 4); > + > +} > + > +void imx_gpc_restore_all(void) > +{ > + void __iomem *reg_imr1 = gpc_base + GPC_IMR1; > + int i; > + > + for (i = 0; i < IMR_NUM; i++) > + writel_relaxed(gpc_saved_imrs[i], reg_imr1 + i * 4); > +} > + > static void imx_gpc_irq_unmask(struct irq_data *d) > { > void __iomem *reg; > diff --git a/arch/arm/mach-imx/pm-imx6q.c b/arch/arm/mach-imx/pm-imx6q.c > index 57ca274..24ac7bc 100644 > --- a/arch/arm/mach-imx/pm-imx6q.c > +++ b/arch/arm/mach-imx/pm-imx6q.c > @@ -36,10 +36,12 @@ static int imx6q_pm_enter(suspend_state_t state) > imx_gpc_pre_suspend(); This call saves mask in gpc_saved_imrs and mask all interrupts except those wakeup sources ... > imx_anatop_pre_suspend(); > imx_set_cpu_jump(0, v7_cpu_resume); > + imx6q_set_rbc(true); At the end of imx6q_set_rbc(), imx_gpc_restore_all() will be called to restore mask bits with gpc_saved_imrs, so wakeup source handling gets lost, and any in-use interrupt will wake up system, which is undesirable. Shawn > imx6q_set_wb(true); > /* Zzz ... */ > cpu_suspend(0, imx6q_suspend_finish); > imx6q_set_wb(false); > + imx6q_set_rbc(false); > imx_smp_prepare(); > imx_anatop_post_resume(); > imx_gpc_post_resume(); > -- > 1.7.9.5 > >
On Wed, Mar 20, 2013 at 01:39:40PM -0400, Anson Huang wrote: > RBC is to control whether some ANATOP sub modules > can enter lpm mode when SOC is into STOP mode, if > RBC is enabled and PMIC_VSTBY_REQ is set, ANATOP > will have below behaviors: > > 1. Digital LDOs(CORE, SOC and PU) are bypassed; > 2. Analog LDOs(1P1, 2P5, 3P0) are disabled; > > As the 2P5 is necessary for DRAM IO pre-drive in > STOP mode, so we need to enable weak 2P5 in STOP > mode when 2P5 LDO is disabled. > > For RBC settings, there are some rules as below > due to hardware designe: s/designe/design > > 1. All interrupts must be masked during operating > RBC registers; > 2. At least 2 CKIL(32K) cycles is needed after the > RBC setting is changed. > > Signed-off-by: Anson Huang <b20788@freescale.com> > --- > arch/arm/mach-imx/anatop.c | 19 +++++++++++++++++++ > arch/arm/mach-imx/clk-imx6q.c | 35 +++++++++++++++++++++++++++++++++++ > arch/arm/mach-imx/common.h | 3 +++ > arch/arm/mach-imx/gpc.c | 21 ++++++++++++++++++++- > arch/arm/mach-imx/pm-imx6q.c | 2 ++ > 5 files changed, 79 insertions(+), 1 deletion(-) > > diff --git a/arch/arm/mach-imx/anatop.c b/arch/arm/mach-imx/anatop.c > index 8f6ab27..38b4f44 100644 > --- a/arch/arm/mach-imx/anatop.c > +++ b/arch/arm/mach-imx/anatop.c > @@ -18,12 +18,29 @@ > > #define REG_SET 0x4 > #define REG_CLR 0x8 > +#define ANA_MISC0 0x150 > #define ANA_REG_CORE 0x140 > +#define ANA_REG_2P5 0x130 Please put these registers in acceding offset. > > +#define BM_ANADIG_REG_2P5_ENABLE_WEAK_LINREG 0x40000 > +#define BM_ANADIG_ANA_MISC0_STOP_MODE_CONFIG 0x1000 > #define BM_ANADIG_REG_CORE_FET_ODRIVE 0x20000000 > > static struct regmap *anatop; > > +void imx_anatop_enable_weak2p5(bool enable) static > +{ > + u32 val; > + > + regmap_read(anatop, ANA_MISC0, &val); > + > + /* can only be enabled when stop_mode_config is clear. */ > + regmap_write(anatop, ANA_REG_2P5 + ((enable && > + ((val & BM_ANADIG_ANA_MISC0_STOP_MODE_CONFIG) > + == 0)) ? REG_SET : REG_CLR), > + BM_ANADIG_REG_2P5_ENABLE_WEAK_LINREG); It's a little difficult to parse the expression even it's on the same line, not mentioning with indentation. There are so many levels of parentheses. ((enable && ((val & BM_ANADIG_ANA_MISC0_STOP_MODE_CONFIG) == 0)) ? REG_SET : REG_CLR) What about rewriting it as below to make it easy for people to read? reg = ANA_REG_2P5; reg += (enable && (val & BM_ANADIG_ANA_MISC0_STOP_MODE_CONFIG) == 0) ? REG_SET : REG_CLR; regmap_write(anatop, reg, BM_ANADIG_REG_2P5_ENABLE_WEAK_LINREG); > +} > + > void imx_anatop_enable_fet_odrive(bool enable) > { > regmap_write(anatop, ANA_REG_CORE + (enable ? > @@ -32,12 +49,14 @@ void imx_anatop_enable_fet_odrive(bool enable) > > void imx_anatop_pre_suspend(void) > { > + imx_anatop_enable_weak2p5(true); > imx_anatop_enable_fet_odrive(true); > } > > void imx_anatop_post_resume(void) > { > imx_anatop_enable_fet_odrive(false); > + imx_anatop_enable_weak2p5(false); > } > > void __init imx_anatop_init(void) > diff --git a/arch/arm/mach-imx/clk-imx6q.c b/arch/arm/mach-imx/clk-imx6q.c > index b365efc..646ce12 100644 > --- a/arch/arm/mach-imx/clk-imx6q.c > +++ b/arch/arm/mach-imx/clk-imx6q.c > @@ -14,6 +14,7 @@ > #include <linux/types.h> > #include <linux/clk.h> > #include <linux/clkdev.h> > +#include <linux/delay.h> > #include <linux/err.h> > #include <linux/io.h> > #include <linux/of.h> > @@ -25,6 +26,8 @@ > > #define CCR 0x0 > #define BM_CCR_WB_COUNT (0x7 << 16) > +#define BM_CCR_RBC_BYPASS_COUNT (0x3f << 21) > +#define BM_CCR_RBC_EN (0x1 << 27) > > #define CCGR0 0x68 > #define CCGR1 0x6c > @@ -70,6 +73,38 @@ void imx6q_set_chicken_bit(void) > writel_relaxed(val, ccm_base + CGPR); > } > > +void imx6q_set_rbc(bool enable) Same question as imx6q_set_wb, can we manage to call it in imx6q_set_lpm()? The intension behind this is we should try to a centralized place to configure CCM registers/bits for suspend instead of exporting so many functions to suspend routine to call individually. > +{ > + u32 val; > + > + /* > + * need to mask all interrupts in GPC before > + * operating RBC configurations > + */ > + imx_gpc_mask_all(); > + > + /* configurate RBC enable bit */ s/configurate/configure > + val = readl_relaxed(ccm_base + CCR); > + val &= ~BM_CCR_RBC_EN; > + val |= enable ? BM_CCR_RBC_EN : 0; > + writel_relaxed(val, ccm_base + CCR); > + > + /* configurate RBC count */ Ditto > + val = readl_relaxed(ccm_base + CCR); > + val &= ~BM_CCR_RBC_BYPASS_COUNT; > + val |= enable ? BM_CCR_RBC_BYPASS_COUNT : 0; > + writel(val, ccm_base + CCR); > + > + /* > + * need to delay at least 2 cycles of CKIL(32K) > + * due to hardware design requirement, which is > + * ~61us, here we use 65us for safe > + */ > + udelay(65); Nit, have a new line here. > + /* restore GPC interrupt mask settings */ > + imx_gpc_restore_all(); > +} > + > void imx6q_set_wb(bool enable) > { > u32 val; > diff --git a/arch/arm/mach-imx/common.h b/arch/arm/mach-imx/common.h > index b9125cf..66fe41c 100644 > --- a/arch/arm/mach-imx/common.h > +++ b/arch/arm/mach-imx/common.h > @@ -129,11 +129,14 @@ extern void imx_src_prepare_restart(void); > extern void imx_gpc_init(void); > extern void imx_gpc_pre_suspend(void); > extern void imx_gpc_post_resume(void); > +extern void imx_gpc_mask_all(void); > +extern void imx_gpc_restore_all(void); > extern void imx_anatop_init(void); > extern void imx_anatop_pre_suspend(void); > extern void imx_anatop_post_resume(void); > extern int imx6q_set_lpm(enum mxc_cpu_pwr_mode mode); > extern void imx6q_set_chicken_bit(void); > +extern void imx6q_set_rbc(bool enable); > extern void imx6q_set_wb(bool enable); Since they are taking "enable" parameter, can we rename them as below? void imx6q_enable_rbc(bool enable); void imx6q_enable_wb(bool enable); > extern void imx_cpu_die(unsigned int cpu); > extern int imx_cpu_kill(unsigned int cpu); > diff --git a/arch/arm/mach-imx/gpc.c b/arch/arm/mach-imx/gpc.c > index a96ccc7..504049e 100644 > --- a/arch/arm/mach-imx/gpc.c > +++ b/arch/arm/mach-imx/gpc.c > @@ -1,5 +1,5 @@ > /* > - * Copyright 2011 Freescale Semiconductor, Inc. > + * Copyright 2011-2013 Freescale Semiconductor, Inc. > * Copyright 2011 Linaro Ltd. > * > * The code contained herein is licensed under the GNU General Public > @@ -68,6 +68,25 @@ static int imx_gpc_irq_set_wake(struct irq_data *d, unsigned int on) > return 0; > } > > +void imx_gpc_mask_all(void) > +{ > + void __iomem *reg_imr1 = gpc_base + GPC_IMR1; > + int i; > + > + for (i = 0; i < IMR_NUM; i++) > + writel_relaxed(0xffffffff, reg_imr1 + i * 4); > + > +} > + > +void imx_gpc_restore_all(void) > +{ > + void __iomem *reg_imr1 = gpc_base + GPC_IMR1; > + int i; > + > + for (i = 0; i < IMR_NUM; i++) > + writel_relaxed(gpc_saved_imrs[i], reg_imr1 + i * 4); > +} > + > static void imx_gpc_irq_unmask(struct irq_data *d) > { > void __iomem *reg; > diff --git a/arch/arm/mach-imx/pm-imx6q.c b/arch/arm/mach-imx/pm-imx6q.c > index 57ca274..24ac7bc 100644 > --- a/arch/arm/mach-imx/pm-imx6q.c > +++ b/arch/arm/mach-imx/pm-imx6q.c > @@ -36,10 +36,12 @@ static int imx6q_pm_enter(suspend_state_t state) > imx_gpc_pre_suspend(); This call saves mask in gpc_saved_imrs and mask all interrupts except those wakeup sources ... > imx_anatop_pre_suspend(); > imx_set_cpu_jump(0, v7_cpu_resume); > + imx6q_set_rbc(true); At the end of imx6q_set_rbc(), imx_gpc_restore_all() will be called to restore mask bits with gpc_saved_imrs, so wakeup source handling gets lost, and any in-use interrupt will wake up system, which is undesirable. Shawn > imx6q_set_wb(true); > /* Zzz ... */ > cpu_suspend(0, imx6q_suspend_finish); > imx6q_set_wb(false); > + imx6q_set_rbc(false); > imx_smp_prepare(); > imx_anatop_post_resume(); > imx_gpc_post_resume(); > -- > 1.7.9.5 > >
On Wed, Mar 20, 2013 at 05:01:19PM +0800, Shawn Guo wrote: > On Wed, Mar 20, 2013 at 01:39:40PM -0400, Anson Huang wrote: > > RBC is to control whether some ANATOP sub modules > > can enter lpm mode when SOC is into STOP mode, if > > RBC is enabled and PMIC_VSTBY_REQ is set, ANATOP > > will have below behaviors: > > > > 1. Digital LDOs(CORE, SOC and PU) are bypassed; > > 2. Analog LDOs(1P1, 2P5, 3P0) are disabled; > > > > As the 2P5 is necessary for DRAM IO pre-drive in > > STOP mode, so we need to enable weak 2P5 in STOP > > mode when 2P5 LDO is disabled. > > > > For RBC settings, there are some rules as below > > due to hardware designe: > > s/designe/design Accepted. > > > > > 1. All interrupts must be masked during operating > > RBC registers; > > 2. At least 2 CKIL(32K) cycles is needed after the > > RBC setting is changed. > > > > Signed-off-by: Anson Huang <b20788@freescale.com> > > --- > > arch/arm/mach-imx/anatop.c | 19 +++++++++++++++++++ > > arch/arm/mach-imx/clk-imx6q.c | 35 +++++++++++++++++++++++++++++++++++ > > arch/arm/mach-imx/common.h | 3 +++ > > arch/arm/mach-imx/gpc.c | 21 ++++++++++++++++++++- > > arch/arm/mach-imx/pm-imx6q.c | 2 ++ > > 5 files changed, 79 insertions(+), 1 deletion(-) > > > > diff --git a/arch/arm/mach-imx/anatop.c b/arch/arm/mach-imx/anatop.c > > index 8f6ab27..38b4f44 100644 > > --- a/arch/arm/mach-imx/anatop.c > > +++ b/arch/arm/mach-imx/anatop.c > > @@ -18,12 +18,29 @@ > > > > #define REG_SET 0x4 > > #define REG_CLR 0x8 > > +#define ANA_MISC0 0x150 > > #define ANA_REG_CORE 0x140 > > +#define ANA_REG_2P5 0x130 > > Please put these registers in acceding offset. Accepted. > > > > > +#define BM_ANADIG_REG_2P5_ENABLE_WEAK_LINREG 0x40000 > > +#define BM_ANADIG_ANA_MISC0_STOP_MODE_CONFIG 0x1000 > > #define BM_ANADIG_REG_CORE_FET_ODRIVE 0x20000000 > > > > static struct regmap *anatop; > > > > +void imx_anatop_enable_weak2p5(bool enable) > > static Accepted. > > > +{ > > + u32 val; > > + > > + regmap_read(anatop, ANA_MISC0, &val); > > + > > + /* can only be enabled when stop_mode_config is clear. */ > > + regmap_write(anatop, ANA_REG_2P5 + ((enable && > > + ((val & BM_ANADIG_ANA_MISC0_STOP_MODE_CONFIG) > > + == 0)) ? REG_SET : REG_CLR), > > + BM_ANADIG_REG_2P5_ENABLE_WEAK_LINREG); > > It's a little difficult to parse the expression even it's on the same > line, not mentioning with indentation. There are so many levels of > parentheses. > > ((enable && ((val & BM_ANADIG_ANA_MISC0_STOP_MODE_CONFIG) == 0)) ? REG_SET : REG_CLR) > > What about rewriting it as below to make it easy for people to read? > > reg = ANA_REG_2P5; > reg += (enable && (val & BM_ANADIG_ANA_MISC0_STOP_MODE_CONFIG) == 0) ? > REG_SET : REG_CLR; > regmap_write(anatop, reg, BM_ANADIG_REG_2P5_ENABLE_WEAK_LINREG); Accepted. > > > +} > > + > > void imx_anatop_enable_fet_odrive(bool enable) > > { > > regmap_write(anatop, ANA_REG_CORE + (enable ? > > @@ -32,12 +49,14 @@ void imx_anatop_enable_fet_odrive(bool enable) > > > > void imx_anatop_pre_suspend(void) > > { > > + imx_anatop_enable_weak2p5(true); > > imx_anatop_enable_fet_odrive(true); > > } > > > > void imx_anatop_post_resume(void) > > { > > imx_anatop_enable_fet_odrive(false); > > + imx_anatop_enable_weak2p5(false); > > } > > > > void __init imx_anatop_init(void) > > diff --git a/arch/arm/mach-imx/clk-imx6q.c b/arch/arm/mach-imx/clk-imx6q.c > > index b365efc..646ce12 100644 > > --- a/arch/arm/mach-imx/clk-imx6q.c > > +++ b/arch/arm/mach-imx/clk-imx6q.c > > @@ -14,6 +14,7 @@ > > #include <linux/types.h> > > #include <linux/clk.h> > > #include <linux/clkdev.h> > > +#include <linux/delay.h> > > #include <linux/err.h> > > #include <linux/io.h> > > #include <linux/of.h> > > @@ -25,6 +26,8 @@ > > > > #define CCR 0x0 > > #define BM_CCR_WB_COUNT (0x7 << 16) > > +#define BM_CCR_RBC_BYPASS_COUNT (0x3f << 21) > > +#define BM_CCR_RBC_EN (0x1 << 27) > > > > #define CCGR0 0x68 > > #define CCGR1 0x6c > > @@ -70,6 +73,38 @@ void imx6q_set_chicken_bit(void) > > writel_relaxed(val, ccm_base + CGPR); > > } > > > > +void imx6q_set_rbc(bool enable) > > Same question as imx6q_set_wb, can we manage to call it in > imx6q_set_lpm()? The intension behind this is we should try to > a centralized place to configure CCM registers/bits for suspend > instead of exporting so many functions to suspend routine to call > individually. Will do it in the way as well bias did. > > > +{ > > + u32 val; > > + > > + /* > > + * need to mask all interrupts in GPC before > > + * operating RBC configurations > > + */ > > + imx_gpc_mask_all(); > > + > > + /* configurate RBC enable bit */ > > s/configurate/configure Accepted. > > > + val = readl_relaxed(ccm_base + CCR); > > + val &= ~BM_CCR_RBC_EN; > > + val |= enable ? BM_CCR_RBC_EN : 0; > > + writel_relaxed(val, ccm_base + CCR); > > + > > + /* configurate RBC count */ > > Ditto Accepted. > > > + val = readl_relaxed(ccm_base + CCR); > > + val &= ~BM_CCR_RBC_BYPASS_COUNT; > > + val |= enable ? BM_CCR_RBC_BYPASS_COUNT : 0; > > + writel(val, ccm_base + CCR); > > + > > + /* > > + * need to delay at least 2 cycles of CKIL(32K) > > + * due to hardware design requirement, which is > > + * ~61us, here we use 65us for safe > > + */ > > + udelay(65); > > Nit, have a new line here. Accepted. > > > + /* restore GPC interrupt mask settings */ > > + imx_gpc_restore_all(); > > +} > > + > > void imx6q_set_wb(bool enable) > > { > > u32 val; > > diff --git a/arch/arm/mach-imx/common.h b/arch/arm/mach-imx/common.h > > index b9125cf..66fe41c 100644 > > --- a/arch/arm/mach-imx/common.h > > +++ b/arch/arm/mach-imx/common.h > > @@ -129,11 +129,14 @@ extern void imx_src_prepare_restart(void); > > extern void imx_gpc_init(void); > > extern void imx_gpc_pre_suspend(void); > > extern void imx_gpc_post_resume(void); > > +extern void imx_gpc_mask_all(void); > > +extern void imx_gpc_restore_all(void); > > extern void imx_anatop_init(void); > > extern void imx_anatop_pre_suspend(void); > > extern void imx_anatop_post_resume(void); > > extern int imx6q_set_lpm(enum mxc_cpu_pwr_mode mode); > > extern void imx6q_set_chicken_bit(void); > > +extern void imx6q_set_rbc(bool enable); > > extern void imx6q_set_wb(bool enable); > > Since they are taking "enable" parameter, can we rename them as below? > > void imx6q_enable_rbc(bool enable); > void imx6q_enable_wb(bool enable); OK. > > > extern void imx_cpu_die(unsigned int cpu); > > extern int imx_cpu_kill(unsigned int cpu); > > diff --git a/arch/arm/mach-imx/gpc.c b/arch/arm/mach-imx/gpc.c > > index a96ccc7..504049e 100644 > > --- a/arch/arm/mach-imx/gpc.c > > +++ b/arch/arm/mach-imx/gpc.c > > @@ -1,5 +1,5 @@ > > /* > > - * Copyright 2011 Freescale Semiconductor, Inc. > > + * Copyright 2011-2013 Freescale Semiconductor, Inc. > > * Copyright 2011 Linaro Ltd. > > * > > * The code contained herein is licensed under the GNU General Public > > @@ -68,6 +68,25 @@ static int imx_gpc_irq_set_wake(struct irq_data *d, unsigned int on) > > return 0; > > } > > > > +void imx_gpc_mask_all(void) > > +{ > > + void __iomem *reg_imr1 = gpc_base + GPC_IMR1; > > + int i; > > + > > + for (i = 0; i < IMR_NUM; i++) > > + writel_relaxed(0xffffffff, reg_imr1 + i * 4); > > + > > +} > > + > > +void imx_gpc_restore_all(void) > > +{ > > + void __iomem *reg_imr1 = gpc_base + GPC_IMR1; > > + int i; > > + > > + for (i = 0; i < IMR_NUM; i++) > > + writel_relaxed(gpc_saved_imrs[i], reg_imr1 + i * 4); > > +} > > + > > static void imx_gpc_irq_unmask(struct irq_data *d) > > { > > void __iomem *reg; > > diff --git a/arch/arm/mach-imx/pm-imx6q.c b/arch/arm/mach-imx/pm-imx6q.c > > index 57ca274..24ac7bc 100644 > > --- a/arch/arm/mach-imx/pm-imx6q.c > > +++ b/arch/arm/mach-imx/pm-imx6q.c > > @@ -36,10 +36,12 @@ static int imx6q_pm_enter(suspend_state_t state) > > imx_gpc_pre_suspend(); > > This call saves mask in gpc_saved_imrs and mask all interrupts except > those wakeup sources ... > > > imx_anatop_pre_suspend(); > > imx_set_cpu_jump(0, v7_cpu_resume); > > + imx6q_set_rbc(true); > > At the end of imx6q_set_rbc(), imx_gpc_restore_all() will be called to > restore mask bits with gpc_saved_imrs, so wakeup source handling gets > lost, and any in-use interrupt will wake up system, which is > undesirable. Oh, my mistake, will fix it in V2. > > Shawn > > > imx6q_set_wb(true); > > /* Zzz ... */ > > cpu_suspend(0, imx6q_suspend_finish); > > imx6q_set_wb(false); > > + imx6q_set_rbc(false); > > imx_smp_prepare(); > > imx_anatop_post_resume(); > > imx_gpc_post_resume(); > > -- > > 1.7.9.5 > > > >
diff --git a/arch/arm/mach-imx/anatop.c b/arch/arm/mach-imx/anatop.c index 8f6ab27..38b4f44 100644 --- a/arch/arm/mach-imx/anatop.c +++ b/arch/arm/mach-imx/anatop.c @@ -18,12 +18,29 @@ #define REG_SET 0x4 #define REG_CLR 0x8 +#define ANA_MISC0 0x150 #define ANA_REG_CORE 0x140 +#define ANA_REG_2P5 0x130 +#define BM_ANADIG_REG_2P5_ENABLE_WEAK_LINREG 0x40000 +#define BM_ANADIG_ANA_MISC0_STOP_MODE_CONFIG 0x1000 #define BM_ANADIG_REG_CORE_FET_ODRIVE 0x20000000 static struct regmap *anatop; +void imx_anatop_enable_weak2p5(bool enable) +{ + u32 val; + + regmap_read(anatop, ANA_MISC0, &val); + + /* can only be enabled when stop_mode_config is clear. */ + regmap_write(anatop, ANA_REG_2P5 + ((enable && + ((val & BM_ANADIG_ANA_MISC0_STOP_MODE_CONFIG) + == 0)) ? REG_SET : REG_CLR), + BM_ANADIG_REG_2P5_ENABLE_WEAK_LINREG); +} + void imx_anatop_enable_fet_odrive(bool enable) { regmap_write(anatop, ANA_REG_CORE + (enable ? @@ -32,12 +49,14 @@ void imx_anatop_enable_fet_odrive(bool enable) void imx_anatop_pre_suspend(void) { + imx_anatop_enable_weak2p5(true); imx_anatop_enable_fet_odrive(true); } void imx_anatop_post_resume(void) { imx_anatop_enable_fet_odrive(false); + imx_anatop_enable_weak2p5(false); } void __init imx_anatop_init(void) diff --git a/arch/arm/mach-imx/clk-imx6q.c b/arch/arm/mach-imx/clk-imx6q.c index b365efc..646ce12 100644 --- a/arch/arm/mach-imx/clk-imx6q.c +++ b/arch/arm/mach-imx/clk-imx6q.c @@ -14,6 +14,7 @@ #include <linux/types.h> #include <linux/clk.h> #include <linux/clkdev.h> +#include <linux/delay.h> #include <linux/err.h> #include <linux/io.h> #include <linux/of.h> @@ -25,6 +26,8 @@ #define CCR 0x0 #define BM_CCR_WB_COUNT (0x7 << 16) +#define BM_CCR_RBC_BYPASS_COUNT (0x3f << 21) +#define BM_CCR_RBC_EN (0x1 << 27) #define CCGR0 0x68 #define CCGR1 0x6c @@ -70,6 +73,38 @@ void imx6q_set_chicken_bit(void) writel_relaxed(val, ccm_base + CGPR); } +void imx6q_set_rbc(bool enable) +{ + u32 val; + + /* + * need to mask all interrupts in GPC before + * operating RBC configurations + */ + imx_gpc_mask_all(); + + /* configurate RBC enable bit */ + val = readl_relaxed(ccm_base + CCR); + val &= ~BM_CCR_RBC_EN; + val |= enable ? BM_CCR_RBC_EN : 0; + writel_relaxed(val, ccm_base + CCR); + + /* configurate RBC count */ + val = readl_relaxed(ccm_base + CCR); + val &= ~BM_CCR_RBC_BYPASS_COUNT; + val |= enable ? BM_CCR_RBC_BYPASS_COUNT : 0; + writel(val, ccm_base + CCR); + + /* + * need to delay at least 2 cycles of CKIL(32K) + * due to hardware design requirement, which is + * ~61us, here we use 65us for safe + */ + udelay(65); + /* restore GPC interrupt mask settings */ + imx_gpc_restore_all(); +} + void imx6q_set_wb(bool enable) { u32 val; diff --git a/arch/arm/mach-imx/common.h b/arch/arm/mach-imx/common.h index b9125cf..66fe41c 100644 --- a/arch/arm/mach-imx/common.h +++ b/arch/arm/mach-imx/common.h @@ -129,11 +129,14 @@ extern void imx_src_prepare_restart(void); extern void imx_gpc_init(void); extern void imx_gpc_pre_suspend(void); extern void imx_gpc_post_resume(void); +extern void imx_gpc_mask_all(void); +extern void imx_gpc_restore_all(void); extern void imx_anatop_init(void); extern void imx_anatop_pre_suspend(void); extern void imx_anatop_post_resume(void); extern int imx6q_set_lpm(enum mxc_cpu_pwr_mode mode); extern void imx6q_set_chicken_bit(void); +extern void imx6q_set_rbc(bool enable); extern void imx6q_set_wb(bool enable); extern void imx_cpu_die(unsigned int cpu); extern int imx_cpu_kill(unsigned int cpu); diff --git a/arch/arm/mach-imx/gpc.c b/arch/arm/mach-imx/gpc.c index a96ccc7..504049e 100644 --- a/arch/arm/mach-imx/gpc.c +++ b/arch/arm/mach-imx/gpc.c @@ -1,5 +1,5 @@ /* - * Copyright 2011 Freescale Semiconductor, Inc. + * Copyright 2011-2013 Freescale Semiconductor, Inc. * Copyright 2011 Linaro Ltd. * * The code contained herein is licensed under the GNU General Public @@ -68,6 +68,25 @@ static int imx_gpc_irq_set_wake(struct irq_data *d, unsigned int on) return 0; } +void imx_gpc_mask_all(void) +{ + void __iomem *reg_imr1 = gpc_base + GPC_IMR1; + int i; + + for (i = 0; i < IMR_NUM; i++) + writel_relaxed(0xffffffff, reg_imr1 + i * 4); + +} + +void imx_gpc_restore_all(void) +{ + void __iomem *reg_imr1 = gpc_base + GPC_IMR1; + int i; + + for (i = 0; i < IMR_NUM; i++) + writel_relaxed(gpc_saved_imrs[i], reg_imr1 + i * 4); +} + static void imx_gpc_irq_unmask(struct irq_data *d) { void __iomem *reg; diff --git a/arch/arm/mach-imx/pm-imx6q.c b/arch/arm/mach-imx/pm-imx6q.c index 57ca274..24ac7bc 100644 --- a/arch/arm/mach-imx/pm-imx6q.c +++ b/arch/arm/mach-imx/pm-imx6q.c @@ -36,10 +36,12 @@ static int imx6q_pm_enter(suspend_state_t state) imx_gpc_pre_suspend(); imx_anatop_pre_suspend(); imx_set_cpu_jump(0, v7_cpu_resume); + imx6q_set_rbc(true); imx6q_set_wb(true); /* Zzz ... */ cpu_suspend(0, imx6q_suspend_finish); imx6q_set_wb(false); + imx6q_set_rbc(false); imx_smp_prepare(); imx_anatop_post_resume(); imx_gpc_post_resume();
RBC is to control whether some ANATOP sub modules can enter lpm mode when SOC is into STOP mode, if RBC is enabled and PMIC_VSTBY_REQ is set, ANATOP will have below behaviors: 1. Digital LDOs(CORE, SOC and PU) are bypassed; 2. Analog LDOs(1P1, 2P5, 3P0) are disabled; As the 2P5 is necessary for DRAM IO pre-drive in STOP mode, so we need to enable weak 2P5 in STOP mode when 2P5 LDO is disabled. For RBC settings, there are some rules as below due to hardware designe: 1. All interrupts must be masked during operating RBC registers; 2. At least 2 CKIL(32K) cycles is needed after the RBC setting is changed. Signed-off-by: Anson Huang <b20788@freescale.com> --- arch/arm/mach-imx/anatop.c | 19 +++++++++++++++++++ arch/arm/mach-imx/clk-imx6q.c | 35 +++++++++++++++++++++++++++++++++++ arch/arm/mach-imx/common.h | 3 +++ arch/arm/mach-imx/gpc.c | 21 ++++++++++++++++++++- arch/arm/mach-imx/pm-imx6q.c | 2 ++ 5 files changed, 79 insertions(+), 1 deletion(-)