Hi Rafael,

On Thursday 14 June 2012 21:37:26 Rafael J. Wysocki wrote:

On Thursday, June 14, 2012, Laurent Pinchart wrote:

quoted

On Thursday 14 June 2012 20:12:33 Magnus Damm wrote:

quoted

On Tue, Jun 12, 2012 at 9:57 PM, Laurent Pinchart wrote:

quoted

The tmio_mmc_set_ios() function configures the MMC power, clock and
bus width. When the MMC controller gets powered off, runtime PM
switches the MSTP clock off. Writing to to CTL_SD_MEM_CARD_OPT
register afterwards fails and prints an error message to the kernel
log.

As configuring the bus width is pointless when the interface gets
powered down, skip the operation when power is off.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

First of all, thanks for reporting this issue and coming up with a fix!

You're welcome. You can expect more of them ;-)

quoted

Can you please explain a bit more about when this triggers? Is this
related to suspend-to-ram perhaps? Which hardware platform? Is
CONFIG_PM_RUNTIME=y set?

I've noticed the problem on the Armadillo board with CONFIG_PM_RUNTIME=y.
The driver spits out "timeout waiting for SD bus idle" error messages
more or less continuously.

quoted

I suspect that this may be a side effect of the current PM code used
on the A1 SoC (which is hooked up on the armadillo board).

In short, the A1 SoC does not yet make use of PM domains, but AP4 and
the mackerel board (sh7372 based) is using PM domains.

Does this mean that runtime PM is a no-op on A1 ? That would surprise me,
as turning CONFIG_PM_RUNTIME off gets rid of the problem, so runtime PM
is somehow involved. Even if the power domain does not get turned off,
can't the MSTP clock be turned off ?

The problem is, most likely, that with CONFIG_PM_RUNTIME set the code in
drivers/sh/pm_runtime.c triggers and that causes default_pm_domain to be
used.

So, I don't really think we need to "fix" every driver in turn,
default_pm_domain seems to be what needs fixing.  I'm not exactly sure
how to fix it at the moment, though.

But isn't there still an issue in the driver itself ? If my understanding is 
correct, calling pm_runtime_put() essentially means "I won't need to touch the 
device from now on, it can be suspended it 
needed/useful/appropriate/whatever". The runtime PM core is then free to turn 
the power domain and/or clock off synchronously or with a delay, or do 
nothing. After signaling that it won't need to touch the device, the driver 
should really avoid touching the device until the next pm_runtime_get_sync() 
call.

You can verify if my suspicion is correct quite easily by replacing the
(&default_pm_domain) in drivers/sh/pm_runtime.c with NULL.

-- 
Regards,

Laurent Pinchart