| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 18 Mar 2021 08:10:57 +0000
From: Avri Altman <Avri.Altman@....com>
To: Yue Hu <zbestahu@...il.com>,
"alim.akhtar@...sung.com" <alim.akhtar@...sung.com>,
"martin.petersen@...cle.com" <martin.petersen@...cle.com>,
"jejb@...ux.ibm.com" <jejb@...ux.ibm.com>,
"linux-scsi@...r.kernel.org" <linux-scsi@...r.kernel.org>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"huyue2@...ong.com" <huyue2@...ong.com>,
"zbestahu@....com" <zbestahu@....com>
Subject: RE: [PATCH v2] scsi: ufs: Tidy up WB configuration code
>
> -int ufshcd_wb_ctrl(struct ufs_hba *hba, bool enable)
> +static int __ufshcd_wb_toggle(struct ufs_hba *hba, bool set, enum flag_idn
> idn)
> {
> - int ret;
> u8 index;
> enum query_opcode opcode;
What I meant is:
enum query_opcode opcode = set ? UPIU_QUERY_OPCODE_SET_FLAG :
UPIU_QUERY_OPCODE_CLEAR_FLAG;
Either way, you can add my Reviewed-by tag.
Thanks,
Avri
>
> + if (set)
> + opcode = UPIU_QUERY_OPCODE_SET_FLAG;
> + else
> + opcode = UPIU_QUERY_OPCODE_CLEAR_FLAG;
> +
> + index = ufshcd_wb_get_query_index(hba);
> + return ufshcd_query_flag_retry(hba, opcode, idn, index, NULL);
> +}
> +
> +int ufshcd_wb_toggle(struct ufs_hba *hba, bool enable)
> +{
> + int ret;
> +
> if (!ufshcd_is_wb_allowed(hba))
> return 0;
>
> if (!(enable ^ hba->dev_info.wb_enabled))
> return 0;
> - if (enable)
> - opcode = UPIU_QUERY_OPCODE_SET_FLAG;
> - else
> - opcode = UPIU_QUERY_OPCODE_CLEAR_FLAG;
>
> - index = ufshcd_wb_get_query_index(hba);
> - ret = ufshcd_query_flag_retry(hba, opcode,
> - QUERY_FLAG_IDN_WB_EN, index, NULL);
> + ret = __ufshcd_wb_toggle(hba, enable, QUERY_FLAG_IDN_WB_EN);
> if (ret) {
> - dev_err(hba->dev, "%s write booster %s failed %d\n",
> + dev_err(hba->dev, "%s Write Booster %s failed %d\n",
> __func__, enable ? "enable" : "disable", ret);
> return ret;
> }
>
> hba->dev_info.wb_enabled = enable;
> - dev_dbg(hba->dev, "%s write booster %s %d\n",
> - __func__, enable ? "enable" : "disable", ret);
> + dev_info(hba->dev, "%s Write Booster %s\n",
> + __func__, enable ? "enabled" : "disabled");
>
> return ret;
> }
>
> -static int ufshcd_wb_toggle_flush_during_h8(struct ufs_hba *hba, bool set)
> +static void ufshcd_wb_toggle_flush_during_h8(struct ufs_hba *hba, bool set)
> {
> - int val;
> - u8 index;
> -
> - if (set)
> - val = UPIU_QUERY_OPCODE_SET_FLAG;
> - else
> - val = UPIU_QUERY_OPCODE_CLEAR_FLAG;
> + int ret;
>
> - index = ufshcd_wb_get_query_index(hba);
> - return ufshcd_query_flag_retry(hba, val,
> - QUERY_FLAG_IDN_WB_BUFF_FLUSH_DURING_HIBERN8,
> - index, NULL);
> + ret = __ufshcd_wb_toggle(hba, set,
> + QUERY_FLAG_IDN_WB_BUFF_FLUSH_DURING_HIBERN8);
> + if (ret) {
> + dev_err(hba->dev, "%s: WB-Buf Flush during H8 %s failed: %d\n",
> + __func__, set ? "enable" : "disable", ret);
> + return;
> + }
> + dev_dbg(hba->dev, "%s WB-Buf Flush during H8 %s\n",
> + __func__, set ? "enabled" : "disabled");
> }
>
> -static inline int ufshcd_wb_toggle_flush(struct ufs_hba *hba, bool enable)
> +static inline void ufshcd_wb_toggle_flush(struct ufs_hba *hba, bool enable)
> {
> int ret;
> - u8 index;
> - enum query_opcode opcode;
>
> if (!ufshcd_is_wb_allowed(hba) ||
> hba->dev_info.wb_buf_flush_enabled == enable)
> - return 0;
> -
> - if (enable)
> - opcode = UPIU_QUERY_OPCODE_SET_FLAG;
> - else
> - opcode = UPIU_QUERY_OPCODE_CLEAR_FLAG;
> + return;
>
> - index = ufshcd_wb_get_query_index(hba);
> - ret = ufshcd_query_flag_retry(hba, opcode,
> - QUERY_FLAG_IDN_WB_BUFF_FLUSH_EN, index,
> - NULL);
> + ret = __ufshcd_wb_toggle(hba, enable,
> QUERY_FLAG_IDN_WB_BUFF_FLUSH_EN);
> if (ret) {
> dev_err(hba->dev, "%s WB-Buf Flush %s failed %d\n", __func__,
> enable ? "enable" : "disable", ret);
> - goto out;
> + return;
> }
>
> hba->dev_info.wb_buf_flush_enabled = enable;
>
> - dev_dbg(hba->dev, "WB-Buf Flush %s\n", enable ? "enabled" : "disabled");
> -out:
> - return ret;
> -
> + dev_dbg(hba->dev, "%s WB-Buf Flush %s\n",
> + __func__, enable ? "enabled" : "disabled");
> }
>
> static bool ufshcd_wb_presrv_usrspc_keep_vcc_on(struct ufs_hba *hba,
> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
> index 18e56c1..eddc819 100644
> --- a/drivers/scsi/ufs/ufshcd.h
> +++ b/drivers/scsi/ufs/ufshcd.h
> @@ -1099,7 +1099,7 @@ int ufshcd_exec_raw_upiu_cmd(struct ufs_hba
> *hba,
> u8 *desc_buff, int *buff_len,
> enum query_opcode desc_op);
>
> -int ufshcd_wb_ctrl(struct ufs_hba *hba, bool enable);
> +int ufshcd_wb_toggle(struct ufs_hba *hba, bool enable);
>
> /* Wrapper functions for safely calling variant operations */
> static inline const char *ufshcd_get_var_name(struct ufs_hba *hba)
> --
> 1.9.1
Powered by blists - more mailing lists