[PATCH 0/3] PCI:hisi: Add DT almost ECAM support for HiSilicon Hip06/Hip07 host controllers

[PATCH 0/3] PCI:hisi: Add DT almost ECAM support for HiSilicon Hip06/Hip07 host controllers

[Dongdong Liu]

The PCIe controller in Hip06/Hip07 SoCs is not completely
ECAM-compliant. It is non-ECAM only for the RC bus config space; for
any other bus underneath the root bus it does support ECAM access.

This series contains
1. Add the almost ECAM support in DT way. 
2. Add quirk to set root port pdev->no_msi = 1. 
3. Add condition #ifndef CONFIG_ARM64 for pci_fixup_irqs.

This patchset is based on 4.10-rc3.

Dongdong Liu (3):
  PCI:hisi: Add DT almost ECAM support for HiSilicon Hip06/Hip07 host
    controllers
  PCI: Set pdev->no_msi=1 for HiSilicon Hip06/Hip07 host controllers
  PCI: generic: Fix the bug of pci_fixup_irqs() for arm64 platform.

 .../devicetree/bindings/pci/hisilicon-pcie.txt     | 37 +++++++++++++++
 drivers/pci/host/pci-host-common.c                 |  2 +
 drivers/pci/host/pcie-hisi.c                       | 54 ++++++++++++++++++++--
 drivers/pci/quirks.c                               |  1 +
 include/linux/pci_ids.h                            |  3 ++
 5 files changed, 94 insertions(+), 3 deletions(-)

-- 
1.9.1

[PATCH 1/3] PCI:hisi: Add DT almost ECAM support for HiSilicon Hip06/Hip07 host controllers

[Dongdong Liu]

The PCIe controller in Hip06/Hip07 SoCs is not completely
ECAM-compliant. It is non-ECAM only for the RC bus config space; for
any other bus underneath the root bus it does support ECAM access.
This is to add the almost ECAM support in DT way.

Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
Reviewed-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
Reviewed-by: Zhou Wang <wangzhou1@hisilicon.com>
---
 .../devicetree/bindings/pci/hisilicon-pcie.txt     | 37 +++++++++++++++
 drivers/pci/host/pcie-hisi.c                       | 54 ++++++++++++++++++++--
 2 files changed, 88 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt b/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
index 59c2f47..38e6dc3 100644
--- a/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
@@ -42,3 +42,40 @@ Hip05 Example (note that Hip06 is the same except compatible):
 				 0x0 0 0 4 &mbigen_pcie 4 13>;
 		status = "ok";
 	};
+
+HiSilicon Hip06/Hip07 PCIe host bridge DT (almost ecam) description
+The properties and their meanings are identical to those described in
+host-generic-pci.txt except as listed below.
+
+Properties of the host controller node that differ from
+host-generic-pci.txt:
+
+- compatible     : Must be "hisilicon,pcie-almost-ecam"
+
+- reg            : Two entries: First the ECAM configuration space for any
+		   other bus underneath the root bus. Second, the base
+		   and size of the HiSilicon host bridge registers inculde
+		   the RC itself config space.
+
+Example:
+	pcie0: pcie@a0090000 {
+		compatible = "hisilicon,pcie-almost-ecam";
+		reg = <0 0xb0000000 0 0x2000000>,  /*  ECAM configuration space */
+		      <0 0xa0090000 0 0x10000>; /* host bridge registers */
+		bus-range = <0  31>;
+		msi-map = <0x0000 &its_dsa 0x0000 0x2000>;
+		msi-map-mask = <0xffff>;
+		#address-cells = <3>;
+		#size-cells = <2>;
+		device_type = "pci";
+		dma-coherent;
+		ranges = <0x02000000 0 0xb2000000 0x0 0xb2000000 0 0x5ff0000
+			  0x01000000 0 0 0 0xb7ff0000 0 0x10000>;
+		#interrupt-cells = <1>;
+		interrupt-map-mask = <0xf800 0 0 7>;
+		interrupt-map = <0x0 0 0 1 &mbigen_pcie0 650 4
+				 0x0 0 0 2 &mbigen_pcie0 650 4
+				 0x0 0 0 3 &mbigen_pcie0 650 4
+				 0x0 0 0 4 &mbigen_pcie0 650 4>;
+		status = "ok";
+	};
diff --git a/drivers/pci/host/pcie-hisi.c b/drivers/pci/host/pcie-hisi.c
index a301a71..f37f5a2 100644
--- a/drivers/pci/host/pcie-hisi.c
+++ b/drivers/pci/host/pcie-hisi.c
@@ -24,7 +24,7 @@
 #include <linux/regmap.h>
 #include "../pci.h"
 
-#if defined(CONFIG_ACPI) && defined(CONFIG_PCI_QUIRKS)
+#if defined(CONFIG_PCI_HISI) || (defined(CONFIG_ACPI) && defined(CONFIG_PCI_QUIRKS))
 
 static int hisi_pcie_acpi_rd_conf(struct pci_bus *bus, u32 devfn, int where,
 				  int size, u32 *val)
@@ -74,6 +74,8 @@ static void __iomem *hisi_pcie_map_bus(struct pci_bus *bus, unsigned int devfn,
 		return pci_ecam_map_bus(bus, devfn, where);
 }
 
+#if defined(CONFIG_ACPI) && defined(CONFIG_PCI_QUIRKS)
+
 static int hisi_pcie_init(struct pci_config_window *cfg)
 {
 	struct device *dev = cfg->parent;
@@ -262,17 +264,23 @@ static int hisi_pcie_probe(struct platform_device *pdev)
 	const struct of_device_id *match;
 	struct resource *reg;
 	struct device_driver *driver;
+	struct pci_ecam_ops *ops;
 	int ret;
 
+	driver = dev->driver;
+	match = of_match_device(driver->of_match_table, dev);
+	if (!strcmp(match->compatible, "hisilicon,pcie-almost-ecam")) {
+		ops = (struct pci_ecam_ops *)match->data;
+		return pci_host_common_probe(pdev, ops);
+	}
+
 	hisi_pcie = devm_kzalloc(dev, sizeof(*hisi_pcie), GFP_KERNEL);
 	if (!hisi_pcie)
 		return -ENOMEM;
 
 	pp = &hisi_pcie->pp;
 	pp->dev = dev;
-	driver = dev->driver;
 
-	match = of_match_device(driver->of_match_table, dev);
 	hisi_pcie->soc_ops = (struct pcie_soc_ops *) match->data;
 
 	hisi_pcie->subctrl =
@@ -302,6 +310,40 @@ static int hisi_pcie_probe(struct platform_device *pdev)
 		&hisi_pcie_link_up_hip06
 };
 
