On 23/06/16 04:33, Shawn Lin wrote: > 在 2016/6/22 22:08, Alex Lemberg 写道: >> HI Shawn, >> >> On 6/21/16, 4:44 AM, "Shawn Lin" <shawn.lin@xxxxxxxxxxxxxx> wrote: >> >>> On 2016/6/20 21:33, Alex Lemberg wrote: >>>> Hi Shawn, >>>> >>>> […] >>>> >>>>>>> + >>>>>>> +static int mmc_stop_auto_bkops(struct mmc_card *card) >>>>>>> +{ >>>>>>> + int err = 0; >>>>>>> + >>>>>>> + if (!card->ext_csd.auto_bkops_en) >>>>>>> + return 0; >>>>>>> + >>>>>> >>>>>> Shouldn’t the BKOPS_STATUS be checked prior to disabling the BKOPS >>>>>> activity of the device? >>>>>> >>>>> >>>>> Hrmm.. I read the whole section of spec for it, and I did find this >>>>> requirement for manul bkops but not for the auto one. So what should we >>>>> do if using the auto one? >>>>> >>>> >>>> In case of AUTO BKOPS, the eMMC Device should perform internal GC >>>> in the same way as in case of MANUAL BKOPS. >>>> The only difference is a host awareness. >>> >>> agree. >>> >>>> Although there is no requirement in the spec, I think the driver can >>>> give some time to the device to perform/complete its internal GC during >>>> the idle time. >>>> Thus I think we can check the BKOPS_STATUS on Runtime suspend. >>> >>> We shouldn't diable bkops on *runtime* suspend as it's just the right >>> time for firmware to do GC. We could consider to check and wait for >>> the status when doing poweroff, although it seems firmware should be >>> able to accept the disable cmd and deal the on-going work perfectly >>> when doing bkops without host's awareness, just the same way as suddent >>> power loss cases. >> >> If I am not wrong, in current implementation of runtime suspend, >> the driver stops BKOPS (send HPI) just before sending sleep command, >> see _mmc_suspend(), depends on “MMC_CAP_AGGRESSIVE_PM” flag. >> In this case, the eMMC device will not have enough time to perform internal >> BKOPS in both – Manual and Auto BKOPS configurations. >> > > ye, so it seems a pre-exiting issue before introducing auto bkops? > I think we can push another patch to improve it but not handling > it for this $SUBJECT, does it sound ok to you? Runtime suspend for eMMC has a default auto-suspend delay of 3 seconds (refer mmc_blk_probe()). Isn't that when auto bkops would happen? > >> For the poweroff, it should be OK with a current implementation of >> PON (mmc_poweroff_notify()) >> >>> >>> Also I don't know whether the firmware will reflect its status on >>> BKOPS_STATUS or not when enabling the auto one. I will do more test. >>> >>> Anyway, thanks for sharing your thought. >>> Also Adrian point out that currently we trigger manual bkosp from >>> userspace via mmc-utils, and I agreed we shouldn't force kernel stack >>> to enable it defaultly. So I'm prone not to update this $SUBJECT and >>> migrate it to mmc-utils later. >>> >>>> >>>> […] >>>> >>>> Thanks, >>>> Alex >>>> >>> >>> >>> -- >>> Best Regards >>> Shawn Lin >>> >> > > -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html