From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.3 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 159DFC433E1 for ; Tue, 19 May 2020 16:29:15 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id AA629207D3 for ; Tue, 19 May 2020 16:29:14 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="KF6tJf3/"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=mg.codeaurora.org header.i=@mg.codeaurora.org header.b="bfmm5oXd" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org AA629207D3 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=codeaurora.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-mediatek-bounces+linux-mediatek=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender:Content-Type: Content-Transfer-Encoding:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date:Message-ID:From: References:To:Subject:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=WVRaY/RuHZAz4Y96NTdzauHPcWssoCm+yTZW+B4mJUU=; b=KF6tJf3/INk5NhU5uBctz1Gd/ wTl/+ig5rSgeZMoM218NeR48h3sYr9o5T2Dqn7paMI/KEXhHEy11ImhpfvNuFYfYn083v9m5JntBx fQwuHJtSyYqu4AZto8uCAE72PqFEZbbiyLkgGVNnkEkFezPCrFTHkvNuInnIu+LM3SxY35aYXG+T4 uOYz3Ttfcb81Jq+3Fh2DscdITSbrGsUspfQw4tGHgROb0gJXtgyWai51Ux4P8RkFd3zwvIcXsEnHd 9k2WX3qzCLqVTj1oNx3W8c1u/dkUS6BaOQ2BtXgDeeBE53oTi/JiHH88SlFAAZiYtuHn23/J+fNWk XpB5GVpCQ==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jb56y-0005kI-18; Tue, 19 May 2020 16:29:00 +0000 Received: from mail26.static.mailgun.info ([104.130.122.26]) by bombadil.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1jb55p-0004fm-R2 for linux-mediatek@lists.infradead.org; Tue, 19 May 2020 16:27:53 +0000 DKIM-Signature: a=rsa-sha256; v=1; c=relaxed/relaxed; d=mg.codeaurora.org; q=dns/txt; s=smtp; t=1589905671; h=Content-Transfer-Encoding: Content-Type: In-Reply-To: MIME-Version: Date: Message-ID: From: References: Cc: To: Subject: Sender; bh=6X9hMFobMkVLgkXSvHsdTSvgP/JXFdU8N066HN1mPZs=; b=bfmm5oXdVPNcBF51ntpyQ7kFj9G/dcXo2uxKGdyPHF8SyM9vposMxuq2QtnCBsiPyI4TeOP3 oQMN5qv+NArt8LazP5YkAmEPRD9swftnbvK5vMHVGRd0xf6a21CLAVXqp/8u9d0ceq2PmPnL M0vZ+MCCvFLab6c0gxdiQI8C1gE= X-Mailgun-Sending-Ip: 104.130.122.26 X-Mailgun-Sid: WyI0ZDIyMyIsICJsaW51eC1tZWRpYXRla0BsaXN0cy5pbmZyYWRlYWQub3JnIiwgImJlOWU0YSJd Received: from smtp.codeaurora.org (ec2-35-166-182-171.us-west-2.compute.amazonaws.com [35.166.182.171]) by mxa.mailgun.org with ESMTP id 5ec408fe.7f9bafb84a40-smtp-out-n01; Tue, 19 May 2020 16:27:42 -0000 (UTC) Received: by smtp.codeaurora.org (Postfix, from userid 1001) id 48B0BC00449; Tue, 19 May 2020 16:27:41 +0000 (UTC) Received: from [192.168.8.176] (cpe-70-95-149-85.san.res.rr.com [70.95.149.85]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) (Authenticated sender: asutoshd) by smtp.codeaurora.org (Postfix) with ESMTPSA id 7A312C433F2; Tue, 19 May 2020 16:27:38 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org 7A312C433F2 Authentication-Results: aws-us-west-2-caf-mail-1.web.codeaurora.org; dmarc=none (p=none dis=none) header.from=codeaurora.org Authentication-Results: aws-us-west-2-caf-mail-1.web.codeaurora.org; spf=none smtp.mailfrom=asutoshd@codeaurora.org Subject: Re: [PATCH v3 5/5] scsi: ufs: Fix possible VCC power drain during runtime suspend To: Stanley Chu , linux-scsi@vger.kernel.org, martin.petersen@oracle.com, avri.altman@wdc.com, alim.akhtar@samsung.com, jejb@linux.ibm.com References: <20200516174615.15445-1-stanley.chu@mediatek.com> <20200516174615.15445-6-stanley.chu@mediatek.com> From: "Asutosh Das (asd)" Message-ID: <6d32fba1-f7c3-f043-42b6-0da065e9795b@codeaurora.org> Date: Tue, 19 May 2020 09:27:37 -0700 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.8.0 MIME-Version: 1.0 In-Reply-To: <20200516174615.15445-6-stanley.chu@mediatek.com> Content-Language: en-US X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200519_092751_580854_8FA5F8D2 X-CRM114-Status: GOOD ( 29.28 ) X-BeenThere: linux-mediatek@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: bvanassche@acm.org, andy.teng@mediatek.com, chun-hung.wu@mediatek.com, kuohong.wang@mediatek.com, linux-kernel@vger.kernel.org, cang@codeaurora.org, linux-mediatek@lists.infradead.org, peter.wang@mediatek.com, matthias.bgg@gmail.com, linux-arm-kernel@lists.infradead.org, beanhuo@micron.com Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "Linux-mediatek" Errors-To: linux-mediatek-bounces+linux-mediatek=archiver.kernel.org@lists.infradead.org Hi Stanley, On 5/16/2020 10:46 AM, Stanley Chu wrote: > The commit "scsi: ufs: Fix WriteBooster flush during runtime > suspend" promises essential resource, i.e., for UFS devices doing > WriteBooster buffer flush and Auto BKOPs. However if device > finishes its job but not resumed for a very long time, system > will have unnecessary power drain because VCC is still supplied. > > To fix this, a method to recheck the threshold of keeping VCC > supply is required. However, the threshold recheck needs to > re-activate the link because the decision depends on the device > status. > > Introduce a delayed work to force runtime resume after a certain > delay during runtime suspend. This makes threshold recheck simpler > which will be done in the next runtime-suspend. > > Signed-off-by: Stanley Chu > --- Is there a reason to have this code as a separate patch? [1] Commit: "scsi: ufs: Fix WriteBooster flush during runtime suspend" introduces 'keep_curr_dev_pwr_mode' and the very next change (this one) removes it. Do you think this change and [1] should be merged? > drivers/scsi/ufs/ufs.h | 1 + > drivers/scsi/ufs/ufshcd.c | 43 ++++++++++++++++++++++++++++++++++----- > drivers/scsi/ufs/ufshcd.h | 1 + > 3 files changed, 40 insertions(+), 5 deletions(-) > > diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h > index db07eedfed96..c70845d41449 100644 > --- a/drivers/scsi/ufs/ufs.h > +++ b/drivers/scsi/ufs/ufs.h > @@ -574,6 +574,7 @@ struct ufs_dev_info { > u32 d_ext_ufs_feature_sup; > u8 b_wb_buffer_type; > u32 d_wb_alloc_units; > + bool b_rpm_dev_flush_capable; > u8 b_presrv_uspc_en; > }; > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c > index f4f2c7b5ab0a..a137553f9b41 100644 > --- a/drivers/scsi/ufs/ufshcd.c > +++ b/drivers/scsi/ufs/ufshcd.c > @@ -94,6 +94,9 @@ > /* default delay of autosuspend: 2000 ms */ > #define RPM_AUTOSUSPEND_DELAY_MS 2000 > > +/* Default delay of RPM device flush delayed work */ > +#define RPM_DEV_FLUSH_RECHECK_WORK_DELAY_MS 5000 > + > /* Default value of wait time before gating device ref clock */ > #define UFSHCD_REF_CLK_GATING_WAIT_US 0xFF /* microsecs */ > > @@ -5310,7 +5313,7 @@ static bool ufshcd_wb_presrv_usrspc_keep_vcc_on(struct ufs_hba *hba, > return false; > } > > -static bool ufshcd_wb_keep_vcc_on(struct ufs_hba *hba) > +static bool ufshcd_wb_need_flush(struct ufs_hba *hba) > { > int ret; > u32 avail_buf; > @@ -5348,6 +5351,21 @@ static bool ufshcd_wb_keep_vcc_on(struct ufs_hba *hba) > return ufshcd_wb_presrv_usrspc_keep_vcc_on(hba, avail_buf); > } > > +static void ufshcd_rpm_dev_flush_recheck_work(struct work_struct *work) > +{ > + struct ufs_hba *hba = container_of(to_delayed_work(work), > + struct ufs_hba, > + rpm_dev_flush_recheck_work); > + /* > + * To prevent unnecessary VCC power drain after device finishes > + * WriteBooster buffer flush or Auto BKOPs, force runtime resume > + * after a certain delay to recheck the threshold by next runtime > + * supsend. > + */ > + pm_runtime_get_sync(hba->dev); > + pm_runtime_put_sync(hba->dev); > +} > + > /** > * ufshcd_exception_event_handler - handle exceptions raised by device > * @work: pointer to work data > @@ -8164,7 +8182,6 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op) > enum ufs_pm_level pm_lvl; > enum ufs_dev_pwr_mode req_dev_pwr_mode; > enum uic_link_state req_link_state; > - bool keep_curr_dev_pwr_mode = false; > > hba->pm_op_in_progress = 1; > if (!ufshcd_is_shutdown_pm(pm_op)) { > @@ -8224,11 +8241,12 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op) > * Hibern8, keep device power mode as "active power mode" > * and VCC supply. > */ > - keep_curr_dev_pwr_mode = hba->auto_bkops_enabled || > + hba->dev_info.b_rpm_dev_flush_capable = > + hba->auto_bkops_enabled || > (((req_link_state == UIC_LINK_HIBERN8_STATE) || > ((req_link_state == UIC_LINK_ACTIVE_STATE) && > ufshcd_is_auto_hibern8_enabled(hba))) && > - ufshcd_wb_keep_vcc_on(hba)); > + ufshcd_wb_need_flush(hba)); > } > > if (req_dev_pwr_mode != hba->curr_dev_pwr_mode) { > @@ -8238,7 +8256,7 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op) > ufshcd_disable_auto_bkops(hba); > } > > - if (!keep_curr_dev_pwr_mode) { > + if (!hba->dev_info.b_rpm_dev_flush_capable) { > ret = ufshcd_set_dev_pwr_mode(hba, req_dev_pwr_mode); > if (ret) > goto enable_gating; > @@ -8295,9 +8313,16 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op) > if (hba->clk_scaling.is_allowed) > ufshcd_resume_clkscaling(hba); > hba->clk_gating.is_suspended = false; > + hba->dev_info.b_rpm_dev_flush_capable = false; > ufshcd_release(hba); > out: > + if (hba->dev_info.b_rpm_dev_flush_capable) { > + schedule_delayed_work(&hba->rpm_dev_flush_recheck_work, > + msecs_to_jiffies(RPM_DEV_FLUSH_RECHECK_WORK_DELAY_MS)); > + } > + > hba->pm_op_in_progress = 0; > + Nitpick; newline, perhaps? > if (ret) > ufshcd_update_reg_hist(&hba->ufs_stats.suspend_err, (u32)ret); > return ret; > @@ -8386,6 +8411,11 @@ static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op) > /* Enable Auto-Hibernate if configured */ > ufshcd_auto_hibern8_enable(hba); > > + if (hba->dev_info.b_rpm_dev_flush_capable) { > + hba->dev_info.b_rpm_dev_flush_capable = false; > + cancel_delayed_work(&hba->rpm_dev_flush_recheck_work); > + } > + > /* Schedule clock gating in case of no access to UFS device yet */ > ufshcd_release(hba); > > @@ -8859,6 +8889,9 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq) > UFS_SLEEP_PWR_MODE, > UIC_LINK_HIBERN8_STATE); > > + INIT_DELAYED_WORK(&hba->rpm_dev_flush_recheck_work, > + ufshcd_rpm_dev_flush_recheck_work); > + > /* Set the default auto-hiberate idle timer value to 150 ms */ > if (ufshcd_is_auto_hibern8_supported(hba) && !hba->ahit) { > hba->ahit = FIELD_PREP(UFSHCI_AHIBERN8_TIMER_MASK, 150) | > diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h > index 8db7a6101892..9acd437037e8 100644 > --- a/drivers/scsi/ufs/ufshcd.h > +++ b/drivers/scsi/ufs/ufshcd.h > @@ -745,6 +745,7 @@ struct ufs_hba { > struct request_queue *bsg_queue; > bool wb_buf_flush_enabled; > bool wb_enabled; > + struct delayed_work rpm_dev_flush_recheck_work; > }; > > /* Returns true if clocks can be gated. Otherwise false */ > -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, Linux Foundation Collaborative Project _______________________________________________ Linux-mediatek mailing list Linux-mediatek@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-mediatek