From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.7 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id EAC6FC31E44 for ; Fri, 14 Jun 2019 05:39:07 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id C329A2133D for ; Fri, 14 Jun 2019 05:39:07 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1725869AbfFNFjG (ORCPT ); Fri, 14 Jun 2019 01:39:06 -0400 Received: from kvm5.telegraphics.com.au ([98.124.60.144]:55994 "EHLO kvm5.telegraphics.com.au" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725808AbfFNFjG (ORCPT ); Fri, 14 Jun 2019 01:39:06 -0400 Received: from localhost (localhost.localdomain [127.0.0.1]) by kvm5.telegraphics.com.au (Postfix) with ESMTP id C026922BFB; Fri, 14 Jun 2019 01:39:01 -0400 (EDT) Date: Fri, 14 Jun 2019 15:39:10 +1000 (AEST) From: Finn Thain To: Ming Lei cc: linux-scsi@vger.kernel.org, "Martin K . Petersen" , James Bottomley , Bart Van Assche , Hannes Reinecke , Christoph Hellwig , Jim Gill , Cathy Avery , "Ewan D . Milne" , Brian King , James Smart , "Juergen E . Fischer" , Michael Schmitz , Greg Kroah-Hartman , devel@driverdev.osuosl.org, linux-usb@vger.kernel.org, Dan Carpenter , Benjamin Block , Oliver Neukum Subject: Re: [PATCH V3 07/15] usb: image: microtek: use sg helper to operate scatterlist In-Reply-To: <20190614025316.7360-8-ming.lei@redhat.com> Message-ID: References: <20190614025316.7360-1-ming.lei@redhat.com> <20190614025316.7360-8-ming.lei@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Sender: linux-usb-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-usb@vger.kernel.org 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 > Cc: Greg Kroah-Hartman > Cc: linux-usb@vger.kernel.org > Signed-off-by: Ming Lei > --- > 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 */ > }; >