All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] sh_tmu / PM: Prevent power from being removed from TMU devices
@ 2012-03-02 23:41 Rafael J. Wysocki
  2012-03-04 21:03 ` Paul Mundt
                   ` (13 more replies)
  0 siblings, 14 replies; 15+ messages in thread
From: Rafael J. Wysocki @ 2012-03-02 23:41 UTC (permalink / raw)
  To: linux-sh

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

To prevent TMU devices from losing power define a fake "always busy"
suspend callback in the sh_tmu driver that will cause all attempts to
suspend the system to fail.  In addition to that, make the driver
change the runtime PM settings of TMU devices so that they appear to
be always "active" and make SH7372 add them to the A4R domain (where
they physically belong), to make the PM domains core take those
settings into consideration when attempting to remove power from A4R.

Proprer power management support will be added to the sh_tmu driver
in the future.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 arch/arm/mach-shmobile/setup-sh7372.c |    2 ++
 drivers/clocksource/sh_tmu.c          |   12 ++++++++++++
 2 files changed, 14 insertions(+)

Index: linux/drivers/clocksource/sh_tmu.c
=================================--- linux.orig/drivers/clocksource/sh_tmu.c
+++ linux/drivers/clocksource/sh_tmu.c
@@ -426,6 +426,10 @@ static int __devinit sh_tmu_probe(struct
 		kfree(p);
 		platform_set_drvdata(pdev, NULL);
 	}
+#ifdef CONFIG_PM_RUNTIME
+	pdev->dev.power.disable_depth = 0;
+	pdev->dev.power.runtime_status = RPM_ACTIVE;
+#endif
 	return ret;
 }
 
@@ -434,11 +438,19 @@ static int __devexit sh_tmu_remove(struc
 	return -EBUSY; /* cannot unregister clockevent and clocksource */
 }
 
+static int sh_tmu_no_suspend(struct device *dev)
+{
+	return -EBUSY;
+}
+
+static SIMPLE_DEV_PM_OPS(sh_tmu_pm_ops, sh_tmu_no_suspend, NULL);
+
 static struct platform_driver sh_tmu_device_driver = {
 	.probe		= sh_tmu_probe,
 	.remove		= __devexit_p(sh_tmu_remove),
 	.driver		= {
 		.name	= "sh_tmu",
+		.pm	= &sh_tmu_pm_ops,
 	}
 };
 
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
@@ -1043,6 +1043,8 @@ void __init sh7372_add_standard_devices(
 	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);
+	sh7372_add_device_to_domain(&sh7372_a4r, &tmu00_device);
+	sh7372_add_device_to_domain(&sh7372_a4r, &tmu01_device);
 }
 
 void __init sh7372_add_early_devices(void)

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] sh_tmu / PM: Prevent power from being removed from TMU devices
  2012-03-02 23:41 [PATCH] sh_tmu / PM: Prevent power from being removed from TMU devices Rafael J. Wysocki
@ 2012-03-04 21:03 ` Paul Mundt
  2012-03-04 21:50 ` Rafael J. Wysocki
                   ` (12 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Paul Mundt @ 2012-03-04 21:03 UTC (permalink / raw)
  To: linux-sh

On Sat, Mar 03, 2012 at 12:41:30AM +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> To prevent TMU devices from losing power define a fake "always busy"
> suspend callback in the sh_tmu driver that will cause all attempts to
> suspend the system to fail.  In addition to that, make the driver
> change the runtime PM settings of TMU devices so that they appear to
> be always "active" and make SH7372 add them to the A4R domain (where
> they physically belong), to make the PM domains core take those
> settings into consideration when attempting to remove power from A4R.
> 
> Proprer power management support will be added to the sh_tmu driver
> in the future.
> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>

Presumably we also need the same for the MTU2 and CMT drivers?

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] sh_tmu / PM: Prevent power from being removed from TMU devices
  2012-03-02 23:41 [PATCH] sh_tmu / PM: Prevent power from being removed from TMU devices Rafael J. Wysocki
  2012-03-04 21:03 ` Paul Mundt
@ 2012-03-04 21:50 ` Rafael J. Wysocki
  2012-03-05  5:47 ` Paul Mundt
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Rafael J. Wysocki @ 2012-03-04 21:50 UTC (permalink / raw)
  To: linux-sh

On Sunday, March 04, 2012, Paul Mundt wrote:
> On Sat, Mar 03, 2012 at 12:41:30AM +0100, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rjw@sisk.pl>
> > 
> > To prevent TMU devices from losing power define a fake "always busy"
> > suspend callback in the sh_tmu driver that will cause all attempts to
> > suspend the system to fail.  In addition to that, make the driver
> > change the runtime PM settings of TMU devices so that they appear to
> > be always "active" and make SH7372 add them to the A4R domain (where
> > they physically belong), to make the PM domains core take those
> > settings into consideration when attempting to remove power from A4R.
> > 
> > Proprer power management support will be added to the sh_tmu driver
> > in the future.
> > 
> > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> 
> Presumably we also need the same for the MTU2 and CMT drivers?

Yes, we do in principle, although this isn't strictly necessary for things to
work at the moment.

I can post analogous patches for the other drivers if you want me to.

Thanks,
Rafael

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] sh_tmu / PM: Prevent power from being removed from TMU devices
  2012-03-02 23:41 [PATCH] sh_tmu / PM: Prevent power from being removed from TMU devices Rafael J. Wysocki
  2012-03-04 21:03 ` Paul Mundt
  2012-03-04 21:50 ` Rafael J. Wysocki
