Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1352624 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v1 1/1] mfd: intel-lpss: save register context on suspend Lee Jones <lee.jones@linaro.org> - 2016-03-08 05:50 +0100
    Re: [PATCH v1 1/1] mfd: intel-lpss: save register context on suspend Andy Shevchenko <andy.shevchenko@gmail.com> - 2016-03-08 08:00 +0100
      Re: [PATCH v1 1/1] mfd: intel-lpss: save register context on suspend Lee Jones <lee.jones@linaro.org> - 2016-03-08 09:00 +0100

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

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

[toc] | [next] | [standalone]

#1352674

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

[toc] | [prev] | [next] | [standalone]

#1352693

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web