libertas: fix suspend and resume for SDIO connected cards

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 <[email protected]>

Cc Ulf Hansson <[email protected]>

Cc Chris Ball <[email protected]>

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 <[email protected]>

Cc Ulf Hansson <[email protected]>

Cc Chris Ball <[email protected]>

---
v1 → v2:
* Reworded patch subject
* Added wait_event(card->pwron_waitq, card->priv->fw_ready)

 drivers/net/wireless/marvell/libertas/dev.h   |  1 +
 .../net/wireless/marvell/libertas/if_sdio.c   | 30 +++++++++++++++----
 2 files changed, 25 insertions(+), 6 deletions(-)

drivers/net/wireless/marvell/libertas/dev.h +1 -0

index dd1ee1f0af48..469134930026 100644
@@ -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);

drivers/net/wireless/marvell/libertas/if_sdio.c +24 -7

index 2300e796c6ab..43743c26c071 100644
@@ -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,11 @@ 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);
+               wait_event(card->pwron_waitq, card->priv->fw_ready);
+       }
+
        ret = lbs_resume(card->priv);
 
        return ret;
--

2.17.1

Quoted 15 lines · Daniel Mack

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().

Quoted 3 lines

Signed-off-by Daniel Mack <[email protected]>

Cc Ulf Hansson <[email protected]>

Cc Chris Ball <[email protected]>

Looks good to me! Should be a candidate for stable as well!?

I have some additional related changes in mind for the libertas SDIO driver, however let me post patches for us to discuss around instead.

Quoted 16 lines · Ulf Hansson

Quoted 15 lines · Daniel Mack

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().

Quoted 5 lines

Quoted 3 lines

Signed-off-by Daniel Mack <[email protected]>

Cc Ulf Hansson <[email protected]>

Cc Chris Ball <[email protected]>

Looks good to me! Should be a candidate for stable as well!?

Thanks!

Yeah, it should probably get a

Fixes 573185cc7e64 ("mmc: core: Invoke sdio func driver's PM

callbacks from the sdio bus")

as well.

Quoted 30 lines · Daniel Mack

Quoted 16 lines · Ulf Hansson

Quoted 15 lines · Daniel Mack

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().

Quoted 5 lines

Quoted 3 lines

Signed-off-by Daniel Mack <[email protected]>

Cc Ulf Hansson <[email protected]>

Cc Chris Ball <[email protected]>

Looks good to me! Should be a candidate for stable as well!?

Thanks!

Yeah, it should probably get a

Fixes 573185cc7e64 ("mmc: core: Invoke sdio func driver's PM

callbacks from the sdio bus")

Quoted 1 line

as well.

So I'll queue this for wireless-drivers-next and add:

Fixes 573185cc7e64 ("mmc: core: Invoke sdio func driver's PM callbacks from the sdio bus")

Cc <[email protected]

Ok?

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().

Fixes 573185cc7e64 ("mmc: core: Invoke sdio func driver's PM callbacks from the sdio bus")

Cc <[email protected]>

Signed-off-by Daniel Mack <[email protected]>

Reviewed-by Chris Ball <[email protected]>

Reviewed-by Ulf Hansson <[email protected]>