| Message ID | 1548148630-7505-4-git-send-email-avri.altman@wdc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | scsi: ufs-bsg: Add read descriptor | expand |
On Fri, Jan 25, 2019 at 2:13 PM Avri Altman <evgreen@chromium.org> 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@wdc.com> > Reviewed-by: Christoph Hellwig <hch@lst.de> > --- > 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.
> > out: > > return 0; > > } > > - > > nit: This blank should be here. Done. > > > 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. The desc_len is being update in ufshcd.c: to the actual size that was read, And to 0 in the case of write descriptor. But I will add this check anyway here as it indeed improves the readability of the code. Thanks, Avri
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; } - 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); + kfree(desc_buff); out: