RE: [PATCH] mmc: host: sdhci-esdhc-imx: let usr to config the wakeup for gpio cd pin through sysfs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> -----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




[Index of Archives]     [Linux Memonry Technology]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux