Messages in this thread | | | Subject | Re: [PATCH v4 3/7] usb: gadget: define free_ep_req as universal function | From | Felipe Ferreri Tonello <> | Date | Tue, 27 Oct 2015 12:53:51 +0000 |
| |
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 <eu@felipetonello.com> >>>> --- >>>> 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 <linux/usb/gadget.h> >>>> #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
| |