Hi Ohad,
On 11/12/2010 12:45 AM, Ohad Ben-Cohen wrote:
>
> 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.
>
Yes.
> > 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).
>
This is also true. I wanted to use this macro to indicate that the
wakeup is possible.
I mean, if the driver preserves the power could be wakeup capable.
>
> > 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.
>
IIUC, the driver should set its capabilities in pm_caps and I could use
the device_can_wakeup
in suspend and resume;
I mean, in the suspend, for example, I should use:
if (!(device_may_wake_up(host->mmc->parent)))
...
instead of
if (!(host->mmc->pm_flags & MMC_PM_KEEP_POWER)) {
...
>
> > 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).
>
Thanks! Now I understand why the pm_flags is clear in the mmc suspend.
Please see the following test scenario.
I suspend the STB, then I want to to remove the card (and the box is
always in power down).
In the end, I want to reinsert the card and wake up the system ;-).
In this case, the pm_flags is cleared (when I enter in the sdhci resume).
If I suspend the box again, I won't be able to get any info about the
wakeup event has to
be triggered. Am I missing anything?
So I've not clear if I should use the pm_caps instead of 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 ?
>
Yes user does.
I wanted to actually use:
MMC_PM_WAKE_SDIO_IRQ
for programming the HC to wakeup on card interrupt
MMC_PM_WAKE_SDIO_CARD_INS
for programming the HC to wakeup on card insertion
MMC_PM_WAKE_SDIO_CARD_REM
for programming the HC to wakeup on card removal
> 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.
>
AFAIK, I can only enable/disable wakeup by using:
/sys/devices/platform/sdhci.0/power/wakeup
I need to pass which events has to wakeup the HC (card int, insert, rem).
Indeed, I wanted to enable/disable them at user level by using the new
option:
bash-3.00 echo X > /sys/module/sdhci/parameters/wakeup
where X could be:
0: no wakeup
1: Card Interrupts
2: Card Insertion
3: Card Removal
Maybe it's worth having more parameters instead of the
wakeup. I mean, something like this (welcome feedback):
wake_on_card_int
wake_on_card_ins
wake_on_card_re
What do you think?
> 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.
>
I think you are right.
Welcome other feedback so I'll rework these patch and align them to
mmc-next as Wolfram suggested.
Many thanks for your feedback and details.
Regards
Peppe
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