@ 2012-03-05  5:47 ` Paul Mundt
  2012-03-05  8:01 ` Magnus Damm
                   ` (10 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Paul Mundt @ 2012-03-05  5:47 UTC (permalink / raw)
  To: linux-sh

On Sun, Mar 04, 2012 at 10:50:53PM +0100, Rafael J. Wysocki wrote:
> On Sunday, March 04, 2012, Paul Mundt wrote:
> > On Sat, Mar 03, 2012 at 12:41:30AM +0100, Rafael J. Wysocki wrote:
> > > From: Rafael J. Wysocki <rjw@sisk.pl>
> > > 
> > > To prevent TMU devices from losing power define a fake "always busy"
> > > suspend callback in the sh_tmu driver that will cause all attempts to
> > > suspend the system to fail.  In addition to that, make the driver
> > > change the runtime PM settings of TMU devices so that they appear to
> > > be always "active" and make SH7372 add them to the A4R domain (where
> > > they physically belong), to make the PM domains core take those
> > > settings into consideration when attempting to remove power from A4R.
> > > 
> > > Proprer power management support will be added to the sh_tmu driver
> > > in the future.
> > > 
> > > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> > 
> > Presumably we also need the same for the MTU2 and CMT drivers?
> 
> Yes, we do in principle, although this isn't strictly necessary for things to
> work at the moment.
> 
> I can post analogous patches for the other drivers if you want me to.
> 
It would be nice to ensure that we don't run in to the same problems on
the other drivers in the interim at least. I'll take a look at getting
proper PM support taken care of for them afterwards.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] sh_tmu / PM: Prevent power from being removed from TMU devices
  2012-03-02 23:41 [PATCH] sh_tmu / PM: Prevent power from being removed from TMU devices Rafael J. Wysocki
                   ` (2 preceding siblings ...)
  2012-03-05  5:47 ` Paul Mundt
@ 2012-03-05  8:01 ` Magnus Damm
  2012-03-05 17:29 ` Paul Mundt
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Magnus Damm @ 2012-03-05  8:01 UTC (permalink / raw)
  To: linux-sh

On Mon, Mar 5, 2012 at 2:47 PM, Paul Mundt <lethal@linux-sh.org> wrote:
> On Sun, Mar 04, 2012 at 10:50:53PM +0100, Rafael J. Wysocki wrote:
>> On Sunday, March 04, 2012, Paul Mundt wrote:
>> > On Sat, Mar 03, 2012 at 12:41:30AM +0100, Rafael J. Wysocki wrote:
>> > > From: Rafael J. Wysocki <rjw@sisk.pl>
>> > >
>> > > To prevent TMU devices from losing power define a fake "always busy"
>> > > suspend callback in the sh_tmu driver that will cause all attempts to
>> > > suspend the system to fail.  In addition to that, make the driver
>> > > change the runtime PM settings of TMU devices so that they appear to
>> > > be always "active" and make SH7372 add them to the A4R domain (where
>> > > they physically belong), to make the PM domains core take those
>> > > settings into consideration when attempting to remove power from A4R.
>> > >
>> > > Proprer power management support will be added to the sh_tmu driver
>> > > in the future.
>> > >
>> > > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
>> >
>> > Presumably we also need the same for the MTU2 and CMT drivers?
>>
>> Yes, we do in principle, although this isn't strictly necessary for things to
>> work at the moment.
>>
>> I can post analogous patches for the other drivers if you want me to.
>>
> It would be nice to ensure that we don't run in to the same problems on
> the other drivers in the interim at least. I'll take a look at getting
> proper PM support taken care of for them afterwards.

I agree, but I wonder how much of an actual issue it is at this point.
In the long run of course the drivers should be fixed up, but this
patch more looks like a workaround for 3.3-rc.

As for actual hardware timers, MTU2 is not used on any platform with
power domain support today and the CMT is located in a always-on power
domain. So they should not cause any issues for 3.3-rc.

/ magnus

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] sh_tmu / PM: Prevent power from being removed from TMU devices
  2012-03-02 23:41 [PATCH] sh_tmu / PM: Prevent power from being removed from TMU devices Rafael J. Wysocki
                   ` (3 preceding siblings ...)
  2012-03-05  8:01 ` Magnus Damm
@ 2012-03-05 17:29 ` Paul Mundt
  2012-03-05 20:32 ` Rafael J. Wysocki
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Paul Mundt @ 2012-03-05 17:29 UTC (permalink / raw)
  To: linux-sh

On Mon, Mar 05, 2012 at 05:01:20PM +0900, Magnus Damm wrote:
> On Mon, Mar 5, 2012 at 2:47 PM, Paul Mundt <lethal@linux-sh.org> wrote:
> > On Sun, Mar 04, 2012 at 10:50:53PM +0100, Rafael J. Wysocki wrote:
> >> On Sunday, March 04, 2012, Paul Mundt wrote:
> >> > Presumably we also need the same for the MTU2 and CMT drivers?
> >>
> >> Yes, we do in principle, although this isn't strictly necessary for things to
> >> work at the moment.
> >>
> >> I can post analogous patches for the other drivers if you want me to.
> >>
> > It would be nice to ensure that we don't run in to the same problems on
> > the other drivers in the interim at least. I'll take a look at getting
> > proper PM support taken care of for them afterwards.
> 
> I agree, but I wonder how much of an actual issue it is at this point.
> In the long run of course the drivers should be fixed up, but this
> patch more looks like a workaround for 3.3-rc.
> 
> As for actual hardware timers, MTU2 is not used on any platform with
> power domain support today and the CMT is located in a always-on power
> domain. So they should not cause any issues for 3.3-rc.
> 
It doesn't matter. If we're going to band-aid around the TMU case for a
PM incompatability then the same fix needs to be applied to the other
relevant drivers, barring a proper fix. We aren't going to be making
random band-aid fixes that in turn bring the drivers behaviourly out of
sync simply because of some imagined timeline issue with 3.3-rc. Again,
none of this thread would have even been necessary had the job been done
right from the beginning.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] sh_tmu / PM: Prevent power from being removed from TMU devices
  2012-03-02 23:41 [PATCH] sh_tmu / PM: Prevent power from being removed from TMU devices Rafael J. Wysocki
                   ` (4 preceding siblings ...)
  2012-03-05 17:29 ` Paul Mundt
@ 2012-03-05 20:32 ` Rafael J. Wysocki
  2012-03-05 22:46 ` Rafael J. Wysocki
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Rafael J. Wysocki @ 2012-03-05 20:32 UTC (permalink / raw)
  To: linux-sh

On Monday, March 05, 2012, Paul Mundt wrote:
> On Mon, Mar 05, 2012 at 05:01:20PM +0900, Magnus Damm wrote:
> > On Mon, Mar 5, 2012 at 2:47 PM, Paul Mundt <lethal@linux-sh.org> wrote:
> > > On Sun, Mar 04, 2012 at 10:50:53PM +0100, Rafael J. Wysocki wrote:
> > >> On Sunday, March 04, 2012, Paul Mundt wrote:
> > >> > Presumably we also need the same for the MTU2 and CMT drivers?
> > >>
> > >> Yes, we do in principle, although this isn't strictly necessary for things to
> > >> work at the moment.
> > >>
> > >> I can post analogous patches for the other drivers if you want me to.
> > >>
> > > It would be nice to ensure that we don't run in to the same problems on
> > > the other drivers in the interim at least. I'll take a look at getting
> > > proper PM support taken care of for them afterwards.
> > 
> > I agree, but I wonder how much of an actual issue it is at this point.
> > In the long run of course the drivers should be fixed up, but this
> > patch more looks like a workaround for 3.3-rc.
> > 
> > As for actual hardware timers, MTU2 is not used on any platform with
> > power domain support today and the CMT is located in a always-on power
> > domain. So they should not cause any issues for 3.3-rc.
> > 
> It doesn't matter. If we're going to band-aid around the TMU case for a
> PM incompatability then the same fix needs to be applied to the other
> relevant drivers, barring a proper fix.

There is a problem applying that "fix" (or perhaps workaround would be a better
name) to the CMT driver, because that would make the system never suspend,
which isn't exactly desirable.  Sorry that I didn't notice that earlier.

> We aren't going to be making
> random band-aid fixes that in turn bring the drivers behaviourly out of
> sync simply because of some imagined timeline issue with 3.3-rc. Again,
> none of this thread would have even been necessary had the job been done
> right from the beginning.

Well, there seems to be a problem with TMU and system suspend that isn't
directly related to turning off the A4R domain (system resume doesn't appear
to work with TMU even if that domain is always on).  And it looks like that
problem has been there forever.

Anyway, to use the same approach in all three drivers we need a flag that
will allow a driver to say "don't power down the domain this device belongs to"
to the core.  It shouldn't be very difficult to introduce it, but I'm still
not sure that will make the "TMU vs system resume" problem mentioned above go
away.

I'll try to prepare a patch for that later today anyway.

Thanks,
Rafael

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] sh_tmu / PM: Prevent power from being removed from TMU devices
  2012-03-02 23:41 [PATCH] sh_tmu / PM: Prevent power from being removed from TMU devices Rafael J. Wysocki
                   ` (5 preceding siblings ...)
  2012-03-05 20:32 ` Rafael J. Wysocki
@ 2012-03-05 22:46 ` Rafael J. Wysocki
  2012-03-06  2:07 ` Paul Mundt
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Rafael J. Wysocki @ 2012-03-05 22:46 UTC (permalink / raw)
  To: linux-sh

On Monday, March 05, 2012, Rafael J. Wysocki wrote:
> On Monday, March 05, 2012, Paul Mundt wrote:
> > On Mon, Mar 05, 2012 at 05:01:20PM +0900, Magnus Damm wrote:
> > > On Mon, Mar 5, 2012 at 2:47 PM, Paul Mundt <lethal@linux-sh.org> wrote:
> > > > On Sun, Mar 04, 2012 at 10:50:53PM +0100, Rafael J. Wysocki wrote:
> > > >> On Sunday, March 04, 2012, Paul Mundt wrote:
> > > >> > Presumably we also need the same for the MTU2 and CMT drivers?
> > > >>
> > > >> Yes, we do in principle, although this isn't strictly necessary for things to
> > > >> work at the moment.
> > > >>
> > > >> I can post analogous patches for the other drivers if you want me to.
> > > >>
> > > > It would be nice to ensure that we don't run in to the same problems on
> > > > the other drivers in the interim at least. I'll take a look at getting
> > > > proper PM support taken care of for them afterwards.
> > > 
> > > I agree, but I wonder how much of an actual issue it is at this point.
> > > In the long run of course the drivers should be fixed up, but this
> > > patch more looks like a workaround for 3.3-rc.
> > > 
> > > As for actual hardware timers, MTU2 is not used on any platform with
> > > power domain support today and the CMT is located in a always-on power
> > > domain. So they should not cause any issues for 3.3-rc.
> > > 
> > It doesn't matter. If we're going to band-aid around the TMU case for a
> > PM incompatability then the same fix needs to be applied to the other
> > relevant drivers, barring a proper fix.
> 
> There is a problem applying that "fix" (or perhaps workaround would be a better
> name) to the CMT driver, because that would make the system never suspend,
> which isn't exactly desirable.  Sorry that I didn't notice that earlier.
> 
> > We aren't going to be making
> > random band-aid fixes that in turn bring the drivers behaviourly out of
> > sync simply because of some imagined timeline issue with 3.3-rc. Again,
> > none of this thread would have even been necessary had the job been done
> > right from the beginning.
> 
> Well, there seems to be a problem with TMU and system suspend that isn't
> directly related to turning off the A4R domain (system resume doesn't appear
> to work with TMU even if that domain is always on).  And it looks like that
> problem has been there forever.

OK, so my previous testing was flawed somehow and the patch below actually
works.

> Anyway, to use the same approach in all three drivers we need a flag that
> will allow a driver to say "don't power down the domain this device belongs to"
> to the core.  It shouldn't be very difficult to introduce it, but I'm still
> not sure that will make the "TMU vs system resume" problem mentioned above go
> away.
> 
> I'll try to prepare a patch for that later today anyway.

Is appended.  Without a changelog for now, because I need to add CMT and
MTU2 to it still.

Thanks,
Rafael

---
 arch/arm/mach-shmobile/setup-sh7372.c |    2 ++
 drivers/base/power/domain.c           |   25 +++++++++++++++++++++++--
 drivers/clocksource/sh_tmu.c          |    4 ++++
 include/linux/pm_domain.h             |    3 +++
 4 files changed, 32 insertions(+), 2 deletions(-)

Index: linux/drivers/base/power/domain.c
=================================--- linux.orig/drivers/base/power/domain.c
+++ linux/drivers/base/power/domain.c
@@ -372,7 +372,7 @@ static int pm_genpd_poweroff(struct gene
 	not_suspended = 0;
 	list_for_each_entry(pdd, &genpd->dev_list, list_node)
 		if (pdd->dev->driver && (!pm_runtime_suspended(pdd->dev)
-		    || pdd->dev->power.irq_safe))
+		    || pdd->dev->power.irq_safe || to_gpd_data(pdd)->always_on))
 			not_suspended++;
 
 	if (not_suspended > genpd->in_progress)
@@ -864,7 +864,8 @@ static int pm_genpd_suspend_noirq(struct
 		return -EINVAL;
 
 	if (genpd->suspend_power_off
-	    || (dev->power.wakeup_path && genpd_dev_active_wakeup(genpd, dev)))
+	    || (dev->power.wakeup_path && genpd_dev_active_wakeup(genpd, dev))
+	    || dev_gpd_data(dev)->always_on)
 		return 0;
 
 	genpd_stop_dev(genpd, dev);
@@ -1281,6 +1282,26 @@ int pm_genpd_remove_device(struct generi
 }
 
 /**
+ * pm_genpd_dev_always_on - Set/unset the "always on" flag for a given device.
+ * @dev: Device to set/unset the flag for.
+ * @val: The new value of the device's "always on" flag.
+ */
+void pm_genpd_dev_always_on(struct device *dev, bool val)
+{
+	struct pm_subsys_data *psd;
+	unsigned long flags;
+
+	spin_lock_irqsave(&dev->power.lock, flags);
+
+	psd = dev_to_psd(dev);
+	if (psd && psd->domain_data)
+		to_gpd_data(psd->domain_data)->always_on = val;
+
+	spin_unlock_irqrestore(&dev->power.lock, flags);
+}
+EXPORT_SYMBOL_GPL(pm_genpd_dev_always_on);
+
+/**
  * pm_genpd_add_subdomain - Add a subdomain to an I/O PM domain.
  * @genpd: Master PM domain to add the subdomain to.
  * @subdomain: Subdomain to be added.
Index: linux/include/linux/pm_domain.h
=================================--- linux.orig/include/linux/pm_domain.h
+++ linux/include/linux/pm_domain.h
@@ -97,6 +97,7 @@ struct generic_pm_domain_data {
 	struct gpd_dev_ops ops;
 	struct gpd_timing_data td;
 	bool need_restore;
+	bool always_on;
 };
 
 #ifdef CONFIG_PM_GENERIC_DOMAINS
@@ -125,6 +126,7 @@ static inline int pm_genpd_add_device(st
 
 extern int pm_genpd_remove_device(struct generic_pm_domain *genpd,
 				  struct device *dev);
+extern void pm_genpd_dev_always_on(struct device *dev, bool val);
 extern int pm_genpd_add_subdomain(struct generic_pm_domain *genpd,
 				  struct generic_pm_domain *new_subdomain);
 extern int pm_genpd_remove_subdomain(struct generic_pm_domain *genpd,
@@ -167,6 +169,7 @@ static inline int pm_genpd_remove_device
 {
 	return -ENOSYS;
 }
+static inline void pm_genpd_dev_always_on(struct device *dev, bool val) {}
 static inline int pm_genpd_add_subdomain(struct generic_pm_domain *genpd,
 					 struct generic_pm_domain *new_sd)
 {
Index: linux/drivers/clocksource/sh_tmu.c
=================================--- linux.orig/drivers/clocksource/sh_tmu.c
+++ linux/drivers/clocksource/sh_tmu.c
@@ -32,6 +32,7 @@
 #include <linux/sh_timer.h>
 #include <linux/slab.h>
 #include <linux/module.h>
+#include <linux/pm_domain.h>
 
 struct sh_tmu_priv {
 	void __iomem *mapbase;
@@ -410,6 +411,9 @@ static int __devinit sh_tmu_probe(struct
 	struct sh_tmu_priv *p = platform_get_drvdata(pdev);
 	int ret;
 
+	if (!is_early_platform_device(pdev))
+		pm_genpd_dev_always_on(&pdev->dev, true);
+
 	if (p) {
 		dev_info(&pdev->dev, "kept as earlytimer\n");
 		return 0;
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
@@ -1043,6 +1043,8 @@ void __init sh7372_add_standard_devices(
 	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);
+	sh7372_add_device_to_domain(&sh7372_a4r, &tmu00_device);
+	sh7372_add_device_to_domain(&sh7372_a4r, &tmu01_device);
 }
 
 void __init sh7372_add_early_devices(void)

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] sh_tmu / PM: Prevent power from being removed from TMU devices
  2012-03-02 23:41 [PATCH] sh_tmu / PM: Prevent power from being removed from TMU devices Rafael J. Wysocki
                   ` (6 preceding siblings ...)
  2012-03-05 22:46 ` Rafael J. Wysocki
@ 2012-03-06  2:07 ` Paul Mundt
  2012-03-06 22:13 ` Rafael J. Wysocki
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Paul Mundt @ 2012-03-06  2:07 UTC (permalink / raw)
  To: linux-sh

On Mon, Mar 05, 2012 at 11:46:08PM +0100, Rafael J. Wysocki wrote:
> On Monday, March 05, 2012, Rafael J. Wysocki wrote:
> > Anyway, to use the same approach in all three drivers we need a flag that
> > will allow a driver to say "don't power down the domain this device belongs to"
> > to the core.  It shouldn't be very difficult to introduce it, but I'm still
> > not sure that will make the "TMU vs system resume" problem mentioned above go
> > away.
> > 
> > I'll try to prepare a patch for that later today anyway.
> 
> Is appended.  Without a changelog for now, because I need to add CMT and
> MTU2 to it still.
> 
The updated version looks much nicer, and the interaction with early
platform entry is quite obvious. If we can reuse this for CMT and MTU2
then that should about take care of it. Thanks for persisting!

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] sh_tmu / PM: Prevent power from being removed from TMU devices
  2012-03-02 23:41 [PATCH] sh_tmu / PM: Prevent power from being removed from TMU devices Rafael J. Wysocki
                   ` (7 preceding siblings ...)
  2012-03-06  2:07 ` Paul Mundt
@ 2012-03-06 22:13 ` Rafael J. Wysocki
  2012-03-12  5:18 ` Simon Horman
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Rafael J. Wysocki @ 2012-03-06 22:13 UTC (permalink / raw)
  To: linux-sh

On Tuesday, March 06, 2012, Paul Mundt wrote:
> On Mon, Mar 05, 2012 at 11:46:08PM +0100, Rafael J. Wysocki wrote:
> > On Monday, March 05, 2012, Rafael J. Wysocki wrote:
> > > Anyway, to use the same approach in all three drivers we need a flag that
> > > will allow a driver to say "don't power down the domain this device belongs to"
> > > to the core.  It shouldn't be very difficult to introduce it, but I'm still
> > > not sure that will make the "TMU vs system resume" problem mentioned above go
> > > away.
> > > 
> > > I'll try to prepare a patch for that later today anyway.
> > 
> > Is appended.  Without a changelog for now, because I need to add CMT and
> > MTU2 to it still.
> > 
> The updated version looks much nicer, and the interaction with early
> platform entry is quite obvious. If we can reuse this for CMT and MTU2
> then that should about take care of it.

I believe that we can.

> Thanks for persisting!

No problem.

Below is a full patch with a changelog and CMT and MTU2 changes too.
This has been tested on Mackerel both with and without the TMU driver
without causing any visible issues to appear.

Thanks,
Rafael

---
From: Rafael J. Wysocki <rjw@sisk.pl>
Subject: PM / shmobile: Mark clocksource devices as "always on"

The TMU device on the Mackerel board belongs to the A4R power domain
and loses power when the domain is turned off.  Unfortunately, the
TMU driver is not prepared to cope with such situations and crashes
the system when that happens.  To work around this problem introduce
a new helper function, pm_genpd_dev_always_on(), allowing a device
driver to mark its device as "always on" in case it belongs to a PM
domain, which will make the generic PM domains core code avoid
powering off the domain containing the device, both at run time and
during system suspend.

Make the TMU driver use pm_genpd_dev_always_on() to prevent the
A4R domain from losing power and make the other SH colocksource
drivers, sh_cmt and sh_mtu2, behave analogously.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 arch/arm/mach-shmobile/setup-sh7372.c |    2 ++
 drivers/base/power/domain.c           |   28 ++++++++++++++++++++++++++--
 drivers/clocksource/sh_cmt.c          |    4 ++++
 drivers/clocksource/sh_mtu2.c         |    4 ++++
 drivers/clocksource/sh_tmu.c          |    4 ++++
 include/linux/pm_domain.h             |    3 +++
 6 files changed, 43 insertions(+), 2 deletions(-)

Index: linux/drivers/base/power/domain.c
=================================--- linux.orig/drivers/base/power/domain.c
+++ linux/drivers/base/power/domain.c
@@ -372,7 +372,7 @@ static int pm_genpd_poweroff(struct gene
 	not_suspended = 0;
 	list_for_each_entry(pdd, &genpd->dev_list, list_node)
 		if (pdd->dev->driver && (!pm_runtime_suspended(pdd->dev)
-		    || pdd->dev->power.irq_safe))
+		    || pdd->dev->power.irq_safe || to_gpd_data(pdd)->always_on))
 			not_suspended++;
 
 	if (not_suspended > genpd->in_progress)
@@ -509,6 +509,9 @@ static int pm_genpd_runtime_suspend(stru
 
 	might_sleep_if(!genpd->dev_irq_safe);
 
+	if (dev_gpd_data(dev)->always_on)
+		return -EBUSY;
+
 	stop_ok = genpd->gov ? genpd->gov->stop_ok : NULL;
 	if (stop_ok && !stop_ok(dev))
 		return -EBUSY;
@@ -864,7 +867,8 @@ static int pm_genpd_suspend_noirq(struct
 		return -EINVAL;
 
 	if (genpd->suspend_power_off
-	    || (dev->power.wakeup_path && genpd_dev_active_wakeup(genpd, dev)))
+	    || (dev->power.wakeup_path && genpd_dev_active_wakeup(genpd, dev))
+	    || dev_gpd_data(dev)->always_on)
 		return 0;
 
 	genpd_stop_dev(genpd, dev);
@@ -1281,6 +1285,26 @@ int pm_genpd_remove_device(struct generi
 }
 
 /**
+ * pm_genpd_dev_always_on - Set/unset the "always on" flag for a given device.
+ * @dev: Device to set/unset the flag for.
+ * @val: The new value of the device's "always on" flag.
+ */
+void pm_genpd_dev_always_on(struct device *dev, bool val)
+{
+	struct pm_subsys_data *psd;
+	unsigned long flags;
+
+	spin_lock_irqsave(&dev->power.lock, flags);
+
+	psd = dev_to_psd(dev);
+	if (psd && psd->domain_data)
+		to_gpd_data(psd->domain_data)->always_on = val;
+
+	spin_unlock_irqrestore(&dev->power.lock, flags);
+}
+EXPORT_SYMBOL_GPL(pm_genpd_dev_always_on);
+
+/**
  * pm_genpd_add_subdomain - Add a subdomain to an I/O PM domain.
  * @genpd: Master PM domain to add the subdomain to.
  * @subdomain: Subdomain to be added.
Index: linux/include/linux/pm_domain.h
=================================--- linux.orig/include/linux/pm_domain.h
+++ linux/include/linux/pm_domain.h
@@ -97,6 +97,7 @@ struct generic_pm_domain_data {
 	struct gpd_dev_ops ops;
 	struct gpd_timing_data td;
 	bool need_restore;
+	bool always_on;
 };
 
 #ifdef CONFIG_PM_GENERIC_DOMAINS
@@ -125,6 +126,7 @@ static inline int pm_genpd_add_device(st
 
 extern int pm_genpd_remove_device(struct generic_pm_domain *genpd,
 				  struct device *dev);
+extern void pm_genpd_dev_always_on(struct device *dev, bool val);
 extern int pm_genpd_add_subdomain(struct generic_pm_domain *genpd,
 				  struct generic_pm_domain *new_subdomain);
 extern int pm_genpd_remove_subdomain(struct generic_pm_domain *genpd,
@@ -167,6 +169,7 @@ static inline int pm_genpd_remove_device
 {
 	return -ENOSYS;
 }
+static inline void pm_genpd_dev_always_on(struct device *dev, bool val) {}
 static inline int pm_genpd_add_subdomain(struct generic_pm_domain *genpd,
 					 struct generic_pm_domain *new_sd)
 {
Index: linux/drivers/clocksource/sh_tmu.c
=================================--- linux.orig/drivers/clocksource/sh_tmu.c
+++ linux/drivers/clocksource/sh_tmu.c
@@ -32,6 +32,7 @@
 #include <linux/sh_timer.h>
 #include <linux/slab.h>
 #include <linux/module.h>
+#include <linux/pm_domain.h>
 
 struct sh_tmu_priv {
 	void __iomem *mapbase;
@@ -410,6 +411,9 @@ static int __devinit sh_tmu_probe(struct
 	struct sh_tmu_priv *p = platform_get_drvdata(pdev);
 	int ret;
 
+	if (!is_early_platform_device(pdev))
+		pm_genpd_dev_always_on(&pdev->dev, true);
+
 	if (p) {
 		dev_info(&pdev->dev, "kept as earlytimer\n");
 		return 0;
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
@@ -1043,6 +1043,8 @@ void __init sh7372_add_standard_devices(
 	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);
+	sh7372_add_device_to_domain(&sh7372_a4r, &tmu00_device);
+	sh7372_add_device_to_domain(&sh7372_a4r, &tmu01_device);
 }
 
 void __init sh7372_add_early_devices(void)
Index: linux/drivers/clocksource/sh_cmt.c
=================================--- linux.orig/drivers/clocksource/sh_cmt.c
+++ linux/drivers/clocksource/sh_cmt.c
@@ -32,6 +32,7 @@
 #include <linux/sh_timer.h>
 #include <linux/slab.h>
 #include <linux/module.h>
+#include <linux/pm_domain.h>
 
 struct sh_cmt_priv {
 	void __iomem *mapbase;
@@ -689,6 +690,9 @@ static int __devinit sh_cmt_probe(struct
 	struct sh_cmt_priv *p = platform_get_drvdata(pdev);
 	int ret;
 
+	if (!is_early_platform_device(pdev))
+		pm_genpd_dev_always_on(&pdev->dev, true);
+
 	if (p) {
 		dev_info(&pdev->dev, "kept as earlytimer\n");
 		return 0;
Index: linux/drivers/clocksource/sh_mtu2.c
=================================--- linux.orig/drivers/clocksource/sh_mtu2.c
+++ linux/drivers/clocksource/sh_mtu2.c
@@ -31,6 +31,7 @@
 #include <linux/sh_timer.h>
 #include <linux/slab.h>
 #include <linux/module.h>
+#include <linux/pm_domain.h>
 
 struct sh_mtu2_priv {
 	void __iomem *mapbase;
@@ -306,6 +307,9 @@ static int __devinit sh_mtu2_probe(struc
 	struct sh_mtu2_priv *p = platform_get_drvdata(pdev);
 	int ret;
 
+	if (!is_early_platform_device(pdev))
+		pm_genpd_dev_always_on(&pdev->dev, true);
+
 	if (p) {
 		dev_info(&pdev->dev, "kept as earlytimer\n");
 		return 0;

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] sh_tmu / PM: Prevent power from being removed from TMU devices
  2012-03-02 23:41 [PATCH] sh_tmu / PM: Prevent power from being removed from TMU devices Rafael J. Wysocki
                   ` (8 preceding siblings ...)
  2012-03-06 22:13 ` Rafael J. Wysocki
@ 2012-03-12  5:18 ` Simon Horman
  2012-03-12  5:23 ` Paul Mundt
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Simon Horman @ 2012-03-12  5:18 UTC (permalink / raw)
  To: linux-sh

On Tue, Mar 06, 2012 at 11:13:10PM +0100, Rafael J. Wysocki wrote:
> On Tuesday, March 06, 2012, Paul Mundt wrote:
> > On Mon, Mar 05, 2012 at 11:46:08PM +0100, Rafael J. Wysocki wrote:
> > > On Monday, March 05, 2012, Rafael J. Wysocki wrote:
> > > > Anyway, to use the same approach in all three drivers we need a flag that
> > > > will allow a driver to say "don't power down the domain this device belongs to"
> > > > to the core.  It shouldn't be very difficult to introduce it, but I'm still
> > > > not sure that will make the "TMU vs system resume" problem mentioned above go
> > > > away.
> > > > 
> > > > I'll try to prepare a patch for that later today anyway.
> > > 
> > > Is appended.  Without a changelog for now, because I need to add CMT and
> > > MTU2 to it still.
> > > 
> > The updated version looks much nicer, and the interaction with early
> > platform entry is quite obvious. If we can reuse this for CMT and MTU2
> > then that should about take care of it.
> 
> I believe that we can.
> 
> > Thanks for persisting!
> 
> No problem.
> 
> Below is a full patch with a changelog and CMT and MTU2 changes too.
> This has been tested on Mackerel both with and without the TMU driver
> without causing any visible issues to appear.

Hi Rafael,

I apologise for not responding earlier, I returned from a vacation
(without my Mackerel) last night,

I have tried the patch below (slightly modified as I have detailed inline)
on top of 3.3-rc7, however, I am observing that the boot hangs in the
usual spot when using the default config.

arm_vmregion_alloc: allocation too big (requested 0x7e9000)
sh_mobile_lcdc_fb sh_mobile_lcdc_fb.1: unable to allocate buffer
sh_mobile_lcdc_fb: probe of sh_mobile_lcdc_fb.1 failed with error -12
SuperH SCI(F) driver initialized
sh-sci.0: ttySC0 at MMIO 0xe6c40000 (irq = 80) is a scifa
sh-sci sh-sci.0: start latency exceeded, new value 6084 ns
console [ttySC0] enabled, bootconsole disabled
console [ttySC0] enabled, bootconsole disabled


> From: Rafael J. Wysocki <rjw@sisk.pl>
> Subject: PM / shmobile: Mark clocksource devices as "always on"
> 
> The TMU device on the Mackerel board belongs to the A4R power domain
> and loses power when the domain is turned off.  Unfortunately, the
> TMU driver is not prepared to cope with such situations and crashes
> the system when that happens.  To work around this problem introduce
> a new helper function, pm_genpd_dev_always_on(), allowing a device
> driver to mark its device as "always on" in case it belongs to a PM
> domain, which will make the generic PM domains core code avoid
> powering off the domain containing the device, both at run time and
> during system suspend.
> 
> Make the TMU driver use pm_genpd_dev_always_on() to prevent the
> A4R domain from losing power and make the other SH colocksource
> drivers, sh_cmt and sh_mtu2, behave analogously.
> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> ---
>  arch/arm/mach-shmobile/setup-sh7372.c |    2 ++
>  drivers/base/power/domain.c           |   28 ++++++++++++++++++++++++++--
>  drivers/clocksource/sh_cmt.c          |    4 ++++
>  drivers/clocksource/sh_mtu2.c         |    4 ++++
>  drivers/clocksource/sh_tmu.c          |    4 ++++
>  include/linux/pm_domain.h             |    3 +++
>  6 files changed, 43 insertions(+), 2 deletions(-)
> 
> Index: linux/drivers/base/power/domain.c
> =================================> --- linux.orig/drivers/base/power/domain.c
> +++ linux/drivers/base/power/domain.c
> @@ -372,7 +372,7 @@ static int pm_genpd_poweroff(struct gene
>  	not_suspended = 0;
>  	list_for_each_entry(pdd, &genpd->dev_list, list_node)
>  		if (pdd->dev->driver && (!pm_runtime_suspended(pdd->dev)
> -		    || pdd->dev->power.irq_safe))
> +		    || pdd->dev->power.irq_safe || to_gpd_data(pdd)->always_on))
>  			not_suspended++;
>  
>  	if (not_suspended > genpd->in_progress)
> @@ -509,6 +509,9 @@ static int pm_genpd_runtime_suspend(stru
>  
>  	might_sleep_if(!genpd->dev_irq_safe);
>  
> +	if (dev_gpd_data(dev)->always_on)
> +		return -EBUSY;
> +
>  	stop_ok = genpd->gov ? genpd->gov->stop_ok : NULL;
>  	if (stop_ok && !stop_ok(dev))
>  		return -EBUSY;
> @@ -864,7 +867,8 @@ static int pm_genpd_suspend_noirq(struct
>  		return -EINVAL;
>  
>  	if (genpd->suspend_power_off
> -	    || (dev->power.wakeup_path && genpd_dev_active_wakeup(genpd, dev)))
> +	    || (dev->power.wakeup_path && genpd_dev_active_wakeup(genpd, dev))
> +	    || dev_gpd_data(dev)->always_on)
>  		return 0;
>  
>  	genpd_stop_dev(genpd, dev);

I modified the above hunk as follows as the genpd_dev_active_wakeup()
line does not seem to be present in pm_genpd_runtime_suspend().
Perhaps I need another patch?


@@ -880,7 +883,7 @@ static int pm_genpd_resume_noirq(struct device *dev)
        if (IS_ERR(genpd))
                return -EINVAL;
 
-       if (genpd->suspend_power_off)
+       if (genpd->suspend_power_off || dev_gpd_data(dev)->always_on)
                return 0;
 
        /*

> @@ -1281,6 +1285,26 @@ int pm_genpd_remove_device(struct generi
>  }
>  
>  /**
> + * pm_genpd_dev_always_on - Set/unset the "always on" flag for a given device.
> + * @dev: Device to set/unset the flag for.
> + * @val: The new value of the device's "always on" flag.
> + */
> +void pm_genpd_dev_always_on(struct device *dev, bool val)
> +{
> +	struct pm_subsys_data *psd;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&dev->power.lock, flags);
> +
> +	psd = dev_to_psd(dev);
> +	if (psd && psd->domain_data)
> +		to_gpd_data(psd->domain_data)->always_on = val;
> +
> +	spin_unlock_irqrestore(&dev->power.lock, flags);
> +}
> +EXPORT_SYMBOL_GPL(pm_genpd_dev_always_on);
> +
> +/**
>   * pm_genpd_add_subdomain - Add a subdomain to an I/O PM domain.
>   * @genpd: Master PM domain to add the subdomain to.
>   * @subdomain: Subdomain to be added.
> Index: linux/include/linux/pm_domain.h
> =================================> --- linux.orig/include/linux/pm_domain.h
> +++ linux/include/linux/pm_domain.h
> @@ -97,6 +97,7 @@ struct generic_pm_domain_data {
>  	struct gpd_dev_ops ops;
>  	struct gpd_timing_data td;
>  	bool need_restore;
> +	bool always_on;
>  };
>  
>  #ifdef CONFIG_PM_GENERIC_DOMAINS
> @@ -125,6 +126,7 @@ static inline int pm_genpd_add_device(st
>  
>  extern int pm_genpd_remove_device(struct generic_pm_domain *genpd,
>  				  struct device *dev);
> +extern void pm_genpd_dev_always_on(struct device *dev, bool val);
>  extern int pm_genpd_add_subdomain(struct generic_pm_domain *genpd,
>  				  struct generic_pm_domain *new_subdomain);
>  extern int pm_genpd_remove_subdomain(struct generic_pm_domain *genpd,
> @@ -167,6 +169,7 @@ static inline int pm_genpd_remove_device
>  {
>  	return -ENOSYS;
>  }
> +static inline void pm_genpd_dev_always_on(struct device *dev, bool val) {}
>  static inline int pm_genpd_add_subdomain(struct generic_pm_domain *genpd,
>  					 struct generic_pm_domain *new_sd)
>  {
> Index: linux/drivers/clocksource/sh_tmu.c
> =================================> --- linux.orig/drivers/clocksource/sh_tmu.c
> +++ linux/drivers/clocksource/sh_tmu.c
> @@ -32,6 +32,7 @@
>  #include <linux/sh_timer.h>
>  #include <linux/slab.h>
>  #include <linux/module.h>
> +#include <linux/pm_domain.h>
>  
>  struct sh_tmu_priv {
>  	void __iomem *mapbase;
> @@ -410,6 +411,9 @@ static int __devinit sh_tmu_probe(struct
>  	struct sh_tmu_priv *p = platform_get_drvdata(pdev);
>  	int ret;
>  
> +	if (!is_early_platform_device(pdev))
> +		pm_genpd_dev_always_on(&pdev->dev, true);
> +
>  	if (p) {
>  		dev_info(&pdev->dev, "kept as earlytimer\n");
>  		return 0;
> 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
> @@ -1043,6 +1043,8 @@ void __init sh7372_add_standard_devices(
>  	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);
> +	sh7372_add_device_to_domain(&sh7372_a4r, &tmu00_device);
> +	sh7372_add_device_to_domain(&sh7372_a4r, &tmu01_device);
>  }
>  
>  void __init sh7372_add_early_devices(void)
> Index: linux/drivers/clocksource/sh_cmt.c
> =================================> --- linux.orig/drivers/clocksource/sh_cmt.c
> +++ linux/drivers/clocksource/sh_cmt.c
> @@ -32,6 +32,7 @@
>  #include <linux/sh_timer.h>
>  #include <linux/slab.h>
>  #include <linux/module.h>
> +#include <linux/pm_domain.h>
>  
>  struct sh_cmt_priv {
>  	void __iomem *mapbase;
> @@ -689,6 +690,9 @@ static int __devinit sh_cmt_probe(struct
>  	struct sh_cmt_priv *p = platform_get_drvdata(pdev);
>  	int ret;
>  
> +	if (!is_early_platform_device(pdev))
> +		pm_genpd_dev_always_on(&pdev->dev, true);
> +
>  	if (p) {
>  		dev_info(&pdev->dev, "kept as earlytimer\n");
>  		return 0;
> Index: linux/drivers/clocksource/sh_mtu2.c
> =================================> --- linux.orig/drivers/clocksource/sh_mtu2.c
> +++ linux/drivers/clocksource/sh_mtu2.c
> @@ -31,6 +31,7 @@
>  #include <linux/sh_timer.h>
>  #include <linux/slab.h>
>  #include <linux/module.h>
> +#include <linux/pm_domain.h>
>  
>  struct sh_mtu2_priv {
>  	void __iomem *mapbase;
> @@ -306,6 +307,9 @@ static int __devinit sh_mtu2_probe(struc
>  	struct sh_mtu2_priv *p = platform_get_drvdata(pdev);
>  	int ret;
>  
> +	if (!is_early_platform_device(pdev))
> +		pm_genpd_dev_always_on(&pdev->dev, true);
> +
>  	if (p) {
>  		dev_info(&pdev->dev, "kept as earlytimer\n");
>  		return 0;
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] sh_tmu / PM: Prevent power from being removed from TMU devices
  2012-03-02 23:41 [PATCH] sh_tmu / PM: Prevent power from being removed from TMU devices Rafael J. Wysocki
                   ` (9 preceding siblings ...)
  2012-03-12  5:18 ` Simon Horman
@ 2012-03-12  5:23 ` Paul Mundt
  2012-03-12  8:06 ` Simon Horman
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Paul Mundt @ 2012-03-12  5:23 UTC (permalink / raw)
  To: linux-sh

On Mon, Mar 12, 2012 at 02:18:45PM +0900, Simon Horman wrote:
> I have tried the patch below (slightly modified as I have detailed inline)
> on top of 3.3-rc7, however, I am observing that the boot hangs in the
> usual spot when using the default config.
> 
> arm_vmregion_alloc: allocation too big (requested 0x7e9000)
> sh_mobile_lcdc_fb sh_mobile_lcdc_fb.1: unable to allocate buffer
> sh_mobile_lcdc_fb: probe of sh_mobile_lcdc_fb.1 failed with error -12
> SuperH SCI(F) driver initialized
> sh-sci.0: ttySC0 at MMIO 0xe6c40000 (irq = 80) is a scifa
> sh-sci sh-sci.0: start latency exceeded, new value 6084 ns
> console [ttySC0] enabled, bootconsole disabled
> console [ttySC0] enabled, bootconsole disabled
> 
You will probably want to use the rmobile-fixes-for-linus branch, as it
contains:

commit 1740d3448012475f6b63172631c60cbcd1994a81
Author: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Date:   Thu Mar 1 15:45:16 2012 +0100

    ARM: mach-shmobile: mackerel: Reserve DMA memory for the frame buffer

    The default 2MB size of DMA coherent memory isn't enough for allocate
    frame buffer memory.

    Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
    Signed-off-by: Paul Mundt <lethal@linux-sh.org>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] sh_tmu / PM: Prevent power from being removed from TMU devices
  2012-03-02 23:41 [PATCH] sh_tmu / PM: Prevent power from being removed from TMU devices Rafael J. Wysocki
                   ` (10 preceding siblings ...)
  2012-03-12  5:23 ` Paul Mundt
@ 2012-03-12  8:06 ` Simon Horman
  2012-03-12 21:37 ` Rafael J. Wysocki
  2012-03-12 21:54 ` Rafael J. Wysocki
  13 siblings, 0 replies; 15+ messages in thread
