On Thu, 19 Oct 2023 at 16:48, Adrian Hunter <adrian.hunter@xxxxxxxxx> wrote: > > On 18/10/23 17:42, Ulf Hansson wrote: > > On Wed, 18 Oct 2023 at 04:07, Bough Chen <haibo.chen@xxxxxxx> wrote: > >> > >>> -----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. > > > > Good point! I am not sure that is the correct way to do it either, but > > maybe it's fine for PCI based devices. Adrian probably knows this > > better. > > PCI / ACPI layers handle configuration of wakeup signaling, so a > device will be put to the appropriate power state. It won't be > forced to be "on" by registering a wakeup source. > Okay, I see. So you are saying that PCI bus/layers are aware of the GPIO pin/irq being used for wakeup for the device in question? Kind regards Uffe
