Re: [PATCH 2/5] mmc: do not clear the host->pm_flags when suspend

Ohad Ben-Cohen
2010-11-11T23:46:01Z

Hi Peppe,

On Thu, Nov 11, 2010 at 9:24 AM, Peppe CAVALLARO <peppe.cavallaro@st.com> wrote:
>> > -                       host->pm_flags = 0;
>>
>> This is elapsed because the card is removed at this point (e.g. SDIO
>> returned -ENOSYS).
>>
>> Why would you want to use flags, set by a function driver of an SDIO
>> card, that was just removed ?
>>
> The define MMC_PM_KEEP_POWER has been used to notify that
> a device driver (e.g. sdhci-pltfm) want to wakeup the system.

MMC_PM_KEEP_POWER indicates that a card wishes to preserve its power
during system suspend; SDIO function drivers set it, if desired, and
if the host supports it.

If you permanently set it in sdhci-pltfm, you completely change the
interface. Drivers will be very surprised with such a default power-on
behavior.

> It is set in the pm_flags. When the sdhci suspend is invoked, it can
> call the mmc_suspend_host without free the interrupt if
> MMC_PM_KEEP_POWER is set.
> Then the sdhci_set_wakeup programs the HC to be able to wake up the system.

MMC_PM_KEEP_POWER does not necessarily mean the user wants the system
to wake up on any SDIO events (e.g. it might be desired to keep a card
powered in a low power mode, without any wake-up expectancies, just to
be able to bring it up very quickly when the system resumes).

> When the resume is invoked the pm_flags is used to avoid to request the irq.
> For this reason I removed the inst "host->pm_flags = 0;" from the
> mmc_suspend_host
> function. Maybe I could not remove it but add a check if the driver
> wants to wakeup

Who is this driver ? Host controller ? I don't think it should decide
whether the system will wake up or not on these events. Host
controller should merely provide capabilities (by means of pm_caps),
and let the upper layers choose.

> the system and the MMC_PM_KEEP_POWER is set. What do you think?

Beyond MMC_PM_WAKE_SDIO_IRQ, which is already established, you seem to
add two additional wake up events: card insertion and card removal.

The insertion event, at least (still need to think about the removal
one), have little to do with the SDIO function driver itself (e.g., if
the system is suspended, and the user inserts a card, obviously there
is no driver loaded for it yet that could have set pm_flags).

These events seem to be more system-wide and not driver-specific: does
the user want the host to wake up on card insertion or not ?

One idea that comes to mind is to use the PM core's wakeup sysfs
device attribute to control this; this way the user will be able to
directly enable/disable these wakeup events.

It might still make sense to use pm_flags and pm_caps here, and then
the user will manipulate the sysfs entries of the mmc host class_dev
rather than manipulating the host controller device directly. Not sure
how big an advantage is that, though.

>
> Thanks
> Regards,
> Peppe
>>
>>
>> >                        err = 0;
>> >                }
>> >        }
>> > --
>> > 1.5.5.6
>> >
>> > --
>> > To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> > the body of a message to majordomo@vger.kernel.org
>> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> >
>>
>