From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lee Jones Subject: Re: [PATCH V2 2/2] mfd: stmpe: Extend DT support in stmpe driver Date: Thu, 22 Nov 2012 15:46:12 +0000 Message-ID: <20121122154612.GC10986@gmail.com> References: <20121122112451.GE4328@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: devicetree-discuss-bounces+gldd-devicetree-discuss=m.gmane.org-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org Sender: "devicetree-discuss" To: Viresh Kumar Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org, spear-devel-nkJGhpqTU55BDgjK7y7TUQ@public.gmane.org, sameo-VuQAYsv1563Yd54FQh9/CA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-Id: devicetree@vger.kernel.org PiA+IEJpZyBmYXQgTkFDSy4KPiA+Cj4gPiBZb3UndmUganVzdCBvdmVyd3JpdHRlbiB0aGUgY3Vy cmVudCBpbXBsZW1lbnRhdGlvbiB3aXRoIHlvdXIgb3duLgo+ID4gUGxlYXNlIHRha2UgdGltZSB0 byB1bmRlcnN0YW5kIHRoZSBtZWNoYW5pc21zIGluIHBsYWNlIGJlZm9yZQo+ID4geW91IHN1Ym1p dCBhbnkgY2hhbmdlcyBvciBhZGRpdGlvbnMgdG8gaXQuCj4gCj4gOikKPiAKPiBNeSBmYXVsdC4g Q29tbWVudHMgb24gYWxsIG92ZXJ3cml0dGVuIHN0dWZmIGFjY2VwdGVkCgpPa2F5LCBncmVhdC4K Cj4gPj4gZGlmZiAtLWdpdCBhL0RvY3VtZW50YXRpb24vZGV2aWNldHJlZS9iaW5kaW5ncy9tZmQv c3RtcGUudHh0IGIvRG9jdW1lbnRhdGlvbi9kZXZpY2V0cmVlL2JpbmRpbmdzL21mZC9zdG1wZS50 eHQKPiA+PiArLSBpcnEtb3Zlci1ncGlvOiBib29sLCB0cnVlIGlmIGdwaW8gaXMgdXNlZCB0byBn ZXQgaXJxCj4gPj4gKy0gaXJxLWdwaW9zOiBncGlvIG51bWJlciBvdmVyIHdoaWNoIGlycSB3aWxs IGJlIHJlcXVlc3RlZCAoc2lnbmlmaWNhbnQgb25seSBpZgo+ID4+ICsgIGlycS1vdmVyLWdwaW8g aXMgdHJ1ZSkKPiA+Cj4gPiBZb3UgZG9uJ3QgbmVlZCB0aGVzZS4gVXNlIGdwaW9fdG9faXJxKCkg aW5zdGVhZC4KPiAKPiBJIGFtIHBhc3NpbmcgZ3BpbyBudW1iZXJzIGhlcmUgYW5kIGFtIGRvaW5n IGdwaW9fdG9faXJxKCkgaW4gZHJpdmVyLgo+IERpZG4ndCBnZXQgdGhpcyBvbmUgOigKCkZvciBh IHN0YXJ0IHlvdSBoYXZlICdpcnEtb3Zlci1ncGlvJyBpbiB0aGUgYmluZGluZyBkb2N1bWVudCBh bmQgRGV2aWNlIFRyZWUKYW5kICdpcnFfb3Zlcl9ncGlvJyBpbiB0aGUgY29kZS4gSGFzIGl0IGV2 ZW4gYmVlbiB0ZXN0ZWQ/CgpHUElPcyBhcmUgdXNlZCBhcyBJUlEgbGluZXMgaW4gbWFueSBvdGhl ciBwcmV2aW91c2x5IERUOmVkIGRyaXZlcnMuIFRha2UgYQpsb29rIHRvIHNlZSBob3cgdGhleSBh cmUgaGFuZGxlZCB3aXRob3V0IGFkZGluZyB1bm5lY2Vzc2FyeSBEVCBiaW5kaW5ncy4gCgo+ID4+ ICBPcHRpb25hbCBwcm9wZXJ0aWVzOgo+ID4+IC0gLSBpbnRlcnJ1cHRzICAgICAgICAgICAgICAg ICAgIDogVGhlIGludGVycnVwdCBvdXRwdXRzIGZyb20gdGhlIGNvbnRyb2xsZXIKPiA+PiAtIC0g aW50ZXJydXB0LWNvbnRyb2xsZXIgICAgICAgICA6IE1hcmtzIHRoZSBkZXZpY2Ugbm9kZSBhcyBh biBpbnRlcnJ1cHQgY29udHJvbGxlcgo+ID4+IC0gLSBpbnRlcnJ1cHQtcGFyZW50ICAgICAgICAg ICAgIDogU3BlY2lmaWVzIHdoaWNoIElSUSBjb250cm9sbGVyIHdlJ3JlIGNvbm5lY3RlZCB0bwo+ ID4+IC0gLSBpMmMtY2xpZW50LXdha2UgICAgICAgICAgICAgIDogTWFya3MgdGhlIGlucHV0IGRl dmljZSBhcyB3YWthYmxlCj4gPj4gLSAtIHN0LGF1dG9zbGVlcC10aW1lb3V0ICAgICAgICAgOiBW YWxpZCBlbnRyaWVzIChtcyk7IDQsIDE2LCAzMiwgNjQsIDEyOCwgMjU2LCA1MTIgYW5kIDEwMjQK PiA+Cj4gPiBBbmQgeW91J3ZlIHJlbW92ZWQgdGhlc2Ugd2h5Pwo+IAo+IE5vLiBUaGV5IGFyZSBy ZWFkanVzdGVkLi4uCgpQbGVhc2UgZG9uJ3QgcmVhZGp1c3QgdGhlbS4KClRoZXkgYXJlIGNsZWFy IGFuZCBlYXNpbHkgcmVhZGFibGUgaW4gdGhlaXIgY3VycmVudCBzdGF0ZS4gSW4gdGhlIG5ldwpp bXBsZW1lbnRhdGlvbiB0aGV5IGFyZSBtZXNzeSBhbmQgaGFyZCB0byBkZWNpcGhlci4KCj4gT25l IHRoaW5nIHJlbW92ZWQgaXMgaW50ZXJydXB0LWNvbnRyb2xsZXIuIEkgaGFkIGEgZG91YnQgb24g dGhpcy4KPiBzdG1wZSwgYnkgaXRzZWxmIGRvZXNuJ3QgZ2l2ZSBhbnkgaW50ZXJydXB0IGxpbmVz IHRvIFNvQyB0byBmcmVlbHkgdXNlCj4gdGhlbS4gSW5zdGVhZAo+IGdwaW8gY29udHJvbGxlciBk cml2ZXIgcGFydCBvZiBpdCBkb2VzLiBBbmQgc28gYWRkaW5nCj4gaW50ZXJydXB0LWNvbnRyb2xs ZXIgZm9yIHRoYXQgaXMKPiB0aGUgcmlnaHQgb3B0aW9uLgoKVGhhdCdzIGZpbmUsIEkgZG9uJ3Qg aGF2ZSBhbiBpc3N1ZSB3aXRoIHRoYXQuCgo+IHN0bXBlIGlzIGFuIGludGVycnVwdCBjb250cm9s bGVyIGZvciB0aGUgSVAncyB3aGljaCBhcmUgcHJlc2VudCBpbnNpZGUKPiBpdDogZ3BpbywgYWRj Lgo+IEJ1dCBpbnRlcnJ1cHQgbGluZXMgZm9yIHRoZW0gYXJlIG1hbmFnZWQgYnkgc3RtcGUgZHJp dmVyIGludGVybmFsbHkuIFNvIHNob3VsZAo+IHdlIHJlYWxseSBhZGQgaW50ZXJydXB0LWNvbnRy b2xsZXIgZm9yIGl0PwoKWW91IGNhbid0IG1hbmFnZSBJUlEgbGluZXMgaW50ZXJuYWxseSwgeW91 IGhhdmUgdG8gZ28gdGhyb3VnaAp0aGUgSVJRIHN1YnN5c3RlbS4gV2hlbiB5b3UgcmVxdWVzdCBh biBJUlEgdmlhIGRldmljZSB0cmVlIHlvdQp3aWxsIGRvIHNvIGxpa2UgdGhpczoKCiBpbnRlcnJ1 cHRzID0gPDI1IDB4MT47CiBpbnRlcnJ1cHQtcGFyZW50ID0gPCZzdG1wZS1ncGlvPjsKCkluIHRo aXMgZXhhbXBsZSB0aGUgc3RtcGUtZ3BpbyBub2RlIG11c3QgYWR2ZXJ0aXNlIGl0c2VsZiB1c2lu Zwp0aGUgaW50ZXJydXB0LWNvbnRyb2xsZXIgcHJvcGVydHkuCgpUaGUgU1RNUEUgR1BJTyBjb250 cm9sbGVyIGNhbid0IGJlIHVzZWQgYnkgRGV2aWNlIFRyZWUgeWV0IGluIGFueSBjYXNlLApiZWNh dXNlIGl0IGRvZXNuJ3QgaGF2ZSBhbiBJUlEgZG9tYWluLiBUaGlzIGlzIGNvbXB1bHNvcnksIG9y IGl0IHdvbid0CndvcmsuIEhhdmUgeW91IHRyaWVkIHRvIHRlc3QgdGhpcyBmdW5jdGlvbmFsaXR5 IHlldD8KCj4gPj4gKy0ga2V5cGFkLHNjYW4tY291bnQ6IG51bWJlciBvZiBrZXkgc2Nhbm5pbmcg Y3ljbGVzIHRvIGNvbmZpcm0ga2V5IGRhdGEuIE1heGltdW0KPiA+PiArICBpcyBTVE1QRV9LRVlQ QURfTUFYX1NDQU5fQ09VTlQuCj4gPj4gKy0ga2V5cGFkLGRlYm91bmNlLW1zOiBkZWJvdW5jZSBp bnRlcnZhbCwgaW4gbXMuIE1heGltdW0gaXMKPiA+PiArICBTVE1QRV9LRVlQQURfTUFYX0RFQk9V TkNFLgo+ID4+ICstIGtleXBhZCxuby1hdXRvcmVwZWF0OiBib29sLCBkaXNhYmxlIGtleSBhdXRv cmVwZWF0Cj4gPgo+ID4gU2VlICJXaGVuIGFkZGluZyBuZXcgYmluZGluZ3MsIGFzayB5b3Vyc2Vs ZiIgYWJvdmUuCj4gCj4gWWVzLCB0aGVzZSBhcmUgcmVxdWlyZWQuIFRoaXMgaXMgcGFydCBvZiBw bGF0Zm9ybSBkYXRhIGl0IGV4cGVjdHMuCgpPa2F5LCBJJ3ZlIGp1c3QgaGFkIGEgbG9vayBhdCBt eSB0ZXN0ZWQgYmluZGluZ3MuCgpXZSBhbHJlYWR5IGhhdmUgdGhlc2U6CgogIGRlYm91bmNlLWlu dGVydmFsICAgLyogVGhpcyBpcyBhIGdlbmVyaWMgYmluZGluZyAqLwogIHN0LHNjYW4tY291bnQg ICAgICAgLyogVGhlc2UgYXJlIHZlbmRvciBzcGVjaWZpYyAqLwogIHN0LG5vLWF1dG9yZXBlYXQg ICAgLyogICAgICAgICAgICAiICAgICAgICAgICAgICAqLwoKVmVuZG9yIHNwZWNpZmljIGJpbmRp bmdzIHNob3VsZCBiZSAiPHZlbmRvcj4sPGJpbmRpbmc+IiwgcmF0aGVyIHRoYW4KIjx0eXBlX29m X2RyaXZlcj4sPGJpbmRpbmc+IgoKPiA+PiArICAgICAgICAgICAgIHJlZyA9IDwwPjsKPiA+Cj4g PiBZb3UgaGF2ZSByZWcgdHdpY2UgaGVyZS4gQWxzbyByZWcgc2hvdWxkIG5ldmVyIGJlICcwJy4K PiAKPiBGb3IgU1BJLCB0aGVyZSBhcmUgY2hpcCBzZWxlY3RzIGFuZCB0aGVyZSBpcyBubyByZWcg b2Zmc2V0LgoKSSB1bmRlcnN0YW5kIHRoZSBhZGRyZXNzaW5nIGlzc3VlcywgYnV0IHlvdSBoYXZl ICdyZWcnIHR3aWNlLgoKV2hpY2ggb25lIHNob3VsZCBiZSB1c2VkPwoKPiA+PiArICAgICAgICAg ICAgIHN0bXBlNjEwLXRzIHsKPiA+PiArICAgICAgICAgICAgICAgICAgICAgY29tcGF0aWJsZSA9 ICJzdG1wZSx0cyI7Cj4gPj4gKyAgICAgICAgICAgICAgICAgICAgIHRzLHNhbXBsZS10aW1lID0g PDQ+Owo+ID4+ICsgICAgICAgICAgICAgICAgICAgICB0cyxtb2QtMTJiID0gPDE+Owo+ID4+ICsg ICAgICAgICAgICAgICAgICAgICB0cyxyZWYtc2VsID0gPDA+Owo+ID4+ICsgICAgICAgICAgICAg ICAgICAgICB0cyxhZGMtZnJlcSA9IDwxPjsKPiA+PiArICAgICAgICAgICAgICAgICAgICAgdHMs YXZlLWN0cmwgPSA8MT47Cj4gPj4gKyAgICAgICAgICAgICAgICAgICAgIHRzLHRvdWNoLWRldC1k ZWxheSA9IDwyPjsKPiA+PiArICAgICAgICAgICAgICAgICAgICAgdHMsc2V0dGxpbmcgPSA8Mj47 Cj4gPj4gKyAgICAgICAgICAgICAgICAgICAgIHRzLGZyYWN0aW9uLXogPSA8Nz47Cj4gPj4gKyAg ICAgICAgICAgICAgICAgICAgIHRzLGktZHJpdmUgPSA8MT47Cj4gPgo+ID4gV293ISBTZWUgIldo ZW4gYWRkaW5nIG5ldyBiaW5kaW5ncywgYXNrIHlvdXJzZWxmIiBhYm92ZS4KPiAKPiA6KQo+IFRo ZXkgYXJlIHJlcXVpcmVkLiBJIGRpZG4ndCBnZXQgeW91ciBwb2ludCwgc29ycnkuCgpJIGRpZG4n dCBnbyB0aHJvdWdoIHRoZW0sIGJ1dCBhcmUgeW91IHN1cmUgdGhhdDoKCjEuIENhbiBJIGRvIHdp dGhvdXQgdGhlbT8KICAgMS4xIENhbiBJIGRlcml2ZSB0aGUgY29uZmlndXJhdGlvbiBmcm9tIG90 aGVyIHRoaW5ncz8KICAgMi4yIEFyZSB0aGV5IF9yZWFsbHlfIHJlcXVpcmVkLCBvciBhbSBJIGp1 c3QgYmxpbmRseSBjb3B5aW5nIHBsYXRmb3JtIGRhdGE/CjIuIERvZXMgYSBzaW1pbGFyIGJpbmRp bmcgYWxyZWFkeSBleGlzdD8KMy4gQ2FuIG90aGVyIGRyaXZlcnMgbWFrZSB1c2Ugb2YgdGhlbT8K ICAgMy4xIElmIHNvLCBjcmVhdGUgYSBnZW5lcmljIGJpbmRpbmcKICAgMy4yIElmIG5vdCwgcHJl cGVuZCB0aGUgYmluZGluZyB3aXRoICI8dmVuZG9yPiwiCgpUaGVyZSBsb29rcyBsaWtlIGEgbG90 IHRoZXJlLiBTbyBlYWNoIG9mIHRob3NlIHZhbHVlcyB2YXJ5LCBvciBjYW4KYW55IG9mIHRoZW0g YmUgaGFyZC1jb2RlZD8gSnVzdCBiZWNhdXNlIHRoZXkncmUgaW4gcGxhdGZvcm0gY29kZSwKaXQg ZG9lc24ndCBtZWFuIHRoZXkgc2hvdWxkIGJlIGluIHRoZSBEVCBiaW5kaW5nLiBBIGxvdCBvZiBw bGF0Zm9ybQpjb2RlIGlzIGFjdHVhbGx5IGNyYXAuCgo+ID4+IGRpZmYgLS1naXQgYS9kcml2ZXJz L21mZC9zdG1wZS5jIGIvZHJpdmVycy9tZmQvc3RtcGUuYwo+ID4+ICtzdGF0aWMgc3RydWN0IHN0 bXBlX2tleXBhZF9wbGF0Zm9ybV9kYXRhICoKPiA+PiArZ2V0X2tleWJvYXJkX3BkYXRhX2R0KHN0 cnVjdCBkZXZpY2UgKmRldiwgc3RydWN0IGRldmljZV9ub2RlICpucCkKPiA+PiArewo+ID4+ICsg ICAgIHN0cnVjdCBzdG1wZV9rZXlwYWRfcGxhdGZvcm1fZGF0YSAqcGRhdGE7Cj4gPj4gKyAgICAg dTMyIHZhbDsKPiA+PiArCj4gPj4gKyAgICAgcGRhdGEgPSBkZXZtX2t6YWxsb2MoZGV2LCBzaXpl b2YoKnBkYXRhKSwgR0ZQX0tFUk5FTCk7Cj4gPj4gKyAgICAgaWYgKCFwZGF0YSkgewo+ID4+ICsg ICAgICAgICAgICAgZGV2X3dhcm4oZGV2LCAic3RtcGUga2V5cGFkIGt6YWxsb2MgZmFpbFxuIik7 Cj4gPj4gKyAgICAgICAgICAgICByZXR1cm4gTlVMTDsKPiA+PiArICAgICB9Cj4gPj4gKwo+ID4+ ICsgICAgIGlmICghb2ZfcHJvcGVydHlfcmVhZF91MzIobnAsICJrZXlwYWQsc2Nhbi1jb3VudCIs ICZ2YWwpKQo+ID4+ICsgICAgICAgICAgICAgcGRhdGEtPnNjYW5fY291bnQgPSB2YWw7Cj4gPj4g KyAgICAgaWYgKCFvZl9wcm9wZXJ0eV9yZWFkX3UzMihucCwgImtleXBhZCxkZWJvdW5jZS1tcyIs ICZ2YWwpKQo+ID4+ICsgICAgICAgICAgICAgcGRhdGEtPmRlYm91bmNlX21zID0gdmFsOwo+ID4+ ICsgICAgIGlmIChvZl9wcm9wZXJ0eV9yZWFkX2Jvb2wobnAsICJrZXlwYWQsbm8tYXV0b3JlcGVh dCIpKQo+ID4+ICsgICAgICAgICAgICAgcGRhdGEtPm5vX2F1dG9yZXBlYXQgPSB0cnVlOwo+ID4K PiA+IFdoeSBhcmUgeW91IChyZSlhZGRpbmcgdGhlc2UgaGVyZT8gSGF2ZSB5b3UgZXZlbiBsb29r ZWQgaW4gdGhlIGRyaXZlcj8KPiAKPiBCZWNhdXNlIGkgd2FudGVkIHRvIGtlZXAgYWxsIERUIHN0 dWZmIHRvZ2V0aGVyLiBPYnZpb3VzbHkgaSBoYXZlIHNlZW4ga2V5cGFkCj4gZHJpdmVyIGVhcmxp ZXIgOikKCklmIHlvdSBoYWQsIHlvdSdkIHJlYWxpc2UgdGhhdCB0aGVzZSBiaW5kaW5ncyBhbHJl YWR5IGV4aXN0LiA7KQoKSSBhcHByZWNpYXRlIHRoYXQgdGhlIHBhdGNoZXMgaGF2ZW4ndCBsYW5k ZWQgeWV0LCBidXQgdGhleSd2ZSBiZWVuCm9uIHRoZSBNTHMgZm9yIHNvbWUgdGltZSBub3csIGFu ZCBhbHJlYWR5IGhhdmUgc29tZSBpbXBvcnRhbnQgQWNrcy4KCj4gSSBhbSBub3Qgc2V0dGluZyBw ZGF0YSBvZiBzdG1wZSBoZXJlLCBidXQgcGRhdGEgb2Yga2V5cGFkLgoKRXhhY3RseSwgc28gd2h5 IGFyZSB5b3UgZG9pbmcgaXQgaW4gdGhlIFNUTVBFIGRyaXZlcj8KCkl0J3Mga2V5cGFkIGRhdGEs IHNldCBpdCB1cCBpbiB0aGUga2V5cGFkIGRyaXZlci4KCj4gPj4gK3N0YXRpYyBzdHJ1Y3Qgc3Rt cGVfZ3Bpb19wbGF0Zm9ybV9kYXRhICpnZXRfZ3Bpb19wZGF0YV9kdChzdHJ1Y3QgZGV2aWNlICpk ZXYsCj4gPj4gKyAgICAgICAgICAgICBzdHJ1Y3QgZGV2aWNlX25vZGUgKm5wKQo+ID4+ICt7Cj4g Pj4gKyAgICAgc3RydWN0IHN0bXBlX2dwaW9fcGxhdGZvcm1fZGF0YSAqcGRhdGE7Cj4gPj4gKyAg ICAgdTMyIHZhbDsKPiA+PiArCj4gPj4gKyAgICAgcGRhdGEgPSBkZXZtX2t6YWxsb2MoZGV2LCBz aXplb2YoKnBkYXRhKSwgR0ZQX0tFUk5FTCk7Cj4gPj4gKyAgICAgaWYgKCFwZGF0YSkgewo+ID4+ ICsgICAgICAgICAgICAgZGV2X3dhcm4oZGV2LCAic3RtcGUgZ3BpbyBremFsbG9jIGZhaWxcbiIp Owo+ID4+ICsgICAgICAgICAgICAgcmV0dXJuIE5VTEw7Cj4gPj4gKyAgICAgfQo+ID4+ICsKPiA+ PiArICAgICBpZiAoIW9mX3Byb3BlcnR5X3JlYWRfdTMyKG5wLCAiZ3Bpbyxub3JlcXVlc3QtbWFz ayIsICZ2YWwpKQo+ID4+ICsgICAgICAgICAgICAgcGRhdGEtPm5vcmVxdWVzdF9tYXNrID0gdmFs Owo+ID4+ICsKPiA+PiArICAgICAvKiBhc3NpZ24gZ3BpbyBudW1iZXJzIGR5bmFtaWNhbGx5ICov Cj4gPj4gKyAgICAgcGRhdGEtPmdwaW9fYmFzZSA9IC0xOwo+ID4+ICsKPiA+PiArICAgICByZXR1 cm4gcGRhdGE7Cj4gPj4gK30KPiA+Cj4gPiBJcyB0aGlzIGZ1bmN0aW9uIHJlYWxseSByZXF1aXJl ZD8gRXZlbiBpZiBpcyBpcywgc2hvdWxkIGl0IGxpdmUgaGVyZQo+ID4gb3IgaW4gdGhlIFNUTVBF IGRyaXZlcj8KPiAKPiBBcyBzYWlkIGVhcmxpZXIsIGVpdGhlciBpIGNhbiBkbyBEVCBwYXJzaW5n IG9mIHN1Yi1tb2R1bGVzIG9mIHN0bXBlIGluCj4gdGhlaXIgc3BlY2lmaWMgZmlsZXMgb3IgaW4g c3RtcGUgZHJpdmVyIGl0c2VsZi4gQmVjYXVzZSBjdXJyZW50bHkgcGxhdGZvcm0KPiBkYXRhIG9m IHRob3NlIHN1Yi1tb2R1bGVzIGlzIHBhc3NlZCBmcm9tIHN0bXBlLCBpIGtlcHQgdGhlbSBoZXJl IG9ubHkuCj4gc3RtcGUgc3ViIG1vZHVsZXMgY2FuJ3QgbGl2ZSB3aXRob3V0IHN0bXBlIGRyaXZl ciBhbmQgc28ga2VlcGluZyBhbGwKPiBiaW5kaW5nIHN0dWZmIGhlcmUgaXNuJ3QgdGhhdCBiYWQg b2YgYW4gaWRlYS4KClllcywgZG9uJ3QgZG8gdGhhdC4KCkRvIERUIHBhcnNpbmcgaW4gdGhlIGRy aXZlciBpdCByZWZlcmVuY2VzLCBub3QgaW4gdGhlIG1vc3QgY29tbW9uIGRlbm9taW5hdG9yLgoK PiA+PiArc3RhdGljIHN0cnVjdCBzdG1wZV90c19wbGF0Zm9ybV9kYXRhICpnZXRfdHNfcGRhdGFf ZHQoc3RydWN0IGRldmljZSAqZGV2LAo+ID4+ICsgICAgICAgICAgICAgc3RydWN0IGRldmljZV9u b2RlICpucCkKPiA+PiArewo+ID4+ICsgICAgIHN0cnVjdCBzdG1wZV90c19wbGF0Zm9ybV9kYXRh ICpwZGF0YTsKPiA+PiArICAgICB1MzIgdmFsOwo+ID4+ICsKPiA+PiArICAgICBwZGF0YSA9IGRl dm1fa3phbGxvYyhkZXYsIHNpemVvZigqcGRhdGEpLCBHRlBfS0VSTkVMKTsKPiA+PiArICAgICBp ZiAoIXBkYXRhKSB7Cj4gPj4gKyAgICAgICAgICAgICBkZXZfd2FybihkZXYsICJzdG1wZSB0cyBr emFsbG9jIGZhaWxcbiIpOwo+ID4+ICsgICAgICAgICAgICAgcmV0dXJuIE5VTEw7Cj4gPj4gKyAg ICAgfQo+ID4+ICsKPiA+PiArICAgICBpZiAoIW9mX3Byb3BlcnR5X3JlYWRfdTMyKG5wLCAidHMs c2FtcGxlLXRpbWUiLCAmdmFsKSkKPiA+PiArICAgICAgICAgICAgIHBkYXRhLT5zYW1wbGVfdGlt ZSA9IHZhbDsKPiA+PiArICAgICBpZiAoIW9mX3Byb3BlcnR5X3JlYWRfdTMyKG5wLCAidHMsbW9k LTEyYiIsICZ2YWwpKQo+ID4+ICsgICAgICAgICAgICAgcGRhdGEtPm1vZF8xMmIgPSB2YWw7Cj4g Pj4gKyAgICAgaWYgKCFvZl9wcm9wZXJ0eV9yZWFkX3UzMihucCwgInRzLHJlZi1zZWwiLCAmdmFs KSkKPiA+PiArICAgICAgICAgICAgIHBkYXRhLT5yZWZfc2VsID0gdmFsOwo+ID4+ICsgICAgIGlm ICghb2ZfcHJvcGVydHlfcmVhZF91MzIobnAsICJ0cyxhZGMtZnJlcSIsICZ2YWwpKQo+ID4+ICsg ICAgICAgICAgICAgcGRhdGEtPmFkY19mcmVxID0gdmFsOwo+ID4+ICsgICAgIGlmICghb2ZfcHJv cGVydHlfcmVhZF91MzIobnAsICJ0cyxhdmUtY3RybCIsICZ2YWwpKQo+ID4+ICsgICAgICAgICAg ICAgcGRhdGEtPmF2ZV9jdHJsID0gdmFsOwo+ID4+ICsgICAgIGlmICghb2ZfcHJvcGVydHlfcmVh ZF91MzIobnAsICJ0cyx0b3VjaC1kZXQtZGVsYXkiLCAmdmFsKSkKPiA+PiArICAgICAgICAgICAg IHBkYXRhLT50b3VjaF9kZXRfZGVsYXkgPSB2YWw7Cj4gPj4gKyAgICAgaWYgKCFvZl9wcm9wZXJ0 eV9yZWFkX3UzMihucCwgInRzLHNldHRsaW5nIiwgJnZhbCkpCj4gPj4gKyAgICAgICAgICAgICBw ZGF0YS0+c2V0dGxpbmcgPSB2YWw7Cj4gPj4gKyAgICAgaWYgKCFvZl9wcm9wZXJ0eV9yZWFkX3Uz MihucCwgInRzLGZyYWN0aW9uLXoiLCAmdmFsKSkKPiA+PiArICAgICAgICAgICAgIHBkYXRhLT5m cmFjdGlvbl96ID0gdmFsOwo+ID4+ICsgICAgIGlmICghb2ZfcHJvcGVydHlfcmVhZF91MzIobnAs ICJ0cyxpLWRyaXZlIiwgJnZhbCkpCj4gPj4gKyAgICAgICAgICAgICBwZGF0YS0+aV9kcml2ZSA9 IHZhbDsKPiA+PiArCj4gPj4gKyAgICAgcmV0dXJuIHBkYXRhOwo+ID4+ICt9Cj4gPgo+ID4gQXMg YWJvdmUuCj4gPgo+ID4gSSdtIGdvaW5nIHRvIHN0b3AgbXkgcmV2aWV3IGhlcmUuIEkgdGhpbmsg eW91IGdldCB0aGUgaWRlYS4KPiAKPiBJIGdvdCBtb3N0IG9mIHlvdXIgd29ycmllcywgYnV0IGNv dWxkbid0IHVuZGVyc3RhbmQgdGhlIGlzc3VlIG9mIHBhc3NpbmcKPiBhbGwgcGRhdGEgZmllbGRz IGFzIERUIGJpbmRpbmdzLgoKSXQncyBhIHJlYWxseSBiaWcgaXNzdWUgd2l0aCBEVCBlbmFibGVt ZW50LiBQbGF0Zm9ybSBkYXRhIGhhcyBiZWVuIGZpbGxlZAp3aXRoIGxvdHMgb2YganVuayBvdmVy IHRoZSB5ZWFycywgdG8gYmxpbmRseSBtb3ZlIGl0IGFsbCBvdmVyIHRvIERUIHdvdWxkCmJlIGZv b2xpc2guIE5vdyBpcyB0aGUgdGltZSB0byBnbyB0aHJvdWdoIGVhY2ggb2YgdGhlbSBhbmQgcHVs bCBvdXQgdGhlCm9uZXMgdGhhdCB3ZSBjYW4gZG8gd2l0aG91dCwgY2FuIGJlIGhhcmQtY29kZWQg b3IgZGVyaXZlZCBpbiB0aGUgZHJpdmVyLgpJJ20gbW9yZSB0aGFuIHN1cmUgdGhhdCBhdCBsZWFz dCBzb21lIG9mIHRoZSBhYm92ZSBjYW4gYmUgcmVtb3ZlZCB3aXRoCnNvbWUgdGhvdWdodGZ1bCBj b2RpbmcuCgo+IFRoYW5rcyBmb3IgeW91ciByZXZpZXcgdGhvdWdoLiA6KQoKTm8gcHJvYmxlbS4K Ci0tIApMZWUgSm9uZXMKTGluYXJvIFNULUVyaWNzc29uIExhbmRpbmcgVGVhbSBMZWFkCkxpbmFy by5vcmcg4pSCIE9wZW4gc291cmNlIHNvZnR3YXJlIGZvciBBUk0gU29DcwpGb2xsb3cgTGluYXJv OiBGYWNlYm9vayB8IFR3aXR0ZXIgfCBCbG9nCl9fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fCmRldmljZXRyZWUtZGlzY3VzcyBtYWlsaW5nIGxpc3QKZGV2aWNl dHJlZS1kaXNjdXNzQGxpc3RzLm96bGFicy5vcmcKaHR0cHM6Ly9saXN0cy5vemxhYnMub3JnL2xp c3RpbmZvL2RldmljZXRyZWUtZGlzY3Vzcwo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752552Ab2KVS3W (ORCPT ); Thu, 22 Nov 2012 13:29:22 -0500 Received: from mail-bk0-f46.google.com ([209.85.214.46]:37528 "EHLO mail-bk0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752346Ab2KVS3M (ORCPT ); Thu, 22 Nov 2012 13:29:12 -0500 Date: Thu, 22 Nov 2012 15:46:12 +0000 From: Lee Jones To: Viresh Kumar Cc: sameo@linux.intel.com, devicetree-discuss@lists.ozlabs.org, linux-kernel@vger.kernel.org, spear-devel@list.st.com, Vipul Kumar Samar Subject: Re: [PATCH V2 2/2] mfd: stmpe: Extend DT support in stmpe driver Message-ID: <20121122154612.GC10986@gmail.com> References: <20121122112451.GE4328@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org > > Big fat NACK. > > > > You've just overwritten the current implementation with your own. > > Please take time to understand the mechanisms in place before > > you submit any changes or additions to it. > > :) > > My fault. Comments on all overwritten stuff accepted Okay, great. > >> diff --git a/Documentation/devicetree/bindings/mfd/stmpe.txt b/Documentation/devicetree/bindings/mfd/stmpe.txt > >> +- irq-over-gpio: bool, true if gpio is used to get irq > >> +- irq-gpios: gpio number over which irq will be requested (significant only if > >> + irq-over-gpio is true) > > > > You don't need these. Use gpio_to_irq() instead. > > I am passing gpio numbers here and am doing gpio_to_irq() in driver. > Didn't get this one :( For a start you have 'irq-over-gpio' in the binding document and Device Tree and 'irq_over_gpio' in the code. Has it even been tested? GPIOs are used as IRQ lines in many other previously DT:ed drivers. Take a look to see how they are handled without adding unnecessary DT bindings. > >> Optional properties: > >> - - interrupts : The interrupt outputs from the controller > >> - - interrupt-controller : Marks the device node as an interrupt controller > >> - - interrupt-parent : Specifies which IRQ controller we're connected to > >> - - i2c-client-wake : Marks the input device as wakable > >> - - st,autosleep-timeout : Valid entries (ms); 4, 16, 32, 64, 128, 256, 512 and 1024 > > > > And you've removed these why? > > No. They are readjusted... Please don't readjust them. They are clear and easily readable in their current state. In the new implementation they are messy and hard to decipher. > One thing removed is interrupt-controller. I had a doubt on this. > stmpe, by itself doesn't give any interrupt lines to SoC to freely use > them. Instead > gpio controller driver part of it does. And so adding > interrupt-controller for that is > the right option. That's fine, I don't have an issue with that. > stmpe is an interrupt controller for the IP's which are present inside > it: gpio, adc. > But interrupt lines for them are managed by stmpe driver internally. So should > we really add interrupt-controller for it? You can't manage IRQ lines internally, you have to go through the IRQ subsystem. When you request an IRQ via device tree you will do so like this: interrupts = <25 0x1>; interrupt-parent = <&stmpe-gpio>; In this example the stmpe-gpio node must advertise itself using the interrupt-controller property. The STMPE GPIO controller can't be used by Device Tree yet in any case, because it doesn't have an IRQ domain. This is compulsory, or it won't work. Have you tried to test this functionality yet? > >> +- keypad,scan-count: number of key scanning cycles to confirm key data. Maximum > >> + is STMPE_KEYPAD_MAX_SCAN_COUNT. > >> +- keypad,debounce-ms: debounce interval, in ms. Maximum is > >> + STMPE_KEYPAD_MAX_DEBOUNCE. > >> +- keypad,no-autorepeat: bool, disable key autorepeat > > > > See "When adding new bindings, ask yourself" above. > > Yes, these are required. This is part of platform data it expects. Okay, I've just had a look at my tested bindings. We already have these: debounce-interval /* This is a generic binding */ st,scan-count /* These are vendor specific */ st,no-autorepeat /* " */ Vendor specific bindings should be ",", rather than "," > >> + reg = <0>; > > > > You have reg twice here. Also reg should never be '0'. > > For SPI, there are chip selects and there is no reg offset. I understand the addressing issues, but you have 'reg' twice. Which one should be used? > >> + stmpe610-ts { > >> + compatible = "stmpe,ts"; > >> + ts,sample-time = <4>; > >> + ts,mod-12b = <1>; > >> + ts,ref-sel = <0>; > >> + ts,adc-freq = <1>; > >> + ts,ave-ctrl = <1>; > >> + ts,touch-det-delay = <2>; > >> + ts,settling = <2>; > >> + ts,fraction-z = <7>; > >> + ts,i-drive = <1>; > > > > Wow! See "When adding new bindings, ask yourself" above. > > :) > They are required. I didn't get your point, sorry. I didn't go through them, but are you sure that: 1. Can I do without them? 1.1 Can I derive the configuration from other things? 2.2 Are they _really_ required, or am I just blindly copying platform data? 2. Does a similar binding already exist? 3. Can other drivers make use of them? 3.1 If so, create a generic binding 3.2 If not, prepend the binding with "," There looks like a lot there. So each of those values vary, or can any of them be hard-coded? Just because they're in platform code, it doesn't mean they should be in the DT binding. A lot of platform code is actually crap. > >> diff --git a/drivers/mfd/stmpe.c b/drivers/mfd/stmpe.c > >> +static struct stmpe_keypad_platform_data * > >> +get_keyboard_pdata_dt(struct device *dev, struct device_node *np) > >> +{ > >> + struct stmpe_keypad_platform_data *pdata; > >> + u32 val; > >> + > >> + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); > >> + if (!pdata) { > >> + dev_warn(dev, "stmpe keypad kzalloc fail\n"); > >> + return NULL; > >> + } > >> + > >> + if (!of_property_read_u32(np, "keypad,scan-count", &val)) > >> + pdata->scan_count = val; > >> + if (!of_property_read_u32(np, "keypad,debounce-ms", &val)) > >> + pdata->debounce_ms = val; > >> + if (of_property_read_bool(np, "keypad,no-autorepeat")) > >> + pdata->no_autorepeat = true; > > > > Why are you (re)adding these here? Have you even looked in the driver? > > Because i wanted to keep all DT stuff together. Obviously i have seen keypad > driver earlier :) If you had, you'd realise that these bindings already exist. ;) I appreciate that the patches haven't landed yet, but they've been on the MLs for some time now, and already have some important Acks. > I am not setting pdata of stmpe here, but pdata of keypad. Exactly, so why are you doing it in the STMPE driver? It's keypad data, set it up in the keypad driver. > >> +static struct stmpe_gpio_platform_data *get_gpio_pdata_dt(struct device *dev, > >> + struct device_node *np) > >> +{ > >> + struct stmpe_gpio_platform_data *pdata; > >> + u32 val; > >> + > >> + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); > >> + if (!pdata) { > >> + dev_warn(dev, "stmpe gpio kzalloc fail\n"); > >> + return NULL; > >> + } > >> + > >> + if (!of_property_read_u32(np, "gpio,norequest-mask", &val)) > >> + pdata->norequest_mask = val; > >> + > >> + /* assign gpio numbers dynamically */ > >> + pdata->gpio_base = -1; > >> + > >> + return pdata; > >> +} > > > > Is this function really required? Even if is is, should it live here > > or in the STMPE driver? > > As said earlier, either i can do DT parsing of sub-modules of stmpe in > their specific files or in stmpe driver itself. Because currently platform > data of those sub-modules is passed from stmpe, i kept them here only. > stmpe sub modules can't live without stmpe driver and so keeping all > binding stuff here isn't that bad of an idea. Yes, don't do that. Do DT parsing in the driver it references, not in the most common denominator. > >> +static struct stmpe_ts_platform_data *get_ts_pdata_dt(struct device *dev, > >> + struct device_node *np) > >> +{ > >> + struct stmpe_ts_platform_data *pdata; > >> + u32 val; > >> + > >> + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); > >> + if (!pdata) { > >> + dev_warn(dev, "stmpe ts kzalloc fail\n"); > >> + return NULL; > >> + } > >> + > >> + if (!of_property_read_u32(np, "ts,sample-time", &val)) > >> + pdata->sample_time = val; > >> + if (!of_property_read_u32(np, "ts,mod-12b", &val)) > >> + pdata->mod_12b = val; > >> + if (!of_property_read_u32(np, "ts,ref-sel", &val)) > >> + pdata->ref_sel = val; > >> + if (!of_property_read_u32(np, "ts,adc-freq", &val)) > >> + pdata->adc_freq = val; > >> + if (!of_property_read_u32(np, "ts,ave-ctrl", &val)) > >> + pdata->ave_ctrl = val; > >> + if (!of_property_read_u32(np, "ts,touch-det-delay", &val)) > >> + pdata->touch_det_delay = val; > >> + if (!of_property_read_u32(np, "ts,settling", &val)) > >> + pdata->settling = val; > >> + if (!of_property_read_u32(np, "ts,fraction-z", &val)) > >> + pdata->fraction_z = val; > >> + if (!of_property_read_u32(np, "ts,i-drive", &val)) > >> + pdata->i_drive = val; > >> + > >> + return pdata; > >> +} > > > > As above. > > > > I'm going to stop my review here. I think you get the idea. > > I got most of your worries, but couldn't understand the issue of passing > all pdata fields as DT bindings. It's a really big issue with DT enablement. Platform data has been filled with lots of junk over the years, to blindly move it all over to DT would be foolish. Now is the time to go through each of them and pull out the ones that we can do without, can be hard-coded or derived in the driver. I'm more than sure that at least some of the above can be removed with some thoughtful coding. > Thanks for your review though. :) No problem. -- Lee Jones Linaro ST-Ericsson Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog