Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753150AbbKZRwf (ORCPT ); Thu, 26 Nov 2015 12:52:35 -0500 Received: from mail-yk0-f170.google.com ([209.85.160.170]:33904 "EHLO mail-yk0-f170.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752096AbbKZRwd convert rfc822-to-8bit (ORCPT ); Thu, 26 Nov 2015 12:52:33 -0500 MIME-Version: 1.0 In-Reply-To: <56571913.1010006@linux.intel.com> References: <56488E6F.3090109@linux.intel.com> <564B3020.5060104@linux.intel.com> <56571913.1010006@linux.intel.com> Date: Thu, 26 Nov 2015 18:52:32 +0100 Message-ID: Subject: Re: [PATCH v3] MMC/SDIO: enable SDIO device to suspend/resume asynchronously From: Ulf Hansson To: "Fu, Zhonghui" Cc: Adrian Hunter , Neil Brown , Jaehoon Chung , Andreas Fenkart , Joe Perches , linux-mmc , "linux-kernel@vger.kernel.org" Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4009 Lines: 81 [...] >> >> Depending if you have SD/(e)MMC card slot(s), the below patch might >> affect your results. So it might be a good idea to re-run your test to >> get some fresh data. >> >> [PATCH 1/2] mmc: core: Make runtime resume default behavior for MMC/SD [3] > I didn't find this patch in mainline kernel, where is this patch? It's queued for 4.5 on my next branch via my mmc git. Thus also available in linux-next. One should know that it's affecting MMC/SD cards and not SDIO. Future wise I was hoping we could do something similar for SDIO, but I need to think that through a bit more. >> >>>>> Signed-off-by: Zhonghui Fu >>>> I think this is an interesting change, but I wonder if you really >>>> understand how this affects the order of how devices may be >>>> suspended/resumed? >>>> >>>> Also, I believe you didn't answer my question for the earlier version >>>> of the patch, so let me try again. >>>> >>>> There are a strict dependency chain when suspending/resuming devices >>>> that must be maintained. Currently this is controlled via device >>>> registration/probe order. >>>> >>>> An SDIO func driver/device must always be suspended *before* the SDIO >>>> card device. Additionally the corresponding MMC host, must be >>>> suspended after the SDIO card device. Vice verse applies to the resume >>>> sequence. >>>> >>>> As this patch enables asynchronous suspend, I am worried that it will >>>> break this dependency chain. What do you think? >>> After enabling asynchronous suspend/resume, PM core still ensures the strict suspend/resume dependency between child and parent devices - child must be suspended before its parent, and parent must be resumed before its child. SDIO function is child of SDIO card, and SDIO card is child of MMC host, and MMC host is child of MMC controller. So the dependency chain is not broken. Actually, many devices have been using asynchronous suspend/resume mode now. >> >> I believe your view of how the PM core works for devices that *don't* >> use async suspend is wrong! The PM core doesn't respect parent/child >> relations during the device system PM phase for these devices. > I agree with you for the following description. But, I never described how PM core works for devices that don't use async mode. Where did you get my view about this? I just said that PM core still ensure the dependency between child and parent devices after using async mode, I never said that the method ensuring dependency is the same for sync devices and async devices. You said it again. "PM core *still* ensures the dependency... after using async mode". I guess it depends what you mean by "still". :-) Anyway, it doesn't matter as I think we have sorted it out now. [...] >> This change will also affect SDIO combo cards. That means the when >> there is a mmc blk device driver bound to the mmc card, its >> ->suspend() methods will be called asynchronously. >> >> Have you considered that? Especially since there are nothing being >> mentioned about it in the change-log? > I have considered this, this patch still work for SDIO combo cards. >> >> Also, within this context I am wondering why you *only* enable async >> suspend for SDIO cards and not all cards (SD/MMC)? Is there a problem >> with doing that? > I am optimizing suspend/resume speed for some Intel's tablet platforms and focusing only on the most time-consuming device path now. I will deliver similar patch for SD/MMC card. Okay, thanks for sharing you plans! I would prefer if we instead enables async suspend for all types of card devices and the sdio func devices within one patch. I suggest we use mmc_add_card() and sdio_add_func(), to where we enable async suspend for the devices. [...] Kind regards Uffe -- 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/