+static int hisi_pcie_platform_init(struct pci_config_window *cfg)
+{
+	struct device *dev = cfg->parent;
+	struct platform_device *pdev = to_platform_device(dev);
+	struct resource *res;
+	void __iomem *reg_base;
+
+	if (!dev->of_node)
+		return -EINVAL;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	if (!res) {
+		dev_err(dev, "missing \"reg[1]\"property\n");
+		return -EINVAL;
+	}
+
+	reg_base = devm_ioremap(dev, res->start, resource_size(res));
+	if (!reg_base)
+		return -ENOMEM;
+
+	cfg->priv = reg_base;
+	return 0;
+}
+
+struct pci_ecam_ops hisi_pcie_platform_ops = {
+	.bus_shift    = 20,
+	.init         =  hisi_pcie_platform_init,
+	.pci_ops      = {
+		.map_bus    = hisi_pcie_map_bus,
+		.read       = hisi_pcie_acpi_rd_conf,
+		.write      = hisi_pcie_acpi_wr_conf,
+	}
+};
+
 static const struct of_device_id hisi_pcie_of_match[] = {
 	{
 			.compatible = "hisilicon,hip05-pcie",
@@ -311,6 +353,11 @@ static int hisi_pcie_probe(struct platform_device *pdev)
 			.compatible = "hisilicon,hip06-pcie",
 			.data	    = (void *) &hip06_ops,
 	},
+
+	{
+			.compatible = "hisilicon,pcie-almost-ecam",
+			.data	    = (void *) &hisi_pcie_platform_ops,
+	},
 	{},
 };
 
@@ -324,3 +371,4 @@ static int hisi_pcie_probe(struct platform_device *pdev)
 builtin_platform_driver(hisi_pcie_driver);
 
 #endif
+#endif
-- 
1.9.1

[PATCH 2/3] PCI: Set pdev->no_msi=1 for HiSilicon Hip06/Hip07 host controllers

[Dongdong Liu]

The PCIe root port in Hip06/Hip07 SoCs does not support MSI/MSI-X,
it can only transfer MSI/MSI-X from EP, so we add the quirk to
set root port pdev->no_msi = 1.

Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
Reviewed-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
Reviewed-by: Zhou Wang <wangzhou1@hisilicon.com>
---
 drivers/pci/quirks.c    | 1 +
 include/linux/pci_ids.h | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 1800bef..20cbdae 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -1634,6 +1634,7 @@ static void quirk_pcie_mch(struct pci_dev *pdev)
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_E7520_MCH,	quirk_pcie_mch);
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_E7320_MCH,	quirk_pcie_mch);
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_E7525_MCH,	quirk_pcie_mch);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_HUAWEI,	PCI_DEVICE_ID_HISILICON_1610,	quirk_pcie_mch);
 
 
 /*
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index 73dda0e..9cc4720 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -3054,4 +3054,7 @@
 
 #define PCI_VENDOR_ID_OCZ		0x1b85
 
+#define PCI_VENDOR_ID_HUAWEI         	0x19e5
+#define PCI_DEVICE_ID_HISILICON_1610 	0x1610
+
 #endif /* _LINUX_PCI_IDS_H */
-- 
1.9.1

[PATCH 3/3] PCI: generic: Fix the bug of pci_fixup_irqs() for arm64 platform.

[Dongdong Liu]

arch/arm64/pci.c pcibios_alloc_irq() has the same function as
pci_fixup_irqs(), so we add condition #ifndef CONFIG_ARM64 for
pci_fixup_irqs().

Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
Reviewed-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
Reviewed-by: Zhou Wang <wangzhou1@hisilicon.com>
---
 drivers/pci/host/pci-host-common.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/pci/host/pci-host-common.c b/drivers/pci/host/pci-host-common.c
index e3c48b5..f160afc 100644
--- a/drivers/pci/host/pci-host-common.c
+++ b/drivers/pci/host/pci-host-common.c
@@ -145,7 +145,9 @@ int pci_host_common_probe(struct platform_device *pdev,
 		return -ENODEV;
 	}
 
+#ifndef CONFIG_ARM64
 	pci_fixup_irqs(pci_common_swizzle, of_irq_parse_and_map_pci);
