Re: [PATCH v3 3/7] scsi: ufs: Add fill task management request

[Christoph Hellwig <hch@xxxxxx>]

In general this looks good, but a question below:

> index ed37914..d18832a 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -5598,6 +5598,32 @@ static int ufshcd_clear_tm_cmd(struct ufs_hba *hba, int tag)
>  	return err;
>  }
>  
> +static void ufshcd_fill_tm_req(struct utp_task_req_desc *task_req_descp,
> +			       int lun_id, int task_id, u8 tm_function,
> +			       int task_tag)
> +{
> +	struct utp_upiu_task_req *task_req_upiup;
> +
> +	/* Configure task request descriptor */
> +	task_req_descp->header.dword_0 = cpu_to_le32(UTP_REQ_DESC_INT_CMD);
> +	task_req_descp->header.dword_2 =
> +			cpu_to_le32(OCS_INVALID_COMMAND_STATUS);
> +
> +	task_req_upiup =
> +		(struct utp_upiu_task_req *)task_req_descp->task_req_upiu;


Why is task_req_upiu a __le32 array instead of using the proper
structure?

Doing that would nicely clean up the code:

static void ufshcd_fill_tm_req(struct utp_task_req_desc *treq,
			       int lun_id, int task_id, u8 tm_function,
			       int task_tag)
{
	/* Configure task request descriptor */
	treq->header.dword_0 = cpu_to_le32(UTP_REQ_DESC_INT_CMD);
	treq->header.dword_2 = cpu_to_le32(OCS_INVALID_COMMAND_STATUS);

	treq->task_req_upiup->header.dword_0 =
		UPIU_HEADER_DWORD(UPIU_TRANSACTION_TASK_REQ, 0, lun_id,
				  task_tag);
	treq->task_req_upiup->header.dword_1 =
		UPIU_HEADER_DWORD(0, tm_function, 0, 0);
	treq->task_req_upiup->input_param1 = cpu_to_be32(lun_id);
	treq->task_req_upiup->input_param2 = cpu_to_be32(task_id);
}