[RFC][PATCH 0/4] PM / shmobile: Use PM QoS latency constraints in touchscreen driver

[RFC][PATCH 0/4] PM / shmobile: Use PM QoS latency constraints in touchscreen driver

[Rafael J. Wysocki]

Hi,

The following series of patches updates the SH7372 PM domain's so that
A4R doesn't need to be turned on during system sleep, drops the SH7372's
PM domains stay_on flag and makes the touchscreen add a PM QoS constraint
on its I2C controller to prevent the A4R domain containing it from being
turned off (due to power management) when the touchscreen is active.

Comments welcome!

Thanks,
Rafael

[RFC][PATCH 1/4] PM / shmobile: Don't include SH7372's INTCS in syscore suspend/resume

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

Since the SH7372's INTCS in included into syscore suspend/resume,
which causes the chip to be accessed when PM domains have been
turned off during system suspend, the A4R domain containing the
INTCS has to stay on during system sleep, which is suboptimal
from the power consumption point of view.

For this reason, add a new INTC flag, skip_syscore_suspend, to mark
the INTCS for intc_suspend() and intc_resume(), so that they don't
touch it.  This allows the A4R domain to be turned off during
system suspend and the INTCS state is resrored during system
resume by the A4R's "power on" code.

Suggested-by: Magnus Damm <damm@opensource.se>
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 arch/arm/mach-shmobile/intc-sh7372.c |    1 +
 drivers/sh/intc/core.c               |    8 ++++++++
 drivers/sh/intc/internals.h          |    1 +
 include/linux/sh_intc.h              |    1 +
 4 files changed, 11 insertions(+)

