From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751472AbdFHKil (ORCPT ); Thu, 8 Jun 2017 06:38:41 -0400 Received: from foss.arm.com ([217.140.101.70]:45800 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750725AbdFHKij (ORCPT ); Thu, 8 Jun 2017 06:38:39 -0400 Date: Thu, 8 Jun 2017 11:39:56 +0100 From: Lorenzo Pieralisi To: Khuong Dinh Cc: Marc Zyngier , msalter@redhat.com, Bjorn Helgaas , 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 Subject: Re: [PATCH v2 pci] PCI/MSI: pci-xgene-msi: Enable MSI support in ACPI boot for X-Gene v1 Message-ID: <20170608103956.GB8644@red-moon> References: <1493340847-25720-1-git-send-email-kdinh@apm.com> <20170505165119.GA8614@red-moon> 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 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 wrote: > > Hi Lorenzo, > > > > On Fri, May 5, 2017 at 9:51 AM, Lorenzo Pieralisi > > 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 wrote: > >>> > On 28/04/17 01:54, Khuong Dinh wrote: > >>> >> From: Khuong Dinh > >>> >> > >>> >> 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 > >>> >> Signed-off-by: Duc Dang > >>> >> Acked-by: Marc Zyngier > >>> >> --- > >>> >> 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 > >>> >> #include > >>> >> #include > >>> >> +#include > >>> >> > >>> >> #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... > >>> > >>> -- From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Return-Path: Date: Thu, 8 Jun 2017 11:39:56 +0100 From: Lorenzo Pieralisi To: Khuong Dinh Subject: Re: [PATCH v2 pci] PCI/MSI: pci-xgene-msi: Enable MSI support in ACPI boot for X-Gene v1 Message-ID: <20170608103956.GB8644@red-moon> References: <1493340847-25720-1-git-send-email-kdinh@apm.com> <20170505165119.GA8614@red-moon> MIME-Version: 1.0 In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: rjw@rjwysocki.net, Duc Dang , Marc Zyngier , linux-pci@vger.kernel.org, msalter@redhat.com, patches@apm.com, linux-kernel@vger.kernel.org, jcm@redhat.com, Bjorn Helgaas , linux-arm-kernel@lists.infradead.org Content-Type: text/plain; charset="utf-8" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+bjorn=helgaas.com@lists.infradead.org List-ID: 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== From mboxrd@z Thu Jan 1 00:00:00 1970 From: lorenzo.pieralisi@arm.com (Lorenzo Pieralisi) Date: Thu, 8 Jun 2017 11:39:56 +0100 Subject: [PATCH v2 pci] PCI/MSI: pci-xgene-msi: Enable MSI support in ACPI boot for X-Gene v1 In-Reply-To: References: <1493340847-25720-1-git-send-email-kdinh@apm.com> <20170505165119.GA8614@red-moon> Message-ID: <20170608103956.GB8644@red-moon> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org 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 wrote: > > Hi Lorenzo, > > > > On Fri, May 5, 2017 at 9:51 AM, Lorenzo Pieralisi > > 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 wrote: > >>> > On 28/04/17 01:54, Khuong Dinh wrote: > >>> >> From: Khuong Dinh > >>> >> > >>> >> 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 > >>> >> Signed-off-by: Duc Dang > >>> >> Acked-by: Marc Zyngier > >>> >> --- > >>> >> 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 > >>> >> #include > >>> >> #include > >>> >> +#include > >>> >> > >>> >> #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... > >>> > >>> --