From: Simon Horman @ 2012-03-12  8:06 UTC (permalink / raw)
  To: linux-sh

On Mon, Mar 12, 2012 at 02:23:37PM +0900, Paul Mundt wrote:
> On Mon, Mar 12, 2012 at 02:18:45PM +0900, Simon Horman wrote:
> > I have tried the patch below (slightly modified as I have detailed inline)
> > on top of 3.3-rc7, however, I am observing that the boot hangs in the
> > usual spot when using the default config.
> > 
> > arm_vmregion_alloc: allocation too big (requested 0x7e9000)
> > sh_mobile_lcdc_fb sh_mobile_lcdc_fb.1: unable to allocate buffer
> > sh_mobile_lcdc_fb: probe of sh_mobile_lcdc_fb.1 failed with error -12
> > SuperH SCI(F) driver initialized
> > sh-sci.0: ttySC0 at MMIO 0xe6c40000 (irq = 80) is a scifa
> > sh-sci sh-sci.0: start latency exceeded, new value 6084 ns
> > console [ttySC0] enabled, bootconsole disabled
> > console [ttySC0] enabled, bootconsole disabled
> > 
> You will probably want to use the rmobile-fixes-for-linus branch, as it
> contains:
> 
> commit 1740d3448012475f6b63172631c60cbcd1994a81
> Author: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Date:   Thu Mar 1 15:45:16 2012 +0100
> 
>     ARM: mach-shmobile: mackerel: Reserve DMA memory for the frame buffer
> 
>     The default 2MB size of DMA coherent memory isn't enough for allocate
>     frame buffer memory.
> 
>     Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>     Signed-off-by: Paul Mundt <lethal@linux-sh.org>
> 

