On 3 February 2016 at 17:09, Tony Lindgren [off-list ref] wrote:

* Alan Stern [off-list ref] [160203 07:46]:

quoted

On Wed, 3 Feb 2016, Ulf Hansson wrote:

quoted

quoted

Let me summarize my understanding of this thread so far.

It looks like the omap3 code initializes hardware in ->probe() and
then it may return -EPROBE_DEFER due to some unmet dependencies.  In
that case the hardware is not reset to the previous state and the
runtime PM framework is left in the state that corresponds to the
current hardware state.  Before we had pm_runtime_reinit(), everything
worked as expected on the second ->probe() call, because things were
in sync then.

Correct!

Well not quite correct. After failed probe PM runtime is set to reset
state while hardware is still enabled.

Yes, but that's *after* pm_runtime_reinit() was added.

I think Rafael was thinking about how it worked *before*.

quoted

quoted

Before pm_runtime_reinit(), the failing probe case (-EPROBE_DEFER)
worked fine because the HW state and the runtime PM status was in
sync. The device was powered and the runtime PM status was RPM_ACTIVE.

quoted

The introduction of pm_runtime_reinit() changed the situation and now
effectively the hardware is required to be reset to the initial state
if -EPROBE_DEFER is to be returned too.

Not correct. The hardware doesn't need a reset as it stays powered
after a failed probe.

It is really best to disable the hardware after a failed probe
like we do with pm_runtime_put_sync_(suspend)() and pm_runtime_disable().

This is because there may never be another probe attempt and we want
to have unclaimed devices shut off (or idled) for PM.

I totally agree!

quoted

quoted

Instead, only the runtime PM status needs to be synchronized with the
HW state the next probe attempt.

In other words, the probe routine assumes the actual state is the same
as the PM status.  This may have been true before pm_runtime_reinit()
came along, but it's not true now.

quoted

In other words, when the PM domain's ->runtime_resume() callbacks gets
called due to a pm_runtime_get_sync() in the second probe attempt, it
may find that the HW is already powered and can return zero instead of
resetting it.

Certainly that should at least produce a warning in the hardware
specific implementation. It's a state mismatch between PM runtime
and the hardware specific implementation.

And as discussed, the problem does not exist as long as we understand
that we need to use pm_runtime_put_sync_suspend() if the driver has
set pm_runtime_use_autoidle(). Or else use the combination of
pm_runtime_dont_use_autoidle() and pm_runtime_put_sync().

Okay, so I understand that you decided to not pick up the omap hwmod
patch I posted.

If you want some further help in fixing the omap drivers, please just
tell me I am at your service. :-)

Also, we must not forget to also update their runtime PM calls in
their ->remove() callbacks, as those seems to suffer from the same
problem as in the -EPROBE_DEFER case.

quoted

What's wrong with going ahead and resetting the hardware anyway?

Nothing, unless a state needs to be maintained for things like GPIO
pins power memory :)

Regards,

Tony

Kind regards
Uffe