Re: [PATCH] sh_tmu / PM: Prevent power from being removed from TMU devices

["Rafael J. Wysocki" <rjw@sisk.pl>]

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