Messages in this thread | | | Subject | Re: [PATCH 05/26] usb: gadget: f_ecm: eliminate abuse of ep->driver data | From | Krzysztof Opasiak <> | Date | Tue, 15 Sep 2015 17:45:58 +0200 |
| |
On 09/15/2015 04:26 PM, Robert Baldyga wrote: > Since ep->driver_data is not used for endpoint claiming, neither for > enabled/disabled state storing, we can reduce number of places where > we read or modify it's value, as now it has no particular meaning for > function or framework logic. > > In case of f_ecm, ep->driver_data was used only for endpoint claiming > and marking endpoints as enabled, so we can simplify code by reducing > it. > > Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
( ... )
> > @@ -820,14 +811,6 @@ fail: > usb_ep_free_request(ecm->notify, ecm->notify_req); > } > > - /* we might as well release our claims on endpoints */ > - if (ecm->notify) > - ecm->notify->driver_data = NULL; > - if (ecm->port.out_ep) > - ecm->port.out_ep->driver_data = NULL; > - if (ecm->port.in_ep) > - ecm->port.in_ep->driver_data = NULL; > - > ERROR(cdev, "%s: can't bind, err %d\n", f->name, status); > > return status; >
You have done this in almost all functions but personally I'm really concern about this change.
By convention function should free all allocated resources when exiting with non 0 code. Endpoints are some kind of resources, they are "allocated" using usb_ep_autoconfig() and if you are not going to use them because error occurred you should free the using usb_ep_autoconfig_release(). Moreover, you have done this in source sink function so why not do this in all other?
Best regards, -- Krzysztof Opasiak Samsung R&D Institute Poland Samsung Electronics
| |