Messages in this thread | | | Date | Thu, 7 Apr 2016 11:11:08 +0200 | Subject | Re: [PATCH] sdhci: wakeup from runtime PM | From | Ulf Hansson <> |
| |
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. >> >> We have already discussed a long time about this and it seems we don't agree. >> In my opinion, even if I can't disable all clocks, I should use runtime PM >> to save some power. >> >> I propose two patches, one which is a draft to try to solve it at sdhci level >> and one at sdhci-of-at91 level. >> >> Concerning the first one, I don't understand why we need to reject irqs if >> runtime_suspended is true. > > The interrupt handler might be called because the interrupt is shared i.e. > the interrupt is for a different device. In that case the host controller > might be off and the registers inaccessible. In that case we cannot even > look at the interrupt register to determine if we were expecting the interrupt. > >> Only SDHCI_INT_CARD_INT irq is enabled so why we >> should have other IRQs than this one? > > In the case of SDIO Card interrupt, it is delivered via the host controller, > so we have to assume the registers are accessible if we are > runtime-suspended with the SDIO IRQ enabled. > >> >> Since you were not in favour of allowing to wakeup on SDHCI_INT_CARD_INSERT or >> SDHCI_INT_CARD_REMOVE, I assume you won't take it so I >> solved my issue only by modifying my driver. > > I don't mind allowing card detect interrupts while runtime suspended, but we > need a flag so that: > - runtime suspend leaves the insert/remove interrupts enabled > - irq handler knows it can access registers
To me, this seem like the wrong way of how to configure wake-ups for these kind of devices.
I don't think the regular IRQs shall be enabled and the driver shouldn't assume the registers are accessible without first runtime resuming the device.
> - irq thread handler knows to runtime resume before doing anything else > > But it seems like you need to persuade Ulf first.
For a more thorough explanation to why I don't like, please have a look at my comment for another related thread on the mmc list. http://www.spinics.net/lists/linux-mmc/msg36132.html
Kind regards Uffe
| |