Re: [PATCH V1] mmc: sdhci-msm: Add wakeup functionality support for sdio cards

[Sarthak Garg <quic_sartgarg@quicinc.com>]

Hi ulf,

Thanks for the review.
Please find comments inline.

Thanks,
Sarthak

On 5/24/2022 5:26 PM, Kamasali Satyanarayan (Consultant) (QUIC) wrote:
> Hi,
> These patches will be further taken by Sarthak.
>
> Thanks,
> Satya
>
> -----Original Message-----
> From: Ulf Hansson <ulf.hansson@linaro.org>
> Sent: Wednesday, April 27, 2022 4:06 AM
> To: quic_spathi <quic_spathi@quicinc.com>
> Cc: u.kleine-koenig@pengutronix.de; YehezkelShB@gmail.com; rmk+kernel@armlinux.org.uk; t.scherer@eckelmann.de; s.shtylyov@omp.ru; sensor1010@163.com; sartgarg@codeaurora.org; hns@goldelico.com; uic_kamasali@quicinc.com; tiantao6@hisilicon.com; linux-mmc@vger.kernel.org; linux-kernel@vger.kernel.org; Kamasali Satyanarayan (Consultant) (QUIC) <quic_kamasali@quicinc.com>
> Subject: Re: [PATCH V1] mmc: sdhci-msm: Add wakeup functionality support for sdio cards
>
> On Tue, 26 Apr 2022 at 11:10, Srinivasarao Pathipati <quic_spathi@quicinc.com> wrote:
>> From: Sarthak Garg <sartgarg@codeaurora.org>
>>
>> This adds external GPIO wakeup support to sdhci-msm driver for sdio
>> cards.
> There is not a single line changed in the sdhci-msm driver, so I am not sure what this is intended to fix.
>
> I assume this is a downstream patch you want to upstream, which is a good thing that we all appreciate. However, before just posting a patch from downstream code, please have a look at the commit message and let it explain what and why you want to change things.
>
> I am deferring to review this, until you post a version with a proper commit message. Sorry.
>
> Kind regards
> Uffe

This is our internal feature where our SDIO card makes use of external 
tlmm pin to wakeup the host from system suspend.
It has approx 200 lines of code changes in the sdhci-msm.c file which 
will create further confusions. For that feature we just need to 
initialize sdio card as a wakeup source and enable wakeup capability for 
that.
Will post V2 patch for that .. please help review.

>
>> Also enables clk gating only in system Suspend/Resume for SDIO card.
>>
>> Also add the below fixes from 4.9 kernel :
>>
>> c363224b: Fix wakeup functionality for SDIO
>> 61fc5bf6: Remove flag MMC_PM_WAKE_SDIO_IRQ in mmc_resume_host
>> a7a2a82e: Set sdio_pending_processing default state to false.
>>
>> Signed-off-by: Sarthak Garg <sartgarg@codeaurora.org>
>> Signed-off-by: kamasali <quic_kamasali@quicinc.com>
>> Signed-off-by: Srinivasarao Pathipati <quic_spathi@quicinc.com>
>> ---
>>   drivers/mmc/core/bus.c  | 7 +++++++
>>   drivers/mmc/core/sdio.c | 1 +
>>   2 files changed, 8 insertions(+)
>>
>> diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c index
>> 58a60af..a475fe1 100644
>> --- a/drivers/mmc/core/bus.c
>> +++ b/drivers/mmc/core/bus.c
>> @@ -364,6 +364,13 @@ int mmc_add_card(struct mmc_card *card)  #endif
>>          card->dev.of_node = mmc_of_find_child_device(card->host, 0);
>>
>> +       if (mmc_card_sdio(card)) {
>> +               ret = device_init_wakeup(&card->dev, true);
>> +               if (ret)
>> +                       pr_err("%s: %s: failed to init wakeup: %d\n",
>> +                               mmc_hostname(card->host), __func__, ret);
>> +       }
>> +
>>          device_enable_async_suspend(&card->dev);
>>
>>          ret = device_add(&card->dev);
>> diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c index
>> 25799ac..9502318 100644
>> --- a/drivers/mmc/core/sdio.c
>> +++ b/drivers/mmc/core/sdio.c
>> @@ -1096,6 +1096,7 @@ static int mmc_sdio_resume(struct mmc_host *host)
>>          mmc_release_host(host);
>>
>>          host->pm_flags &= ~MMC_PM_KEEP_POWER;
>> +       host->pm_flags &= ~MMC_PM_WAKE_SDIO_IRQ;
>>          return err;
>>   }
>>
>> --
>> 2.7.4
>>