Unfortunately not.

I still had to modify the patch as per my previous post in order to apply it
and the boot still hung.


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] sh_tmu / PM: Prevent power from being removed from TMU devices
  2012-03-02 23:41 [PATCH] sh_tmu / PM: Prevent power from being removed from TMU devices Rafael J. Wysocki
                   ` (11 preceding siblings ...)
  2012-03-12  8:06 ` Simon Horman
@ 2012-03-12 21:37 ` Rafael J. Wysocki
  2012-03-12 21:54 ` Rafael J. Wysocki
  13 siblings, 0 replies; 15+ messages in thread
From: Rafael J. Wysocki @ 2012-03-12 21:37 UTC (permalink / raw)
  To: linux-sh

On Monday, March 12, 2012, Simon Horman wrote:
> On Tue, Mar 06, 2012 at 11:13:10PM +0100, Rafael J. Wysocki wrote:
> > On Tuesday, March 06, 2012, Paul Mundt wrote:
> > > On Mon, Mar 05, 2012 at 11:46:08PM +0100, Rafael J. Wysocki wrote:
> > > > On Monday, March 05, 2012, Rafael J. Wysocki wrote:
> > > > > Anyway, to use the same approach in all three drivers we need a flag that
> > > > > will allow a driver to say "don't power down the domain this device belongs to"
> > > > > to the core.  It shouldn't be very difficult to introduce it, but I'm still
> > > > > not sure that will make the "TMU vs system resume" problem mentioned above go
> > > > > away.
> > > > > 
> > > > > I'll try to prepare a patch for that later today anyway.
> > > > 
> > > > Is appended.  Without a changelog for now, because I need to add CMT and
> > > > MTU2 to it still.
> > > > 
> > > The updated version looks much nicer, and the interaction with early
> > > platform entry is quite obvious. If we can reuse this for CMT and MTU2
> > > then that should about take care of it.
> > 
> > I believe that we can.
> > 
> > > Thanks for persisting!
> > 
> > No problem.
> > 
> > Below is a full patch with a changelog and CMT and MTU2 changes too.
> > This has been tested on Mackerel both with and without the TMU driver
> > without causing any visible issues to appear.
> 
> Hi Rafael,
> 
> I apologise for not responding earlier, I returned from a vacation
> (without my Mackerel) last night,
> 
> I have tried the patch below (slightly modified as I have detailed inline)
> on top of 3.3-rc7, however, I am observing that the boot hangs in the
> usual spot when using the default config.
> 
> arm_vmregion_alloc: allocation too big (requested 0x7e9000)
> sh_mobile_lcdc_fb sh_mobile_lcdc_fb.1: unable to allocate buffer
> sh_mobile_lcdc_fb: probe of sh_mobile_lcdc_fb.1 failed with error -12
> SuperH SCI(F) driver initialized
> sh-sci.0: ttySC0 at MMIO 0xe6c40000 (irq = 80) is a scifa
> sh-sci sh-sci.0: start latency exceeded, new value 6084 ns
> console [ttySC0] enabled, bootconsole disabled
> console [ttySC0] enabled, bootconsole disabled

