> > ---
> > drivers/scsi/ufs/ufshcd.c | 35 ++++++++++++++++++++++++++---------
> > 1 file changed, 26 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> > index d18832a..15be103 100644
> > --- a/drivers/scsi/ufs/ufshcd.c
> > +++ b/drivers/scsi/ufs/ufshcd.c
> > @@ -5599,6 +5599,7 @@ static int ufshcd_clear_tm_cmd(struct ufs_hba
> *hba, int tag)
> > }
> >
> > static void ufshcd_fill_tm_req(struct utp_task_req_desc *task_req_descp,
> > + struct utp_upiu_task_req *raw_task_req_upiup,
> > int lun_id, int task_id, u8 tm_function,
> > int task_tag)
> > {
> > @@ -5609,6 +5610,13 @@ static void ufshcd_fill_tm_req(struct
> utp_task_req_desc *task_req_descp,
> > task_req_descp->header.dword_2 =
> > cpu_to_le32(OCS_INVALID_COMMAND_STATUS);
> >
> > + if (raw_task_req_upiup) {
> > + raw_task_req_upiup->header.dword_0 |=
> cpu_to_be32(task_tag);
> > + memcpy(task_req_descp->task_req_upiu,
> raw_task_req_upiup,
> > + GENERAL_UPIU_REQUEST_SIZE);
> > + return;
> > + }
>
> The only thing actually shared here is initializing two fields. Why
> can't we always pass in the a raw
See below
>
> > +static int ufshcd_issue_tm_cmd(struct ufs_hba *hba,
> > + struct utp_upiu_task_req *raw_task_req_upiup,
> > + struct utp_upiu_task_req *raw_task_rsp_upiup,
> > + int lun_id, int task_id, u8 tm_function,
> > + u8 *tm_response)
> > {
>
> And the architecture here seems similar odd. I think someone needs
> to untangle how the data structures are used in this part of the code.
>
> E.g. aways prepare a raw structure in the caller (possibly on stack)
> and pass it into ufshcd_issue_tm_cmd, which copies it into the
> DMAable region.
Task management is sent on device reset and task abort.
This is actually a rare event, and on some platforms, I find it quite difficult to trigger.
So preparing the request upiu as a separate step seems a little bit excessive.
Another reason why it is a good idea to do it together, Is that both
preparing the task request upiu and ringing its doorbell are done under
the same host lock.
We can however, as you proposed, prepare a "raw" upiu if none is given,
But I think we'll end up with the same amount of code,
Maybe even less obvious than filling the upiu in one place.
