[PATCH v3 0/9] PM / Domains: Fix race conditions during boot

[Rafael J. Wysocki]

On Tuesday, November 04, 2014 09:54:19 AM Ulf Hansson wrote:
> [...]
> 
> > Generally, there are two or even three levels of runtime PM handling,
> > driver, (possibly) bus type and (possibly) PM domain (and multiple levels
> > of these are possible in principle).  All of them have to be initialized
> > at different times.
> >
> > Quite arguably, the PM domain and/or bus type runtime PM handling should
> > be initialized even before registerind the device or during device
> > registration.  Doing that later may be too late.  When the device has been
> > registered, runtime PM should work to an extent allowing the driver to access
> > the device and configure it further after calling pm_runtime_resume().
> >
> > Of course, if ->probe() is to call pm_runtime_resume() for this purpose,
> > it must take the fact that the driver's own ->runtime_resume() may be called
> > as a result of this into account.  That's why I'm asking whether or not the
> > core should call pm_runtime_resume() before calling really_probe() in a
> > followup branch of this thread.
> 
> I am reading the other thread, let's see.
> 
> >
> > The driver's own runtime PM handling must be initialized in the driver and
> > the only place suitable for that is ->probe().  However, it needs to be done
> > *before* the driver's own ->runtime_resume() or ->runtime_suspend() callback
> > is executed.  If that is done properly, it should be possible to cover
> > both the CONFIG_PM_RUNTIME set/unset cases in that code.
> >
> > And I wouldn't recommend anyone to do the runtime PM initialization in
> > ->runtime_resume() (when it is called for the first time), as that would be
> > error prone and fragile.
> 
> Great! That's means we are at least aligned on this topic. :-)
> 
> >
> >> The AMBA bus and some of its drivers a good example of how this has
> >> been implemented:
> >> driver/amba/bus.c
> >> drivers/mmc/host/mmci.c
> >> drivers/spi/spi-pl022.c
> >>
> >> This conclusion I have made from this is:
> >> - Using pm_runtime_get_sync() during the ->probe() path to explicitly
> >> power up a PM domain, is not suitable as the _common_ solution to
> >> solve the race condition. It certainly may work for some scenarios,
> >> but not for those I am looking at.
> >
> > I think, however, that it might work if the core calls pm_runtime_get_sync()
> > from driver_probe_device().
> 
> Currently this won't work.
> 
> That's because the buses' ->probe() are invoked in this path and they
> are doing the attachment of the device to its PM domain.
> 
> In other words, we can't power up the PM domain using
> pm_runtime_get_sync(), until the device has been attached to its PM
> domain. Right?

Yes, but my point was that those bus types might need to be changed.

We can't make everyone happy at the same time if their ideas about what to do
are different.

> 
> [...]
> 
> >> For PM domains that are initialized in powered off state, we can't
> >> rely on CONFIG_PM_RUNTIME and thus not on pm_runtime_get_sync() to
> >> power on these PM domains. We need a different mechanism, which is
> >> suggested in this v3 patchset.
> >
> > That is quite simple to address, though.  You can register a bus type
> > notifier that will power up the domain on BUS_NOTIFY_ADD_DEVICE events
> > (where the target device belongs to the domain), and do that only for
> > CONFIG_PM_RUNTIME unset (otherwise runtime PM should take care of this).
> 
> I guess we could use notifiers, but I am not sure I see any benefit.
> The code will be more complex and we need error handling as well.

Like "oh, I can't power up this thing, so I should fail ->probe()"?

Then your driver would need to depend on the specific knowledge about the
given PM domain, I'm afraid.

If you want error handling like that, it needs to be handled by the core,
so as to avoid calling the bus type's ->probe() as well in that case.

So to summarize:

- Devices need to be added to power domains before really_probe() is called
  for them.  Otherwise we'll have ordering problems all over.

- Runtime PM (if compiled in) needs to be enabled for all devices in power
  domains by default.  Otherwise devices may lose power as a result for
  power management of the other devices in the same domain.

- The core should try to power up domains before calling really_probe() both
  for CONFIG_PM_RUNTIME set and unset, so ->probe() can always make the
  "device is accessible" assumption.

- Bus types may need to do more on top of that in their ->probe(), so the
  driver's ->probe() can make that assumption too in all cases.

Does that make sense to you?

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.