On 4 March 2016 at 14:48, Ludovic Desroches <ludovic.desroches@xxxxxxxxx> wrote: > Hi Ulf, > > On Fri, Mar 04, 2016 at 10:09:37AM +0100, Ulf Hansson wrote: >> On 17 February 2016 at 11:35, Ludovic Desroches > > [snip] > >> > I am wondering if I should take account of sdio irq enabled or not here. >> > >> > I have a sdio device which drives me crazy because of power management. >> > The driver of this device is in staging, it is wilc1000. It seems that I >> > am stuck because the sdio irq are not received. If I don't disable the >> > clock of the controller (hclock), I should receive the sdio IRQ as I >> > receive card detect ones, isn't it? >> > >> > It doesn't work, it seems I have also to not disabled mainck and gck >> > which are clocks needed to generate the clock sent to the sdio device. >> > If none of the clocks have to be disabled, where it has to be managed? >> >> That's a typical issue for SDIO IRQs, especially when the controller >> HW manages IRQs (there are other ways to deal with SDIO IRQs as well). >> >> Currently, the simplest way to deal with this in the driver is to do a >> pm_runtime_get_sync() when the SDIO IRQ gets enabled, and >> pm_runtime_put() when it gets disabled. > > Which driver? sdio device driver or controller driver? In the mmc controller driver. > >> > >> > Do I have to anticipate this use case in the driver of my sdhci >> > controller or does it have to be managed in the sdio device driver? They >> > are using sdio_claim/release_host to suspend or resume the host but >> > maybe they use it in a bad way. >> >> The wilc100 SDIO func driver should *not* keep the host claimed to >> deal with SDIO irqs. Only when it configures them. >> >> Instead, you need to deal with this in the sdhci driver, when you get >> the call to enable/disable SDIO IRQs. >> > > Do you mean in sdhci_enable_sdio_irq? I am not sure exactly where to check. As it may be depending on the sdhci variant and the SoC you probably need to deal with this in non-common sdhci code. > >> Moreover, from a system PM point of view. If the wilc100 SDIO func >> driver wants the platform to wake up on SDIO IRQs, it needs to set >> MMC_PM_KEEP_POWER and MMC_PM_WAKE_SDIO_IRQ from its ->suspend() >> callback. >> >> In that way, your sdhci driver can act accordingly from its system PM >> callbacks. In other words, depending on MMC_PM_KEEP_POWER and >> MMC_PM_WAKE_SDIO_IRQ to *not* call pm_runtime_force_suspend(). >> > > Ok, it makes sense. > > > Coming back to the initial discussion and patch which were motivated by > the fact that after runtime suspend I can't wake-up on card detect event, > I have the feeling we don't have the same assumption about runtime PM. > > From what you and Adrian told me, I should not use runtime PM if I have > no way to wake-up. In your minds, the way to wake-up is to use an > externel GPIO because the controller will be 'totally' disabled, isn't it? I agree to the first part here. Although, as you also have the option to use polling for card detect, this actually means you don't really *need* to have a wakeup configured. Especially in the case where you don't have GPIO card detect. In that way, *all* the clocks can gated in between the polling attempts, thus you will save power even in the polling mode configuration and when runtime PM is enabled. > > On my side, runtime PM allows me to save power when the sdhci controller > is not used. If I can disable two clocks out of three, I should use > runtime PM. Do you agree? > > If not, tell me how I can convince you :) Otherwise, next step is to rework > my patch but I think I have no other solution that not calling > sdhci_runtime_suspend_host if I expect to use the card detect irq of the > controller. So, to summarize. I think the best fix is to add a clever check in ->probe() and then enable polling when you can't rely on GPIO card detect IRQ. Moreover, to have a robust solution, you also need to clear SDHCI_QUIRK_BROKEN_CARD_DETECTION for your sdhci variant, as otherwise the "broken-cd" DT binding could wrongly be used for this variant. Does that make sense? 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
