diff mbox

[v11,13/15] PCI: Add MCFG quirks for Cavium ThunderX pass2.x host controller

Message ID 20161206204556.GB19700@bhelgaas-glaptop.roam.corp.google.com (mailing list archive)
State New, archived
Delegated to: Bjorn Helgaas
Headers show

Commit Message

Bjorn Helgaas Dec. 6, 2016, 8:45 p.m. UTC 
On Tue, Dec 06, 2016 at 02:07:45PM +0100, Tomasz Nowicki wrote:
> Hi Bjorn,
> 
> On 06.12.2016 00:26, Bjorn Helgaas wrote:
> >diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
> >index d7e7c0a..1239a8e 100644
> >--- a/drivers/pci/host/Kconfig
> >+++ b/drivers/pci/host/Kconfig
> >@@ -240,7 +240,8 @@ config PCIE_QCOM
> >
> > config PCI_HOST_THUNDER_PEM
> > 	bool "Cavium Thunder PCIe controller to off-chip devices"
> >-	depends on OF && ARM64
> >+	depends on ARM64
> >+	depends on OF || (ACPI && PCI_QUIRKS)
> > 	select PCI_HOST_COMMON
> > 	help
> > 	  Say Y here if you want PCIe support for CN88XX Cavium Thunder SoCs.
> >diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
> >index 64845f0..97e6bfc 100644
> >--- a/drivers/pci/host/Makefile
> >+++ b/drivers/pci/host/Makefile
> >@@ -28,7 +28,7 @@ obj-$(CONFIG_PCIE_ALTERA_MSI) += pcie-altera-msi.o
> > obj-$(CONFIG_ARM64) += pcie-hisi.o
> > obj-$(CONFIG_PCIE_QCOM) += pcie-qcom.o
> > obj-$(CONFIG_PCI_HOST_THUNDER_ECAM) += pci-thunder-ecam.o
> >-obj-$(CONFIG_PCI_HOST_THUNDER_PEM) += pci-thunder-pem.o
> >+obj-$(CONFIG_ARM64) += pci-thunder-pem.o
> 
> Since we build pci-thunder-pem.o for ARM64 and ...
> 
> > obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o
> > obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o
> > obj-$(CONFIG_PCIE_ROCKCHIP) += pcie-rockchip.o
> >diff --git a/drivers/pci/host/pci-thunder-pem.c b/drivers/pci/host/pci-thunder-pem.c
> >index c3276ee..af722eb 100644
> >--- a/drivers/pci/host/pci-thunder-pem.c
> >+++ b/drivers/pci/host/pci-thunder-pem.c
> >@@ -18,8 +18,12 @@
> > #include <linux/init.h>
> > #include <linux/of_address.h>
> > #include <linux/of_pci.h>
> >+#include <linux/pci-acpi.h>
> > #include <linux/pci-ecam.h>
> > #include <linux/platform_device.h>
> >+#include "../pci.h"
> >+
> >+#if defined(CONFIG_PCI_HOST_THUNDER_PEM) || (defined(CONFIG_ACPI) && defined(CONFIG_PCI_QUIRKS))
> 
> have this ^^^ ifdef IMO we can drop Kconfig changes in
> drivers/pci/host/Kconfig. The same for PCI_HOST_THUNDER_ECAM in
> patch[14/15].

I think you're right that we *could* drop the Kconfig changes.  I
didn't actually drop them because I think it might be slightly
confusing that with the previous Kconfig:

  config PCI_HOST_THUNDER_PEM
        bool "Cavium Thunder PCIe controller to off-chip devices"
        depends on OF && ARM64

we still build pci-thunder-pem.o, even when CONFIG_OF isn't defined.

There's no question that this whole thing is confusing no matter what
we do.  That reminds me that I meant to add some comments in the
Makefile.  What do you think about the following:


commit 5d06f9125ec00215fa59b789491fdd51186a4ee1
Author: Bjorn Helgaas <bhelgaas@google.com>
Date:   Tue Dec 6 14:27:59 2016 -0600

    PCI: Explain ARM64 ACPI/MCFG quirk Kconfig and build strategy
    
    Add Makefile comments to explain the Kconfig and build strategy for ARM64
    drivers that work around not-quite-ECAM issues.  No functional change
    intended.
    
    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Tomasz Nowicki Dec. 7, 2016, 12:23 p.m. UTC | #1 
On 06.12.2016 21:45, Bjorn Helgaas wrote:
> On Tue, Dec 06, 2016 at 02:07:45PM +0100, Tomasz Nowicki wrote:
>> Hi Bjorn,
>>
>> On 06.12.2016 00:26, Bjorn Helgaas wrote:
>>> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
>>> index d7e7c0a..1239a8e 100644
>>> --- a/drivers/pci/host/Kconfig
>>> +++ b/drivers/pci/host/Kconfig
>>> @@ -240,7 +240,8 @@ config PCIE_QCOM
>>>
>>> config PCI_HOST_THUNDER_PEM
>>> 	bool "Cavium Thunder PCIe controller to off-chip devices"
>>> -	depends on OF && ARM64
>>> +	depends on ARM64
>>> +	depends on OF || (ACPI && PCI_QUIRKS)
>>> 	select PCI_HOST_COMMON
>>> 	help
>>> 	  Say Y here if you want PCIe support for CN88XX Cavium Thunder SoCs.
>>> diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
>>> index 64845f0..97e6bfc 100644
>>> --- a/drivers/pci/host/Makefile
>>> +++ b/drivers/pci/host/Makefile
>>> @@ -28,7 +28,7 @@ obj-$(CONFIG_PCIE_ALTERA_MSI) += pcie-altera-msi.o
>>> obj-$(CONFIG_ARM64) += pcie-hisi.o
>>> obj-$(CONFIG_PCIE_QCOM) += pcie-qcom.o
>>> obj-$(CONFIG_PCI_HOST_THUNDER_ECAM) += pci-thunder-ecam.o
>>> -obj-$(CONFIG_PCI_HOST_THUNDER_PEM) += pci-thunder-pem.o
>>> +obj-$(CONFIG_ARM64) += pci-thunder-pem.o
>>
>> Since we build pci-thunder-pem.o for ARM64 and ...
>>
>>> obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o
>>> obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o
>>> obj-$(CONFIG_PCIE_ROCKCHIP) += pcie-rockchip.o
>>> diff --git a/drivers/pci/host/pci-thunder-pem.c b/drivers/pci/host/pci-thunder-pem.c
>>> index c3276ee..af722eb 100644
>>> --- a/drivers/pci/host/pci-thunder-pem.c
>>> +++ b/drivers/pci/host/pci-thunder-pem.c
>>> @@ -18,8 +18,12 @@
>>> #include <linux/init.h>
>>> #include <linux/of_address.h>
>>> #include <linux/of_pci.h>
>>> +#include <linux/pci-acpi.h>
>>> #include <linux/pci-ecam.h>
>>> #include <linux/platform_device.h>
>>> +#include "../pci.h"
>>> +
>>> +#if defined(CONFIG_PCI_HOST_THUNDER_PEM) || (defined(CONFIG_ACPI) && defined(CONFIG_PCI_QUIRKS))
>>
>> have this ^^^ ifdef IMO we can drop Kconfig changes in
>> drivers/pci/host/Kconfig. The same for PCI_HOST_THUNDER_ECAM in
>> patch[14/15].
>
> I think you're right that we *could* drop the Kconfig changes.  I
> didn't actually drop them because I think it might be slightly
> confusing that with the previous Kconfig:
>
>   config PCI_HOST_THUNDER_PEM
>         bool "Cavium Thunder PCIe controller to off-chip devices"
>         depends on OF && ARM64
>
> we still build pci-thunder-pem.o, even when CONFIG_OF isn't defined.
>
> There's no question that this whole thing is confusing no matter what
> we do.  That reminds me that I meant to add some comments in the
> Makefile.  What do you think about the following:
>
>
> commit 5d06f9125ec00215fa59b789491fdd51186a4ee1
> Author: Bjorn Helgaas <bhelgaas@google.com>
> Date:   Tue Dec 6 14:27:59 2016 -0600
>
>     PCI: Explain ARM64 ACPI/MCFG quirk Kconfig and build strategy
>
>     Add Makefile comments to explain the Kconfig and build strategy for ARM64
>     drivers that work around not-quite-ECAM issues.  No functional change
>     intended.
>
>     Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
>
> diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
> index 6cc84b4..bfe3179 100644
> --- a/drivers/pci/host/Makefile
> +++ b/drivers/pci/host/Makefile
> @@ -15,7 +15,6 @@ obj-$(CONFIG_PCIE_SPEAR13XX) += pcie-spear13xx.o
>  obj-$(CONFIG_PCI_KEYSTONE) += pci-keystone-dw.o pci-keystone.o
>  obj-$(CONFIG_PCIE_XILINX) += pcie-xilinx.o
>  obj-$(CONFIG_PCIE_XILINX_NWL) += pcie-xilinx-nwl.o
> -obj-$(CONFIG_ARM64) += pci-xgene.o
>  obj-$(CONFIG_PCI_XGENE_MSI) += pci-xgene-msi.o
>  obj-$(CONFIG_PCI_LAYERSCAPE) += pci-layerscape.o
>  obj-$(CONFIG_PCI_VERSATILE) += pci-versatile.o
> @@ -25,11 +24,23 @@ obj-$(CONFIG_PCIE_IPROC_PLATFORM) += pcie-iproc-platform.o
>  obj-$(CONFIG_PCIE_IPROC_BCMA) += pcie-iproc-bcma.o
>  obj-$(CONFIG_PCIE_ALTERA) += pcie-altera.o
>  obj-$(CONFIG_PCIE_ALTERA_MSI) += pcie-altera-msi.o
> -obj-$(CONFIG_ARM64) += pcie-hisi.o
>  obj-$(CONFIG_PCIE_QCOM) += pcie-qcom.o
> -obj-$(CONFIG_ARM64) += pci-thunder-ecam.o
> -obj-$(CONFIG_ARM64) += pci-thunder-pem.o
>  obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o
>  obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o
>  obj-$(CONFIG_PCIE_ROCKCHIP) += pcie-rockchip.o
>  obj-$(CONFIG_VMD) += vmd.o
> +
> +# The following drivers are for devices that use the generic ACPI
> +# pci_root.c driver but don't support standard ECAM config access.
> +# They contain MCFG quirks to replace the generic ECAM accessors with
> +# device-specific ones that are shared with the DT driver.
> +
> +# The ACPI driver is generic and should not require driver-specific
> +# config options to be enabled, so we always build these drivers on
> +# ARM64 and use internal ifdefs to only build the pieces we need
> +# depending on whether ACPI, the DT driver, or both are enabled.
> +
> +obj-$(CONFIG_ARM64) += pcie-hisi.o
> +obj-$(CONFIG_ARM64) += pci-thunder-ecam.o
> +obj-$(CONFIG_ARM64) += pci-thunder-pem.o
> +obj-$(CONFIG_ARM64) += pci-xgene.o
>

Looks good to me.

Tomasz
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
index 6cc84b4..bfe3179 100644
--- a/drivers/pci/host/Makefile
+++ b/drivers/pci/host/Makefile
@@ -15,7 +15,6 @@  obj-$(CONFIG_PCIE_SPEAR13XX) += pcie-spear13xx.o
 obj-$(CONFIG_PCI_KEYSTONE) += pci-keystone-dw.o pci-keystone.o
 obj-$(CONFIG_PCIE_XILINX) += pcie-xilinx.o
 obj-$(CONFIG_PCIE_XILINX_NWL) += pcie-xilinx-nwl.o
-obj-$(CONFIG_ARM64) += pci-xgene.o
 obj-$(CONFIG_PCI_XGENE_MSI) += pci-xgene-msi.o
 obj-$(CONFIG_PCI_LAYERSCAPE) += pci-layerscape.o
 obj-$(CONFIG_PCI_VERSATILE) += pci-versatile.o
@@ -25,11 +24,23 @@  obj-$(CONFIG_PCIE_IPROC_PLATFORM) += pcie-iproc-platform.o
 obj-$(CONFIG_PCIE_IPROC_BCMA) += pcie-iproc-bcma.o
 obj-$(CONFIG_PCIE_ALTERA) += pcie-altera.o
 obj-$(CONFIG_PCIE_ALTERA_MSI) += pcie-altera-msi.o
-obj-$(CONFIG_ARM64) += pcie-hisi.o
 obj-$(CONFIG_PCIE_QCOM) += pcie-qcom.o
-obj-$(CONFIG_ARM64) += pci-thunder-ecam.o
-obj-$(CONFIG_ARM64) += pci-thunder-pem.o
 obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o
 obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o
 obj-$(CONFIG_PCIE_ROCKCHIP) += pcie-rockchip.o
 obj-$(CONFIG_VMD) += vmd.o
+
+# The following drivers are for devices that use the generic ACPI
+# pci_root.c driver but don't support standard ECAM config access.
+# They contain MCFG quirks to replace the generic ECAM accessors with
+# device-specific ones that are shared with the DT driver.
+
+# The ACPI driver is generic and should not require driver-specific
+# config options to be enabled, so we always build these drivers on
+# ARM64 and use internal ifdefs to only build the pieces we need
+# depending on whether ACPI, the DT driver, or both are enabled.
+
+obj-$(CONFIG_ARM64) += pcie-hisi.o
+obj-$(CONFIG_ARM64) += pci-thunder-ecam.o
+obj-$(CONFIG_ARM64) += pci-thunder-pem.o
+obj-$(CONFIG_ARM64) += pci-xgene.o