Re: [PATCH] libertas: fix return codes in if_sdio_suspend()

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

 



On 26 June 2018 at 22:50, Daniel Mack <daniel@xxxxxxxxxx> wrote:
> Ah, the subject line of this patch could actually be improved. Let me know
> if you're happy with the contents of this patch, so I can resend.
>
> Thanks,
> Daniel
>
>
>
> On Tuesday, June 26, 2018 10:48 PM, Daniel Mack wrote:
>>
>> Prior to commit 573185cc7e64 ("mmc: core: Invoke sdio func driver's PM
>> callbacks from the sdio bus"), the MMC core used to call into the power
>> management functions of SDIO clients itself and removed the card if the
>> return code was non-zero. IOW, the mmc handled errors gracefully and
>> didn't
>> upchain them to the pm core.
>>
>> Since this change, the mmc core relies on generic power management
>> functions which treat all errors as a reason to cancel the suspend
>> immediately. This causes suspend attempts to fail when the libertas
>> driver is loaded.
>>
>> To fix this, power down the card explicitly in if_sdio_suspend() when we
>> know we're about to lose power and return success. Also set a flag in
>> these
>> cases, and power up the card again in if_sdio_resume().
>>
>> Signed-off-by: Daniel Mack <daniel@xxxxxxxxxx>
>> Cc: Ulf Hansson <ulf.hansson@xxxxxxxxxx>
>> Cc: Chris Ball <chris@xxxxxxxxxx>
>> ---
>>   drivers/net/wireless/marvell/libertas/dev.h   |  1 +
>>   .../net/wireless/marvell/libertas/if_sdio.c   | 28 +++++++++++++++----
>>   2 files changed, 23 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/net/wireless/marvell/libertas/dev.h
>> b/drivers/net/wireless/marvell/libertas/dev.h
>> index dd1ee1f0af48..469134930026 100644
>> --- a/drivers/net/wireless/marvell/libertas/dev.h
>> +++ b/drivers/net/wireless/marvell/libertas/dev.h
>> @@ -104,6 +104,7 @@ struct lbs_private {
>>         u8 fw_ready;
>>         u8 surpriseremoved;
>>         u8 setup_fw_on_resume;
>> +       u8 power_up_on_resume;
>>         int (*hw_host_to_card) (struct lbs_private *priv, u8 type, u8
>> *payload, u16 nb);
>>         void (*reset_card) (struct lbs_private *priv);
>>         int (*power_save) (struct lbs_private *priv);
>> diff --git a/drivers/net/wireless/marvell/libertas/if_sdio.c
>> b/drivers/net/wireless/marvell/libertas/if_sdio.c
>> index 2300e796c6ab..f43807663b1b 100644
>> --- a/drivers/net/wireless/marvell/libertas/if_sdio.c
>> +++ b/drivers/net/wireless/marvell/libertas/if_sdio.c
>> @@ -1290,15 +1290,23 @@ static void if_sdio_remove(struct sdio_func *func)
>>   static int if_sdio_suspend(struct device *dev)
>>   {
>>         struct sdio_func *func = dev_to_sdio_func(dev);
>> -       int ret;
>>         struct if_sdio_card *card = sdio_get_drvdata(func);
>> +       struct lbs_private *priv = card->priv;
>> +       int ret;
>>         mmc_pm_flag_t flags = sdio_get_host_pm_caps(func);
>> +       priv->power_up_on_resume = false;
>>         /* If we're powered off anyway, just let the mmc layer remove the
>>          * card. */
>> -       if (!lbs_iface_active(card->priv))
>> -               return -ENOSYS;
>> +       if (!lbs_iface_active(priv)) {
>> +               if (priv->fw_ready) {
>> +                       priv->power_up_on_resume = true;
>> +                       if_sdio_power_off(card);
>> +               }
>> +
>> +               return 0;
>> +       }
>>         dev_info(dev, "%s: suspend: PM flags = 0x%x\n",
>>                  sdio_func_id(func), flags);
>> @@ -1306,9 +1314,14 @@ static int if_sdio_suspend(struct device *dev)
>>         /* If we aren't being asked to wake on anything, we should bail
>> out
>>          * and let the SD stack power down the card.
>>          */
>> -       if (card->priv->wol_criteria == EHS_REMOVE_WAKEUP) {
>> +       if (priv->wol_criteria == EHS_REMOVE_WAKEUP) {
>>                 dev_info(dev, "Suspend without wake params -- powering
>> down card\n");
>> -               return -ENOSYS;
>> +               if (priv->fw_ready) {
>> +                       priv->power_up_on_resume = true;
>> +                       if_sdio_power_off(card);
>> +               }
>> +
>> +               return 0;
>>         }
>>         if (!(flags & MMC_PM_KEEP_POWER)) {
>> @@ -1321,7 +1334,7 @@ static int if_sdio_suspend(struct device *dev)
>>         if (ret)
>>                 return ret;
>>   -     ret = lbs_suspend(card->priv);
>> +       ret = lbs_suspend(priv);
>>         if (ret)
>>                 return ret;
>>   @@ -1336,6 +1349,9 @@ static int if_sdio_resume(struct device *dev)
>>         dev_info(dev, "%s: resume: we're back\n", sdio_func_id(func));
>>   +     if (card->priv->power_up_on_resume)
>> +               if_sdio_power_on(card);
>> +

To guarantee firmware is loaded, don't you need the below as well?

wait_event(card->pwron_waitq, priv->fw_ready);

>>         ret = lbs_resume(card->priv);
>>         return ret;
>>
>

Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



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

  Powered by Linux