[PATCH v1 1/1] mfd: intel-lpss: save register context on suspend

[PATCH v1 1/1] mfd: intel-lpss: save register context on suspend

[Andy Shevchenko]

From: Heikki Krogerus <heikki.krogerus@linux.intel.com>

All configurations are lost and the registers will have
default values when the hardware is suspended and resumed,
so saving the private register space context on suspend, and
restoring it on resume.

Fixes: 4b45efe85263 (mfd: Add support for Intel Sunrisepoint LPSS devices)
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/mfd/intel-lpss.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/mfd/intel-lpss.c b/drivers/mfd/intel-lpss.c
index 1743788..2a7c6a7 100644
--- a/drivers/mfd/intel-lpss.c
+++ b/drivers/mfd/intel-lpss.c
@@ -34,6 +34,7 @@
 #define LPSS_DEV_SIZE		0x200
 #define LPSS_PRIV_OFFSET	0x200
 #define LPSS_PRIV_SIZE		0x100
+#define LPSS_PRIV_REG_COUNT	(LPSS_PRIV_SIZE / 4)
 #define LPSS_IDMA64_OFFSET	0x800
 #define LPSS_IDMA64_SIZE	0x800
 
@@ -76,6 +77,7 @@ struct intel_lpss {
 	struct mfd_cell *cell;
 	struct device *dev;
 	void __iomem *priv;
+	u32 priv_ctx[LPSS_PRIV_REG_COUNT];
 	int devid;
 	u32 caps;
 	u32 active_ltr;
@@ -492,6 +494,16 @@ EXPORT_SYMBOL_GPL(intel_lpss_prepare);
 
 int intel_lpss_suspend(struct device *dev)
 {
+	struct intel_lpss *lpss = dev_get_drvdata(dev);
+	unsigned int i;
+
+	/* Save device context */
+	for (i = 0; i < LPSS_PRIV_REG_COUNT; i++)
+		lpss->priv_ctx[i] = readl(lpss->priv + i * 4);
+
+	/* Put the device into reset state */
+	writel(0, lpss->priv + LPSS_PRIV_RESETS);
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(intel_lpss_suspend);
@@ -499,8 +511,13 @@ EXPORT_SYMBOL_GPL(intel_lpss_suspend);
 int intel_lpss_resume(struct device *dev)
 {
 	struct intel_lpss *lpss = dev_get_drvdata(dev);
+	unsigned int i;
 
-	intel_lpss_init_dev(lpss);
+	intel_lpss_deassert_reset(lpss);
+
+	/* Restore device context */
+	for (i = 0; i < LPSS_PRIV_REG_COUNT; i++)
+		writel(lpss->priv_ctx[i], lpss->priv + i * 4);
 
 	return 0;
 }
-- 
2.7.0.rc3

Re: [PATCH v1 1/1] mfd: intel-lpss: save register context on suspend

[Heikki Krogerus]

Hi Lee,

Gentle ping.

On Tue, Jan 26, 2016 at 01:42:26PM +0200, Andy Shevchenko wrote:
> From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> 
> All configurations are lost and the registers will have
> default values when the hardware is suspended and resumed,
> so saving the private register space context on suspend, and
> restoring it on resume.
> 
> Fixes: 4b45efe85263 (mfd: Add support for Intel Sunrisepoint LPSS devices)
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/mfd/intel-lpss.c | 19 ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mfd/intel-lpss.c b/drivers/mfd/intel-lpss.c
> index 1743788..2a7c6a7 100644
> --- a/drivers/mfd/intel-lpss.c
> +++ b/drivers/mfd/intel-lpss.c
> @@ -34,6 +34,7 @@
>  #define LPSS_DEV_SIZE		0x200
>  #define LPSS_PRIV_OFFSET	0x200
>  #define LPSS_PRIV_SIZE		0x100
> +#define LPSS_PRIV_REG_COUNT	(LPSS_PRIV_SIZE / 4)
>  #define LPSS_IDMA64_OFFSET	0x800
>  #define LPSS_IDMA64_SIZE	0x800
>  
> @@ -76,6 +77,7 @@ struct intel_lpss {
>  	struct mfd_cell *cell;
>  	struct device *dev;
>  	void __iomem *priv;
> +	u32 priv_ctx[LPSS_PRIV_REG_COUNT];
>  	int devid;
>  	u32 caps;
>  	u32 active_ltr;
> @@ -492,6 +494,16 @@ EXPORT_SYMBOL_GPL(intel_lpss_prepare);
>  
>  int intel_lpss_suspend(struct device *dev)
>  {
> +	struct intel_lpss *lpss = dev_get_drvdata(dev);
> +	unsigned int i;
> +
> +	/* Save device context */
> +	for (i = 0; i < LPSS_PRIV_REG_COUNT; i++)
> +		lpss->priv_ctx[i] = readl(lpss->priv + i * 4);
> +
> +	/* Put the device into reset state */
> +	writel(0, lpss->priv + LPSS_PRIV_RESETS);
> +
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(intel_lpss_suspend);
> @@ -499,8 +511,13 @@ EXPORT_SYMBOL_GPL(intel_lpss_suspend);
>  int intel_lpss_resume(struct device *dev)
>  {
>  	struct intel_lpss *lpss = dev_get_drvdata(dev);
> +	unsigned int i;
>  
> -	intel_lpss_init_dev(lpss);
> +	intel_lpss_deassert_reset(lpss);
> +
> +	/* Restore device context */
> +	for (i = 0; i < LPSS_PRIV_REG_COUNT; i++)
> +		writel(lpss->priv_ctx[i], lpss->priv + i * 4);
>  
>  	return 0;
>  }
> -- 
> 2.7.0.rc3

-- 
heikki

Re: [PATCH v1 1/1] mfd: intel-lpss: save register context on suspend

[Lee Jones]

On Tue, 26 Jan 2016, Andy Shevchenko wrote:

> From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> 
> All configurations are lost and the registers will have
> default values when the hardware is suspended and resumed,
> so saving the private register space context on suspend, and
> restoring it on resume.
> 
> Fixes: 4b45efe85263 (mfd: Add support for Intel Sunrisepoint LPSS devices)

I'd say this sumbission is more of a functional extention than a
bug-fix.  No real need for this statement.

> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Did it really take 2 of you to write this patch?

> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/mfd/intel-lpss.c | 19 ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)

Code looks okay though:
  Acked-by: Lee Jones <lee.jones@linaro.org>

> diff --git a/drivers/mfd/intel-lpss.c b/drivers/mfd/intel-lpss.c
> index 1743788..2a7c6a7 100644
> --- a/drivers/mfd/intel-lpss.c
> +++ b/drivers/mfd/intel-lpss.c
> @@ -34,6 +34,7 @@
>  #define LPSS_DEV_SIZE		0x200
>  #define LPSS_PRIV_OFFSET	0x200
>  #define LPSS_PRIV_SIZE		0x100
> +#define LPSS_PRIV_REG_COUNT	(LPSS_PRIV_SIZE / 4)
>  #define LPSS_IDMA64_OFFSET	0x800
>  #define LPSS_IDMA64_SIZE	0x800
>  
> @@ -76,6 +77,7 @@ struct intel_lpss {
>  	struct mfd_cell *cell;
>  	struct device *dev;
>  	void __iomem *priv;
> +	u32 priv_ctx[LPSS_PRIV_REG_COUNT];
>  	int devid;
>  	u32 caps;
>  	u32 active_ltr;
> @@ -492,6 +494,16 @@ EXPORT_SYMBOL_GPL(intel_lpss_prepare);
>  
>  int intel_lpss_suspend(struct device *dev)
>  {
> +	struct intel_lpss *lpss = dev_get_drvdata(dev);
> +	unsigned int i;
> +
> +	/* Save device context */
> +	for (i = 0; i < LPSS_PRIV_REG_COUNT; i++)
> +		lpss->priv_ctx[i] = readl(lpss->priv + i * 4);
> +
> +	/* Put the device into reset state */
> +	writel(0, lpss->priv + LPSS_PRIV_RESETS);
> +
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(intel_lpss_suspend);
> @@ -499,8 +511,13 @@ EXPORT_SYMBOL_GPL(intel_lpss_suspend);
>  int intel_lpss_resume(struct device *dev)
>  {
>  	struct intel_lpss *lpss = dev_get_drvdata(dev);
> +	unsigned int i;
>  
> -	intel_lpss_init_dev(lpss);
> +	intel_lpss_deassert_reset(lpss);
> +
> +	/* Restore device context */
> +	for (i = 0; i < LPSS_PRIV_REG_COUNT; i++)
> +		writel(lpss->priv_ctx[i], lpss->priv + i * 4);
>  
>  	return 0;
>  }

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v1 1/1] mfd: intel-lpss: save register context on suspend

[Andy Shevchenko]

On Tue, Mar 8, 2016 at 6:48 AM, Lee Jones <lee.jones@linaro.org> wrote:
> On Tue, 26 Jan 2016, Andy Shevchenko wrote:
>
>> From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
>>
>> All configurations are lost and the registers will have
>> default values when the hardware is suspended and resumed,
>> so saving the private register space context on suspend, and
>> restoring it on resume.
>>
>> Fixes: 4b45efe85263 (mfd: Add support for Intel Sunrisepoint LPSS devices)
>
> I'd say this sumbission is more of a functional extention than a
> bug-fix.  No real need for this statement.

But we would like send it later to stable@ to have this back ported.

>> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
>> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
>
> Did it really take 2 of you to write this patch?

It's probably my mistake, since I took this one from Mika's tree and
kept his SoB.

>
>> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> ---
>>  drivers/mfd/intel-lpss.c | 19 ++++++++++++++++++-
>>  1 file changed, 18 insertions(+), 1 deletion(-)
>
> Code looks okay though:
>   Acked-by: Lee Jones <lee.jones@linaro.org>

Thanks.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/1] mfd: intel-lpss: save register context on suspend

[Lee Jones]

On Tue, 08 Mar 2016, Andy Shevchenko wrote:

> On Tue, Mar 8, 2016 at 6:48 AM, Lee Jones <lee.jones@linaro.org> wrote:
> > On Tue, 26 Jan 2016, Andy Shevchenko wrote:
> >
> >> From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> >>
> >> All configurations are lost and the registers will have
> >> default values when the hardware is suspended and resumed,
> >> so saving the private register space context on suspend, and
> >> restoring it on resume.
> >>
> >> Fixes: 4b45efe85263 (mfd: Add support for Intel Sunrisepoint LPSS devices)
> >
> > I'd say this sumbission is more of a functional extention than a
> > bug-fix.  No real need for this statement.
> 
> But we would like send it later to stable@ to have this back ported.

Yes, I see.

Although, as it "fixes" a first submission, my personal preference
would be a:

  Cc: stable@vger.kernel.org # v4.{whatever}

.. tag instead.

> >> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> >> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> >
> > Did it really take 2 of you to write this patch?
> 
> It's probably my mistake, since I took this one from Mika's tree and
> kept his SoB.

No problem.

Please omit his SoB when upstreaming patches from BSP trees.

> >> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> >> ---
> >>  drivers/mfd/intel-lpss.c | 19 ++++++++++++++++++-
> >>  1 file changed, 18 insertions(+), 1 deletion(-)
> >
> > Code looks okay though:
> >   Acked-by: Lee Jones <lee.jones@linaro.org>
> 
> Thanks.
> 

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v1 1/1] mfd: intel-lpss: save register context on suspend

[Heikki Krogerus]

Hi guys,

On Tue, Mar 08, 2016 at 02:54:21PM +0700, Lee Jones wrote:
> On Tue, 08 Mar 2016, Andy Shevchenko wrote:
> > >> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > >> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > >
> > > Did it really take 2 of you to write this patch?
> > 
> > It's probably my mistake, since I took this one from Mika's tree and
> > kept his SoB.
> 
> No problem.
> 
> Please omit his SoB when upstreaming patches from BSP trees.

So what is the plan with this patch?

Lee! Do you want us to resend with the SoB removed?


Thanks,

-- 
heikki

Re: [PATCH v1 1/1] mfd: intel-lpss: save register context on suspend

[Lee Jones]

On Wed, 30 Mar 2016, Heikki Krogerus wrote:

> Hi guys,
> 
> On Tue, Mar 08, 2016 at 02:54:21PM +0700, Lee Jones wrote:
> > On Tue, 08 Mar 2016, Andy Shevchenko wrote:
> > > >> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > > >> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > > >
> > > > Did it really take 2 of you to write this patch?
> > > 
> > > It's probably my mistake, since I took this one from Mika's tree and
> > > kept his SoB.
> > 
> > No problem.
> > 
> > Please omit his SoB when upstreaming patches from BSP trees.
> 
> So what is the plan with this patch?
> 
> Lee! Do you want us to resend with the SoB removed?

I want you to resubmit with my review comments rectified, as is
normal.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog