[PATCH V3 07/15] usb: image: microtek: use sg helper to operate scatterlist

Finn Thain fthain at telegraphics.com.au
Fri Jun 14 05:39:10 UTC 2019


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 at neukum.org>
> Cc: Greg Kroah-Hartman <gregkh at linuxfoundation.org>
> Cc: linux-usb at vger.kernel.org
> Signed-off-by: Ming Lei <ming.lei at redhat.com>
> ---
>  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 */
>  };
> 


More information about the devel mailing list