+#endif
 
 	/*
 	 * We insert PCI resources into the iomem_resource and
-- 
1.9.1

Re: [PATCH 0/3] PCI:hisi: Add DT almost ECAM support for HiSilicon Hip06/Hip07 host controllers

[Dongdong Liu]

Hi Bjorn

Please look at this patchset if you have time.

Thanks,
Dongdong

在 2017/1/12 14:28, Dongdong Liu 写道:
> The PCIe controller in Hip06/Hip07 SoCs is not completely
> ECAM-compliant. It is non-ECAM only for the RC bus config space; for
> any other bus underneath the root bus it does support ECAM access.
>
> This series contains
> 1. Add the almost ECAM support in DT way.
> 2. Add quirk to set root port pdev->no_msi = 1.
> 3. Add condition #ifndef CONFIG_ARM64 for pci_fixup_irqs.
>
> This patchset is based on 4.10-rc3.
>
> Dongdong Liu (3):
>   PCI:hisi: Add DT almost ECAM support for HiSilicon Hip06/Hip07 host
>     controllers
>   PCI: Set pdev->no_msi=1 for HiSilicon Hip06/Hip07 host controllers
>   PCI: generic: Fix the bug of pci_fixup_irqs() for arm64 platform.
>
>  .../devicetree/bindings/pci/hisilicon-pcie.txt     | 37 +++++++++++++++
>  drivers/pci/host/pci-host-common.c                 |  2 +
>  drivers/pci/host/pcie-hisi.c                       | 54 ++++++++++++++++++++--
>  drivers/pci/quirks.c                               |  1 +
>  include/linux/pci_ids.h                            |  3 ++
>  5 files changed, 94 insertions(+), 3 deletions(-)
>

Re: [PATCH 1/3] PCI:hisi: Add DT almost ECAM support for HiSilicon Hip06/Hip07 host controllers

[Bjorn Helgaas]

On Thu, Jan 12, 2017 at 02:28:22PM +0800, Dongdong Liu wrote:
> The PCIe controller in Hip06/Hip07 SoCs is not completely
> ECAM-compliant. It is non-ECAM only for the RC bus config space; for
> any other bus underneath the root bus it does support ECAM access.
> This is to add the almost ECAM support in DT way.
> 
> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
> Reviewed-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
> Reviewed-by: Zhou Wang <wangzhou1@hisilicon.com>

Applied to pci/host-hisi for v4.11, thanks!

> ---
>  .../devicetree/bindings/pci/hisilicon-pcie.txt     | 37 +++++++++++++++
>  drivers/pci/host/pcie-hisi.c                       | 54 ++++++++++++++++++++--
>  2 files changed, 88 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt b/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
> index 59c2f47..38e6dc3 100644
> --- a/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
> @@ -42,3 +42,40 @@ Hip05 Example (note that Hip06 is the same except compatible):
>  				 0x0 0 0 4 &mbigen_pcie 4 13>;
>  		status = "ok";
>  	};
> +
> +HiSilicon Hip06/Hip07 PCIe host bridge DT (almost ecam) description
> +The properties and their meanings are identical to those described in
> +host-generic-pci.txt except as listed below.
> +
> +Properties of the host controller node that differ from
> +host-generic-pci.txt:
> +
> +- compatible     : Must be "hisilicon,pcie-almost-ecam"
> +
> +- reg            : Two entries: First the ECAM configuration space for any
> +		   other bus underneath the root bus. Second, the base
> +		   and size of the HiSilicon host bridge registers inculde
> +		   the RC itself config space.
> +
> +Example:
> +	pcie0: pcie@a0090000 {
> +		compatible = "hisilicon,pcie-almost-ecam";
> +		reg = <0 0xb0000000 0 0x2000000>,  /*  ECAM configuration space */
> +		      <0 0xa0090000 0 0x10000>; /* host bridge registers */
> +		bus-range = <0  31>;
> +		msi-map = <0x0000 &its_dsa 0x0000 0x2000>;
> +		msi-map-mask = <0xffff>;
> +		#address-cells = <3>;
> +		#size-cells = <2>;
> +		device_type = "pci";
> +		dma-coherent;
> +		ranges = <0x02000000 0 0xb2000000 0x0 0xb2000000 0 0x5ff0000
> +			  0x01000000 0 0 0 0xb7ff0000 0 0x10000>;
> +		#interrupt-cells = <1>;
> +		interrupt-map-mask = <0xf800 0 0 7>;
> +		interrupt-map = <0x0 0 0 1 &mbigen_pcie0 650 4
> +				 0x0 0 0 2 &mbigen_pcie0 650 4
> +				 0x0 0 0 3 &mbigen_pcie0 650 4
> +				 0x0 0 0 4 &mbigen_pcie0 650 4>;
> +		status = "ok";
> +	};
> diff --git a/drivers/pci/host/pcie-hisi.c b/drivers/pci/host/pcie-hisi.c
> index a301a71..f37f5a2 100644
> --- a/drivers/pci/host/pcie-hisi.c
> +++ b/drivers/pci/host/pcie-hisi.c
> @@ -24,7 +24,7 @@
>  #include <linux/regmap.h>
>  #include "../pci.h"
>  
> -#if defined(CONFIG_ACPI) && defined(CONFIG_PCI_QUIRKS)
> +#if defined(CONFIG_PCI_HISI) || (defined(CONFIG_ACPI) && defined(CONFIG_PCI_QUIRKS))
>  
>  static int hisi_pcie_acpi_rd_conf(struct pci_bus *bus, u32 devfn, int where,
>  				  int size, u32 *val)
> @@ -74,6 +74,8 @@ static void __iomem *hisi_pcie_map_bus(struct pci_bus *bus, unsigned int devfn,
>  		return pci_ecam_map_bus(bus, devfn, where);
>  }
>  
> +#if defined(CONFIG_ACPI) && defined(CONFIG_PCI_QUIRKS)
> +
>  static int hisi_pcie_init(struct pci_config_window *cfg)
>  {
>  	struct device *dev = cfg->parent;
> @@ -262,17 +264,23 @@ static int hisi_pcie_probe(struct platform_device *pdev)
>  	const struct of_device_id *match;
>  	struct resource *reg;
>  	struct device_driver *driver;
> +	struct pci_ecam_ops *ops;
>  	int ret;
>  
> +	driver = dev->driver;
> +	match = of_match_device(driver->of_match_table, dev);
> +	if (!strcmp(match->compatible, "hisilicon,pcie-almost-ecam")) {
> +		ops = (struct pci_ecam_ops *)match->data;
> +		return pci_host_common_probe(pdev, ops);
> +	}
> +
>  	hisi_pcie = devm_kzalloc(dev, sizeof(*hisi_pcie), GFP_KERNEL);
>  	if (!hisi_pcie)
>  		return -ENOMEM;
>  
>  	pp = &hisi_pcie->pp;
>  	pp->dev = dev;
> -	driver = dev->driver;
>  
> -	match = of_match_device(driver->of_match_table, dev);
>  	hisi_pcie->soc_ops = (struct pcie_soc_ops *) match->data;
>  
>  	hisi_pcie->subctrl =
> @@ -302,6 +310,40 @@ static int hisi_pcie_probe(struct platform_device *pdev)
>  		&hisi_pcie_link_up_hip06
>  };
>  
> +static int hisi_pcie_platform_init(struct pci_config_window *cfg)
> +{
> +	struct device *dev = cfg->parent;
> +	struct platform_device *pdev = to_platform_device(dev);
> +	struct resource *res;
> +	void __iomem *reg_base;
> +
> +	if (!dev->of_node)
> +		return -EINVAL;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> +	if (!res) {
> +		dev_err(dev, "missing \"reg[1]\"property\n");
> +		return -EINVAL;
> +	}
> +
> +	reg_base = devm_ioremap(dev, res->start, resource_size(res));
> +	if (!reg_base)
> +		return -ENOMEM;
> +
> +	cfg->priv = reg_base;
> +	return 0;
> +}
> +
> +struct pci_ecam_ops hisi_pcie_platform_ops = {
> +	.bus_shift    = 20,
> +	.init         =  hisi_pcie_platform_init,
> +	.pci_ops      = {
> +		.map_bus    = hisi_pcie_map_bus,
> +		.read       = hisi_pcie_acpi_rd_conf,
> +		.write      = hisi_pcie_acpi_wr_conf,
> +	}
> +};
> +
>  static const struct of_device_id hisi_pcie_of_match[] = {
>  	{
>  			.compatible = "hisilicon,hip05-pcie",
> @@ -311,6 +353,11 @@ static int hisi_pcie_probe(struct platform_device *pdev)
>  			.compatible = "hisilicon,hip06-pcie",
>  			.data	    = (void *) &hip06_ops,
>  	},
> +
> +	{
> +			.compatible = "hisilicon,pcie-almost-ecam",
> +			.data	    = (void *) &hisi_pcie_platform_ops,
> +	},
>  	{},
>  };
>  
> @@ -324,3 +371,4 @@ static int hisi_pcie_probe(struct platform_device *pdev)
>  builtin_platform_driver(hisi_pcie_driver);
>  
>  #endif
> +#endif
> -- 
> 1.9.1
> 

Re: [PATCH 2/3] PCI: Set pdev->no_msi=1 for HiSilicon Hip06/Hip07 host controllers

[Bjorn Helgaas]

On Thu, Jan 12, 2017 at 02:28:23PM +0800, Dongdong Liu wrote:
> The PCIe root port in Hip06/Hip07 SoCs does not support MSI/MSI-X,
> it can only transfer MSI/MSI-X from EP, so we add the quirk to
> set root port pdev->no_msi = 1.
> 
> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
> Reviewed-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
> Reviewed-by: Zhou Wang <wangzhou1@hisilicon.com>

Applied as follows to pci/host-hisi for v4.11.    I removed the device
ID, since we don't add those to pci_ids.h unless they're used in more
than one place.  I also reworded the changelog; let me know if I
didn't understand it correctly.

commit c2f8051a8a0c7ea9e93d80e484948cab583b7605
Author: Dongdong Liu <liudongdong3@huawei.com>
Date:   Thu Jan 12 14:28:23 2017 +0800

    PCI: Disable MSI for HiSilicon Hip06/Hip07 Root Ports
    
    The PCIe Root Port in Hip06/Hip07 SoCs can transfer MSI/MSI-X from
    downstream devices, but does not support MSI/MSI-X itself.
    
    Add a quirk to prevent use of MSI/MSI-X by the Root Port.
    
    [bhelgaas: changelog, sort vendor ID #define, drop device ID #define]
    Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
    Reviewed-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
    Reviewed-by: Zhou Wang <wangzhou1@hisilicon.com>

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 1800befa8b8b..c49ac99bda4b 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -1634,6 +1634,7 @@ static void quirk_pcie_mch(struct pci_dev *pdev)
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_E7520_MCH,	quirk_pcie_mch);
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_E7320_MCH,	quirk_pcie_mch);
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_E7525_MCH,	quirk_pcie_mch);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_HUAWEI,	0x1610,	quirk_pcie_mch);
 
 
 /*
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index 73dda0edcb97..a4f77feecbb0 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -2516,6 +2516,8 @@
 #define PCI_DEVICE_ID_KORENIX_JETCARDF2	0x1700
 #define PCI_DEVICE_ID_KORENIX_JETCARDF3	0x17ff
 
+#define PCI_VENDOR_ID_HUAWEI         	0x19e5
+
 #define PCI_VENDOR_ID_NETRONOME		0x19ee
 #define PCI_DEVICE_ID_NETRONOME_NFP3200	0x3200
 #define PCI_DEVICE_ID_NETRONOME_NFP3240	0x3240

> ---
>  drivers/pci/quirks.c    | 1 +
>  include/linux/pci_ids.h | 3 +++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 1800bef..20cbdae 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -1634,6 +1634,7 @@ static void quirk_pcie_mch(struct pci_dev *pdev)
>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_E7520_MCH,	quirk_pcie_mch);
>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_E7320_MCH,	quirk_pcie_mch);
>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_E7525_MCH,	quirk_pcie_mch);
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_HUAWEI,	PCI_DEVICE_ID_HISILICON_1610,	quirk_pcie_mch);
>  
>  
>  /*
> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> index 73dda0e..9cc4720 100644
> --- a/include/linux/pci_ids.h
> +++ b/include/linux/pci_ids.h
> @@ -3054,4 +3054,7 @@
>  
>  #define PCI_VENDOR_ID_OCZ		0x1b85
>  
> +#define PCI_VENDOR_ID_HUAWEI         	0x19e5
> +#define PCI_DEVICE_ID_HISILICON_1610 	0x1610
> +
>  #endif /* _LINUX_PCI_IDS_H */
> -- 
> 1.9.1
> 

