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=-8.2 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_AGENT_MUTT 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 4FA14C31E44 for ; Fri, 14 Jun 2019 06:58:00 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 2F63F2063F for ; Fri, 14 Jun 2019 06:58:00 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1725996AbfFNG56 (ORCPT ); Fri, 14 Jun 2019 02:57:58 -0400 Received: from mx1.redhat.com ([209.132.183.28]:34620 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725837AbfFNG56 (ORCPT ); Fri, 14 Jun 2019 02:57:58 -0400 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id F32B259463; Fri, 14 Jun 2019 06:57:57 +0000 (UTC) Received: from ming.t460p (ovpn-8-17.pek2.redhat.com [10.72.8.17]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 7AC7A39C3; Fri, 14 Jun 2019 06:57:43 +0000 (UTC) Date: Fri, 14 Jun 2019 14:57:38 +0800 From: Ming Lei To: Finn Thain 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 Message-ID: <20190614065737.GB24393@ming.t460p> 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 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.11.3 (2019-02-01) X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Fri, 14 Jun 2019 06:57:58 +0000 (UTC) Sender: linux-usb-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-usb@vger.kernel.org On Fri, Jun 14, 2019 at 03:39:10PM +1000, Finn Thain wrote: > 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? I think either way is fine given desc->context.curr_sg is used only if 'context->data' isn't NULL, see mts_command_done(). So I'd keep the patch as it is. Thanks, Ming