Can you please send me your .config?

> > From: Rafael J. Wysocki <rjw@sisk.pl>
> > Subject: PM / shmobile: Mark clocksource devices as "always on"
> > 
> > The TMU device on the Mackerel board belongs to the A4R power domain
> > and loses power when the domain is turned off.  Unfortunately, the
> > TMU driver is not prepared to cope with such situations and crashes
> > the system when that happens.  To work around this problem introduce
> > a new helper function, pm_genpd_dev_always_on(), allowing a device
> > driver to mark its device as "always on" in case it belongs to a PM
> > domain, which will make the generic PM domains core code avoid
> > powering off the domain containing the device, both at run time and
> > during system suspend.
> > 
> > Make the TMU driver use pm_genpd_dev_always_on() to prevent the
> > A4R domain from losing power and make the other SH colocksource
> > drivers, sh_cmt and sh_mtu2, behave analogously.
> > 
> > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> > ---
> >  arch/arm/mach-shmobile/setup-sh7372.c |    2 ++
> >  drivers/base/power/domain.c           |   28 ++++++++++++++++++++++++++--
> >  drivers/clocksource/sh_cmt.c          |    4 ++++
> >  drivers/clocksource/sh_mtu2.c         |    4 ++++
> >  drivers/clocksource/sh_tmu.c          |    4 ++++
> >  include/linux/pm_domain.h             |    3 +++
> >  6 files changed, 43 insertions(+), 2 deletions(-)
> > 
> > Index: linux/drivers/base/power/domain.c
> > =================================> > --- linux.orig/drivers/base/power/domain.c
> > +++ linux/drivers/base/power/domain.c
> > @@ -372,7 +372,7 @@ static int pm_genpd_poweroff(struct gene
> >  	not_suspended = 0;
> >  	list_for_each_entry(pdd, &genpd->dev_list, list_node)
> >  		if (pdd->dev->driver && (!pm_runtime_suspended(pdd->dev)
> > -		    || pdd->dev->power.irq_safe))
> > +		    || pdd->dev->power.irq_safe || to_gpd_data(pdd)->always_on))
> >  			not_suspended++;
> >  
> >  	if (not_suspended > genpd->in_progress)
> > @@ -509,6 +509,9 @@ static int pm_genpd_runtime_suspend(stru
> >  
> >  	might_sleep_if(!genpd->dev_irq_safe);
> >  
> > +	if (dev_gpd_data(dev)->always_on)
> > +		return -EBUSY;
> > +
> >  	stop_ok = genpd->gov ? genpd->gov->stop_ok : NULL;
> >  	if (stop_ok && !stop_ok(dev))
> >  		return -EBUSY;
> > @@ -864,7 +867,8 @@ static int pm_genpd_suspend_noirq(struct
> >  		return -EINVAL;
> >  
> >  	if (genpd->suspend_power_off
> > -	    || (dev->power.wakeup_path && genpd_dev_active_wakeup(genpd, dev)))
> > +	    || (dev->power.wakeup_path && genpd_dev_active_wakeup(genpd, dev))
> > +	    || dev_gpd_data(dev)->always_on)
> >  		return 0;
> >  
> >  	genpd_stop_dev(genpd, dev);
> 
> I modified the above hunk as follows as the genpd_dev_active_wakeup()
> line does not seem to be present in pm_genpd_runtime_suspend().
> Perhaps I need another patch?

They are present in pm_genpd_suspend_noirq() surely in -rc7.  What kernel
version are you using?

Rafael

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] sh_tmu / PM: Prevent power from being removed from TMU devices
  2012-03-02 23:41 [PATCH] sh_tmu / PM: Prevent power from being removed from TMU devices Rafael J. Wysocki
                   ` (12 preceding siblings ...)
  2012-03-12 21:37 ` Rafael J. Wysocki
