On 9/21/2015 4:41 PM, Adrian Hunter wrote: > On 21/09/15 11:15, Adrian Hunter wrote: >> On 21/09/15 08:29, Fu, Zhonghui wrote: >>> >>> On 2015/8/24 15:07, Fu, Zhonghui wrote: >>>> On 2015/8/17 14:48, Adrian Hunter wrote: >>>>> On 17/08/15 06:26, Fu, Zhonghui wrote: >>>>>> Hi, >>>>>> >>>>>> Any comments are welcome. >>>>>> >>>>>> >>>>>> Thanks, >>>>>> Zhonghui >>>>>> >>>>>> On 2015/7/30 15:40, Fu, Zhonghui wrote: >>>>>>> Enable SDIO card and function device to suspend/resume asynchronously. >>>>>>> This can improve system suspend/resume speed. >>>>> For me, it needs more explanation. >>>>> >>>>> For example, why is this worth doing? Can you give an example where it >>>>> does >>>>> significantly improve suspend/resume speed? Are there any cases where it >>>>> could be worse? >>>>> >>>>> Why is it safe? Presumably it is safe if there are no dependencies on the >>>>> device outside the device hierarchy. Is that so? Are there any other >>>>> potential pitfalls to enabling async_suspend? >>>> Now, PM core support asynchronous device suspend/resume mode. If one >>>> device has been set to support asynchronous PM mode, it's suspend/resume >>>> operation can be performed in a separate kernel thread and take advantage >>>> of multicore feature to improve overall system suspend/resume speed. The >>>> worst case is that all device suspend/resume threads will be scheduled to >>>> the same CPU, it hardly occur. >>>> >>>> PM core ensure all the suspend/resume dependency related to one device. >>>> Actually, async suspend/resume mode is one feature of PM core, every >>>> device subsystem may use it or not use it. Once one device subsystem >>>> choose to use this feature, its safety is up to PM core as long as device >>>> subsystem has initialized fully this device. >>> The original suspend time is 1645ms and resume time is 940ms on ASUS T100TA >>> machine. After enabling "wiphy device", "SDIO device", "mmc host" and >>> "sdhci-acpi device" to suspend/resume asynchronously, the suspend time is >>> 1096ms and resume time is 908ms. The test environment is listed as follows: >>> >>> OS: Ubuntu 14.04 >>> Kernel: mainline v4.1 >>> Machine: ASUS T100TA(Baytrail-T platform) >>> Tool: analyze_suspend >>> “analyze_suspend.py –f –m freeze” to suspend system >>> Press power button to resume system >>> >>> I have resent this patch with updated commit message - "[PATCH v2] MMC/SDIO: >>> enable SDIO device to suspend/resume asynchronously". >> It is really cool that you tested this. Thank you! >> >> I am a bit baffled and bemused that you didn't put a summary of the testing >> and results in the commit message. Can you do that. >> >> As I wrote, we are assuming that there are no dependencies on the device >> outside the device hierarchy. That is a reasonable assumption for an SDIO >> controller because it doesn't provide resources for other devices to use, >> except for the card itself which is a child device, and therefore a >> dependency that PM core knows about. >> >> Does that make sense? If it does then shouldn't that explanation be added >> to the commit message too. i.e. this is why we think it is always going to >> work? > Just realized this patch is for the card, not the host controller, so those > last 2 paragraphs don't apply.
Sorry for replying this mail after long leave. Thanks for your comments. I have added the test result into commit message and sent the latest version of this patch: "[PATCH v3] MMC/SDIO: enable SDIO device to suspend/resume asynchronously" Thanks, Zhonghui > >>> >>> Thanks, >>> Zhonghui >>>> >>>> Thanks, >>>> Zhonghui >>>>>>> Signed-off-by: Zhonghui Fu <zhonghui...@linux.intel.com> >>>>>>> --- >>>>>>> drivers/mmc/core/sdio.c | 4 ++++ >>>>>>> 1 files changed, 4 insertions(+), 0 deletions(-) >>>>>>> >>>>>>> diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c >>>>>>> index b91abed..6719b77 100644 >>>>>>> --- a/drivers/mmc/core/sdio.c >>>>>>> +++ b/drivers/mmc/core/sdio.c >>>>>>> @@ -1106,6 +1106,8 @@ int mmc_attach_sdio(struct mmc_host *host) >>>>>>> pm_runtime_enable(&card->dev); >>>>>>> } >>>>>>> >>>>>>> + device_enable_async_suspend(&card->dev); >>>>>>> + >>>>>>> /* >>>>>>> * The number of functions on the card is encoded inside >>>>>>> * the ocr. >>>>>>> @@ -1126,6 +1128,8 @@ int mmc_attach_sdio(struct mmc_host *host) >>>>>>> */ >>>>>>> if (host->caps & MMC_CAP_POWER_OFF_CARD) >>>>>>> pm_runtime_enable(&card->sdio_func[i]->dev); >>>>>>> + >>>>>>> + device_enable_async_suspend(&card->sdio_func[i]->dev); >>>>>>> } >>>>>>> >>>>>>> /* >>>>>>> -- 1.7.1 >>>>>>> >>>>> -- >>>>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in >>>>> the body of a message to majord...@vger.kernel.org >>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>>>> Please read the FAQ at http://www.tux.org/lkml/ >> > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
