From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753958AbcI2KVf (ORCPT ); Thu, 29 Sep 2016 06:21:35 -0400 Received: from mail-db5eur01on0059.outbound.protection.outlook.com ([104.47.2.59]:41252 "EHLO EUR01-DB5-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751257AbcI2KV1 (ORCPT ); Thu, 29 Sep 2016 06:21:27 -0400 From: "C.H. Zhao" To: Scott Wood CC: "linuxppc-dev@lists.ozlabs.org" , "linux-kernel@vger.kernel.org" , "z.chenhui@gmail.com" , Jason Jin Subject: Re: [v3,4/5] powerpc/pm: support deep sleep feature on T104x Thread-Topic: [v3,4/5] powerpc/pm: support deep sleep feature on T104x Thread-Index: AQHSFv3RkjcR0Fx3JkS0yU/lS8MPJKCNI4LDgAI1MoCAAOqC2w== Date: Thu, 29 Sep 2016 10:21:23 +0000 Message-ID: References: <1470139172-12699-5-git-send-email-chenhui.zhao@nxp.com> ,<20160925072400.GA17928@home.buserror.net> ,<1475092993.4283.51.camel@buserror.net> In-Reply-To: <1475092993.4283.51.camel@buserror.net> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=chenhui.zhao@nxp.com; x-originating-ip: [25.167.253.132] x-ms-office365-filtering-correlation-id: 8e1c8e66-cc94-4af2-b01b-08d3e8525d64 x-microsoft-exchange-diagnostics: 1;AM3PR04MB258;7:nZAoEvyf6aJGU9ITj2Fm+Fn1wupfwPWlZPXp+8GERKqfjzAp4NxFkfFTqEGn5QiOZHsdQQPKPajFCGGh5iYPjbMj+YprjVTwIOCyhwjTwGi5Rcaa0dLgwR+XXW2F/McovrKCaYp8eSK/Sz4Pw3D7U9nc1N/khDE09QGnZsbmcE2IVjqpsO0eFP/pS9UcqeXLt8pSD5QJnRXpjj69I/bDgNkJXN2a9r+duIOmSn2B+d7tFa+lO7gxIoTLT6QN/0DL5kvcJJmNvrUvM7uR4UrcWceIcJVjH39bKzVkcoHgoY5ouBZCtkMu4QasoDzy+M9owCZ6vIDuJ/mpMgAwVM53TQ== x-microsoft-antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:AM3PR04MB258; x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(9452136761055)(65623756079841)(185117386973197)(158362468548515); x-exchange-antispam-report-cfa-test: BCL:0;PCL:0;RULEID:(6040176)(601004)(2401047)(8121501046)(5005006)(3002001)(10201501046)(6055026);SRVR:AM3PR04MB258;BCL:0;PCL:0;RULEID:;SRVR:AM3PR04MB258; x-forefront-prvs: 00808B16F3 x-forefront-antispam-report: SFV:NSPM;SFS:(10009020)(6009001)(7916002)(189002)(24454002)(377454003)(377424004)(199003)(3846002)(6116002)(102836003)(97736004)(305945005)(189998001)(586003)(54356999)(33656002)(50986999)(76176999)(101416001)(5660300001)(66066001)(4326007)(106116001)(76576001)(5002640100001)(11100500001)(93886004)(7736002)(3900700001)(105586002)(3660700001)(8676002)(19580405001)(19580395003)(81156014)(81166006)(2906002)(7846002)(106356001)(87936001)(68736007)(77096005)(86362001)(92566002)(575784001)(9686002)(7696004)(10400500002)(110136003)(2950100002)(3280700002)(122556002)(8936002)(6916009)(2900100001)(74316002);DIR:OUT;SFP:1101;SCL:1;SRVR:AM3PR04MB258;H:AM4PR0401MB2355.eurprd04.prod.outlook.com;FPR:;SPF:None;PTR:InfoNoRecords;MX:1;A:1;LANG:en; spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="iso-8859-1" MIME-Version: 1.0 X-OriginatorOrg: nxp.com X-MS-Exchange-CrossTenant-originalarrivaltime: 29 Sep 2016 10:21:23.8821 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635 X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM3PR04MB258 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by mail.home.local id u8TALfkZ020359 From: Scott Wood Sent: Thursday, September 29, 2016 4:03 AM To: C.H. Zhao Cc: linuxppc-dev@lists.ozlabs.org; linux-kernel@vger.kernel.org; z.chenhui@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 > Sent: Sunday, September 25, 2016 3:24 PM > To: C.H. Zhao > Cc: linuxppc-dev@lists.ozlabs.org; linux-kernel@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 > > --- > >   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, > > 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. It is supported, and deep sleep needs to work with it. [Chenhui] OK. I'm going to work something out. > > +static void fsl_dp_set_resume_pointer(void) > > +{ > > +     u32 resume_addr; > > + > > +     /* the bootloader will finally jump to this address to return kernel > > */ > > +#ifdef CONFIG_PPC32 > > +     resume_addr = (u32)(__pa(fsl_booke_deep_sleep_resume)); > > +#else > > +     resume_addr = (u32)(__pa(*(u64 *)fsl_booke_deep_sleep_resume) > > +                         & 0xffffffff); > > +#endif > Why are you masking the physical address by 0xffffffff?  Besides the > (u32) cast accomplishing the same thing, wouldn't it be a problem if > (e.g. due to a relocatable kernel) the address is above 4 GiB? > > [Chenhui] Here, I assumed kernel is below 4 GiB. Maybe I should add a > comment here. It needs a fix rather than a comment, unless you can show that the relocatable mechanism doesn't support kernels over 4 GiB (I don't remember of the top of my head whether it does). -Scott [Chenhui] OK. I'm going to work something out.