Hi, On Tue, Apr 23, 2019 at 5:57 PM Shawn Lin <shawn.lin@xxxxxxxxxxxxxx> wrote: > > The intention seems reasonable to me, but just wonder if we need > mask/unmask SDIO interrupt when it's never used? I don't think we do. Specifically "client_sdio_enb" starts out as false. If nobody ever calls dw_mci_enable_sdio_irq() then "client_sdio_enb" will always continue to be false. Now at suspend time we'll call "__dw_mci_enable_sdio_irq". Because "client_sdio_enb" is false then the local variable "enb" will always be false. Sure we'll clear the "SDMMC_INT_SDIO" from the "INTMASK" register, but it should already have been cleared so this is a no-op. ...at resume time we'll have a similar situation where "client_sdio_enb" is false and thus we'll (again) just clear the "SDMMC_INT_SDIO" from the "INTMASK". I could potentially optimize away the "mci_writel()" if we're not changing anything if you're worried about that? > It's the same > situation for SDMMC_CLKEN_LOW_PWR that we couldn't stop providing > clock for SDIO cards, so I guess we need to check MMC_CAP_SDIO_IRQ > as well. I think it might be a slightly different situation though. In this case I believe it's not just a problem with clock stoppage. I believe the problem is that the interrupt will be passed to the SDIO device driver right away and that'll call back into dw_mmc. dw_mmc is just not in a state to handle it until we've more fully resumed things. In any case with my patch the only way we'd ever end up unmasking the SDIO IRQ here would be if dw_mci_enable_sdio_irq() was called. That will only happen if there's an SDIO card plugged in. -Doug
