From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932341AbbJ0Mx5 (ORCPT ); Tue, 27 Oct 2015 08:53:57 -0400 Received: from mail-wi0-f178.google.com ([209.85.212.178]:36940 "EHLO mail-wi0-f178.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932303AbbJ0Mxz (ORCPT ); Tue, 27 Oct 2015 08:53:55 -0400 Subject: Re: [PATCH v4 3/7] usb: gadget: define free_ep_req as universal function To: Robert Baldyga , linux-usb@vger.kernel.org References: <1445878509-12084-1-git-send-email-eu@felipetonello.com> <1445878509-12084-4-git-send-email-eu@felipetonello.com> <562F1F7E.3040908@samsung.com> <562F4156.50104@felipetonello.com> <562F483C.9000307@samsung.com> Cc: linux-kernel@vger.kernel.org, Felipe Balbi , Greg Kroah-Hartman , Andrzej Pietrasiewicz , Clemens Ladisch From: Felipe Ferreri Tonello Message-ID: <562F73DF.6000307@felipetonello.com> Date: Tue, 27 Oct 2015 12:53:51 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: <562F483C.9000307@samsung.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Robert, On 27/10/15 09:47, Robert Baldyga wrote: > On 10/27/2015 10:18 AM, Felipe Ferreri Tonello wrote: >> Hi Robert, >> >> On 27/10/15 06:53, Robert Baldyga wrote: >>> On 10/26/2015 05:55 PM, Felipe F. Tonello wrote: >>>> This function is shared between gadget functions, so this avoid unnecessary >>>> duplicated code and potentially avoid memory leaks. >>>> >>>> Signed-off-by: Felipe F. Tonello >>>> --- >>>> drivers/usb/gadget/function/f_midi.c | 6 ------ >>>> drivers/usb/gadget/function/f_sourcesink.c | 6 ------ >>>> drivers/usb/gadget/function/g_zero.h | 1 - >>>> drivers/usb/gadget/u_f.c | 8 ++++++++ >>>> drivers/usb/gadget/u_f.h | 3 +-- >>>> 5 files changed, 9 insertions(+), 15 deletions(-) >>>> >>>> diff --git a/drivers/usb/gadget/function/f_midi.c b/drivers/usb/gadget/function/f_midi.c >>>> index c19f154..4c01c8a 100644 >>>> --- a/drivers/usb/gadget/function/f_midi.c >>>> +++ b/drivers/usb/gadget/function/f_midi.c >>>> @@ -202,12 +202,6 @@ static inline struct usb_request *midi_alloc_ep_req(struct usb_ep *ep, >>>> return alloc_ep_req(ep, length, length); >>>> } >>>> >>>> -static void free_ep_req(struct usb_ep *ep, struct usb_request *req) >>>> -{ >>>> - kfree(req->buf); >>>> - usb_ep_free_request(ep, req); >>>> -} >>>> - >>>> static const uint8_t f_midi_cin_length[] = { >>>> 0, 0, 2, 3, 3, 1, 2, 3, 3, 3, 3, 3, 2, 2, 3, 1 >>>> }; >>>> diff --git a/drivers/usb/gadget/function/f_sourcesink.c b/drivers/usb/gadget/function/f_sourcesink.c >>>> index 3a5ae99..eedea7f 100644 >>>> --- a/drivers/usb/gadget/function/f_sourcesink.c >>>> +++ b/drivers/usb/gadget/function/f_sourcesink.c >>>> @@ -307,12 +307,6 @@ static inline struct usb_request *ss_alloc_ep_req(struct usb_ep *ep, int len) >>>> return alloc_ep_req(ep, len, buflen); >>>> } >>>> >>>> -void free_ep_req(struct usb_ep *ep, struct usb_request *req) >>>> -{ >>>> - kfree(req->buf); >>>> - usb_ep_free_request(ep, req); >>>> -} >>>> - >>>> static void disable_ep(struct usb_composite_dev *cdev, struct usb_ep *ep) >>>> { >>>> int value; >>>> diff --git a/drivers/usb/gadget/function/g_zero.h b/drivers/usb/gadget/function/g_zero.h >>>> index 15f1809..5ed90b4 100644 >>>> --- a/drivers/usb/gadget/function/g_zero.h >>>> +++ b/drivers/usb/gadget/function/g_zero.h >>>> @@ -59,7 +59,6 @@ void lb_modexit(void); >>>> int lb_modinit(void); >>>> >>>> /* common utilities */ >>>> -void free_ep_req(struct usb_ep *ep, struct usb_request *req); >>>> void disable_endpoints(struct usb_composite_dev *cdev, >>>> struct usb_ep *in, struct usb_ep *out, >>>> struct usb_ep *iso_in, struct usb_ep *iso_out); >>>> diff --git a/drivers/usb/gadget/u_f.c b/drivers/usb/gadget/u_f.c >>>> index c6276f0..f78bd1f 100644 >>>> --- a/drivers/usb/gadget/u_f.c >>>> +++ b/drivers/usb/gadget/u_f.c >>>> @@ -14,6 +14,7 @@ >>>> #include >>>> #include "u_f.h" >>>> >>>> +/* Requests allocated via alloc_ep_req() must be freed by free_ep_req(). */ >>>> struct usb_request *alloc_ep_req(struct usb_ep *ep, int len, int default_len) >>>> { >>>> struct usb_request *req; >>>> @@ -30,3 +31,10 @@ struct usb_request *alloc_ep_req(struct usb_ep *ep, int len, int default_len) >>>> return req; >>>> } >>>> EXPORT_SYMBOL_GPL(alloc_ep_req); >>>> + >>>> +void free_ep_req(struct usb_ep *ep, struct usb_request *req) >>>> +{ >>>> + kfree(req->buf); >>>> + usb_ep_free_request(ep, req); >>>> +} >>>> +EXPORT_SYMBOL_GPL(free_ep_req); >>>> diff --git a/drivers/usb/gadget/u_f.h b/drivers/usb/gadget/u_f.h >>>> index 1d5f0eb..2a1a6fb 100644 >>>> --- a/drivers/usb/gadget/u_f.h >>>> +++ b/drivers/usb/gadget/u_f.h >>>> @@ -46,7 +46,6 @@ struct usb_ep; >>>> struct usb_request; >>>> >>>> struct usb_request *alloc_ep_req(struct usb_ep *ep, int len, int default_len); >>>> +void free_ep_req(struct usb_ep *ep, struct usb_request *req); >>>> >>>> #endif /* __U_F_H__ */ >>>> - >>>> - >>>> >>> >>> Isn't it simple enough to be static inline? >> >> inline yes. And the compiler will do it automatically. But I can add it >> for clarity. > > No, compiler will never make function inline when you export its symbol. > To make it inline you should place it in header. Correct. I will improve it on next revision. -- Felipe