Re: [PATCH 3/3] PCI: generic: Fix the bug of pci_fixup_irqs() for arm64 platform.

[Bjorn Helgaas]

[+cc Tomasz, Lorenzo]

On Thu, Jan 12, 2017 at 02:28:24PM +0800, Dongdong Liu wrote:
> arch/arm64/pci.c pcibios_alloc_irq() has the same function as
> pci_fixup_irqs(), so we add condition #ifndef CONFIG_ARM64 for
> pci_fixup_irqs().
> 
> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
> Reviewed-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
> Reviewed-by: Zhou Wang <wangzhou1@hisilicon.com>
> ---
>  drivers/pci/host/pci-host-common.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/pci/host/pci-host-common.c b/drivers/pci/host/pci-host-common.c
> index e3c48b5..f160afc 100644
> --- a/drivers/pci/host/pci-host-common.c
> +++ b/drivers/pci/host/pci-host-common.c
> @@ -145,7 +145,9 @@ int pci_host_common_probe(struct platform_device *pdev,
>  		return -ENODEV;
>  	}
>  
> +#ifndef CONFIG_ARM64
>  	pci_fixup_irqs(pci_common_swizzle, of_irq_parse_and_map_pci);
> +#endif

d8ed75d59332 ("ARM64: PCI: ACPI support for legacy IRQs parsing and
consolidation with DT code") added pcibios_alloc_irq() for arm64.

arm64 is the only arch that implements pcibios_alloc_irq().  And now
you want to add an ifdef here so arm64 is the only arch that doesn't
call pci_fixup_irqs().

I don't remember the details of why arm64 is so special here.
Obviously we'd prefer not to have the ifdef and not to have the
arm64-specific pcibios_alloc_irq().

>  	/*
>  	 * We insert PCI resources into the iomem_resource and
> -- 
> 1.9.1
> 

Re: [PATCH 1/3] PCI:hisi: Add DT almost ECAM support for HiSilicon Hip06/Hip07 host controllers

[Bjorn Helgaas]

On Fri, Feb 03, 2017 at 02:40:42PM -0600, Bjorn Helgaas wrote:
> On Thu, Jan 12, 2017 at 02:28:22PM +0800, Dongdong Liu wrote:
> > The PCIe controller in Hip06/Hip07 SoCs is not completely
> > ECAM-compliant. It is non-ECAM only for the RC bus config space; for
> > any other bus underneath the root bus it does support ECAM access.
> > This is to add the almost ECAM support in DT way.
> > 
> > Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
> > Reviewed-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
> > Reviewed-by: Zhou Wang <wangzhou1@hisilicon.com>
> 
> Applied to pci/host-hisi for v4.11, thanks!

Oops, I take this back, see below.

> > @@ -262,17 +264,23 @@ static int hisi_pcie_probe(struct platform_device *pdev)
> >  	const struct of_device_id *match;
> >  	struct resource *reg;
> >  	struct device_driver *driver;
> > +	struct pci_ecam_ops *ops;
> >  	int ret;
> >  
> > +	driver = dev->driver;
> > +	match = of_match_device(driver->of_match_table, dev);
> > +	if (!strcmp(match->compatible, "hisilicon,pcie-almost-ecam")) {
> > +		ops = (struct pci_ecam_ops *)match->data;
> > +		return pci_host_common_probe(pdev, ops);
> > +	}

Please make this a separate probe function() with a separate struct
platform_driver.

That way you won't have to strcmp() for "hisilicon,pcie-almost-ecam",
and you can use of_device_get_match_data() to get "ops", so you won't
need to use of_match_device() at all.

> >  	hisi_pcie = devm_kzalloc(dev, sizeof(*hisi_pcie), GFP_KERNEL);
> >  	if (!hisi_pcie)
> >  		return -ENOMEM;
> >  
> >  	pp = &hisi_pcie->pp;
> >  	pp->dev = dev;
> > -	driver = dev->driver;
> >  
> > -	match = of_match_device(driver->of_match_table, dev);
> >  	hisi_pcie->soc_ops = (struct pcie_soc_ops *) match->data;
> >  
> >  	hisi_pcie->subctrl =
> > @@ -302,6 +310,40 @@ static int hisi_pcie_probe(struct platform_device *pdev)
> >  		&hisi_pcie_link_up_hip06
> >  };
> >  
> > +static int hisi_pcie_platform_init(struct pci_config_window *cfg)
> > +{
> > +	struct device *dev = cfg->parent;
> > +	struct platform_device *pdev = to_platform_device(dev);
> > +	struct resource *res;
> > +	void __iomem *reg_base;
> > +
> > +	if (!dev->of_node)
> > +		return -EINVAL;
> > +
> > +	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> > +	if (!res) {
> > +		dev_err(dev, "missing \"reg[1]\"property\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	reg_base = devm_ioremap(dev, res->start, resource_size(res));
> > +	if (!reg_base)
> > +		return -ENOMEM;
> > +
> > +	cfg->priv = reg_base;
> > +	return 0;
> > +}
> > +
> > +struct pci_ecam_ops hisi_pcie_platform_ops = {
> > +	.bus_shift    = 20,
> > +	.init         =  hisi_pcie_platform_init,
> > +	.pci_ops      = {
> > +		.map_bus    = hisi_pcie_map_bus,
> > +		.read       = hisi_pcie_acpi_rd_conf,
> > +		.write      = hisi_pcie_acpi_wr_conf,
> > +	}
> > +};
> > +
> >  static const struct of_device_id hisi_pcie_of_match[] = {
> >  	{
> >  			.compatible = "hisilicon,hip05-pcie",
> > @@ -311,6 +353,11 @@ static int hisi_pcie_probe(struct platform_device *pdev)
> >  			.compatible = "hisilicon,hip06-pcie",
> >  			.data	    = (void *) &hip06_ops,
> >  	},
> > +
> > +	{
> > +			.compatible = "hisilicon,pcie-almost-ecam",
> > +			.data	    = (void *) &hisi_pcie_platform_ops,
> > +	},
> >  	{},
> >  };
> >  
> > @@ -324,3 +371,4 @@ static int hisi_pcie_probe(struct platform_device *pdev)
> >  builtin_platform_driver(hisi_pcie_driver);
> >  
> >  #endif
> > +#endif
> > -- 
> > 1.9.1
> > 

Re: [PATCH 0/3] PCI:hisi: Add DT almost ECAM support for HiSilicon Hip06/Hip07 host controllers

[Bjorn Helgaas]

On Thu, Jan 12, 2017 at 02:28:21PM +0800, Dongdong Liu wrote:
> The PCIe controller in Hip06/Hip07 SoCs is not completely
> ECAM-compliant. It is non-ECAM only for the RC bus config space; for
> any other bus underneath the root bus it does support ECAM access.
> 
> This series contains
> 1. Add the almost ECAM support in DT way. 
> 2. Add quirk to set root port pdev->no_msi = 1. 
> 3. Add condition #ifndef CONFIG_ARM64 for pci_fixup_irqs.
> 
> This patchset is based on 4.10-rc3.
> 
> Dongdong Liu (3):
>   PCI:hisi: Add DT almost ECAM support for HiSilicon Hip06/Hip07 host
>     controllers
>   PCI: Set pdev->no_msi=1 for HiSilicon Hip06/Hip07 host controllers
>   PCI: generic: Fix the bug of pci_fixup_irqs() for arm64 platform.
> 
>  .../devicetree/bindings/pci/hisilicon-pcie.txt     | 37 +++++++++++++++
>  drivers/pci/host/pci-host-common.c                 |  2 +
>  drivers/pci/host/pcie-hisi.c                       | 54 ++++++++++++++++++++--
>  drivers/pci/quirks.c                               |  1 +
>  include/linux/pci_ids.h                            |  3 ++
>  5 files changed, 94 insertions(+), 3 deletions(-)

Current state: I applied the "no MSI" patch on pci/host-hisi and
dropped the other two for now.  If you could update those and repost
them, that'd be great.

Re: [PATCH 1/3] PCI:hisi: Add DT almost ECAM support for HiSilicon Hip06/Hip07 host controllers

[Dongdong Liu]

Hi Bjorn

Many thanks for your review.

e\x1c( 2017/2/4 5:00, Bjorn Helgaas e\x06\x19i\x01\x13:
> On Fri, Feb 03, 2017 at 02:40:42PM -0600, Bjorn Helgaas wrote:
>> On Thu, Jan 12, 2017 at 02:28:22PM +0800, Dongdong Liu wrote:
>>> The PCIe controller in Hip06/Hip07 SoCs is not completely
>>> ECAM-compliant. It is non-ECAM only for the RC bus config space; for
>>> any other bus underneath the root bus it does support ECAM access.
>>> This is to add the almost ECAM support in DT way.
>>>
>>> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
>>> Reviewed-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
>>> Reviewed-by: Zhou Wang <wangzhou1@hisilicon.com>
>>
>> Applied to pci/host-hisi for v4.11, thanks!
>
> Oops, I take this back, see below.
>
>>> @@ -262,17 +264,23 @@ static int hisi_pcie_probe(struct platform_device *pdev)
>>>  	const struct of_device_id *match;
>>>  	struct resource *reg;
>>>  	struct device_driver *driver;
>>> +	struct pci_ecam_ops *ops;
>>>  	int ret;
>>>
>>> +	driver = dev->driver;
>>> +	match = of_match_device(driver->of_match_table, dev);
>>> +	if (!strcmp(match->compatible, "hisilicon,pcie-almost-ecam")) {
>>> +		ops = (struct pci_ecam_ops *)match->data;
>>> +		return pci_host_common_probe(pdev, ops);
>>> +	}
>
> Please make this a separate probe function() with a separate struct
> platform_driver.
>
> That way you won't have to strcmp() for "hisilicon,pcie-almost-ecam",
> and you can use of_device_get_match_data() to get "ops", so you won't
> need to use of_match_device() at all.

Good catch, will fix.

Thanks,
Dongdong

Re: [PATCH 2/3] PCI: Set pdev->no_msi=1 for HiSilicon Hip06/Hip07 host controllers

[Dongdong Liu]

Hi Bjorn

在 2017/2/4 4:42, Bjorn Helgaas 写道:
> On Thu, Jan 12, 2017 at 02:28:23PM +0800, Dongdong Liu wrote:
>> The PCIe root port in Hip06/Hip07 SoCs does not support MSI/MSI-X,
>> it can only transfer MSI/MSI-X from EP, so we add the quirk to
>> set root port pdev->no_msi = 1.
>>
>> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
>> Reviewed-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
>> Reviewed-by: Zhou Wang <wangzhou1@hisilicon.com>
>
> Applied as follows to pci/host-hisi for v4.11.    I removed the device
> ID, since we don't add those to pci_ids.h unless they're used in more
> than one place.  I also reworded the changelog; let me know if I
> didn't understand it correctly.
>
> commit c2f8051a8a0c7ea9e93d80e484948cab583b7605
> Author: Dongdong Liu <liudongdong3@huawei.com>
> Date:   Thu Jan 12 14:28:23 2017 +0800
>
>     PCI: Disable MSI for HiSilicon Hip06/Hip07 Root Ports
>
>     The PCIe Root Port in Hip06/Hip07 SoCs can transfer MSI/MSI-X from
>     downstream devices, but does not support MSI/MSI-X itself.
>
>     Add a quirk to prevent use of MSI/MSI-X by the Root Port.
>
>     [bhelgaas: changelog, sort vendor ID #define, drop device ID #define]
>     Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
>     Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
>     Reviewed-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
>     Reviewed-by: Zhou Wang <wangzhou1@hisilicon.com>
>
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 1800befa8b8b..c49ac99bda4b 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -1634,6 +1634,7 @@ static void quirk_pcie_mch(struct pci_dev *pdev)
>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_E7520_MCH,	quirk_pcie_mch);
>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_E7320_MCH,	quirk_pcie_mch);
>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_E7525_MCH,	quirk_pcie_mch);
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_HUAWEI,	0x1610,	quirk_pcie_mch);
>

This looks good to me, thanks for your work on this patch.

Thanks,
Dongdong
>
>  /*
> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> index 73dda0edcb97..a4f77feecbb0 100644
> --- a/include/linux/pci_ids.h
> +++ b/include/linux/pci_ids.h
> @@ -2516,6 +2516,8 @@
>  #define PCI_DEVICE_ID_KORENIX_JETCARDF2	0x1700
>  #define PCI_DEVICE_ID_KORENIX_JETCARDF3	0x17ff
>
> +#define PCI_VENDOR_ID_HUAWEI         	0x19e5
> +
>  #define PCI_VENDOR_ID_NETRONOME		0x19ee
>  #define PCI_DEVICE_ID_NETRONOME_NFP3200	0x3200
>  #define PCI_DEVICE_ID_NETRONOME_NFP3240	0x3240
>
>> ---
>>  drivers/pci/quirks.c    | 1 +
>>  include/linux/pci_ids.h | 3 +++
>>  2 files changed, 4 insertions(+)
>>
>> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
>> index 1800bef..20cbdae 100644
>> --- a/drivers/pci/quirks.c
>> +++ b/drivers/pci/quirks.c
>> @@ -1634,6 +1634,7 @@ static void quirk_pcie_mch(struct pci_dev *pdev)
>>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_E7520_MCH,	quirk_pcie_mch);
>>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_E7320_MCH,	quirk_pcie_mch);
>>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_E7525_MCH,	quirk_pcie_mch);
>> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_HUAWEI,	PCI_DEVICE_ID_HISILICON_1610,	quirk_pcie_mch);
>>
>>
>>  /*
>> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
>> index 73dda0e..9cc4720 100644
>> --- a/include/linux/pci_ids.h
>> +++ b/include/linux/pci_ids.h
>> @@ -3054,4 +3054,7 @@
>>
>>  #define PCI_VENDOR_ID_OCZ		0x1b85
>>
>> +#define PCI_VENDOR_ID_HUAWEI         	0x19e5
>> +#define PCI_DEVICE_ID_HISILICON_1610 	0x1610
>> +
>>  #endif /* _LINUX_PCI_IDS_H */
>> --
>> 1.9.1
>>
>
> .
>

Re: [PATCH 2/3] PCI: Set pdev->no_msi=1 for HiSilicon Hip06/Hip07 host controllers

[Bjorn Helgaas]

On Sat, Feb 04, 2017 at 11:37:14AM +0800, Dongdong Liu wrote:
> Hi Bjorn
> 
> 在 2017/2/4 4:42, Bjorn Helgaas 写道:
> >On Thu, Jan 12, 2017 at 02:28:23PM +0800, Dongdong Liu wrote:
> >>The PCIe root port in Hip06/Hip07 SoCs does not support MSI/MSI-X,
> >>it can only transfer MSI/MSI-X from EP, so we add the quirk to
> >>set root port pdev->no_msi = 1.
> >>
> >>Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
> >>Reviewed-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
> >>Reviewed-by: Zhou Wang <wangzhou1@hisilicon.com>
> >
> >Applied as follows to pci/host-hisi for v4.11.    I removed the device
> >ID, since we don't add those to pci_ids.h unless they're used in more
> >than one place.  I also reworded the changelog; let me know if I
> >didn't understand it correctly.
> >
> >commit c2f8051a8a0c7ea9e93d80e484948cab583b7605
> >Author: Dongdong Liu <liudongdong3@huawei.com>
> >Date:   Thu Jan 12 14:28:23 2017 +0800
> >
> >    PCI: Disable MSI for HiSilicon Hip06/Hip07 Root Ports
> >
> >    The PCIe Root Port in Hip06/Hip07 SoCs can transfer MSI/MSI-X from
> >    downstream devices, but does not support MSI/MSI-X itself.
> >
> >    Add a quirk to prevent use of MSI/MSI-X by the Root Port.
> >
> >    [bhelgaas: changelog, sort vendor ID #define, drop device ID #define]
> >    Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
> >    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> >    Reviewed-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
> >    Reviewed-by: Zhou Wang <wangzhou1@hisilicon.com>
> >
> >diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> >index 1800befa8b8b..c49ac99bda4b 100644
> >--- a/drivers/pci/quirks.c
> >+++ b/drivers/pci/quirks.c
> >@@ -1634,6 +1634,7 @@ static void quirk_pcie_mch(struct pci_dev *pdev)
> > DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_E7520_MCH,	quirk_pcie_mch);
> > DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_E7320_MCH,	quirk_pcie_mch);
> > DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_E7525_MCH,	quirk_pcie_mch);
> >+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_HUAWEI,	0x1610,	quirk_pcie_mch);
> >
> 
> This looks good to me, thanks for your work on this patch.

