From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932642AbbFVHzY (ORCPT ); Mon, 22 Jun 2015 03:55:24 -0400 Received: from mail-ie0-f177.google.com ([209.85.223.177]:35230 "EHLO mail-ie0-f177.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754350AbbFVHzO (ORCPT ); Mon, 22 Jun 2015 03:55:14 -0400 MIME-Version: 1.0 In-Reply-To: <20150620180435.GG16386@windriver.com> References: <1430316005-16480-1-git-send-email-shobhit.kumar@intel.com> <1430316005-16480-7-git-send-email-shobhit.kumar@intel.com> <1430428322.2187.24.camel@x220> <1434652916.2385.124.camel@x220> <1434799402.19497.47.camel@x220> <20150620180435.GG16386@windriver.com> Date: Mon, 22 Jun 2015 13:25:13 +0530 Message-ID: Subject: Re: [Intel-gfx] [PATCH 6/8] drivers/pwm: Add Crystalcove (CRC) PWM driver From: Shobhit Kumar To: Paul Gortmaker Cc: Paul Bolle , Shobhit Kumar , linux-pwm , Jani Nikula , Samuel Ortiz , Alexandre Courbot , David Airlie , Povilas Staniulis , intel-gfx , linux-kernel , dri-devel , linux-gpio , Chih-Wei Huang , Thierry Reding , Daniel Vetter , Lee Jones , Linus Walleij Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sat, Jun 20, 2015 at 11:34 PM, Paul Gortmaker wrote: > [Re: [Intel-gfx] [PATCH 6/8] drivers/pwm: Add Crystalcove (CRC) PWM driver] On 20/06/2015 (Sat 13:23) Paul Bolle wrote: > >> [Added Paul Gortmaker.] >> >> Hi Shobhit, >> >> On Fri, 2015-06-19 at 12:16 +0530, Shobhit Kumar wrote: >> > So what is the exact big problem with this ? >> >> The main problem I have is that it's hard to read a submitter's mind. >> And, I think, in cases like this we need to know if the submitter just >> made some silly mistake or that the mismatch (between Kconfig type and >> code) was intentional. So each time such a mismatch is spotted the >> submitter ought to be asked about it. >> >> (I'd guess that one or two new drivers are submitted _each_ day. And >> these mismatches are quite common. I'd say I receive answers like: >> - "Oops, yes bool should have been tristate"; or >> - "Oops, forgot to clean up after noticing tristate didn't work"; or >> - "I just copy-and-pasted a similar driver, the module stuff isn't >> actually needed" >> at least once a week. Not sure, I don't keep track of this stuff.) >> >> Furthermore, it appears that Paul Gortmaker is on a mission to, badly >> summarized, untangle the module and init code. See for instance >> https://lkml.org/lkml/2015/5/28/809 and >> https://lkml.org/lkml/2015/5/31/205 . >> >> Now, I don't know whether (other) Paul is bothered by these MODULE_* >> macros. But Paul did submit a series that adds > > Yes, I agree that it would be nice to not see these mismatches, > regardless of whether we can get away with it or not. > >> builtin_platform_driver(), see https://lkml.org/lkml/2015/5/10/131 . >> That new macro ensures built-in only code doesn't have to use >> module_platform_driver(), which your patch also uses. So perhaps Paul >> can explain some of the non-obvious issues caused by built-in only code >> using module specific constructs. > > In https://lkml.org/lkml/2015/5/10/125 I'd written: > > There are several downsides to this: > 1) The code can appear modular to a reader of the code, and they > won't know if the code really is modular without checking the > Makefile and Kconfig to see if compilation is governed by a > bool or tristate. > 2) Coders of drivers may be tempted to code up an __exit function > that is never used, just in order to satisfy the required three > args of the modular registration function. > 3) Non-modular code ends up including the which increases > CPP overhead that they don't need. > 4) It hinders us from performing better separation of the module > init code and the generic init code. > Okay. Get the idea and the need in terms of clear separation. Its just that there are quite a few built-in drivers using module initialization that I assumed its okay. > The nature of linux means that thousands of developers are reading the > code every day -- so I think that there is a genuine value in having the > code convey a clear message on how it was designed to be used. Only > using module related headers/macros for genuinely modular code helps us > (albeit in a small way) towards achieving that. > > Looking at this thread, I see that one of the reasons given for this > code's ambiguous module vs. built-in identity was the observation of a > similar identity crisis of the related INTEL_SOC_PMIC code. Does that > not back up the point above about the value in having the code speak for > itself? So IMHO we probably should clarify the PMIC code vs. adding > another example that looks just like it. > Okay agree. I think there are quite of them lurking in the sources which would need correction. For this PWM driver I will take care as suggested. Regards Shobhit > Paul. > -- > >> >> > I can anyway shove out these macros to end the discussion. >> >> I'd rather convince you than annoy you into doing as I suggested. >> >> > BTW whether you buy the argument or not, please do treat yourself >> > with ice cream for being able to make such a comment. >> >> Will do. >> >> Thanks, >> >> >> Paul Bolle >> -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in Please read the FAQ at http://www.tux.org/lkml/ From mboxrd@z Thu Jan 1 00:00:00 1970 From: Shobhit Kumar Subject: Re: [PATCH 6/8] drivers/pwm: Add Crystalcove (CRC) PWM driver Date: Mon, 22 Jun 2015 13:25:13 +0530 Message-ID: References: <1430316005-16480-1-git-send-email-shobhit.kumar@intel.com> <1430316005-16480-7-git-send-email-shobhit.kumar@intel.com> <1430428322.2187.24.camel@x220> <1434652916.2385.124.camel@x220> <1434799402.19497.47.camel@x220> <20150620180435.GG16386@windriver.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <20150620180435.GG16386@windriver.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Paul Gortmaker Cc: Alexandre Courbot , Paul Bolle , Samuel Ortiz , linux-pwm , Jani Nikula , Shobhit Kumar , intel-gfx , linux-kernel , Povilas Staniulis , David Airlie , Chih-Wei Huang , Thierry Reding , dri-devel , linux-gpio , Daniel Vetter , Lee Jones , Linus Walleij List-Id: linux-gpio@vger.kernel.org T24gU2F0LCBKdW4gMjAsIDIwMTUgYXQgMTE6MzQgUE0sIFBhdWwgR29ydG1ha2VyCjxwYXVsLmdv cnRtYWtlckB3aW5kcml2ZXIuY29tPiB3cm90ZToKPiBbUmU6IFtJbnRlbC1nZnhdIFtQQVRDSCA2 LzhdIGRyaXZlcnMvcHdtOiBBZGQgQ3J5c3RhbGNvdmUgKENSQykgUFdNIGRyaXZlcl0gT24gMjAv MDYvMjAxNSAoU2F0IDEzOjIzKSBQYXVsIEJvbGxlIHdyb3RlOgo+Cj4+IFtBZGRlZCBQYXVsIEdv cnRtYWtlci5dCj4+Cj4+IEhpIFNob2JoaXQsCj4+Cj4+IE9uIEZyaSwgMjAxNS0wNi0xOSBhdCAx MjoxNiArMDUzMCwgU2hvYmhpdCBLdW1hciB3cm90ZToKPj4gPiBTbyB3aGF0IGlzIHRoZSBleGFj dCBiaWcgcHJvYmxlbSB3aXRoIHRoaXMgPwo+Pgo+PiBUaGUgbWFpbiBwcm9ibGVtIEkgaGF2ZSBp cyB0aGF0IGl0J3MgaGFyZCB0byByZWFkIGEgc3VibWl0dGVyJ3MgbWluZC4KPj4gQW5kLCBJIHRo aW5rLCBpbiBjYXNlcyBsaWtlIHRoaXMgd2UgbmVlZCB0byBrbm93IGlmIHRoZSBzdWJtaXR0ZXIg anVzdAo+PiBtYWRlIHNvbWUgc2lsbHkgbWlzdGFrZSBvciB0aGF0IHRoZSBtaXNtYXRjaCAoYmV0 d2VlbiBLY29uZmlnIHR5cGUgYW5kCj4+IGNvZGUpIHdhcyBpbnRlbnRpb25hbC4gU28gZWFjaCB0 aW1lIHN1Y2ggYSBtaXNtYXRjaCBpcyBzcG90dGVkIHRoZQo+PiBzdWJtaXR0ZXIgb3VnaHQgdG8g YmUgYXNrZWQgYWJvdXQgaXQuCj4+Cj4+IChJJ2QgZ3Vlc3MgdGhhdCBvbmUgb3IgdHdvIG5ldyBk cml2ZXJzIGFyZSBzdWJtaXR0ZWQgX2VhY2hfIGRheS4gQW5kCj4+IHRoZXNlIG1pc21hdGNoZXMg YXJlIHF1aXRlIGNvbW1vbi4gSSdkIHNheSBJIHJlY2VpdmUgYW5zd2VycyBsaWtlOgo+PiAtICJP b3BzLCB5ZXMgYm9vbCBzaG91bGQgaGF2ZSBiZWVuIHRyaXN0YXRlIjsgb3IKPj4gLSAiT29wcywg Zm9yZ290IHRvIGNsZWFuIHVwIGFmdGVyIG5vdGljaW5nIHRyaXN0YXRlIGRpZG4ndCB3b3JrIjsg b3IKPj4gLSAiSSBqdXN0IGNvcHktYW5kLXBhc3RlZCBhIHNpbWlsYXIgZHJpdmVyLCB0aGUgbW9k dWxlIHN0dWZmIGlzbid0Cj4+ICAgYWN0dWFsbHkgbmVlZGVkIgo+PiBhdCBsZWFzdCBvbmNlIGEg d2Vlay4gTm90IHN1cmUsIEkgZG9uJ3Qga2VlcCB0cmFjayBvZiB0aGlzIHN0dWZmLikKPj4KPj4g RnVydGhlcm1vcmUsIGl0IGFwcGVhcnMgdGhhdCBQYXVsIEdvcnRtYWtlciBpcyBvbiBhIG1pc3Np b24gdG8sIGJhZGx5Cj4+IHN1bW1hcml6ZWQsIHVudGFuZ2xlIHRoZSBtb2R1bGUgYW5kIGluaXQg Y29kZS4gU2VlIGZvciBpbnN0YW5jZQo+PiBodHRwczovL2xrbWwub3JnL2xrbWwvMjAxNS81LzI4 LzgwOSBhbmQKPj4gaHR0cHM6Ly9sa21sLm9yZy9sa21sLzIwMTUvNS8zMS8yMDUgLgo+Pgo+PiBO b3csIEkgZG9uJ3Qga25vdyB3aGV0aGVyIChvdGhlcikgUGF1bCBpcyBib3RoZXJlZCBieSB0aGVz ZSBNT0RVTEVfKgo+PiBtYWNyb3MuIEJ1dCBQYXVsIGRpZCBzdWJtaXQgYSBzZXJpZXMgdGhhdCBh ZGRzCj4KPiBZZXMsIEkgYWdyZWUgdGhhdCBpdCB3b3VsZCBiZSBuaWNlIHRvIG5vdCBzZWUgdGhl c2UgbWlzbWF0Y2hlcywKPiByZWdhcmRsZXNzIG9mIHdoZXRoZXIgd2UgY2FuIGdldCBhd2F5IHdp dGggaXQgb3Igbm90Lgo+Cj4+IGJ1aWx0aW5fcGxhdGZvcm1fZHJpdmVyKCksIHNlZSBodHRwczov L2xrbWwub3JnL2xrbWwvMjAxNS81LzEwLzEzMSAuCj4+IFRoYXQgbmV3IG1hY3JvIGVuc3VyZXMg YnVpbHQtaW4gb25seSBjb2RlIGRvZXNuJ3QgaGF2ZSB0byB1c2UKPj4gbW9kdWxlX3BsYXRmb3Jt X2RyaXZlcigpLCB3aGljaCB5b3VyIHBhdGNoIGFsc28gdXNlcy4gU28gcGVyaGFwcyBQYXVsCj4+ IGNhbiBleHBsYWluIHNvbWUgb2YgdGhlIG5vbi1vYnZpb3VzIGlzc3VlcyBjYXVzZWQgYnkgYnVp bHQtaW4gb25seSBjb2RlCj4+IHVzaW5nIG1vZHVsZSBzcGVjaWZpYyBjb25zdHJ1Y3RzLgo+Cj4g SW4gIGh0dHBzOi8vbGttbC5vcmcvbGttbC8yMDE1LzUvMTAvMTI1IEknZCB3cml0dGVuOgo+Cj4g ICBUaGVyZSBhcmUgc2V2ZXJhbCBkb3duc2lkZXMgdG8gdGhpczoKPiAgIDEpIFRoZSBjb2RlIGNh biBhcHBlYXIgbW9kdWxhciB0byBhIHJlYWRlciBvZiB0aGUgY29kZSwgYW5kIHRoZXkKPiAgICAg IHdvbid0IGtub3cgaWYgdGhlIGNvZGUgcmVhbGx5IGlzIG1vZHVsYXIgd2l0aG91dCBjaGVja2lu ZyB0aGUKPiAgICAgIE1ha2VmaWxlIGFuZCBLY29uZmlnIHRvIHNlZSBpZiBjb21waWxhdGlvbiBp cyBnb3Zlcm5lZCBieSBhCj4gICAgICBib29sIG9yIHRyaXN0YXRlLgo+ICAgMikgQ29kZXJzIG9m IGRyaXZlcnMgbWF5IGJlIHRlbXB0ZWQgdG8gY29kZSB1cCBhbiBfX2V4aXQgZnVuY3Rpb24KPiAg ICAgIHRoYXQgaXMgbmV2ZXIgdXNlZCwganVzdCBpbiBvcmRlciB0byBzYXRpc2Z5IHRoZSByZXF1 aXJlZCB0aHJlZQo+ICAgICAgYXJncyBvZiB0aGUgbW9kdWxhciByZWdpc3RyYXRpb24gZnVuY3Rp b24uCj4gICAzKSBOb24tbW9kdWxhciBjb2RlIGVuZHMgdXAgaW5jbHVkaW5nIHRoZSA8bW9kdWxl Lmg+IHdoaWNoIGluY3JlYXNlcwo+ICAgICAgQ1BQIG92ZXJoZWFkIHRoYXQgdGhleSBkb24ndCBu ZWVkLgo+ICAgNCkgSXQgaGluZGVycyB1cyBmcm9tIHBlcmZvcm1pbmcgYmV0dGVyIHNlcGFyYXRp b24gb2YgdGhlIG1vZHVsZQo+ICAgICAgaW5pdCBjb2RlIGFuZCB0aGUgZ2VuZXJpYyBpbml0IGNv ZGUuCj4KCk9rYXkuIEdldCB0aGUgaWRlYSBhbmQgdGhlIG5lZWQgaW4gdGVybXMgb2YgY2xlYXIg c2VwYXJhdGlvbi4gSXRzIGp1c3QKdGhhdCB0aGVyZSBhcmUgcXVpdGUgYSBmZXcgYnVpbHQtaW4g ZHJpdmVycyB1c2luZyBtb2R1bGUKaW5pdGlhbGl6YXRpb24gdGhhdCBJIGFzc3VtZWQgaXRzIG9r YXkuCgo+IFRoZSBuYXR1cmUgb2YgbGludXggbWVhbnMgdGhhdCB0aG91c2FuZHMgb2YgZGV2ZWxv cGVycyBhcmUgcmVhZGluZyB0aGUKPiBjb2RlIGV2ZXJ5IGRheSAtLSBzbyBJIHRoaW5rIHRoYXQg dGhlcmUgaXMgYSBnZW51aW5lIHZhbHVlIGluIGhhdmluZyB0aGUKPiBjb2RlIGNvbnZleSBhIGNs ZWFyIG1lc3NhZ2Ugb24gaG93IGl0IHdhcyBkZXNpZ25lZCB0byBiZSB1c2VkLiAgT25seQo+IHVz aW5nIG1vZHVsZSByZWxhdGVkIGhlYWRlcnMvbWFjcm9zIGZvciBnZW51aW5lbHkgbW9kdWxhciBj b2RlIGhlbHBzIHVzCj4gKGFsYmVpdCBpbiBhIHNtYWxsIHdheSkgdG93YXJkcyBhY2hpZXZpbmcg dGhhdC4KPgo+IExvb2tpbmcgYXQgdGhpcyB0aHJlYWQsIEkgc2VlIHRoYXQgb25lIG9mIHRoZSBy ZWFzb25zIGdpdmVuIGZvciB0aGlzCj4gY29kZSdzIGFtYmlndW91cyBtb2R1bGUgdnMuIGJ1aWx0 LWluIGlkZW50aXR5IHdhcyB0aGUgb2JzZXJ2YXRpb24gb2YgYQo+IHNpbWlsYXIgaWRlbnRpdHkg Y3Jpc2lzIG9mIHRoZSByZWxhdGVkIElOVEVMX1NPQ19QTUlDIGNvZGUuIERvZXMgdGhhdAo+IG5v dCBiYWNrIHVwIHRoZSBwb2ludCBhYm92ZSBhYm91dCB0aGUgdmFsdWUgaW4gaGF2aW5nIHRoZSBj b2RlIHNwZWFrIGZvcgo+IGl0c2VsZj8gIFNvIElNSE8gd2UgcHJvYmFibHkgc2hvdWxkIGNsYXJp ZnkgdGhlIFBNSUMgY29kZSB2cy4gYWRkaW5nCj4gYW5vdGhlciBleGFtcGxlIHRoYXQgbG9va3Mg anVzdCBsaWtlIGl0Lgo+CgpPa2F5IGFncmVlLiBJIHRoaW5rIHRoZXJlIGFyZSBxdWl0ZSBvZiB0 aGVtIGx1cmtpbmcgaW4gdGhlIHNvdXJjZXMKd2hpY2ggd291bGQgbmVlZCBjb3JyZWN0aW9uLiBG b3IgdGhpcyBQV00gZHJpdmVyIEkgd2lsbCB0YWtlIGNhcmUgYXMKc3VnZ2VzdGVkLgoKUmVnYXJk cwpTaG9iaGl0Cgo+IFBhdWwuCj4gLS0KPgo+Pgo+PiA+IEkgY2FuIGFueXdheSBzaG92ZSBvdXQg dGhlc2UgbWFjcm9zIHRvIGVuZCB0aGUgZGlzY3Vzc2lvbi4KPj4KPj4gSSdkIHJhdGhlciBjb252 aW5jZSB5b3UgdGhhbiBhbm5veSB5b3UgaW50byBkb2luZyBhcyBJIHN1Z2dlc3RlZC4KPj4KPj4g PiBCVFcgd2hldGhlciB5b3UgIGJ1eSB0aGUgYXJndW1lbnQgb3Igbm90LCBwbGVhc2UgZG8gdHJl YXQgeW91cnNlbGYKPj4gPiB3aXRoIGljZSBjcmVhbSBmb3IgYmVpbmcgYWJsZSB0byBtYWtlIHN1 Y2ggYSBjb21tZW50Lgo+Pgo+PiBXaWxsIGRvLgo+Pgo+PiBUaGFua3MsCj4+Cj4+Cj4+IFBhdWwg Qm9sbGUKPj4KX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18K SW50ZWwtZ2Z4IG1haWxpbmcgbGlzdApJbnRlbC1nZnhAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0 dHA6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9pbnRlbC1nZngK