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=-15.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 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 DC000C49EAB for ; Sun, 27 Jun 2021 14:23:58 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id C523D619C6 for ; Sun, 27 Jun 2021 14:23:58 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231189AbhF0O0V (ORCPT ); Sun, 27 Jun 2021 10:26:21 -0400 Received: from netrider.rowland.org ([192.131.102.5]:45673 "HELO netrider.rowland.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S230268AbhF0O0U (ORCPT ); Sun, 27 Jun 2021 10:26:20 -0400 Received: (qmail 625898 invoked by uid 1000); 27 Jun 2021 10:23:55 -0400 Date: Sun, 27 Jun 2021 10:23:55 -0400 From: Alan Stern To: Igor Kononenko Cc: Felipe Balbi , Greg Kroah-Hartman , openbmc@lists.ozlabs.org, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 2/6] usb:gadget:mass-storage: refactoring the SCSI command handling Message-ID: <20210627142355.GD624763@rowland.harvard.edu> References: <20210626211820.107310-1-i.kononenko@yadro.com> <20210626211820.107310-3-i.kononenko@yadro.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210626211820.107310-3-i.kononenko@yadro.com> User-Agent: Mutt/1.10.1 (2018-07-13) Precedence: bulk List-ID: X-Mailing-List: linux-usb@vger.kernel.org On Sun, Jun 27, 2021 at 12:18:15AM +0300, Igor Kononenko wrote: > Implements a universal way to define SCSI commands and configure > precheck handlers. What is the reason for doing this? At first glance, it appears you have added a great deal of complexity to the driver. The patch replaces a large amount of easily understood (albeit rather repetitious) code with an approximately equal amount of rather complicated code. This does not seem like an improvement. Furthermore, the code you removed is flexible; it easily allows for small variations as neede by some command handlers. But the code you added is all table-driven, which does not easily permit arbitrary variations. > Tested: By probing the USBGadget Mass-Storage on the YADRO VEGMAN > BMC(AST2500) sample, each SCSI command was sent through HOST->BMC; the > USBGadget MassStorage debug print showed all sent commands works > properly. > > Signed-off-by: Igor Kononenko > --- > drivers/usb/gadget/function/f_mass_storage.c | 540 +++++++++++-------- > drivers/usb/gadget/function/storage_common.h | 5 + > 2 files changed, 310 insertions(+), 235 deletions(-) I don't see the point of adding 75 lines to the kernel source if they don't accomplish anything new. Alan Stern