From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753378AbbIOQa4 (ORCPT ); Tue, 15 Sep 2015 12:30:56 -0400 Received: from bear.ext.ti.com ([192.94.94.41]:40427 "EHLO bear.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753099AbbIOQay (ORCPT ); Tue, 15 Sep 2015 12:30:54 -0400 Date: Tue, 15 Sep 2015 11:30:48 -0500 From: Felipe Balbi To: Krzysztof Opasiak CC: , Robert Baldyga , , , , , , Subject: Re: [PATCH 04/26] usb: gadget: introduce 'enabled' flag in struct usb_ep Message-ID: <20150915163048.GL19948@saruman.tx.rr.com> Reply-To: References: <1442327229-28320-1-git-send-email-r.baldyga@samsung.com> <1442327229-28320-5-git-send-email-r.baldyga@samsung.com> <55F83B37.20408@samsung.com> <20150915154324.GI19948@saruman.tx.rr.com> <55F8441D.5080301@samsung.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="95CBLwa+io9O2zXc" Content-Disposition: inline In-Reply-To: <55F8441D.5080301@samsung.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --95CBLwa+io9O2zXc Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Hi, On Tue, Sep 15, 2015 at 06:15:25PM +0200, Krzysztof Opasiak wrote: > >>>+ } > >>>+ > >>>+ return ret; > >>> } > >>> > >> > >>Personally I don't like this convention. In my opinion usb_ep_disable()= & > >>usb_ep_enable() should fail if ep is already disabled/enabled. Then in > >>function code we should check if endpoint is enabled (maybe even we sho= uld > >>have usb_ep_is_enabled()) and call disable only when it is really enabl= ed. > > > >usb_ep_is_enabled() should be a good addition but I don't see an issue > >ignoring usb_ep_enabled() for something that's already enabled. > > > >Imagine if you got an error when you tried to push the light switch to > >the 'on' position while the light was already on :-p > > >=20 > Hmmm not sure right now, didn't test this recently :D as usually I check = if > light isn't already "on" before I touch the switch to turn it on:P not my best analogy :) > Just joking. Personally I just prefer to don't touch things which are > already in desired condition. Let's take close() as example which > could be a little bit equivalent of our usb_ep_disable(). It is not > legal to call it twice on some fd and second call ends up with error. I understand that, it's just the difference between adding usb_ep_is_enabled() to every single gadget/function driver or adding it to usb_ep_enable() itself to keep gadget/function drivers cleaner. --=20 balbi --95CBLwa+io9O2zXc Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJV+Ee4AAoJEIaOsuA1yqREDykP/i3nJc0MaWseAHAECatUuld2 JD/2PFXTbZ0m9EH+D7dL8JSPCBV2ISCK4veQSjBlXiwvzHDN1SVXhIysVX+jQVtF eLNg0h7GeOpnbGSRB2YsbvSkSsVz2NZOZFfuqKRDhjY6mElDWOahPCY6ybg/ghpl A2zv3vLh3QYFWJhSf7qS3HwFe8Pj8vCKQbdSgWLaAIYfQiOvWMCnmtzyjYaB2AYn J5wK+H5BZ82L7e5rm/hqqiPViVkP3XG48f8TL3LAmeI2B+YKXYEhpKa3PyJLI5HB 0OTN49Bxvd8qRMRJ25npLt0oXkCXVPm26bVH81/mpX569ICr/st4nkWmw05Sf+RA uEUSXkqOLbS9jwf7GieOW4I15sWWYikQuAm9NOFu4c3xzSfqnRNXxQcxO2uHASQS MajWmSO8T+qGCAfjeNH1lckL69B5ZW/alTlGKCyte79NHiJv7SP9vwbqjh5zZn8d 4hmv8OS9r4SofKZhEwUgSDXtQvMZSVoZGUIdWXse8vQuTyQuhIOeYwerioQwb4va LgwCpzIvqORRmL59HdeCsLEEMihvyMHwoSQ2tQ7isaF7rg7tDoU+YP6zZVtiahRj sHteZIPDx8FNchIWaS8mCxnjN9zG9cnj++dEW0TY335X8vi7IPPV5CDZwU38D1BC yDRYUTpO+IgRuubxgULQ =vuGQ -----END PGP SIGNATURE----- --95CBLwa+io9O2zXc--