@ 2012-03-12 21:54 ` Rafael J. Wysocki
  13 siblings, 0 replies; 15+ messages in thread
From: Rafael J. Wysocki @ 2012-03-12 21:54 UTC (permalink / raw)
  To: linux-sh

On Monday, March 12, 2012, Simon Horman wrote:
> On Tue, Mar 06, 2012 at 11:13:10PM +0100, Rafael J. Wysocki wrote:
> > On Tuesday, March 06, 2012, Paul Mundt wrote:
> > > On Mon, Mar 05, 2012 at 11:46:08PM +0100, Rafael J. Wysocki wrote:
> > > > On Monday, March 05, 2012, Rafael J. Wysocki wrote:
> > > > > Anyway, to use the same approach in all three drivers we need a flag that
> > > > > will allow a driver to say "don't power down the domain this device belongs to"
> > > > > to the core.  It shouldn't be very difficult to introduce it, but I'm still
> > > > > not sure that will make the "TMU vs system resume" problem mentioned above go
> > > > > away.
> > > > > 
> > > > > I'll try to prepare a patch for that later today anyway.
> > > > 
> > > > Is appended.  Without a changelog for now, because I need to add CMT and
> > > > MTU2 to it still.
> > > > 
> > > The updated version looks much nicer, and the interaction with early
> > > platform entry is quite obvious. If we can reuse this for CMT and MTU2
> > > then that should about take care of it.
> > 
> > I believe that we can.
> > 
> > > Thanks for persisting!
> > 
> > No problem.
> > 
> > Below is a full patch with a changelog and CMT and MTU2 changes too.
> > This has been tested on Mackerel both with and without the TMU driver
> > without causing any visible issues to appear.
> 
> Hi Rafael,
> 
> I apologise for not responding earlier, I returned from a vacation
> (without my Mackerel) last night,
> 
> I have tried the patch below (slightly modified as I have detailed inline)
> on top of 3.3-rc7, however, I am observing that the boot hangs in the
> usual spot when using the default config.

First off, the patch applies to 3.3-rc7 without any modifications for me.

> arm_vmregion_alloc: allocation too big (requested 0x7e9000)
> sh_mobile_lcdc_fb sh_mobile_lcdc_fb.1: unable to allocate buffer
> sh_mobile_lcdc_fb: probe of sh_mobile_lcdc_fb.1 failed with error -12
> SuperH SCI(F) driver initialized
> sh-sci.0: ttySC0 at MMIO 0xe6c40000 (irq = 80) is a scifa
> sh-sci sh-sci.0: start latency exceeded, new value 6084 ns
> console [ttySC0] enabled, bootconsole disabled
> console [ttySC0] enabled, bootconsole disabled

I've just tried the default .config for Mackerel and I cannot reproduce the
problem with the patch applied.

Can you double-check, please?

Thanks,
Rafael

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2012-03-12 21:54 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-03-02 23:41 [PATCH] sh_tmu / PM: Prevent power from being removed from TMU devices Rafael J. Wysocki
2012-03-04 21:03 ` Paul Mundt
2012-03-04 21:50 ` Rafael J. Wysocki
2012-03-05  5:47 ` Paul Mundt
2012-03-05  8:01 ` Magnus Damm
2012-03-05 17:29 ` Paul Mundt
2012-03-05 20:32 ` Rafael J. Wysocki
2012-03-05 22:46 ` Rafael J. Wysocki
2012-03-06  2:07 ` Paul Mundt
2012-03-06 22:13 ` Rafael J. Wysocki
2012-03-12  5:18 ` Simon Horman
2012-03-12  5:23 ` Paul Mundt
2012-03-12  8:06 ` Simon Horman
2012-03-12 21:37 ` Rafael J. Wysocki
2012-03-12 21:54 ` Rafael J. Wysocki

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.