Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756395Ab3FQOWb (ORCPT ); Mon, 17 Jun 2013 10:22:31 -0400 Received: from mail-ve0-f178.google.com ([209.85.128.178]:38652 "EHLO mail-ve0-f178.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752362Ab3FQOW3 (ORCPT ); Mon, 17 Jun 2013 10:22:29 -0400 MIME-Version: 1.0 In-Reply-To: References: <1371146190-16786-1-git-send-email-zoran.markovic@linaro.org> Date: Mon, 17 Jun 2013 16:22:28 +0200 Message-ID: Subject: Re: [RFC PATCH] mmc: Enable wakeup_sources for mmc core From: Ulf Hansson To: Colin Cross Cc: Zoran Markovic , lkml , linux-mmc@vger.kernel.org, San Mehat , John Stultz , Chris Ball , Johan Rudholm , Jaehoon Chung , Konstantin Dorfman , Guennadi Liakhovetski , Tejun Heo , Andrew Morton Content-Type: text/plain; charset=ISO-8859-1 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2853 Lines: 58 On 14 June 2013 22:52, Colin Cross wrote: > On Fri, Jun 14, 2013 at 11:42 AM, Zoran Markovic > 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/