From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Rafael J. Wysocki" Subject: Re: use of pm_runtime_disable() from driver probe? Date: Thu, 12 Jul 2012 01:16:00 +0200 Message-ID: <201207120116.00862.rjw@sisk.pl> References: <201207111936.45639.rjw@sisk.pl> <87ipdthp6b.fsf@ti.com> Mime-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <87ipdthp6b.fsf@ti.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-pm-bounces@lists.linux-foundation.org Errors-To: linux-pm-bounces@lists.linux-foundation.org To: Kevin Hilman Cc: linux-pm@lists.linux-foundation.org List-Id: linux-pm@vger.kernel.org On Thursday, July 12, 2012, Kevin Hilman wrote: > "Rafael J. Wysocki" writes: > > > On Wednesday, July 11, 2012, Alan Stern wrote: > >> On Wed, 11 Jul 2012, Rafael J. Wysocki wrote: > >> > >> > > This opens up the possibility of calling probe while a runtime resume > >> > > or suspend is in progress. (On the other hand, the existing code > >> > > doesn't prevent a concurrent runtime resume.) Maybe it would be best > >> > > to leave the pm_runtime_barrier(). > >> > > >> > That wouldn't close the race, though, because the suspend/resume may still > >> > be started right after _barrier has returned. > >> > >> True, but see below. > >> > >> > The race is only possible if runtime PM is enabled by a subsystem or PM domain > >> > code before the first eligible driver is registered and if that code is not > >> > careful enough to get ready for driver registration. I'm not sure how likely > >> > it is to happen in practice. > >> > >> It's not just the first eligible driver. Drivers can be bound and > >> unbound dynamically, and suspends/resume operations can sit on the wait > >> queue or wait until a timer expires. We don't want an old request > >> suddenly to take effect in the middle of a probe. > >> > >> The barrier will get rid of any old requests. New ones would have to > >> be added after the probe starts, which as you say, is unlikely. > > > > OK, I'll keep the barrier, then. > > > > Kevin, can you please double check the patch below? > > Yup, this verion looks right and I tested it and verified that it still > fixes the problem I'm seeing. > > Reviewed-by: Kevin Hilman > Tested-by: Kevin Hilman Cool, thanks! I'll submit it officially (with a changelog and tags) tomorrow. Rafael > > --- > > drivers/base/dd.c | 6 ++---- > > 1 file changed, 2 insertions(+), 4 deletions(-) > > > > Index: linux/drivers/base/dd.c > > =================================================================== > > --- linux.orig/drivers/base/dd.c > > +++ linux/drivers/base/dd.c > > @@ -356,10 +356,9 @@ int driver_probe_device(struct device_dr > > pr_debug("bus: '%s': %s: matched device %s with driver %s\n", > > drv->bus->name, __func__, dev_name(dev), drv->name); > > > > - pm_runtime_get_noresume(dev); > > pm_runtime_barrier(dev); > > ret = really_probe(dev, drv); > > - pm_runtime_put_sync(dev); > > + pm_runtime_idle(dev); > > > > return ret; > > } > > @@ -406,9 +405,8 @@ int device_attach(struct device *dev) > > ret = 0; > > } > > } else { > > - pm_runtime_get_noresume(dev); > > ret = bus_for_each_drv(dev->bus, NULL, dev, __device_attach); > > - pm_runtime_put_sync(dev); > > + pm_runtime_idle(dev); > > } > > out_unlock: > > device_unlock(dev); > >