Just to confirm, is the problem that this device (PCI_VENDOR_ID_HUAWEI
0x1610) advertises an MSI or MSI-X capability, but MSI/MSI-X doesn't
work?  If it doesn't advertise a capability, I don't think we should
try to use MSI, so we wouldn't need the quirk.

Bjorn

Re: [PATCH 2/3] PCI: Set pdev->no_msi=1 for HiSilicon Hip06/Hip07 host controllers

[Dongdong Liu]

在 2017/2/5 8:38, Bjorn Helgaas 写道:
> On Sat, Feb 04, 2017 at 11:37:14AM +0800, Dongdong Liu wrote:
>> Hi Bjorn
>>
>> 在 2017/2/4 4:42, Bjorn Helgaas 写道:
>>> On Thu, Jan 12, 2017 at 02:28:23PM +0800, Dongdong Liu wrote:
>>>> The PCIe root port in Hip06/Hip07 SoCs does not support MSI/MSI-X,
>>>> it can only transfer MSI/MSI-X from EP, so we add the quirk to
>>>> set root port pdev->no_msi = 1.
>>>>
>>>> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
>>>> Reviewed-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
>>>> Reviewed-by: Zhou Wang <wangzhou1@hisilicon.com>
>>>
>>> Applied as follows to pci/host-hisi for v4.11.    I removed the device
>>> ID, since we don't add those to pci_ids.h unless they're used in more
>>> than one place.  I also reworded the changelog; let me know if I
>>> didn't understand it correctly.
>>>
>>> commit c2f8051a8a0c7ea9e93d80e484948cab583b7605
>>> Author: Dongdong Liu <liudongdong3@huawei.com>
>>> Date:   Thu Jan 12 14:28:23 2017 +0800
>>>
>>>    PCI: Disable MSI for HiSilicon Hip06/Hip07 Root Ports
>>>
>>>    The PCIe Root Port in Hip06/Hip07 SoCs can transfer MSI/MSI-X from
>>>    downstream devices, but does not support MSI/MSI-X itself.
>>>
>>>    Add a quirk to prevent use of MSI/MSI-X by the Root Port.
>>>
>>>    [bhelgaas: changelog, sort vendor ID #define, drop device ID #define]
>>>    Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
>>>    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
>>>    Reviewed-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
>>>    Reviewed-by: Zhou Wang <wangzhou1@hisilicon.com>
>>>
>>> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
>>> index 1800befa8b8b..c49ac99bda4b 100644
>>> --- a/drivers/pci/quirks.c
>>> +++ b/drivers/pci/quirks.c
>>> @@ -1634,6 +1634,7 @@ static void quirk_pcie_mch(struct pci_dev *pdev)
>>> DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_E7520_MCH,	quirk_pcie_mch);
>>> DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_E7320_MCH,	quirk_pcie_mch);
>>> DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_E7525_MCH,	quirk_pcie_mch);
>>> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_HUAWEI,	0x1610,	quirk_pcie_mch);
>>>
>>
>> This looks good to me, thanks for your work on this patch.
>
> Just to confirm, is the problem that this device (PCI_VENDOR_ID_HUAWEI
> 0x1610) advertises an MSI or MSI-X capability, but MSI/MSI-X doesn't
> work?  If it doesn't advertise a capability, I don't think we should
> try to use MSI, so we wouldn't need the quirk.

