[PATCH] ARM: mach-shmobile: sh7372 A4R support V2

[PATCH] ARM: mach-shmobile: sh7372 A4R support V2

[Magnus Damm]

From: Magnus Damm <damm@opensource.se>

This patch is V2 of sh7372 A4R power domain support.

The sh7372 A4R hardware power domain contains the
SH CPU Core and a set of I/O devices including
multimedia accelerators and I2C controllers.

One special case about A4R is the INTCS interrupt
controller that needs to be saved and restored to
keep working as expected. Also the LCDC hardware
blocks are in a different hardware power domain
but have their IRQs routed only through INTCS. So
as long as LCDCs are active we cannot power down
INTCS because that would risk losing interrupts.

Signed-off-by: Magnus Damm <damm@opensource.se>
---

 Changes since V1:
 - rebased to fit on pm-test branch of linux-pm github repo
 - use simple power_down_ok() governor
 - let A4LC depend on A4R to support INTCS

 arch/arm/mach-shmobile/board-ap4evb.c        |    1 
 arch/arm/mach-shmobile/board-mackerel.c      |    1 
 arch/arm/mach-shmobile/include/mach/sh7372.h |    4 ++
 arch/arm/mach-shmobile/intc-sh7372.c         |   52 +++++++++++++++++++++++++-
 arch/arm/mach-shmobile/pm-sh7372.c           |   35 ++++++++++++++++-
 arch/arm/mach-shmobile/setup-sh7372.c        |    8 ++++
 6 files changed, 98 insertions(+), 3 deletions(-)

--- 0002/arch/arm/mach-shmobile/board-ap4evb.c
+++ work/arch/arm/mach-shmobile/board-ap4evb.c	2011-09-27 16:37:56.000000000 +0900
@@ -1412,6 +1412,7 @@ static void __init ap4evb_init(void)
 	sh7372_add_device_to_domain(&sh7372_a3sp, &sh_mmcif_device);
 	sh7372_add_device_to_domain(&sh7372_a3sp, &sdhi0_device);
 	sh7372_add_device_to_domain(&sh7372_a3sp, &sdhi1_device);
+	sh7372_add_device_to_domain(&sh7372_a4r, &ceu_device);
 
 	hdmi_init_pm_clock();
 	fsi_init_pm_clock();
--- 0002/arch/arm/mach-shmobile/board-mackerel.c
+++ work/arch/arm/mach-shmobile/board-mackerel.c	2011-09-27 16:37:56.000000000 +0900
@@ -1596,6 +1596,7 @@ static void __init mackerel_init(void)
 	sh7372_add_device_to_domain(&sh7372_a3sp, &sdhi1_device);
 #endif
 	sh7372_add_device_to_domain(&sh7372_a3sp, &sdhi2_device);
+	sh7372_add_device_to_domain(&sh7372_a4r, &ceu_device);
 
 	hdmi_init_pm_clock();
 	sh7372_pm_init();
--- 0002/arch/arm/mach-shmobile/include/mach/sh7372.h
+++ work/arch/arm/mach-shmobile/include/mach/sh7372.h	2011-09-27 16:37:56.000000000 +0900
@@ -491,6 +491,7 @@ static inline struct sh7372_pm_domain *t
 extern struct sh7372_pm_domain sh7372_a4lc;
 extern struct sh7372_pm_domain sh7372_a4mp;
 extern struct sh7372_pm_domain sh7372_d4;
+extern struct sh7372_pm_domain sh7372_a4r;
 extern struct sh7372_pm_domain sh7372_a3rv;
 extern struct sh7372_pm_domain sh7372_a3ri;
 extern struct sh7372_pm_domain sh7372_a3sp;
