Re: [PATCH v2] libertas: fix suspend and resume for SDIO connected cards

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

 



On 27 June 2018 at 20:58, Daniel Mack <daniel@xxxxxxxxxx> 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>

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

Reviewed-by: Ulf Hansson <ulf.hansson@xxxxxxxxxx>

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

Kind regards
Uffe

> ---
> 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(-)
>
> 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..43743c26c071 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,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
>
--
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