From mboxrd@z Thu Jan 1 00:00:00 1970 From: Greg KH Subject: Re: [PATCH v2 2/2] Embedded USB Debugger (EUD) driver Date: Wed, 12 Sep 2018 08:29:05 +0200 Message-ID: <20180912062905.GD10268@kroah.com> References: <1536096853-30473-1-git-send-email-pheragu@codeaurora.org> <1536096853-30473-3-git-send-email-pheragu@codeaurora.org> <20180905111800.GA30003@kroah.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org To: pheragu@codeaurora.org Cc: Manu Gautam , linux-arm-msm@vger.kernel.org, linux-usb@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, ckadabi@codeaurora.org, tsoni@codeaurora.org, rnayak@codeaurora.org, bryanh@codeaurora.org, psodagud@codeaurora.org, Satya Durga Srinivasu Prabhala List-Id: linux-arm-msm@vger.kernel.org On Tue, Sep 11, 2018 at 01:40:19PM -0700, pheragu@codeaurora.org wrote: > On 2018-09-05 04:18, Greg KH wrote: > > On Wed, Sep 05, 2018 at 03:01:26PM +0530, Manu Gautam wrote: > > > Hi, > > > > > > > > > On 9/5/2018 3:04 AM, Prakruthi Deepak Heragu wrote: > > > > Add support for control peripheral of EUD (Embedded USB Debugger) to > > > > listen to events such as USB attach/detach, charger enable/disable, pet > > > > EUD to indicate software is functional. > > > > > > > > Signed-off-by: Satya Durga Srinivasu Prabhala > > > > Signed-off-by: Prakruthi Deepak Heragu > > > > --- > > > > drivers/soc/qcom/Kconfig | 12 ++ > > > > drivers/soc/qcom/Makefile | 1 + > > > > drivers/soc/qcom/eud.c | 338 ++++++++++++++++++++++++++++++++++++++++++++++ > > > > 3 files changed, 351 insertions(+) > > > > create mode 100644 drivers/soc/qcom/eud.c > > > > > > > > diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig > > > [snip] > > > > + > > > > +#define EUD_ENABLE_CMD 1 > > > > +#define EUD_DISABLE_CMD 0 > > > > > > Why not use module param as boolean? I mean zero to disable and > > > non-zero to enable? > > > > Never use module parameters on new code, as it's really hard to ever > > change them. It also doesn't work for multiple devices of the same > > type. > > > If not for module_param, do you suggest we use sysfs? It depends on what you want to control. > module_param also provides a facility to control these parameters > through command line too. If we can't use module_param, do you expect > us to use __setup(..) API? Again, it depends on what you are wanting to handle here. module parameters and setup variables are all "global" in that they affect all devices of the same type. sysfs can be "per device" which is almost always what you want to have happen. But don't use any of those for things that should be "automatic" in that the driver should figure out what needs to be done here and not need any external configuration. That's the best way of all to handle this. thanks, greg k-h From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Subject: [v2,2/2] Embedded USB Debugger (EUD) driver From: Greg Kroah-Hartman Message-Id: <20180912062905.GD10268@kroah.com> Date: Wed, 12 Sep 2018 08:29:05 +0200 To: pheragu@codeaurora.org Cc: Manu Gautam , linux-arm-msm@vger.kernel.org, linux-usb@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, ckadabi@codeaurora.org, tsoni@codeaurora.org, rnayak@codeaurora.org, bryanh@codeaurora.org, psodagud@codeaurora.org, Satya Durga Srinivasu Prabhala List-ID: T24gVHVlLCBTZXAgMTEsIDIwMTggYXQgMDE6NDA6MTlQTSAtMDcwMCwgcGhlcmFndUBjb2RlYXVy b3JhLm9yZyB3cm90ZToKPiBPbiAyMDE4LTA5LTA1IDA0OjE4LCBHcmVnIEtIIHdyb3RlOgo+ID4g T24gV2VkLCBTZXAgMDUsIDIwMTggYXQgMDM6MDE6MjZQTSArMDUzMCwgTWFudSBHYXV0YW0gd3Jv dGU6Cj4gPiA+IEhpLAo+ID4gPiAKPiA+ID4gCj4gPiA+IE9uIDkvNS8yMDE4IDM6MDQgQU0sIFBy YWtydXRoaSBEZWVwYWsgSGVyYWd1IHdyb3RlOgo+ID4gPiA+IEFkZCBzdXBwb3J0IGZvciBjb250 cm9sIHBlcmlwaGVyYWwgb2YgRVVEIChFbWJlZGRlZCBVU0IgRGVidWdnZXIpIHRvCj4gPiA+ID4g bGlzdGVuIHRvIGV2ZW50cyBzdWNoIGFzIFVTQiBhdHRhY2gvZGV0YWNoLCBjaGFyZ2VyIGVuYWJs ZS9kaXNhYmxlLCBwZXQKPiA+ID4gPiBFVUQgdG8gaW5kaWNhdGUgc29mdHdhcmUgaXMgZnVuY3Rp b25hbC4KPiA+ID4gPgo+ID4gPiA+IFNpZ25lZC1vZmYtYnk6IFNhdHlhIER1cmdhIFNyaW5pdmFz dSBQcmFiaGFsYSA8c2F0eWFwQGNvZGVhdXJvcmEub3JnPgo+ID4gPiA+IFNpZ25lZC1vZmYtYnk6 IFByYWtydXRoaSBEZWVwYWsgSGVyYWd1IDxwaGVyYWd1QGNvZGVhdXJvcmEub3JnPgo+ID4gPiA+ IC0tLQo+ID4gPiA+ICBkcml2ZXJzL3NvYy9xY29tL0tjb25maWcgIHwgIDEyICsrCj4gPiA+ID4g IGRyaXZlcnMvc29jL3Fjb20vTWFrZWZpbGUgfCAgIDEgKwo+ID4gPiA+ICBkcml2ZXJzL3NvYy9x Y29tL2V1ZC5jICAgIHwgMzM4ICsrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysr KysrKysrKysKPiA+ID4gPiAgMyBmaWxlcyBjaGFuZ2VkLCAzNTEgaW5zZXJ0aW9ucygrKQo+ID4g PiA+ICBjcmVhdGUgbW9kZSAxMDA2NDQgZHJpdmVycy9zb2MvcWNvbS9ldWQuYwo+ID4gPiA+Cj4g PiA+ID4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvc29jL3Fjb20vS2NvbmZpZyBiL2RyaXZlcnMvc29j L3Fjb20vS2NvbmZpZwo+ID4gPiBbc25pcF0KPiA+ID4gPiArCj4gPiA+ID4gKyNkZWZpbmUgRVVE X0VOQUJMRV9DTUQgMQo+ID4gPiA+ICsjZGVmaW5lIEVVRF9ESVNBQkxFX0NNRCAwCj4gPiA+IAo+ ID4gPiBXaHkgbm90IHVzZSBtb2R1bGUgcGFyYW0gYXMgYm9vbGVhbj8gSSBtZWFuIHplcm8gdG8g ZGlzYWJsZSBhbmQKPiA+ID4gbm9uLXplcm8gdG8gZW5hYmxlPwo+ID4gCj4gPiBOZXZlciB1c2Ug bW9kdWxlIHBhcmFtZXRlcnMgb24gbmV3IGNvZGUsIGFzIGl0J3MgcmVhbGx5IGhhcmQgdG8gZXZl cgo+ID4gY2hhbmdlIHRoZW0uICBJdCBhbHNvIGRvZXNuJ3Qgd29yayBmb3IgbXVsdGlwbGUgZGV2 aWNlcyBvZiB0aGUgc2FtZQo+ID4gdHlwZS4KPiA+IAo+IElmIG5vdCBmb3IgbW9kdWxlX3BhcmFt LCBkbyB5b3Ugc3VnZ2VzdCB3ZSB1c2Ugc3lzZnM/CgpJdCBkZXBlbmRzIG9uIHdoYXQgeW91IHdh bnQgdG8gY29udHJvbC4KCj4gbW9kdWxlX3BhcmFtIGFsc28gcHJvdmlkZXMgYSBmYWNpbGl0eSB0 byBjb250cm9sIHRoZXNlIHBhcmFtZXRlcnMKPiB0aHJvdWdoIGNvbW1hbmQgbGluZSB0b28uIElm IHdlIGNhbid0IHVzZSBtb2R1bGVfcGFyYW0sIGRvIHlvdSBleHBlY3QKPiB1cyB0byB1c2UgX19z ZXR1cCguLikgQVBJPwoKQWdhaW4sIGl0IGRlcGVuZHMgb24gd2hhdCB5b3UgYXJlIHdhbnRpbmcg dG8gaGFuZGxlIGhlcmUuICBtb2R1bGUKcGFyYW1ldGVycyBhbmQgc2V0dXAgdmFyaWFibGVzIGFy ZSBhbGwgImdsb2JhbCIgaW4gdGhhdCB0aGV5IGFmZmVjdCBhbGwKZGV2aWNlcyBvZiB0aGUgc2Ft ZSB0eXBlLiAgc3lzZnMgY2FuIGJlICJwZXIgZGV2aWNlIiB3aGljaCBpcyBhbG1vc3QKYWx3YXlz IHdoYXQgeW91IHdhbnQgdG8gaGF2ZSBoYXBwZW4uCgpCdXQgZG9uJ3QgdXNlIGFueSBvZiB0aG9z ZSBmb3IgdGhpbmdzIHRoYXQgc2hvdWxkIGJlICJhdXRvbWF0aWMiIGluIHRoYXQKdGhlIGRyaXZl ciBzaG91bGQgZmlndXJlIG91dCB3aGF0IG5lZWRzIHRvIGJlIGRvbmUgaGVyZSBhbmQgbm90IG5l ZWQgYW55CmV4dGVybmFsIGNvbmZpZ3VyYXRpb24uICBUaGF0J3MgdGhlIGJlc3Qgd2F5IG9mIGFs bCB0byBoYW5kbGUgdGhpcy4KCnRoYW5rcywKCmdyZWcgay1oCg==