Message-ID: <CAPDyKFqy9EEb6o8wYABYT6zE-7Rb0xxo8Y0DuAwooeYZp5=Grw@mail.gmail.com>
Date:	Mon, 17 Jun 2013 16:22:28 +0200
From:	Ulf Hansson <ulf.hansson@...aro.org>
To:	Colin Cross <ccross@...roid.com>
Cc:	Zoran Markovic <zoran.markovic@...aro.org>,
	lkml <linux-kernel@...r.kernel.org>, linux-mmc@...r.kernel.org,
	San Mehat <san@...gle.com>,
	John Stultz <john.stultz@...aro.org>,
	Chris Ball <cjb@...top.org>,
	Johan Rudholm <johan.rudholm@...ricsson.com>,
	Jaehoon Chung <jh80.chung@...sung.com>,
	Konstantin Dorfman <kdorfman@...eaurora.org>,
	Guennadi Liakhovetski <g.liakhovetski@....de>,
	Tejun Heo <tj@...nel.org>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [RFC PATCH] mmc: Enable wakeup_sources for mmc core

On 14 June 2013 22:52, Colin Cross <ccross@...roid.com> wrote:
> On Fri, Jun 14, 2013 at 11:42 AM, Zoran Markovic
> <zoran.markovic@...aro.org> wrote:
>>> I am not sure I understand why this patch is needed. When a new card
>>> is inserted/removed and the upper levels gets notification about the
>>> new card, triggering the mounting/un-mounting of the file system, why
>>> should it be the lowest layer (mmc) that prevents the platform from
>>> enter suspend/sleep? Why do we need to prevent it at all?
>>>
>>> Note that notifier handling in mmc_pm_notify, was if I remember
>>> correctly, not completely developed when the original version of this
>>> patch was being discussed. mmc_pm_notify prevents cards from being
>>> inserted/removed in the middle of suspend->resume sequence, is that
>>> not enough?
>>
>> I will try to speak on behalf of the original implementers in a hope
>> they would provide the original motivation for the patch.
>>
>> My understanding is that any preemption in the procedure could be an
>> opportunity to suspend, as there may be a suspend request racing with
>> this code. This is why the calls to __pm_stay_awake() and
>> queue_delayed_work() are so tightly coupled. It would be up to the
>> delayed work procedure (mmc_rescan()) to decide whether or not it is
>> safe to suspend. If there are no changes in the MMC state or all
>> changes can be handled by mmc_rescan(), it is safe to call
>> __pm_relax(). Otherwise, userland may take over processing of this
>> event, and this is why the awake state needs to be extended by 1/2
>> second.
>
> The __pm_stay_awake() is required to prevent autosleep during the time
> between the card detect interrupt and when the userspace process that
> gets the notification runs.  The 1/2 second delay is used because it
> is easier than trying to detect when the userspace process has
> received the notification, at which time it should hold its own
> wakelock and the mmc subsystem can call __pm_relax().

Hi Colin,

I don't have the in-depth knowledge about how the userspace deamons
handles the event notifications, so please bare with me while I am
trying to understand more here.

First of all, are we trying to solve an  issue here or just improving
some specific situation, that is not clear to me.

I might have misunderstood this patch, but it seems like your concern
is that you believe the event notification can get lost - if userspace
are about to trigger a suspend while a card is being inserted/removed.
If that is the case, could you elaborate on what level the
notification can get lost?

Kind regards
Ulf Hansson
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists