From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Shevchenko, Andriy" Subject: Re: [PATCH 1/3 v2] GPIO: gpio-dwapb: Enable platform driver binding to MFD driver Date: Fri, 5 Sep 2014 12:02:01 +0000 Message-ID: <1409918522.30155.89.camel@intel.com> References: <1409928798-31895-1-git-send-email-alvin.chen@intel.com> <1409928798-31895-2-git-send-email-alvin.chen@intel.com> <1409908917.30155.84.camel@intel.com> <4656BEB6164FC34F8171C6538F1A595B2E9829DA@SHSMSX101.ccr.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mga09.intel.com ([134.134.136.24]:21225 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932171AbaIEMEO (ORCPT ); Fri, 5 Sep 2014 08:04:14 -0400 In-Reply-To: <4656BEB6164FC34F8171C6538F1A595B2E9829DA@SHSMSX101.ccr.corp.intel.com> Content-Language: en-US Content-ID: <4716A4D10706B54DB49DB39C9F341257@intel.com> Sender: linux-gpio-owner@vger.kernel.org List-Id: linux-gpio@vger.kernel.org To: "Chen, Alvin" Cc: "linux-kernel@vger.kernel.org" , "robh+dt@kernel.org" , "Kweh, Hock Leong" , "sebastian@breakpoint.cc" , "devicetree@vger.kernel.org" , "Ong, Boon Leong" , "gnurou@gmail.com" , "linus.walleij@linaro.org" , "linux-gpio@vger.kernel.org" , "grant.likely@linaro.org" , "Westerberg, Mika" , "dvhart@linux.intel.com" , "atull@opensource.altera.com" T24gRnJpLCAyMDE0LTA5LTA1IGF0IDEwOjIwICswMDAwLCBDaGVuLCBBbHZpbiB3cm90ZToNCj4g PiA+IC0JcG9ydC0+YmdjLmdjLm5ncGlvID0gbmdwaW87DQo+ID4gPiAtCXBvcnQtPmJnYy5nYy5v Zl9ub2RlID0gcG9ydF9ucDsNCj4gPiA+ICsjaWZkZWYgQ09ORklHX09GX0dQSU8NCj4gPiANCj4g PiBEbyB3ZSByZWFsbHkgbmVlZCB0aGlzICNpZmRlZiA/DQo+ID4gb2Zfbm9kZSB3aWxsIGJlIE5V TEwgYW55d2F5LCBvciBJIG1pc3NlZCBzb21ldGhpbmc/DQo+IFllcywgb3RoZXJ3aXNlLCBjYW4n dCBjb21waWxlIGl0LiBQbGVhc2UgcmVmZXIgJ3N0cnVjdCBncGlvX2NoaXAnLCAnZ2Mub2Zfbm9k ZScgaXMgaW4gT0ZfR1BJTyBtaWNybyBhbHNvLg0KDQpBaCwgb2theS4gVGh1cywgaXQgZGVwZW5k cyB0byBMaW51cyBvcGluaW9uLCBzaW5jZSBJLCBmb3IgZXhhbXBsZSwgd291bGQNCmxpa2UgdG8g c2VlIHRoaXMgZmllbGQgcHJlc2VudCBpbiB0aGUgc3RydWN0dXJlIGluZGVwZW5kZW50bHkgb2YN Ck9GX0dQSU8uDQoNCj4gDQo+ID4gPiArCWlmIChwcC0+aXJxKQ0KPiA+IA0KPiA+IGlycSA9PSAw IGlzIGEgdmFsaWQgaHdpcnEgKGhhcmR3YXJlIGlycSkgbnVtYmVyLiBZZXMsIHRoZXJlIGlzIHVu bGlrZWx5IHdlIGhhdmUgaXQNCj4gPiBzb21ld2hlcmUsIGJ1dCBzdGlsbCBpdCdzIHBvc3NpYmxl LiBBbmQgeWVzLCBJUlEgZnJhbWV3b3JrIGRvZXNuJ3Qgd29yayB3aXRoDQo+ID4gdmlycSA9PSAw ICgqdmlydHVhbCogaXJxKSwgYnV0IGFjY2VwdHMgaHdpcnEgPT0gMC4gSSByZWNvbW1lbmQgdG8g dXNlIGludCB0eXBlIGZvcg0KPiA+IGlycSBsaW5lIG51bWJlciwgYW5kIHJlY29nbml6ZSBuZWdh dGl2ZSB2YWx1ZSAodXN1YWxseSAtMSkgYXMgbm8gaXJxIG5lZWRlZCAvDQo+ID4gZm91bmQuDQo+ IFVuZGVyc3RhbmQuIEJ1dCBpZiB5b3UgcmVmZXIgdGhlIG9yaWdpbmFsIGNvZGUsIHlvdSBjYW4g c2VlOg0KPiBpcnEgPSBpcnFfb2ZfcGFyc2VfYW5kX21hcChub2RlLCAwKTsNCj4gSWYgKCFpcnEp IHsNCj4gLi4uLi4uDQo+IHJldHVybjsNCj4gfQ0KPiBGcm9tIGFib3ZlIGNvZGUsIGlmIGlycT0w LCBpdCBpbmRpY2F0ZXMgaXJxIGlzIG5vdCBzdXBwb3J0ZWQgZm9yIE9GIGRldmljZXMuIElmIHdl IHVzZSAnLTEnIHRvIGluZGljYXRlIGlycSBpcyBub3Qgc3VwcG9ydGVkLiBUbyBtYWtlIE9GIHdv cmssIHRoZW4gb3VyIGNvZGUgc2hvdWxkIGJlOg0KDQpZZXMsIGxpa2UgSSBzYWlkIGFib3ZlLiBZ b3UgaW50cm9kdWNlIGh3IGlycSBpbiB0aGUgcGRhdGEsIHdoaWNoIGNvdWxkDQpiZSAwLg0KDQoN Cj4gaXJxID0gaXJxX29mX3BhcnNlX2FuZF9tYXAobm9kZSwgMCk7DQo+IElmICghaXJxKSB7DQo+ IHBwLT5pcnEgPSAtMTsNCj4gcmV0dXJuOw0KPiB9IGVsc2Ugew0KPiBwcC0+aXJxID0gaXJxOw0K PiB9DQo+IFRoZW4gdGhlIGNvZGUgbG9va3Mgc3RyYW5nZS4NCj4gDQo+IEhvdyBkbyB5b3UgdGhp bms/DQoNCklmIEkgdW5kZXJzdG9vZCBjb3JyZWN0bHkgeW91IG1lc3NlZCB1cCB3aXRoIGh3aXJx IHZzLiB2aXJxLg0KT3RoZXJ3aXNlIHlvdSBoYXZlIG1lbnRpb24gdGhhdCB5b3UgYXJlIHVzaW5n IHZpcnEgZXZlcnl3aGVyZSAoSSBndWVzcw0KeW91IG1heSByZW5hbWUgdGhlIGZpZWxkIGluIHRo ZSBzdHJ1Y3R1cmUpLCBidXQgaW4gdGhpcyBjYXNlIHRoZSBmaWVsZA0KaW4gdGhlIHBsYXRmb3Jt X2RhdGEgbG9va3MgYSBiaXQgc3RyYW5nZS4gTGludXMsIHdoYXQgZG8geW91IHRoaW5rPw0KDQoN ClAuUy4gUGxlYXNlLCByZW1vdmUgbXkgUmV2aWV3ZWQtYnkgdGFnIHNpbmNlIGNvZGUgaXMgY2hh bmdlZCBlbm91Z2guDQoNCi0tIA0KQW5keSBTaGV2Y2hlbmtvIDxhbmRyaXkuc2hldmNoZW5rb0Bp bnRlbC5jb20+DQpJbnRlbCBGaW5sYW5kIE95DQotLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0KSW50ZWwgRmlubGFuZCBP eQpSZWdpc3RlcmVkIEFkZHJlc3M6IFBMIDI4MSwgMDAxODEgSGVsc2lua2kgCkJ1c2luZXNzIElk ZW50aXR5IENvZGU6IDAzNTc2MDYgLSA0IApEb21pY2lsZWQgaW4gSGVsc2lua2kgCgpUaGlzIGUt bWFpbCBhbmQgYW55IGF0dGFjaG1lbnRzIG1heSBjb250YWluIGNvbmZpZGVudGlhbCBtYXRlcmlh bCBmb3IKdGhlIHNvbGUgdXNlIG9mIHRoZSBpbnRlbmRlZCByZWNpcGllbnQocykuIEFueSByZXZp ZXcgb3IgZGlzdHJpYnV0aW9uCmJ5IG90aGVycyBpcyBzdHJpY3RseSBwcm9oaWJpdGVkLiBJZiB5 b3UgYXJlIG5vdCB0aGUgaW50ZW5kZWQKcmVjaXBpZW50LCBwbGVhc2UgY29udGFjdCB0aGUgc2Vu ZGVyIGFuZCBkZWxldGUgYWxsIGNvcGllcy4K From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932191AbaIEMEQ (ORCPT ); Fri, 5 Sep 2014 08:04:16 -0400 Received: from mga09.intel.com ([134.134.136.24]:21225 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932171AbaIEMEO (ORCPT ); Fri, 5 Sep 2014 08:04:14 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.04,472,1406617200"; d="scan'208";a="598475282" From: "Shevchenko, Andriy" To: "Chen, Alvin" CC: "linux-kernel@vger.kernel.org" , "robh+dt@kernel.org" , "Kweh, Hock Leong" , "sebastian@breakpoint.cc" , "devicetree@vger.kernel.org" , "Ong, Boon Leong" , "gnurou@gmail.com" , "linus.walleij@linaro.org" , "linux-gpio@vger.kernel.org" , "grant.likely@linaro.org" , "Westerberg, Mika" , "dvhart@linux.intel.com" , "atull@opensource.altera.com" Subject: Re: [PATCH 1/3 v2] GPIO: gpio-dwapb: Enable platform driver binding to MFD driver Thread-Topic: [PATCH 1/3 v2] GPIO: gpio-dwapb: Enable platform driver binding to MFD driver Thread-Index: AQHPyNc/kBMLSfD+Nk6gyOY6CiHQz5vyMzuAgAAX0oCAABToAA== Date: Fri, 5 Sep 2014 12:02:01 +0000 Message-ID: <1409918522.30155.89.camel@intel.com> References: <1409928798-31895-1-git-send-email-alvin.chen@intel.com> <1409928798-31895-2-git-send-email-alvin.chen@intel.com> <1409908917.30155.84.camel@intel.com> <4656BEB6164FC34F8171C6538F1A595B2E9829DA@SHSMSX101.ccr.corp.intel.com> In-Reply-To: <4656BEB6164FC34F8171C6538F1A595B2E9829DA@SHSMSX101.ccr.corp.intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.237.72.168] Content-Type: text/plain; charset="utf-8" Content-ID: <4716A4D10706B54DB49DB39C9F341257@intel.com> MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from base64 to 8bit by mail.home.local id s85C4PYh017073 On Fri, 2014-09-05 at 10:20 +0000, Chen, Alvin wrote: > > > - port->bgc.gc.ngpio = ngpio; > > > - port->bgc.gc.of_node = port_np; > > > +#ifdef CONFIG_OF_GPIO > > > > Do we really need this #ifdef ? > > of_node will be NULL anyway, or I missed something? > Yes, otherwise, can't compile it. Please refer 'struct gpio_chip', 'gc.of_node' is in OF_GPIO micro also. Ah, okay. Thus, it depends to Linus opinion, since I, for example, would like to see this field present in the structure independently of OF_GPIO. > > > > + if (pp->irq) > > > > irq == 0 is a valid hwirq (hardware irq) number. Yes, there is unlikely we have it > > somewhere, but still it's possible. And yes, IRQ framework doesn't work with > > virq == 0 (*virtual* irq), but accepts hwirq == 0. I recommend to use int type for > > irq line number, and recognize negative value (usually -1) as no irq needed / > > found. > Understand. But if you refer the original code, you can see: > irq = irq_of_parse_and_map(node, 0); > If (!irq) { > ...... > return; > } > From above code, if irq=0, it indicates irq is not supported for OF devices. If we use '-1' to indicate irq is not supported. To make OF work, then our code should be: Yes, like I said above. You introduce hw irq in the pdata, which could be 0. > irq = irq_of_parse_and_map(node, 0); > If (!irq) { > pp->irq = -1; > return; > } else { > pp->irq = irq; > } > Then the code looks strange. > > How do you think? If I understood correctly you messed up with hwirq vs. virq. Otherwise you have mention that you are using virq everywhere (I guess you may rename the field in the structure), but in this case the field in the platform_data looks a bit strange. Linus, what do you think? P.S. Please, remove my Reviewed-by tag since code is changed enough. -- Andy Shevchenko Intel Finland Oy --------------------------------------------------------------------- Intel Finland Oy Registered Address: PL 281, 00181 Helsinki Business Identity Code: 0357606 - 4 Domiciled in Helsinki This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. {.n++%ݶw{.n+{G{ayʇڙ,jfhz_(階ݢj"mG?&~iOzv^m ?I