Re: [RFC 2/2] soc: samsung: pmu: Add the PMU data of exynos4412 to support low-power state

2019-02-15 Thread Anand Moon
Hi Krzysztof,

On Fri, 15 Feb 2019 at 13:03, Krzysztof Kozlowski  wrote:
>
> On Thu, 14 Feb 2019 at 19:37, Anand Moon  wrote:
> >
> > Hi Krzysztof,
> >
> > Thanks for your review comments.
> >
> > On Thu, 14 Feb 2019 at 18:29, Krzysztof Kozlowski  wrote:
> > >
> > > On Wed, 13 Feb 2019 at 22:41, Anand Moon  wrote:
> > > >
> > > > This patch adds configration for PMU (Power Management Unit) state
> > > > tuning for exynos4412 SoC in order to enter low-power mode during
> > > > suspend power modes and help resume from suspend state.
> > >
> > > The U3 and Trats2 already enter STOP/S2R so please describe what
> > > exactly you change.
> > >
> > > > Fixes: bfce552d0b1 ("drivers: soc: Add support for Exynos PMU driver")
> > >
> > > How it fixes it? What was broken in that commit?
> >
> > * I was not aware on their is common framework for suspend and resume
> > other than setting this here.I only look in to some the other exynos
> > pmu architecture
> > and referring 3.10.x kernel to model my changes.*
>
> Suspend in general should be working already so adding this code just
> because is not a valid reason. Please specify what is not working
> first.
>

Please accept *my apology* for not studying the code changes required.
Sorry for wasting your precious time in review.

My only intention with this patch was to correctly initialize some PMU
parameters.
which might be wrong. so lets drop this.

