Re: [PATCH 102/117] ufs: Convert to the scsi_status union

Can Guo
2021-05-07T00:10:09Z

Hi Bart,

On 2021-04-20 10:13, Bart Van Assche wrote:
> An explanation of the purpose of this patch is available in the patch
> "scsi: Introduce the scsi_status union".
>
> Cc: Can Guo <cang@codeaurora.org>
> Cc: Yue Hu <huyue2@yulong.com>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
> drivers/scsi/ufs/ufs_bsg.c | 2 +-
> drivers/scsi/ufs/ufshcd.c | 27 +++++++++++++++------------
> 2 files changed, 16 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/scsi/ufs/ufs_bsg.c b/drivers/scsi/ufs/ufs_bsg.c
> index 5b2bc1a6f922..4dcc09136a50 100644
> --- a/drivers/scsi/ufs/ufs_bsg.c
> +++ b/drivers/scsi/ufs/ufs_bsg.c
> @@ -152,7 +152,7 @@ static int ufs_bsg_request(struct bsg_job *job)
> kfree(desc_buff);
>
> out:
> - bsg_reply->result = ret;
> + bsg_reply->status.combined = ret;
> job->reply_len = sizeof(struct ufs_bsg_reply);
> /* complete the job here only if no error */
> if (ret == 0)
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index d966d80838fb..cec555d3fcd7 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -4933,7 +4933,7 @@ ufshcd_scsi_cmd_status(struct ufshcd_lrb *lrbp,
> enum sam_status scsi_status)
> static inline int
> ufshcd_transfer_rsp_status(struct ufs_hba *hba, struct ufshcd_lrb
> *lrbp)
> {
> - int result = 0;
> + union scsi_status result;
> int ocs;
>
> /* overall command status of utrd */
> @@ -4951,7 +4951,7 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba,
> struct ufshcd_lrb *lrbp)
> switch (ufshcd_get_req_rsp(lrbp->ucd_rsp_ptr)) {
> case UPIU_TRANSACTION_RESPONSE:
> /* Propagate the SCSI status to the SCSI midlayer. */
> - result = ufshcd_scsi_cmd_status(lrbp,
> + result.combined = ufshcd_scsi_cmd_status(lrbp,
> ufshcd_get_rsp_upiu_result(lrbp->ucd_rsp_ptr) &
> MASK_SCSI_STATUS);
>
> @@ -4981,23 +4981,23 @@ ufshcd_transfer_rsp_status(struct ufs_hba
> *hba, struct ufshcd_lrb *lrbp)
> break;
> case UPIU_TRANSACTION_REJECT_UPIU:
> /* TODO: handle Reject UPIU Response */
> - result = DID_ERROR << 16;
> + result.combined = DID_ERROR << 16;
> dev_err(hba->dev,
> "Reject UPIU not fully implemented\n");
> break;
> default:
> dev_err(hba->dev,
> "Unexpected request response code = %x\n",
> - result);
> - result = DID_ERROR << 16;
> + result.combined);
> + result.combined = DID_ERROR << 16;
> break;
> }
> break;
> case OCS_ABORTED:
> - result |= DID_ABORT << 16;
> + result.combined |= DID_ABORT << 16;
> break;
> case OCS_INVALID_COMMAND_STATUS:
> - result |= DID_REQUEUE << 16;
> + result.combined |= DID_REQUEUE << 16;
> break;
> case OCS_INVALID_CMD_TABLE_ATTR:
> case OCS_INVALID_PRDT_ATTR:
> @@ -5009,7 +5009,7 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba,
> struct ufshcd_lrb *lrbp)
> case OCS_INVALID_CRYPTO_CONFIG:
> case OCS_GENERAL_CRYPTO_ERROR:
> default:
> - result |= DID_ERROR << 16;
> + result.combined |= DID_ERROR << 16;
> dev_err(hba->dev,
> "OCS error from controller = %x for tag %d\n",
> ocs, lrbp->task_tag);
> @@ -5021,7 +5021,7 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba,
> struct ufshcd_lrb *lrbp)
> if ((host_byte(result) != DID_OK) &&
> (host_byte(result) != DID_REQUEUE) && !hba->silence_err_logs)
> ufshcd_print_trs(hba, 1 << lrbp->task_tag, true);
> - return result;
> + return result.combined;
> }
>
> /**
> @@ -5083,7 +5083,7 @@ static void __ufshcd_transfer_req_compl(struct
> ufs_hba *hba,
> ufshcd_add_command_trace(hba, index, UFS_CMD_COMP);
> result = ufshcd_transfer_rsp_status(hba, lrbp);
> scsi_dma_unmap(cmd);
> - cmd->result = result;
> + cmd->status.combined = result;
> /* Mark completed command as NULL in LRB */
> lrbp->cmd = NULL;
> /* Do not touch lrbp after scsi done */
> @@ -8437,6 +8437,7 @@ static int ufshcd_set_dev_pwr_mode(struct ufs_hba
> *hba,
> struct scsi_sense_hdr sshdr;
> struct scsi_device *sdp;
> unsigned long flags;
> + union scsi_status start_stop_res;
> int ret;
>
> spin_lock_irqsave(hba->host->host_lock, flags);
> @@ -8471,13 +8472,15 @@ static int ufshcd_set_dev_pwr_mode(struct
> ufs_hba *hba,
> * callbacks hence set the RQF_PM flag so that it doesn't resume the
> * already suspended childs.
> */
> - ret = scsi_execute(sdp, cmd, DMA_NONE, NULL, 0, NULL, &sshdr,
> + start_stop_res.combined =
> + scsi_execute(sdp, cmd, DMA_NONE, NULL, 0, NULL, &sshdr,
> START_STOP_TIMEOUT, 0, 0, RQF_PM, NULL);
> + ret = start_stop_res.combined;
> if (ret) {
> sdev_printk(KERN_WARNING, sdp,
> "START_STOP failed for power mode: %d, result %x\n",
> pwr_mode, ret);
> - if (driver_byte(ret) == DRIVER_SENSE)
> + if (driver_byte(start_stop_res) == DRIVER_SENSE)
> scsi_print_sense_hdr(sdp, NULL, &sshdr);
> }

Thanks for the change, do you miss ufshcd_send_request_sense()?

Regards,
Can Guo.