Yes, It advertises a MSI capability but MSI/MSI-X doesn't work,so we need the quirk.

Thanks,
Dongdong

>
> Bjorn
>
> .
>

Re: [PATCH 3/3] PCI: generic: Fix the bug of pci_fixup_irqs() for arm64 platform.

[Lorenzo Pieralisi]

On Fri, Feb 03, 2017 at 02:45:30PM -0600, Bjorn Helgaas wrote:
> [+cc Tomasz, Lorenzo]
> 
> On Thu, Jan 12, 2017 at 02:28:24PM +0800, Dongdong Liu wrote:
> > arch/arm64/pci.c pcibios_alloc_irq() has the same function as
> > pci_fixup_irqs(), so we add condition #ifndef CONFIG_ARM64 for
> > pci_fixup_irqs().
> > 
> > Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
> > Reviewed-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
> > Reviewed-by: Zhou Wang <wangzhou1@hisilicon.com>
> > ---
> >  drivers/pci/host/pci-host-common.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/pci/host/pci-host-common.c b/drivers/pci/host/pci-host-common.c
> > index e3c48b5..f160afc 100644
> > --- a/drivers/pci/host/pci-host-common.c
> > +++ b/drivers/pci/host/pci-host-common.c
> > @@ -145,7 +145,9 @@ int pci_host_common_probe(struct platform_device *pdev,
> >  		return -ENODEV;
> >  	}
> >  
> > +#ifndef CONFIG_ARM64
> >  	pci_fixup_irqs(pci_common_swizzle, of_irq_parse_and_map_pci);
> > +#endif
> 
> d8ed75d59332 ("ARM64: PCI: ACPI support for legacy IRQs parsing and
> consolidation with DT code") added pcibios_alloc_irq() for arm64.
> 
> arm64 is the only arch that implements pcibios_alloc_irq().  And now
> you want to add an ifdef here so arm64 is the only arch that doesn't
> call pci_fixup_irqs().
> 
> I don't remember the details of why arm64 is so special here.
> Obviously we'd prefer not to have the ifdef and not to have the
> arm64-specific pcibios_alloc_irq().

Well, I am not sure ARM64 is more special than other architectures
in this respect, actually I think that what ARM64 does is what we
will end up doing when Matthew Minter's patches are merged and
that's my aim for v4.12, pci_fixup_irqs() should not be used
any longer, at least on ARM/ARM64 and I know what to do to make
it disappear.

ARM64 relies on pcibios_alloc_irq() because of probe sequence. Before
d8ed75d59332 ("ARM64: PCI: ACPI support for legacy IRQs parsing and
consolidation with DT code") legacy IRQs were assigned at
pcibios_add_device() time. This does not work at all on ACPI (ie scan
handlers dependency), so to have a single callback DT/ACPI we
moved the allocation at pcibios_alloc_irq() time and I suspect
that's where (in pci_device_probe()) the host bridge hook to
init legacy IRQs will be called when we manage to merge Matt's
code. I will send you Matt's code rebased for v4.12, I take an
action on that (except for x86 code that IIRC is unfathomable
from this perspective and that's what blocked Matt's attempt).

Now this patch. Yes, using pci_fixup_irqs() is wrong on ARM64
(because we may reallocate an IRQ for a device that is already
bound to a driver - but that's a problem on ARM too - ie multiple
host controllers), the call has to be there for legacy IRQs to "work"
on ARM though so the ifdef.

How are we fixing this ? We merge this patch and I will remove
this stuff completely for v4.12 by using Matt's approach (ie moving
the legacy IRQ allocation to a per-host bridge callback ?)

Thanks,
Lorenzo

Re: [PATCH 3/3] PCI: generic: Fix the bug of pci_fixup_irqs() for arm64 platform.

[Bjorn Helgaas]

On Mon, Feb 06, 2017 at 02:08:27PM +0000, Lorenzo Pieralisi wrote:
> On Fri, Feb 03, 2017 at 02:45:30PM -0600, Bjorn Helgaas wrote:
> > [+cc Tomasz, Lorenzo]
> > 
> > On Thu, Jan 12, 2017 at 02:28:24PM +0800, Dongdong Liu wrote:
> > > arch/arm64/pci.c pcibios_alloc_irq() has the same function as
> > > pci_fixup_irqs(), so we add condition #ifndef CONFIG_ARM64 for
> > > pci_fixup_irqs().
> > > 
> > > Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
> > > Reviewed-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
> > > Reviewed-by: Zhou Wang <wangzhou1@hisilicon.com>
> > > ---
> > >  drivers/pci/host/pci-host-common.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/drivers/pci/host/pci-host-common.c b/drivers/pci/host/pci-host-common.c
> > > index e3c48b5..f160afc 100644
> > > --- a/drivers/pci/host/pci-host-common.c
> > > +++ b/drivers/pci/host/pci-host-common.c
> > > @@ -145,7 +145,9 @@ int pci_host_common_probe(struct platform_device *pdev,
> > >  		return -ENODEV;
> > >  	}
> > >  
> > > +#ifndef CONFIG_ARM64
> > >  	pci_fixup_irqs(pci_common_swizzle, of_irq_parse_and_map_pci);
> > > +#endif
> > 
> > d8ed75d59332 ("ARM64: PCI: ACPI support for legacy IRQs parsing and
> > consolidation with DT code") added pcibios_alloc_irq() for arm64.
> > 
> > arm64 is the only arch that implements pcibios_alloc_irq().  And now
> > you want to add an ifdef here so arm64 is the only arch that doesn't
> > call pci_fixup_irqs().
> > 
> > I don't remember the details of why arm64 is so special here.
> > Obviously we'd prefer not to have the ifdef and not to have the
> > arm64-specific pcibios_alloc_irq().
> 
> Well, I am not sure ARM64 is more special than other architectures
> in this respect, actually I think that what ARM64 does is what we
> will end up doing when Matthew Minter's patches are merged and
> that's my aim for v4.12, pci_fixup_irqs() should not be used
> any longer, at least on ARM/ARM64 and I know what to do to make
> it disappear.

ARM64 is different from x86, even though both arches support both ACPI
and DT: on ARM64 we currently call of_irq_parse_and_map_pci() when we
call the driver's probe() method, whereas on x86 we don't call it
until the driver calls pci_enable_device().

Obviously it would be better if this were done at the same point on
both arches.  I don't know what that point *is*.  I could imagine
enumeration-time, probe-time, or enable-time.

OK, back to the patch at hand.  I think we're agreed that having
pci_host_common_probe() call pci_fixup_irqs() is wrong on all
architectures (it doesn't cover hot-added devices and it may clobber
an IRQ after a driver has claimed the device (since it "fixes" all PCI
devices).  And we hope to fix this in the next cycle using Matt's
work.

So I think I'm OK with this transient ugliness.  Although
pci-host-common.c is currently only available on ARM and ARM64, in
theory, it should be usable on x86.  I would probably use "#ifdef ARM"
instead of "#ifndef ARM64" so that when it is made available on x86,
we won't call pci_fixup_irqs() there.

What do you think of the following?


commit 4e3538fb84dcba2de5d5b0990ced7f3f881fda1e
Author: Dongdong Liu <liudongdong3@huawei.com>
Date:   Thu Jan 12 14:28:24 2017 +0800

    PCI: generic: Call pci_fixup_irqs() only on ARM
    
    pci_fixup_irqs() is problematic because:
    
      - it's called when we enumerate a host bridge, so we don't fixup IRQs for
        hot-added PCI devices, and
    
      - it fixes up IRQs for all PCI devices in the system, so if we call it
        multiple times, e.g., if we have several host controllers, we may
        reallocate an IRQ for a device after a driver has already claimed it.
    
    We plan to replace pci_fixup_irqs() soon, but we still need it on ARM
    because we don't have any other generic method for doing this.
    
    On ARM64, we don't need pci_fixup_irqs() because we do IRQ setup when we
    bind a driver to the device (in the pci_device_probe() ->
    pcibios_alloc_irq() path).
    
    pci-host-common.c is currently only used on ARM and ARM64.  In principle,
    it could be used on x86, and we wouldn't want pci_fixup_irqs() there
    either, because x86 does IRQ setup in the pci_enable_device() path.
    
    [bhelgaas: changelog, use #ifdef ARM, not #ifndef ARM64]
    Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
    Reviewed-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
    Reviewed-by: Zhou Wang <wangzhou1@hisilicon.com>

diff --git a/drivers/pci/host/pci-host-common.c b/drivers/pci/host/pci-host-common.c
index e3c48b5deb93..e9a53bae1c25 100644
--- a/drivers/pci/host/pci-host-common.c
+++ b/drivers/pci/host/pci-host-common.c
@@ -145,7 +145,9 @@ int pci_host_common_probe(struct platform_device *pdev,
 		return -ENODEV;
 	}
 
+#ifdef CONFIG_ARM
 	pci_fixup_irqs(pci_common_swizzle, of_irq_parse_and_map_pci);
+#endif
 
 	/*
 	 * We insert PCI resources into the iomem_resource and

Re: [PATCH 3/3] PCI: generic: Fix the bug of pci_fixup_irqs() for arm64 platform.

[Lorenzo Pieralisi]

On Tue, Feb 07, 2017 at 03:46:45PM -0600, Bjorn Helgaas wrote:
> On Mon, Feb 06, 2017 at 02:08:27PM +0000, Lorenzo Pieralisi wrote:
> > On Fri, Feb 03, 2017 at 02:45:30PM -0600, Bjorn Helgaas wrote:
> > > [+cc Tomasz, Lorenzo]
> > > 
> > > On Thu, Jan 12, 2017 at 02:28:24PM +0800, Dongdong Liu wrote:
> > > > arch/arm64/pci.c pcibios_alloc_irq() has the same function as
> > > > pci_fixup_irqs(), so we add condition #ifndef CONFIG_ARM64 for
> > > > pci_fixup_irqs().
> > > > 
> > > > Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
> > > > Reviewed-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
> > > > Reviewed-by: Zhou Wang <wangzhou1@hisilicon.com>
> > > > ---
> > > >  drivers/pci/host/pci-host-common.c | 2 ++
> > > >  1 file changed, 2 insertions(+)
> > > > 
> > > > diff --git a/drivers/pci/host/pci-host-common.c b/drivers/pci/host/pci-host-common.c
> > > > index e3c48b5..f160afc 100644
> > > > --- a/drivers/pci/host/pci-host-common.c
> > > > +++ b/drivers/pci/host/pci-host-common.c
> > > > @@ -145,7 +145,9 @@ int pci_host_common_probe(struct platform_device *pdev,
> > > >  		return -ENODEV;
> > > >  	}
> > > >  
> > > > +#ifndef CONFIG_ARM64
> > > >  	pci_fixup_irqs(pci_common_swizzle, of_irq_parse_and_map_pci);
> > > > +#endif
> > > 
> > > d8ed75d59332 ("ARM64: PCI: ACPI support for legacy IRQs parsing and
> > > consolidation with DT code") added pcibios_alloc_irq() for arm64.
> > > 
> > > arm64 is the only arch that implements pcibios_alloc_irq().  And now
> > > you want to add an ifdef here so arm64 is the only arch that doesn't
> > > call pci_fixup_irqs().
> > > 
> > > I don't remember the details of why arm64 is so special here.
> > > Obviously we'd prefer not to have the ifdef and not to have the
> > > arm64-specific pcibios_alloc_irq().
> > 
> > Well, I am not sure ARM64 is more special than other architectures
> > in this respect, actually I think that what ARM64 does is what we
> > will end up doing when Matthew Minter's patches are merged and
> > that's my aim for v4.12, pci_fixup_irqs() should not be used
> > any longer, at least on ARM/ARM64 and I know what to do to make
> > it disappear.
> 
> ARM64 is different from x86, even though both arches support both ACPI
> and DT: on ARM64 we currently call of_irq_parse_and_map_pci() when we
> call the driver's probe() method, whereas on x86 we don't call it
> until the driver calls pci_enable_device().

I can move the of_irq_parse_and_map_pci() call (along with the
acpi_pci_irq_enable() counterpart - that unfortunately has a
different behaviour because it _modifies_ the struct pci_dev*
pointer passed to it) at pci_enable_device() time, that's what
Matt's patchset was doing, the risk is always triggering
regressions by moving things around.

I suspect you want to get rid of pcibios_alloc_irq(), I will make
that part of the series I will post on top of Matt's code.

On top of that, through copy'n'paste pci_fixup_irqs() is spreading
like plague, it has to be removed, at least on all ARM host controllers.

> Obviously it would be better if this were done at the same point on
> both arches.  I don't know what that point *is*.  I could imagine
> enumeration-time, probe-time, or enable-time.
> 
> OK, back to the patch at hand.  I think we're agreed that having
> pci_host_common_probe() call pci_fixup_irqs() is wrong on all
> architectures (it doesn't cover hot-added devices and it may clobber
> an IRQ after a driver has claimed the device (since it "fixes" all PCI
> devices).  And we hope to fix this in the next cycle using Matt's
> work.
> 
> So I think I'm OK with this transient ugliness.  Although
> pci-host-common.c is currently only available on ARM and ARM64, in
> theory, it should be usable on x86.  I would probably use "#ifdef ARM"
> instead of "#ifndef ARM64" so that when it is made available on x86,
> we won't call pci_fixup_irqs() there.
> 
> What do you think of the following?
> 
> 
> commit 4e3538fb84dcba2de5d5b0990ced7f3f881fda1e
> Author: Dongdong Liu <liudongdong3@huawei.com>
> Date:   Thu Jan 12 14:28:24 2017 +0800
> 
>     PCI: generic: Call pci_fixup_irqs() only on ARM
>     
>     pci_fixup_irqs() is problematic because:
>     
>       - it's called when we enumerate a host bridge, so we don't fixup IRQs for
>         hot-added PCI devices, and
>     
>       - it fixes up IRQs for all PCI devices in the system, so if we call it
>         multiple times, e.g., if we have several host controllers, we may
>         reallocate an IRQ for a device after a driver has already claimed it.
>     
>     We plan to replace pci_fixup_irqs() soon, but we still need it on ARM
>     because we don't have any other generic method for doing this.
>     
>     On ARM64, we don't need pci_fixup_irqs() because we do IRQ setup when we
>     bind a driver to the device (in the pci_device_probe() ->
>     pcibios_alloc_irq() path).
>     
>     pci-host-common.c is currently only used on ARM and ARM64.  In principle,
>     it could be used on x86, and we wouldn't want pci_fixup_irqs() there
>     either, because x86 does IRQ setup in the pci_enable_device() path.
>     
>     [bhelgaas: changelog, use #ifdef ARM, not #ifndef ARM64]
>     Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
>     Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
>     Reviewed-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
>     Reviewed-by: Zhou Wang <wangzhou1@hisilicon.com>
> 
> diff --git a/drivers/pci/host/pci-host-common.c b/drivers/pci/host/pci-host-common.c
> index e3c48b5deb93..e9a53bae1c25 100644
> --- a/drivers/pci/host/pci-host-common.c
> +++ b/drivers/pci/host/pci-host-common.c

Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>

> @@ -145,7 +145,9 @@ int pci_host_common_probe(struct platform_device *pdev,
>  		return -ENODEV;
>  	}
>  
> +#ifdef CONFIG_ARM
>  	pci_fixup_irqs(pci_common_swizzle, of_irq_parse_and_map_pci);
> +#endif
>  
>  	/*
>  	 * We insert PCI resources into the iomem_resource and