From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754405AbbIOPqL (ORCPT ); Tue, 15 Sep 2015 11:46:11 -0400 Received: from mailout2.w1.samsung.com ([210.118.77.12]:56074 "EHLO mailout2.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754370AbbIOPqH (ORCPT ); Tue, 15 Sep 2015 11:46:07 -0400 X-AuditID: cbfec7f5-f794b6d000001495-a8-55f83d3ca70a Subject: Re: [PATCH 05/26] usb: gadget: f_ecm: eliminate abuse of ep->driver data To: Robert Baldyga , balbi@ti.com References: <1442327229-28320-1-git-send-email-r.baldyga@samsung.com> <1442327229-28320-6-git-send-email-r.baldyga@samsung.com> Cc: gregkh@linuxfoundation.org, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, b.zolnierkie@samsung.com, m.szyprowski@samsung.com, andrzej.p@samsung.com From: Krzysztof Opasiak Message-id: <55F83D36.7090208@samsung.com> Date: Tue, 15 Sep 2015 17:45:58 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 MIME-version: 1.0 In-reply-to: <1442327229-28320-6-git-send-email-r.baldyga@samsung.com> Content-type: text/plain; charset=windows-1252; format=flowed Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFrrFLMWRmVeSWpSXmKPExsVy+t/xa7o2tj9CDZ7e0LCY9bKdxWLjjPWs Fgfv11s0L17PZnF51xw2i0XLWpkt1h65y27x4PBOdgcOj/1z17B79G1Zxehx/MZ2Jo/Pm+QC WKK4bFJSczLLUov07RK4Mn59e8FaMJ+nYtKE/UwNjA84uxg5OSQETCQmfetigrDFJC7cW88G YgsJLGWUmPUlsouRC8h+zijx7MFSdpCEsECwxM7ZR1hBbBEBC4n7e4+yQRQ1MkpMOtjHDuIw CyxjlHgyYTGQw8HBJqAvMW+XKEgDr4CWRPv9e2DbWARUJSa/+sgIYosKREicOvuWDaJGUOLH 5HssIK2cAm4Sm+67g4SZBWwlFrxfxwJhy0tsXvOWeQKjwCwkHbOQlM1CUraAkXkVo2hqaXJB cVJ6rpFecWJucWleul5yfu4mRkiIf93BuPSY1SFGAQ5GJR7eiPbvoUKsiWXFlbmHGCU4mJVE eBt1foQK8aYkVlalFuXHF5XmpBYfYpTmYFES5525632IkEB6YklqdmpqQWoRTJaJg1OqgbEz +OoKXmmv8Jt2Ous6nm/VmrgmRuLul88ua5RcTs8zfXrMae2RaUFlu/d8YTRR7jKoFtA32SQo ulZOJFZs66JrSzlOrd1rLNHs6poyR7rZLJ9fM/V4gcm5c4WCTcfFdmjqdLcvijRqFsvbL/3a z/HzhtvKu+carEhbXST079rNL79aJmwPNVdiKc5INNRiLipOBAC+bnLJbQIAAA== Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 ( ... ) > > @@ -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