From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756526AbcH2H4Q (ORCPT ); Mon, 29 Aug 2016 03:56:16 -0400 Received: from mga07.intel.com ([134.134.136.100]:2814 "EHLO mga07.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750860AbcH2H4O (ORCPT ); Mon, 29 Aug 2016 03:56:14 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.28,595,1464678000"; d="asc'?scan'208";a="2198518" From: Felipe Balbi To: Felipe Ferreri Tonello , linux-usb@vger.kernel.org Cc: linux-kernel@vger.kernel.org, Michal Nazarewicz Subject: Re: [PATCH v4 08/10] usb: gadget: remove useless parameter in alloc_ep_req() In-Reply-To: <731bcf18-f151-a242-52b1-b8732ee89c92@felipetonello.com> References: <20160808203013.19283-1-eu@felipetonello.com> <20160808203013.19283-9-eu@felipetonello.com> <87oa4q5z4q.fsf@linux.intel.com> <55c2d393-0157-384c-0b93-dec8eaaf1c6d@felipetonello.com> <871t1fiwb6.fsf@linux.intel.com> <731bcf18-f151-a242-52b1-b8732ee89c92@felipetonello.com> User-Agent: Notmuch/0.22.1+63~g994277e (https://notmuchmail.org) Emacs/25.1.1 (x86_64-pc-linux-gnu) Date: Mon, 29 Aug 2016 10:55:57 +0300 Message-ID: <87d1ksdn5e.fsf@linux.intel.com> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha256; protocol="application/pgp-signature" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable Hi, Felipe Ferreri Tonello writes: >> Felipe Ferreri Tonello writes: >>>> "Felipe F. Tonello" writes: >>>>> The default_length parameter of alloc_ep_req was not really necessary >>>>> and gadget drivers would almost always create an inline function to p= ass >>>>> the same value to len and default_len. >>>>> >>>>> So this patch also removes duplicate code from few drivers. >>>>> >>>>> Signed-off-by: Felipe F. Tonello >>>>> --- >>>>> drivers/usb/gadget/function/f_hid.c | 10 ++-------- >>>>> drivers/usb/gadget/function/f_loopback.c | 9 +-------- >>>>> drivers/usb/gadget/function/f_midi.c | 10 ++-------- >>>>> drivers/usb/gadget/function/f_sourcesink.c | 11 ++--------- >>>>> drivers/usb/gadget/u_f.c | 7 +++---- >>>>> drivers/usb/gadget/u_f.h | 3 +-- >>>>> 6 files changed, 11 insertions(+), 39 deletions(-) >>>>> >>>>> diff --git a/drivers/usb/gadget/function/f_hid.c b/drivers/usb/gadget= /function/f_hid.c >>>>> index 51980c50546d..e82a7468252e 100644 >>>>> --- a/drivers/usb/gadget/function/f_hid.c >>>>> +++ b/drivers/usb/gadget/function/f_hid.c >>>>> @@ -362,12 +362,6 @@ static int f_hidg_open(struct inode *inode, stru= ct file *fd) >>>>> /*------------------------------------------------------------------= -------*/ >>>>> /* usb_function = */ >>>>>=20=20 >>>>> -static inline struct usb_request *hidg_alloc_ep_req(struct usb_ep *e= p, >>>>> - unsigned length) >>>>> -{ >>>>> - return alloc_ep_req(ep, length, length); >>>>> -} >>>> >>>> actually, I prefer to keep these little helpers. I was recently playing >>>> with adding SG list support to g_zero (I should have patches soon) and >>>> it was actually very nice to have the sourcesink helper as I could just >>>> ditch alloc_ep_req(). The change to the driver was local to >>>> ss_alloc_ep_req() and nothing else changed :-) >>>> >>> >>> Right, but then it is worth to have the helper function. In this >>> particular case, I am removing a useless helper functions, especially >>> that the previous patch removes the need for the optional parameter in >>> alloc_ep_req. >>=20 >> it's a static inline :-) It won't do any bad to keep it. And, as I said, >> if we want to ditch aloc_ep_req() eventually, then we have just one >> place to patch ;-) > > Yes, sure. But why drop alloc_ep_req()? because we can't find a generic way of allocating sglist with enough entries :-) Some drivers (like f_fs.c) could actually have zero-copy sglist by just pinning user pages with get_user_pages_fast() and following with sg_alloc_from_pages(). g_zero, however, would just "emulate" sglist by just allocating a statically sized sg table and initializing chunks of the linear req->buf to each sg entry. > So should I keep all these helper functions? If so, I actually still > need to fix them to use the newer alloc_ep_req() API. yeah, keeping the helper functions would be nice. IMHO, alloc_ep_req() doesn't have a long life, but it's pretty good for the time being. =2D-=20 balbi --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIbBAEBCAAGBQJXw+qNAAoJEIaOsuA1yqRE3dgP92bfQEucDSCowkIDGnILCSS5 E0GrGSzDYKj9SFmJHHoycMQksB5TKTUJZEzmzfgSTuVwGfwDQaKx1SmPBcbU3zvw 0+R1ulNpSokc8QDW7eUrYEWGLfZ+84gdPR7OWYylIGM4o2WLOYsYc9SlT8oGR5Yr ymauq/iFIh07oQN/2X2oW9TgAjSGDqvb7jZfeHfrQVREMbW6GZkAONa8ct46X3mw 2q2GbMD9k3UQzwmI51nBP7Z6g2QluDVNZbbNq/fDB21cN04ciQJtbsFr+eIX0V6S ydsxypN/gg3LFOt/YXmk5EthUGHcBN5wZuXOA4kR+ieT5aZh4/K2BRQk66T9LfL+ QeBRZ8qS4a5iRue3eOScGQRyZV5hEKa4WmCG1Z7f+JCKwDXwtyhdT7gvt8ATJNqT aXV/Fy0AyChHynMMjNDXYjMo6RqEQbzr+56nBPLabTAMA46mItUctQb7DRJ1sA5C ICgj7gIEkbXfAxOjr5NRi7BcaYy7Cw1QO8anpig07Vi3+IP3/ivspw5U4y1QKcq4 +K9tqfk67KoZLnXGTuIjAJeYYojEwG9HUzuBx9sKxZREAk3k4tzean6MUteDQrBc Qu8flW68GftY5t/aXW2t7KQ1Hk4t/wmNlWcxYvyz/Z//RvCXNo8vvdbmbVB8uDO2 hIZqh6ouWkUPY1gpKdU= =BYQv -----END PGP SIGNATURE----- --=-=-=--