* [PATCH] ARM: imx6ull: Make suspend/resume work like on 6ul @ 2017-05-30 16:11 Leonard Crestez 2017-06-05 5:37 ` Shawn Guo 0 siblings, 1 reply; 6+ messages in thread From: Leonard Crestez @ 2017-05-30 16:11 UTC (permalink / raw) To: linux-arm-kernel Suspend and resume on imx6ull is currenty not working because of some missed checks where behavior should match imx6ul. Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com> --- arch/arm/mach-imx/mxc.h | 6 ++++++ arch/arm/mach-imx/pm-imx6.c | 6 ++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/arch/arm/mach-imx/mxc.h b/arch/arm/mach-imx/mxc.h index 34f2ff6..e00d626 100644 --- a/arch/arm/mach-imx/mxc.h +++ b/arch/arm/mach-imx/mxc.h @@ -39,6 +39,7 @@ #define MXC_CPU_IMX6SX 0x62 #define MXC_CPU_IMX6Q 0x63 #define MXC_CPU_IMX6UL 0x64 +#define MXC_CPU_IMX6ULL 0x65 #define MXC_CPU_IMX7D 0x72 #define IMX_DDR_TYPE_LPDDR2 1 @@ -73,6 +74,11 @@ static inline bool cpu_is_imx6ul(void) return __mxc_cpu_type == MXC_CPU_IMX6UL; } +static inline bool cpu_is_imx6ull(void) +{ + return __mxc_cpu_type == MXC_CPU_IMX6ULL; +} + static inline bool cpu_is_imx6q(void) { return __mxc_cpu_type == MXC_CPU_IMX6Q; diff --git a/arch/arm/mach-imx/pm-imx6.c b/arch/arm/mach-imx/pm-imx6.c index e61b1d1..ecdf071 100644 --- a/arch/arm/mach-imx/pm-imx6.c +++ b/arch/arm/mach-imx/pm-imx6.c @@ -295,7 +295,8 @@ int imx6_set_lpm(enum mxc_cpu_pwr_mode mode) val &= ~BM_CLPCR_SBYOS; if (cpu_is_imx6sl()) val |= BM_CLPCR_BYPASS_PMIC_READY; - if (cpu_is_imx6sl() || cpu_is_imx6sx() || cpu_is_imx6ul()) + if (cpu_is_imx6sl() || cpu_is_imx6sx() || cpu_is_imx6ul() || + cpu_is_imx6ull()) val |= BM_CLPCR_BYP_MMDC_CH0_LPM_HS; else val |= BM_CLPCR_BYP_MMDC_CH1_LPM_HS; @@ -312,7 +313,8 @@ int imx6_set_lpm(enum mxc_cpu_pwr_mode mode) val |= BM_CLPCR_SBYOS; if (cpu_is_imx6sl() || cpu_is_imx6sx()) val |= BM_CLPCR_BYPASS_PMIC_READY; - if (cpu_is_imx6sl() || cpu_is_imx6sx() || cpu_is_imx6ul()) + if (cpu_is_imx6sl() || cpu_is_imx6sx() || cpu_is_imx6ul() || + cpu_is_imx6ull()) val |= BM_CLPCR_BYP_MMDC_CH0_LPM_HS; else val |= BM_CLPCR_BYP_MMDC_CH1_LPM_HS; -- 2.7.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH] ARM: imx6ull: Make suspend/resume work like on 6ul 2017-05-30 16:11 [PATCH] ARM: imx6ull: Make suspend/resume work like on 6ul Leonard Crestez @ 2017-06-05 5:37 ` Shawn Guo 2017-06-06 10:51 ` Leonard Crestez 0 siblings, 1 reply; 6+ messages in thread From: Shawn Guo @ 2017-06-05 5:37 UTC (permalink / raw) To: linux-arm-kernel On Tue, May 30, 2017 at 07:11:19PM +0300, Leonard Crestez wrote: > Suspend and resume on imx6ull is currenty not working because of some > missed checks where behavior should match imx6ul. > > Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com> > --- > arch/arm/mach-imx/mxc.h | 6 ++++++ > arch/arm/mach-imx/pm-imx6.c | 6 ++++-- > 2 files changed, 10 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/mach-imx/mxc.h b/arch/arm/mach-imx/mxc.h > index 34f2ff6..e00d626 100644 > --- a/arch/arm/mach-imx/mxc.h > +++ b/arch/arm/mach-imx/mxc.h > @@ -39,6 +39,7 @@ > #define MXC_CPU_IMX6SX 0x62 > #define MXC_CPU_IMX6Q 0x63 > #define MXC_CPU_IMX6UL 0x64 > +#define MXC_CPU_IMX6ULL 0x65 Since you are adding a new CPU type, you should probably patch imx_soc_device_init() for it as well. Shawn > #define MXC_CPU_IMX7D 0x72 > > #define IMX_DDR_TYPE_LPDDR2 1 > @@ -73,6 +74,11 @@ static inline bool cpu_is_imx6ul(void) > return __mxc_cpu_type == MXC_CPU_IMX6UL; > } > > +static inline bool cpu_is_imx6ull(void) > +{ > + return __mxc_cpu_type == MXC_CPU_IMX6ULL; > +} > + > static inline bool cpu_is_imx6q(void) > { > return __mxc_cpu_type == MXC_CPU_IMX6Q; > diff --git a/arch/arm/mach-imx/pm-imx6.c b/arch/arm/mach-imx/pm-imx6.c > index e61b1d1..ecdf071 100644 > --- a/arch/arm/mach-imx/pm-imx6.c > +++ b/arch/arm/mach-imx/pm-imx6.c > @@ -295,7 +295,8 @@ int imx6_set_lpm(enum mxc_cpu_pwr_mode mode) > val &= ~BM_CLPCR_SBYOS; > if (cpu_is_imx6sl()) > val |= BM_CLPCR_BYPASS_PMIC_READY; > - if (cpu_is_imx6sl() || cpu_is_imx6sx() || cpu_is_imx6ul()) > + if (cpu_is_imx6sl() || cpu_is_imx6sx() || cpu_is_imx6ul() || > + cpu_is_imx6ull()) > val |= BM_CLPCR_BYP_MMDC_CH0_LPM_HS; > else > val |= BM_CLPCR_BYP_MMDC_CH1_LPM_HS; > @@ -312,7 +313,8 @@ int imx6_set_lpm(enum mxc_cpu_pwr_mode mode) > val |= BM_CLPCR_SBYOS; > if (cpu_is_imx6sl() || cpu_is_imx6sx()) > val |= BM_CLPCR_BYPASS_PMIC_READY; > - if (cpu_is_imx6sl() || cpu_is_imx6sx() || cpu_is_imx6ul()) > + if (cpu_is_imx6sl() || cpu_is_imx6sx() || cpu_is_imx6ul() || > + cpu_is_imx6ull()) > val |= BM_CLPCR_BYP_MMDC_CH0_LPM_HS; > else > val |= BM_CLPCR_BYP_MMDC_CH1_LPM_HS; > -- > 2.7.4 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel at lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] ARM: imx6ull: Make suspend/resume work like on 6ul 2017-06-05 5:37 ` Shawn Guo @ 2017-06-06 10:51 ` Leonard Crestez 2017-06-07 3:21 ` Shawn Guo 0 siblings, 1 reply; 6+ messages in thread From: Leonard Crestez @ 2017-06-06 10:51 UTC (permalink / raw) To: linux-arm-kernel On Mon, 2017-06-05 at 13:37 +0800, Shawn Guo wrote: > On Tue, May 30, 2017 at 07:11:19PM +0300, Leonard Crestez wrote: > > > > Suspend and resume on imx6ull is currenty not working because of some > > missed checks where behavior should match imx6ul. > > > > Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com> > > --- > > ?arch/arm/mach-imx/mxc.h?????| 6 ++++++ > > ?arch/arm/mach-imx/pm-imx6.c | 6 ++++-- > > ?2 files changed, 10 insertions(+), 2 deletions(-) > > > > diff --git a/arch/arm/mach-imx/mxc.h b/arch/arm/mach-imx/mxc.h > > index 34f2ff6..e00d626 100644 > > --- a/arch/arm/mach-imx/mxc.h > > +++ b/arch/arm/mach-imx/mxc.h > > @@ -39,6 +39,7 @@ > > ?#define MXC_CPU_IMX6SX 0x62 > > ?#define MXC_CPU_IMX6Q 0x63 > > ?#define MXC_CPU_IMX6UL 0x64 > > +#define MXC_CPU_IMX6ULL 0x65 > Since you are adding a new CPU type, you should probably patch > imx_soc_device_init() for it as well. Ok, I will resend as a 2-patch series. BTW, it actually seems to me that setting BM_CLPCR_BYP_MMDC_CH0_LPM_HS on imx6sl/sx/ul/ull is not actually needed. That bit (19) is documented as "reserved" in the Reference Manual and likely ignored by hardware. As far as I understand the MMDC on imx6qdl has two channels and unless 2-channel mode is enabled (not currently supported) the handshake with CH1 needs to be disabled. Other reduced chips only have one MMDC channel and that is CH1 (CH0 was removed) and nothing needs to be done from them. The only important thing is to avoid setting BM_CLPCR_BYP_MMDC_CH1_LPM_HS. However perhaps what I am saying is wrong for some early chip versions? Because this behavior of setting BM_CLPCR_BYP_MMDC_CH0_LPM_HS has been in the kernel for a long time. -- Regards, Leonard ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] ARM: imx6ull: Make suspend/resume work like on 6ul 2017-06-06 10:51 ` Leonard Crestez @ 2017-06-07 3:21 ` Shawn Guo 2017-06-07 3:38 ` Anson Huang 0 siblings, 1 reply; 6+ messages in thread From: Shawn Guo @ 2017-06-07 3:21 UTC (permalink / raw) To: linux-arm-kernel On Tue, Jun 06, 2017 at 01:51:53PM +0300, Leonard Crestez wrote: > On Mon, 2017-06-05 at 13:37 +0800, Shawn Guo wrote: > > On Tue, May 30, 2017 at 07:11:19PM +0300, Leonard Crestez wrote: > > > > > > Suspend and resume on imx6ull is currenty not working because of some > > > missed checks where behavior should match imx6ul. > > > > > > Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com> > > > --- > > > ?arch/arm/mach-imx/mxc.h?????| 6 ++++++ > > > ?arch/arm/mach-imx/pm-imx6.c | 6 ++++-- > > > ?2 files changed, 10 insertions(+), 2 deletions(-) > > > > > > diff --git a/arch/arm/mach-imx/mxc.h b/arch/arm/mach-imx/mxc.h > > > index 34f2ff6..e00d626 100644 > > > --- a/arch/arm/mach-imx/mxc.h > > > +++ b/arch/arm/mach-imx/mxc.h > > > @@ -39,6 +39,7 @@ > > > ?#define MXC_CPU_IMX6SX 0x62 > > > ?#define MXC_CPU_IMX6Q 0x63 > > > ?#define MXC_CPU_IMX6UL 0x64 > > > +#define MXC_CPU_IMX6ULL 0x65 > > Since you are adding a new CPU type, you should probably patch > > imx_soc_device_init() for it as well. > > Ok, I will resend as a 2-patch series. > > BTW, it actually seems to me that setting BM_CLPCR_BYP_MMDC_CH0_LPM_HS > on imx6sl/sx/ul/ull is not actually needed. That bit (19) is documented > as "reserved" in the Reference Manual and likely ignored by hardware. > > As far as I understand the MMDC on imx6qdl has two channels and unless > 2-channel mode is enabled (not currently supported) the handshake with > CH1 needs to be disabled. Other reduced chips only have one MMDC > channel and that is CH1 (CH0 was removed) and nothing needs to be done > from them. The only important thing is to avoid setting > BM_CLPCR_BYP_MMDC_CH1_LPM_HS. > > However perhaps what I am saying is wrong for some early chip versions? > Because this behavior of setting BM_CLPCR_BYP_MMDC_CH0_LPM_HS has been > in the kernel for a long time. @Anson, you might be the right person to comment here? Shawn ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] ARM: imx6ull: Make suspend/resume work like on 6ul 2017-06-07 3:21 ` Shawn Guo @ 2017-06-07 3:38 ` Anson Huang 2017-06-07 9:44 ` Leonard Crestez 0 siblings, 1 reply; 6+ messages in thread From: Anson Huang @ 2017-06-07 3:38 UTC (permalink / raw) To: linux-arm-kernel Best Regards! Anson Huang > -----Original Message----- > From: Shawn Guo [mailto:shawnguo at kernel.org] > Sent: 2017-06-07 11:21 AM > To: Anson Huang <anson.huang@nxp.com> > Cc: Leonard Crestez <leonard.crestez@nxp.com>; Peter Chen > <peter.chen@nxp.com>; linux-kernel at vger.kernel.org; Fabio Estevam > <fabio.estevam@nxp.com>; linux-arm-kernel at lists.infradead.org; Lucas Stach > <l.stach@pengutronix.de> > Subject: Re: [PATCH] ARM: imx6ull: Make suspend/resume work like on 6ul > > On Tue, Jun 06, 2017 at 01:51:53PM +0300, Leonard Crestez wrote: > > On Mon, 2017-06-05 at 13:37 +0800, Shawn Guo wrote: > > > On Tue, May 30, 2017 at 07:11:19PM +0300, Leonard Crestez wrote: > > > > > > > > Suspend and resume on imx6ull is currenty not working because of > > > > some missed checks where behavior should match imx6ul. > > > > > > > > Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com> > > > > --- > > > > ?arch/arm/mach-imx/mxc.h?????| 6 ++++++ > > > > ?arch/arm/mach-imx/pm-imx6.c | 6 ++++-- > > > > ?2 files changed, 10 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/arch/arm/mach-imx/mxc.h b/arch/arm/mach-imx/mxc.h > > > > index 34f2ff6..e00d626 100644 > > > > --- a/arch/arm/mach-imx/mxc.h > > > > +++ b/arch/arm/mach-imx/mxc.h > > > > @@ -39,6 +39,7 @@ > > > > ?#define MXC_CPU_IMX6SX 0x62 > > > > ?#define MXC_CPU_IMX6Q 0x63 > > > > ?#define MXC_CPU_IMX6UL 0x64 > > > > +#define MXC_CPU_IMX6ULL 0x65 > > > Since you are adding a new CPU type, you should probably patch > > > imx_soc_device_init() for it as well. > > > > Ok, I will resend as a 2-patch series. > > > > BTW, it actually seems to me that setting > BM_CLPCR_BYP_MMDC_CH0_LPM_HS > > on imx6sl/sx/ul/ull is not actually needed. That bit (19) is > > documented as "reserved" in the Reference Manual and likely ignored by > hardware. > > > > As far as I understand the MMDC on imx6qdl has two channels and unless > > 2-channel mode is enabled (not currently supported) the handshake with > > CH1 needs to be disabled. Other reduced chips only have one MMDC > > channel and that is CH1 (CH0 was removed) and nothing needs to be done > > from them. The only important thing is to avoid setting > > BM_CLPCR_BYP_MMDC_CH1_LPM_HS. > > > > However perhaps what I am saying is wrong for some early chip versions? > > Because this behavior of setting BM_CLPCR_BYP_MMDC_CH0_LPM_HS has > been > > in the kernel for a long time. As far as I know, since i.MX6SX, we only use MMDC_CH1, but MMDC_CH0 is still there and we need to bypass its handshake when entering low power mode. The bit 19 in DOC is incorrect, I remembered I ever tried it and discuss with design time, they request DOC team to update DOC, but I think doc team forgot to do it. You can try removing this bit 19 setting and see if standby/mem suspend can still work? And try to modify this bit 19 value to see if it can be modified. Anson. > > @Anson, you might be the right person to comment here? > > Shawn ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] ARM: imx6ull: Make suspend/resume work like on 6ul 2017-06-07 3:38 ` Anson Huang @ 2017-06-07 9:44 ` Leonard Crestez 0 siblings, 0 replies; 6+ messages in thread From: Leonard Crestez @ 2017-06-07 9:44 UTC (permalink / raw) To: linux-arm-kernel On Wed, 2017-06-07 at 03:38 +0000, Anson Huang wrote: > > -----Original Message----- > > From: Shawn Guo [mailto:shawnguo at kernel.org] > > Sent: 2017-06-07 11:21 AM > > To: Anson Huang <anson.huang@nxp.com> > > Cc: Leonard Crestez <leonard.crestez@nxp.com>; Peter Chen > > <peter.chen@nxp.com>; linux-kernel at vger.kernel.org; Fabio Estevam > > <fabio.estevam@nxp.com>; linux-arm-kernel at lists.infradead.org; Lucas Stach > > <l.stach@pengutronix.de> > > Subject: Re: [PATCH] ARM: imx6ull: Make suspend/resume work like on 6ul > > > > On Tue, Jun 06, 2017 at 01:51:53PM +0300, Leonard Crestez wrote: > > > > > > On Mon, 2017-06-05 at 13:37 +0800, Shawn Guo wrote: > > > > > > > > On Tue, May 30, 2017 at 07:11:19PM +0300, Leonard Crestez wrote: > > > > > > > > > > > > > > > Suspend and resume on imx6ull is currenty not working because of > > > > > some missed checks where behavior should match imx6ul. > > > > > > > > > > Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com> > > > > > --- > > > > > ?arch/arm/mach-imx/mxc.h?????| 6 ++++++ > > > > > ?arch/arm/mach-imx/pm-imx6.c | 6 ++++-- > > > > > ?2 files changed, 10 insertions(+), 2 deletions(-) > > > > > > > > > > diff --git a/arch/arm/mach-imx/mxc.h b/arch/arm/mach-imx/mxc.h > > > > > index 34f2ff6..e00d626 100644 > > > > > --- a/arch/arm/mach-imx/mxc.h > > > > > +++ b/arch/arm/mach-imx/mxc.h > > > > > @@ -39,6 +39,7 @@ > > > > > ?#define MXC_CPU_IMX6SX 0x62 > > > > > ?#define MXC_CPU_IMX6Q 0x63 > > > > > ?#define MXC_CPU_IMX6UL 0x64 > > > > > +#define MXC_CPU_IMX6ULL 0x65 > > > > Since you are adding a new CPU type, you should probably patch > > > > imx_soc_device_init() for it as well. > > > Ok, I will resend as a 2-patch series. > > > > > > BTW, it actually seems to me that setting > > BM_CLPCR_BYP_MMDC_CH0_LPM_HS > > > > > > on imx6sl/sx/ul/ull is not actually needed. That bit (19) is > > > documented as "reserved" in the Reference Manual and likely ignored by > > hardware. > > > > > > > > > As far as I understand the MMDC on imx6qdl has two channels and unless > > > 2-channel mode is enabled (not currently supported) the handshake with > > > CH1 needs to be disabled. Other reduced chips only have one MMDC > > > channel and that is CH1 (CH0 was removed) and nothing needs to be done > > > from them. The only important thing is to avoid setting > > > BM_CLPCR_BYP_MMDC_CH1_LPM_HS. > > > > > > However perhaps what I am saying is wrong for some early chip versions? > > > Because this behavior of setting BM_CLPCR_BYP_MMDC_CH0_LPM_HS has > > been > > > > > > in the kernel for a long time. > As far as I know, since i.MX6SX, we only use MMDC_CH1, but MMDC_CH0 is still > there and we need to bypass its handshake when entering low power mode. > > The bit 19 in DOC is incorrect, I remembered I ever tried it and discuss with design > time, they request DOC team to update DOC, > but I think doc team forgot to do it. You can try removing this bit 19 setting > and see if standby/mem suspend can still work? And try to modify this bit 19 value > to see if it can be modified. After some testing it seems that setting MMDC_CH0 is indeed required. So nevermind I remember testing this earlier and getting the opposite result but I probably confused the STOP_POWER_ON and STOP_POWER_OFF paths. -- Regards, Leonard ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-06-07 9:44 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-05-30 16:11 [PATCH] ARM: imx6ull: Make suspend/resume work like on 6ul Leonard Crestez 2017-06-05 5:37 ` Shawn Guo 2017-06-06 10:51 ` Leonard Crestez 2017-06-07 3:21 ` Shawn Guo 2017-06-07 3:38 ` Anson Huang 2017-06-07 9:44 ` Leonard Crestez
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).