On Fri, 14 Jun 2019, Ming Lei wrote:
> Use the scatterlist iterators and remove direct indexing of the
> scatterlist array.
>
> This way allows us to pre-allocate one small scatterlist, which can be
> chained with one runtime allocated scatterlist if the pre-allocated one
> isn't enough for the whole request.
>
> Cc: Oliver Neukum <oliver@xxxxxxxxxx>
> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> Cc: linux-usb@xxxxxxxxxxxxxxx
> Signed-off-by: Ming Lei <ming.lei@xxxxxxxxxx>
> ---
> drivers/usb/image/microtek.c | 20 ++++++++------------
> drivers/usb/image/microtek.h | 2 +-
> 2 files changed, 9 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/usb/image/microtek.c b/drivers/usb/image/microtek.c
> index 607be1f4fe27..0a57c2cc8e5a 100644
> --- a/drivers/usb/image/microtek.c
> +++ b/drivers/usb/image/microtek.c
> @@ -488,7 +488,6 @@ static void mts_command_done( struct urb *transfer )
>
> static void mts_do_sg (struct urb* transfer)
> {
> - struct scatterlist * sg;
> int status = transfer->status;
> MTS_INT_INIT();
>
> @@ -500,13 +499,12 @@ static void mts_do_sg (struct urb* transfer)
> mts_transfer_cleanup(transfer);
> }
>
> - sg = scsi_sglist(context->srb);
> - context->fragment++;
> + context->curr_sg = sg_next(context->curr_sg);
> mts_int_submit_urb(transfer,
> context->data_pipe,
> - sg_virt(&sg[context->fragment]),
> - sg[context->fragment].length,
> - context->fragment + 1 == scsi_sg_count(context->srb) ?
> + sg_virt(context->curr_sg),
> + context->curr_sg->length,
> + sg_is_last(context->curr_sg) ?
> mts_data_done : mts_do_sg);
> }
>
> @@ -526,22 +524,20 @@ static void
> mts_build_transfer_context(struct scsi_cmnd *srb, struct mts_desc* desc)
> {
> int pipe;
> - struct scatterlist * sg;
> -
> +
> MTS_DEBUG_GOT_HERE();
>
> desc->context.instance = desc;
> desc->context.srb = srb;
> - desc->context.fragment = 0;
>
> if (!scsi_bufflen(srb)) {
> desc->context.data = NULL;
> desc->context.data_length = 0;
> return;
> } else {
> - sg = scsi_sglist(srb);
> - desc->context.data = sg_virt(&sg[0]);
> - desc->context.data_length = sg[0].length;
> + desc->context.curr_sg = scsi_sglist(srb);
> + desc->context.data = sg_virt(desc->context.curr_sg);
> + desc->context.data_length = desc->context.curr_sg->length;
> }
>
Would it not be better to initialize desc->context.curr_sg in both
branches of this conditional?
--
>
> diff --git a/drivers/usb/image/microtek.h b/drivers/usb/image/microtek.h
> index 66685e59241a..7bd5f4639c4a 100644
> --- a/drivers/usb/image/microtek.h
> +++ b/drivers/usb/image/microtek.h
> @@ -21,7 +21,7 @@ struct mts_transfer_context
> void *data;
> unsigned data_length;
> int data_pipe;
> - int fragment;
> + struct scatterlist *curr_sg;
>
> u8 *scsi_status; /* status returned from ep_response after command completion */
> };
>
