On 2020-08-06 02:11, Daejun Park wrote:
> +static void ufshpb_issue_hpb_reset_query(struct ufs_hba *hba)
> +{
> + int err;
> + int retries;
> +
> + for (retries = 0; retries < HPB_RESET_REQ_RETRIES; retries++) {
> + err = ufshcd_query_flag(hba, UPIU_QUERY_OPCODE_SET_FLAG,
> + QUERY_FLAG_IDN_HPB_RESET, 0, NULL);
> + if (err)
> + dev_dbg(hba->dev,
> + "%s: failed with error %d, retries %d\n",
> + __func__, err, retries);
> + else
> + break;
> + }
> +
> + if (err) {
> + dev_err(hba->dev,
> + "%s setting fHpbReset flag failed with error %d\n",
> + __func__, err);
> + return;
> + }
> +}
Please change the "break" into an early return, remove the last
occurrence "if (err)" and remove the final return statement.
> +static void ufshpb_check_hpb_reset_query(struct ufs_hba *hba)
> +{
> + int err;
> + bool flag_res = true;
> + int try = 0;
> +
> + /* wait for the device to complete HPB reset query */
> + do {
> + if (++try == HPB_RESET_REQ_RETRIES)
> + break;
> +
> + dev_info(hba->dev,
> + "%s start flag reset polling %d times\n",
> + __func__, try);
> +
> + /* Poll fHpbReset flag to be cleared */
> + err = ufshcd_query_flag(hba, UPIU_QUERY_OPCODE_READ_FLAG,
> + QUERY_FLAG_IDN_HPB_RESET, 0, &flag_res);
> + usleep_range(1000, 1100);
> + } while (flag_res);
> +
> + if (err) {
> + dev_err(hba->dev,
> + "%s reading fHpbReset flag failed with error %d\n",
> + __func__, err);
> + return;
> + }
> +
> + if (flag_res) {
> + dev_err(hba->dev,
> + "%s fHpbReset was not cleared by the device\n",
> + __func__);
> + }
> +}
Should "polling %d times" perhaps be changed into "attempt %d"?
The "if (err)" statement may be reached without "err" having been
initialized. Please fix.
Additionally, please change the do-while loop into a for-loop, e.g. as
follows:
for (try = 0; try < HPB_RESET_REQ_RETRIES; try++)
...
Thanks,
Bart.
