From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751798AbcFWF0g (ORCPT ); Thu, 23 Jun 2016 01:26:36 -0400 Received: from mga02.intel.com ([134.134.136.20]:55580 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750971AbcFWF0e (ORCPT ); Thu, 23 Jun 2016 01:26:34 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.26,509,1459839600"; d="scan'208";a="993402553" Subject: Re: [PATCH] mmc: core: add auto bkops support To: Shawn Lin , Alex Lemberg , Ulf Hansson References: <1465182439-27963-1-git-send-email-shawn.lin@rock-chips.com> <7078F2B9-63B6-4170-BFA1-5AC370F0D4DD@sandisk.com> <29966a6f-eb14-0307-08cc-f91a97f50382@rock-chips.com> <23729258-7a6b-80ac-6c90-b48eed64e0ea@rock-chips.com> Cc: Jaehoon Chung , "linux-mmc@vger.kernel.org" , "linux-kernel@vger.kernel.org" , Doug Anderson , "linux-rockchip@lists.infradead.org" From: Adrian Hunter Organization: Intel Finland Oy, Registered Address: PL 281, 00181 Helsinki, Business Identity Code: 0357606 - 4, Domiciled in Helsinki Message-ID: <576B7211.2030208@intel.com> Date: Thu, 23 Jun 2016 08:22:25 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.7.2 MIME-Version: 1.0 In-Reply-To: <23729258-7a6b-80ac-6c90-b48eed64e0ea@rock-chips.com> 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 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" 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 >>> >> > >