On Thu, 19 Oct 2023 at 18:59, Adrian Hunter <adrian.hunter@xxxxxxxxx> wrote: > > On 19/10/23 18:24, Ulf Hansson wrote: > > 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? > > No, it is more like PCI / ACPI take care of most things but the > wakeup framework provides sysfs wakeup API. We can add that and > use it to determine if wakeup is enabled (by the user) to then > enable the CD GPIO IRQ wakeup. That way, the sysfs wakeup API is > on the correct device, and the user doesn't need to know anything > about GPIO. > Okay, so it's really ACPI that is needed here too. Anyway, thanks for sharing! Kind regards Uffe