> > >
> > > > Cc: Marek Szyprowski 
> > > > Cc: Krzysztof Kozlowski 
> > > > Cc: Chanwoo Choi 
> > > > Signed-off-by: Anand Moon 
> > > > ---
> > > >
> > > > Changes from previous patch.
> > > > New patch to this series to support suspend and resume state
> > > >
> > > > Changes have been tested on microSD card but fails to resume on cMMC.
> > > > It need to be investigated and more debuging
> > > > ---
> > > >  drivers/soc/samsung/exynos4-pmu.c   | 83 +
> > > >  include/linux/soc/samsung/exynos-regs-pmu.h | 21 ++
> > > >  2 files changed, 104 insertions(+)
> > > >
> > > > diff --git a/drivers/soc/samsung/exynos4-pmu.c 
> > > > b/drivers/soc/samsung/exynos4-pmu.c
> > > > index a7cdbf1aac0c..d261a0d2371e 100644
> > > > --- a/drivers/soc/samsung/exynos4-pmu.c
> > > > +++ b/drivers/soc/samsung/exynos4-pmu.c
> > > > @@ -200,10 +200,93 @@ static const struct exynos_pmu_conf 
> > > > exynos4412_pmu_config[] = {
> > > > { PMU_TABLE_END,},
> > > >  };
> > > >
> > > > +static unsigned int const exynos4412_list_feed[] = {
> > > > +   EXYNOS4_ARM_CORE0_OPTION,
> > > > +   EXYNOS4_ARM_CORE1_OPTION,
> > > > +   EXYNOS4_ARM_CORE2_OPTION,
> > > > +   EXYNOS4_ARM_CORE3_OPTION,
> > > > +   EXYNOS4_ARM_COMMON_OPTION,
> > > > +   EXYNOS4_CAM_OPTION,
> > > > +   EXYNOS4_TV_OPTION,
> > > > +   EXYNOS4_MFC_OPTION,
> > > > +   EXYNOS4_G3D_OPTION,
> > > > +   EXYNOS4_LCD0_OPTION,
> > > > +   EXYNOS4_ISP_OPTION,
> > > > +   EXYNOS4_MAUDIO_OPTION,
> > > > +   EXYNOS4_GPS_OPTION,
> > > > +   EXYNOS4_GPS_ALIVE_OPTION,
> > > > +};
> > > > +
> > > > +static void exynos4412_pmu_central_seq(bool enable)
> > >
> > > You name the argument as "enable"  but during initialization and
> > > system running you pass here false. It confuses me. What do you enable
> > > here?
> > >
> >
> > Yep your are correct need to drop this function as already done in
> > common frame work.
> >
> > > > +{
> > > > +   unsigned int value;
> > > > +
> > > > +   value = pmu_raw_readl(S5P_CENTRAL_SEQ_CONFIGURATION);
> > > > +   if (enable)
> > > > +   value &= ~S5P_CENTRAL_LOWPWR_CFG;
> > > > +   else
> > > > +   value |= S5P_CENTRAL_LOWPWR_CFG;
> > > > +   pmu_raw_writel(value, S5P_CENTRAL_SEQ_CONFIGURATION);
> > >
> > > You duplicate exynos_pm_central_suspend() without removing the original 
> > > code.
> > >
> > > > +
> > > > +   value = pmu_raw_readl(S5P_CENTRAL_SEQ_CONFIGURATION_COREBLK);
> > > > +   if (enable)
> > > > +   value &= ~S5P_CENTRAL_LOWPWR_CFG;
> > > > +   else
> > > > +   value |= S5P_CENTRAL_LOWPWR_CFG;
> > >
> > > As manual says - set this register only if you disable C2C. Our entire
> > > low power configuration for STOP mode is for C2C enabled case so you
> > > add inconsistent configuration.
> >
> > Ok Sorry I overlook this code change.
> >
> > Enable system power down. Set only CENTRAL_SEQ_CONFIGURATION register
> > if you disable C2C.
> > Set both CENTRAL_SEQ_CONFIGURATION and
> > CENTRAL_SEQ_CONFIGURATION_COREBLK registers if you enable C2C.
> >
> > > > +   pmu_raw_writel(value, S5P_CENTRAL_SEQ_CONFIGURATION_COREBLK);
> > > > +}
> > > > +
> > > > +static void exynos4412_pmu_init(void)
> > > > +{
> > > > +   unsigned int value;
> > > > +   int i;
> > > > +
> > > > +   /* Enable USE_STANDBY_WFI for all CORE */
> > > > +   pmu_raw_writel(S5P_USE_STANDBY_WFI_ALL, S5P_CENTRAL_SEQ_OPTION);
> > >
> > > This does not look related to improving suspend... it looks 

Re: [RFC 2/2] soc: samsung: pmu: Add the PMU data of exynos4412 to support low-power state

2019-02-14 Thread Krzysztof Kozlowski
On Thu, 14 Feb 2019 at 19:37, Anand Moon  wrote:
>
> Hi Krzysztof,
>
> Thanks for your review comments.
>
> On Thu, 14 Feb 2019 at 18:29, Krzysztof Kozlowski  wrote:
> >
> > On Wed, 13 Feb 2019 at 22:41, Anand Moon  wrote:
> > >
> > > This patch adds configration for PMU (Power Management Unit) state
> > > tuning for exynos4412 SoC in order to enter low-power mode during
> > > suspend power modes and help resume from suspend state.
> >
> > The U3 and Trats2 already enter STOP/S2R so please describe what
> > exactly you change.
> >
> > > Fixes: bfce552d0b1 ("drivers: soc: Add support for Exynos PMU driver")
> >
> > How it fixes it? What was broken in that commit?
>
> * I was not aware on their is common framework for suspend and resume
> other than setting this here.I only look in to some the other exynos
> pmu architecture
> and referring 3.10.x kernel to model my changes.*

Suspend in general should be working already so adding this code just
because is not a valid reason. Please specify what is not working
first.

> >
> > > Cc: Marek Szyprowski 
> > > Cc: Krzysztof Kozlowski 
> > > Cc: Chanwoo Choi 
> > > Signed-off-by: Anand Moon 
> > > ---
> > >
> > > Changes from previous patch.
> > > New patch to this series to support suspend and resume state
> > >
> > > Changes have been tested on microSD card but fails to resume on cMMC.
> > > It need to be investigated and more debuging
> > > ---
> > >  drivers/soc/samsung/exynos4-pmu.c   | 83 +
> > >  include/linux/soc/samsung/exynos-regs-pmu.h | 21 ++
> > >  2 files changed, 104 insertions(+)
> > >
> > > diff --git a/drivers/soc/samsung/exynos4-pmu.c 
> > > b/drivers/soc/samsung/exynos4-pmu.c
> > > index a7cdbf1aac0c..d261a0d2371e 100644
> > > --- a/drivers/soc/samsung/exynos4-pmu.c
> > > +++ b/drivers/soc/samsung/exynos4-pmu.c
> > > @@ -200,10 +200,93 @@ static const struct exynos_pmu_conf 
> > > exynos4412_pmu_config[] = {
> > > { PMU_TABLE_END,},
> > >  };
> > >
> > > +static unsigned int const exynos4412_list_feed[] = {
> > > +   EXYNOS4_ARM_CORE0_OPTION,
> > > +   EXYNOS4_ARM_CORE1_OPTION,
> > > +   EXYNOS4_ARM_CORE2_OPTION,
> > > +   EXYNOS4_ARM_CORE3_OPTION,
> > > +   EXYNOS4_ARM_COMMON_OPTION,
> > > +   EXYNOS4_CAM_OPTION,
> > > +   EXYNOS4_TV_OPTION,
> > > +   EXYNOS4_MFC_OPTION,
> > > +   EXYNOS4_G3D_OPTION,
> > > +   EXYNOS4_LCD0_OPTION,
> > > +   EXYNOS4_ISP_OPTION,
> > > +   EXYNOS4_MAUDIO_OPTION,
> > > +   EXYNOS4_GPS_OPTION,
> > > +   EXYNOS4_GPS_ALIVE_OPTION,
> > > +};
> > > +
> > > +static void exynos4412_pmu_central_seq(bool enable)
> >
> > You name the argument as "enable"  but during initialization and
> > system running you pass here false. It confuses me. What do you enable
> > here?
> >
>
> Yep your are correct need to drop this function as already done in
> common frame work.
>
> > > +{
> > > +   unsigned int value;
> > > +
> > > +   value = pmu_raw_readl(S5P_CENTRAL_SEQ_CONFIGURATION);
> > > +   if (enable)
> > > +   value &= ~S5P_CENTRAL_LOWPWR_CFG;
> > > +   else
> > > +   value |= S5P_CENTRAL_LOWPWR_CFG;
> > > +   pmu_raw_writel(value, S5P_CENTRAL_SEQ_CONFIGURATION);
> >
> > You duplicate exynos_pm_central_suspend() without removing the original 
> > code.
> >
> > > +
> > > +   value = pmu_raw_readl(S5P_CENTRAL_SEQ_CONFIGURATION_COREBLK);
> > > +   if (enable)
> > > +   value &= ~S5P_CENTRAL_LOWPWR_CFG;
> > > +   else
> > > +   value |= S5P_CENTRAL_LOWPWR_CFG;
> >
> > As manual says - set this register only if you disable C2C. Our entire
> > low power configuration for STOP mode is for C2C enabled case so you
> > add inconsistent configuration.
>
> Ok Sorry I overlook this code change.
>
> Enable system power down. Set only CENTRAL_SEQ_CONFIGURATION register
> if you disable C2C.
> Set both CENTRAL_SEQ_CONFIGURATION and
> CENTRAL_SEQ_CONFIGURATION_COREBLK registers if you enable C2C.
>
> > > +   pmu_raw_writel(value, S5P_CENTRAL_SEQ_CONFIGURATION_COREBLK);
> > > +}
> > > +
> > > +static void exynos4412_pmu_init(void)
> > > +{
> > > +   unsigned int value;
> > > +   int i;
> > > +
> > > +   /* Enable USE_STANDBY_WFI for all CORE */
> > > +   pmu_raw_writel(S5P_USE_STANDBY_WFI_ALL, S5P_CENTRAL_SEQ_OPTION);
> >
> > This does not look related to improving suspend... it looks unrelated.
> >
>
> Ok  yes your are correct all ready done in comment frame work.
>
> "Execute WFI/WFE for all CPU cores. As soon as all the CPU cores in
> Exynos 4412 SCP enter STANDBY mode"
> > > +
> > > +   /* Decides whether to use retention capability */
> > > +   value  = pmu_raw_readl(S5P_ARM_L2_0_OPTION);
> > > +   value &= ~EXYNOS_L2_USE_RETENTION;
> > > +   pmu_raw_writel(value, S5P_ARM_L2_0_OPTION);
> > > +
> > > +   value = pmu_raw_readl(S5P_ARM_L2_1_OPTION);
> > > +   value &= ~EXYNOS_L2_USE_RETENTION;
> > > +   pmu_raw_writel(value, 

Re: [RFC 2/2] soc: samsung: pmu: Add the PMU data of exynos4412 to support low-power state

2019-02-14 Thread Anand Moon
Hi Krzysztof,

Thanks for your review comments.

On Thu, 14 Feb 2019 at 18:29, Krzysztof Kozlowski  wrote:
>
> On Wed, 13 Feb 2019 at 22:41, Anand Moon  wrote:
> >
> > This patch adds configration for PMU (Power Management Unit) state
> > tuning for exynos4412 SoC in order to enter low-power mode during
> > suspend power modes and help resume from suspend state.
>
> The U3 and Trats2 already enter STOP/S2R so please describe what
> exactly you change.
>
> > Fixes: bfce552d0b1 ("drivers: soc: Add support for Exynos PMU driver")
>
> How it fixes it? What was broken in that commit?

* I was not aware on their is common framework for suspend and resume
other than setting this here.I only look in to some the other exynos
pmu architecture
and referring 3.10.x kernel to model my changes.*

>
> > Cc: Marek Szyprowski 
> > Cc: Krzysztof Kozlowski 
> > Cc: Chanwoo Choi 
> > Signed-off-by: Anand Moon 
> > ---
> >
> > Changes from previous patch.
> > New patch to this series to support suspend and resume state
> >
> > Changes have been tested on microSD card but fails to resume on cMMC.
> > It need to be investigated and more debuging
> > ---
> >  drivers/soc/samsung/exynos4-pmu.c   | 83 +
> >  include/linux/soc/samsung/exynos-regs-pmu.h | 21 ++
> >  2 files changed, 104 insertions(+)
> >
> > diff --git a/drivers/soc/samsung/exynos4-pmu.c 
> > b/drivers/soc/samsung/exynos4-pmu.c
> > index a7cdbf1aac0c..d261a0d2371e 100644
> > --- a/drivers/soc/samsung/exynos4-pmu.c
> > +++ b/drivers/soc/samsung/exynos4-pmu.c
> > @@ -200,10 +200,93 @@ static const struct exynos_pmu_conf 
> > exynos4412_pmu_config[] = {
> > { PMU_TABLE_END,},
> >  };
> >
> > +static unsigned int const exynos4412_list_feed[] = {
> > +   EXYNOS4_ARM_CORE0_OPTION,
> > +   EXYNOS4_ARM_CORE1_OPTION,
> > +   EXYNOS4_ARM_CORE2_OPTION,
> > +   EXYNOS4_ARM_CORE3_OPTION,
> > +   EXYNOS4_ARM_COMMON_OPTION,
> > +   EXYNOS4_CAM_OPTION,
> > +   EXYNOS4_TV_OPTION,
> > +   EXYNOS4_MFC_OPTION,
> > +   EXYNOS4_G3D_OPTION,
> > +   EXYNOS4_LCD0_OPTION,
> > +   EXYNOS4_ISP_OPTION,
> > +   EXYNOS4_MAUDIO_OPTION,
> > +   EXYNOS4_GPS_OPTION,
> > +   EXYNOS4_GPS_ALIVE_OPTION,
> > +};
> > +
> > +static void exynos4412_pmu_central_seq(bool enable)
>
> You name the argument as "enable"  but during initialization and
> system running you pass here false. It confuses me. What do you enable
> here?
>

Yep your are correct need to drop this function as already done in
common frame work.

> > +{
> > +   unsigned int value;
> > +
> > +   value = pmu_raw_readl(S5P_CENTRAL_SEQ_CONFIGURATION);
> > +   if (enable)
> > +   value &= ~S5P_CENTRAL_LOWPWR_CFG;
> > +   else
> > +   value |= S5P_CENTRAL_LOWPWR_CFG;
> > +   pmu_raw_writel(value, S5P_CENTRAL_SEQ_CONFIGURATION);
>
> You duplicate exynos_pm_central_suspend() without removing the original code.
>
> > +
> > +   value = pmu_raw_readl(S5P_CENTRAL_SEQ_CONFIGURATION_COREBLK);
> > +   if (enable)
> > +   value &= ~S5P_CENTRAL_LOWPWR_CFG;
> > +   else
> > +   value |= S5P_CENTRAL_LOWPWR_CFG;
>
> As manual says - set this register only if you disable C2C. Our entire
> low power configuration for STOP mode is for C2C enabled case so you
> add inconsistent configuration.

Ok Sorry I overlook this code change.

Enable system power down. Set only CENTRAL_SEQ_CONFIGURATION register
if you disable C2C.
Set both CENTRAL_SEQ_CONFIGURATION and
CENTRAL_SEQ_CONFIGURATION_COREBLK registers if you enable C2C.

> > +   pmu_raw_writel(value, S5P_CENTRAL_SEQ_CONFIGURATION_COREBLK);
> > +}
> > +
> > +static void exynos4412_pmu_init(void)
> > +{
> > +   unsigned int value;
> > +   int i;
> > +
> > +   /* Enable USE_STANDBY_WFI for all CORE */
> > +   pmu_raw_writel(S5P_USE_STANDBY_WFI_ALL, S5P_CENTRAL_SEQ_OPTION);
>
> This does not look related to improving suspend... it looks unrelated.
>

Ok  yes your are correct all ready done in comment frame work.

"Execute WFI/WFE for all CPU cores. As soon as all the CPU cores in
Exynos 4412 SCP enter STANDBY mode"
> > +
> > +   /* Decides whether to use retention capability */
> > +   value  = pmu_raw_readl(S5P_ARM_L2_0_OPTION);
> > +   value &= ~EXYNOS_L2_USE_RETENTION;
> > +   pmu_raw_writel(value, S5P_ARM_L2_0_OPTION);
> > +
> > +   value = pmu_raw_readl(S5P_ARM_L2_1_OPTION);
> > +   value &= ~EXYNOS_L2_USE_RETENTION;
> > +   pmu_raw_writel(value, S5P_ARM_L2_1_OPTION);
> > +
> > +   /* Set PSHOLD port for output high */
> > +   value = pmu_raw_readl(S5P_PS_HOLD_CONTROL);
> > +   value |= S5P_PS_HOLD_OUTPUT_HIGH;
> > +   pmu_raw_writel(value, S5P_PS_HOLD_CONTROL);
> > +
> > +   /* Enable signal for PSHOLD port */
> > +   value = pmu_raw_readl(S5P_PS_HOLD_CONTROL);
> > +   value |= S5P_PS_HOLD_EN;
> > +   pmu_raw_writel(value, S5P_PS_HOLD_CONTROL);
> > +
>
> The same 

Re: [RFC 2/2] soc: samsung: pmu: Add the PMU data of exynos4412 to support low-power state

2019-02-14 Thread Krzysztof Kozlowski
On Wed, 13 Feb 2019 at 22:41, Anand Moon  wrote:
>
> This patch adds configration for PMU (Power Management Unit) state
> tuning for exynos4412 SoC in order to enter low-power mode during
> suspend power modes and help resume from suspend state.

The U3 and Trats2 already enter STOP/S2R so please describe what
exactly you change.

> Fixes: bfce552d0b1 ("drivers: soc: Add support for Exynos PMU driver")

How it fixes it? What was broken in that commit?

> Cc: Marek Szyprowski 
> Cc: Krzysztof Kozlowski 
> Cc: Chanwoo Choi 
> Signed-off-by: Anand Moon 
> ---
>
> Changes from previous patch.
> New patch to this series to support suspend and resume state
>
> Changes have been tested on microSD card but fails to resume on cMMC.
> It need to be investigated and more debuging
> ---
>  drivers/soc/samsung/exynos4-pmu.c   | 83 +
>  include/linux/soc/samsung/exynos-regs-pmu.h | 21 ++
>  2 files changed, 104 insertions(+)
>
> diff --git a/drivers/soc/samsung/exynos4-pmu.c 
> b/drivers/soc/samsung/exynos4-pmu.c
> index a7cdbf1aac0c..d261a0d2371e 100644
> --- a/drivers/soc/samsung/exynos4-pmu.c
> +++ b/drivers/soc/samsung/exynos4-pmu.c
> @@ -200,10 +200,93 @@ static const struct exynos_pmu_conf 
> exynos4412_pmu_config[] = {
> { PMU_TABLE_END,},
>  };
>
> +static unsigned int const exynos4412_list_feed[] = {
> +   EXYNOS4_ARM_CORE0_OPTION,
> +   EXYNOS4_ARM_CORE1_OPTION,
> +   EXYNOS4_ARM_CORE2_OPTION,
> +   EXYNOS4_ARM_CORE3_OPTION,
> +   EXYNOS4_ARM_COMMON_OPTION,
> +   EXYNOS4_CAM_OPTION,
> +   EXYNOS4_TV_OPTION,
> +   EXYNOS4_MFC_OPTION,
> +   EXYNOS4_G3D_OPTION,
> +   EXYNOS4_LCD0_OPTION,
> +   EXYNOS4_ISP_OPTION,
> +   EXYNOS4_MAUDIO_OPTION,
> +   EXYNOS4_GPS_OPTION,
> +   EXYNOS4_GPS_ALIVE_OPTION,
> +};
> +
> +static void exynos4412_pmu_central_seq(bool enable)

You name the argument as "enable"  but during initialization and
system running you pass here false. It confuses me. What do you enable
here?

> +{
> +   unsigned int value;
> +
> +   value = pmu_raw_readl(S5P_CENTRAL_SEQ_CONFIGURATION);
> +   if (enable)
> +   value &= ~S5P_CENTRAL_LOWPWR_CFG;
> +   else
> +   value |= S5P_CENTRAL_LOWPWR_CFG;
> +   pmu_raw_writel(value, S5P_CENTRAL_SEQ_CONFIGURATION);

You duplicate exynos_pm_central_suspend() without removing the original code.

> +
> +   value = pmu_raw_readl(S5P_CENTRAL_SEQ_CONFIGURATION_COREBLK);
> +   if (enable)
> +   value &= ~S5P_CENTRAL_LOWPWR_CFG;
> +   else
> +   value |= S5P_CENTRAL_LOWPWR_CFG;

As manual says - set this register only if you disable C2C. Our entire
low power configuration for STOP mode is for C2C enabled case so you
add inconsistent configuration.

> +   pmu_raw_writel(value, S5P_CENTRAL_SEQ_CONFIGURATION_COREBLK);
> +}
> +
> +static void exynos4412_pmu_init(void)
> +{
> +   unsigned int value;
> +   int i;
> +
> +   /* Enable USE_STANDBY_WFI for all CORE */
> +   pmu_raw_writel(S5P_USE_STANDBY_WFI_ALL, S5P_CENTRAL_SEQ_OPTION);

This does not look related to improving suspend... it looks unrelated.

> +
> +   /* Decides whether to use retention capability */
> +   value  = pmu_raw_readl(S5P_ARM_L2_0_OPTION);
> +   value &= ~EXYNOS_L2_USE_RETENTION;
> +   pmu_raw_writel(value, S5P_ARM_L2_0_OPTION);
> +
> +   value = pmu_raw_readl(S5P_ARM_L2_1_OPTION);
> +   value &= ~EXYNOS_L2_USE_RETENTION;
> +   pmu_raw_writel(value, S5P_ARM_L2_1_OPTION);
> +
> +   /* Set PSHOLD port for output high */
> +   value = pmu_raw_readl(S5P_PS_HOLD_CONTROL);
> +   value |= S5P_PS_HOLD_OUTPUT_HIGH;
> +   pmu_raw_writel(value, S5P_PS_HOLD_CONTROL);
> +
> +   /* Enable signal for PSHOLD port */
> +   value = pmu_raw_readl(S5P_PS_HOLD_CONTROL);
> +   value |= S5P_PS_HOLD_EN;
> +   pmu_raw_writel(value, S5P_PS_HOLD_CONTROL);
> +

The same - not related.

> +   /* Enable only SC_FEEDBACK */
> +   for (i = 0; i < ARRAY_SIZE(exynos4412_list_feed); i++) {
> +   value =  pmu_raw_readl(exynos4412_list_feed[i]);
> +   value &= ~(EXYNOS_USE_SC_COUNTER);
> +   value |= EXYNOS_USE_SC_FEEDBACK;
> +   pmu_raw_writel(value, exynos4412_list_feed[i]);

Why do you prefer to use feedback instead of counter?

> +   }
> +
> +   exynos4412_pmu_central_seq(false);
> +
> +   pr_info("EXYNOS4x12 PMU Initialize\n");
> +}
> +
> +static void exynos4412_powerdown_conf(enum sys_powerdown mode)
> +{
> +   exynos4412_pmu_central_seq(true);
> +}
> +
>  const struct exynos_pmu_data exynos4210_pmu_data = {
> .pmu_config = exynos4210_pmu_config,
>  };
>
>  const struct exynos_pmu_data exynos4412_pmu_data = {
> .pmu_config = exynos4412_pmu_config,
> +   .pmu_init   = exynos4412_pmu_init,
> +   .powerdown_conf = exynos4412_powerdown_conf,
>  };
> diff 

[RFC 2/2] soc: samsung: pmu: Add the PMU data of exynos4412 to support low-power state

2019-02-13 Thread Anand Moon
This patch adds configration for PMU (Power Management Unit) state
tuning for exynos4412 SoC in order to enter low-power mode during
suspend power modes and help resume from suspend state.

Fixes: bfce552d0b1 ("drivers: soc: Add support for Exynos PMU driver")
Cc: Marek Szyprowski 
Cc: Krzysztof Kozlowski 
Cc: Chanwoo Choi 
Signed-off-by: Anand Moon 
---

Changes from previous patch.
New patch to this series to support suspend and resume state

Changes have been tested on microSD card but fails to resume on cMMC.
It need to be investigated and more debuging
---
 drivers/soc/samsung/exynos4-pmu.c   | 83 +
 include/linux/soc/samsung/exynos-regs-pmu.h | 21 ++
 2 files changed, 104 insertions(+)

diff --git a/drivers/soc/samsung/exynos4-pmu.c 
b/drivers/soc/samsung/exynos4-pmu.c
index a7cdbf1aac0c..d261a0d2371e 100644
--- a/drivers/soc/samsung/exynos4-pmu.c
+++ b/drivers/soc/samsung/exynos4-pmu.c
@@ -200,10 +200,93 @@ static const struct exynos_pmu_conf 
exynos4412_pmu_config[] = {
{ PMU_TABLE_END,},
 };
 
+static unsigned int const exynos4412_list_feed[] = {
+   EXYNOS4_ARM_CORE0_OPTION,
+   EXYNOS4_ARM_CORE1_OPTION,
+   EXYNOS4_ARM_CORE2_OPTION,
+   EXYNOS4_ARM_CORE3_OPTION,
+   EXYNOS4_ARM_COMMON_OPTION,
+   EXYNOS4_CAM_OPTION,
+   EXYNOS4_TV_OPTION,
+   EXYNOS4_MFC_OPTION,
+   EXYNOS4_G3D_OPTION,
+   EXYNOS4_LCD0_OPTION,
+   EXYNOS4_ISP_OPTION,
+   EXYNOS4_MAUDIO_OPTION,
+   EXYNOS4_GPS_OPTION,
+   EXYNOS4_GPS_ALIVE_OPTION,
+};
+
+static void exynos4412_pmu_central_seq(bool enable)
+{
+   unsigned int value;
+
+   value = pmu_raw_readl(S5P_CENTRAL_SEQ_CONFIGURATION);
+   if (enable)
+   value &= ~S5P_CENTRAL_LOWPWR_CFG;
+   else
+   value |= S5P_CENTRAL_LOWPWR_CFG;
+   pmu_raw_writel(value, S5P_CENTRAL_SEQ_CONFIGURATION);
+
+   value = pmu_raw_readl(S5P_CENTRAL_SEQ_CONFIGURATION_COREBLK);
+   if (enable)
+   value &= ~S5P_CENTRAL_LOWPWR_CFG;
+   else
+   value |= S5P_CENTRAL_LOWPWR_CFG;
+   pmu_raw_writel(value, S5P_CENTRAL_SEQ_CONFIGURATION_COREBLK);
+}
+
+static void exynos4412_pmu_init(void)
+{
+   unsigned int value;
+   int i;
+
+   /* Enable USE_STANDBY_WFI for all CORE */
+   pmu_raw_writel(S5P_USE_STANDBY_WFI_ALL, S5P_CENTRAL_SEQ_OPTION);
+
+   /* Decides whether to use retention capability */
+   value  = pmu_raw_readl(S5P_ARM_L2_0_OPTION);
+   value &= ~EXYNOS_L2_USE_RETENTION;
+   pmu_raw_writel(value, S5P_ARM_L2_0_OPTION);
+
+   value = pmu_raw_readl(S5P_ARM_L2_1_OPTION);
+   value &= ~EXYNOS_L2_USE_RETENTION;
+   pmu_raw_writel(value, S5P_ARM_L2_1_OPTION);
+
+   /* Set PSHOLD port for output high */
+   value = pmu_raw_readl(S5P_PS_HOLD_CONTROL);
+   value |= S5P_PS_HOLD_OUTPUT_HIGH;
+   pmu_raw_writel(value, S5P_PS_HOLD_CONTROL);
+
+   /* Enable signal for PSHOLD port */
+   value = pmu_raw_readl(S5P_PS_HOLD_CONTROL);
+   value |= S5P_PS_HOLD_EN;
+   pmu_raw_writel(value, S5P_PS_HOLD_CONTROL);
+
+   /* Enable only SC_FEEDBACK */
+   for (i = 0; i < ARRAY_SIZE(exynos4412_list_feed); i++) {
+   value =  pmu_raw_readl(exynos4412_list_feed[i]);
+   value &= ~(EXYNOS_USE_SC_COUNTER);
+   value |= EXYNOS_USE_SC_FEEDBACK;
+   pmu_raw_writel(value, exynos4412_list_feed[i]);
+   }
+
+   exynos4412_pmu_central_seq(false);
+
+   pr_info("EXYNOS4x12 PMU Initialize\n");
+}
+
+static void exynos4412_powerdown_conf(enum sys_powerdown mode)
+{
+   exynos4412_pmu_central_seq(true);
+}
+
 const struct exynos_pmu_data exynos4210_pmu_data = {
.pmu_config = exynos4210_pmu_config,
 };
 
 const struct exynos_pmu_data exynos4412_pmu_data = {
.pmu_config = exynos4412_pmu_config,
+   .pmu_init   = exynos4412_pmu_init,
+   .powerdown_conf = exynos4412_powerdown_conf,
 };
diff --git a/include/linux/soc/samsung/exynos-regs-pmu.h 
b/include/linux/soc/samsung/exynos-regs-pmu.h
index 5addaf5ccbce..6beed3e669d2 100644
--- a/include/linux/soc/samsung/exynos-regs-pmu.h
+++ b/include/linux/soc/samsung/exynos-regs-pmu.h
@@ -16,6 +16,8 @@
 
 #define S5P_CENTRAL_SEQ_CONFIGURATION  0x0200
 
+#define S5P_CENTRAL_SEQ_CONFIGURATION_COREBLK   0x0240
+
 #define S5P_CENTRAL_LOWPWR_CFG (1 << 16)
 
 #define S5P_CENTRAL_SEQ_OPTION 0x0208
@@ -347,6 +349,25 @@
 #define EXYNOS3_OPTION_USE_SC_FEEDBACK (1 << 1)
 #define EXYNOS3_OPTION_SKIP_DEACTIVATE_ACEACP_IN_PWDN  (1 << 7)
 
+/* EXYNOS4 */
+#define EXYNOS_USE_SC_FEEDBACK BIT(1)
+#define EXYNOS_USE_SC_COUNTER  BIT(0)
+
+#define EXYNOS4_ARM_CORE0_OPTION   0x2008
+#define EXYNOS4_ARM_CORE1_OPTION   0x2088
+#define EXYNOS4_ARM_CORE2_OPTION