> -----Original Message----- > From: Ulf Hansson <ulf.hansson@xxxxxxxxxx> > Sent: 2023年10月17日 19:35 > To: Bough Chen <haibo.chen@xxxxxxx> > Cc: adrian.hunter@xxxxxxxxx; linux-mmc@xxxxxxxxxxxxxxx; dl-linux-imx > <linux-imx@xxxxxxx>; shawnguo@xxxxxxxxxx; s.hauer@xxxxxxxxxxxxxx; > kernel@xxxxxxxxxxxxxx; festevam@xxxxxxxxx > Subject: Re: [PATCH] mmc: host: sdhci-esdhc-imx: let usr to config the wakeup > for gpio cd pin through sysfs > > On Tue, 17 Oct 2023 at 09:26, <haibo.chen@xxxxxxx> wrote: > > > > From: Haibo Chen <haibo.chen@xxxxxxx> > > > > Currently default enable the gpio cd pin wakeup, this will waste power > > after system suspend if usr do not need this cd pin wakeup feature. > > Now let usr to config the wakeup for gpio cd pin through sysfs. > > > > Signed-off-by: Haibo Chen <haibo.chen@xxxxxxx> > > --- > > drivers/mmc/host/sdhci-esdhc-imx.c | 14 ++++++++++---- > > 1 file changed, 10 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c > > b/drivers/mmc/host/sdhci-esdhc-imx.c > > index 40a6e2f8145a..2e46648344ba 100644 > > --- a/drivers/mmc/host/sdhci-esdhc-imx.c > > +++ b/drivers/mmc/host/sdhci-esdhc-imx.c > > @@ -1797,9 +1797,12 @@ static int sdhci_esdhc_imx_probe(struct > platform_device *pdev) > > /* > > * Setup the wakeup capability here, let user to decide > > * whether need to enable this wakeup through sysfs interface. > > + * First check the SDIO device, second check the gpio CD pin. > > */ > > - if ((host->mmc->pm_caps & MMC_PM_KEEP_POWER) && > > - (host->mmc->pm_caps & > MMC_PM_WAKE_SDIO_IRQ)) > > + if (((host->mmc->pm_caps & MMC_PM_KEEP_POWER) && > > + (host->mmc->pm_caps & > MMC_PM_WAKE_SDIO_IRQ)) || > > + ((host->mmc->caps & MMC_CAP_CD_WAKE) && > > + host->mmc->slot.cd_irq >= 0)) > > device_set_wakeup_capable(&pdev->dev, true); > > If the wakeup is GPIO based, it doesn't mean that the device is wakeup capable, > so this is wrong. > > In principle this must be managed through the GPIO irqchip driver instead. Yes, I know GPIO and USDHC are different device, combine GPIO wakeup to USDHC is not that reasonable. I send this patch because I notice there is similar code in sdhci-pci-core.c, refer to sdhci_pci_suspend_host() Seems PCI subsystem combine GPIO wakeup to PCI controller. Anyway, I will try to implement this in GPIO irqchip driver. Best Regards Haibo Chen > > > > > pm_runtime_set_active(&pdev->dev); > > @@ -1878,7 +1881,8 @@ static int sdhci_esdhc_suspend(struct device *dev) > > if (ret) > > return ret; > > > > - ret = mmc_gpio_set_cd_wake(host->mmc, true); > > + if (device_may_wakeup(dev)) > > As I indicated above. It's not really the device that corresponds to the mmc > controller that can wake up the system, but rather the gpio irqchip. > > > + ret = mmc_gpio_set_cd_wake(host->mmc, true); > > > > return ret; > > } > > @@ -1901,8 +1905,10 @@ static int sdhci_esdhc_resume(struct device > > *dev) > > > > if (host->mmc->caps2 & MMC_CAP2_CQE) > > ret = cqhci_resume(host->mmc); > > + if (ret) > > + return ret; > > > > - if (!ret) > > + if (device_may_wakeup(dev)) > > Ditto. > > > ret = mmc_gpio_set_cd_wake(host->mmc, false); > > > > return ret; > > -- > > 2.34.1 > > > > Kind regards > Uffe