On Fri, Jan 25, 2019 at 2:13 PM Avri Altman <evgreen@xxxxxxxxxxxx> wrote:
>
> Add this functionality, placing the descriptor being read in the actual
> data buffer in the bio.
>
> That is, for both read and write descriptors query upiu, we are using
> the job's request_payload. This in turn, is mapped back in user land to
> the applicable sg_io_v4 xferp: dout_xferp for write descriptor,
> and din_xferp for read descriptor.
>
> Signed-off-by: Avri Altman <avri.altman@xxxxxxx>
> Reviewed-by: Christoph Hellwig <hch@xxxxxx>
> ---
> Documentation/scsi/ufs.txt | 7 ++++++-
> drivers/scsi/ufs/ufs_bsg.c | 21 ++++++++++++---------
> 2 files changed, 18 insertions(+), 10 deletions(-)
>
> diff --git a/Documentation/scsi/ufs.txt b/Documentation/scsi/ufs.txt
> index 78fe7cb..1769f71 100644
> --- a/Documentation/scsi/ufs.txt
> +++ b/Documentation/scsi/ufs.txt
> @@ -150,9 +150,14 @@ send SG_IO with the applicable sg_io_v4:
> if (dir == SG_DXFER_TO_DEV) {
> io_hdr_v4.dout_xfer_len = (uint32_t)byte_cnt;
> io_hdr_v4.dout_xferp = (uintptr_t)(__u64)buff;
> + } else {
> + io_hdr_v4.din_xfer_len = (uint32_t)byte_cnt;
> + io_hdr_v4.din_xferp = (uintptr_t)(__u64)buff;
> }
>
> -If you wish to write a descriptor, use the dout_xferp sg_io_v4.
> +If you wish to read or write a descriptor, use the appropriate xferp of
> +sg_io_v4.
> +
>
> UFS Specifications can be found at,
> UFS - http://www.jedec.org/sites/default/files/docs/JESD220.pdf
> diff --git a/drivers/scsi/ufs/ufs_bsg.c b/drivers/scsi/ufs/ufs_bsg.c
> index 2fd0769..52cfd49 100644
> --- a/drivers/scsi/ufs/ufs_bsg.c
> +++ b/drivers/scsi/ufs/ufs_bsg.c
> @@ -48,12 +48,8 @@ static int ufs_bsg_alloc_desc_buffer(struct ufs_hba *hba, struct bsg_job *job,
> struct utp_upiu_query *qr;
> u8 *descp;
>
> - if (desc_op == UPIU_QUERY_OPCODE_READ_DESC) {
> - dev_err(hba->dev, "unsupported opcode %d\n", desc_op);
> - return -ENOTSUPP;
> - }
> -
> - if (desc_op != UPIU_QUERY_OPCODE_WRITE_DESC)
> + if (desc_op != UPIU_QUERY_OPCODE_WRITE_DESC &&
> + desc_op != UPIU_QUERY_OPCODE_READ_DESC)
> goto out;
>
> qr = &bsg_request->upiu_req.qr;
> @@ -71,15 +67,16 @@ static int ufs_bsg_alloc_desc_buffer(struct ufs_hba *hba, struct bsg_job *job,
> if (!descp)
> return -ENOMEM;
>
> - sg_copy_to_buffer(job->request_payload.sg_list,
> - job->request_payload.sg_cnt, descp, *desc_len);
> + if (desc_op == UPIU_QUERY_OPCODE_WRITE_DESC)
> + sg_copy_to_buffer(job->request_payload.sg_list,
> + job->request_payload.sg_cnt, descp,
> + *desc_len);
>
> *desc_buff = descp;
>
> out:
> return 0;
> }
> -
nit: This blank should be here.
> static int ufs_bsg_request(struct bsg_job *job)
> {
> struct ufs_bsg_request *bsg_request = job->request;
> @@ -140,6 +137,12 @@ static int ufs_bsg_request(struct bsg_job *job)
> if (!desc_buff)
> goto out;
>
> + if (desc_len)
> + bsg_reply->reply_payload_rcv_len =
> + sg_copy_from_buffer(job->request_payload.sg_list,
> + job->request_payload.sg_cnt,
> + desc_buff, desc_len);
> +
So you copy the buffer to request_payload even if it was a write
request? I guess that should be okay? I just expected to see a check
of desc_op mirroring the one you had for the read case in
ufs_bsg_alloc_desc_buffer.
