On 5 April 2016 at 14:40, Adrian Hunter <adrian.hunter@xxxxxxxxx> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
