From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Rafael J. Wysocki" Date: Mon, 12 Mar 2012 21:37:53 +0000 Subject: Re: [PATCH] sh_tmu / PM: Prevent power from being removed from TMU devices Message-Id: <201203122237.53636.rjw@sisk.pl> List-Id: References: <201203030041.30244.rjw@sisk.pl> In-Reply-To: <201203030041.30244.rjw@sisk.pl> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: linux-sh@vger.kernel.org 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 > > 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 > > --- > > 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