Re: [PATCH] sdhci: wakeup from runtime PM

[Ludovic Desroches <ludovic.desroches@atmel.com>]

On Fri, Apr 08, 2016 at 10:35:00AM +0200, Ulf Hansson wrote:
> - linux-kernel list
> 
> On 7 April 2016 at 17:12, Ludovic Desroches <ludovic.desroches@atmel.com> wrote:
> > On Thu, Apr 07, 2016 at 11:11:08AM +0200, Ulf Hansson wrote:
> >> On 5 April 2016 at 14:40, Adrian Hunter <adrian.hunter@intel.com> wrote:
> >> > On 25/03/16 18:05, Ludovic Desroches wrote:
> >> >> Hi,
> >> >>
> >> >> When not using the SDHCI controller, it is logical to save power by suspending
> >> >> it. The issue is that SDHCI assumes that the controller is completely disabled.
> >> >> It means the only way to wake up on a card event is to have a gpio for the card
> >> >> detection (so two pins for the same signal). A possible workaround is to use
> >> >> polling but the controller will be resumed/suspended between each attempts.
> >> >>
> 
> From power consumption point if view we already discussed this
> particular case in an earlier thread, but let me elaborate what I
> think one more time:
> 
> Now, this is relevant for removable cards lacking GPIO card detect, as
> for other cases and non-removable cards I think we are in agreement
> that their is no issue from power consumption point of view, right!?
> 
> 1. The current solution:
> - Use MMC_CAP_NEEDS_POLL which makes the mmc core to re-schedule a
> work once every second to poll for a card (Why is it one second? Could
> we perhaps have that configurable?).
> 
> In case of no card inserted, the polling consists of runtime resuming
> the SDHCI controller and then reading a couple of registers to find
> out if there is a new card. I assume this will be a fast operation. In
> the below calculation I have neglected its impact which of course is a
> simplification.
> 
> This solution allows the driver in runtime suspend to *gate all three
> clocks* used by the SDHCI controller. In-between polling attempts it
> will thus save power.
> 
> Currently the mmc core *always* respects runtime PM autosuspend when
> putting the controller device back to runtime suspend. I suggest we
> change this in cases when the polling operation doesn't detect any
> changes.
> 
> For the sdhci-of-at91 driver case, the autosuspend timeout is set to
> 50 ms (which is a common value among mmc host drivers).
> 
> In the current code this means for a period over ~10 s, *all three
> clocks* will be ungated for 50 ms x 10 times = ~500 ms.
> 
> If we adopt to what I propose above, this time will become significant
> lesser as the autosuspend timer will not be respected.
> 
> 2. What you propose (I think):
> * Don't use MMC_CAP_NEEDS_POLL, but instead rely on the SDHCI
> controller to pick up card detect IRQs in runtime suspended state.
> 
> In case of no card inserted, the controller would then stay runtime
> suspended but with IRQs enabled to deal with card detect IRQs.
> According to what you told me earlier, this means that the controller
> requires one of the three clocks to be ungated in the runtime suspend
> state. So, only two clocks can be managed by runtime PM.
> 
> Assuming that no cards get inserted over a period of ~10 s, *one clock
> will be ungated* for ~10 s.
> 
> 
> Can you show that option 2) is better in saving power than option 1) ?

Unfortunately not. I have discussed with hardware guys. They confirmed me
that the power consumption to keep the interface enabled is very low but it
will be difficult to measure the consumption difference between the two
options because it would be minimal.

If in practice the result was almost the same, it is not useful to
continue this debate. I'll keep the polling workaround and I'll update system
PM callbacks.

[...]

Regards

Ludovic