Re: [v3,4/5] powerpc/pm: support deep sleep feature on T104x
From: Scott Wood <o...@buserror.net> Sent: Thursday, September 29, 2016 4:03 AM To: C.H. Zhao Cc: linuxppc-dev@lists.ozlabs.org; linux-ker...@vger.kernel.org; z.chen...@gmail.com; Jason Jin Subject: Re: [v3,4/5] powerpc/pm: support deep sleep feature on T104x On Tue, 2016-09-27 at 11:05 +0000, C.H. Zhao wrote: > From: Scott Wood <o...@buserror.net> > Sent: Sunday, September 25, 2016 3:24 PM > To: C.H. Zhao > Cc: linuxppc-dev@lists.ozlabs.org; linux-ker...@vger.kernel.org; z.chenhui@g > mail.com; Jason Jin > Subject: Re: [v3,4/5] powerpc/pm: support deep sleep feature on T104x > > On Tue, Aug 02, 2016 at 07:59:31PM +0800, Chenhui Zhao wrote: > > > > T104x has deep sleep feature, which can switch off most parts of > > the SoC when it is in deep sleep mode. This way, it becomes more > > energy-efficient. > > > > The DDR controller will also be powered off in deep sleep. Therefore, > > the last stage (the latter part of fsl_dp_enter_low) will run without DDR > > access. This piece of code and related TLBs are prefetched in advance. > > > > Due to the different initialization code between 32-bit and 64-bit, they > > have separate resume entry and precedure. > > > > The feature supports 32-bit and 64-bit kernel mode. > > > > Signed-off-by: Chenhui Zhao <chenhui.z...@nxp.com> > > --- > > arch/powerpc/include/asm/fsl_pm.h | 24 ++ > > arch/powerpc/kernel/asm-offsets.c | 12 + > > arch/powerpc/kernel/fsl_booke_entry_mapping.S | 10 + > > arch/powerpc/kernel/head_64.S | 2 +- > > arch/powerpc/platforms/85xx/Makefile | 1 + > > arch/powerpc/platforms/85xx/deepsleep.c | 278 ++ > > arch/powerpc/platforms/85xx/qoriq_pm.c | 25 ++ > > arch/powerpc/platforms/85xx/t104x_deepsleep.S | 531 > > ++ > > arch/powerpc/sysdev/fsl_rcpm.c | 8 +- > > 9 files changed, 889 insertions(+), 2 deletions(-) > > create mode 100644 arch/powerpc/platforms/85xx/deepsleep.c > > create mode 100644 arch/powerpc/platforms/85xx/t104x_deepsleep.S > > > > diff --git a/arch/powerpc/include/asm/fsl_pm.h > > b/arch/powerpc/include/asm/fsl_pm.h > > index e05049b..48c2631 100644 > > --- a/arch/powerpc/include/asm/fsl_pm.h > > +++ b/arch/powerpc/include/asm/fsl_pm.h > > @@ -20,6 +20,7 @@ > > > > #define PLAT_PM_SLEEP 20 > > #define PLAT_PM_LPM20 30 > > +#define PLAT_PM_LPM35 40 > > > > #define FSL_PM_SLEEP (1 << 0) > > #define FSL_PM_DEEP_SLEEP (1 << 1) > > @@ -48,4 +49,27 @@ extern const struct fsl_pm_ops *qoriq_pm_ops; > > > > int __init fsl_rcpm_init(void); > > > > +#ifdef CONFIG_FSL_QORIQ_PM > > +int fsl_enter_deepsleep(void); > > +int fsl_deepsleep_init(void); > > +#else > > +static inline int fsl_enter_deepsleep(void) { return -1; } > > +static inline int fsl_deepsleep_init(void) { return -1; } > > +#endif > Please return proper error codes. > > Where can fsl_deepsleep_init() be called without CONFIG_FSL_QORIQ_PM? > > [Chenhui] I can get rid of the ifdef here. And add it > in arch/powerpc/sysdev/fsl_rcpm.c. No, this is the right place for the ifdef for functions that are called from code that doesn't depend on CONFIG_FSL_QORIQ_PM. But fsl_deepsleep_init() is called from deepsleep.c which is only built with CONFIG_FSL_QORIQ_PM, and it's hard to picture a scenario where it would be called from elsewhere. [Chenhui] You are right. No need to enclose fsl_deepsleep_init() in the ifdef. But regarding fsl_enter_deepsleep(), it is called in rcpm_v2_plat_enter_sleep() in arch/powerpc/sysdev/fsl_rcpm.c. It still needs to be enclosed in the ifdef. I would change it like: int fsl_deepsleep_init(void); #ifdef CONFIG_FSL_QORIQ_PM int fsl_enter_deepsleep(void); #else static inline int fsl_enter_deepsleep(void) { return -EINVAL; } #endif > > diff --git a/arch/powerpc/kernel/fsl_booke_entry_mapping.S > > b/arch/powerpc/kernel/fsl_booke_entry_mapping.S > > index 83dd0f6..659b059 100644 > > --- a/arch/powerpc/kernel/fsl_booke_entry_mapping.S > > +++ b/arch/powerpc/kernel/fsl_booke_entry_mapping.S > > @@ -173,6 +173,10 @@ skpinv: addi r6,r6,1 /* > > Increment */ > > lis r6,MAS2_VAL(PAGE_OFFSET, BOOK3E_PAGESZ_64M, M_IF_NEEDED)@h > > ori r6,r6,MAS2_VAL(PAGE_OFFSET, BOOK3E_PAGESZ_64M, > >
Re: [v3,4/5] powerpc/pm: support deep sleep feature on T104x
From: Scott Wood <o...@buserror.net> Sent: Sunday, September 25, 2016 3:24 PM To: C.H. Zhao Cc: linuxppc-dev@lists.ozlabs.org; linux-ker...@vger.kernel.org; z.chen...@gmail.com; Jason Jin Subject: Re: [v3,4/5] powerpc/pm: support deep sleep feature on T104x On Tue, Aug 02, 2016 at 07:59:31PM +0800, Chenhui Zhao wrote: > T104x has deep sleep feature, which can switch off most parts of > the SoC when it is in deep sleep mode. This way, it becomes more > energy-efficient. > > The DDR controller will also be powered off in deep sleep. Therefore, > the last stage (the latter part of fsl_dp_enter_low) will run without DDR > access. This piece of code and related TLBs are prefetched in advance. > > Due to the different initialization code between 32-bit and 64-bit, they > have separate resume entry and precedure. > > The feature supports 32-bit and 64-bit kernel mode. > > Signed-off-by: Chenhui Zhao <chenhui.z...@nxp.com> > --- > arch/powerpc/include/asm/fsl_pm.h | 24 ++ > arch/powerpc/kernel/asm-offsets.c | 12 + > arch/powerpc/kernel/fsl_booke_entry_mapping.S | 10 + > arch/powerpc/kernel/head_64.S | 2 +- > arch/powerpc/platforms/85xx/Makefile | 1 + > arch/powerpc/platforms/85xx/deepsleep.c | 278 ++ > arch/powerpc/platforms/85xx/qoriq_pm.c | 25 ++ > arch/powerpc/platforms/85xx/t104x_deepsleep.S | 531 >++ > arch/powerpc/sysdev/fsl_rcpm.c | 8 +- > 9 files changed, 889 insertions(+), 2 deletions(-) > create mode 100644 arch/powerpc/platforms/85xx/deepsleep.c > create mode 100644 arch/powerpc/platforms/85xx/t104x_deepsleep.S > > diff --git a/arch/powerpc/include/asm/fsl_pm.h > b/arch/powerpc/include/asm/fsl_pm.h > index e05049b..48c2631 100644 > --- a/arch/powerpc/include/asm/fsl_pm.h > +++ b/arch/powerpc/include/asm/fsl_pm.h > @@ -20,6 +20,7 @@ > > #define PLAT_PM_SLEEP 20 > #define PLAT_PM_LPM20 30 > +#define PLAT_PM_LPM35 40 > > #define FSL_PM_SLEEP (1 << 0) > #define FSL_PM_DEEP_SLEEP (1 << 1) > @@ -48,4 +49,27 @@ extern const struct fsl_pm_ops *qoriq_pm_ops; > > int __init fsl_rcpm_init(void); > > +#ifdef CONFIG_FSL_QORIQ_PM > +int fsl_enter_deepsleep(void); > +int fsl_deepsleep_init(void); > +#else > +static inline int fsl_enter_deepsleep(void) { return -1; } > +static inline int fsl_deepsleep_init(void) { return -1; } > +#endif Please return proper error codes. Where can fsl_deepsleep_init() be called without CONFIG_FSL_QORIQ_PM? [Chenhui] I can get rid of the ifdef here. And add it in arch/powerpc/sysdev/fsl_rcpm.c. > + > +extern void fsl_dp_enter_low(void *priv); > +extern void fsl_booke_deep_sleep_resume(void); > + > +struct fsl_iomap { > + void *ccsr_scfg_base; > + void *ccsr_rcpm_base; > + void *ccsr_ddr_base; > + void *ccsr_gpio1_base; > + void *ccsr_cpc_base; > + void *dcsr_epu_base; > + void *dcsr_npc_base; > + void *dcsr_rcpm_base; > + void *cpld_base; > + void *fpga_base; > +}; __iomem [Chenhui] Yes. Will add it. > #endif /* __PPC_FSL_PM_H */ > diff --git a/arch/powerpc/kernel/asm-offsets.c > b/arch/powerpc/kernel/asm-offsets.c > index 9ea0955..cc488f9 100644 > --- a/arch/powerpc/kernel/asm-offsets.c > +++ b/arch/powerpc/kernel/asm-offsets.c > @@ -68,6 +68,10 @@ > #include "../mm/mmu_decl.h" > #endif > > +#ifdef CONFIG_FSL_QORIQ_PM > +#include > +#endif I know this file ifdefs headers a lot, but it's generally not good practice. Does including this file cause any harm on other platforms? [Chenhui] Not at all. Will remove it. > diff --git a/arch/powerpc/kernel/fsl_booke_entry_mapping.S > b/arch/powerpc/kernel/fsl_booke_entry_mapping.S > index 83dd0f6..659b059 100644 > --- a/arch/powerpc/kernel/fsl_booke_entry_mapping.S > +++ b/arch/powerpc/kernel/fsl_booke_entry_mapping.S > @@ -173,6 +173,10 @@ skpinv: addi r6,r6,1 /* > Increment */ > lis r6,MAS2_VAL(PAGE_OFFSET, BOOK3E_PAGESZ_64M, M_IF_NEEDED)@h > ori r6,r6,MAS2_VAL(PAGE_OFFSET, BOOK3E_PAGESZ_64M, M_IF_NEEDED)@l > mtspr SPRN_MAS2,r6 > +#ifdef ENTRY_DEEPSLEEP_SETUP > + LOAD_REG_IMMEDIATE(r8, MEMORY_START) > + ori r8,r8,(MAS3_SX|MAS3_SW|MAS3_SR) > +#endif > mtspr SPRN_MAS3,r8 > tlbwe > Have you tried this with a relocatable kernel? [Chenhui] Not yet. Not sure whether it has been supported on QorIQ platform. > +static void fsl_dp_set_resume_pointer(void) > +{ > + u32 resume_addr; > + > + /* the bootloader will finally jump to this address to return ke