@@ -507,4 +508,7 @@ extern void sh7372_pm_add_subdomain(stru
 #define sh7372_pm_add_subdomain(pd, sd) do { } while(0)
 #endif /* CONFIG_PM */
 
+extern void sh7372_intcs_suspend(void);
+extern void sh7372_intcs_resume(void);
+
 #endif /* __ASM_SH7372_H__ */
--- 0001/arch/arm/mach-shmobile/intc-sh7372.c
+++ work/arch/arm/mach-shmobile/intc-sh7372.c	2011-09-27 16:37:56.000000000 +0900
@@ -606,9 +606,16 @@ static void intcs_demux(unsigned int irq
 	generic_handle_irq(intcs_evt2irq(evtcodeas));
 }
 
+static void __iomem *intcs_ffd2;
+static void __iomem *intcs_ffd5;
+
 void __init sh7372_init_irq(void)
 {
-	void __iomem *intevtsa = ioremap_nocache(0xffd20100, PAGE_SIZE);
+	void __iomem *intevtsa;
+
+	intcs_ffd2 = ioremap_nocache(0xffd20000, PAGE_SIZE);
+	intevtsa = intcs_ffd2 + 0x100;
+	intcs_ffd5 = ioremap_nocache(0xffd50000, PAGE_SIZE);
 
 	register_intc_controller(&intca_desc);
 	register_intc_controller(&intcs_desc);
@@ -617,3 +624,46 @@ void __init sh7372_init_irq(void)
 	irq_set_handler_data(evt2irq(0xf80), (void *)intevtsa);
 	irq_set_chained_handler(evt2irq(0xf80), intcs_demux);
 }
+
+static unsigned short ffd2[0x200];
+static unsigned short ffd5[0x100];
+
+void sh7372_intcs_suspend(void)
+{
+	int k;
+
+	for (k = 0x00; k <= 0x30; k += 4)
+		ffd2[k] = __raw_readw(intcs_ffd2 + k);
+
+	for (k = 0x80; k <= 0xb0; k += 4)
+		ffd2[k] = __raw_readb(intcs_ffd2 + k);
+
+	for (k = 0x180; k <= 0x188; k += 4)
+		ffd2[k] = __raw_readb(intcs_ffd2 + k);
+
+	for (k = 0x00; k <= 0x3c; k += 4)
+		ffd5[k] = __raw_readw(intcs_ffd5 + k);
+
+	for (k = 0x80; k <= 0x9c; k += 4)
+		ffd5[k] = __raw_readb(intcs_ffd5 + k);
+}
+
+void sh7372_intcs_resume(void)
+{
+	int k;
+
+	for (k = 0x00; k <= 0x30; k += 4)
+		__raw_writew(ffd2[k], intcs_ffd2 + k);
+
+	for (k = 0x80; k <= 0xb0; k += 4)
+		__raw_writeb(ffd2[k], intcs_ffd2 + k);
+
+	for (k = 0x180; k <= 0x188; k += 4)
+		__raw_writeb(ffd2[k], intcs_ffd2 + k);
+
+	for (k = 0x00; k <= 0x3c; k += 4)
+		__raw_writew(ffd5[k], intcs_ffd5 + k);
+
+	for (k = 0x80; k <= 0x9c; k += 4)
+		__raw_writeb(ffd5[k], intcs_ffd5 + k);
+}
--- 0002/arch/arm/mach-shmobile/pm-sh7372.c
+++ work/arch/arm/mach-shmobile/pm-sh7372.c	2011-09-27 16:59:26.000000000 +0900
@@ -44,6 +44,7 @@
 #define SPDCR 0xe6180008
 #define SWUCR 0xe6180014
 #define SBAR 0xe6180020
+#define WUPRMSK 0xe6180028
 #define WUPSMSK 0xe618002c
 #define WUPSMSK2 0xe6180048
 #define PSTR 0xe6180080
@@ -132,6 +133,24 @@ static int pd_power_up(struct generic_pm
 	return ret;
 }
 
+static int pd_power_up_a4r(struct generic_pm_domain *genpd)
+{
+	int ret = pd_power_up(genpd);
+
+	if (ret = 0)
+		sh7372_intcs_resume();
+
+	return ret;
+}
+
+static int pd_power_down_a4r(struct generic_pm_domain *genpd)
+{
+	sh7372_intcs_suspend();
+	__raw_writel(0x300fffff, WUPRMSK); /* avoid wakeup */
+
+	return pd_power_down(genpd);
+}
+
 static bool pd_active_wakeup(struct device *dev)
 {
 	return true;
@@ -150,6 +169,9 @@ static bool sh7372_power_down_ok(struct
 	if (genpd = &sh7372_a3sp.genpd)
 		return false;
 
+	if (genpd = &sh7372_a4r.genpd)
+		return false;
+
 	return true;
 }
 
@@ -166,8 +188,13 @@ void sh7372_init_pm_domain(struct sh7372
 	genpd->start_device = pm_clk_resume;
 	genpd->dev_irq_safe = true;
 	genpd->active_wakeup = pd_active_wakeup;
-	genpd->power_off = pd_power_down;
-	genpd->power_on = pd_power_up;
+	if (genpd = &sh7372_a4r.genpd) {
+		genpd->power_off = pd_power_down_a4r;
+		genpd->power_on = pd_power_up_a4r;
+	} else {
+		genpd->power_off = pd_power_down;
+		genpd->power_on = pd_power_up;
+	}
 	genpd->power_on(&sh7372_pd->genpd);
 }
 
@@ -199,6 +226,10 @@ struct sh7372_pm_domain sh7372_d4 = {
 	.bit_shift = 3,
 };
 
+struct sh7372_pm_domain sh7372_a4r = {
+	.bit_shift = 5,
+};
+
 struct sh7372_pm_domain sh7372_a3rv = {
 	.bit_shift = 6,
 };
--- 0002/arch/arm/mach-shmobile/setup-sh7372.c
+++ work/arch/arm/mach-shmobile/setup-sh7372.c	2011-09-27 16:45:56.000000000 +0900
@@ -991,12 +991,14 @@ void __init sh7372_add_standard_devices(
 	sh7372_init_pm_domain(&sh7372_a4lc);
 	sh7372_init_pm_domain(&sh7372_a4mp);
 	sh7372_init_pm_domain(&sh7372_d4);
+	sh7372_init_pm_domain(&sh7372_a4r);
 	sh7372_init_pm_domain(&sh7372_a3rv);
 	sh7372_init_pm_domain(&sh7372_a3ri);
 	sh7372_init_pm_domain(&sh7372_a3sg);
 	sh7372_init_pm_domain(&sh7372_a3sp);
 
 	sh7372_pm_add_subdomain(&sh7372_a4lc, &sh7372_a3rv);
+	sh7372_pm_add_subdomain(&sh7372_a4r, &sh7372_a4lc);
 
 	platform_add_devices(sh7372_early_devices,
 			    ARRAY_SIZE(sh7372_early_devices));
@@ -1020,6 +1022,12 @@ void __init sh7372_add_standard_devices(
 	sh7372_add_device_to_domain(&sh7372_a3sp, &dma2_device);
 	sh7372_add_device_to_domain(&sh7372_a3sp, &usb_dma0_device);
 	sh7372_add_device_to_domain(&sh7372_a3sp, &usb_dma1_device);
+	sh7372_add_device_to_domain(&sh7372_a4r, &iic0_device);
+	sh7372_add_device_to_domain(&sh7372_a4r, &veu0_device);
+	sh7372_add_device_to_domain(&sh7372_a4r, &veu1_device);
+	sh7372_add_device_to_domain(&sh7372_a4r, &veu2_device);
+	sh7372_add_device_to_domain(&sh7372_a4r, &veu3_device);
+	sh7372_add_device_to_domain(&sh7372_a4r, &jpu_device);
 }
 
 void __init sh7372_add_early_devices(void)

Re: [PATCH] ARM: mach-shmobile: sh7372 A4R support V2

[Rafael J. Wysocki]

On Tuesday, September 27, 2011, Magnus Damm wrote:
> From: Magnus Damm <damm@opensource.se>
> 
> This patch is V2 of sh7372 A4R power domain support.
> 
> The sh7372 A4R hardware power domain contains the
> SH CPU Core and a set of I/O devices including
> multimedia accelerators and I2C controllers.
> 
> One special case about A4R is the INTCS interrupt
> controller that needs to be saved and restored to
> keep working as expected. Also the LCDC hardware
> blocks are in a different hardware power domain
> but have their IRQs routed only through INTCS. So
> as long as LCDCs are active we cannot power down
> INTCS because that would risk losing interrupts.
> 
> Signed-off-by: Magnus Damm <damm@opensource.se>

Unfortunately, the original patch breaks resume from system suspend,
because it causes the A4R domain be turned off during the _noirq
phase of device suspend.  The domain stays off during syscore_ops
resume, so intc_resume() crashes, because it can't access the
device.  To address this problem it is sufficient to prevent the
domain from being turned off, which is done in my version of the
patch below (on top of my version of the A3SP patch sent a while
ago: http://article.gmane.org/gmane.linux.ports.sh.devel/12565).

Thanks,
Rafael

---
From: Magnus Damm <damm@opensource.se>
Subject: ARM: mach-shmobile: sh7372 A4R support

This change adds support for the sh7372 A4R power domain.

The sh7372 A4R hardware power domain contains the
SH CPU Core and a set of I/O devices including
multimedia accelerators and I2C controllers.

One special case about A4R is the INTCS interrupt
controller that needs to be saved and restored to
keep working as expected. Also the LCDC hardware
blocks are in a different hardware power domain
but have their IRQs routed only through INTCS. So
as long as LCDCs are active we cannot power down
INTCS because that would risk losing interrupts.

Signed-off-by: Magnus Damm <damm@opensource.se>
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 arch/arm/mach-shmobile/board-ap4evb.c        |    1 
 arch/arm/mach-shmobile/board-mackerel.c      |    1 
 arch/arm/mach-shmobile/include/mach/sh7372.h |    7 +++
 arch/arm/mach-shmobile/intc-sh7372.c         |   52 ++++++++++++++++++++++++++-
 arch/arm/mach-shmobile/pm-sh7372.c           |   29 ++++++++++++++-
 arch/arm/mach-shmobile/setup-sh7372.c        |    8 ++++
 6 files changed, 96 insertions(+), 2 deletions(-)

Index: linux/arch/arm/mach-shmobile/board-ap4evb.c
=================================--- linux.orig/arch/arm/mach-shmobile/board-ap4evb.c
+++ linux/arch/arm/mach-shmobile/board-ap4evb.c
@@ -1412,6 +1412,7 @@ static void __init ap4evb_init(void)
 	sh7372_add_device_to_domain(&sh7372_a3sp, &sh_mmcif_device);
 	sh7372_add_device_to_domain(&sh7372_a3sp, &sdhi0_device);
 	sh7372_add_device_to_domain(&sh7372_a3sp, &sdhi1_device);
+	sh7372_add_device_to_domain(&sh7372_a4r, &ceu_device);
 
 	hdmi_init_pm_clock();
 	fsi_init_pm_clock();
Index: linux/arch/arm/mach-shmobile/board-mackerel.c
=================================--- linux.orig/arch/arm/mach-shmobile/board-mackerel.c
+++ linux/arch/arm/mach-shmobile/board-mackerel.c
@@ -1596,6 +1596,7 @@ static void __init mackerel_init(void)
 	sh7372_add_device_to_domain(&sh7372_a3sp, &sdhi1_device);
 #endif
 	sh7372_add_device_to_domain(&sh7372_a3sp, &sdhi2_device);
+	sh7372_add_device_to_domain(&sh7372_a4r, &ceu_device);
 
 	hdmi_init_pm_clock();
 	sh7372_pm_init();
Index: linux/arch/arm/mach-shmobile/include/mach/sh7372.h
=================================--- linux.orig/arch/arm/mach-shmobile/include/mach/sh7372.h
+++ linux/arch/arm/mach-shmobile/include/mach/sh7372.h
@@ -480,8 +480,11 @@ struct platform_device;
 struct sh7372_pm_domain {
 	struct generic_pm_domain genpd;
 	struct dev_power_governor *gov;
+	void (*suspend)(void);
+	void (*resume)(void);
 	unsigned int bit_shift;
 	bool no_debug;
+	bool stay_on;
 };
 
 static inline struct sh7372_pm_domain *to_sh7372_pd(struct generic_pm_domain *d)
@@ -493,6 +496,7 @@ static inline struct sh7372_pm_domain *t
 extern struct sh7372_pm_domain sh7372_a4lc;
 extern struct sh7372_pm_domain sh7372_a4mp;
 extern struct sh7372_pm_domain sh7372_d4;
+extern struct sh7372_pm_domain sh7372_a4r;
 extern struct sh7372_pm_domain sh7372_a3rv;
 extern struct sh7372_pm_domain sh7372_a3ri;
 extern struct sh7372_pm_domain sh7372_a3sp;
@@ -512,4 +516,7 @@ extern void sh7372_pm_add_subdomain(stru
 #define sh7372_pm_add_subdomain(pd, sd) do { } while(0)
 #endif /* CONFIG_PM */
 
+extern void sh7372_intcs_suspend(void);
+extern void sh7372_intcs_resume(void);
+
 #endif /* __ASM_SH7372_H__ */
Index: linux/arch/arm/mach-shmobile/intc-sh7372.c
=================================--- linux.orig/arch/arm/mach-shmobile/intc-sh7372.c
+++ linux/arch/arm/mach-shmobile/intc-sh7372.c
@@ -606,9 +606,16 @@ static void intcs_demux(unsigned int irq
 	generic_handle_irq(intcs_evt2irq(evtcodeas));
 }
 
+static void __iomem *intcs_ffd2;
+static void __iomem *intcs_ffd5;
+
 void __init sh7372_init_irq(void)
 {
-	void __iomem *intevtsa = ioremap_nocache(0xffd20100, PAGE_SIZE);
+	void __iomem *intevtsa;
+
+	intcs_ffd2 = ioremap_nocache(0xffd20000, PAGE_SIZE);
+	intevtsa = intcs_ffd2 + 0x100;
+	intcs_ffd5 = ioremap_nocache(0xffd50000, PAGE_SIZE);
 
 	register_intc_controller(&intca_desc);
 	register_intc_controller(&intcs_desc);
@@ -617,3 +624,46 @@ void __init sh7372_init_irq(void)
 	irq_set_handler_data(evt2irq(0xf80), (void *)intevtsa);
 	irq_set_chained_handler(evt2irq(0xf80), intcs_demux);
 }
+
+static unsigned short ffd2[0x200];
+static unsigned short ffd5[0x100];
+
+void sh7372_intcs_suspend(void)
+{
+	int k;
+
+	for (k = 0x00; k <= 0x30; k += 4)
+		ffd2[k] = __raw_readw(intcs_ffd2 + k);
+
+	for (k = 0x80; k <= 0xb0; k += 4)
+		ffd2[k] = __raw_readb(intcs_ffd2 + k);
+
+	for (k = 0x180; k <= 0x188; k += 4)
+		ffd2[k] = __raw_readb(intcs_ffd2 + k);
+
+	for (k = 0x00; k <= 0x3c; k += 4)
+		ffd5[k] = __raw_readw(intcs_ffd5 + k);
+
+	for (k = 0x80; k <= 0x9c; k += 4)
+		ffd5[k] = __raw_readb(intcs_ffd5 + k);
+}
+
+void sh7372_intcs_resume(void)
+{
+	int k;
+
+	for (k = 0x00; k <= 0x30; k += 4)
+		__raw_writew(ffd2[k], intcs_ffd2 + k);
+
+	for (k = 0x80; k <= 0xb0; k += 4)
+		__raw_writeb(ffd2[k], intcs_ffd2 + k);
+
+	for (k = 0x180; k <= 0x188; k += 4)
+		__raw_writeb(ffd2[k], intcs_ffd2 + k);
+
+	for (k = 0x00; k <= 0x3c; k += 4)
+		__raw_writew(ffd5[k], intcs_ffd5 + k);
+
+	for (k = 0x80; k <= 0x9c; k += 4)
+		__raw_writeb(ffd5[k], intcs_ffd5 + k);
+}
Index: linux/arch/arm/mach-shmobile/pm-sh7372.c
=================================--- linux.orig/arch/arm/mach-shmobile/pm-sh7372.c
+++ linux/arch/arm/mach-shmobile/pm-sh7372.c
@@ -44,6 +44,7 @@
 #define SPDCR 0xe6180008
 #define SWUCR 0xe6180014
 #define SBAR 0xe6180020
+#define WUPRMSK 0xe6180028
 #define WUPSMSK 0xe618002c
 #define WUPSMSK2 0xe6180048
 #define PSTR 0xe6180080
@@ -80,6 +81,12 @@ static int pd_power_down(struct generic_
 	struct sh7372_pm_domain *sh7372_pd = to_sh7372_pd(genpd);
 	unsigned int mask = 1 << sh7372_pd->bit_shift;
 
+	if (sh7372_pd->suspend)
+		sh7372_pd->suspend();
+
+	if (sh7372_pd->stay_on)
+		return 0;
+
 	if (__raw_readl(PSTR) & mask) {
 		unsigned int retry_count;
 
@@ -106,6 +113,9 @@ static int pd_power_up(struct generic_pm
 	unsigned int retry_count;
 	int ret = 0;
 
+	if (sh7372_pd->stay_on)
+		goto out;
+
 	if (__raw_readl(PSTR) & mask)
 		goto out;
 
@@ -122,14 +132,23 @@ static int pd_power_up(struct generic_pm
 	if (__raw_readl(SWUCR) & mask)
 		ret = -EIO;
 
- out:
 	if (!sh7372_pd->no_debug)
 		pr_debug("sh7372 power domain up 0x%08x -> PSTR = 0x%08x\n",
 			 mask, __raw_readl(PSTR));
 
+ out:
+	if (ret = 0 && sh7372_pd->resume)
+		sh7372_pd->resume();
+
 	return ret;
 }
 
+static void sh7372_a4r_suspend(void)
+{
+	sh7372_intcs_suspend();
+	__raw_writel(0x300fffff, WUPRMSK); /* avoid wakeup */
+}
+
 static bool pd_active_wakeup(struct device *dev)
 {
 	return true;
@@ -214,6 +233,14 @@ struct sh7372_pm_domain sh7372_d4 = {
 	.bit_shift = 3,
 };
 
+struct sh7372_pm_domain sh7372_a4r = {
+	.bit_shift = 5,
+	.gov = &sh7372_always_on_gov,
+	.suspend = sh7372_a4r_suspend,
+	.resume = sh7372_intcs_resume,
+	.stay_on = true,
+};
+
 struct sh7372_pm_domain sh7372_a3rv = {
 	.bit_shift = 6,
 };
Index: linux/arch/arm/mach-shmobile/setup-sh7372.c
=================================--- linux.orig/arch/arm/mach-shmobile/setup-sh7372.c
+++ linux/arch/arm/mach-shmobile/setup-sh7372.c
@@ -991,12 +991,14 @@ void __init sh7372_add_standard_devices(
 	sh7372_init_pm_domain(&sh7372_a4lc);
 	sh7372_init_pm_domain(&sh7372_a4mp);
 	sh7372_init_pm_domain(&sh7372_d4);
+	sh7372_init_pm_domain(&sh7372_a4r);
 	sh7372_init_pm_domain(&sh7372_a3rv);
 	sh7372_init_pm_domain(&sh7372_a3ri);
 	sh7372_init_pm_domain(&sh7372_a3sg);
 	sh7372_init_pm_domain(&sh7372_a3sp);
 
 	sh7372_pm_add_subdomain(&sh7372_a4lc, &sh7372_a3rv);
+	sh7372_pm_add_subdomain(&sh7372_a4r, &sh7372_a4lc);
 
 	platform_add_devices(sh7372_early_devices,
 			    ARRAY_SIZE(sh7372_early_devices));
@@ -1020,6 +1022,12 @@ void __init sh7372_add_standard_devices(
 	sh7372_add_special_dev_to_domain(&sh7372_a3sp, &dma2_device);
 	sh7372_add_special_dev_to_domain(&sh7372_a3sp, &usb_dma0_device);
 	sh7372_add_special_dev_to_domain(&sh7372_a3sp, &usb_dma1_device);
+	sh7372_add_device_to_domain(&sh7372_a4r, &iic0_device);
+	sh7372_add_device_to_domain(&sh7372_a4r, &veu0_device);
+	sh7372_add_device_to_domain(&sh7372_a4r, &veu1_device);
+	sh7372_add_device_to_domain(&sh7372_a4r, &veu2_device);
+	sh7372_add_device_to_domain(&sh7372_a4r, &veu3_device);
+	sh7372_add_device_to_domain(&sh7372_a4r, &jpu_device);
 }
 
 void __init sh7372_add_early_devices(void)

Re: [PATCH] ARM: mach-shmobile: sh7372 A4R suspend/resume order fix

[Rafael J. Wysocki]

On Wednesday, November 09, 2011, Magnus Damm wrote:
> From: Magnus Damm <damm@opensource.se>
> 
> Update the sh7372 A4R code to make sure the suspend
> callback gets to be called before the resume callback.
> 
> Without this fix the A4R resume callback is called before
> suspend. The resume callback restores the INTCS hardware
> registers with incorrect data which results in interrupts
> being masked when they shouldn't be. The user will notice
> this issue as IIC0 timing out during boot.
> 
> Reported-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> Signed-off-by: Magnus Damm <damm@opensource.se>

Well, I'd prefer to do something like the patch below instead.

Please let me know what you think.

Thanks,
Rafael

---
From: Rafael J. Wysocki <rjw@sisk.pl>
Subject: PM / shmobile: Avoid restoring the INTCS state during initialization

The SH7372 PM domain initialization routine calls pd_power_up()
that executes the domain's .resume() callback, if present, and for
the A4R domain this callback attepmts to restore the INTCS state from
uninitialized data.  To avoid that, introduce __pd_power_up() that
will only execute the domain's .resume() callback if its second
argument is 'true' and make the SH7372 PM domain initialization
use it with 'false' as its second argument.  Redefine pd_power_up()
as a wrapper around __pd_power_up().

Reported-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 arch/arm/mach-shmobile/pm-sh7372.c |   12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

Index: linux/arch/arm/mach-shmobile/pm-sh7372.c
=================================--- linux.orig/arch/arm/mach-shmobile/pm-sh7372.c
+++ linux/arch/arm/mach-shmobile/pm-sh7372.c
@@ -107,9 +107,8 @@ static int pd_power_down(struct generic_
 	return 0;
 }
 
-static int pd_power_up(struct generic_pm_domain *genpd)
+static int __pd_power_up(struct sh7372_pm_domain *sh7372_pd, bool do_resume)
 {
-	struct sh7372_pm_domain *sh7372_pd = to_sh7372_pd(genpd);
 	unsigned int mask = 1 << sh7372_pd->bit_shift;
 	unsigned int retry_count;
 	int ret = 0;
@@ -138,12 +137,17 @@ static int pd_power_up(struct generic_pm
 			 mask, __raw_readl(PSTR));
 
  out:
-	if (ret = 0 && sh7372_pd->resume)
+	if (ret = 0 && sh7372_pd->resume && do_resume)
 		sh7372_pd->resume();
 
 	return ret;
 }
 
+static int pd_power_up(struct generic_pm_domain *genpd)
+{
+	 return __pd_power_up(to_sh7372_pd(genpd), true);
+}
+
 static void sh7372_a4r_suspend(void)
 {
 	sh7372_intcs_suspend();
@@ -209,7 +213,7 @@ void sh7372_init_pm_domain(struct sh7372
 	genpd->dev_irq_safe = true;
 	genpd->power_off = pd_power_down;
 	genpd->power_on = pd_power_up;
-	genpd->power_on(&sh7372_pd->genpd);
+	__pd_power_up(sh7372_pd, false);
 }
 
 void sh7372_add_device_to_domain(struct sh7372_pm_domain *sh7372_pd,

Re: [PATCH] ARM: mach-shmobile: sh7372 A4R suspend/resume order fix

[Magnus Damm]

> From: Rafael J. Wysocki <rjw@sisk.pl>
> Subject: PM / shmobile: Avoid restoring the INTCS state during initialization
>
> The SH7372 PM domain initialization routine calls pd_power_up()
> that executes the domain's .resume() callback, if present, and for
> the A4R domain this callback attepmts to restore the INTCS state from
> uninitialized data.  To avoid that, introduce __pd_power_up() that
> will only execute the domain's .resume() callback if its second
> argument is 'true' and make the SH7372 PM domain initialization
> use it with 'false' as its second argument.  Redefine pd_power_up()
> as a wrapper around __pd_power_up().
>
> Reported-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>

Looks fine to me, from my point of view your implementation is better
than the other alternatives.

It is correct that Guennadi reported this issue, and the Signed-off
clearly shows that this specific implementation is coming from you. I
suppose there are more important things in life than git attribution,
but I do feel that I did spend a significant amount of time tracking
down this issue, writing the initial patch and then also communicating
with you guys. I may be wrong, but I suspect that if didn't do this
then this issue wouldn't have been fixed.

Unfortunately there does not seem to be any tag for that kind of work,
but if it existed then I would prefer to add this in between you and
Guennadi:

Tracked-down-by: Magnus Damm <damm@opensource.se>

Cheers,

/ magnus

Re: [PATCH] ARM: mach-shmobile: sh7372 A4R suspend/resume order fix

[Guennadi Liakhovetski]

On Fri, 11 Nov 2011, Magnus Damm wrote:

> > From: Rafael J. Wysocki <rjw@sisk.pl>
> > Subject: PM / shmobile: Avoid restoring the INTCS state during initialization
> >
> > The SH7372 PM domain initialization routine calls pd_power_up()
> > that executes the domain's .resume() callback, if present, and for
> > the A4R domain this callback attepmts to restore the INTCS state from
> > uninitialized data.  To avoid that, introduce __pd_power_up() that
> > will only execute the domain's .resume() callback if its second
> > argument is 'true' and make the SH7372 PM domain initialization
> > use it with 'false' as its second argument.  Redefine pd_power_up()
> > as a wrapper around __pd_power_up().
> >
> > Reported-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> 
> Looks fine to me, from my point of view your implementation is better
> than the other alternatives.
> 
> It is correct that Guennadi reported this issue, and the Signed-off
> clearly shows that this specific implementation is coming from you. I
> suppose there are more important things in life than git attribution,
> but I do feel that I did spend a significant amount of time tracking
> down this issue, writing the initial patch and then also communicating
> with you guys. I may be wrong, but I suspect that if didn't do this
> then this issue wouldn't have been fixed.
> 
> Unfortunately there does not seem to be any tag for that kind of work,
> but if it existed then I would prefer to add this in between you and
> Guennadi:
> 
> Tracked-down-by: Magnus Damm <damm@opensource.se>

Yes, I also was looking for a suitable attribution to mention you in my 
version of the patch, and ended up just saying "thanks." But, I think, 
we're allowed to be creative here, we're not prohibited to create our own 
tags - whether or not they're going to be standardised. Maybe we can put 
it even stronger to emphasise, that you not only tracked the problem down, 
but also proposed the first fix. Something like

Initial-fix-by:
or
Original-fix-by:

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH] ARM: mach-shmobile: sh7372 A4R suspend/resume order fix

[Rafael J. Wysocki]

On Friday, November 11, 2011, Magnus Damm wrote:
> > From: Rafael J. Wysocki <rjw@sisk.pl>
> > Subject: PM / shmobile: Avoid restoring the INTCS state during initialization
> >
> > The SH7372 PM domain initialization routine calls pd_power_up()
> > that executes the domain's .resume() callback, if present, and for
> > the A4R domain this callback attepmts to restore the INTCS state from
> > uninitialized data.  To avoid that, introduce __pd_power_up() that
> > will only execute the domain's .resume() callback if its second
> > argument is 'true' and make the SH7372 PM domain initialization
> > use it with 'false' as its second argument.  Redefine pd_power_up()
> > as a wrapper around __pd_power_up().
> >
> > Reported-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> 
> Looks fine to me, from my point of view your implementation is better
> than the other alternatives.
> 
> It is correct that Guennadi reported this issue, and the Signed-off
> clearly shows that this specific implementation is coming from you. I
> suppose there are more important things in life than git attribution,
> but I do feel that I did spend a significant amount of time tracking
> down this issue, writing the initial patch and then also communicating
> with you guys. I may be wrong, but I suspect that if didn't do this
> then this issue wouldn't have been fixed.
> 
> Unfortunately there does not seem to be any tag for that kind of work,
> but if it existed then I would prefer to add this in between you and
> Guennadi:
> 
> Tracked-down-by: Magnus Damm <damm@opensource.se>

OK

I'll add your ack too, if you don't mind. :-)

Thanks,
Rafael