mmc: core "MMC_PM_KEEP_POWER"

In mmc_resume_host we clear the MMC_PM_KEEP_POWER flag from the pm_flags bitfield. This is done by a patch from Eliad Peller a while ago, "mmc: clear MMC_PM_KEEP_POWER flag on resume"

I would like to understand if there are any reason to why we want to clear this flag after we done a resume. I think it will add complexity to an sdio function driver since it must update this flag for after each suspend/resume sequence. Should it not just be enough to do this during the initialization of the sdio func driver.

hi Ulf,

Quoted 10 lines · Ulf Hansson

In mmc_resume_host we clear the MMC_PM_KEEP_POWER flag from the pm_flags bitfield. This is done by a patch from Eliad Peller a while ago, "mmc: clear MMC_PM_KEEP_POWER flag on resume"

I would like to understand if there are any reason to why we want to clear this flag after we done a resume. I think it will add complexity to an sdio function driver since it must update this flag for after each suspend/resume sequence. Should it not just be enough to do this during the initialization of the sdio func driver.

please check out the sdio_set_host_pm_flags() documentation:

/**
* sdio_set_host_pm_flags - set wanted host power management capabilities
* @func: SDIO function attached to host
*
* Set a capability bitmask corresponding to wanted host controller
* power management features for the upcoming suspend state.
* This must be called, if needed, each time the suspend method of
* the function driver is called, and must contain only bits that
* were returned by sdio_get_host_pm_caps().
* The host doesn't need to be claimed, nor the function active,
* for this information to be set.
*/
int sdio_set_host_pm_flags(struct sdio_func *func, mmc_pm_flag_t flags)

as we have to set the flags on each suspend, we have to clear them on resume.

Hi,

In mmc_resume_host we clear the MMC_PM_KEEP_POWER flag from the pm_flags bitfield. This is done by a patch from Eliad Peller a while ago, "mmc: clear MMC_PM_KEEP_POWER flag on resume"

I would like to understand if there are any reason to why we want to clear this flag after we done a resume. I think it will add complexity to an sdio function driver since it must update this flag for after each suspend/resume sequence. Should it not just be enough to do this during the initialization of the sdio func driver.

Hi,

Quoted 11 lines · Ulf Hansson

Hi,

In mmc_resume_host we clear the MMC_PM_KEEP_POWER flag from the pm_flags bitfield. This is done by a patch from Eliad Peller a while ago, "mmc: clear MMC_PM_KEEP_POWER flag on resume"

I would like to understand if there are any reason to why we want to clear this flag after we done a resume. I think it will add complexity to an sdio function driver since it must update this flag for after each suspend/resume sequence. Should it not just be enough to do this during the initialization of the sdio func driver.

The sdio function driver should just call sdio_set_host_pm_flags() in its suspend function, if it wants to stay awake at the next suspend. It's just one line of code, so I don't think it's overly complex.

We clear the flag because that's how the API is defined -- sdio_set_host_pm_flags() says "This must be called, if needed, each time the suspend method of the function driver is called".

There's a separate question of "Why is the API this way, and should we change it?". I think the API ended up this way because it was created for a wifi device where a suspend would usually happen while you're unassociated, so you don't need to preserve power (because nothing interesting can happen to wake you up), but will sometimes happen while you're associated, in which case you do. It sounds like you're considering a device that *always* wants to stay awake, which is different to the rest of the users of this API.

But it's just the difference between (your proposal) having sdio_set_host_pm_flags() be called during init, and (the API) having sdio_set_host_pm_flags() be called during suspend. It's the same line of code either way, so I don't see why it's annoying this way.

Thanks,

- Chris.

Thanks for the quick reply! I understand the idea better now. Just a minor follow-up question..

The power_restore function for sdio also uses this flag, which is a little bit strange I think.

Especially since the mmc_power_off|up is always called no matter of the value of this flag.

Hi Ulf,

Quoted 5 lines · Ulf Hansson

Thanks for the quick reply! I understand the idea better now. Just a minor follow-up question..

The power_restore function for sdio also uses this flag, which is a little bit strange I think.

Yeah, it's being used to detect whether we're in the middle of a powered resume, and therefore shouldn't be trying to reinit the card. Then the value of MMC_PM_KEEP_POWER gets reset once the resume's finished.

Quoted 2 lines

Especially since the mmc_power_off|up is always called no matter of the value of this flag.

Hm, I don't think that's true.

mmc_suspend_host() checks !mmc_card_keep_power(host) before mmc_power_off() mmc_resume_host() checks !mmc_card_keep_power(host) before mmc_power_up()

static inline int mmc_card_keep_power(struct mmc_host *host)

{
        return host->pm_flags & MMC_PM_KEEP_POWER;
}

Thanks,

- Chris.