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. > >> >> >> 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-mmc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
