Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752399AbdDDCcl (ORCPT ); Mon, 3 Apr 2017 22:32:41 -0400 Received: from gateway34.websitewelcome.com ([192.185.148.194]:41565 "EHLO gateway34.websitewelcome.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751362AbdDDCcj (ORCPT ); Mon, 3 Apr 2017 22:32:39 -0400 X-Greylist: delayed 1271 seconds by postgrey-1.27 at vger.kernel.org; Mon, 03 Apr 2017 22:32:39 EDT Date: Mon, 03 Apr 2017 21:11:25 -0500 Message-ID: <20170403211125.Horde._t9sGRmSsqLqwLXQzC5_O7h@gator4166.hostgator.com> From: "Gustavo A. R. Silva" To: Alan Stern Cc: Greg Kroah-Hartman , Felipe Balbi , Peter Chen , Chunfeng Yun , Mathias Nyman , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, Peter Senna Tschudin Subject: Re: [PATCH 2/2] usb: misc: refactor code References: <20170403195042.GA12225@embeddedgus> In-Reply-To: User-Agent: Horde Application Framework 5 Content-Type: text/plain; charset=utf-8; format=flowed; DelSp=Yes MIME-Version: 1.0 Content-Disposition: inline X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - gator4166.hostgator.com X-AntiAbuse: Original Domain - vger.kernel.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - embeddedor.com X-BWhitelist: no X-Source-IP: 108.167.133.22 X-Exim-ID: 1cvDwL-000Tt7-BR X-Source: X-Source-Args: X-Source-Dir: X-Source-Sender: gator4166.hostgator.com [108.167.133.22]:49077 X-Source-Auth: garsilva@embeddedor.com X-Email-Count: 2 X-Source-Cap: Z3V6aWRpbmU7Z3V6aWRpbmU7Z2F0b3I0MTY2Lmhvc3RnYXRvci5jb20= Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4288 Lines: 165 Hi Alan, Quoting Alan Stern : > On Mon, 3 Apr 2017, Gustavo A. R. Silva wrote: > >> Code refactoring to make the flow easier to follow. >> >> Cc: Alan Stern >> Cc: Greg Kroah-Hartman >> Signed-off-by: Gustavo A. R. Silva >> --- >> drivers/usb/misc/usbtest.c | 67 >> +++++++++++++++++++++------------------------- >> 1 file changed, 30 insertions(+), 37 deletions(-) >> >> diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c >> index 7bfb6b78..382491e 100644 >> --- a/drivers/usb/misc/usbtest.c >> +++ b/drivers/usb/misc/usbtest.c >> @@ -124,18 +124,32 @@ static struct usb_device >> *testdev_to_usbdev(struct usbtest_dev *test) >> >> >> /*-------------------------------------------------------------------------*/ >> >> +static inline void endpoint_update(int edi, >> + struct usb_host_endpoint **in, >> + struct usb_host_endpoint **out, >> + struct usb_host_endpoint *e) >> +{ >> + if (edi) { >> + if (!*in) >> + *in = e; >> + } else { >> + if (!*out) >> + *out = e; >> + } >> +} >> + >> static int >> get_endpoints(struct usbtest_dev *dev, struct usb_interface *intf) >> { >> - int tmp; >> - struct usb_host_interface *alt; >> - struct usb_host_endpoint *in, *out; >> - struct usb_host_endpoint *iso_in, *iso_out; >> - struct usb_host_endpoint *int_in, *int_out; >> - struct usb_device *udev; >> + int tmp; >> + struct usb_host_interface *alt; >> + struct usb_host_endpoint *in, *out; >> + struct usb_host_endpoint *iso_in, *iso_out; >> + struct usb_host_endpoint *int_in, *int_out; >> + struct usb_device *udev; > > What's the difference between these 6 lines you added and the 6 lines > that were there originally? > Yeah, well, certainly none at all. This is what happened: I created a local copy of my changes(this piece of code included) and fixed some issues in a previous patch, then I did a git revert and moved my changes back to the original file. During this process the tabs were replaced with spaces in the original file, then I had to add the tabs again and this was the resulting patch. >> for (tmp = 0; tmp < intf->num_altsetting; tmp++) { >> - unsigned ep; >> + unsigned ep; > > And here? > Same as above. >> >> in = out = NULL; >> iso_in = iso_out = NULL; >> @@ -150,48 +164,27 @@ get_endpoints(struct usbtest_dev *dev, struct >> usb_interface *intf) >> * ignore other endpoints and altsettings. >> */ >> for (ep = 0; ep < alt->desc.bNumEndpoints; ep++) { >> - struct usb_host_endpoint *e; >> + struct usb_host_endpoint *e; > > And here? > Same as above. >> + int edi; >> >> e = alt->endpoint + ep; >> + edi = usb_endpoint_dir_in(&e->desc); >> + >> switch (usb_endpoint_type(&e->desc)) { >> case USB_ENDPOINT_XFER_BULK: >> - break; >> + endpoint_update(edi, &in, &out, e); >> + continue; >> case USB_ENDPOINT_XFER_INT: >> if (dev->info->intr) >> - goto try_intr; >> + endpoint_update(edi, &int_in, &int_out, e); >> continue; >> case USB_ENDPOINT_XFER_ISOC: >> if (dev->info->iso) >> - goto try_iso; >> - /* FALLTHROUGH */ >> + endpoint_update(edi, &iso_in, &iso_out, e); >> + /* fall through */ > > Why change the comment? > Oh, I based this change in the following comment to another patch I sent some weeks ago: https://lkml.org/lkml/2017/2/10/293 It is due to Coding Style. > Alan Stern > >> default: >> continue; >> } >> - if (usb_endpoint_dir_in(&e->desc)) { >> - if (!in) >> - in = e; >> - } else { >> - if (!out) >> - out = e; >> - } >> - continue; >> -try_intr: >> - if (usb_endpoint_dir_in(&e->desc)) { >> - if (!int_in) >> - int_in = e; >> - } else { >> - if (!int_out) >> - int_out = e; >> - } >> - continue; >> -try_iso: >> - if (usb_endpoint_dir_in(&e->desc)) { >> - if (!iso_in) >> - iso_in = e; >> - } else { >> - if (!iso_out) >> - iso_out = e; >> - } >> } >> if ((in && out) || iso_in || iso_out || int_in || int_out) >> goto found; >> Thanks for your comments. -- Gustavo A. R. Silva