Skip to contentSkip to conversation

mfd: intel-lpss: save register context on suspend

PATCH Superseded Andy Shevchenko 4 participants 7 messages Jan 26, 2016 – Mar 30, 2016

Subsystems (2)

v1 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

Andy Shevchenko

author reviewer
#0 lore raw

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

From: Heikki Krogerus <[email protected]>

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)
---
 drivers/mfd/intel-lpss.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)
drivers/mfd/intel-lpss.c +18 -1
index 1743788..2a7c6a7 100644
@@ -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;
 }
Signature · 1 line
2.7.0.rc3
#1 lore raw

Hi Lee,

Gentle ping.

Quoted 69 lines · Andy Shevchenko

From: Heikki Krogerus <[email protected]>

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)
---
 drivers/mfd/intel-lpss.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)
drivers/mfd/intel-lpss.c +18 -1
index 1743788..2a7c6a7 100644
@@ -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;
 }
Signature · 1 line
2.7.0.rc3
Signature · 1 line
heikki
Acked-by on v1 1/1
#2 lore raw
In reply to Andy Shevchenko
Quoted 8 lines · Andy Shevchenko

From: Heikki Krogerus <[email protected]>

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.

Quoted 2 lines

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

Quoted 4 lines
---
 drivers/mfd/intel-lpss.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

Code looks okay though:

Quoted 52 lines
drivers/mfd/intel-lpss.c +18 -1
index 1743788..2a7c6a7 100644
@@ -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;
 }
Signature · 4 lines
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
#3 lore raw
Quoted 9 lines · Lee Jones
Quoted 7 lines · Andy Shevchenko

From: Heikki Krogerus <[email protected]>

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.

Quoted 4 lines
Quoted 1 line
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.

Quoted 4 lines
Quoted 2 lines

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.

Quoted 8 lines
Quoted 5 lines
---
 drivers/mfd/intel-lpss.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

Code looks okay though:

Thanks.

Signature · 2 lines
With Best Regards,
Andy Shevchenko
#4 lore raw
Quoted 10 lines · Andy Shevchenko
Quoted 9 lines · Lee Jones
Quoted 7 lines · Andy Shevchenko

From: Heikki Krogerus <[email protected]>

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.

Quoted 6 lines
Quoted 4 lines
Quoted 1 line
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 [email protected] # v4.{whatever}

.. tag instead.

Quoted 7 lines
Quoted 4 lines
Quoted 2 lines

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.

Quoted 10 lines
Quoted 7 lines
Quoted 4 lines
---
 drivers/mfd/intel-lpss.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

Code looks okay though:

Thanks.

Signature · 4 lines
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
#5 lore raw

Hi guys,

Quoted 4 lines · Lee Jones
Quoted 3 lines · Andy Shevchenko
Quoted 3 lines
Quoted 2 lines
Quoted 8 lines
Quoted 4 lines
Quoted 1 line

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,

Signature · 1 line
heikki
#6 lore raw
Quoted 12 lines · Heikki Krogerus

Hi guys,

Quoted 9 lines · Lee Jones
Quoted 7 lines · Andy Shevchenko
Quoted 4 lines
Quoted 2 lines

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.

Quoted 7 lines
Quoted 3 lines

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.

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