From: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
To: Khuong Dinh <kdinh@apm.com>
Cc: rjw@rjwysocki.net, Duc Dang <dhdang@apm.com>,
Marc Zyngier <marc.zyngier@arm.com>,
linux-pci@vger.kernel.org, msalter@redhat.com, patches@apm.com,
linux-kernel@vger.kernel.org, jcm@redhat.com,
Bjorn Helgaas <bhelgaas@google.com>,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v2 pci] PCI/MSI: pci-xgene-msi: Enable MSI support in ACPI boot for X-Gene v1
Date: Thu, 8 Jun 2017 11:39:56 +0100 [thread overview]
Message-ID: <20170608103956.GB8644@red-moon> (raw)
In-Reply-To: <CAAsHzqvnGWkG_KN6m_+HzGFaNOHU-m2FocTxy3pyM7Hhd9rz7w@mail.gmail.com>
T24gVHVlLCBKdW4gMDYsIDIwMTcgYXQgMDk6NDQ6MTVBTSAtMDcwMCwgS2h1b25nIERpbmggd3Jv
dGU6Cj4gSGkgTG9yZW56bywKPiAKPiBPbiBUdWUsIE1heSA5LCAyMDE3IGF0IDM6NTUgUE0sIEto
dW9uZyBEaW5oIDxrZGluaEBhcG0uY29tPiB3cm90ZToKPiA+IEhpIExvcmVuem8sCj4gPgo+ID4g
T24gRnJpLCBNYXkgNSwgMjAxNyBhdCA5OjUxIEFNLCBMb3JlbnpvIFBpZXJhbGlzaQo+ID4gPGxv
cmVuem8ucGllcmFsaXNpQGFybS5jb20+IHdyb3RlOgo+ID4+IE9uIFRodSwgTWF5IDA0LCAyMDE3
IGF0IDA1OjM2OjA2UE0gLTA3MDAsIEtodW9uZyBEaW5oIHdyb3RlOgo+ID4+PiBIaSBNYXJjLAo+
ID4+PiBUaGVyZSdzIG5vIGV4cGxpY2l0IGRlcGVuZGVuY3kgYmV0d2VlbiBwY2llIGRyaXZlciBh
bmQgbXNpCj4gPj4+IGNvbnRyb2xsZXIuICBUaGUgY3VycmVudCBzb2x1dGlvbiB0aGF0IHdlIGRp
ZCBpcyByZWx5aW5nIG9uIHRoZQo+ID4+PiBub2RlIG9yZGVyaW5nIGluIEJJT1MuICBBQ1BJIDUu
MCBpbnRyb2R1Y2VkIF9ERVAgbWV0aG9kIHRvIGFzc2lnbiBhCj4gPj4+IGhpZ2hlciBwcmlvcml0
eSBpbiBzdGFydCBvcmRlcmluZy4gIFRoaXMgbWV0aG9kIGNvdWxkIGJlIGFwcGxpZWQgaW4KPiA+
Pj4gY2FzZSBvZiBtc2kgYW5kIHBjaWUgYXJlIHRoZSBzYW1lIGxldmVsIG9mIHN1YnN5c19pbml0
IChpbiBBQ1BJCj4gPj4+IGJvb3QpLiAgSG93ZXZlciwgUENJZSBkcml2ZXIgaGFzIG5vdCBzdXBw
b3J0ZWQgZm9yIHRoaXMgZGVwZW5kZW5jeQo+ID4+PiBjaGVjayB5ZXQuICBIb3cgZG8geW91IHRo
aW5rIGFib3V0IHRoaXMgc29sdXRpb24uCj4gPj4KPiA+PiBGaXJzdCBvZmYsIHlvdSBjYW4ndCBw
b3N0IGVtYWlscyB3aXRoIGNvbmZpZGVudGlhbGl0eSBub3RpY2VzIG9uCj4gPj4gbWFpbGluZyBs
aXN0cyBzbyByZW1vdmUgaXQgZnJvbSBub3cgb253YXJkcy4KPiA+Cj4gPiBGaXhlZAo+ID4KPiA+
PiBTZWNvbmRseSwgSSBjb21tZW50ZWQgb24gdGhpcyBiZWZvcmUsIHNvIHlvdSBrbm93IHdoYXQg
bXkgb3BpbmlvbiBpcy4KPiA+Cj4gPiBJIGdvdCB5b3VyIG9waW5pb24uIEknbGwgaW1wbGVtZW50
IGFzIHlvdXIgc3VnZ2VzdGlvbi4KPiA+Cj4gCj4gICBSZWdhcmRpbmcgdG8geW91ciBwcmV2aW91
cyBzdWdnZXN0aW9uIHRvIGFkZCBhIGhvb2sgd2Fsa2luZyB0aGUgQUNQSQo+ICAgbmFtZXNwYWNl
IGJlZm9yZSBhY3BpX2J1c19zY2FuKCkgaW4gYWNwaV9zY2FuX2luaXQoKSB0byBtYWtlIE1TSQo+
ICAgY29udHJvbGxlcnMgbXVzdCBiZSBwcm9iZWQgYmVmb3JlIFBDSS4gIEkgaGF2ZSBhIGNvbmNl
cm4gdGhhdCB0aGUKPiAgIE1TSSBuYW1lc3BhY2Ug4oCcIF9TQi5NU0lY4oCdIGlzIG5vdCBhIHVu
aXF1ZSBuYW1lIGFuZCBob3cgY2FuIHdlIHdhbGsKPiAgIHRoZSBBQ1BJIG5hbWVzcGFjZSBhbmQg
dXNlIHRoaXMgbmFtZSB0byBtYWtlIE1TSSBwcm9iZWQgYmVmb3JlIFBDSS4KPiAgIE1heSB5b3Ug
aGF2ZSBsaXR0bGUgYml0IG1vcmUgaW5mb3JtYXRpb24gZm9yIHRoaXMgb3IgZG8geW91IGhhdmUK
PiAgIGFub3RoZXIgc3VnZ2VzdGlvbiBmb3IgdGhpcz8KPiAKPiAgICBUaGVyZeKAmXMgYW5vdGhl
ciBzb2x1dGlvbiB3aGljaCBtYWtlcyB0aGlzIHNpbXBsZXIgYW5kIEnigJlkIGxpa2UgdG8KPiAg
ICBhc2sgeW91ciBvcGluaW9uIGFib3V0IHRoaXMuCj4gICAgVGhlIHNvbHV0aW9uIGlzIHRvIG1h
a2UgYW4gaGllcmFyY2h5IGJldHdlZW4gTVNJIGFuZCBQQ0kgbm9kZXMgIGFzIGJlbG93Ogo+IERl
dmljZShcX1NCLk1TSVgpIHsKPiAgICBEZXZpY2UoXF9TQi5QQ0kwKQo+ICAgIERldmljZShcX1NC
LlBDSTEpCj4gICAg4oCm4oCmCj4gfQo+ICAgSW4gb3RoZXIgd29yZCwgTVNJWCBub2RlIGlzIGEg
cGFyZW50IG5vZGUgb2YgUENJIG5vZGVzIGluIEFDUEkKPiAgIHRhYmxlLiAgSW4gdGhpcyBzZW5z
ZSwgdGhlcmXigJlzIGFuIGV4cGxpY2l0IGRlcGVuZGVuY3kgYmV0d2VlbiBNU0kKPiAgIGFuZCBQ
Q0ksIE1TSSBjb250cm9sbGVyIG11c3QgYmUgcHJvYmVkIGJlZm9yZSBQQ0kgYW5kIGl0IHdpbGwK
PiAgIGd1YXJhbnRlZSBub3QgYnJlYWtpbmcgbmV4dCBrZXJuZWwgcmVsZWFzZXMuICBIb3cgZG8g
eW91IHRoaW5rIGFib3V0Cj4gICB0aGlzIHNvbHV0aW9uLgoKSSB0aGluayB0aGF0J3MgYSBwbGFz
dGVyIGFzIGluZWZmZWN0aXZlIGFzIHJlb3JkZXJpbmcgbm9kZXMsIGluIHNob3J0Cml0IGlzIG5v
dCBhIHNvbHV0aW9uIGFuZCB5b3Ugc3RpbGwgcmVseSBvbiBrZXJuZWwgbGluayBvcmRlcmluZywg
eW91CmNhbiBmaWRkbGUgd2l0aCBBQ1BJIHRhYmxlcyBhcyBtdWNoIGFzIHlvdSB3YW50IGJ1dCB0
aGF0IGRvZXMgbm90IGNoYW5nZS4KCj4gIEkgYWxzbyB0cmllZCB0byB1c2UgX0RFUCBtZXRob2Qg
dG8gZGVzY3JpYmUgdGhlIGRlcGVuZGVuY3kgb2YgUENJZQo+ICBub2RlcywgYnV0IGl0IGxvb2tz
IHRoYXQgaXQgZG9lcyBub3Qgd29yayBhcyBQQ0kgYW5kIE1TSSBhcmUgaGFuZGVkCj4gIGJ5IGFj
cGlfYnVzX3NjYW4gYW5kIHN0aWxsIGhhdmluZyBpc3N1ZSB3aGVuIHdlIHJlLXByb2JlIFBDSS4K
ClRoYXQncyBhIHRhZCB2YWd1ZSB0byBiZSBmcmFuaywgYW55d2F5IGl0IGlzIHByZXR0eSBjbGVh
ciB0byBtZSB0aGF0IHdlCmhhdmUgaGl0IGEgd2FsbC4gSW4gQUNQSSB0aGVyZSBpcyBubyB3YXkg
dG8gZGVzY3JpYmUgcHJvYmUgZGVwZW5kZW5jaWVzCmxpa2UgdGhlIG9uZSB5b3UgaGF2ZSwgaXQg
aXMgYXMgc2ltcGxlIGFzIHRoYXQsIGFuZCB0aGlzIE1TSSBpc3N1ZSB5b3UKYXJlIGZhY2luZyBp
cyBqdXN0IGFuIGV4YW1wbGUsIHRoZXJlIGFyZSBtb3JlIGVnOgoKaHR0cHM6Ly93d3cuc3Bpbmlj
cy5uZXQvbGlzdHMvYXJtLWtlcm5lbC9tc2c1ODU4MjUuaHRtbAoKQXQgdGhlIGVuZCBvZiB0aGUg
ZGF5IHRoZSBjaG9pY2UgaXMgc2ltcGxlIGVpdGhlciB3ZSBhY2NlcHQgKGFuZCB3ZQptYWludGFp
biBiZWNhdXNlIHRoYXQncyB0aGUgcHJvYmxlbSkgdGhlc2UgaGFja3MgLSBhbmQgSSBhbSBub3Qg
d2lsbGluZwp0byBkbyB0aGF0IC0gb3Igd2UgZmluZCBhIHdheSB0byBzb2x2ZSB0aGlzIGZyb20g
YSBnZW5lcmFsIHBlcnNwZWN0aXZlIG5vdAphcyBhIHBvaW50IGhhY2suCgpJIGNhbiBoYXZlIGEg
bG9vayBhdCBzb2x2aW5nIHRoZSB3aG9sZSBpc3N1ZSBidXQgaXQgd29uJ3QgaGFwcGVuCnRvbW9y
cm93LgoKVGhhbmtzLApMb3JlbnpvCgo+IFRoYW5rcywKPiBLaHVvbmcgRGluaAo+IAo+ID4+IEZp
bmFsbHksIHBsZWFzZSBleGVjdXRlIHRoaXMgY29tbWFuZCBvbiB0aGUgdm1saW51eCB0aGF0ICJ3
b3JrcyIKPiA+PiBmb3IgeW91Ogo+ID4+Cj4gPj4gbm0gdm1saW51eCB8IGdyZXAgLUUgJ19faW5p
dGNhbGxfKHhnZW5lX3BjaWVfbXNpX2luaXR8YWNwaV9pbml0KScKPiA+Cj4gPiAkIG5tIHZtbGlu
dXggfCBncmVwIC1FICdfX2luaXRjYWxsXyh4Z2VuZV9wY2llX21zaV9pbml0fGFjcGlfaW5pdCkn
Cj4gPiBmZmZmMDAwMDA4ZGFiMmQ4IHQgX19pbml0Y2FsbF9hY3BpX2luaXQ0Cj4gPiBmZmZmMDAw
MDA4ZGFiMmM4IHQgX19pbml0Y2FsbF94Z2VuZV9wY2llX21zaV9pbml0NAo+ID4KPiA+IEJlc3Qg
cmVnYXJkcywKPiA+IEtodW9uZyBEaW5oCj4gPgo+ID4+IEV2ZW4gYnkgb3JkZXJpbmcgZGV2aWNl
cyBpbiB0aGUgQUNQSSB0YWJsZXMgKHRoYXQgSSBhYmhvcikgSSBzdGlsbCBkbwo+ID4+IG5vdCB1
bmRlcnN0YW5kIGhvdyB0aGlzIHdvcmtzIChJIG1lYW4gd2l0aG91dCByZWx5aW5nIG9uIGtlcm5l
bCBsaW5rCj4gPj4gb3JkZXIgdG8gZW5zdXJlIHRoYXQgdGhlIE1TSSBkcml2ZXIgaXMgcHJvYmVk
IGJlZm9yZSBQQ0kgZGV2aWNlcyBhcmUKPiA+PiBlbnVtZXJhdGVkIGluIGFjcGlfaW5pdCgpKS4K
PiA+Pgo+ID4+IFRoYW5rcywKPiA+PiBMb3JlbnpvCj4gPj4KPiA+Pj4gQmVzdCByZWdhcmRzLAo+
ID4+PiBLaHVvbmcKPiA+Pj4KPiA+Pj4gT24gRnJpLCBBcHIgMjgsIDIwMTcgYXQgMjoyNyBBTSwg
TWFyYyBaeW5naWVyIDxtYXJjLnp5bmdpZXJAYXJtLmNvbT4gd3JvdGU6Cj4gPj4+ID4gT24gMjgv
MDQvMTcgMDE6NTQsIEtodW9uZyBEaW5oIHdyb3RlOgo+ID4+PiA+PiBGcm9tOiBLaHVvbmcgRGlu
aCA8a2RpbmhAYXBtLmNvbT4KPiA+Pj4gPj4KPiA+Pj4gPj4gVGhpcyBwYXRjaCBtYWtlcyBwY2kt
eGdlbmUtbXNpIGRyaXZlciBBQ1BJLWF3YXJlIGFuZCBwcm92aWRlcwo+ID4+PiA+PiBNU0kgY2Fw
YWJpbGl0eSBmb3IgWC1HZW5lIHYxIFBDSWUgY29udHJvbGxlcnMgaW4gQUNQSSBib290IG1vZGUu
Cj4gPj4+ID4+Cj4gPj4+ID4+IFNpZ25lZC1vZmYtYnk6IEtodW9uZyBEaW5oIDxrZGluaEBhcG0u
Y29tPgo+ID4+PiA+PiBTaWduZWQtb2ZmLWJ5OiBEdWMgRGFuZyA8ZGhkYW5nQGFwbS5jb20+Cj4g
Pj4+ID4+IEFja2VkLWJ5OiBNYXJjIFp5bmdpZXIgPG1hcmMuenluZ2llckBhcm0uY29tPgo+ID4+
PiA+PiAtLS0KPiA+Pj4gPj4gdjI6Cj4gPj4+ID4+ICAtIFZlcmlmeSB3aXRoIEJJT1MgdmVyc2lv
biAzLjA2LjI1IGFuZCAzLjA3LjA5Cj4gPj4+ID4+IHYxOgo+ID4+PiA+PiAgLSBJbml0aWFsIHZl
cnNpb24KPiA+Pj4gPj4gLS0tCj4gPj4+ID4+ICBkcml2ZXJzL3BjaS9ob3N0L3BjaS14Z2VuZS1t
c2kuYyB8ICAgMzUgKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKystLS0KPiA+Pj4gPj4g
IDEgZmlsZXMgY2hhbmdlZCwgMzIgaW5zZXJ0aW9ucygrKSwgMyBkZWxldGlvbnMoLSkKPiA+Pj4g
Pj4KPiA+Pj4gPj4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvcGNpL2hvc3QvcGNpLXhnZW5lLW1zaS5j
IGIvZHJpdmVycy9wY2kvaG9zdC9wY2kteGdlbmUtbXNpLmMKPiA+Pj4gPj4gaW5kZXggZjFiNjMz
Yi4uMDBhYWEzZCAxMDA2NDQKPiA+Pj4gPj4gLS0tIGEvZHJpdmVycy9wY2kvaG9zdC9wY2kteGdl
bmUtbXNpLmMKPiA+Pj4gPj4gKysrIGIvZHJpdmVycy9wY2kvaG9zdC9wY2kteGdlbmUtbXNpLmMK
PiA+Pj4gPj4gQEAgLTI0LDYgKzI0LDcgQEAKPiA+Pj4gPj4gICNpbmNsdWRlIDxsaW51eC9wY2ku
aD4KPiA+Pj4gPj4gICNpbmNsdWRlIDxsaW51eC9wbGF0Zm9ybV9kZXZpY2UuaD4KPiA+Pj4gPj4g
ICNpbmNsdWRlIDxsaW51eC9vZl9wY2kuaD4KPiA+Pj4gPj4gKyNpbmNsdWRlIDxsaW51eC9hY3Bp
Lmg+Cj4gPj4+ID4+Cj4gPj4+ID4+ICAjZGVmaW5lIE1TSV9JUjAgICAgICAgICAgICAgICAgICAg
ICAgMHgwMDAwMDAKPiA+Pj4gPj4gICNkZWZpbmUgTVNJX0lOVDAgICAgICAgICAgICAgMHg4MDAw
MDAKPiA+Pj4gPj4gQEAgLTM5LDcgKzQwLDcgQEAgc3RydWN0IHhnZW5lX21zaV9ncm91cCB7Cj4g
Pj4+ID4+ICB9Owo+ID4+PiA+Pgo+ID4+PiA+PiAgc3RydWN0IHhnZW5lX21zaSB7Cj4gPj4+ID4+
IC0gICAgIHN0cnVjdCBkZXZpY2Vfbm9kZSAgICAgICpub2RlOwo+ID4+PiA+PiArICAgICBzdHJ1
Y3QgZndub2RlX2hhbmRsZSAgICAqZndub2RlOwo+ID4+PiA+PiAgICAgICBzdHJ1Y3QgaXJxX2Rv
bWFpbiAgICAgICAqaW5uZXJfZG9tYWluOwo+ID4+PiA+PiAgICAgICBzdHJ1Y3QgaXJxX2RvbWFp
biAgICAgICAqbXNpX2RvbWFpbjsKPiA+Pj4gPj4gICAgICAgdTY0ICAgICAgICAgICAgICAgICAg
ICAgbXNpX2FkZHI7Cj4gPj4+ID4+IEBAIC0yNDksNiArMjUwLDEzIEBAIHN0YXRpYyB2b2lkIHhn
ZW5lX2lycV9kb21haW5fZnJlZShzdHJ1Y3QgaXJxX2RvbWFpbiAqZG9tYWluLAo+ID4+PiA+PiAg
ICAgICAuZnJlZSAgID0geGdlbmVfaXJxX2RvbWFpbl9mcmVlLAo+ID4+PiA+PiAgfTsKPiA+Pj4g
Pj4KPiA+Pj4gPj4gKyNpZmRlZiBDT05GSUdfQUNQSQo+ID4+PiA+PiArc3RhdGljIHN0cnVjdCBm
d25vZGVfaGFuZGxlICp4Z2VuZV9tc2lfZ2V0X2Z3bm9kZShzdHJ1Y3QgZGV2aWNlICpkZXYpCj4g
Pj4+ID4+ICt7Cj4gPj4+ID4+ICsgICAgIHJldHVybiB4Z2VuZV9tc2lfY3RybC5md25vZGU7Cj4g
Pj4+ID4+ICt9Cj4gPj4+ID4+ICsjZW5kaWYKPiA+Pj4gPj4gKwo+ID4+PiA+PiAgc3RhdGljIGlu
dCB4Z2VuZV9hbGxvY2F0ZV9kb21haW5zKHN0cnVjdCB4Z2VuZV9tc2kgKm1zaSkKPiA+Pj4gPj4g
IHsKPiA+Pj4gPj4gICAgICAgbXNpLT5pbm5lcl9kb21haW4gPSBpcnFfZG9tYWluX2FkZF9saW5l
YXIoTlVMTCwgTlJfTVNJX1ZFQywKPiA+Pj4gPj4gQEAgLTI1Niw3ICsyNjQsNyBAQCBzdGF0aWMg
aW50IHhnZW5lX2FsbG9jYXRlX2RvbWFpbnMoc3RydWN0IHhnZW5lX21zaSAqbXNpKQo+ID4+PiA+
PiAgICAgICBpZiAoIW1zaS0+aW5uZXJfZG9tYWluKQo+ID4+PiA+PiAgICAgICAgICAgICAgIHJl
dHVybiAtRU5PTUVNOwo+ID4+PiA+Pgo+ID4+PiA+PiAtICAgICBtc2ktPm1zaV9kb21haW4gPSBw
Y2lfbXNpX2NyZWF0ZV9pcnFfZG9tYWluKG9mX25vZGVfdG9fZndub2RlKG1zaS0+bm9kZSksCj4g
Pj4+ID4+ICsgICAgIG1zaS0+bXNpX2RvbWFpbiA9IHBjaV9tc2lfY3JlYXRlX2lycV9kb21haW4o
bXNpLT5md25vZGUsCj4gPj4+ID4+ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICAgJnhnZW5lX21zaV9kb21haW5faW5mbywKPiA+Pj4gPj4gICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBtc2ktPmlubmVyX2RvbWFp
bik7Cj4gPj4+ID4+Cj4gPj4+ID4+IEBAIC0yNjUsNiArMjczLDkgQEAgc3RhdGljIGludCB4Z2Vu
ZV9hbGxvY2F0ZV9kb21haW5zKHN0cnVjdCB4Z2VuZV9tc2kgKm1zaSkKPiA+Pj4gPj4gICAgICAg
ICAgICAgICByZXR1cm4gLUVOT01FTTsKPiA+Pj4gPj4gICAgICAgfQo+ID4+PiA+Pgo+ID4+PiA+
PiArI2lmZGVmIENPTkZJR19BQ1BJCj4gPj4+ID4+ICsgICAgIHBjaV9tc2lfcmVnaXN0ZXJfZndu
b2RlX3Byb3ZpZGVyKCZ4Z2VuZV9tc2lfZ2V0X2Z3bm9kZSk7Cj4gPj4+ID4+ICsjZW5kaWYKPiA+
Pj4gPj4gICAgICAgcmV0dXJuIDA7Cj4gPj4+ID4+ICB9Cj4gPj4+ID4+Cj4gPj4+ID4+IEBAIC00
NDksNiArNDYwLDEzIEBAIHN0YXRpYyBpbnQgeGdlbmVfbXNpX2h3aXJxX2ZyZWUodW5zaWduZWQg
aW50IGNwdSkKPiA+Pj4gPj4gICAgICAge30sCj4gPj4+ID4+ICB9Owo+ID4+PiA+Pgo+ID4+PiA+
PiArI2lmZGVmIENPTkZJR19BQ1BJCj4gPj4+ID4+ICtzdGF0aWMgY29uc3Qgc3RydWN0IGFjcGlf
ZGV2aWNlX2lkIHhnZW5lX21zaV9hY3BpX2lkc1tdID0gewo+ID4+PiA+PiArICAgICB7IkFQTUMw
RDBFIiwgMH0sCj4gPj4+ID4+ICsgICAgIHsgfSwKPiA+Pj4gPj4gK307Cj4gPj4+ID4+ICsjZW5k
aWYKPiA+Pj4gPj4gKwo+ID4+PiA+PiAgc3RhdGljIGludCB4Z2VuZV9tc2lfcHJvYmUoc3RydWN0
IHBsYXRmb3JtX2RldmljZSAqcGRldikKPiA+Pj4gPj4gIHsKPiA+Pj4gPj4gICAgICAgc3RydWN0
IHJlc291cmNlICpyZXM7Cj4gPj4+ID4+IEBAIC00NjksNyArNDg3LDE3IEBAIHN0YXRpYyBpbnQg
eGdlbmVfbXNpX3Byb2JlKHN0cnVjdCBwbGF0Zm9ybV9kZXZpY2UgKnBkZXYpCj4gPj4+ID4+ICAg
ICAgICAgICAgICAgZ290byBlcnJvcjsKPiA+Pj4gPj4gICAgICAgfQo+ID4+PiA+PiAgICAgICB4
Z2VuZV9tc2ktPm1zaV9hZGRyID0gcmVzLT5zdGFydDsKPiA+Pj4gPj4gLSAgICAgeGdlbmVfbXNp
LT5ub2RlID0gcGRldi0+ZGV2Lm9mX25vZGU7Cj4gPj4+ID4+ICsKPiA+Pj4gPj4gKyAgICAgeGdl
bmVfbXNpLT5md25vZGUgPSBvZl9ub2RlX3RvX2Z3bm9kZShwZGV2LT5kZXYub2Zfbm9kZSk7Cj4g
Pj4+ID4+ICsgICAgIGlmICgheGdlbmVfbXNpLT5md25vZGUpIHsKPiA+Pj4gPj4gKyAgICAgICAg
ICAgICB4Z2VuZV9tc2ktPmZ3bm9kZSA9IGlycV9kb21haW5fYWxsb2NfZndub2RlKE5VTEwpOwo+
ID4+PiA+Cj4gPj4+ID4gUGxlYXNlIHByb3ZpZGUgc29tZXRoaW5nIG90aGVyIHRoYW4gTlVMTCwg
c3VjaCBhcyB0aGUgYmFzZSBhZGRyZXNzIGlmCj4gPj4+ID4gdGhlIGRldmljZS4gVGhhdCdzIHF1
aXRlIHVzZWZ1bCBmb3IgZGVidWdnaW5nLgo+ID4+PiA+Cj4gPj4+ID4+ICsgICAgICAgICAgICAg
aWYgKCF4Z2VuZV9tc2ktPmZ3bm9kZSkgewo+ID4+PiA+PiArICAgICAgICAgICAgICAgICAgICAg
ZGV2X2VycigmcGRldi0+ZGV2LCAiRmFpbGVkIHRvIGNyZWF0ZSBmd25vZGVcbiIpOwo+ID4+PiA+
PiArICAgICAgICAgICAgICAgICAgICAgcmMgPSBFTk9NRU07Cj4gPj4+ID4+ICsgICAgICAgICAg
ICAgICAgICAgICBnb3RvIGVycm9yOwo+ID4+PiA+PiArICAgICAgICAgICAgIH0KPiA+Pj4gPj4g
KyAgICAgfQo+ID4+PiA+PiArCj4gPj4+ID4+ICAgICAgIHhnZW5lX21zaS0+bnVtX2NwdXMgPSBu
dW1fcG9zc2libGVfY3B1cygpOwo+ID4+PiA+Pgo+ID4+PiA+PiAgICAgICByYyA9IHhnZW5lX21z
aV9pbml0X2FsbG9jYXRvcih4Z2VuZV9tc2kpOwo+ID4+PiA+PiBAQCAtNTQwLDYgKzU2OCw3IEBA
IHN0YXRpYyBpbnQgeGdlbmVfbXNpX3Byb2JlKHN0cnVjdCBwbGF0Zm9ybV9kZXZpY2UgKnBkZXYp
Cj4gPj4+ID4+ICAgICAgIC5kcml2ZXIgPSB7Cj4gPj4+ID4+ICAgICAgICAgICAgICAgLm5hbWUg
PSAieGdlbmUtbXNpIiwKPiA+Pj4gPj4gICAgICAgICAgICAgICAub2ZfbWF0Y2hfdGFibGUgPSB4
Z2VuZV9tc2lfbWF0Y2hfdGFibGUsCj4gPj4+ID4+ICsgICAgICAgICAgICAgLmFjcGlfbWF0Y2hf
dGFibGUgPSBBQ1BJX1BUUih4Z2VuZV9tc2lfYWNwaV9pZHMpLAo+ID4+PiA+PiAgICAgICB9LAo+
ID4+PiA+PiAgICAgICAucHJvYmUgPSB4Z2VuZV9tc2lfcHJvYmUsCj4gPj4+ID4+ICAgICAgIC5y
ZW1vdmUgPSB4Z2VuZV9tc2lfcmVtb3ZlLAo+ID4+PiA+Pgo+ID4+PiA+Cj4gPj4+ID4gVGhlIGNv
ZGUgaXMgdHJpdmlhbCwgYnV0IHJlbGllcyBvbiB0aGUgTVNJIGNvbnRyb2xsZXIgdG8gcHJvYmUg
YmVmb3JlCj4gPj4+ID4gdGhlIFBDSSBidXMuIFdoYXQgZW5mb3JjZXMgdGhpcz8gSG93IGlzIGl0
IG1ha2luZyBzdXJlIHRoYXQgdGhpcyBpcyBub3QKPiA+Pj4gPiBnb2luZyB0byBicmVhayBpbiB0
aGUgbmV4dCBrZXJuZWwgcmVsZWFzZT8gQXMgZmFyIGFzIEkgY2FuIHRlbGwsIHRoZXJlCj4gPj4+
ID4gaXMgbm8gZXhwbGljaXQgZGVwZW5kZW5jeSBiZXR3ZWVuIHRoZSB0d28sIG1ha2luZyBpdCB0
aGUgd2hvbGUgdGhpbmcKPiA+Pj4gPiBleHRyZW1lbHkgZnJhZ2lsZS4KPiA+Pj4gPgo+ID4+PiA+
IFRoYW5rcywKPiA+Pj4gPgo+ID4+PiA+ICAgICAgICAgTS4KPiA+Pj4gPiAtLQo+ID4+PiA+IEph
enogaXMgbm90IGRlYWQuIEl0IGp1c3Qgc21lbGxzIGZ1bm55Li4uCj4gPj4+Cj4gPj4+IC0tCgpf
X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpsaW51eC1hcm0t
a2VybmVsIG1haWxpbmcgbGlzdApsaW51eC1hcm0ta2VybmVsQGxpc3RzLmluZnJhZGVhZC5vcmcK
aHR0cDovL2xpc3RzLmluZnJhZGVhZC5vcmcvbWFpbG1hbi9saXN0aW5mby9saW51eC1hcm0ta2Vy
bmVsCg==
WARNING: multiple messages have this Message-ID (diff)
From: lorenzo.pieralisi@arm.com (Lorenzo Pieralisi)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2 pci] PCI/MSI: pci-xgene-msi: Enable MSI support in ACPI boot for X-Gene v1
Date: Thu, 8 Jun 2017 11:39:56 +0100 [thread overview]
Message-ID: <20170608103956.GB8644@red-moon> (raw)
In-Reply-To: <CAAsHzqvnGWkG_KN6m_+HzGFaNOHU-m2FocTxy3pyM7Hhd9rz7w@mail.gmail.com>
On Tue, Jun 06, 2017 at 09:44:15AM -0700, Khuong Dinh wrote:
> Hi Lorenzo,
>
> On Tue, May 9, 2017 at 3:55 PM, Khuong Dinh <kdinh@apm.com> wrote:
> > Hi Lorenzo,
> >
> > On Fri, May 5, 2017 at 9:51 AM, Lorenzo Pieralisi
> > <lorenzo.pieralisi@arm.com> wrote:
> >> On Thu, May 04, 2017 at 05:36:06PM -0700, Khuong Dinh wrote:
> >>> Hi Marc,
> >>> There's no explicit dependency between pcie driver and msi
> >>> controller. The current solution that we did is relying on the
> >>> node ordering in BIOS. ACPI 5.0 introduced _DEP method to assign a
> >>> higher priority in start ordering. This method could be applied in
> >>> case of msi and pcie are the same level of subsys_init (in ACPI
> >>> boot). However, PCIe driver has not supported for this dependency
> >>> check yet. How do you think about this solution.
> >>
> >> First off, you can't post emails with confidentiality notices on
> >> mailing lists so remove it from now onwards.
> >
> > Fixed
> >
> >> Secondly, I commented on this before, so you know what my opinion is.
> >
> > I got your opinion. I'll implement as your suggestion.
> >
>
> Regarding to your previous suggestion to add a hook walking the ACPI
> namespace before acpi_bus_scan() in acpi_scan_init() to make MSI
> controllers must be probed before PCI. I have a concern that the
> MSI namespace ? _SB.MSIX? is not a unique name and how can we walk
> the ACPI namespace and use this name to make MSI probed before PCI.
> May you have little bit more information for this or do you have
> another suggestion for this?
>
> There?s another solution which makes this simpler and I?d like to
> ask your opinion about this.
> The solution is to make an hierarchy between MSI and PCI nodes as below:
> Device(\_SB.MSIX) {
> Device(\_SB.PCI0)
> Device(\_SB.PCI1)
> ??
> }
> In other word, MSIX node is a parent node of PCI nodes in ACPI
> table. In this sense, there?s an explicit dependency between MSI
> and PCI, MSI controller must be probed before PCI and it will
> guarantee not breaking next kernel releases. How do you think about
> this solution.
I think that's a plaster as ineffective as reordering nodes, in short
it is not a solution and you still rely on kernel link ordering, you
can fiddle with ACPI tables as much as you want but that does not change.
> I also tried to use _DEP method to describe the dependency of PCIe
> nodes, but it looks that it does not work as PCI and MSI are handed
> by acpi_bus_scan and still having issue when we re-probe PCI.
That's a tad vague to be frank, anyway it is pretty clear to me that we
have hit a wall. In ACPI there is no way to describe probe dependencies
like the one you have, it is as simple as that, and this MSI issue you
are facing is just an example, there are more eg:
https://www.spinics.net/lists/arm-kernel/msg585825.html
At the end of the day the choice is simple either we accept (and we
maintain because that's the problem) these hacks - and I am not willing
to do that - or we find a way to solve this from a general perspective not
as a point hack.
I can have a look at solving the whole issue but it won't happen
tomorrow.
Thanks,
Lorenzo
> Thanks,
> Khuong Dinh
>
> >> Finally, please execute this command on the vmlinux that "works"
> >> for you:
> >>
> >> nm vmlinux | grep -E '__initcall_(xgene_pcie_msi_init|acpi_init)'
> >
> > $ nm vmlinux | grep -E '__initcall_(xgene_pcie_msi_init|acpi_init)'
> > ffff000008dab2d8 t __initcall_acpi_init4
> > ffff000008dab2c8 t __initcall_xgene_pcie_msi_init4
> >
> > Best regards,
> > Khuong Dinh
> >
> >> Even by ordering devices in the ACPI tables (that I abhor) I still do
> >> not understand how this works (I mean without relying on kernel link
> >> order to ensure that the MSI driver is probed before PCI devices are
> >> enumerated in acpi_init()).
> >>
> >> Thanks,
> >> Lorenzo
> >>
> >>> Best regards,
> >>> Khuong
> >>>
> >>> On Fri, Apr 28, 2017 at 2:27 AM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> >>> > On 28/04/17 01:54, Khuong Dinh wrote:
> >>> >> From: Khuong Dinh <kdinh@apm.com>
> >>> >>
> >>> >> This patch makes pci-xgene-msi driver ACPI-aware and provides
> >>> >> MSI capability for X-Gene v1 PCIe controllers in ACPI boot mode.
> >>> >>
> >>> >> Signed-off-by: Khuong Dinh <kdinh@apm.com>
> >>> >> Signed-off-by: Duc Dang <dhdang@apm.com>
> >>> >> Acked-by: Marc Zyngier <marc.zyngier@arm.com>
> >>> >> ---
> >>> >> v2:
> >>> >> - Verify with BIOS version 3.06.25 and 3.07.09
> >>> >> v1:
> >>> >> - Initial version
> >>> >> ---
> >>> >> drivers/pci/host/pci-xgene-msi.c | 35 ++++++++++++++++++++++++++++++++---
> >>> >> 1 files changed, 32 insertions(+), 3 deletions(-)
> >>> >>
> >>> >> diff --git a/drivers/pci/host/pci-xgene-msi.c b/drivers/pci/host/pci-xgene-msi.c
> >>> >> index f1b633b..00aaa3d 100644
> >>> >> --- a/drivers/pci/host/pci-xgene-msi.c
> >>> >> +++ b/drivers/pci/host/pci-xgene-msi.c
> >>> >> @@ -24,6 +24,7 @@
> >>> >> #include <linux/pci.h>
> >>> >> #include <linux/platform_device.h>
> >>> >> #include <linux/of_pci.h>
> >>> >> +#include <linux/acpi.h>
> >>> >>
> >>> >> #define MSI_IR0 0x000000
> >>> >> #define MSI_INT0 0x800000
> >>> >> @@ -39,7 +40,7 @@ struct xgene_msi_group {
> >>> >> };
> >>> >>
> >>> >> struct xgene_msi {
> >>> >> - struct device_node *node;
> >>> >> + struct fwnode_handle *fwnode;
> >>> >> struct irq_domain *inner_domain;
> >>> >> struct irq_domain *msi_domain;
> >>> >> u64 msi_addr;
> >>> >> @@ -249,6 +250,13 @@ static void xgene_irq_domain_free(struct irq_domain *domain,
> >>> >> .free = xgene_irq_domain_free,
> >>> >> };
> >>> >>
> >>> >> +#ifdef CONFIG_ACPI
> >>> >> +static struct fwnode_handle *xgene_msi_get_fwnode(struct device *dev)
> >>> >> +{
> >>> >> + return xgene_msi_ctrl.fwnode;
> >>> >> +}
> >>> >> +#endif
> >>> >> +
> >>> >> static int xgene_allocate_domains(struct xgene_msi *msi)
> >>> >> {
> >>> >> msi->inner_domain = irq_domain_add_linear(NULL, NR_MSI_VEC,
> >>> >> @@ -256,7 +264,7 @@ static int xgene_allocate_domains(struct xgene_msi *msi)
> >>> >> if (!msi->inner_domain)
> >>> >> return -ENOMEM;
> >>> >>
> >>> >> - msi->msi_domain = pci_msi_create_irq_domain(of_node_to_fwnode(msi->node),
> >>> >> + msi->msi_domain = pci_msi_create_irq_domain(msi->fwnode,
> >>> >> &xgene_msi_domain_info,
> >>> >> msi->inner_domain);
> >>> >>
> >>> >> @@ -265,6 +273,9 @@ static int xgene_allocate_domains(struct xgene_msi *msi)
> >>> >> return -ENOMEM;
> >>> >> }
> >>> >>
> >>> >> +#ifdef CONFIG_ACPI
> >>> >> + pci_msi_register_fwnode_provider(&xgene_msi_get_fwnode);
> >>> >> +#endif
> >>> >> return 0;
> >>> >> }
> >>> >>
> >>> >> @@ -449,6 +460,13 @@ static int xgene_msi_hwirq_free(unsigned int cpu)
> >>> >> {},
> >>> >> };
> >>> >>
> >>> >> +#ifdef CONFIG_ACPI
> >>> >> +static const struct acpi_device_id xgene_msi_acpi_ids[] = {
> >>> >> + {"APMC0D0E", 0},
> >>> >> + { },
> >>> >> +};
> >>> >> +#endif
> >>> >> +
> >>> >> static int xgene_msi_probe(struct platform_device *pdev)
> >>> >> {
> >>> >> struct resource *res;
> >>> >> @@ -469,7 +487,17 @@ static int xgene_msi_probe(struct platform_device *pdev)
> >>> >> goto error;
> >>> >> }
> >>> >> xgene_msi->msi_addr = res->start;
> >>> >> - xgene_msi->node = pdev->dev.of_node;
> >>> >> +
> >>> >> + xgene_msi->fwnode = of_node_to_fwnode(pdev->dev.of_node);
> >>> >> + if (!xgene_msi->fwnode) {
> >>> >> + xgene_msi->fwnode = irq_domain_alloc_fwnode(NULL);
> >>> >
> >>> > Please provide something other than NULL, such as the base address if
> >>> > the device. That's quite useful for debugging.
> >>> >
> >>> >> + if (!xgene_msi->fwnode) {
> >>> >> + dev_err(&pdev->dev, "Failed to create fwnode\n");
> >>> >> + rc = ENOMEM;
> >>> >> + goto error;
> >>> >> + }
> >>> >> + }
> >>> >> +
> >>> >> xgene_msi->num_cpus = num_possible_cpus();
> >>> >>
> >>> >> rc = xgene_msi_init_allocator(xgene_msi);
> >>> >> @@ -540,6 +568,7 @@ static int xgene_msi_probe(struct platform_device *pdev)
> >>> >> .driver = {
> >>> >> .name = "xgene-msi",
> >>> >> .of_match_table = xgene_msi_match_table,
> >>> >> + .acpi_match_table = ACPI_PTR(xgene_msi_acpi_ids),
> >>> >> },
> >>> >> .probe = xgene_msi_probe,
> >>> >> .remove = xgene_msi_remove,
> >>> >>
> >>> >
> >>> > The code is trivial, but relies on the MSI controller to probe before
> >>> > the PCI bus. What enforces this? How is it making sure that this is not
> >>> > going to break in the next kernel release? As far as I can tell, there
> >>> > is no explicit dependency between the two, making it the whole thing
> >>> > extremely fragile.
> >>> >
> >>> > Thanks,
> >>> >
> >>> > M.
> >>> > --
> >>> > Jazz is not dead. It just smells funny...
> >>>
> >>> --
WARNING: multiple messages have this Message-ID (diff)
From: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
To: Khuong Dinh <kdinh@apm.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>,
msalter@redhat.com, Bjorn Helgaas <bhelgaas@google.com>,
linux-pci@vger.kernel.org, jcm@redhat.com, patches@apm.com,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, rjw@rjwysocki.net,
Duc Dang <dhdang@apm.com>
Subject: Re: [PATCH v2 pci] PCI/MSI: pci-xgene-msi: Enable MSI support in ACPI boot for X-Gene v1
Date: Thu, 8 Jun 2017 11:39:56 +0100 [thread overview]
Message-ID: <20170608103956.GB8644@red-moon> (raw)
In-Reply-To: <CAAsHzqvnGWkG_KN6m_+HzGFaNOHU-m2FocTxy3pyM7Hhd9rz7w@mail.gmail.com>
On Tue, Jun 06, 2017 at 09:44:15AM -0700, Khuong Dinh wrote:
> Hi Lorenzo,
>
> On Tue, May 9, 2017 at 3:55 PM, Khuong Dinh <kdinh@apm.com> wrote:
> > Hi Lorenzo,
> >
> > On Fri, May 5, 2017 at 9:51 AM, Lorenzo Pieralisi
> > <lorenzo.pieralisi@arm.com> wrote:
> >> On Thu, May 04, 2017 at 05:36:06PM -0700, Khuong Dinh wrote:
> >>> Hi Marc,
> >>> There's no explicit dependency between pcie driver and msi
> >>> controller. The current solution that we did is relying on the
> >>> node ordering in BIOS. ACPI 5.0 introduced _DEP method to assign a
> >>> higher priority in start ordering. This method could be applied in
> >>> case of msi and pcie are the same level of subsys_init (in ACPI
> >>> boot). However, PCIe driver has not supported for this dependency
> >>> check yet. How do you think about this solution.
> >>
> >> First off, you can't post emails with confidentiality notices on
> >> mailing lists so remove it from now onwards.
> >
> > Fixed
> >
> >> Secondly, I commented on this before, so you know what my opinion is.
> >
> > I got your opinion. I'll implement as your suggestion.
> >
>
> Regarding to your previous suggestion to add a hook walking the ACPI
> namespace before acpi_bus_scan() in acpi_scan_init() to make MSI
> controllers must be probed before PCI. I have a concern that the
> MSI namespace “ _SB.MSIX” is not a unique name and how can we walk
> the ACPI namespace and use this name to make MSI probed before PCI.
> May you have little bit more information for this or do you have
> another suggestion for this?
>
> There’s another solution which makes this simpler and I’d like to
> ask your opinion about this.
> The solution is to make an hierarchy between MSI and PCI nodes as below:
> Device(\_SB.MSIX) {
> Device(\_SB.PCI0)
> Device(\_SB.PCI1)
> ……
> }
> In other word, MSIX node is a parent node of PCI nodes in ACPI
> table. In this sense, there’s an explicit dependency between MSI
> and PCI, MSI controller must be probed before PCI and it will
> guarantee not breaking next kernel releases. How do you think about
> this solution.
I think that's a plaster as ineffective as reordering nodes, in short
it is not a solution and you still rely on kernel link ordering, you
can fiddle with ACPI tables as much as you want but that does not change.
> I also tried to use _DEP method to describe the dependency of PCIe
> nodes, but it looks that it does not work as PCI and MSI are handed
> by acpi_bus_scan and still having issue when we re-probe PCI.
That's a tad vague to be frank, anyway it is pretty clear to me that we
have hit a wall. In ACPI there is no way to describe probe dependencies
like the one you have, it is as simple as that, and this MSI issue you
are facing is just an example, there are more eg:
https://www.spinics.net/lists/arm-kernel/msg585825.html
At the end of the day the choice is simple either we accept (and we
maintain because that's the problem) these hacks - and I am not willing
to do that - or we find a way to solve this from a general perspective not
as a point hack.
I can have a look at solving the whole issue but it won't happen
tomorrow.
Thanks,
Lorenzo
> Thanks,
> Khuong Dinh
>
> >> Finally, please execute this command on the vmlinux that "works"
> >> for you:
> >>
> >> nm vmlinux | grep -E '__initcall_(xgene_pcie_msi_init|acpi_init)'
> >
> > $ nm vmlinux | grep -E '__initcall_(xgene_pcie_msi_init|acpi_init)'
> > ffff000008dab2d8 t __initcall_acpi_init4
> > ffff000008dab2c8 t __initcall_xgene_pcie_msi_init4
> >
> > Best regards,
> > Khuong Dinh
> >
> >> Even by ordering devices in the ACPI tables (that I abhor) I still do
> >> not understand how this works (I mean without relying on kernel link
> >> order to ensure that the MSI driver is probed before PCI devices are
> >> enumerated in acpi_init()).
> >>
> >> Thanks,
> >> Lorenzo
> >>
> >>> Best regards,
> >>> Khuong
> >>>
> >>> On Fri, Apr 28, 2017 at 2:27 AM, Marc Zyngier <marc.zyngier@arm.com> wrote:
> >>> > On 28/04/17 01:54, Khuong Dinh wrote:
> >>> >> From: Khuong Dinh <kdinh@apm.com>
> >>> >>
> >>> >> This patch makes pci-xgene-msi driver ACPI-aware and provides
> >>> >> MSI capability for X-Gene v1 PCIe controllers in ACPI boot mode.
> >>> >>
> >>> >> Signed-off-by: Khuong Dinh <kdinh@apm.com>
> >>> >> Signed-off-by: Duc Dang <dhdang@apm.com>
> >>> >> Acked-by: Marc Zyngier <marc.zyngier@arm.com>
> >>> >> ---
> >>> >> v2:
> >>> >> - Verify with BIOS version 3.06.25 and 3.07.09
> >>> >> v1:
> >>> >> - Initial version
> >>> >> ---
> >>> >> drivers/pci/host/pci-xgene-msi.c | 35 ++++++++++++++++++++++++++++++++---
> >>> >> 1 files changed, 32 insertions(+), 3 deletions(-)
> >>> >>
> >>> >> diff --git a/drivers/pci/host/pci-xgene-msi.c b/drivers/pci/host/pci-xgene-msi.c
> >>> >> index f1b633b..00aaa3d 100644
> >>> >> --- a/drivers/pci/host/pci-xgene-msi.c
> >>> >> +++ b/drivers/pci/host/pci-xgene-msi.c
> >>> >> @@ -24,6 +24,7 @@
> >>> >> #include <linux/pci.h>
> >>> >> #include <linux/platform_device.h>
> >>> >> #include <linux/of_pci.h>
> >>> >> +#include <linux/acpi.h>
> >>> >>
> >>> >> #define MSI_IR0 0x000000
> >>> >> #define MSI_INT0 0x800000
> >>> >> @@ -39,7 +40,7 @@ struct xgene_msi_group {
> >>> >> };
> >>> >>
> >>> >> struct xgene_msi {
> >>> >> - struct device_node *node;
> >>> >> + struct fwnode_handle *fwnode;
> >>> >> struct irq_domain *inner_domain;
> >>> >> struct irq_domain *msi_domain;
> >>> >> u64 msi_addr;
> >>> >> @@ -249,6 +250,13 @@ static void xgene_irq_domain_free(struct irq_domain *domain,
> >>> >> .free = xgene_irq_domain_free,
> >>> >> };
> >>> >>
> >>> >> +#ifdef CONFIG_ACPI
> >>> >> +static struct fwnode_handle *xgene_msi_get_fwnode(struct device *dev)
> >>> >> +{
> >>> >> + return xgene_msi_ctrl.fwnode;
> >>> >> +}
> >>> >> +#endif
> >>> >> +
> >>> >> static int xgene_allocate_domains(struct xgene_msi *msi)
> >>> >> {
> >>> >> msi->inner_domain = irq_domain_add_linear(NULL, NR_MSI_VEC,
> >>> >> @@ -256,7 +264,7 @@ static int xgene_allocate_domains(struct xgene_msi *msi)
> >>> >> if (!msi->inner_domain)
> >>> >> return -ENOMEM;
> >>> >>
> >>> >> - msi->msi_domain = pci_msi_create_irq_domain(of_node_to_fwnode(msi->node),
> >>> >> + msi->msi_domain = pci_msi_create_irq_domain(msi->fwnode,
> >>> >> &xgene_msi_domain_info,
> >>> >> msi->inner_domain);
> >>> >>
> >>> >> @@ -265,6 +273,9 @@ static int xgene_allocate_domains(struct xgene_msi *msi)
> >>> >> return -ENOMEM;
> >>> >> }
> >>> >>
> >>> >> +#ifdef CONFIG_ACPI
> >>> >> + pci_msi_register_fwnode_provider(&xgene_msi_get_fwnode);
> >>> >> +#endif
> >>> >> return 0;
> >>> >> }
> >>> >>
> >>> >> @@ -449,6 +460,13 @@ static int xgene_msi_hwirq_free(unsigned int cpu)
> >>> >> {},
> >>> >> };
> >>> >>
> >>> >> +#ifdef CONFIG_ACPI
> >>> >> +static const struct acpi_device_id xgene_msi_acpi_ids[] = {
> >>> >> + {"APMC0D0E", 0},
> >>> >> + { },
> >>> >> +};
> >>> >> +#endif
> >>> >> +
> >>> >> static int xgene_msi_probe(struct platform_device *pdev)
> >>> >> {
> >>> >> struct resource *res;
> >>> >> @@ -469,7 +487,17 @@ static int xgene_msi_probe(struct platform_device *pdev)
> >>> >> goto error;
> >>> >> }
> >>> >> xgene_msi->msi_addr = res->start;
> >>> >> - xgene_msi->node = pdev->dev.of_node;
> >>> >> +
> >>> >> + xgene_msi->fwnode = of_node_to_fwnode(pdev->dev.of_node);
> >>> >> + if (!xgene_msi->fwnode) {
> >>> >> + xgene_msi->fwnode = irq_domain_alloc_fwnode(NULL);
> >>> >
> >>> > Please provide something other than NULL, such as the base address if
> >>> > the device. That's quite useful for debugging.
> >>> >
> >>> >> + if (!xgene_msi->fwnode) {
> >>> >> + dev_err(&pdev->dev, "Failed to create fwnode\n");
> >>> >> + rc = ENOMEM;
> >>> >> + goto error;
> >>> >> + }
> >>> >> + }
> >>> >> +
> >>> >> xgene_msi->num_cpus = num_possible_cpus();
> >>> >>
> >>> >> rc = xgene_msi_init_allocator(xgene_msi);
> >>> >> @@ -540,6 +568,7 @@ static int xgene_msi_probe(struct platform_device *pdev)
> >>> >> .driver = {
> >>> >> .name = "xgene-msi",
> >>> >> .of_match_table = xgene_msi_match_table,
> >>> >> + .acpi_match_table = ACPI_PTR(xgene_msi_acpi_ids),
> >>> >> },
> >>> >> .probe = xgene_msi_probe,
> >>> >> .remove = xgene_msi_remove,
> >>> >>
> >>> >
> >>> > The code is trivial, but relies on the MSI controller to probe before
> >>> > the PCI bus. What enforces this? How is it making sure that this is not
> >>> > going to break in the next kernel release? As far as I can tell, there
> >>> > is no explicit dependency between the two, making it the whole thing
> >>> > extremely fragile.
> >>> >
> >>> > Thanks,
> >>> >
> >>> > M.
> >>> > --
> >>> > Jazz is not dead. It just smells funny...
> >>>
> >>> --
next prev parent reply other threads:[~2017-06-08 10:39 UTC|newest]
Thread overview: 47+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-04-28 0:54 [PATCH v2 pci] PCI/MSI: pci-xgene-msi: Enable MSI support in ACPI boot for X-Gene v1 Khuong Dinh
2017-04-28 0:54 ` Khuong Dinh
2017-04-28 0:54 ` Khuong Dinh
2017-04-28 5:11 ` Jon Masters
2017-04-28 5:11 ` Jon Masters
2017-04-28 5:11 ` Jon Masters
2017-04-28 16:36 ` Jon Masters
2017-04-28 16:36 ` Jon Masters
2017-04-28 16:36 ` Jon Masters
2017-05-04 23:24 ` Jon Masters
2017-05-04 23:24 ` Jon Masters
2017-05-04 23:24 ` Jon Masters
2017-04-28 9:27 ` Marc Zyngier
2017-04-28 9:27 ` Marc Zyngier
2017-04-28 9:27 ` Marc Zyngier
2017-05-05 0:36 ` Khuong Dinh
2017-05-05 0:36 ` Khuong Dinh
2017-05-05 0:36 ` Khuong Dinh
2017-05-05 16:51 ` Lorenzo Pieralisi
2017-05-05 16:51 ` Lorenzo Pieralisi
2017-05-05 16:51 ` Lorenzo Pieralisi
2017-05-09 22:55 ` Khuong Dinh
2017-05-09 22:55 ` Khuong Dinh
2017-05-09 22:55 ` Khuong Dinh
2017-06-06 16:44 ` Khuong Dinh
2017-06-06 16:44 ` Khuong Dinh
2017-06-06 16:44 ` Khuong Dinh
2017-06-08 10:39 ` Lorenzo Pieralisi [this message]
2017-06-08 10:39 ` Lorenzo Pieralisi
2017-06-08 10:39 ` Lorenzo Pieralisi
2017-06-13 20:56 ` Khuong Dinh
2017-06-13 20:56 ` Khuong Dinh
2017-06-13 20:56 ` Khuong Dinh
2017-06-14 12:59 ` Lorenzo Pieralisi
2017-06-14 12:59 ` Lorenzo Pieralisi
2017-06-14 12:59 ` Lorenzo Pieralisi
2017-06-14 17:43 ` Khuong Dinh
2017-06-14 17:43 ` Khuong Dinh
2017-06-14 17:43 ` Khuong Dinh
2017-06-15 11:06 ` Lorenzo Pieralisi
2017-06-15 11:06 ` Lorenzo Pieralisi
2017-06-15 11:06 ` Lorenzo Pieralisi
2017-05-31 11:29 ` Jonathan Toppins
2017-05-31 11:29 ` Jonathan Toppins
2017-05-31 16:13 ` Loc Ho
2017-05-31 16:13 ` Loc Ho
2017-05-31 16:13 ` Loc Ho
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170608103956.GB8644@red-moon \
--to=lorenzo.pieralisi@arm.com \
--cc=bhelgaas@google.com \
--cc=dhdang@apm.com \
--cc=jcm@redhat.com \
--cc=kdinh@apm.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=marc.zyngier@arm.com \
--cc=msalter@redhat.com \
--cc=patches@apm.com \
--cc=rjw@rjwysocki.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.