From patchwork Mon Mar 16 06:20:51 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Can Guo X-Patchwork-Id: 11439805 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 785C817E6 for ; Mon, 16 Mar 2020 07:00:13 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 454E920575 for ; Mon, 16 Mar 2020 07:00:13 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729907AbgCPHAK (ORCPT ); Mon, 16 Mar 2020 03:00:10 -0400 Received: from labrats.qualcomm.com ([199.106.110.90]:14037 "EHLO labrats.qualcomm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729722AbgCPHAK (ORCPT ); Mon, 16 Mar 2020 03:00:10 -0400 IronPort-SDR: V9+lcIGj8fAWHhmms8a2GKflCIdZk6RdrPSbVpQer0CkLu+pZfAAiwBaJEBgdKQBNIWI3RHqJf zvLWS3H5+pE9cACVYLU5Kh2v+HeddUaUZ4qGnjNZl0jq33I4A7j1Dgsl6kse3LFTsjoz6QqkhX QWW3vbpdAWmFf7KMH+nQ25NMy+nS1/x6AIIh6NDmTbAVTkmjBhbjbNn7ZuQhMjSPIROwLkRD33 5EVxHTtTvmoT1q0Wewa8aoMw+odxmpBLQAU5KdOOwheXNKV8m3YBunhP+CH6syitsc4Qrt9xWt bck= X-IronPort-AV: E=Sophos;i="5.70,559,1574150400"; d="scan'208";a="28594804" Received: from unknown (HELO ironmsg04-sd.qualcomm.com) ([10.53.140.144]) by labrats.qualcomm.com with ESMTP; 15 Mar 2020 23:21:04 -0700 Received: from pacamara-linux.qualcomm.com ([192.168.140.135]) by ironmsg04-sd.qualcomm.com with ESMTP; 15 Mar 2020 23:21:03 -0700 Received: by pacamara-linux.qualcomm.com (Postfix, from userid 359480) id 1F1AD3A61; Sun, 15 Mar 2020 23:21:03 -0700 (PDT) From: Can Guo To: asutoshd@codeaurora.org, nguyenb@codeaurora.org, hongwus@codeaurora.org, rnayak@codeaurora.org, linux-scsi@vger.kernel.org, kernel-team@android.com, saravanak@google.com, salyzyn@google.com, cang@codeaurora.org Cc: Subhash Jadavani , Alim Akhtar , Avri Altman , "James E.J. Bottomley" , "Martin K. Petersen" , Stanley Chu , Bean Huo , Bart Van Assche , Venkat Gopalakrishnan , Tomas Winkler , linux-kernel@vger.kernel.org (open list) Subject: [PATCH 1/2] scsi: ufs: Clean up ufshcd_scale_clks() and clock scaling error out path Date: Sun, 15 Mar 2020 23:20:51 -0700 Message-Id: <1584339655-20337-2-git-send-email-cang@codeaurora.org> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1584339655-20337-1-git-send-email-cang@codeaurora.org> References: <1584339655-20337-1-git-send-email-cang@codeaurora.org> Sender: linux-scsi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org From: Subhash Jadavani This change introduces a func ufshcd_set_clk_freq() to explicitly set clock frequency so that it can be used in reset_and_resotre path and in ufshcd_scale_clks(). Meanwhile, this change cleans up the clock scaling error out path. Signed-off-by: Subhash Jadavani Signed-off-by: Can Guo diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index 2a2a63b..63aaa88f 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -855,28 +855,29 @@ static bool ufshcd_is_unipro_pa_params_tuning_req(struct ufs_hba *hba) return false; } -static int ufshcd_scale_clks(struct ufs_hba *hba, bool scale_up) +/** + * ufshcd_set_clk_freq - set UFS controller clock frequencies + * @hba: per adapter instance + * @scale_up: If True, set max possible frequency othewise set low frequency + * + * Returns 0 if successful + * Returns < 0 for any other errors + */ +static int ufshcd_set_clk_freq(struct ufs_hba *hba, bool scale_up) { int ret = 0; struct ufs_clk_info *clki; struct list_head *head = &hba->clk_list_head; - ktime_t start = ktime_get(); - bool clk_state_changed = false; if (list_empty(head)) goto out; - ret = ufshcd_vops_clk_scale_notify(hba, scale_up, PRE_CHANGE); - if (ret) - return ret; - list_for_each_entry(clki, head, list) { if (!IS_ERR_OR_NULL(clki->clk)) { if (scale_up && clki->max_freq) { if (clki->curr_freq == clki->max_freq) continue; - clk_state_changed = true; ret = clk_set_rate(clki->clk, clki->max_freq); if (ret) { dev_err(hba->dev, "%s: %s clk set rate(%dHz) failed, %d\n", @@ -895,7 +896,6 @@ static int ufshcd_scale_clks(struct ufs_hba *hba, bool scale_up) if (clki->curr_freq == clki->min_freq) continue; - clk_state_changed = true; ret = clk_set_rate(clki->clk, clki->min_freq); if (ret) { dev_err(hba->dev, "%s: %s clk set rate(%dHz) failed, %d\n", @@ -914,13 +914,36 @@ static int ufshcd_scale_clks(struct ufs_hba *hba, bool scale_up) clki->name, clk_get_rate(clki->clk)); } +out: + return ret; +} + +/** + * ufshcd_scale_clks - scale up or scale down UFS controller clocks + * @hba: per adapter instance + * @scale_up: True if scaling up and false if scaling down + * + * Returns 0 if successful + * Returns < 0 for any other errors + */ +static int ufshcd_scale_clks(struct ufs_hba *hba, bool scale_up) +{ + int ret = 0; + + ret = ufshcd_vops_clk_scale_notify(hba, scale_up, PRE_CHANGE); + if (ret) + return ret; + + ret = ufshcd_set_clk_freq(hba, scale_up); + if (ret) + return ret; + ret = ufshcd_vops_clk_scale_notify(hba, scale_up, POST_CHANGE); + if (ret) { + ufshcd_set_clk_freq(hba, !scale_up); + return ret; + } -out: - if (clk_state_changed) - trace_ufshcd_profile_clk_scaling(dev_name(hba->dev), - (scale_up ? "up" : "down"), - ktime_to_us(ktime_sub(ktime_get(), start)), ret); return ret; } @@ -1106,35 +1129,36 @@ static int ufshcd_devfreq_scale(struct ufs_hba *hba, bool scale_up) ret = ufshcd_clock_scaling_prepare(hba); if (ret) - return ret; + goto out; /* scale down the gear before scaling down clocks */ if (!scale_up) { ret = ufshcd_scale_gear(hba, false); if (ret) - goto out; + goto clk_scaling_unprepare; } ret = ufshcd_scale_clks(hba, scale_up); - if (ret) { - if (!scale_up) - ufshcd_scale_gear(hba, true); - goto out; - } + if (ret) + goto scale_up_gear; /* scale up the gear after scaling up clocks */ if (scale_up) { ret = ufshcd_scale_gear(hba, true); if (ret) { ufshcd_scale_clks(hba, false); - goto out; + goto clk_scaling_unprepare; } } - ret = ufshcd_vops_clk_scale_notify(hba, scale_up, POST_CHANGE); + goto clk_scaling_unprepare; -out: +scale_up_gear: + if (!scale_up) + ufshcd_scale_gear(hba, true); +clk_scaling_unprepare: ufshcd_clock_scaling_unprepare(hba); +out: ufshcd_release(hba); return ret; } @@ -6251,7 +6275,7 @@ static int ufshcd_host_reset_and_restore(struct ufs_hba *hba) spin_unlock_irqrestore(hba->host->host_lock, flags); /* scale up clocks to max frequency before full reinitialization */ - ufshcd_scale_clks(hba, true); + ufshcd_set_clk_freq(hba, true); err = ufshcd_hba_enable(hba); if (err) From patchwork Mon Mar 16 06:20:52 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Can Guo X-Patchwork-Id: 11449317 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id AE349139A for ; Fri, 20 Mar 2020 14:24:54 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 966622070A for ; Fri, 20 Mar 2020 14:24:54 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727351AbgCTOYv (ORCPT ); Fri, 20 Mar 2020 10:24:51 -0400 Received: from labrats.qualcomm.com ([199.106.110.90]:9007 "EHLO labrats.qualcomm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727128AbgCTOYv (ORCPT ); Fri, 20 Mar 2020 10:24:51 -0400 IronPort-SDR: KXdCz/c+E28b++i04tbA1hUhmr2u6IKKr5IZwp6u4IxBjC473fkiz0MWcWHndlUWrI1e79BTHc UyXOqZCo8OVhVBxaD6+c3QIMD4BliWknc5YczJ8vcYt3GkOpy3eLgwam1RPjwhIAVrkdUE5OIu z1KK5bIlt4MBTP3Titq9LF3yidFqqRxXgWRTps3LoeyMJpilLocIsB+tHWPNoUH+ZUtu6xeGVl AjUOokmAvdm4ZNsT2LyHtV1Bd52TPOximN2EtsP2d5NjRGIrQhHhc5vFV6mRx5biMjqr7FTqq8 ZCY= X-IronPort-AV: E=Sophos;i="5.70,559,1574150400"; d="scan'208";a="28594805" Received: from unknown (HELO ironmsg05-sd.qualcomm.com) ([10.53.140.145]) by labrats.qualcomm.com with ESMTP; 15 Mar 2020 23:21:12 -0700 Received: from pacamara-linux.qualcomm.com ([192.168.140.135]) by ironmsg05-sd.qualcomm.com with ESMTP; 15 Mar 2020 23:21:11 -0700 Received: by pacamara-linux.qualcomm.com (Postfix, from userid 359480) id A00EA3A61; Sun, 15 Mar 2020 23:21:11 -0700 (PDT) From: Can Guo To: asutoshd@codeaurora.org, nguyenb@codeaurora.org, hongwus@codeaurora.org, rnayak@codeaurora.org, linux-scsi@vger.kernel.org, kernel-team@android.com, saravanak@google.com, salyzyn@google.com, cang@codeaurora.org Cc: Alim Akhtar , Avri Altman , "James E.J. Bottomley" , "Martin K. Petersen" , Stanley Chu , Bean Huo , Bart Van Assche , Venkat Gopalakrishnan , Tomas Winkler , Bjorn Andersson , linux-kernel@vger.kernel.org (open list) Subject: [PATCH 2/2] scsi: ufs: Do not rely on prefetched data Date: Sun, 15 Mar 2020 23:20:52 -0700 Message-Id: <1584339655-20337-3-git-send-email-cang@codeaurora.org> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1584339655-20337-1-git-send-email-cang@codeaurora.org> References: <1584339655-20337-1-git-send-email-cang@codeaurora.org> Sender: linux-scsi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org We were setting bActiveICCLevel attribute for UFS device only once but type of this attribute has changed from persistent to volatile since UFS device specification v2.1. This attribute is set to the default value after power cycle or hardware reset event. It isn't safe to rely on prefetched data (only used for bActiveICCLevel attribute now). Hence this change removes the code related to data prefetching and set this parameter on every attempt to probe the UFS device. Signed-off-by: Can Guo diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index 63aaa88f..99c7fca 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -6458,11 +6458,12 @@ static u32 ufshcd_find_max_sup_active_icc_level(struct ufs_hba *hba, return icc_level; } -static void ufshcd_init_icc_levels(struct ufs_hba *hba) +static void ufshcd_set_active_icc_lvl(struct ufs_hba *hba) { int ret; int buff_len = hba->desc_size.pwr_desc; u8 *desc_buf; + u32 icc_level; desc_buf = kmalloc(buff_len, GFP_KERNEL); if (!desc_buf) @@ -6477,20 +6478,17 @@ static void ufshcd_init_icc_levels(struct ufs_hba *hba) goto out; } - hba->init_prefetch_data.icc_level = - ufshcd_find_max_sup_active_icc_level(hba, - desc_buf, buff_len); - dev_dbg(hba->dev, "%s: setting icc_level 0x%x", - __func__, hba->init_prefetch_data.icc_level); + icc_level = ufshcd_find_max_sup_active_icc_level(hba, desc_buf, + buff_len); + dev_dbg(hba->dev, "%s: setting icc_level 0x%x", __func__, icc_level); ret = ufshcd_query_attr_retry(hba, UPIU_QUERY_OPCODE_WRITE_ATTR, - QUERY_ATTR_IDN_ACTIVE_ICC_LVL, 0, 0, - &hba->init_prefetch_data.icc_level); + QUERY_ATTR_IDN_ACTIVE_ICC_LVL, 0, 0, &icc_level); if (ret) dev_err(hba->dev, "%s: Failed configuring bActiveICCLevel = %d ret = %d", - __func__, hba->init_prefetch_data.icc_level , ret); + __func__, icc_level, ret); out: kfree(desc_buf); @@ -6996,8 +6994,6 @@ static int ufshcd_add_lus(struct ufs_hba *hba) { int ret; - ufshcd_init_icc_levels(hba); - /* Add required well known logical units to scsi mid layer */ ret = ufshcd_scsi_add_wlus(hba); if (ret) @@ -7095,6 +7091,14 @@ static int ufshcd_probe_hba(struct ufs_hba *hba, bool async) } } + /* + * bActiveICCLevel is volatile for UFS device (as per latest v2.1 spec) + * and for removable UFS card as well, hence always set the parameter. + * Note: Error handler may issue the device reset hence resetting + * bActiveICCLevel as well so it is always safe to set this here. + */ + ufshcd_set_active_icc_lvl(hba); + /* set the state as operational after switching to desired gear */ hba->ufshcd_state = UFSHCD_STATE_OPERATIONAL; diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h index d45a044..5652d39 100644 --- a/drivers/scsi/ufs/ufshcd.h +++ b/drivers/scsi/ufs/ufshcd.h @@ -404,15 +404,6 @@ struct ufs_clk_scaling { bool is_suspended; }; -/** - * struct ufs_init_prefetch - contains data that is pre-fetched once during - * initialization - * @icc_level: icc level which was read during initialization - */ -struct ufs_init_prefetch { - u32 icc_level; -}; - #define UFS_ERR_REG_HIST_LENGTH 8 /** * struct ufs_err_reg_hist - keeps history of errors @@ -544,7 +535,6 @@ enum ufshcd_quirks { * @intr_mask: Interrupt Mask Bits * @ee_ctrl_mask: Exception event control mask * @is_powered: flag to check if HBA is powered - * @init_prefetch_data: data pre-fetched during initialization * @eh_work: Worker to handle UFS errors that require s/w attention * @eeh_work: Worker to handle exception events * @errors: HBA errors @@ -632,7 +622,6 @@ struct ufs_hba { u32 intr_mask; u16 ee_ctrl_mask; bool is_powered; - struct ufs_init_prefetch init_prefetch_data; /* Work Queues */ struct work_struct eh_work;