| Message ID | 1506448161-30961-1-git-send-email-kdinh@apm.com |
|---|---|
| Headers | show |
| Series | PCI/MSI: pci-xgene-msi: Enable MSI support in ACPI boot for X-Gene v1 | expand |
Hi Lorenzo, Do you have any comments for this patch? Best regards, Khuong Dinh On Tue, Sep 26, 2017 at 10:49 AM, Khuong Dinh <kdinh@apm.com> wrote: > This patch set enables ACPI MSI support for X-Gene PCIe v1 hardware > and provides the proper MSI driver initialization ordering. > > Signed-off-by: Khuong Dinh <kdinh@apm.com> > --- > v4: > - Remove Marc Zyngier ACK in v2 > - Use acpi_bus_scan on MSI controller handle when MSI device is found > - Register ACPI MSI driver when MSI device is found instead of using > subsys_initcall > - Split ACPI MSI driver support into two patches - one to enable MSI > support for X-Gene PCIe v1 hardware, one to enforce MSI driver loaded > before PCIe controller driver in ACPI boot mode > v3: > - Input X-Gene MSI base address for irq_domain_alloc_fwnode > - Add a hook to enforce X-Gene MSI be probed prior acpi_bus_scan happens > v2: > - Verify with BIOS version 3.06.25 and 3.07.09 > v1: > - Initial version > --- > > Khuong Dinh (2): > PCI/MSI: pci-xgene-msi: Enable MSI support in ACPI boot for X-Gene v1 > PCI/MSI: Enforce MSI driver loaded before PCIe in ACPI boot > > drivers/acpi/Makefile | 2 +- > drivers/acpi/acpi_msi.c | 86 ++++++++++++++++++++++++++++++++++++++ > drivers/acpi/acpi_platform.c | 3 +- > drivers/acpi/internal.h | 1 + > drivers/acpi/scan.c | 1 + > drivers/pci/host/pci-xgene-msi.c | 60 ++++++++++++++++++++++++-- > include/linux/acpi_msi.h | 37 ++++++++++++++++ > 7 files changed, 183 insertions(+), 7 deletions(-) > create mode 100644 drivers/acpi/acpi_msi.c > create mode 100644 include/linux/acpi_msi.h >
Hi Khuong, On Mon, Oct 16, 2017 at 02:19:50PM -0700, Khuong Dinh wrote: > Hi Lorenzo, > Do you have any comments for this patch? I'd have some comments but given that there are related issues with ACPI probe ordering that Marc is trying to solve on his side - I will work with him to see if we can accommodate changes that can solve this issue too. Again - I recognize it is a complex problem (that is not even contemplated by the current ACPI specs), we have to try to make the solution as generic as we can to prevent reinventing the wheel anytime a sligthly different issue (related to ACPI probe ordering) comes up. Leave it to me (us) and I will get back to you on this. Thanks, Lorenzo > Best regards, > Khuong Dinh > > On Tue, Sep 26, 2017 at 10:49 AM, Khuong Dinh <kdinh@apm.com> wrote: > > This patch set enables ACPI MSI support for X-Gene PCIe v1 hardware > > and provides the proper MSI driver initialization ordering. > > > > Signed-off-by: Khuong Dinh <kdinh@apm.com> > > --- > > v4: > > - Remove Marc Zyngier ACK in v2 > > - Use acpi_bus_scan on MSI controller handle when MSI device is found > > - Register ACPI MSI driver when MSI device is found instead of using > > subsys_initcall > > - Split ACPI MSI driver support into two patches - one to enable MSI > > support for X-Gene PCIe v1 hardware, one to enforce MSI driver loaded > > before PCIe controller driver in ACPI boot mode > > v3: > > - Input X-Gene MSI base address for irq_domain_alloc_fwnode > > - Add a hook to enforce X-Gene MSI be probed prior acpi_bus_scan happens > > v2: > > - Verify with BIOS version 3.06.25 and 3.07.09 > > v1: > > - Initial version > > --- > > > > Khuong Dinh (2): > > PCI/MSI: pci-xgene-msi: Enable MSI support in ACPI boot for X-Gene v1 > > PCI/MSI: Enforce MSI driver loaded before PCIe in ACPI boot > > > > drivers/acpi/Makefile | 2 +- > > drivers/acpi/acpi_msi.c | 86 ++++++++++++++++++++++++++++++++++++++ > > drivers/acpi/acpi_platform.c | 3 +- > > drivers/acpi/internal.h | 1 + > > drivers/acpi/scan.c | 1 + > > drivers/pci/host/pci-xgene-msi.c | 60 ++++++++++++++++++++++++-- > > include/linux/acpi_msi.h | 37 ++++++++++++++++ > > 7 files changed, 183 insertions(+), 7 deletions(-) > > create mode 100644 drivers/acpi/acpi_msi.c > > create mode 100644 include/linux/acpi_msi.h > >
On Tue, Sep 26, 2017 at 11:49:19AM -0600, Khuong Dinh wrote: > This patch set enables ACPI MSI support for X-Gene PCIe v1 hardware > and provides the proper MSI driver initialization ordering. > > Signed-off-by: Khuong Dinh <kdinh@apm.com> > --- > v4: > - Remove Marc Zyngier ACK in v2 > - Use acpi_bus_scan on MSI controller handle when MSI device is found > - Register ACPI MSI driver when MSI device is found instead of using > subsys_initcall > - Split ACPI MSI driver support into two patches - one to enable MSI > support for X-Gene PCIe v1 hardware, one to enforce MSI driver loaded > before PCIe controller driver in ACPI boot mode > v3: > - Input X-Gene MSI base address for irq_domain_alloc_fwnode > - Add a hook to enforce X-Gene MSI be probed prior acpi_bus_scan happens > v2: > - Verify with BIOS version 3.06.25 and 3.07.09 > v1: > - Initial version > --- > > Khuong Dinh (2): > PCI/MSI: pci-xgene-msi: Enable MSI support in ACPI boot for X-Gene v1 > PCI/MSI: Enforce MSI driver loaded before PCIe in ACPI boot > > drivers/acpi/Makefile | 2 +- > drivers/acpi/acpi_msi.c | 86 ++++++++++++++++++++++++++++++++++++++ > drivers/acpi/acpi_platform.c | 3 +- > drivers/acpi/internal.h | 1 + > drivers/acpi/scan.c | 1 + > drivers/pci/host/pci-xgene-msi.c | 60 ++++++++++++++++++++++++-- > include/linux/acpi_msi.h | 37 ++++++++++++++++ > 7 files changed, 183 insertions(+), 7 deletions(-) > create mode 100644 drivers/acpi/acpi_msi.c > create mode 100644 include/linux/acpi_msi.h The changelogs don't really give the nitty-gritty details of the problem, e.g., apparently the host bridge and the MSI controller are enumerated as two separate devices and there's an ordering issue with binding drivers to them. They should also contrast the X-Gene situation with other systems so we can see why X-Gene has this problem when other systems don't. I'm not thrilled about all the ACPI code this adds, but I'm guessing there'll be some better solution eventually. I'm going to mark these as "changes requested" even though I haven't asked for anything specific to be changed because it sounds like Lorenzo et al. may have more concrete proposals soon. Are there currently systems that do not work and need to be fixed ASAP? Bjorn
Hi Lorenzo, On Tue, Oct 17, 2017 at 6:38 AM, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote: > Hi Khuong, > > On Mon, Oct 16, 2017 at 02:19:50PM -0700, Khuong Dinh wrote: >> Hi Lorenzo, >> Do you have any comments for this patch? > > I'd have some comments but given that there are related issues with ACPI > probe ordering that Marc is trying to solve on his side - I will work > with him to see if we can accommodate changes that can solve this issue > too. > > Again - I recognize it is a complex problem (that is not even > contemplated by the current ACPI specs), we have to try to make > the solution as generic as we can to prevent reinventing the wheel > anytime a sligthly different issue (related to ACPI probe ordering) > comes up. > > Leave it to me (us) and I will get back to you on this. Thanks for helping to take care of the generic ACPI probe ordering issue. Given that the patch 'PCI/MSI: pci-xgene-msi: Enable MSI support in ACPI boot for X-Gene v1" adds only the ACPI ID, can you pull in this patch independently from the ACPI probe ordering issue? Best regards, Khuong Dinh > Thanks, > Lorenzo > >> Best regards, >> Khuong Dinh >> >> On Tue, Sep 26, 2017 at 10:49 AM, Khuong Dinh <kdinh@apm.com> wrote: >> > This patch set enables ACPI MSI support for X-Gene PCIe v1 hardware >> > and provides the proper MSI driver initialization ordering. >> > >> > Signed-off-by: Khuong Dinh <kdinh@apm.com> >> > --- >> > v4: >> > - Remove Marc Zyngier ACK in v2 >> > - Use acpi_bus_scan on MSI controller handle when MSI device is found >> > - Register ACPI MSI driver when MSI device is found instead of using >> > subsys_initcall >> > - Split ACPI MSI driver support into two patches - one to enable MSI >> > support for X-Gene PCIe v1 hardware, one to enforce MSI driver loaded >> > before PCIe controller driver in ACPI boot mode >> > v3: >> > - Input X-Gene MSI base address for irq_domain_alloc_fwnode >> > - Add a hook to enforce X-Gene MSI be probed prior acpi_bus_scan happens >> > v2: >> > - Verify with BIOS version 3.06.25 and 3.07.09 >> > v1: >> > - Initial version >> > --- >> > >> > Khuong Dinh (2): >> > PCI/MSI: pci-xgene-msi: Enable MSI support in ACPI boot for X-Gene v1 >> > PCI/MSI: Enforce MSI driver loaded before PCIe in ACPI boot >> > >> > drivers/acpi/Makefile | 2 +- >> > drivers/acpi/acpi_msi.c | 86 ++++++++++++++++++++++++++++++++++++++ >> > drivers/acpi/acpi_platform.c | 3 +- >> > drivers/acpi/internal.h | 1 + >> > drivers/acpi/scan.c | 1 + >> > drivers/pci/host/pci-xgene-msi.c | 60 ++++++++++++++++++++++++-- >> > include/linux/acpi_msi.h | 37 ++++++++++++++++ >> > 7 files changed, 183 insertions(+), 7 deletions(-) >> > create mode 100644 drivers/acpi/acpi_msi.c >> > create mode 100644 include/linux/acpi_msi.h >> >
On Tue, Oct 17, 2017 at 10:45:35PM -0700, Khuong Dinh wrote: > Hi Lorenzo, > > On Tue, Oct 17, 2017 at 6:38 AM, Lorenzo Pieralisi > <lorenzo.pieralisi@arm.com> wrote: > > Hi Khuong, > > > > On Mon, Oct 16, 2017 at 02:19:50PM -0700, Khuong Dinh wrote: > >> Hi Lorenzo, > >> Do you have any comments for this patch? > > > > I'd have some comments but given that there are related issues with ACPI > > probe ordering that Marc is trying to solve on his side - I will work > > with him to see if we can accommodate changes that can solve this issue > > too. > > > > Again - I recognize it is a complex problem (that is not even > > contemplated by the current ACPI specs), we have to try to make > > the solution as generic as we can to prevent reinventing the wheel > > anytime a sligthly different issue (related to ACPI probe ordering) > > comes up. > > > > Leave it to me (us) and I will get back to you on this. > > Thanks for helping to take care of the generic ACPI probe ordering issue. > Given that the patch 'PCI/MSI: pci-xgene-msi: Enable MSI support in ACPI > boot for X-Gene v1" adds only the ACPI ID, can you pull in this patch > independently from the ACPI probe ordering issue? No. For two reasons: 1) It's Bjorn who decides whether that code can be merged or not, not me 2) That patch sneaks in ACPI MSI support for X-gene v1 that depends on kernel link ordering. As soon as it is pulled in the mainline it creates a dependency on pseudo-working code that may break anytime and as I said many times before I am not willing to rely on that. Lorenzo IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
[removed unintended disclaimer] On Tue, Oct 17, 2017 at 10:45:35PM -0700, Khuong Dinh wrote: > Hi Lorenzo, > > On Tue, Oct 17, 2017 at 6:38 AM, Lorenzo Pieralisi > <lorenzo.pieralisi@arm.com> wrote: > > Hi Khuong, > > > > On Mon, Oct 16, 2017 at 02:19:50PM -0700, Khuong Dinh wrote: > >> Hi Lorenzo, > >> Do you have any comments for this patch? > > > > I'd have some comments but given that there are related issues with ACPI > > probe ordering that Marc is trying to solve on his side - I will work > > with him to see if we can accommodate changes that can solve this issue > > too. > > > > Again - I recognize it is a complex problem (that is not even > > contemplated by the current ACPI specs), we have to try to make > > the solution as generic as we can to prevent reinventing the wheel > > anytime a sligthly different issue (related to ACPI probe ordering) > > comes up. > > > > Leave it to me (us) and I will get back to you on this. > > Thanks for helping to take care of the generic ACPI probe ordering issue. > Given that the patch 'PCI/MSI: pci-xgene-msi: Enable MSI support in ACPI > boot for X-Gene v1" adds only the ACPI ID, can you pull in this patch > independently from the ACPI probe ordering issue? No. For two reasons: 1) It's Bjorn who decides whether that code can be merged or not, not me 2) That patch sneaks in ACPI MSI support for X-gene v1 that depends on kernel link ordering. As soon as it is pulled in the mainline it creates a dependency on pseudo-working code that may break anytime and as I said many times before I am not willing to rely on that. Lorenzo
Hi Lorenzo, On Wed, Oct 18, 2017 at 2:26 AM, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote: > [removed unintended disclaimer] > > On Tue, Oct 17, 2017 at 10:45:35PM -0700, Khuong Dinh wrote: >> Hi Lorenzo, >> >> On Tue, Oct 17, 2017 at 6:38 AM, Lorenzo Pieralisi >> <lorenzo.pieralisi@arm.com> wrote: >> > Hi Khuong, >> > >> > On Mon, Oct 16, 2017 at 02:19:50PM -0700, Khuong Dinh wrote: >> >> Hi Lorenzo, >> >> Do you have any comments for this patch? >> > >> > I'd have some comments but given that there are related issues with ACPI >> > probe ordering that Marc is trying to solve on his side - I will work >> > with him to see if we can accommodate changes that can solve this issue >> > too. >> > >> > Again - I recognize it is a complex problem (that is not even >> > contemplated by the current ACPI specs), we have to try to make >> > the solution as generic as we can to prevent reinventing the wheel >> > anytime a sligthly different issue (related to ACPI probe ordering) >> > comes up. >> > >> > Leave it to me (us) and I will get back to you on this. >> >> Thanks for helping to take care of the generic ACPI probe ordering issue. >> Given that the patch 'PCI/MSI: pci-xgene-msi: Enable MSI support in ACPI >> boot for X-Gene v1" adds only the ACPI ID, can you pull in this patch >> independently from the ACPI probe ordering issue? > > No. For two reasons: > > 1) It's Bjorn who decides whether that code can be merged or not, not me > 2) That patch sneaks in ACPI MSI support for X-gene v1 that depends on > kernel link ordering. As soon as it is pulled in the mainline it > creates a dependency on pseudo-working code that may break anytime > and as I said many times before I am not willing to rely on that. Thanks for your information. I got it. I look forward to hearing your updated information about this generic ACPI probe ordering issue. > Lorenzo
This patch set enables ACPI MSI support for X-Gene PCIe v1 hardware and provides the proper MSI driver initialization ordering. Signed-off-by: Khuong Dinh <kdinh@apm.com> --- v4: - Remove Marc Zyngier ACK in v2 - Use acpi_bus_scan on MSI controller handle when MSI device is found - Register ACPI MSI driver when MSI device is found instead of using subsys_initcall - Split ACPI MSI driver support into two patches - one to enable MSI support for X-Gene PCIe v1 hardware, one to enforce MSI driver loaded before PCIe controller driver in ACPI boot mode v3: - Input X-Gene MSI base address for irq_domain_alloc_fwnode - Add a hook to enforce X-Gene MSI be probed prior acpi_bus_scan happens v2: - Verify with BIOS version 3.06.25 and 3.07.09 v1: - Initial version --- Khuong Dinh (2): PCI/MSI: pci-xgene-msi: Enable MSI support in ACPI boot for X-Gene v1 PCI/MSI: Enforce MSI driver loaded before PCIe in ACPI boot drivers/acpi/Makefile | 2 +- drivers/acpi/acpi_msi.c | 86 ++++++++++++++++++++++++++++++++++++++ drivers/acpi/acpi_platform.c | 3 +- drivers/acpi/internal.h | 1 + drivers/acpi/scan.c | 1 + drivers/pci/host/pci-xgene-msi.c | 60 ++++++++++++++++++++++++-- include/linux/acpi_msi.h | 37 ++++++++++++++++ 7 files changed, 183 insertions(+), 7 deletions(-) create mode 100644 drivers/acpi/acpi_msi.c create mode 100644 include/linux/acpi_msi.h