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
