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
>
