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

[Christoph Hellwig]

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);
}