Index: linux/include/linux/sh_intc.h
=================================--- linux.orig/include/linux/sh_intc.h
+++ linux/include/linux/sh_intc.h
@@ -95,6 +95,7 @@ struct intc_desc {
 	unsigned int num_resources;
 	intc_enum force_enable;
 	intc_enum force_disable;
+	bool skip_syscore_suspend;
 	struct intc_hw_desc hw;
 };
 
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
@@ -535,6 +535,7 @@ static struct resource intcs_resources[]
 static struct intc_desc intcs_desc __initdata = {
 	.name = "sh7372-intcs",
 	.force_enable = ENABLED_INTCS,
+	.skip_syscore_suspend = true,
 	.resource = intcs_resources,
 	.num_resources = ARRAY_SIZE(intcs_resources),
 	.hw = INTC_HW_DESC(intcs_vectors, intcs_groups, intcs_mask_registers,
Index: linux/drivers/sh/intc/core.c
=================================--- linux.orig/drivers/sh/intc/core.c
+++ linux/drivers/sh/intc/core.c
@@ -354,6 +354,8 @@ int __init register_intc_controller(stru
 	if (desc->force_enable)
 		intc_enable_disable_enum(desc, d, desc->force_enable, 1);
 
+	d->skip_suspend = desc->skip_syscore_suspend;
+
 	nr_intc_controllers++;
 
 	return 0;
@@ -386,6 +388,9 @@ static int intc_suspend(void)
 	list_for_each_entry(d, &intc_list, list) {
 		int irq;
 
+		if (d->skip_suspend)
+			continue;
+
 		/* enable wakeup irqs belonging to this intc controller */
 		for_each_active_irq(irq) {
 			struct irq_data *data;
@@ -409,6 +414,9 @@ static void intc_resume(void)
 	list_for_each_entry(d, &intc_list, list) {
 		int irq;
 
+		if (d->skip_suspend)
+			continue;
+
 		for_each_active_irq(irq) {
 			struct irq_data *data;
 			struct irq_chip *chip;
Index: linux/drivers/sh/intc/internals.h
=================================--- linux.orig/drivers/sh/intc/internals.h
+++ linux/drivers/sh/intc/internals.h
@@ -67,6 +67,7 @@ struct intc_desc_int {
 	struct intc_window *window;
 	unsigned int nr_windows;
 	struct irq_chip chip;
+	bool skip_suspend;
 };
 
 

[RFC][PATCH 2/4] PM / shmobile: Remove the stay_on flag from SH7372's PM domains

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

SH7372 uses two independent mechanisms for ensuring that power
domains will never be turned off: the stay_on flag and the "always
on" domain governor.  Moreover, the "always on" governor is only taken
into accout by runtime PM code paths, while the stay_on flag affects
all attempts to turn the given domain off.  Thus setting the stay_on
flag causes the "always on" governor to be unnecessary, which is
quite confusing.

However, the stay_on flag is currently only set for the A3SP PM
domain and only if console_suspend_enabled, so it may be replaced
by checking console_suspend_enabled directly in the A3SP's
.suspend() routine.  [This requires domain .suspend() to return
a result, but that is a minor modification.]

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 arch/arm/mach-shmobile/include/mach/sh7372.h |    3 --
 arch/arm/mach-shmobile/pm-sh7372.c           |   36 ++++++++++++---------------
 2 files changed, 18 insertions(+), 21 deletions(-)

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,11 +480,10 @@ struct platform_device;
 struct sh7372_pm_domain {
 	struct generic_pm_domain genpd;
 	struct dev_power_governor *gov;
-	void (*suspend)(void);
+	int (*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)
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
@@ -82,11 +82,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->suspend) {
+		int ret = sh7372_pd->suspend();
 
-	if (sh7372_pd->stay_on)
-		return 0;
+		if (ret)
+			return ret;
+	}
 
 	if (__raw_readl(PSTR) & mask) {
 		unsigned int retry_count;
@@ -113,9 +114,6 @@ static int __pd_power_up(struct sh7372_p
 	unsigned int retry_count;
 	int ret = 0;
 
-	if (sh7372_pd->stay_on)
-		goto out;
-
 	if (__raw_readl(PSTR) & mask)
 		goto out;
 
@@ -148,10 +146,11 @@ static int pd_power_up(struct generic_pm
 	 return __pd_power_up(to_sh7372_pd(genpd), true);
 }
 
-static void sh7372_a4r_suspend(void)
+static int sh7372_a4r_suspend(void)
 {
 	sh7372_intcs_suspend();
 	__raw_writel(0x300fffff, WUPRMSK); /* avoid wakeup */
+	return 0;
 }
 
 static bool pd_active_wakeup(struct device *dev)
@@ -243,7 +242,6 @@ struct sh7372_pm_domain sh7372_a4r = {
 	.gov = &pm_domain_always_on_gov,
 	.suspend = sh7372_a4r_suspend,
 	.resume = sh7372_intcs_resume,
-	.stay_on = true,
 };
 
 struct sh7372_pm_domain sh7372_a3rv = {
@@ -256,21 +254,23 @@ struct sh7372_pm_domain sh7372_a3ri = {
 	.bit_shift = 8,
 };
 
+static int sh7372_a3sp_suspend(void)
+{
+	/*
+	 * Serial consoles make use of SCIF hardware located in A3SP,
+	 * keep such power domain on if "no_console_suspend" is set.
+	 */
+	return console_suspend_enabled ? -EBUSY : 0;
+}
+
 struct sh7372_pm_domain sh7372_a3sp = {
 	.genpd.name = "A3SP",
 	.bit_shift = 11,
 	.gov = &pm_domain_always_on_gov,
 	.no_debug = true,
+	.suspend = sh7372_a3sp_suspend,
 };
 
-static void sh7372_a3sp_init(void)
-{
-	/* serial consoles make use of SCIF hardware located in A3SP,
-	 * keep such power domain on if "no_console_suspend" is set.
-	 */
-	sh7372_a3sp.stay_on = !console_suspend_enabled;
-}
-
 struct sh7372_pm_domain sh7372_a3sg = {
 	.genpd.name = "A3SG",
 	.bit_shift = 13,
@@ -514,8 +514,6 @@ void __init sh7372_pm_init(void)
 	/* do not convert A3SM, A3SP, A3SG, A4R power down into A4S */
 	__raw_writel(0, PDNSEL);
 
-	sh7372_a3sp_init();
-
 	sh7372_suspend_init();
 	sh7372_cpuidle_init();
 }

[RFC][PATCH 3/4] PM / QoS: Introduce dev_pm_qos_add_ancestor_request()

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

Some devices, like the I2C controller on SH7372, are not
necessary for providing power to their children or forwarding
wakeup signals (and generally interrupts) from them.  They are
only needed by their children when there's some data to transfer,
so they may be suspended for the majority of time and resumed
on demand, when the children have data to send or receive.  For this
purpose, however, their power.ignore_children flags have to be set,
or the PM core wouldn't allow them to be suspended while their
children were active.

Unfortunately, in some situations it may take too much time to
resume such devices so that they can assist their children in
transferring data.  For example, if such a device belongs to a PM
domain which goes to the "power off" state when that device is
suspended, it may take too much time to restore power to the
domain in response to the request from one of the device's
children.  In that case, if the parent's resume time is critical,
the domain should stay in the "power on" state, although it still may
be desirable to power manage the parent itself (e.g. by manipulating
its clock).

In general, device PM QoS may be used to address this problem.
Namely, if the device's children added PM QoS latency constraints
for it, they would be able to prevent it from being put into an
overly deep low-power state.  However, in some cases the devices
needing to be serviced are not the immediate children of a
"children-ignoring" device, but its grandchildren or even less
direct descendants.  In those cases, the entity wanting to add a
PM QoS request for a given device's ancestor that ignores its
children will have to find it in the first place, so introduce a new
helper function that may be used to achieve that.  This function,
dev_pm_qos_add_ancestor_request(), will search for the first
ancestor of the given device whose power.ignore_children flag is
set and will add a device PM QoS latency request for that ancestor
on behalf of the caller.  The request added this way may be removed
with the help of dev_pm_qos_remove_request() in the future, like
any other device PM QoS latency request.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/base/power/qos.c |   25 +++++++++++++++++++++++++
 include/linux/pm_qos.h   |    5 +++++
 2 files changed, 30 insertions(+)

Index: linux/drivers/base/power/qos.c
=================================--- linux.orig/drivers/base/power/qos.c
+++ linux/drivers/base/power/qos.c
@@ -420,3 +420,28 @@ int dev_pm_qos_remove_global_notifier(st
 	return blocking_notifier_chain_unregister(&dev_pm_notifiers, notifier);
 }
 EXPORT_SYMBOL_GPL(dev_pm_qos_remove_global_notifier);
+
+/**
+ * dev_pm_qos_add_ancestor_request - Add PM QoS request for device's ancestor.
+ * @dev: Device whose ancestor to add the request for.
+ * @req: Pointer to the preallocated handle.
+ * @value: Constraint latency value.
+ */
+int dev_pm_qos_add_ancestor_request(struct device *dev,
+				    struct dev_pm_qos_request *req, s32 value)
+{
+	struct device *ancestor = dev->parent;
+	int error = -ENODEV;
+
+	while (ancestor && !ancestor->power.ignore_children)
+		ancestor = ancestor->parent;
+
+	if (ancestor)
+		error = dev_pm_qos_add_request(ancestor, req, value);
+
+	if (error)
+		req->dev = NULL;
+
+	return error;
+}
+EXPORT_SYMBOL_GPL(dev_pm_qos_add_ancestor_request);
Index: linux/include/linux/pm_qos.h
=================================--- linux.orig/include/linux/pm_qos.h
+++ linux/include/linux/pm_qos.h
@@ -92,6 +92,8 @@ int dev_pm_qos_add_global_notifier(struc
 int dev_pm_qos_remove_global_notifier(struct notifier_block *notifier);
 void dev_pm_qos_constraints_init(struct device *dev);
 void dev_pm_qos_constraints_destroy(struct device *dev);
+int dev_pm_qos_add_ancestor_request(struct device *dev,
+				    struct dev_pm_qos_request *req, s32 value);
 #else
 static inline int pm_qos_update_target(struct pm_qos_constraints *c,
 				       struct plist_node *node,
@@ -153,6 +155,9 @@ static inline void dev_pm_qos_constraint
 {
 	dev->power.power_state = PMSG_INVALID;
 }
+int dev_pm_qos_add_ancestor_request(struct device *dev,
+				    struct dev_pm_qos_request *req, s32 value)
+			{ return 0; }
 #endif
 
 #endif

[RFC][PATCH 4/4] PM / input / touchscreen: Make st1232 use device PM QoS constraints

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

Make the st1232 driver use dev_pm_qos_add_ancestor_request() to
add a device PM QoS latency constraint for the I2C controller it
depends on, so that the controller won't go into an overly deep
low-power state when the touchscreen has to be particularly
responsive (e.g. when the user moves his or her finger on it)
and allow the A4R domain to be turned off if that doesn't violate
the PM QoS latency constraints.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 arch/arm/mach-shmobile/pm-sh7372.c |   28 +++++++++++++++++++++++++++-
 drivers/input/touchscreen/st1232.c |   14 +++++++++++++-
 2 files changed, 40 insertions(+), 2 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
@@ -239,7 +239,6 @@ struct sh7372_pm_domain sh7372_d4 = {
 struct sh7372_pm_domain sh7372_a4r = {
 	.genpd.name = "A4R",
 	.bit_shift = 5,
-	.gov = &pm_domain_always_on_gov,
 	.suspend = sh7372_a4r_suspend,
 	.resume = sh7372_intcs_resume,
 };
@@ -496,9 +495,36 @@ static int sh7372_enter_suspend(suspend_
 	return 0;
 }
 
+/**
+ * sh7372_pm_notifier_fn - SH7372 PM notifier routine.
+ * @notifier: Unused.
+ * @pm_event: Event being handled.
+ * @unused: Unused.
+ *
+ * This is necessary, because the A4R domain has to be "on" when
+ * suspend_device_irqs() and resume_device_irqs() are executed during system
+ * suspend and resume, respectively, so that those functions don't crash
+ * while accessing the INTCS.
+ */
+static int sh7372_pm_notifier_fn(struct notifier_block *notifier,
+				 unsigned long pm_event, void *unused)
+{
+	switch (pm_event) {
+	case PM_SUSPEND_PREPARE:
+		pm_genpd_poweron(&sh7372_a4r.genpd);
+		break;
+	case PM_POST_SUSPEND:
+		pm_genpd_poweroff_unused();
+		break;
+	}
+
+	return NOTIFY_DONE;
+}
+
 static void sh7372_suspend_init(void)
 {
 	shmobile_suspend_ops.enter = sh7372_enter_suspend;
+	pm_notifier(sh7372_pm_notifier_fn, 0);
 }
 #else
 static void sh7372_suspend_init(void) {}
Index: linux/drivers/input/touchscreen/st1232.c
=================================--- linux.orig/drivers/input/touchscreen/st1232.c
+++ linux/drivers/input/touchscreen/st1232.c
@@ -25,6 +25,7 @@
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/types.h>
+#include <linux/pm_qos.h>
 
 #define ST1232_TS_NAME	"st1232-ts"
 
@@ -46,6 +47,8 @@ struct st1232_ts_data {
 	struct i2c_client *client;
 	struct input_dev *input_dev;
 	struct st1232_ts_finger finger[MAX_FINGERS];
+	struct dev_pm_qos_request low_latency_req;
+	bool pen_down;
 };
 
 static int st1232_ts_read_data(struct st1232_ts_data *ts)
@@ -118,8 +121,17 @@ static irqreturn_t st1232_ts_irq_handler
 	}
 
 	/* SYN_MT_REPORT only if no contact */
-	if (!count)
+	if (!count) {
 		input_mt_sync(input_dev);
+		if (ts->pen_down) {
+			dev_pm_qos_remove_request(&ts->low_latency_req);
+			ts->pen_down = false;
+		}
+	} else if (!ts->pen_down) {
+		/* First contact, request 100 mcs latency. */
+		dev_pm_qos_add_ancestor_request(&ts->client->dev,
+						&ts->low_latency_req, 100);
+	}
 
 	/* SYN_REPORT */
 	input_sync(input_dev);

Re: [RFC][PATCH 4/4] PM / input / touchscreen: Make st1232 use device

[Guennadi Liakhovetski]

Hi Rafael

On Sat, 10 Dec 2011, Rafael J. Wysocki wrote:

> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> Make the st1232 driver use dev_pm_qos_add_ancestor_request() to
> add a device PM QoS latency constraint for the I2C controller it
> depends on, so that the controller won't go into an overly deep
> low-power state when the touchscreen has to be particularly
> responsive (e.g. when the user moves his or her finger on it)
> and allow the A4R domain to be turned off if that doesn't violate
> the PM QoS latency constraints.
> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> ---
>  arch/arm/mach-shmobile/pm-sh7372.c |   28 +++++++++++++++++++++++++++-
>  drivers/input/touchscreen/st1232.c |   14 +++++++++++++-

I don't think you wanted to put these two files in one patch.

>  2 files changed, 40 insertions(+), 2 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

[snip]

> Index: linux/drivers/input/touchscreen/st1232.c
> =================================> --- linux.orig/drivers/input/touchscreen/st1232.c
> +++ linux/drivers/input/touchscreen/st1232.c

After you have separated this file, you might as well notice, that it is 
almost identical to my version, that we discussed privately previously. 
Your changes include:

> @@ -25,6 +25,7 @@
>  #include <linux/module.h>
>  #include <linux/slab.h>
>  #include <linux/types.h>
> +#include <linux/pm_qos.h>

in my version I preserved the alphabetic order of include files, you break 
it

>  
>  #define ST1232_TS_NAME	"st1232-ts"
>  
> @@ -46,6 +47,8 @@ struct st1232_ts_data {
>  	struct i2c_client *client;
>  	struct input_dev *input_dev;
>  	struct st1232_ts_finger finger[MAX_FINGERS];
> +	struct dev_pm_qos_request low_latency_req;
> +	bool pen_down;
>  };
>  
>  static int st1232_ts_read_data(struct st1232_ts_data *ts)
> @@ -118,8 +121,17 @@ static irqreturn_t st1232_ts_irq_handler
>  	}
>  
>  	/* SYN_MT_REPORT only if no contact */
> -	if (!count)
> +	if (!count) {
>  		input_mt_sync(input_dev);
> +		if (ts->pen_down) {
> +			dev_pm_qos_remove_request(&ts->low_latency_req);
> +			ts->pen_down = false;
> +		}
> +	} else if (!ts->pen_down) {
> +		/* First contact, request 100 mcs latency. */
> +		dev_pm_qos_add_ancestor_request(&ts->client->dev,
> +						&ts->low_latency_req, 100);

in my version I also was actually setting pen_down to true, which you seem 
to have dropped, so, your version just wouldn't work. Of course, I wasn't 
using the "ancestor" version of the above call, because it didn't exist 
back then.

You also removed my debugging, which is, of course, correct - thanks for 
doing that.

Given the above, I would appreciate it, if you split this patch into two 
and make the st1232 part of it

From: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>

after which you can add your

[rjw@sisk.pl: use dev_pm_qos_add_ancestor_request() to reach the correct ancestor]

and your Sob.

Thanks
Guennadi

> +	}
>  
>  	/* SYN_REPORT */
>  	input_sync(input_dev);
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

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

[Update][PATCH 0/5] PM / shmobile: Use PM QoS latency constraints in touchscreen driver on SH7372

[Rafael J. Wysocki]

Hi,

I have reworked the patchset slightly.

On Saturday, December 10, 2011, Rafael J. Wysocki wrote:
> 
> The following series of patches updates the SH7372 PM domain's so that
> A4R doesn't need to be turned on during system sleep, drops the SH7372's
> PM domains stay_on flag and makes the touchscreen add a PM QoS constraint
> on its I2C controller to prevent the A4R domain containing it from being
> turned off (due to power management) when the touchscreen is active.

The first two patches are unchaged (apart from the Magnus' ACKs), as well
as the third one.  The fourth one, though, has been split as suggested by
Guennadi and the touchscreen part has been simplified slightly.

[1/5] - Don't include SH7372's INTCS in syscore suspend/resume.
[2/5] - Remove the stay_on flag from SH7372's PM domains.
[3/5] - Introduce dev_pm_qos_add_ancestor_request().
[4/5] - Make st1232 use device PM QoS constraints.
[5/5] - Allow the A4R domain to be turned off and on at runtime.

Thanks,
Rafael

[Update][PATCH 1/5] PM / shmobile: Don't include SH7372's INTCS in syscore suspend/resume

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

Since the SH7372's INTCS in included into syscore suspend/resume,
which causes the chip to be accessed when PM domains have been
turned off during system suspend, the A4R domain containing the
INTCS has to stay on during system sleep, which is suboptimal
from the power consumption point of view.

For this reason, add a new INTC flag, skip_syscore_suspend, to mark
the INTCS for intc_suspend() and intc_resume(), so that they don't
touch it.  This allows the A4R domain to be turned off during
system suspend and the INTCS state is resrored during system
resume by the A4R's "power on" code.

Suggested-by: Magnus Damm <damm@opensource.se>
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Acked-by: Magnus Damm <damm@opensource.se>
---
 arch/arm/mach-shmobile/intc-sh7372.c |    1 +
 drivers/sh/intc/core.c               |    8 ++++++++
 drivers/sh/intc/internals.h          |    1 +
 include/linux/sh_intc.h              |    1 +
 4 files changed, 11 insertions(+)

Index: linux/include/linux/sh_intc.h
=================================--- linux.orig/include/linux/sh_intc.h
+++ linux/include/linux/sh_intc.h
@@ -95,6 +95,7 @@ struct intc_desc {
 	unsigned int num_resources;
 	intc_enum force_enable;
 	intc_enum force_disable;
+	bool skip_syscore_suspend;
 	struct intc_hw_desc hw;
 };
 
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
@@ -535,6 +535,7 @@ static struct resource intcs_resources[]
 static struct intc_desc intcs_desc __initdata = {
 	.name = "sh7372-intcs",
 	.force_enable = ENABLED_INTCS,
+	.skip_syscore_suspend = true,
 	.resource = intcs_resources,
 	.num_resources = ARRAY_SIZE(intcs_resources),
 	.hw = INTC_HW_DESC(intcs_vectors, intcs_groups, intcs_mask_registers,
Index: linux/drivers/sh/intc/core.c
=================================--- linux.orig/drivers/sh/intc/core.c
+++ linux/drivers/sh/intc/core.c
@@ -354,6 +354,8 @@ int __init register_intc_controller(stru
 	if (desc->force_enable)
 		intc_enable_disable_enum(desc, d, desc->force_enable, 1);
 
+	d->skip_suspend = desc->skip_syscore_suspend;
+
 	nr_intc_controllers++;
 
 	return 0;
@@ -386,6 +388,9 @@ static int intc_suspend(void)
 	list_for_each_entry(d, &intc_list, list) {
 		int irq;
 
+		if (d->skip_suspend)
+			continue;
+
 		/* enable wakeup irqs belonging to this intc controller */
 		for_each_active_irq(irq) {
 			struct irq_data *data;
@@ -409,6 +414,9 @@ static void intc_resume(void)
 	list_for_each_entry(d, &intc_list, list) {
 		int irq;
 
+		if (d->skip_suspend)
+			continue;
+
 		for_each_active_irq(irq) {
 			struct irq_data *data;
 			struct irq_chip *chip;
Index: linux/drivers/sh/intc/internals.h
=================================--- linux.orig/drivers/sh/intc/internals.h
+++ linux/drivers/sh/intc/internals.h
@@ -67,6 +67,7 @@ struct intc_desc_int {
 	struct intc_window *window;
 	unsigned int nr_windows;
 	struct irq_chip chip;
+	bool skip_suspend;
 };
 
 

[Update][PATCH 2/5] PM / shmobile: Remove the stay_on flag from SH7372's PM domains

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

SH7372 uses two independent mechanisms for ensuring that power
domains will never be turned off: the stay_on flag and the "always
on" domain governor.  Moreover, the "always on" governor is only taken
into accout by runtime PM code paths, while the stay_on flag affects
all attempts to turn the given domain off.  Thus setting the stay_on
flag causes the "always on" governor to be unnecessary, which is
quite confusing.

However, the stay_on flag is currently only set for the A3SP PM
domain and only if console_suspend_enabled, so it may be replaced
by checking console_suspend_enabled directly in the A3SP's
.suspend() routine.  [This requires domain .suspend() to return
a result, but that is a minor modification.]

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Acked-by: Magnus Damm <damm@opensource.se>
---
 arch/arm/mach-shmobile/include/mach/sh7372.h |    3 --
 arch/arm/mach-shmobile/pm-sh7372.c           |   36 ++++++++++++---------------
 2 files changed, 18 insertions(+), 21 deletions(-)

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,11 +480,10 @@ struct platform_device;
 struct sh7372_pm_domain {
 	struct generic_pm_domain genpd;
 	struct dev_power_governor *gov;
-	void (*suspend)(void);
+	int (*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)
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
@@ -82,11 +82,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->suspend) {
+		int ret = sh7372_pd->suspend();
 
-	if (sh7372_pd->stay_on)
-		return 0;
+		if (ret)
+			return ret;
+	}
 
 	if (__raw_readl(PSTR) & mask) {
 		unsigned int retry_count;
@@ -113,9 +114,6 @@ static int __pd_power_up(struct sh7372_p
 	unsigned int retry_count;
 	int ret = 0;
 
-	if (sh7372_pd->stay_on)
-		goto out;
-
 	if (__raw_readl(PSTR) & mask)
 		goto out;
 
@@ -148,10 +146,11 @@ static int pd_power_up(struct generic_pm
 	 return __pd_power_up(to_sh7372_pd(genpd), true);
 }
 
-static void sh7372_a4r_suspend(void)
+static int sh7372_a4r_suspend(void)
 {
 	sh7372_intcs_suspend();
 	__raw_writel(0x300fffff, WUPRMSK); /* avoid wakeup */
+	return 0;
 }
 
 static bool pd_active_wakeup(struct device *dev)
@@ -243,7 +242,6 @@ struct sh7372_pm_domain sh7372_a4r = {
 	.gov = &pm_domain_always_on_gov,
 	.suspend = sh7372_a4r_suspend,
 	.resume = sh7372_intcs_resume,
-	.stay_on = true,
 };
 
 struct sh7372_pm_domain sh7372_a3rv = {
@@ -256,21 +254,23 @@ struct sh7372_pm_domain sh7372_a3ri = {
 	.bit_shift = 8,
 };
 
+static int sh7372_a3sp_suspend(void)
+{
+	/*
+	 * Serial consoles make use of SCIF hardware located in A3SP,
+	 * keep such power domain on if "no_console_suspend" is set.
+	 */
+	return console_suspend_enabled ? -EBUSY : 0;
+}
+
 struct sh7372_pm_domain sh7372_a3sp = {
 	.genpd.name = "A3SP",
 	.bit_shift = 11,
 	.gov = &pm_domain_always_on_gov,
 	.no_debug = true,
+	.suspend = sh7372_a3sp_suspend,
 };
 
-static void sh7372_a3sp_init(void)
-{
-	/* serial consoles make use of SCIF hardware located in A3SP,
-	 * keep such power domain on if "no_console_suspend" is set.
-	 */
-	sh7372_a3sp.stay_on = !console_suspend_enabled;
-}
-
 struct sh7372_pm_domain sh7372_a3sg = {
 	.genpd.name = "A3SG",
 	.bit_shift = 13,
@@ -514,8 +514,6 @@ void __init sh7372_pm_init(void)
 	/* do not convert A3SM, A3SP, A3SG, A4R power down into A4S */
 	__raw_writel(0, PDNSEL);
 
-	sh7372_a3sp_init();
-
 	sh7372_suspend_init();
 	sh7372_cpuidle_init();
 }

[Update][PATCH 3/5] PM / QoS: Introduce dev_pm_qos_add_ancestor_request()

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

Some devices, like the I2C controller on SH7372, are not
necessary for providing power to their children or forwarding
wakeup signals (and generally interrupts) from them.  They are
only needed by their children when there's some data to transfer,
so they may be suspended for the majority of time and resumed
on demand, when the children have data to send or receive.  For this
purpose, however, their power.ignore_children flags have to be set,
or the PM core wouldn't allow them to be suspended while their
children were active.

Unfortunately, in some situations it may take too much time to
resume such devices so that they can assist their children in
transferring data.  For example, if such a device belongs to a PM
domain which goes to the "power off" state when that device is
suspended, it may take too much time to restore power to the
domain in response to the request from one of the device's
children.  In that case, if the parent's resume time is critical,
the domain should stay in the "power on" state, although it still may
be desirable to power manage the parent itself (e.g. by manipulating
its clock).

In general, device PM QoS may be used to address this problem.
Namely, if the device's children added PM QoS latency constraints
for it, they would be able to prevent it from being put into an
overly deep low-power state.  However, in some cases the devices
needing to be serviced are not the immediate children of a
"children-ignoring" device, but its grandchildren or even less
direct descendants.  In those cases, the entity wanting to add a
PM QoS request for a given device's ancestor that ignores its
children will have to find it in the first place, so introduce a new
helper function that may be used to achieve that.  This function,
dev_pm_qos_add_ancestor_request(), will search for the first
ancestor of the given device whose power.ignore_children flag is
set and will add a device PM QoS latency request for that ancestor
on behalf of the caller.  The request added this way may be removed
with the help of dev_pm_qos_remove_request() in the future, like
any other device PM QoS latency request.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/base/power/qos.c |   25 +++++++++++++++++++++++++
 include/linux/pm_qos.h   |    5 +++++
 2 files changed, 30 insertions(+)

Index: linux/drivers/base/power/qos.c
=================================--- linux.orig/drivers/base/power/qos.c
+++ linux/drivers/base/power/qos.c
@@ -420,3 +420,28 @@ int dev_pm_qos_remove_global_notifier(st
 	return blocking_notifier_chain_unregister(&dev_pm_notifiers, notifier);
 }
 EXPORT_SYMBOL_GPL(dev_pm_qos_remove_global_notifier);
+
+/**
+ * dev_pm_qos_add_ancestor_request - Add PM QoS request for device's ancestor.
+ * @dev: Device whose ancestor to add the request for.
+ * @req: Pointer to the preallocated handle.
+ * @value: Constraint latency value.
+ */
+int dev_pm_qos_add_ancestor_request(struct device *dev,
+				    struct dev_pm_qos_request *req, s32 value)
+{
+	struct device *ancestor = dev->parent;
+	int error = -ENODEV;
+
+	while (ancestor && !ancestor->power.ignore_children)
+		ancestor = ancestor->parent;
+
+	if (ancestor)
+		error = dev_pm_qos_add_request(ancestor, req, value);
+
+	if (error)
+		req->dev = NULL;
+
+	return error;
+}
+EXPORT_SYMBOL_GPL(dev_pm_qos_add_ancestor_request);
Index: linux/include/linux/pm_qos.h
=================================--- linux.orig/include/linux/pm_qos.h
+++ linux/include/linux/pm_qos.h
@@ -92,6 +92,8 @@ int dev_pm_qos_add_global_notifier(struc
 int dev_pm_qos_remove_global_notifier(struct notifier_block *notifier);
 void dev_pm_qos_constraints_init(struct device *dev);
 void dev_pm_qos_constraints_destroy(struct device *dev);
+int dev_pm_qos_add_ancestor_request(struct device *dev,
+				    struct dev_pm_qos_request *req, s32 value);
 #else
 static inline int pm_qos_update_target(struct pm_qos_constraints *c,
 				       struct plist_node *node,
@@ -153,6 +155,9 @@ static inline void dev_pm_qos_constraint
 {
 	dev->power.power_state = PMSG_INVALID;
 }
+int dev_pm_qos_add_ancestor_request(struct device *dev,
+				    struct dev_pm_qos_request *req, s32 value)
+			{ return 0; }
 #endif
 
 #endif

[Update][PATCH 4/5] PM / input / touchscreen: Make st1232 use device PM QoS constraints

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

Make the st1232 driver use dev_pm_qos_add_ancestor_request() to
add a device PM QoS latency constraint for the controller it
depends on, so that the controller won't go into an overly deep
low-power state when the touchscreen has to be particularly
responsive (e.g. when the user moves his or her finger on it).

This change is based on a prototype patch from Guennadi Liakhovetski.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/input/touchscreen/st1232.c |   13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

Index: linux/drivers/input/touchscreen/st1232.c
=================================--- linux.orig/drivers/input/touchscreen/st1232.c
+++ linux/drivers/input/touchscreen/st1232.c
@@ -23,6 +23,7 @@
 #include <linux/input.h>
 #include <linux/interrupt.h>
 #include <linux/module.h>
+#include <linux/pm_qos.h>
 #include <linux/slab.h>
 #include <linux/types.h>
 
@@ -46,6 +47,7 @@ struct st1232_ts_data {
 	struct i2c_client *client;
 	struct input_dev *input_dev;
 	struct st1232_ts_finger finger[MAX_FINGERS];
+	struct dev_pm_qos_request low_latency_req;
 };
 
 static int st1232_ts_read_data(struct st1232_ts_data *ts)
@@ -118,8 +120,17 @@ static irqreturn_t st1232_ts_irq_handler
 	}
 
 	/* SYN_MT_REPORT only if no contact */
-	if (!count)
+	if (!count) {
 		input_mt_sync(input_dev);
+		if (ts->low_latency_req.dev) {
+			dev_pm_qos_remove_request(&ts->low_latency_req);
+			ts->low_latency_req.dev = NULL;
+		}
+	} else if (!ts->low_latency_req.dev) {
+		/* First contact, request 100 us latency. */
+		dev_pm_qos_add_ancestor_request(&ts->client->dev,
+						&ts->low_latency_req, 100);
+	}
 
 	/* SYN_REPORT */
 	input_sync(input_dev);

[Update][PATCH 5/5] PM / shmobile: Allow the A4R domain to be turned off at run time

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

After adding PM QoS constraints for the I2C controller in the A4R
domain, that domain can be allowed to be turned off and on by runtime
PM, so remove the "always on" governor from it.

However, the A4R domain has to be "on" when suspend_device_irqs() and
resume_device_irqs() are executed during system suspend and resume,
respectively, so that those functions don't crash while accessing the
INTCS.  For this reason, add a PM notifier to the SH7372 PM code and
make it restore power to A4R before system suspend and remove power
from all unused PM domains after system resume.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Acked-by: Magnus Damm <damm@opensource.se>
---
 arch/arm/mach-shmobile/pm-sh7372.c |   29 ++++++++++++++++++++++++++++-
 1 file changed, 28 insertions(+), 1 deletion(-)

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
@@ -239,7 +239,6 @@ struct sh7372_pm_domain sh7372_d4 = {
 struct sh7372_pm_domain sh7372_a4r = {
 	.genpd.name = "A4R",
 	.bit_shift = 5,
-	.gov = &pm_domain_always_on_gov,
 	.suspend = sh7372_a4r_suspend,
 	.resume = sh7372_intcs_resume,
 };
@@ -496,9 +495,37 @@ static int sh7372_enter_suspend(suspend_
 	return 0;
 }
 
+/**
+ * sh7372_pm_notifier_fn - SH7372 PM notifier routine.
+ * @notifier: Unused.
+ * @pm_event: Event being handled.
+ * @unused: Unused.
+ */
+static int sh7372_pm_notifier_fn(struct notifier_block *notifier,
+				 unsigned long pm_event, void *unused)
+{
+	switch (pm_event) {
+	case PM_SUSPEND_PREPARE:
+		/*
+		 * This is necessary, because the A4R domain has to be "on"
+		 * when suspend_device_irqs() and resume_device_irqs() are
+		 * executed during system suspend and resume, respectively, so
+		 * that those functions don't crash while accessing the INTCS.
+		 */
+		pm_genpd_poweron(&sh7372_a4r.genpd);
+		break;
+	case PM_POST_SUSPEND:
+		pm_genpd_poweroff_unused();
+		break;
+	}
+
+	return NOTIFY_DONE;
+}
+
 static void sh7372_suspend_init(void)
 {
 	shmobile_suspend_ops.enter = sh7372_enter_suspend;
+	pm_notifier(sh7372_pm_notifier_fn, 0);
 }
 #else
 static void sh7372_suspend_init(void) {}