[PATCH V4 0/2] Add ACPI support for HiSilicon SoCs Host Controllers

[PATCH V4 0/2] Add ACPI support for HiSilicon SoCs Host Controllers

[Dongdong Liu]

This patchset adds ACPI support for the HiSilicon Hip05/Hip06/Hip07 SoC
PCIe controllers.
The two patches respectively:
        - rework the current HiSilicon driver to add support for ECAM
          platforms(not RC).
        - adds the HiSilicon ACPI specific quirks.

This patchset is based on branch pci/ecam-v6
It can be found here:
https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git(pci/ecam-v6)

v3 -> v4:
- rebase on pci/ecam-v6.
- delete the unnecessary link_up check code. 

v2 -> v3:
- rebase against 4.9-rc1 and add Tomasz quirks V6 pathcset. 
- obtain rc base addresses from PNP0C02 as subdevice of PNP0A03 instead of
  hardcode the addresses.
- modify hisi_pcie_acpi_rd_conf/hisi_pcie_acpi_wr_conf() according to
  Arnd comments.

v1 -> v2:
- rebase against Tomasz RFC V5 quirk mechanism
- add ACPI support for the HiSilicon Hip07 SoC PCIe controllers

Dongdong Liu (2):
  PCI: hisi: Add ECAM support for devices that are not RC
  PCI/ACPI: hisi: Add ACPI support for HiSilicon SoCs Host Controllers

 .../devicetree/bindings/pci/hisilicon-pcie.txt     |  15 +-
 MAINTAINERS                                        |   1 +
 drivers/acpi/pci_mcfg.c                            |  13 ++
 drivers/pci/host/Kconfig                           |  12 +-
 drivers/pci/host/Makefile                          |   1 +
 drivers/pci/host/pcie-designware.c                 |   4 +-
 drivers/pci/host/pcie-designware.h                 |   2 +
 drivers/pci/host/pcie-hisi-acpi.c                  | 157 +++++++++++++++++++++
 drivers/pci/host/pcie-hisi.c                       |  45 ++++++
 include/linux/pci-ecam.h                           |   5 +
 10 files changed, 245 insertions(+), 10 deletions(-)
 create mode 100644 drivers/pci/host/pcie-hisi-acpi.c

-- 
1.9.1

[PATCH V4 1/2] PCI: hisi: Add ECAM support for devices that are not RC

[Dongdong Liu]

This patch modifies the current Hip05/Hip06 PCIe host
controller driver to add support for 'almost ECAM'
compliant platforms. Some controllers are ECAM compliant
for all the devices of the hierarchy except the root
complex; this patch adds support for such controllers.

This is needed in preparation for the ACPI based driver
to allow both DT and ACPI drivers to use the same BIOS
(that configure the Designware iATUs).
This commit doesn't break backward compatibility with
previous non-ECAM platforms.

Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
---
 .../devicetree/bindings/pci/hisilicon-pcie.txt     | 15 +++++---
 drivers/pci/host/Kconfig                           |  4 +-
 drivers/pci/host/pcie-designware.c                 |  4 +-
 drivers/pci/host/pcie-designware.h                 |  2 +
 drivers/pci/host/pcie-hisi.c                       | 45 ++++++++++++++++++++++
 5 files changed, 60 insertions(+), 10 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt b/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
index 59c2f47..87a597a 100644
--- a/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
@@ -9,10 +9,13 @@ Additional properties are described here:
 
 Required properties
 - compatible: Should contain "hisilicon,hip05-pcie" or "hisilicon,hip06-pcie".
-- reg: Should contain rc_dbi, config registers location and length.
-- reg-names: Must include the following entries:
+- reg: Should contain rc_dbi and  either config or ecam-cfg registers
+       location and length (it depends on the platform BIOS).
+- reg-names: Must include
   "rc_dbi": controller configuration registers;
-  "config": PCIe configuration space registers.
+  and one of the following entries:
+    "config": PCIe configuration space registers for non-ECAM platforms.
+    "ecam-cfg": PCIe configuration space registers for ECAM platforms
 - msi-parent: Should be its_pcie which is an ITS receiving MSI interrupts.
 - port-id: Should be 0, 1, 2 or 3.
 
@@ -23,8 +26,10 @@ Optional properties:
 Hip05 Example (note that Hip06 is the same except compatible):
 	pcie@0xb0080000 {
 		compatible = "hisilicon,hip05-pcie", "snps,dw-pcie";
-		reg = <0 0xb0080000 0 0x10000>, <0x220 0x00000000 0 0x2000>;
-		reg-names = "rc_dbi", "config";
+		reg = <0 0xb0080000 0 0x10000>,
+		      <0x220 0x00000000 0 0x2000>
+		/* or <0x220 0x00100000 0 0x0f00000> for ecam-cfg*/;
+		reg-names = "rc_dbi", "config" /* or "ecam-cfg" */;
 		bus-range = <0  15>;
 		msi-parent = <&its_pcie>;
 		#address-cells = <3>;
diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
index d7e7c0a..ae98644 100644
--- a/drivers/pci/host/Kconfig
+++ b/drivers/pci/host/Kconfig
@@ -219,13 +219,13 @@ config PCIE_ALTERA_MSI
 
 config PCI_HISI
 	depends on OF && ARM64
-	bool "HiSilicon Hip05 and Hip06 SoCs PCIe controllers"
+	bool "HiSilicon Hip05 and Hip06 and Hip07 SoCs PCIe controllers"
 	depends on PCI_MSI_IRQ_DOMAIN
 	select PCIEPORTBUS
 	select PCIE_DW
 	help
 	  Say Y here if you want PCIe controller support on HiSilicon
-	  Hip05 and Hip06 SoCs
+	  Hip05 and Hip06 and Hip07 SoCs
 
 config PCIE_QCOM
 	bool "Qualcomm PCIe controller"
diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index 035f50c..da11644 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -101,8 +101,6 @@
 #define PCIE_PHY_DEBUG_R1_LINK_UP	(0x1 << 4)
 #define PCIE_PHY_DEBUG_R1_LINK_IN_TRAINING	(0x1 << 29)
 
-static struct pci_ops dw_pcie_ops;
-
 int dw_pcie_cfg_read(void __iomem *addr, int size, u32 *val)
 {
 	if ((uintptr_t)addr & (size - 1)) {
@@ -800,7 +798,7 @@ static int dw_pcie_wr_conf(struct pci_bus *bus, u32 devfn,
 	return dw_pcie_wr_other_conf(pp, bus, devfn, where, size, val);
 }
 
-static struct pci_ops dw_pcie_ops = {
+struct pci_ops dw_pcie_ops = {
 	.read = dw_pcie_rd_conf,
 	.write = dw_pcie_wr_conf,
 };
diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h
index a567ea2..30d228a 100644
--- a/drivers/pci/host/pcie-designware.h
+++ b/drivers/pci/host/pcie-designware.h
@@ -83,4 +83,6 @@ struct pcie_host_ops {
 void dw_pcie_setup_rc(struct pcie_port *pp);
 int dw_pcie_host_init(struct pcie_port *pp);
 
+extern struct pci_ops dw_pcie_ops;
+
 #endif /* _PCIE_DESIGNWARE_H */
diff --git a/drivers/pci/host/pcie-hisi.c b/drivers/pci/host/pcie-hisi.c
index 56154c2..555c964 100644
--- a/drivers/pci/host/pcie-hisi.c
+++ b/drivers/pci/host/pcie-hisi.c
@@ -43,6 +43,34 @@ struct hisi_pcie {
 	struct pcie_soc_ops *soc_ops;
 };
 
+static inline int hisi_rd_ecam_conf(struct pcie_port *pp, struct pci_bus *bus,
+				    unsigned int devfn, int where, int size,
+				    u32 *value)
+{
+	return pci_generic_config_read(bus, devfn, where, size, value);
+}
+
+static inline int hisi_wr_ecam_conf(struct pcie_port *pp, struct pci_bus *bus,
+				    unsigned int devfn, int where, int size,
+				    u32 value)
+{
+	return pci_generic_config_write(bus, devfn, where, size, value);
+}
+
+static void __iomem *hisi_pci_map_cfg_bus_cam(struct pci_bus *bus,
+					      unsigned int devfn,
+					      int where)
+{
+	void __iomem *addr;
+	struct pcie_port *pp = bus->sysdata;
+
+	addr = pp->va_cfg1_base - (pp->busn->start << 20) +
+		((bus->number << 20) | (devfn << 12)) +
+		where;
+
+	return addr;
+}
+
 /* HipXX PCIe host only supports 32-bit config access */
 static int hisi_pcie_cfg_read(struct pcie_port *pp, int where, int size,
 			      u32 *val)
@@ -190,6 +218,23 @@ static int hisi_pcie_probe(struct platform_device *pdev)
 		return PTR_ERR(pp->dbi_base);
 	}
 
+	reg = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ecam-cfg");
+	if (reg) {
+		/* ECAM driver version */
+		hisi_pcie->pp.va_cfg0_base =
+		devm_ioremap_resource(&pdev->dev, reg);
+		if (IS_ERR(hisi_pcie->pp.va_cfg0_base)) {
+			dev_err(pp->dev, "cannot get ecam-cfg\n");
+			return PTR_ERR(hisi_pcie->pp.va_cfg0_base);
+		}
+		hisi_pcie->pp.va_cfg1_base = hisi_pcie->pp.va_cfg0_base;
+
+		dw_pcie_ops.map_bus = hisi_pci_map_cfg_bus_cam;
+
+		hisi_pcie_host_ops.rd_other_conf = hisi_rd_ecam_conf;
+		hisi_pcie_host_ops.wr_other_conf = hisi_wr_ecam_conf;
+	}
+
 	ret = hisi_add_pcie_port(hisi_pcie, pdev);
 	if (ret)
 		return ret;
-- 
1.9.1

[PATCH V4 2/2] PCI/ACPI: hisi: Add ACPI support for HiSilicon SoCs Host Controllers

[Dongdong Liu]

PCIe controller in Hip05/HIP06/HIP07 SoCs is not ECAM compliant.
It is non ECAM only for the RC bus config space;for any other bus
underneath the root bus we support ECAM access.
Add specific quirks for PCI config space accessors.This involves:
1. New initialization call hisi_pcie_init() to obtain rc base
addresses from PNP0C02 as subdevice of PNP0A03.
2. New entry in common quirk array.

Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
---
 MAINTAINERS                       |   1 +
 drivers/acpi/pci_mcfg.c           |  13 ++++
 drivers/pci/host/Kconfig          |   8 ++
 drivers/pci/host/Makefile         |   1 +
 drivers/pci/host/pcie-hisi-acpi.c | 157 ++++++++++++++++++++++++++++++++++++++
 include/linux/pci-ecam.h          |   5 ++
 6 files changed, 185 insertions(+)
 create mode 100644 drivers/pci/host/pcie-hisi-acpi.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 1cd38a7..b224caa 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9358,6 +9358,7 @@ L:	linux-pci@vger.kernel.org
 S:	Maintained
 F:	Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
 F:	drivers/pci/host/pcie-hisi.c
+F:	drivers/pci/host/pcie-hisi-acpi.c
 
 PCIE DRIVER FOR ROCKCHIP
 M:	Shawn Lin <shawn.lin@rock-chips.com>
diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
index ac21db3..b1b6fc7 100644
--- a/drivers/acpi/pci_mcfg.c
+++ b/drivers/acpi/pci_mcfg.c
@@ -57,6 +57,19 @@ struct mcfg_fixup {
 	{ "QCOM  ", "QDF2432 ", 1, 5, MCFG_BUS_ANY, &pci_32b_ops },
 	{ "QCOM  ", "QDF2432 ", 1, 6, MCFG_BUS_ANY, &pci_32b_ops },
 	{ "QCOM  ", "QDF2432 ", 1, 7, MCFG_BUS_ANY, &pci_32b_ops },
+#ifdef CONFIG_PCI_HISI_ACPI
+	#define PCI_ACPI_QUIRK_QUAD_DOM(table_id, seg, ops) \
+	{ "HISI  ", table_id, 0, seg + 0, MCFG_BUS_ANY, ops }, \
+	{ "HISI  ", table_id, 0, seg + 1, MCFG_BUS_ANY, ops }, \
+	{ "HISI  ", table_id, 0, seg + 2, MCFG_BUS_ANY, ops }, \
+	{ "HISI  ", table_id, 0, seg + 3, MCFG_BUS_ANY, ops }
+	PCI_ACPI_QUIRK_QUAD_DOM("HIP05   ", 0, &hisi_pcie_ops),
+	PCI_ACPI_QUIRK_QUAD_DOM("HIP06   ", 0, &hisi_pcie_ops),
+	PCI_ACPI_QUIRK_QUAD_DOM("HIP07   ", 0, &hisi_pcie_ops),
+	PCI_ACPI_QUIRK_QUAD_DOM("HIP07   ", 4, &hisi_pcie_ops),
+	PCI_ACPI_QUIRK_QUAD_DOM("HIP07   ", 8, &hisi_pcie_ops),
+	PCI_ACPI_QUIRK_QUAD_DOM("HIP07   ", 12, &hisi_pcie_ops),
+#endif
 };
 
 static char mcfg_oem_id[ACPI_OEM_ID_SIZE];
diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
index ae98644..9ff2bcd 100644
--- a/drivers/pci/host/Kconfig
+++ b/drivers/pci/host/Kconfig
@@ -227,6 +227,14 @@ config PCI_HISI
 	  Say Y here if you want PCIe controller support on HiSilicon
 	  Hip05 and Hip06 and Hip07 SoCs
 
+config PCI_HISI_ACPI
+	depends on ACPI && ARM64
+	bool "HiSilicon Hip05 and Hip06 and Hip07 SoCs ACPI PCIe controllers"
+	select PNP
+	help
+	  Say Y here if you want ACPI PCIe controller support on HiSilicon
+	  Hip05 and Hip06 and Hip07 SoCs
+
 config PCIE_QCOM
 	bool "Qualcomm PCIe controller"
 	depends on ARCH_QCOM && OF
diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
index 084cb49..9402858 100644
--- a/drivers/pci/host/Makefile
+++ b/drivers/pci/host/Makefile
@@ -26,6 +26,7 @@ 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_PCI_HISI) += pcie-hisi.o
+obj-$(CONFIG_PCI_HISI_ACPI) += pcie-hisi-acpi.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
diff --git a/drivers/pci/host/pcie-hisi-acpi.c b/drivers/pci/host/pcie-hisi-acpi.c
new file mode 100644
index 0000000..aade4b5
--- /dev/null
+++ b/drivers/pci/host/pcie-hisi-acpi.c
@@ -0,0 +1,157 @@
+/*
+ * PCIe host controller driver for HiSilicon HipXX SoCs
+ *
+ * Copyright (C) 2016 HiSilicon Co., Ltd. http://www.hisilicon.com
+ *
+ * Author: Dongdong Liu <liudongdong3@huawei.com>
+ *         Gabriele Paoloni <gabriele.paoloni@huawei.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#include <linux/pci.h>
+#include <linux/pci-acpi.h>
+#include <linux/pci-ecam.h>
+
+#define DEBUG0				0x728
+#define PCIE_LTSSM_LINKUP_STATE		0x11
+#define PCIE_LTSSM_STATE_MASK		0x3F
+
+static const struct acpi_device_id hisi_pcie_rc_res_ids[] = {
+	{"HISI0081", 0},
+	{"", 0},
+};
+
+static int hisi_pcie_acpi_rd_conf(struct pci_bus *bus, u32 devfn, int where,
+				  int size, u32 *val)
+{
+	struct pci_config_window *cfg = bus->sysdata;
+	int dev = PCI_SLOT(devfn);
+
+	if (bus->number == cfg->busr.start) {
+		/* access only one slot on each root port */
+		if (dev > 0)
+			return PCIBIOS_DEVICE_NOT_FOUND;
+		else
+			return pci_generic_config_read32(bus, devfn, where,
+							 size, val);
+	}
+
+	return pci_generic_config_read(bus, devfn, where, size, val);
+}
+
+static int hisi_pcie_acpi_wr_conf(struct pci_bus *bus, u32 devfn,
+				  int where, int size, u32 val)
+{
+	struct pci_config_window *cfg = bus->sysdata;
+	int dev = PCI_SLOT(devfn);
+
+	if (bus->number == cfg->busr.start) {
+		/* access only one slot on each root port */
+		if (dev > 0)
+			return PCIBIOS_DEVICE_NOT_FOUND;
+		else
+			return pci_generic_config_write32(bus, devfn, where,
+							  size, val);
+	}
+
+	return pci_generic_config_write(bus, devfn, where, size, val);
+}
+
+static void __iomem *hisi_pcie_map_bus(struct pci_bus *bus, unsigned int devfn,
+				       int where)
+{
+	struct pci_config_window *cfg = bus->sysdata;
+	void __iomem *reg_base = cfg->priv;
+
+	if (bus->number == cfg->busr.start)
+		return reg_base + where;
+	else
+		return pci_ecam_map_bus(bus, devfn, where);
+}
+
+/*
+ * Retrieve RC base and size from sub-device under the RC
+ * Device (RES1)
+ * {
+ *	Name (_HID, "HISI0081")
+ *	Name (_CID, "PNP0C02")
+ *	Name (_CRS, ResourceTemplate (){
+ *		Memory32Fixed (ReadWrite, 0xb0080000, 0x10000)
+ *	})
+ * }
+ */
+static int hisi_pcie_rc_addr_get(struct acpi_device *adev,
+				 void __iomem **addr)
+{
+	struct acpi_device *child_adev;
+	struct list_head list;
+	struct resource *res;
+	struct resource_entry *entry;
+	unsigned long flags;
+	int ret;
+
+	list_for_each_entry(child_adev, &adev->children, node) {
+		ret = acpi_match_device_ids(child_adev, hisi_pcie_rc_res_ids);
+		if (ret)
+			continue;
+
+		INIT_LIST_HEAD(&list);
+		flags = IORESOURCE_MEM;
+		ret = acpi_dev_get_resources(child_adev, &list,
+					     acpi_dev_filter_resource_type_cb,
+					     (void *)flags);
+		if (ret < 0) {
+			dev_err(&child_adev->dev,
+				"failed to parse _CRS method, error code %d\n",
+				ret);
+			return ret;
+		} else if (ret == 0) {
+			dev_err(&child_adev->dev,
+				"no IO and memory resources present in _CRS\n");
+			return -EINVAL;
+		}
+
+		entry = list_first_entry(&list, struct resource_entry, node);
+		res = entry->res;
+		*addr = devm_ioremap(&child_adev->dev,
+				     res->start, resource_size(res));
+		acpi_dev_free_resource_list(&list);
+		if (IS_ERR(*addr)) {
+			dev_err(&child_adev->dev, "error with ioremap\n");
+			return -ENOMEM;
+		}
+
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
+static int hisi_pcie_init(struct pci_config_window *cfg)
+{
+	int ret;
+	struct acpi_device *adev = to_acpi_device(cfg->parent);
+	void __iomem *reg_base;
+
+	ret = hisi_pcie_rc_addr_get(adev, &reg_base);
+	if (ret) {
+		dev_err(&adev->dev, "can't get rc base address");
+		return ret;
+	}
+
+	cfg->priv = reg_base;
+
+	return 0;
+}
+
+struct pci_ecam_ops hisi_pcie_ops = {
+	.bus_shift    = 20,
+	.init         =  hisi_pcie_init,
+	.pci_ops      = {
+		.map_bus    = hisi_pcie_map_bus,
+		.read       = hisi_pcie_acpi_rd_conf,
+		.write      = hisi_pcie_acpi_wr_conf,
+	}
+};
diff --git a/include/linux/pci-ecam.h b/include/linux/pci-ecam.h
index f5740b7..1b24d97 100644
--- a/include/linux/pci-ecam.h
+++ b/include/linux/pci-ecam.h
@@ -67,4 +67,9 @@ void __iomem *pci_ecam_map_bus(struct pci_bus *bus, unsigned int devfn,
 int pci_host_common_probe(struct platform_device *pdev,
 			  struct pci_ecam_ops *ops);
 #endif
+
+#ifdef CONFIG_PCI_HISI_ACPI
+extern struct pci_ecam_ops hisi_pcie_ops;
+#endif
+
 #endif
-- 
1.9.1

Re: [PATCH V4 1/2] PCI: hisi: Add ECAM support for devices that are not RC

[Bjorn Helgaas]

[+cc Jingoo]

On Wed, Nov 09, 2016 at 05:14:56PM +0800, Dongdong Liu wrote:
> This patch modifies the current Hip05/Hip06 PCIe host
> controller driver to add support for 'almost ECAM'
> compliant platforms. Some controllers are ECAM compliant
> for all the devices of the hierarchy except the root
> complex; this patch adds support for such controllers.
> 
> This is needed in preparation for the ACPI based driver
> to allow both DT and ACPI drivers to use the same BIOS
> (that configure the Designware iATUs).
> This commit doesn't break backward compatibility with
> previous non-ECAM platforms.
> 
> Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
> ---
>  .../devicetree/bindings/pci/hisilicon-pcie.txt     | 15 +++++---
>  drivers/pci/host/Kconfig                           |  4 +-
>  drivers/pci/host/pcie-designware.c                 |  4 +-
>  drivers/pci/host/pcie-designware.h                 |  2 +
>  drivers/pci/host/pcie-hisi.c                       | 45 ++++++++++++++++++++++
>  5 files changed, 60 insertions(+), 10 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt b/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
> index 59c2f47..87a597a 100644
> --- a/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
> @@ -9,10 +9,13 @@ Additional properties are described here:
>  
>  Required properties
>  - compatible: Should contain "hisilicon,hip05-pcie" or "hisilicon,hip06-pcie".
> -- reg: Should contain rc_dbi, config registers location and length.
> -- reg-names: Must include the following entries:
> +- reg: Should contain rc_dbi and  either config or ecam-cfg registers
> +       location and length (it depends on the platform BIOS).
> +- reg-names: Must include
>    "rc_dbi": controller configuration registers;
> -  "config": PCIe configuration space registers.
> +  and one of the following entries:
> +    "config": PCIe configuration space registers for non-ECAM platforms.
> +    "ecam-cfg": PCIe configuration space registers for ECAM platforms
>  - msi-parent: Should be its_pcie which is an ITS receiving MSI interrupts.
>  - port-id: Should be 0, 1, 2 or 3.
>  
> @@ -23,8 +26,10 @@ Optional properties:
>  Hip05 Example (note that Hip06 is the same except compatible):
>  	pcie@0xb0080000 {
>  		compatible = "hisilicon,hip05-pcie", "snps,dw-pcie";
> -		reg = <0 0xb0080000 0 0x10000>, <0x220 0x00000000 0 0x2000>;
> -		reg-names = "rc_dbi", "config";
> +		reg = <0 0xb0080000 0 0x10000>,
> +		      <0x220 0x00000000 0 0x2000>
> +		/* or <0x220 0x00100000 0 0x0f00000> for ecam-cfg*/;

Add space before closing "*/".

> +		reg-names = "rc_dbi", "config" /* or "ecam-cfg" */;
>  		bus-range = <0  15>;
>  		msi-parent = <&its_pcie>;
>  		#address-cells = <3>;
> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
> index d7e7c0a..ae98644 100644
> --- a/drivers/pci/host/Kconfig
> +++ b/drivers/pci/host/Kconfig
> @@ -219,13 +219,13 @@ config PCIE_ALTERA_MSI
>  
>  config PCI_HISI
>  	depends on OF && ARM64
> -	bool "HiSilicon Hip05 and Hip06 SoCs PCIe controllers"
> +	bool "HiSilicon Hip05 and Hip06 and Hip07 SoCs PCIe controllers"
>  	depends on PCI_MSI_IRQ_DOMAIN
>  	select PCIEPORTBUS
>  	select PCIE_DW
>  	help
>  	  Say Y here if you want PCIe controller support on HiSilicon
> -	  Hip05 and Hip06 SoCs
> +	  Hip05 and Hip06 and Hip07 SoCs
>  
>  config PCIE_QCOM
>  	bool "Qualcomm PCIe controller"
> diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
> index 035f50c..da11644 100644
> --- a/drivers/pci/host/pcie-designware.c
> +++ b/drivers/pci/host/pcie-designware.c
> @@ -101,8 +101,6 @@
>  #define PCIE_PHY_DEBUG_R1_LINK_UP	(0x1 << 4)
>  #define PCIE_PHY_DEBUG_R1_LINK_IN_TRAINING	(0x1 << 29)
>  
> -static struct pci_ops dw_pcie_ops;
> -
>  int dw_pcie_cfg_read(void __iomem *addr, int size, u32 *val)
>  {
>  	if ((uintptr_t)addr & (size - 1)) {
> @@ -800,7 +798,7 @@ static int dw_pcie_wr_conf(struct pci_bus *bus, u32 devfn,
>  	return dw_pcie_wr_other_conf(pp, bus, devfn, where, size, val);
>  }
>  
> -static struct pci_ops dw_pcie_ops = {
> +struct pci_ops dw_pcie_ops = {
>  	.read = dw_pcie_rd_conf,
>  	.write = dw_pcie_wr_conf,
>  };
> diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h
> index a567ea2..30d228a 100644
> --- a/drivers/pci/host/pcie-designware.h
> +++ b/drivers/pci/host/pcie-designware.h
> @@ -83,4 +83,6 @@ struct pcie_host_ops {
>  void dw_pcie_setup_rc(struct pcie_port *pp);
>  int dw_pcie_host_init(struct pcie_port *pp);
>  
> +extern struct pci_ops dw_pcie_ops;
> +
>  #endif /* _PCIE_DESIGNWARE_H */
> diff --git a/drivers/pci/host/pcie-hisi.c b/drivers/pci/host/pcie-hisi.c
> index 56154c2..555c964 100644
> --- a/drivers/pci/host/pcie-hisi.c
> +++ b/drivers/pci/host/pcie-hisi.c
> @@ -43,6 +43,34 @@ struct hisi_pcie {
>  	struct pcie_soc_ops *soc_ops;
>  };
>  
> +static inline int hisi_rd_ecam_conf(struct pcie_port *pp, struct pci_bus *bus,
> +				    unsigned int devfn, int where, int size,
> +				    u32 *value)
> +{
> +	return pci_generic_config_read(bus, devfn, where, size, value);
> +}
> +
> +static inline int hisi_wr_ecam_conf(struct pcie_port *pp, struct pci_bus *bus,
> +				    unsigned int devfn, int where, int size,
> +				    u32 value)
> +{
> +	return pci_generic_config_write(bus, devfn, where, size, value);
> +}
> +
> +static void __iomem *hisi_pci_map_cfg_bus_cam(struct pci_bus *bus,
> +					      unsigned int devfn,
> +					      int where)
> +{
> +	void __iomem *addr;
> +	struct pcie_port *pp = bus->sysdata;
> +
> +	addr = pp->va_cfg1_base - (pp->busn->start << 20) +
> +		((bus->number << 20) | (devfn << 12)) +
> +		where;
> +
> +	return addr;
> +}
> +
>  /* HipXX PCIe host only supports 32-bit config access */
>  static int hisi_pcie_cfg_read(struct pcie_port *pp, int where, int size,
>  			      u32 *val)
> @@ -190,6 +218,23 @@ static int hisi_pcie_probe(struct platform_device *pdev)
>  		return PTR_ERR(pp->dbi_base);
>  	}
>  
> +	reg = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ecam-cfg");
> +	if (reg) {
> +		/* ECAM driver version */
> +		hisi_pcie->pp.va_cfg0_base =
> +		devm_ioremap_resource(&pdev->dev, reg);
> +		if (IS_ERR(hisi_pcie->pp.va_cfg0_base)) {
> +			dev_err(pp->dev, "cannot get ecam-cfg\n");
> +			return PTR_ERR(hisi_pcie->pp.va_cfg0_base);
> +		}
> +		hisi_pcie->pp.va_cfg1_base = hisi_pcie->pp.va_cfg0_base;
> +
> +		dw_pcie_ops.map_bus = hisi_pci_map_cfg_bus_cam;

I don't really like this part.  We're scribbling on dw_pcie_ops, which this
driver does not own.  That breaks the encapsulation and means that using
two different DW-based drivers in the same system (admittedly unlikely)
would fail mysteriously.

I wonder if this could be solved by allowing the DW-based drivers to supply
their own pci_ops.  If we did that, it might let us get rid of the
rd_own_conf, wr_own_conf, rd_other_conf, and wr_other_conf function
pointers.  I haven't worked through it, but it's possible the result would
be easier to read.

It's also possible it would be a lot of code churn for no real gain, and
breaking the encapsulation makes the most sense.  What do you think?

> +
> +		hisi_pcie_host_ops.rd_other_conf = hisi_rd_ecam_conf;
> +		hisi_pcie_host_ops.wr_other_conf = hisi_wr_ecam_conf;
> +	}
> +
>  	ret = hisi_add_pcie_port(hisi_pcie, pdev);
>  	if (ret)
>  		return ret;
> -- 
> 1.9.1
> 

Re: [PATCH V4 2/2] PCI/ACPI: hisi: Add ACPI support for HiSilicon SoCs Host Controllers

[Bjorn Helgaas]

On Wed, Nov 09, 2016 at 05:14:57PM +0800, Dongdong Liu wrote:
> PCIe controller in Hip05/HIP06/HIP07 SoCs is not ECAM compliant.
> It is non ECAM only for the RC bus config space;for any other bus
> underneath the root bus we support ECAM access.
> Add specific quirks for PCI config space accessors.This involves:
> 1. New initialization call hisi_pcie_init() to obtain rc base
> addresses from PNP0C02 as subdevice of PNP0A03.
> 2. New entry in common quirk array.
> 
> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
> Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
> ---
>  MAINTAINERS                       |   1 +
>  drivers/acpi/pci_mcfg.c           |  13 ++++
>  drivers/pci/host/Kconfig          |   8 ++
>  drivers/pci/host/Makefile         |   1 +
>  drivers/pci/host/pcie-hisi-acpi.c | 157 ++++++++++++++++++++++++++++++++++++++
>  include/linux/pci-ecam.h          |   5 ++
>  6 files changed, 185 insertions(+)
>  create mode 100644 drivers/pci/host/pcie-hisi-acpi.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 1cd38a7..b224caa 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -9358,6 +9358,7 @@ L:	linux-pci@vger.kernel.org
>  S:	Maintained
>  F:	Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
>  F:	drivers/pci/host/pcie-hisi.c
> +F:	drivers/pci/host/pcie-hisi-acpi.c
>  
>  PCIE DRIVER FOR ROCKCHIP
>  M:	Shawn Lin <shawn.lin@rock-chips.com>
> diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
> index ac21db3..b1b6fc7 100644
> --- a/drivers/acpi/pci_mcfg.c
> +++ b/drivers/acpi/pci_mcfg.c
> @@ -57,6 +57,19 @@ struct mcfg_fixup {
>  	{ "QCOM  ", "QDF2432 ", 1, 5, MCFG_BUS_ANY, &pci_32b_ops },
>  	{ "QCOM  ", "QDF2432 ", 1, 6, MCFG_BUS_ANY, &pci_32b_ops },
>  	{ "QCOM  ", "QDF2432 ", 1, 7, MCFG_BUS_ANY, &pci_32b_ops },
> +#ifdef CONFIG_PCI_HISI_ACPI
> +	#define PCI_ACPI_QUIRK_QUAD_DOM(table_id, seg, ops) \
> +	{ "HISI  ", table_id, 0, seg + 0, MCFG_BUS_ANY, ops }, \
> +	{ "HISI  ", table_id, 0, seg + 1, MCFG_BUS_ANY, ops }, \
> +	{ "HISI  ", table_id, 0, seg + 2, MCFG_BUS_ANY, ops }, \
> +	{ "HISI  ", table_id, 0, seg + 3, MCFG_BUS_ANY, ops }
> +	PCI_ACPI_QUIRK_QUAD_DOM("HIP05   ", 0, &hisi_pcie_ops),
> +	PCI_ACPI_QUIRK_QUAD_DOM("HIP06   ", 0, &hisi_pcie_ops),
> +	PCI_ACPI_QUIRK_QUAD_DOM("HIP07   ", 0, &hisi_pcie_ops),
> +	PCI_ACPI_QUIRK_QUAD_DOM("HIP07   ", 4, &hisi_pcie_ops),
> +	PCI_ACPI_QUIRK_QUAD_DOM("HIP07   ", 8, &hisi_pcie_ops),
> +	PCI_ACPI_QUIRK_QUAD_DOM("HIP07   ", 12, &hisi_pcie_ops),
> +#endif
>  };
>  
>  static char mcfg_oem_id[ACPI_OEM_ID_SIZE];
> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
> index ae98644..9ff2bcd 100644
> --- a/drivers/pci/host/Kconfig
> +++ b/drivers/pci/host/Kconfig
> @@ -227,6 +227,14 @@ config PCI_HISI
>  	  Say Y here if you want PCIe controller support on HiSilicon
>  	  Hip05 and Hip06 and Hip07 SoCs
>  
> +config PCI_HISI_ACPI
> +	depends on ACPI && ARM64
> +	bool "HiSilicon Hip05 and Hip06 and Hip07 SoCs ACPI PCIe controllers"
> +	select PNP
> +	help
> +	  Say Y here if you want ACPI PCIe controller support on HiSilicon
> +	  Hip05 and Hip06 and Hip07 SoCs
> +
>  config PCIE_QCOM
>  	bool "Qualcomm PCIe controller"
>  	depends on ARCH_QCOM && OF
> diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
> index 084cb49..9402858 100644
> --- a/drivers/pci/host/Makefile
> +++ b/drivers/pci/host/Makefile
> @@ -26,6 +26,7 @@ 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_PCI_HISI) += pcie-hisi.o
> +obj-$(CONFIG_PCI_HISI_ACPI) += pcie-hisi-acpi.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
> diff --git a/drivers/pci/host/pcie-hisi-acpi.c b/drivers/pci/host/pcie-hisi-acpi.c
> new file mode 100644
> index 0000000..aade4b5
> --- /dev/null
> +++ b/drivers/pci/host/pcie-hisi-acpi.c
> @@ -0,0 +1,157 @@
> +/*
> + * PCIe host controller driver for HiSilicon HipXX SoCs
> + *
> + * Copyright (C) 2016 HiSilicon Co., Ltd. http://www.hisilicon.com
> + *
> + * Author: Dongdong Liu <liudongdong3@huawei.com>
> + *         Gabriele Paoloni <gabriele.paoloni@huawei.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +#include <linux/pci.h>
> +#include <linux/pci-acpi.h>
> +#include <linux/pci-ecam.h>
> +
> +#define DEBUG0				0x728
> +#define PCIE_LTSSM_LINKUP_STATE		0x11
> +#define PCIE_LTSSM_STATE_MASK		0x3F

These are now unused.

> +static const struct acpi_device_id hisi_pcie_rc_res_ids[] = {
> +	{"HISI0081", 0},
> +	{"", 0},
> +};
> +
> +static int hisi_pcie_acpi_rd_conf(struct pci_bus *bus, u32 devfn, int where,
> +				  int size, u32 *val)
> +{
> +	struct pci_config_window *cfg = bus->sysdata;
> +	int dev = PCI_SLOT(devfn);
> +
> +	if (bus->number == cfg->busr.start) {
> +		/* access only one slot on each root port */
> +		if (dev > 0)
> +			return PCIBIOS_DEVICE_NOT_FOUND;
> +		else
> +			return pci_generic_config_read32(bus, devfn, where,
> +							 size, val);
> +	}
> +
> +	return pci_generic_config_read(bus, devfn, where, size, val);
> +}
> +
> +static int hisi_pcie_acpi_wr_conf(struct pci_bus *bus, u32 devfn,
> +				  int where, int size, u32 val)
> +{
> +	struct pci_config_window *cfg = bus->sysdata;
> +	int dev = PCI_SLOT(devfn);
> +
> +	if (bus->number == cfg->busr.start) {
> +		/* access only one slot on each root port */
> +		if (dev > 0)
> +			return PCIBIOS_DEVICE_NOT_FOUND;
> +		else
> +			return pci_generic_config_write32(bus, devfn, where,
> +							  size, val);
> +	}
> +
> +	return pci_generic_config_write(bus, devfn, where, size, val);
> +}
> +
> +static void __iomem *hisi_pcie_map_bus(struct pci_bus *bus, unsigned int devfn,
> +				       int where)
> +{
> +	struct pci_config_window *cfg = bus->sysdata;
> +	void __iomem *reg_base = cfg->priv;
> +
> +	if (bus->number == cfg->busr.start)
> +		return reg_base + where;
> +	else
> +		return pci_ecam_map_bus(bus, devfn, where);
> +}
> +
> +/*
> + * Retrieve RC base and size from sub-device under the RC
> + * Device (RES1)
> + * {
> + *	Name (_HID, "HISI0081")
> + *	Name (_CID, "PNP0C02")
> + *	Name (_CRS, ResourceTemplate (){
> + *		Memory32Fixed (ReadWrite, 0xb0080000, 0x10000)
> + *	})
> + * }
> + */
> +static int hisi_pcie_rc_addr_get(struct acpi_device *adev,
> +				 void __iomem **addr)
> +{
> +	struct acpi_device *child_adev;
> +	struct list_head list;
> +	struct resource *res;
> +	struct resource_entry *entry;
> +	unsigned long flags;
> +	int ret;
> +
> +	list_for_each_entry(child_adev, &adev->children, node) {
> +		ret = acpi_match_device_ids(child_adev, hisi_pcie_rc_res_ids);
> +		if (ret)
> +			continue;
> +
> +		INIT_LIST_HEAD(&list);
> +		flags = IORESOURCE_MEM;
> +		ret = acpi_dev_get_resources(child_adev, &list,
> +					     acpi_dev_filter_resource_type_cb,
> +					     (void *)flags);
> +		if (ret < 0) {
> +			dev_err(&child_adev->dev,
> +				"failed to parse _CRS method, error code %d\n",
> +				ret);
> +			return ret;
> +		} else if (ret == 0) {
> +			dev_err(&child_adev->dev,
> +				"no IO and memory resources present in _CRS\n");
> +			return -EINVAL;
> +		}
> +
> +		entry = list_first_entry(&list, struct resource_entry, node);
> +		res = entry->res;
> +		*addr = devm_ioremap(&child_adev->dev,
> +				     res->start, resource_size(res));
> +		acpi_dev_free_resource_list(&list);
> +		if (IS_ERR(*addr)) {
> +			dev_err(&child_adev->dev, "error with ioremap\n");
> +			return -ENOMEM;
> +		}
> +
> +		return 0;
> +	}
> +
> +	return -EINVAL;
> +}

OK, so MCFG (with the possible help of quirks) tells us where *most*
of this RC's ECAM space is, but it doesn't tell us where the root bus
ECAM space is, which is why we need this block of ugly code.  Right?

Is there any way we can compute the root bus reg_base from the address
we got from MCFG?  Is it a constant that realistically is not going to
be modified?

I would rather have the PNP0C02 device at the root of the ACPI
namespace (under \_SB) if possible, the way x86 systems do it.
I don't know whether that's actually a requirement, but the PCI
Firmware Spec does suggest it, and it's better to follow that existing
practice whenever possible.

I'm still looking for the specifics of how these resources are
described, i.e., a dmesg log, /proc/iomem, and maybe an ACPI dump.
These could go in a bugzilla entry, with a URL in this changelog.

> +
> +static int hisi_pcie_init(struct pci_config_window *cfg)
> +{
> +	int ret;
> +	struct acpi_device *adev = to_acpi_device(cfg->parent);
> +	void __iomem *reg_base;
> +
> +	ret = hisi_pcie_rc_addr_get(adev, &reg_base);
> +	if (ret) {
> +		dev_err(&adev->dev, "can't get rc base address");
> +		return ret;
> +	}
> +
> +	cfg->priv = reg_base;
> +
> +	return 0;
> +}
> +
> +struct pci_ecam_ops hisi_pcie_ops = {
> +	.bus_shift    = 20,
> +	.init         =  hisi_pcie_init,
> +	.pci_ops      = {
> +		.map_bus    = hisi_pcie_map_bus,
> +		.read       = hisi_pcie_acpi_rd_conf,
> +		.write      = hisi_pcie_acpi_wr_conf,
> +	}
> +};
> diff --git a/include/linux/pci-ecam.h b/include/linux/pci-ecam.h
> index f5740b7..1b24d97 100644
> --- a/include/linux/pci-ecam.h
> +++ b/include/linux/pci-ecam.h
> @@ -67,4 +67,9 @@ void __iomem *pci_ecam_map_bus(struct pci_bus *bus, unsigned int devfn,
>  int pci_host_common_probe(struct platform_device *pdev,
>  			  struct pci_ecam_ops *ops);
>  #endif
> +
> +#ifdef CONFIG_PCI_HISI_ACPI
> +extern struct pci_ecam_ops hisi_pcie_ops;
> +#endif
> +
>  #endif
> -- 
> 1.9.1
> 

RE: [PATCH V4 1/2] PCI: hisi: Add ECAM support for devices that are not RC

[Gabriele Paoloni]

Hi Bjorn

Many Thanks for your review

> -----Original Message-----
> From: Bjorn Helgaas [mailto:helgaas@kernel.org]
> Sent: 14 November 2016 23:07
> To: liudongdong (C)
> Cc: arnd@arndb.de; rafael@kernel.org; Lorenzo.Pieralisi@arm.com;
> tn@semihalf.com; Wangzhou (B); pratyush.anand@gmail.com; linux-
> pci@vger.kernel.org; linux-acpi@vger.kernel.org; linux-
> kernel@vger.kernel.org; jcm@redhat.com; Gabriele Paoloni; Chenxin
> (Charles); hanjun.guo@linaro.org; Linuxarm; Jingoo Han
> Subject: Re: [PATCH V4 1/2] PCI: hisi: Add ECAM support for devices
> that are not RC
> 
> [+cc Jingoo]
> 
> On Wed, Nov 09, 2016 at 05:14:56PM +0800, Dongdong Liu wrote:
> > This patch modifies the current Hip05/Hip06 PCIe host
> > controller driver to add support for 'almost ECAM'
> > compliant platforms. Some controllers are ECAM compliant
> > for all the devices of the hierarchy except the root
> > complex; this patch adds support for such controllers.
> >
> > This is needed in preparation for the ACPI based driver
> > to allow both DT and ACPI drivers to use the same BIOS
> > (that configure the Designware iATUs).
> > This commit doesn't break backward compatibility with
> > previous non-ECAM platforms.
> >
> > Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
> > Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
> > ---
> >  .../devicetree/bindings/pci/hisilicon-pcie.txt     | 15 +++++---
> >  drivers/pci/host/Kconfig                           |  4 +-
> >  drivers/pci/host/pcie-designware.c                 |  4 +-
> >  drivers/pci/host/pcie-designware.h                 |  2 +
> >  drivers/pci/host/pcie-hisi.c                       | 45
> ++++++++++++++++++++++
> >  5 files changed, 60 insertions(+), 10 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
> b/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
> > index 59c2f47..87a597a 100644
> > --- a/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
> > +++ b/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
> > @@ -9,10 +9,13 @@ Additional properties are described here:
> >
> >  Required properties
> >  - compatible: Should contain "hisilicon,hip05-pcie" or
> "hisilicon,hip06-pcie".
> > -- reg: Should contain rc_dbi, config registers location and length.
> > -- reg-names: Must include the following entries:
> > +- reg: Should contain rc_dbi and  either config or ecam-cfg
> registers
> > +       location and length (it depends on the platform BIOS).
> > +- reg-names: Must include
> >    "rc_dbi": controller configuration registers;
> > -  "config": PCIe configuration space registers.
> > +  and one of the following entries:
> > +    "config": PCIe configuration space registers for non-ECAM
> platforms.
> > +    "ecam-cfg": PCIe configuration space registers for ECAM
> platforms
> >  - msi-parent: Should be its_pcie which is an ITS receiving MSI
> interrupts.
> >  - port-id: Should be 0, 1, 2 or 3.
> >
> > @@ -23,8 +26,10 @@ Optional properties:
> >  Hip05 Example (note that Hip06 is the same except compatible):
> >  	pcie@0xb0080000 {
> >  		compatible = "hisilicon,hip05-pcie", "snps,dw-pcie";
> > -		reg = <0 0xb0080000 0 0x10000>, <0x220 0x00000000 0
> 0x2000>;
> > -		reg-names = "rc_dbi", "config";
> > +		reg = <0 0xb0080000 0 0x10000>,
> > +		      <0x220 0x00000000 0 0x2000>
> > +		/* or <0x220 0x00100000 0 0x0f00000> for ecam-cfg*/;
> 
> Add space before closing "*/".

Oops, thanks for spotting

> 
> > +		reg-names = "rc_dbi", "config" /* or "ecam-cfg" */;
> >  		bus-range = <0  15>;
> >  		msi-parent = <&its_pcie>;
> >  		#address-cells = <3>;
> > diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
> > index d7e7c0a..ae98644 100644
> > --- a/drivers/pci/host/Kconfig
> > +++ b/drivers/pci/host/Kconfig
> > @@ -219,13 +219,13 @@ config PCIE_ALTERA_MSI
> >
> >  config PCI_HISI
> >  	depends on OF && ARM64
> > -	bool "HiSilicon Hip05 and Hip06 SoCs PCIe controllers"
> > +	bool "HiSilicon Hip05 and Hip06 and Hip07 SoCs PCIe controllers"
> >  	depends on PCI_MSI_IRQ_DOMAIN
> >  	select PCIEPORTBUS
> >  	select PCIE_DW
> >  	help
> >  	  Say Y here if you want PCIe controller support on HiSilicon
> > -	  Hip05 and Hip06 SoCs
> > +	  Hip05 and Hip06 and Hip07 SoCs
> >
> >  config PCIE_QCOM
> >  	bool "Qualcomm PCIe controller"
> > diff --git a/drivers/pci/host/pcie-designware.c
> b/drivers/pci/host/pcie-designware.c
> > index 035f50c..da11644 100644
> > --- a/drivers/pci/host/pcie-designware.c
> > +++ b/drivers/pci/host/pcie-designware.c
> > @@ -101,8 +101,6 @@
> >  #define PCIE_PHY_DEBUG_R1_LINK_UP	(0x1 << 4)
> >  #define PCIE_PHY_DEBUG_R1_LINK_IN_TRAINING	(0x1 << 29)
> >
> > -static struct pci_ops dw_pcie_ops;
> > -
> >  int dw_pcie_cfg_read(void __iomem *addr, int size, u32 *val)
> >  {
> >  	if ((uintptr_t)addr & (size - 1)) {
> > @@ -800,7 +798,7 @@ static int dw_pcie_wr_conf(struct pci_bus *bus,
> u32 devfn,
> >  	return dw_pcie_wr_other_conf(pp, bus, devfn, where, size, val);
> >  }
> >
> > -static struct pci_ops dw_pcie_ops = {
> > +struct pci_ops dw_pcie_ops = {
> >  	.read = dw_pcie_rd_conf,
> >  	.write = dw_pcie_wr_conf,
> >  };
> > diff --git a/drivers/pci/host/pcie-designware.h
> b/drivers/pci/host/pcie-designware.h
> > index a567ea2..30d228a 100644
> > --- a/drivers/pci/host/pcie-designware.h
> > +++ b/drivers/pci/host/pcie-designware.h
> > @@ -83,4 +83,6 @@ struct pcie_host_ops {
> >  void dw_pcie_setup_rc(struct pcie_port *pp);
> >  int dw_pcie_host_init(struct pcie_port *pp);
> >
> > +extern struct pci_ops dw_pcie_ops;
> > +
> >  #endif /* _PCIE_DESIGNWARE_H */
> > diff --git a/drivers/pci/host/pcie-hisi.c b/drivers/pci/host/pcie-
> hisi.c
> > index 56154c2..555c964 100644
> > --- a/drivers/pci/host/pcie-hisi.c
> > +++ b/drivers/pci/host/pcie-hisi.c
> > @@ -43,6 +43,34 @@ struct hisi_pcie {
> >  	struct pcie_soc_ops *soc_ops;
> >  };
> >
> > +static inline int hisi_rd_ecam_conf(struct pcie_port *pp, struct
> pci_bus *bus,
> > +				    unsigned int devfn, int where, int size,
> > +				    u32 *value)
> > +{
> > +	return pci_generic_config_read(bus, devfn, where, size, value);
> > +}
> > +
> > +static inline int hisi_wr_ecam_conf(struct pcie_port *pp, struct
> pci_bus *bus,
> > +				    unsigned int devfn, int where, int size,
> > +				    u32 value)
> > +{
> > +	return pci_generic_config_write(bus, devfn, where, size, value);
> > +}
> > +
> > +static void __iomem *hisi_pci_map_cfg_bus_cam(struct pci_bus *bus,
> > +					      unsigned int devfn,
> > +					      int where)
> > +{
> > +	void __iomem *addr;
> > +	struct pcie_port *pp = bus->sysdata;
> > +
> > +	addr = pp->va_cfg1_base - (pp->busn->start << 20) +
> > +		((bus->number << 20) | (devfn << 12)) +
> > +		where;
> > +
> > +	return addr;
> > +}
> > +
> >  /* HipXX PCIe host only supports 32-bit config access */
> >  static int hisi_pcie_cfg_read(struct pcie_port *pp, int where, int
> size,
> >  			      u32 *val)
> > @@ -190,6 +218,23 @@ static int hisi_pcie_probe(struct
> platform_device *pdev)
> >  		return PTR_ERR(pp->dbi_base);
> >  	}
> >
> > +	reg = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ecam-
> cfg");
> > +	if (reg) {
> > +		/* ECAM driver version */
> > +		hisi_pcie->pp.va_cfg0_base =
> > +		devm_ioremap_resource(&pdev->dev, reg);
> > +		if (IS_ERR(hisi_pcie->pp.va_cfg0_base)) {
> > +			dev_err(pp->dev, "cannot get ecam-cfg\n");
> > +			return PTR_ERR(hisi_pcie->pp.va_cfg0_base);
> > +		}
> > +		hisi_pcie->pp.va_cfg1_base = hisi_pcie->pp.va_cfg0_base;
> > +
> > +		dw_pcie_ops.map_bus = hisi_pci_map_cfg_bus_cam;
> 
> I don't really like this part.  We're scribbling on dw_pcie_ops, which
> this
> driver does not own.  That breaks the encapsulation and means that
> using
> two different DW-based drivers in the same system (admittedly unlikely)
> would fail mysteriously.
> 
> I wonder if this could be solved by allowing the DW-based drivers to
> supply
> their own pci_ops.  If we did that, it might let us get rid of the
> rd_own_conf, wr_own_conf, rd_other_conf, and wr_other_conf function
> pointers.  I haven't worked through it, but it's possible the result
> would
> be easier to read.
> 
> It's also possible it would be a lot of code churn for no real gain,
> and
> breaking the encapsulation makes the most sense.  What do you think?

Yes you're right...

I am thinking that maybe we can split this in few steps:

1) We get the host controllers to pass their own pci_ops ( so we can modify
   dw_pcie_host_init() API to get pci_ops as input ) and move the pci_ops
   definitions to the respective host controller drivers

2) We investigate controller by controller if it makes sense to get rid of
   dw_pcie_rd_conf() and dw_pcie_wr_conf() and which desingware functions
   must be kept (probably dw_pcie_rd_other_conf() and dw_pcie_wr_other_conf()
   should be kept and used by the respective controllers)
	
3) We rework controller by controller

4) We clean up the Designware framework to get rid of the functions not needed
   anymore

I think that 1) can go into this patchset, whereas 2), 3), 4) may go in a separate
one....

What do you think?

Thanks

Gab

> 
> > +
> > +		hisi_pcie_host_ops.rd_other_conf = hisi_rd_ecam_conf;
> > +		hisi_pcie_host_ops.wr_other_conf = hisi_wr_ecam_conf;
> > +	}
> > +
> >  	ret = hisi_add_pcie_port(hisi_pcie, pdev);
> >  	if (ret)
> >  		return ret;
> > --
> > 1.9.1
> >

Re: [PATCH V4 2/2] PCI/ACPI: hisi: Add ACPI support for HiSilicon SoCs Host Controllers

[Dongdong Liu]

Hi Bjorn

Many Thanks for your review

在 2016/11/15 7:33, Bjorn Helgaas 写道:
> On Wed, Nov 09, 2016 at 05:14:57PM +0800, Dongdong Liu wrote:
>> PCIe controller in Hip05/HIP06/HIP07 SoCs is not ECAM compliant.
>> It is non ECAM only for the RC bus config space;for any other bus
>> underneath the root bus we support ECAM access.
>> Add specific quirks for PCI config space accessors.This involves:
>> 1. New initialization call hisi_pcie_init() to obtain rc base
>> addresses from PNP0C02 as subdevice of PNP0A03.
>> 2. New entry in common quirk array.
>>
>> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
>> Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
>> ---
>>  MAINTAINERS                       |   1 +
>>  drivers/acpi/pci_mcfg.c           |  13 ++++
>>  drivers/pci/host/Kconfig          |   8 ++
>>  drivers/pci/host/Makefile         |   1 +
>>  drivers/pci/host/pcie-hisi-acpi.c | 157 ++++++++++++++++++++++++++++++++++++++
>>  include/linux/pci-ecam.h          |   5 ++
>>  6 files changed, 185 insertions(+)
>>  create mode 100644 drivers/pci/host/pcie-hisi-acpi.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 1cd38a7..b224caa 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -9358,6 +9358,7 @@ L:	linux-pci@vger.kernel.org
>>  S:	Maintained
>>  F:	Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
>>  F:	drivers/pci/host/pcie-hisi.c
>> +F:	drivers/pci/host/pcie-hisi-acpi.c
>>
>>  PCIE DRIVER FOR ROCKCHIP
>>  M:	Shawn Lin <shawn.lin@rock-chips.com>
>> diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
>> index ac21db3..b1b6fc7 100644
>> --- a/drivers/acpi/pci_mcfg.c
>> +++ b/drivers/acpi/pci_mcfg.c
>> @@ -57,6 +57,19 @@ struct mcfg_fixup {
>>  	{ "QCOM  ", "QDF2432 ", 1, 5, MCFG_BUS_ANY, &pci_32b_ops },
>>  	{ "QCOM  ", "QDF2432 ", 1, 6, MCFG_BUS_ANY, &pci_32b_ops },
>>  	{ "QCOM  ", "QDF2432 ", 1, 7, MCFG_BUS_ANY, &pci_32b_ops },
>> +#ifdef CONFIG_PCI_HISI_ACPI
>> +	#define PCI_ACPI_QUIRK_QUAD_DOM(table_id, seg, ops) \
>> +	{ "HISI  ", table_id, 0, seg + 0, MCFG_BUS_ANY, ops }, \
>> +	{ "HISI  ", table_id, 0, seg + 1, MCFG_BUS_ANY, ops }, \
>> +	{ "HISI  ", table_id, 0, seg + 2, MCFG_BUS_ANY, ops }, \
>> +	{ "HISI  ", table_id, 0, seg + 3, MCFG_BUS_ANY, ops }
>> +	PCI_ACPI_QUIRK_QUAD_DOM("HIP05   ", 0, &hisi_pcie_ops),
>> +	PCI_ACPI_QUIRK_QUAD_DOM("HIP06   ", 0, &hisi_pcie_ops),
>> +	PCI_ACPI_QUIRK_QUAD_DOM("HIP07   ", 0, &hisi_pcie_ops),
>> +	PCI_ACPI_QUIRK_QUAD_DOM("HIP07   ", 4, &hisi_pcie_ops),
>> +	PCI_ACPI_QUIRK_QUAD_DOM("HIP07   ", 8, &hisi_pcie_ops),
>> +	PCI_ACPI_QUIRK_QUAD_DOM("HIP07   ", 12, &hisi_pcie_ops),
>> +#endif
>>  };
>>
>>  static char mcfg_oem_id[ACPI_OEM_ID_SIZE];
>> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
>> index ae98644..9ff2bcd 100644
>> --- a/drivers/pci/host/Kconfig
>> +++ b/drivers/pci/host/Kconfig
>> @@ -227,6 +227,14 @@ config PCI_HISI
>>  	  Say Y here if you want PCIe controller support on HiSilicon
>>  	  Hip05 and Hip06 and Hip07 SoCs
>>
>> +config PCI_HISI_ACPI
>> +	depends on ACPI && ARM64
>> +	bool "HiSilicon Hip05 and Hip06 and Hip07 SoCs ACPI PCIe controllers"
>> +	select PNP
>> +	help
>> +	  Say Y here if you want ACPI PCIe controller support on HiSilicon
>> +	  Hip05 and Hip06 and Hip07 SoCs
>> +
>>  config PCIE_QCOM
>>  	bool "Qualcomm PCIe controller"
>>  	depends on ARCH_QCOM && OF
>> diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
>> index 084cb49..9402858 100644
>> --- a/drivers/pci/host/Makefile
>> +++ b/drivers/pci/host/Makefile
>> @@ -26,6 +26,7 @@ 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_PCI_HISI) += pcie-hisi.o
>> +obj-$(CONFIG_PCI_HISI_ACPI) += pcie-hisi-acpi.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
>> diff --git a/drivers/pci/host/pcie-hisi-acpi.c b/drivers/pci/host/pcie-hisi-acpi.c
>> new file mode 100644
>> index 0000000..aade4b5
>> --- /dev/null
>> +++ b/drivers/pci/host/pcie-hisi-acpi.c
>> @@ -0,0 +1,157 @@
>> +/*
>> + * PCIe host controller driver for HiSilicon HipXX SoCs
>> + *
>> + * Copyright (C) 2016 HiSilicon Co., Ltd. http://www.hisilicon.com
>> + *
>> + * Author: Dongdong Liu <liudongdong3@huawei.com>
>> + *         Gabriele Paoloni <gabriele.paoloni@huawei.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +#include <linux/pci.h>
>> +#include <linux/pci-acpi.h>
>> +#include <linux/pci-ecam.h>
>> +
>> +#define DEBUG0				0x728
>> +#define PCIE_LTSSM_LINKUP_STATE		0x11
>> +#define PCIE_LTSSM_STATE_MASK		0x3F
>
> These are now unused.

Thanks for pointing that, will delete them.

>
>> +static const struct acpi_device_id hisi_pcie_rc_res_ids[] = {
>> +	{"HISI0081", 0},
>> +	{"", 0},
>> +};
>> +
>> +static int hisi_pcie_acpi_rd_conf(struct pci_bus *bus, u32 devfn, int where,
>> +				  int size, u32 *val)
>> +{
>> +	struct pci_config_window *cfg = bus->sysdata;
>> +	int dev = PCI_SLOT(devfn);
>> +
>> +	if (bus->number == cfg->busr.start) {
>> +		/* access only one slot on each root port */
>> +		if (dev > 0)
>> +			return PCIBIOS_DEVICE_NOT_FOUND;
>> +		else
>> +			return pci_generic_config_read32(bus, devfn, where,
>> +							 size, val);
>> +	}
>> +
>> +	return pci_generic_config_read(bus, devfn, where, size, val);
>> +}
>> +
>> +static int hisi_pcie_acpi_wr_conf(struct pci_bus *bus, u32 devfn,
>> +				  int where, int size, u32 val)
>> +{
>> +	struct pci_config_window *cfg = bus->sysdata;
>> +	int dev = PCI_SLOT(devfn);
>> +
>> +	if (bus->number == cfg->busr.start) {
>> +		/* access only one slot on each root port */
>> +		if (dev > 0)
>> +			return PCIBIOS_DEVICE_NOT_FOUND;
>> +		else
>> +			return pci_generic_config_write32(bus, devfn, where,
>> +							  size, val);
>> +	}
>> +
>> +	return pci_generic_config_write(bus, devfn, where, size, val);
>> +}
>> +
>> +static void __iomem *hisi_pcie_map_bus(struct pci_bus *bus, unsigned int devfn,
>> +				       int where)
>> +{
>> +	struct pci_config_window *cfg = bus->sysdata;
>> +	void __iomem *reg_base = cfg->priv;
>> +
>> +	if (bus->number == cfg->busr.start)
>> +		return reg_base + where;
>> +	else
>> +		return pci_ecam_map_bus(bus, devfn, where);
>> +}
>> +
>> +/*
>> + * Retrieve RC base and size from sub-device under the RC
>> + * Device (RES1)
>> + * {
>> + *	Name (_HID, "HISI0081")
>> + *	Name (_CID, "PNP0C02")
>> + *	Name (_CRS, ResourceTemplate (){
>> + *		Memory32Fixed (ReadWrite, 0xb0080000, 0x10000)
>> + *	})
>> + * }
>> + */
>> +static int hisi_pcie_rc_addr_get(struct acpi_device *adev,
>> +				 void __iomem **addr)
>> +{
>> +	struct acpi_device *child_adev;
>> +	struct list_head list;
>> +	struct resource *res;
>> +	struct resource_entry *entry;
>> +	unsigned long flags;
>> +	int ret;
>> +
>> +	list_for_each_entry(child_adev, &adev->children, node) {
>> +		ret = acpi_match_device_ids(child_adev, hisi_pcie_rc_res_ids);
>> +		if (ret)
>> +			continue;
>> +
>> +		INIT_LIST_HEAD(&list);
>> +		flags = IORESOURCE_MEM;
>> +		ret = acpi_dev_get_resources(child_adev, &list,
>> +					     acpi_dev_filter_resource_type_cb,
>> +					     (void *)flags);
>> +		if (ret < 0) {
>> +			dev_err(&child_adev->dev,
>> +				"failed to parse _CRS method, error code %d\n",
>> +				ret);
>> +			return ret;
>> +		} else if (ret == 0) {
>> +			dev_err(&child_adev->dev,
>> +				"no IO and memory resources present in _CRS\n");
>> +			return -EINVAL;
>> +		}
>> +
>> +		entry = list_first_entry(&list, struct resource_entry, node);
>> +		res = entry->res;
>> +		*addr = devm_ioremap(&child_adev->dev,
>> +				     res->start, resource_size(res));
>> +		acpi_dev_free_resource_list(&list);
>> +		if (IS_ERR(*addr)) {
>> +			dev_err(&child_adev->dev, "error with ioremap\n");
>> +			return -ENOMEM;
>> +		}
>> +
>> +		return 0;
>> +	}
>> +
>> +	return -EINVAL;
>> +}
>
> OK, so MCFG (with the possible help of quirks) tells us where *most*
> of this RC's ECAM space is, but it doesn't tell us where the root bus
> ECAM space is, which is why we need this block of ugly code.  Right?
Yes, correct.
>
> Is there any way we can compute the root bus reg_base from the address
> we got from MCFG?

No, we can not,because RC base addresses are sequntial and MCFG impose a 4k boundaries.

> Is it a constant that realistically is not going to be modified?

Yes, this is a hardware limitation

>
> I would rather have the PNP0C02 device at the root of the ACPI
> namespace (under \_SB) if possible, the way x86 systems do it.
> I don't know whether that's actually a requirement, but the PCI
> Firmware Spec does suggest it, and it's better to follow that existing
> practice whenever possible.

Ok, will fix it in PATCH V5.

>
> I'm still looking for the specifics of how these resources are
> described, i.e., a dmesg log, /proc/iomem, and maybe an ACPI dump.
> These could go in a bugzilla entry, with a URL in this changelog.

Ok, will fix it in PATCH V5.

Thanks
Dongdong
>
>> +
>> +static int hisi_pcie_init(struct pci_config_window *cfg)
>> +{
>> +	int ret;
>> +	struct acpi_device *adev = to_acpi_device(cfg->parent);
>> +	void __iomem *reg_base;
>> +
>> +	ret = hisi_pcie_rc_addr_get(adev, &reg_base);
>> +	if (ret) {
>> +		dev_err(&adev->dev, "can't get rc base address");
>> +		return ret;
>> +	}
>> +
>> +	cfg->priv = reg_base;
>> +
>> +	return 0;
>> +}
>> +
>> +struct pci_ecam_ops hisi_pcie_ops = {
>> +	.bus_shift    = 20,
>> +	.init         =  hisi_pcie_init,
>> +	.pci_ops      = {
>> +		.map_bus    = hisi_pcie_map_bus,
>> +		.read       = hisi_pcie_acpi_rd_conf,
>> +		.write      = hisi_pcie_acpi_wr_conf,
>> +	}
>> +};
>> diff --git a/include/linux/pci-ecam.h b/include/linux/pci-ecam.h
>> index f5740b7..1b24d97 100644
>> --- a/include/linux/pci-ecam.h
>> +++ b/include/linux/pci-ecam.h
>> @@ -67,4 +67,9 @@ void __iomem *pci_ecam_map_bus(struct pci_bus *bus, unsigned int devfn,
>>  int pci_host_common_probe(struct platform_device *pdev,
>>  			  struct pci_ecam_ops *ops);
>>  #endif
>> +
>> +#ifdef CONFIG_PCI_HISI_ACPI
>> +extern struct pci_ecam_ops hisi_pcie_ops;
>> +#endif
>> +
>>  #endif
>> --
>> 1.9.1
>>
>
> .
>

Re: [PATCH V4 2/2] PCI/ACPI: hisi: Add ACPI support for HiSilicon SoCs Host Controllers

[Bjorn Helgaas]

On Wed, Nov 16, 2016 at 07:59:38PM +0800, Dongdong Liu wrote:
> Hi Bjorn
> 
> Many Thanks for your review
> 
> 在 2016/11/15 7:33, Bjorn Helgaas 写道:
> >On Wed, Nov 09, 2016 at 05:14:57PM +0800, Dongdong Liu wrote:
> >>PCIe controller in Hip05/HIP06/HIP07 SoCs is not ECAM compliant.
> >>It is non ECAM only for the RC bus config space;for any other bus
> >>underneath the root bus we support ECAM access.
> >>Add specific quirks for PCI config space accessors.This involves:
> >>1. New initialization call hisi_pcie_init() to obtain rc base
> >>addresses from PNP0C02 as subdevice of PNP0A03.
> >>2. New entry in common quirk array.
> >>
> >>Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
> >>Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
> >>---
> >> MAINTAINERS                       |   1 +
> >> drivers/acpi/pci_mcfg.c           |  13 ++++
> >> drivers/pci/host/Kconfig          |   8 ++
> >> drivers/pci/host/Makefile         |   1 +
> >> drivers/pci/host/pcie-hisi-acpi.c | 157 ++++++++++++++++++++++++++++++++++++++
> >> include/linux/pci-ecam.h          |   5 ++
> >> 6 files changed, 185 insertions(+)
> >> create mode 100644 drivers/pci/host/pcie-hisi-acpi.c
> >>
> >>diff --git a/MAINTAINERS b/MAINTAINERS
> >>index 1cd38a7..b224caa 100644
> >>--- a/MAINTAINERS
> >>+++ b/MAINTAINERS
> >>@@ -9358,6 +9358,7 @@ L:	linux-pci@vger.kernel.org
> >> S:	Maintained
> >> F:	Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
> >> F:	drivers/pci/host/pcie-hisi.c
> >>+F:	drivers/pci/host/pcie-hisi-acpi.c
> >>
> >> PCIE DRIVER FOR ROCKCHIP
> >> M:	Shawn Lin <shawn.lin@rock-chips.com>
> >>diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
> >>index ac21db3..b1b6fc7 100644
> >>--- a/drivers/acpi/pci_mcfg.c
> >>+++ b/drivers/acpi/pci_mcfg.c
> >>@@ -57,6 +57,19 @@ struct mcfg_fixup {
> >> 	{ "QCOM  ", "QDF2432 ", 1, 5, MCFG_BUS_ANY, &pci_32b_ops },
> >> 	{ "QCOM  ", "QDF2432 ", 1, 6, MCFG_BUS_ANY, &pci_32b_ops },
> >> 	{ "QCOM  ", "QDF2432 ", 1, 7, MCFG_BUS_ANY, &pci_32b_ops },
> >>+#ifdef CONFIG_PCI_HISI_ACPI
> >>+	#define PCI_ACPI_QUIRK_QUAD_DOM(table_id, seg, ops) \
> >>+	{ "HISI  ", table_id, 0, seg + 0, MCFG_BUS_ANY, ops }, \
> >>+	{ "HISI  ", table_id, 0, seg + 1, MCFG_BUS_ANY, ops }, \
> >>+	{ "HISI  ", table_id, 0, seg + 2, MCFG_BUS_ANY, ops }, \
> >>+	{ "HISI  ", table_id, 0, seg + 3, MCFG_BUS_ANY, ops }
> >>+	PCI_ACPI_QUIRK_QUAD_DOM("HIP05   ", 0, &hisi_pcie_ops),
> >>+	PCI_ACPI_QUIRK_QUAD_DOM("HIP06   ", 0, &hisi_pcie_ops),
> >>+	PCI_ACPI_QUIRK_QUAD_DOM("HIP07   ", 0, &hisi_pcie_ops),
> >>+	PCI_ACPI_QUIRK_QUAD_DOM("HIP07   ", 4, &hisi_pcie_ops),
> >>+	PCI_ACPI_QUIRK_QUAD_DOM("HIP07   ", 8, &hisi_pcie_ops),
> >>+	PCI_ACPI_QUIRK_QUAD_DOM("HIP07   ", 12, &hisi_pcie_ops),
> >>+#endif
> >> };
> >>
> >> static char mcfg_oem_id[ACPI_OEM_ID_SIZE];
> >>diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
> >>index ae98644..9ff2bcd 100644
> >>--- a/drivers/pci/host/Kconfig
> >>+++ b/drivers/pci/host/Kconfig
> >>@@ -227,6 +227,14 @@ config PCI_HISI
> >> 	  Say Y here if you want PCIe controller support on HiSilicon
> >> 	  Hip05 and Hip06 and Hip07 SoCs
> >>
> >>+config PCI_HISI_ACPI
> >>+	depends on ACPI && ARM64
> >>+	bool "HiSilicon Hip05 and Hip06 and Hip07 SoCs ACPI PCIe controllers"
> >>+	select PNP
> >>+	help
> >>+	  Say Y here if you want ACPI PCIe controller support on HiSilicon
> >>+	  Hip05 and Hip06 and Hip07 SoCs
> >>+
> >> config PCIE_QCOM
> >> 	bool "Qualcomm PCIe controller"
> >> 	depends on ARCH_QCOM && OF
> >>diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
> >>index 084cb49..9402858 100644
> >>--- a/drivers/pci/host/Makefile
> >>+++ b/drivers/pci/host/Makefile
> >>@@ -26,6 +26,7 @@ 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_PCI_HISI) += pcie-hisi.o
> >>+obj-$(CONFIG_PCI_HISI_ACPI) += pcie-hisi-acpi.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
> >>diff --git a/drivers/pci/host/pcie-hisi-acpi.c b/drivers/pci/host/pcie-hisi-acpi.c
> >>new file mode 100644
> >>index 0000000..aade4b5
> >>--- /dev/null
> >>+++ b/drivers/pci/host/pcie-hisi-acpi.c
> >>@@ -0,0 +1,157 @@
> >>+/*
> >>+ * PCIe host controller driver for HiSilicon HipXX SoCs
> >>+ *
> >>+ * Copyright (C) 2016 HiSilicon Co., Ltd. http://www.hisilicon.com
> >>+ *
> >>+ * Author: Dongdong Liu <liudongdong3@huawei.com>
> >>+ *         Gabriele Paoloni <gabriele.paoloni@huawei.com>
> >>+ *
> >>+ * This program is free software; you can redistribute it and/or modify
> >>+ * it under the terms of the GNU General Public License version 2 as
> >>+ * published by the Free Software Foundation.
> >>+ */
> >>+#include <linux/pci.h>
> >>+#include <linux/pci-acpi.h>
> >>+#include <linux/pci-ecam.h>
> >>+
> >>+#define DEBUG0				0x728
> >>+#define PCIE_LTSSM_LINKUP_STATE		0x11
> >>+#define PCIE_LTSSM_STATE_MASK		0x3F
> >
> >These are now unused.
> 
> Thanks for pointing that, will delete them.
> 
> >
> >>+static const struct acpi_device_id hisi_pcie_rc_res_ids[] = {
> >>+	{"HISI0081", 0},
> >>+	{"", 0},
> >>+};
> >>+
> >>+static int hisi_pcie_acpi_rd_conf(struct pci_bus *bus, u32 devfn, int where,
> >>+				  int size, u32 *val)
> >>+{
> >>+	struct pci_config_window *cfg = bus->sysdata;
> >>+	int dev = PCI_SLOT(devfn);
> >>+
> >>+	if (bus->number == cfg->busr.start) {
> >>+		/* access only one slot on each root port */
> >>+		if (dev > 0)
> >>+			return PCIBIOS_DEVICE_NOT_FOUND;
> >>+		else
> >>+			return pci_generic_config_read32(bus, devfn, where,
> >>+							 size, val);
> >>+	}
> >>+
> >>+	return pci_generic_config_read(bus, devfn, where, size, val);
> >>+}
> >>+
> >>+static int hisi_pcie_acpi_wr_conf(struct pci_bus *bus, u32 devfn,
> >>+				  int where, int size, u32 val)
> >>+{
> >>+	struct pci_config_window *cfg = bus->sysdata;
> >>+	int dev = PCI_SLOT(devfn);
> >>+
> >>+	if (bus->number == cfg->busr.start) {
> >>+		/* access only one slot on each root port */
> >>+		if (dev > 0)
> >>+			return PCIBIOS_DEVICE_NOT_FOUND;
> >>+		else
> >>+			return pci_generic_config_write32(bus, devfn, where,
> >>+							  size, val);
> >>+	}
> >>+
> >>+	return pci_generic_config_write(bus, devfn, where, size, val);
> >>+}
> >>+
> >>+static void __iomem *hisi_pcie_map_bus(struct pci_bus *bus, unsigned int devfn,
> >>+				       int where)
> >>+{
> >>+	struct pci_config_window *cfg = bus->sysdata;
> >>+	void __iomem *reg_base = cfg->priv;
> >>+
> >>+	if (bus->number == cfg->busr.start)
> >>+		return reg_base + where;
> >>+	else
> >>+		return pci_ecam_map_bus(bus, devfn, where);
> >>+}
> >>+
> >>+/*
> >>+ * Retrieve RC base and size from sub-device under the RC
> >>+ * Device (RES1)
> >>+ * {
> >>+ *	Name (_HID, "HISI0081")
> >>+ *	Name (_CID, "PNP0C02")
> >>+ *	Name (_CRS, ResourceTemplate (){
> >>+ *		Memory32Fixed (ReadWrite, 0xb0080000, 0x10000)
> >>+ *	})
> >>+ * }
> >>+ */
> >>+static int hisi_pcie_rc_addr_get(struct acpi_device *adev,
> >>+				 void __iomem **addr)
> >>+{
> >>+	struct acpi_device *child_adev;
> >>+	struct list_head list;
> >>+	struct resource *res;
> >>+	struct resource_entry *entry;
> >>+	unsigned long flags;
> >>+	int ret;
> >>+
> >>+	list_for_each_entry(child_adev, &adev->children, node) {
> >>+		ret = acpi_match_device_ids(child_adev, hisi_pcie_rc_res_ids);
> >>+		if (ret)
> >>+			continue;
> >>+
> >>+		INIT_LIST_HEAD(&list);
> >>+		flags = IORESOURCE_MEM;
> >>+		ret = acpi_dev_get_resources(child_adev, &list,
> >>+					     acpi_dev_filter_resource_type_cb,
> >>+					     (void *)flags);
> >>+		if (ret < 0) {
> >>+			dev_err(&child_adev->dev,
> >>+				"failed to parse _CRS method, error code %d\n",
> >>+				ret);
> >>+			return ret;
> >>+		} else if (ret == 0) {
> >>+			dev_err(&child_adev->dev,
> >>+				"no IO and memory resources present in _CRS\n");
> >>+			return -EINVAL;
> >>+		}
> >>+
> >>+		entry = list_first_entry(&list, struct resource_entry, node);
> >>+		res = entry->res;
> >>+		*addr = devm_ioremap(&child_adev->dev,
> >>+				     res->start, resource_size(res));
> >>+		acpi_dev_free_resource_list(&list);
> >>+		if (IS_ERR(*addr)) {
> >>+			dev_err(&child_adev->dev, "error with ioremap\n");
> >>+			return -ENOMEM;
> >>+		}
> >>+
> >>+		return 0;
> >>+	}
> >>+
> >>+	return -EINVAL;
> >>+}
> >
> >OK, so MCFG (with the possible help of quirks) tells us where *most*
> >of this RC's ECAM space is, but it doesn't tell us where the root bus
> >ECAM space is, which is why we need this block of ugly code.  Right?
> Yes, correct.
> >
> >Is there any way we can compute the root bus reg_base from the address
> >we got from MCFG?
> 
> No, we can not,because RC base addresses are sequntial and MCFG impose a 4k boundaries.

I understand the "no" part, but not the "because ..." part :)

I think the regions described by MCFG are aligned on 1MB boundaries,
not 4K, because MCFG tells you about ECAM space for a given bus number
range.  Each bus can have 256 devices (5 bit device number + 3 bit
function number), and each device has 4K of config space.  256 * 4KB =
1MB.

In any case, it wouldn't have to be a simple function like this:

  reg_base = pci_mcfg_lookup(seg, [bus 01]) - 1MB

It could be *anything*, e.g.,

  reg_base = 0x00100000 + (bus_num * 4096);

> >Is it a constant that realistically is not going to be modified?
> 
> Yes, this is a hardware limitation

If there's only one RC and the address is a constant, maybe you can
just hard-code the address in the .map_bus() function?

Bjorn

Re: [PATCH V4 2/2] PCI/ACPI: hisi: Add ACPI support for HiSilicon SoCs Host Controllers

[Bjorn Helgaas]

On Mon, Nov 14, 2016 at 05:33:20PM -0600, Bjorn Helgaas wrote:
> On Wed, Nov 09, 2016 at 05:14:57PM +0800, Dongdong Liu wrote:
> > PCIe controller in Hip05/HIP06/HIP07 SoCs is not ECAM compliant.
> > It is non ECAM only for the RC bus config space;for any other bus
> > underneath the root bus we support ECAM access.
> > Add specific quirks for PCI config space accessors.This involves:
> > 1. New initialization call hisi_pcie_init() to obtain rc base
> > addresses from PNP0C02 as subdevice of PNP0A03.
> > 2. New entry in common quirk array.
> > 
> > Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
> > Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
> > ---
> >  MAINTAINERS                       |   1 +
> >  drivers/acpi/pci_mcfg.c           |  13 ++++
> >  drivers/pci/host/Kconfig          |   8 ++
> >  drivers/pci/host/Makefile         |   1 +
> >  drivers/pci/host/pcie-hisi-acpi.c | 157 ++++++++++++++++++++++++++++++++++++++
> >  include/linux/pci-ecam.h          |   5 ++
> >  6 files changed, 185 insertions(+)
> >  create mode 100644 drivers/pci/host/pcie-hisi-acpi.c
> > 
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 1cd38a7..b224caa 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -9358,6 +9358,7 @@ L:	linux-pci@vger.kernel.org
> >  S:	Maintained
> >  F:	Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
> >  F:	drivers/pci/host/pcie-hisi.c
> > +F:	drivers/pci/host/pcie-hisi-acpi.c
> >  
> >  PCIE DRIVER FOR ROCKCHIP
> >  M:	Shawn Lin <shawn.lin@rock-chips.com>
> > diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
> > index ac21db3..b1b6fc7 100644
> > --- a/drivers/acpi/pci_mcfg.c
> > +++ b/drivers/acpi/pci_mcfg.c
> > @@ -57,6 +57,19 @@ struct mcfg_fixup {
> >  	{ "QCOM  ", "QDF2432 ", 1, 5, MCFG_BUS_ANY, &pci_32b_ops },
> >  	{ "QCOM  ", "QDF2432 ", 1, 6, MCFG_BUS_ANY, &pci_32b_ops },
> >  	{ "QCOM  ", "QDF2432 ", 1, 7, MCFG_BUS_ANY, &pci_32b_ops },
> > +#ifdef CONFIG_PCI_HISI_ACPI
> > +	#define PCI_ACPI_QUIRK_QUAD_DOM(table_id, seg, ops) \
> > +	{ "HISI  ", table_id, 0, seg + 0, MCFG_BUS_ANY, ops }, \
> > +	{ "HISI  ", table_id, 0, seg + 1, MCFG_BUS_ANY, ops }, \
> > +	{ "HISI  ", table_id, 0, seg + 2, MCFG_BUS_ANY, ops }, \
> > +	{ "HISI  ", table_id, 0, seg + 3, MCFG_BUS_ANY, ops }
> > +	PCI_ACPI_QUIRK_QUAD_DOM("HIP05   ", 0, &hisi_pcie_ops),
> > +	PCI_ACPI_QUIRK_QUAD_DOM("HIP06   ", 0, &hisi_pcie_ops),
> > +	PCI_ACPI_QUIRK_QUAD_DOM("HIP07   ", 0, &hisi_pcie_ops),
> > +	PCI_ACPI_QUIRK_QUAD_DOM("HIP07   ", 4, &hisi_pcie_ops),
> > +	PCI_ACPI_QUIRK_QUAD_DOM("HIP07   ", 8, &hisi_pcie_ops),
> > +	PCI_ACPI_QUIRK_QUAD_DOM("HIP07   ", 12, &hisi_pcie_ops),
> > +#endif
> >  };
> >  
> >  static char mcfg_oem_id[ACPI_OEM_ID_SIZE];
> > diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
> > index ae98644..9ff2bcd 100644
> > --- a/drivers/pci/host/Kconfig
> > +++ b/drivers/pci/host/Kconfig
> > @@ -227,6 +227,14 @@ config PCI_HISI
> >  	  Say Y here if you want PCIe controller support on HiSilicon
> >  	  Hip05 and Hip06 and Hip07 SoCs
> >  
> > +config PCI_HISI_ACPI
> > +	depends on ACPI && ARM64
> > +	bool "HiSilicon Hip05 and Hip06 and Hip07 SoCs ACPI PCIe controllers"
> > +	select PNP
> > +	help
> > +	  Say Y here if you want ACPI PCIe controller support on HiSilicon
> > +	  Hip05 and Hip06 and Hip07 SoCs
> > +
> >  config PCIE_QCOM
> >  	bool "Qualcomm PCIe controller"
> >  	depends on ARCH_QCOM && OF
> > diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
> > index 084cb49..9402858 100644
> > --- a/drivers/pci/host/Makefile
> > +++ b/drivers/pci/host/Makefile
> > @@ -26,6 +26,7 @@ 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_PCI_HISI) += pcie-hisi.o
> > +obj-$(CONFIG_PCI_HISI_ACPI) += pcie-hisi-acpi.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
> > diff --git a/drivers/pci/host/pcie-hisi-acpi.c b/drivers/pci/host/pcie-hisi-acpi.c
> > new file mode 100644
> > index 0000000..aade4b5
> > --- /dev/null
> > +++ b/drivers/pci/host/pcie-hisi-acpi.c
> > @@ -0,0 +1,157 @@
> > +/*
> > + * PCIe host controller driver for HiSilicon HipXX SoCs
> > + *
> > + * Copyright (C) 2016 HiSilicon Co., Ltd. http://www.hisilicon.com
> > + *
> > + * Author: Dongdong Liu <liudongdong3@huawei.com>
> > + *         Gabriele Paoloni <gabriele.paoloni@huawei.com>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + */
> > +#include <linux/pci.h>
> > +#include <linux/pci-acpi.h>
> > +#include <linux/pci-ecam.h>
> > +
> > +#define DEBUG0				0x728
> > +#define PCIE_LTSSM_LINKUP_STATE		0x11
> > +#define PCIE_LTSSM_STATE_MASK		0x3F
> 
> These are now unused.
> 
> > +static const struct acpi_device_id hisi_pcie_rc_res_ids[] = {
> > +	{"HISI0081", 0},
> > +	{"", 0},
> > +};
> > +
> > +static int hisi_pcie_acpi_rd_conf(struct pci_bus *bus, u32 devfn, int where,
> > +				  int size, u32 *val)
> > +{
> > +	struct pci_config_window *cfg = bus->sysdata;
> > +	int dev = PCI_SLOT(devfn);
> > +
> > +	if (bus->number == cfg->busr.start) {
> > +		/* access only one slot on each root port */
> > +		if (dev > 0)
> > +			return PCIBIOS_DEVICE_NOT_FOUND;
> > +		else
> > +			return pci_generic_config_read32(bus, devfn, where,
> > +							 size, val);
> > +	}
> > +
> > +	return pci_generic_config_read(bus, devfn, where, size, val);
> > +}
> > +
> > +static int hisi_pcie_acpi_wr_conf(struct pci_bus *bus, u32 devfn,
> > +				  int where, int size, u32 val)
> > +{
> > +	struct pci_config_window *cfg = bus->sysdata;
> > +	int dev = PCI_SLOT(devfn);
> > +
> > +	if (bus->number == cfg->busr.start) {
> > +		/* access only one slot on each root port */
> > +		if (dev > 0)
> > +			return PCIBIOS_DEVICE_NOT_FOUND;
> > +		else
> > +			return pci_generic_config_write32(bus, devfn, where,
> > +							  size, val);
> > +	}
> > +
> > +	return pci_generic_config_write(bus, devfn, where, size, val);
> > +}
> > +
> > +static void __iomem *hisi_pcie_map_bus(struct pci_bus *bus, unsigned int devfn,
> > +				       int where)
> > +{
> > +	struct pci_config_window *cfg = bus->sysdata;
> > +	void __iomem *reg_base = cfg->priv;
> > +
> > +	if (bus->number == cfg->busr.start)
> > +		return reg_base + where;
> > +	else
> > +		return pci_ecam_map_bus(bus, devfn, where);
> > +}
> > +
> > +/*
> > + * Retrieve RC base and size from sub-device under the RC
> > + * Device (RES1)
> > + * {
> > + *	Name (_HID, "HISI0081")
> > + *	Name (_CID, "PNP0C02")
> > + *	Name (_CRS, ResourceTemplate (){
> > + *		Memory32Fixed (ReadWrite, 0xb0080000, 0x10000)
> > + *	})
> > + * }
> > + */
> > +static int hisi_pcie_rc_addr_get(struct acpi_device *adev,
> > +				 void __iomem **addr)
> > +{
> > +	struct acpi_device *child_adev;
> > +	struct list_head list;
> > +	struct resource *res;
> > +	struct resource_entry *entry;
> > +	unsigned long flags;
> > +	int ret;
> > +
> > +	list_for_each_entry(child_adev, &adev->children, node) {
> > +		ret = acpi_match_device_ids(child_adev, hisi_pcie_rc_res_ids);
> > +		if (ret)
> > +			continue;
> > +
> > +		INIT_LIST_HEAD(&list);
> > +		flags = IORESOURCE_MEM;
> > +		ret = acpi_dev_get_resources(child_adev, &list,
> > +					     acpi_dev_filter_resource_type_cb,
> > +					     (void *)flags);
> > +		if (ret < 0) {
> > +			dev_err(&child_adev->dev,
> > +				"failed to parse _CRS method, error code %d\n",
> > +				ret);
> > +			return ret;
> > +		} else if (ret == 0) {
> > +			dev_err(&child_adev->dev,
> > +				"no IO and memory resources present in _CRS\n");
> > +			return -EINVAL;
> > +		}
> > +
> > +		entry = list_first_entry(&list, struct resource_entry, node);
> > +		res = entry->res;
> > +		*addr = devm_ioremap(&child_adev->dev,
> > +				     res->start, resource_size(res));
> > +		acpi_dev_free_resource_list(&list);
> > +		if (IS_ERR(*addr)) {
> > +			dev_err(&child_adev->dev, "error with ioremap\n");
> > +			return -ENOMEM;
> > +		}
> > +
> > +		return 0;
> > +	}
> > +
> > +	return -EINVAL;
> > +}
> 
> OK, so MCFG (with the possible help of quirks) tells us where *most*
> of this RC's ECAM space is, but it doesn't tell us where the root bus
> ECAM space is, which is why we need this block of ugly code.  Right?

Tomasz is doing the same thing for ThunderX.  Can we share the code
somehow, e.g., add something that takes the _HID to look for and just
returns the register address?  Maybe in pci-acpi.c, decl in
drivers/pci/pci.h, under #ifdef CONFIG_ARM64?0

Re: [PATCH V4 2/2] PCI/ACPI: hisi: Add ACPI support for HiSilicon SoCs Host Controllers

[Dongdong Liu]

Hi Bjorn

在 2016/11/17 3:31, Bjorn Helgaas 写道:
> On Wed, Nov 16, 2016 at 07:59:38PM +0800, Dongdong Liu wrote:
>> Hi Bjorn
>>
>> Many Thanks for your review
>>
>> 在 2016/11/15 7:33, Bjorn Helgaas 写道:
>>> On Wed, Nov 09, 2016 at 05:14:57PM +0800, Dongdong Liu wrote:
>>>> PCIe controller in Hip05/HIP06/HIP07 SoCs is not ECAM compliant.
>>>> It is non ECAM only for the RC bus config space;for any other bus
>>>> underneath the root bus we support ECAM access.
>>>> Add specific quirks for PCI config space accessors.This involves:
>>>> 1. New initialization call hisi_pcie_init() to obtain rc base
>>>> addresses from PNP0C02 as subdevice of PNP0A03.
>>>> 2. New entry in common quirk array.
>>>>
>>>> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
>>>> Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
>>>> ---
>>>> MAINTAINERS                       |   1 +
>>>> drivers/acpi/pci_mcfg.c           |  13 ++++
>>>> drivers/pci/host/Kconfig          |   8 ++
>>>> drivers/pci/host/Makefile         |   1 +
>>>> drivers/pci/host/pcie-hisi-acpi.c | 157 ++++++++++++++++++++++++++++++++++++++
>>>> include/linux/pci-ecam.h          |   5 ++
>>>> 6 files changed, 185 insertions(+)
>>>> create mode 100644 drivers/pci/host/pcie-hisi-acpi.c
>>>>
>>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>>> index 1cd38a7..b224caa 100644
>>>> --- a/MAINTAINERS
>>>> +++ b/MAINTAINERS
>>>> @@ -9358,6 +9358,7 @@ L:	linux-pci@vger.kernel.org
>>>> S:	Maintained
>>>> F:	Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
>>>> F:	drivers/pci/host/pcie-hisi.c
>>>> +F:	drivers/pci/host/pcie-hisi-acpi.c
>>>>
>>>> PCIE DRIVER FOR ROCKCHIP
>>>> M:	Shawn Lin <shawn.lin@rock-chips.com>
>>>> diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
>>>> index ac21db3..b1b6fc7 100644
>>>> --- a/drivers/acpi/pci_mcfg.c
>>>> +++ b/drivers/acpi/pci_mcfg.c
>>>> @@ -57,6 +57,19 @@ struct mcfg_fixup {
>>>> 	{ "QCOM  ", "QDF2432 ", 1, 5, MCFG_BUS_ANY, &pci_32b_ops },
>>>> 	{ "QCOM  ", "QDF2432 ", 1, 6, MCFG_BUS_ANY, &pci_32b_ops },
>>>> 	{ "QCOM  ", "QDF2432 ", 1, 7, MCFG_BUS_ANY, &pci_32b_ops },
>>>> +#ifdef CONFIG_PCI_HISI_ACPI
>>>> +	#define PCI_ACPI_QUIRK_QUAD_DOM(table_id, seg, ops) \
>>>> +	{ "HISI  ", table_id, 0, seg + 0, MCFG_BUS_ANY, ops }, \
>>>> +	{ "HISI  ", table_id, 0, seg + 1, MCFG_BUS_ANY, ops }, \
>>>> +	{ "HISI  ", table_id, 0, seg + 2, MCFG_BUS_ANY, ops }, \
>>>> +	{ "HISI  ", table_id, 0, seg + 3, MCFG_BUS_ANY, ops }
>>>> +	PCI_ACPI_QUIRK_QUAD_DOM("HIP05   ", 0, &hisi_pcie_ops),
>>>> +	PCI_ACPI_QUIRK_QUAD_DOM("HIP06   ", 0, &hisi_pcie_ops),
>>>> +	PCI_ACPI_QUIRK_QUAD_DOM("HIP07   ", 0, &hisi_pcie_ops),
>>>> +	PCI_ACPI_QUIRK_QUAD_DOM("HIP07   ", 4, &hisi_pcie_ops),
>>>> +	PCI_ACPI_QUIRK_QUAD_DOM("HIP07   ", 8, &hisi_pcie_ops),
>>>> +	PCI_ACPI_QUIRK_QUAD_DOM("HIP07   ", 12, &hisi_pcie_ops),
>>>> +#endif
>>>> };
>>>>
>>>> static char mcfg_oem_id[ACPI_OEM_ID_SIZE];
>>>> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
>>>> index ae98644..9ff2bcd 100644
>>>> --- a/drivers/pci/host/Kconfig
>>>> +++ b/drivers/pci/host/Kconfig
>>>> @@ -227,6 +227,14 @@ config PCI_HISI
>>>> 	  Say Y here if you want PCIe controller support on HiSilicon
>>>> 	  Hip05 and Hip06 and Hip07 SoCs
>>>>
>>>> +config PCI_HISI_ACPI
>>>> +	depends on ACPI && ARM64
>>>> +	bool "HiSilicon Hip05 and Hip06 and Hip07 SoCs ACPI PCIe controllers"
>>>> +	select PNP
>>>> +	help
>>>> +	  Say Y here if you want ACPI PCIe controller support on HiSilicon
>>>> +	  Hip05 and Hip06 and Hip07 SoCs
>>>> +
>>>> config PCIE_QCOM
>>>> 	bool "Qualcomm PCIe controller"
>>>> 	depends on ARCH_QCOM && OF
>>>> diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
>>>> index 084cb49..9402858 100644
>>>> --- a/drivers/pci/host/Makefile
>>>> +++ b/drivers/pci/host/Makefile
>>>> @@ -26,6 +26,7 @@ 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_PCI_HISI) += pcie-hisi.o
>>>> +obj-$(CONFIG_PCI_HISI_ACPI) += pcie-hisi-acpi.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
>>>> diff --git a/drivers/pci/host/pcie-hisi-acpi.c b/drivers/pci/host/pcie-hisi-acpi.c
>>>> new file mode 100644
>>>> index 0000000..aade4b5
>>>> --- /dev/null
>>>> +++ b/drivers/pci/host/pcie-hisi-acpi.c
>>>> @@ -0,0 +1,157 @@
>>>> +/*
>>>> + * PCIe host controller driver for HiSilicon HipXX SoCs
>>>> + *
>>>> + * Copyright (C) 2016 HiSilicon Co., Ltd. http://www.hisilicon.com
>>>> + *
>>>> + * Author: Dongdong Liu <liudongdong3@huawei.com>
>>>> + *         Gabriele Paoloni <gabriele.paoloni@huawei.com>
>>>> + *
>>>> + * This program is free software; you can redistribute it and/or modify
>>>> + * it under the terms of the GNU General Public License version 2 as
>>>> + * published by the Free Software Foundation.
>>>> + */
>>>> +#include <linux/pci.h>
>>>> +#include <linux/pci-acpi.h>
>>>> +#include <linux/pci-ecam.h>
>>>> +
>>>> +#define DEBUG0				0x728
>>>> +#define PCIE_LTSSM_LINKUP_STATE		0x11
>>>> +#define PCIE_LTSSM_STATE_MASK		0x3F
>>>
>>> These are now unused.
>>
>> Thanks for pointing that, will delete them.
>>
>>>
>>>> +static const struct acpi_device_id hisi_pcie_rc_res_ids[] = {
>>>> +	{"HISI0081", 0},
>>>> +	{"", 0},
>>>> +};
>>>> +
>>>> +static int hisi_pcie_acpi_rd_conf(struct pci_bus *bus, u32 devfn, int where,
>>>> +				  int size, u32 *val)
>>>> +{
>>>> +	struct pci_config_window *cfg = bus->sysdata;
>>>> +	int dev = PCI_SLOT(devfn);
>>>> +
>>>> +	if (bus->number == cfg->busr.start) {
>>>> +		/* access only one slot on each root port */
>>>> +		if (dev > 0)
>>>> +			return PCIBIOS_DEVICE_NOT_FOUND;
>>>> +		else
>>>> +			return pci_generic_config_read32(bus, devfn, where,
>>>> +							 size, val);
>>>> +	}
>>>> +
>>>> +	return pci_generic_config_read(bus, devfn, where, size, val);
>>>> +}
>>>> +
>>>> +static int hisi_pcie_acpi_wr_conf(struct pci_bus *bus, u32 devfn,
>>>> +				  int where, int size, u32 val)
>>>> +{
>>>> +	struct pci_config_window *cfg = bus->sysdata;
>>>> +	int dev = PCI_SLOT(devfn);
>>>> +
>>>> +	if (bus->number == cfg->busr.start) {
>>>> +		/* access only one slot on each root port */
>>>> +		if (dev > 0)
>>>> +			return PCIBIOS_DEVICE_NOT_FOUND;
>>>> +		else
>>>> +			return pci_generic_config_write32(bus, devfn, where,
>>>> +							  size, val);
>>>> +	}
>>>> +
>>>> +	return pci_generic_config_write(bus, devfn, where, size, val);
>>>> +}
>>>> +
>>>> +static void __iomem *hisi_pcie_map_bus(struct pci_bus *bus, unsigned int devfn,
>>>> +				       int where)
>>>> +{
>>>> +	struct pci_config_window *cfg = bus->sysdata;
>>>> +	void __iomem *reg_base = cfg->priv;
>>>> +
>>>> +	if (bus->number == cfg->busr.start)
>>>> +		return reg_base + where;
>>>> +	else
>>>> +		return pci_ecam_map_bus(bus, devfn, where);
>>>> +}
>>>> +
>>>> +/*
>>>> + * Retrieve RC base and size from sub-device under the RC
>>>> + * Device (RES1)
>>>> + * {
>>>> + *	Name (_HID, "HISI0081")
>>>> + *	Name (_CID, "PNP0C02")
>>>> + *	Name (_CRS, ResourceTemplate (){
>>>> + *		Memory32Fixed (ReadWrite, 0xb0080000, 0x10000)
>>>> + *	})
>>>> + * }
>>>> + */
>>>> +static int hisi_pcie_rc_addr_get(struct acpi_device *adev,
>>>> +				 void __iomem **addr)
>>>> +{
>>>> +	struct acpi_device *child_adev;
>>>> +	struct list_head list;
>>>> +	struct resource *res;
>>>> +	struct resource_entry *entry;
>>>> +	unsigned long flags;
>>>> +	int ret;
>>>> +
>>>> +	list_for_each_entry(child_adev, &adev->children, node) {
>>>> +		ret = acpi_match_device_ids(child_adev, hisi_pcie_rc_res_ids);
>>>> +		if (ret)
>>>> +			continue;
>>>> +
>>>> +		INIT_LIST_HEAD(&list);
>>>> +		flags = IORESOURCE_MEM;
>>>> +		ret = acpi_dev_get_resources(child_adev, &list,
>>>> +					     acpi_dev_filter_resource_type_cb,
>>>> +					     (void *)flags);
>>>> +		if (ret < 0) {
>>>> +			dev_err(&child_adev->dev,
>>>> +				"failed to parse _CRS method, error code %d\n",
>>>> +				ret);
>>>> +			return ret;
>>>> +		} else if (ret == 0) {
>>>> +			dev_err(&child_adev->dev,
>>>> +				"no IO and memory resources present in _CRS\n");
>>>> +			return -EINVAL;
>>>> +		}
>>>> +
>>>> +		entry = list_first_entry(&list, struct resource_entry, node);
>>>> +		res = entry->res;
>>>> +		*addr = devm_ioremap(&child_adev->dev,
>>>> +				     res->start, resource_size(res));
>>>> +		acpi_dev_free_resource_list(&list);
>>>> +		if (IS_ERR(*addr)) {
>>>> +			dev_err(&child_adev->dev, "error with ioremap\n");
>>>> +			return -ENOMEM;
>>>> +		}
>>>> +
>>>> +		return 0;
>>>> +	}
>>>> +
>>>> +	return -EINVAL;
>>>> +}
>>>
>>> OK, so MCFG (with the possible help of quirks) tells us where *most*
>>> of this RC's ECAM space is, but it doesn't tell us where the root bus
>>> ECAM space is, which is why we need this block of ugly code.  Right?
>> Yes, correct.
>>>
>>> Is there any way we can compute the root bus reg_base from the address
>>> we got from MCFG?
>>
>> No, we can not,because RC base addresses are sequntial and MCFG impose a 4k boundaries.
>
> I understand the "no" part, but not the "because ..." part :)
>
> I think the regions described by MCFG are aligned on 1MB boundaries,
> not 4K, because MCFG tells you about ECAM space for a given bus number
> range.  Each bus can have 256 devices (5 bit device number + 3 bit
> function number), and each device has 4K of config space.  256 * 4KB =
> 1MB.

Yes, your are right, thanks for your explanation.

>
> In any case, it wouldn't have to be a simple function like this:
>
>   reg_base = pci_mcfg_lookup(seg, [bus 01]) - 1MB
>
> It could be *anything*, e.g.,
>
>   reg_base = 0x00100000 + (bus_num * 4096);

Our hip06 SoC has 4 RCs. Each RCs has one root port. The RCs base addresses are as below.

RC0 0xa0090000~0xa009ffff
RC2 0xa00a0000~0xa00affff
RC3 0xa00b0000~0xa00bffff
RC1 0xa0200000~0xa020ffff

The size is 64K ,the base addresses can be changed by UEFI but the size is fixed.
4k is for RC itself config space, the left 60K spaces are for other uses.

The regions described by MCFG are aligned on 1MB boundaries,
So we can not get our RC base adreses from MCFG.
>
>>> Is it a constant that realistically is not going to be modified?
>>
>> Yes, this is a hardware limitation
>
> If there's only one RC and the address is a constant, maybe you can
> just hard-code the address in the .map_bus() function?

As above said, the base adresses can be changed by UEFI but the size is fixed.
hard-code maybe not a good way.

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

Re: [PATCH V4 2/2] PCI/ACPI: hisi: Add ACPI support for HiSilicon SoCs Host Controllers

[Dongdong Liu]

Hi Bjorn
在 2016/11/17 7:00, Bjorn Helgaas 写道:
> On Mon, Nov 14, 2016 at 05:33:20PM -0600, Bjorn Helgaas wrote:
>> On Wed, Nov 09, 2016 at 05:14:57PM +0800, Dongdong Liu wrote:
>>> PCIe controller in Hip05/HIP06/HIP07 SoCs is not ECAM compliant.
>>> It is non ECAM only for the RC bus config space;for any other bus
>>> underneath the root bus we support ECAM access.
>>> Add specific quirks for PCI config space accessors.This involves:
>>> 1. New initialization call hisi_pcie_init() to obtain rc base
>>> addresses from PNP0C02 as subdevice of PNP0A03.
>>> 2. New entry in common quirk array.
>>>
>>> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
>>> Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
>>> ---
>>>  MAINTAINERS                       |   1 +
>>>  drivers/acpi/pci_mcfg.c           |  13 ++++
>>>  drivers/pci/host/Kconfig          |   8 ++
>>>  drivers/pci/host/Makefile         |   1 +
>>>  drivers/pci/host/pcie-hisi-acpi.c | 157 ++++++++++++++++++++++++++++++++++++++
>>>  include/linux/pci-ecam.h          |   5 ++
>>>  6 files changed, 185 insertions(+)
>>>  create mode 100644 drivers/pci/host/pcie-hisi-acpi.c
>>>
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index 1cd38a7..b224caa 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -9358,6 +9358,7 @@ L:	linux-pci@vger.kernel.org
>>>  S:	Maintained
>>>  F:	Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
>>>  F:	drivers/pci/host/pcie-hisi.c
>>> +F:	drivers/pci/host/pcie-hisi-acpi.c
>>>
>>>  PCIE DRIVER FOR ROCKCHIP
>>>  M:	Shawn Lin <shawn.lin@rock-chips.com>
>>> diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
>>> index ac21db3..b1b6fc7 100644
>>> --- a/drivers/acpi/pci_mcfg.c
>>> +++ b/drivers/acpi/pci_mcfg.c
>>> @@ -57,6 +57,19 @@ struct mcfg_fixup {
>>>  	{ "QCOM  ", "QDF2432 ", 1, 5, MCFG_BUS_ANY, &pci_32b_ops },
>>>  	{ "QCOM  ", "QDF2432 ", 1, 6, MCFG_BUS_ANY, &pci_32b_ops },
>>>  	{ "QCOM  ", "QDF2432 ", 1, 7, MCFG_BUS_ANY, &pci_32b_ops },
>>> +#ifdef CONFIG_PCI_HISI_ACPI
>>> +	#define PCI_ACPI_QUIRK_QUAD_DOM(table_id, seg, ops) \
>>> +	{ "HISI  ", table_id, 0, seg + 0, MCFG_BUS_ANY, ops }, \
>>> +	{ "HISI  ", table_id, 0, seg + 1, MCFG_BUS_ANY, ops }, \
>>> +	{ "HISI  ", table_id, 0, seg + 2, MCFG_BUS_ANY, ops }, \
>>> +	{ "HISI  ", table_id, 0, seg + 3, MCFG_BUS_ANY, ops }
>>> +	PCI_ACPI_QUIRK_QUAD_DOM("HIP05   ", 0, &hisi_pcie_ops),
>>> +	PCI_ACPI_QUIRK_QUAD_DOM("HIP06   ", 0, &hisi_pcie_ops),
>>> +	PCI_ACPI_QUIRK_QUAD_DOM("HIP07   ", 0, &hisi_pcie_ops),
>>> +	PCI_ACPI_QUIRK_QUAD_DOM("HIP07   ", 4, &hisi_pcie_ops),
>>> +	PCI_ACPI_QUIRK_QUAD_DOM("HIP07   ", 8, &hisi_pcie_ops),
>>> +	PCI_ACPI_QUIRK_QUAD_DOM("HIP07   ", 12, &hisi_pcie_ops),
>>> +#endif
>>>  };
>>>
>>>  static char mcfg_oem_id[ACPI_OEM_ID_SIZE];
>>> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
>>> index ae98644..9ff2bcd 100644
>>> --- a/drivers/pci/host/Kconfig
>>> +++ b/drivers/pci/host/Kconfig
>>> @@ -227,6 +227,14 @@ config PCI_HISI
>>>  	  Say Y here if you want PCIe controller support on HiSilicon
>>>  	  Hip05 and Hip06 and Hip07 SoCs
>>>
>>> +config PCI_HISI_ACPI
>>> +	depends on ACPI && ARM64
>>> +	bool "HiSilicon Hip05 and Hip06 and Hip07 SoCs ACPI PCIe controllers"
>>> +	select PNP
>>> +	help
>>> +	  Say Y here if you want ACPI PCIe controller support on HiSilicon
>>> +	  Hip05 and Hip06 and Hip07 SoCs
>>> +
>>>  config PCIE_QCOM
>>>  	bool "Qualcomm PCIe controller"
>>>  	depends on ARCH_QCOM && OF
>>> diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
>>> index 084cb49..9402858 100644
>>> --- a/drivers/pci/host/Makefile
>>> +++ b/drivers/pci/host/Makefile
>>> @@ -26,6 +26,7 @@ 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_PCI_HISI) += pcie-hisi.o
>>> +obj-$(CONFIG_PCI_HISI_ACPI) += pcie-hisi-acpi.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
>>> diff --git a/drivers/pci/host/pcie-hisi-acpi.c b/drivers/pci/host/pcie-hisi-acpi.c
>>> new file mode 100644
>>> index 0000000..aade4b5
>>> --- /dev/null
>>> +++ b/drivers/pci/host/pcie-hisi-acpi.c
>>> @@ -0,0 +1,157 @@
>>> +/*
>>> + * PCIe host controller driver for HiSilicon HipXX SoCs
>>> + *
>>> + * Copyright (C) 2016 HiSilicon Co., Ltd. http://www.hisilicon.com
>>> + *
>>> + * Author: Dongdong Liu <liudongdong3@huawei.com>
>>> + *         Gabriele Paoloni <gabriele.paoloni@huawei.com>
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License version 2 as
>>> + * published by the Free Software Foundation.
>>> + */
>>> +#include <linux/pci.h>
>>> +#include <linux/pci-acpi.h>
>>> +#include <linux/pci-ecam.h>
>>> +
>>> +#define DEBUG0				0x728
>>> +#define PCIE_LTSSM_LINKUP_STATE		0x11
>>> +#define PCIE_LTSSM_STATE_MASK		0x3F
>>
>> These are now unused.
>>
>>> +static const struct acpi_device_id hisi_pcie_rc_res_ids[] = {
>>> +	{"HISI0081", 0},
>>> +	{"", 0},
>>> +};
>>> +
>>> +static int hisi_pcie_acpi_rd_conf(struct pci_bus *bus, u32 devfn, int where,
>>> +				  int size, u32 *val)
>>> +{
>>> +	struct pci_config_window *cfg = bus->sysdata;
>>> +	int dev = PCI_SLOT(devfn);
>>> +
>>> +	if (bus->number == cfg->busr.start) {
>>> +		/* access only one slot on each root port */
>>> +		if (dev > 0)
>>> +			return PCIBIOS_DEVICE_NOT_FOUND;
>>> +		else
>>> +			return pci_generic_config_read32(bus, devfn, where,
>>> +							 size, val);
>>> +	}
>>> +
>>> +	return pci_generic_config_read(bus, devfn, where, size, val);
>>> +}
>>> +
>>> +static int hisi_pcie_acpi_wr_conf(struct pci_bus *bus, u32 devfn,
>>> +				  int where, int size, u32 val)
>>> +{
>>> +	struct pci_config_window *cfg = bus->sysdata;
>>> +	int dev = PCI_SLOT(devfn);
>>> +
>>> +	if (bus->number == cfg->busr.start) {
>>> +		/* access only one slot on each root port */
>>> +		if (dev > 0)
>>> +			return PCIBIOS_DEVICE_NOT_FOUND;
>>> +		else
>>> +			return pci_generic_config_write32(bus, devfn, where,
>>> +							  size, val);
>>> +	}
>>> +
>>> +	return pci_generic_config_write(bus, devfn, where, size, val);
>>> +}
>>> +
>>> +static void __iomem *hisi_pcie_map_bus(struct pci_bus *bus, unsigned int devfn,
>>> +				       int where)
>>> +{
>>> +	struct pci_config_window *cfg = bus->sysdata;
>>> +	void __iomem *reg_base = cfg->priv;
>>> +
>>> +	if (bus->number == cfg->busr.start)
>>> +		return reg_base + where;
>>> +	else
>>> +		return pci_ecam_map_bus(bus, devfn, where);
>>> +}
>>> +
>>> +/*
>>> + * Retrieve RC base and size from sub-device under the RC
>>> + * Device (RES1)
>>> + * {
>>> + *	Name (_HID, "HISI0081")
>>> + *	Name (_CID, "PNP0C02")
>>> + *	Name (_CRS, ResourceTemplate (){
>>> + *		Memory32Fixed (ReadWrite, 0xb0080000, 0x10000)
>>> + *	})
>>> + * }
>>> + */
>>> +static int hisi_pcie_rc_addr_get(struct acpi_device *adev,
>>> +				 void __iomem **addr)
>>> +{
>>> +	struct acpi_device *child_adev;
>>> +	struct list_head list;
>>> +	struct resource *res;
>>> +	struct resource_entry *entry;
>>> +	unsigned long flags;
>>> +	int ret;
>>> +
>>> +	list_for_each_entry(child_adev, &adev->children, node) {
>>> +		ret = acpi_match_device_ids(child_adev, hisi_pcie_rc_res_ids);
>>> +		if (ret)
>>> +			continue;
>>> +
>>> +		INIT_LIST_HEAD(&list);
>>> +		flags = IORESOURCE_MEM;
>>> +		ret = acpi_dev_get_resources(child_adev, &list,
>>> +					     acpi_dev_filter_resource_type_cb,
>>> +					     (void *)flags);
>>> +		if (ret < 0) {
>>> +			dev_err(&child_adev->dev,
>>> +				"failed to parse _CRS method, error code %d\n",
>>> +				ret);
>>> +			return ret;
>>> +		} else if (ret == 0) {
>>> +			dev_err(&child_adev->dev,
>>> +				"no IO and memory resources present in _CRS\n");
>>> +			return -EINVAL;
>>> +		}
>>> +
>>> +		entry = list_first_entry(&list, struct resource_entry, node);
>>> +		res = entry->res;
>>> +		*addr = devm_ioremap(&child_adev->dev,
>>> +				     res->start, resource_size(res));
>>> +		acpi_dev_free_resource_list(&list);
>>> +		if (IS_ERR(*addr)) {
>>> +			dev_err(&child_adev->dev, "error with ioremap\n");
>>> +			return -ENOMEM;
>>> +		}
>>> +
>>> +		return 0;
>>> +	}
>>> +
>>> +	return -EINVAL;
>>> +}
>>
>> OK, so MCFG (with the possible help of quirks) tells us where *most*
>> of this RC's ECAM space is, but it doesn't tell us where the root bus
>> ECAM space is, which is why we need this block of ugly code.  Right?
>
> Tomasz is doing the same thing for ThunderX.  Can we share the code
> somehow, e.g., add something that takes the _HID to look for and just
> returns the register address?  Maybe in pci-acpi.c, decl in
> drivers/pci/pci.h, under #ifdef CONFIG_ARM64?0

As we have the PNP0C02 device at the root of the ACPI namespace (under \_SB).
We need a way to tell which root bus the PNP0C02 resource belong to.

Firstly, use _HID(HISI0081) to get the PNP0C02 devices that we need..
Secondly, use _UID to match segment to tell which root bus the PNP0C02 resource belong to.

The implementaton is as below. What do you think about this?
If the code is ok for other manufacturers, we can share the code.

struct hisi_pcie_acpi {
	u16 segment;
	acpi_handle handle;
};

/*
  * Retrieve RC base and size from PNP0C02 at the root of the ACPI namespace
  * (under \_SB).
  * Device (RES0)
  * {
  *	Name (_HID, "HISI0081")
  *	Name (_CID, "PNP0C02")
  *	Name (_UID, 0x0)
  *	Name (_CRS, ResourceTemplate (){
  *		Memory32Fixed (ReadWrite, 0xa0090000, 0x10000)
  *	})
  * }
  */
static int hisi_pcie_rc_addr_get(struct acpi_device *adev,
				 void __iomem **addr)
{
	struct list_head list;
	struct resource *res;
	struct resource_entry *entry;
	unsigned long flags;
	int ret;

	INIT_LIST_HEAD(&list);
	flags = IORESOURCE_MEM;
	ret = acpi_dev_get_resources(adev, &list,
				     acpi_dev_filter_resource_type_cb,
				     (void *)flags);
	if (ret < 0) {
		dev_err(&adev->dev,
			"failed to parse _CRS method, error code %d\n",
			ret);
		return ret;
	} else if (ret == 0) {
		dev_err(&adev->dev,
			"no IO and memory resources present in _CRS\n");
		return -EINVAL;
	}

	entry = list_first_entry(&list, struct resource_entry, node);
	res = entry->res;
	*addr = devm_ioremap(&adev->dev,
			     res->start, resource_size(res));
	acpi_dev_free_resource_list(&list);
	if (IS_ERR(*addr)) {
		dev_err(&adev->dev, "error with ioremap\n");
		return -ENOMEM;
	}

	return 0;
}

static acpi_status find_rc_resource(acpi_handle handle, u32 lvl,
				    void *context, void **rv)
{
	struct hisi_pcie_acpi *pcie_acpi = context;
	acpi_status status;
	unsigned long long uid;

	status = acpi_evaluate_integer(handle, "_UID", NULL, &uid);
	if (ACPI_FAILURE(status) || uid != pcie_acpi->segment)
		return AE_CTRL_DEPTH;

	pcie_acpi->handle = handle;
	return AE_CTRL_TERMINATE;
}

static int hisi_pcie_init(struct pci_config_window *cfg)
{
	struct acpi_device *adev = to_acpi_device(cfg->parent);
	struct acpi_pci_root *root = acpi_driver_data(adev);
	struct hisi_pcie_acpi *pcie_acpi;
	int ret;
	void __iomem *reg_base;

	pcie_acpi = devm_kzalloc(&adev->dev, sizeof(*pcie_acpi), GFP_KERNEL);
	if (!pcie_acpi)
		return -ENOMEM;

	pcie_acpi->segment = root->segment;
	acpi_get_devices("HISI0081", find_rc_resource, pcie_acpi, NULL);

	ret = acpi_bus_get_device(pcie_acpi->handle, &adev);
	if (ret)
		return ret;

	ret = hisi_pcie_rc_addr_get(adev, &reg_base);
	if (ret) {
		dev_err(&adev->dev, "can't get rc base address");
		return ret;
	}

	cfg->priv = pcie_acpi->reg_base;
	return 0;
}

Thanks
Dongdong
>
> .
>

Re: [PATCH V4 2/2] PCI/ACPI: hisi: Add ACPI support for HiSilicon SoCs Host Controllers

[Tomasz Nowicki]

Hi Dongdong,

I rewrite your code so that it could be used for ThunderX as well.
This assumes _UID&segment is the right way of looking up corelated RC.
Of course acpi_get_rc_resources() and its acpi_* helpers should go to 
pci-acpi.c.

Tomasz

static int acpi_get_rc_addr(struct acpi_device *adev, struct resource *res)
{
	struct resource_entry *entry;
	struct list_head list;
	unsigned long flags;
	int ret;

	INIT_LIST_HEAD(&list);
	flags = IORESOURCE_MEM;
	ret = acpi_dev_get_resources(adev, &list,
			acpi_dev_filter_resource_type_cb, (void *) flags);
	if (ret < 0) {
		dev_err(&adev->dev,
				"failed to parse _CRS method, error code %d\n",
				ret);
		return ret;
	} else if (ret == 0) {
		dev_err(&adev->dev,
				"no IO and memory resources present in _CRS\n");
		return -EINVAL;
	}

	entry = list_first_entry(&list, struct resource_entry, node);
	*res = *entry->res;
	acpi_dev_free_resource_list(&list);
	return 0;
}

static acpi_status acpi_match_rc(acpi_handle handle, u32 lvl, void *context,
				 void **retval)
{
	u16 *segment = context;
	unsigned long long uid;
	acpi_status status;

	status = acpi_evaluate_integer(handle, "_UID", NULL, &uid);
	if (ACPI_FAILURE(status) || uid != *segment)
		return AE_CTRL_DEPTH;

	*(acpi_handle *)retval = handle;
	return AE_CTRL_TERMINATE;
}

static int acpi_get_rc_resources(const char *hid, u16 segment,
				 struct resource *res)
{
	struct acpi_device *adev;
	acpi_status status;
	acpi_handle handle;
	int ret;

	status = acpi_get_devices(hid, acpi_match_rc, &segment, &handle);
	if (ACPI_FAILURE(status)) {
		pr_err("Can't find _HID %s device", hid);
		return -ENODEV;
	}

	ret = acpi_bus_get_device(handle, &adev);
	if (ret)
		return ret;

	ret = acpi_get_rc_addr(adev, res);
	if (ret) {
		dev_err(&adev->dev, "can't get RC resource");
		return ret;
	}

	return 0;
}

static int hisi_pcie_init(struct pci_config_window *cfg)
{
	struct acpi_device *adev = to_acpi_device(cfg->parent);
	struct acpi_pci_root *root = acpi_driver_data(adev);
	void __iomem *reg_base;
	struct resource *res;
	acpi_status status;
	acpi_handle handle;
	int ret;

	res = devm_kzalloc(&adev->dev, sizeof(*res), GFP_KERNEL);
	if (!res)
		return -ENOMEM;

	ret = acpi_get_rc_resources("HISI0081", root->segment, res);
	if (ret) {
		dev_err(&adev->dev, "can't get rc base address");
		return ret;
	}

	reg_base = devm_ioremap(&adev->dev, res->start, resource_size(res));
	if (!reg_base)
		return -ENOMEM;

	cfg->priv = reg_base;
	return 0;
}

On 17.11.2016 04:02, Dongdong Liu wrote:
> Hi Bjorn
> 在 2016/11/17 7:00, Bjorn Helgaas 写道:
>> On Mon, Nov 14, 2016 at 05:33:20PM -0600, Bjorn Helgaas wrote:
>>> On Wed, Nov 09, 2016 at 05:14:57PM +0800, Dongdong Liu wrote:
>>>> PCIe controller in Hip05/HIP06/HIP07 SoCs is not ECAM compliant.
>>>> It is non ECAM only for the RC bus config space;for any other bus
>>>> underneath the root bus we support ECAM access.
>>>> Add specific quirks for PCI config space accessors.This involves:
>>>> 1. New initialization call hisi_pcie_init() to obtain rc base
>>>> addresses from PNP0C02 as subdevice of PNP0A03.
>>>> 2. New entry in common quirk array.
>>>>
>>>> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
>>>> Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
>>>> ---
>>>>  MAINTAINERS                       |   1 +
>>>>  drivers/acpi/pci_mcfg.c           |  13 ++++
>>>>  drivers/pci/host/Kconfig          |   8 ++
>>>>  drivers/pci/host/Makefile         |   1 +
>>>>  drivers/pci/host/pcie-hisi-acpi.c | 157
>>>> ++++++++++++++++++++++++++++++++++++++
>>>>  include/linux/pci-ecam.h          |   5 ++
>>>>  6 files changed, 185 insertions(+)
>>>>  create mode 100644 drivers/pci/host/pcie-hisi-acpi.c
>>>>
>>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>>> index 1cd38a7..b224caa 100644
>>>> --- a/MAINTAINERS
>>>> +++ b/MAINTAINERS
>>>> @@ -9358,6 +9358,7 @@ L:    linux-pci@vger.kernel.org
>>>>  S:    Maintained
>>>>  F:    Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
>>>>  F:    drivers/pci/host/pcie-hisi.c
>>>> +F:    drivers/pci/host/pcie-hisi-acpi.c
>>>>
>>>>  PCIE DRIVER FOR ROCKCHIP
>>>>  M:    Shawn Lin <shawn.lin@rock-chips.com>
>>>> diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
>>>> index ac21db3..b1b6fc7 100644
>>>> --- a/drivers/acpi/pci_mcfg.c
>>>> +++ b/drivers/acpi/pci_mcfg.c
>>>> @@ -57,6 +57,19 @@ struct mcfg_fixup {
>>>>      { "QCOM  ", "QDF2432 ", 1, 5, MCFG_BUS_ANY, &pci_32b_ops },
>>>>      { "QCOM  ", "QDF2432 ", 1, 6, MCFG_BUS_ANY, &pci_32b_ops },
>>>>      { "QCOM  ", "QDF2432 ", 1, 7, MCFG_BUS_ANY, &pci_32b_ops },
>>>> +#ifdef CONFIG_PCI_HISI_ACPI
>>>> +    #define PCI_ACPI_QUIRK_QUAD_DOM(table_id, seg, ops) \
>>>> +    { "HISI  ", table_id, 0, seg + 0, MCFG_BUS_ANY, ops }, \
>>>> +    { "HISI  ", table_id, 0, seg + 1, MCFG_BUS_ANY, ops }, \
>>>> +    { "HISI  ", table_id, 0, seg + 2, MCFG_BUS_ANY, ops }, \
>>>> +    { "HISI  ", table_id, 0, seg + 3, MCFG_BUS_ANY, ops }
>>>> +    PCI_ACPI_QUIRK_QUAD_DOM("HIP05   ", 0, &hisi_pcie_ops),
>>>> +    PCI_ACPI_QUIRK_QUAD_DOM("HIP06   ", 0, &hisi_pcie_ops),
>>>> +    PCI_ACPI_QUIRK_QUAD_DOM("HIP07   ", 0, &hisi_pcie_ops),
>>>> +    PCI_ACPI_QUIRK_QUAD_DOM("HIP07   ", 4, &hisi_pcie_ops),
>>>> +    PCI_ACPI_QUIRK_QUAD_DOM("HIP07   ", 8, &hisi_pcie_ops),
>>>> +    PCI_ACPI_QUIRK_QUAD_DOM("HIP07   ", 12, &hisi_pcie_ops),
>>>> +#endif
>>>>  };
>>>>
>>>>  static char mcfg_oem_id[ACPI_OEM_ID_SIZE];
>>>> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
>>>> index ae98644..9ff2bcd 100644
>>>> --- a/drivers/pci/host/Kconfig
>>>> +++ b/drivers/pci/host/Kconfig
>>>> @@ -227,6 +227,14 @@ config PCI_HISI
>>>>        Say Y here if you want PCIe controller support on HiSilicon
>>>>        Hip05 and Hip06 and Hip07 SoCs
>>>>
>>>> +config PCI_HISI_ACPI
>>>> +    depends on ACPI && ARM64
>>>> +    bool "HiSilicon Hip05 and Hip06 and Hip07 SoCs ACPI PCIe
>>>> controllers"
>>>> +    select PNP
>>>> +    help
>>>> +      Say Y here if you want ACPI PCIe controller support on HiSilicon
>>>> +      Hip05 and Hip06 and Hip07 SoCs
>>>> +
>>>>  config PCIE_QCOM
>>>>      bool "Qualcomm PCIe controller"
>>>>      depends on ARCH_QCOM && OF
>>>> diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
>>>> index 084cb49..9402858 100644
>>>> --- a/drivers/pci/host/Makefile
>>>> +++ b/drivers/pci/host/Makefile
>>>> @@ -26,6 +26,7 @@ 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_PCI_HISI) += pcie-hisi.o
>>>> +obj-$(CONFIG_PCI_HISI_ACPI) += pcie-hisi-acpi.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
>>>> diff --git a/drivers/pci/host/pcie-hisi-acpi.c
>>>> b/drivers/pci/host/pcie-hisi-acpi.c
>>>> new file mode 100644
>>>> index 0000000..aade4b5
>>>> --- /dev/null
>>>> +++ b/drivers/pci/host/pcie-hisi-acpi.c
>>>> @@ -0,0 +1,157 @@
>>>> +/*
>>>> + * PCIe host controller driver for HiSilicon HipXX SoCs
>>>> + *
>>>> + * Copyright (C) 2016 HiSilicon Co., Ltd. http://www.hisilicon.com
>>>> + *
>>>> + * Author: Dongdong Liu <liudongdong3@huawei.com>
>>>> + *         Gabriele Paoloni <gabriele.paoloni@huawei.com>
>>>> + *
>>>> + * This program is free software; you can redistribute it and/or
>>>> modify
>>>> + * it under the terms of the GNU General Public License version 2 as
>>>> + * published by the Free Software Foundation.
>>>> + */
>>>> +#include <linux/pci.h>
>>>> +#include <linux/pci-acpi.h>
>>>> +#include <linux/pci-ecam.h>
>>>> +
>>>> +#define DEBUG0                0x728
>>>> +#define PCIE_LTSSM_LINKUP_STATE        0x11
>>>> +#define PCIE_LTSSM_STATE_MASK        0x3F
>>>
>>> These are now unused.
>>>
>>>> +static const struct acpi_device_id hisi_pcie_rc_res_ids[] = {
>>>> +    {"HISI0081", 0},
>>>> +    {"", 0},
>>>> +};
>>>> +
>>>> +static int hisi_pcie_acpi_rd_conf(struct pci_bus *bus, u32 devfn,
>>>> int where,
>>>> +                  int size, u32 *val)
>>>> +{
>>>> +    struct pci_config_window *cfg = bus->sysdata;
>>>> +    int dev = PCI_SLOT(devfn);
>>>> +
>>>> +    if (bus->number == cfg->busr.start) {
>>>> +        /* access only one slot on each root port */
>>>> +        if (dev > 0)
>>>> +            return PCIBIOS_DEVICE_NOT_FOUND;
>>>> +        else
>>>> +            return pci_generic_config_read32(bus, devfn, where,
>>>> +                             size, val);
>>>> +    }
>>>> +
>>>> +    return pci_generic_config_read(bus, devfn, where, size, val);
>>>> +}
>>>> +
>>>> +static int hisi_pcie_acpi_wr_conf(struct pci_bus *bus, u32 devfn,
>>>> +                  int where, int size, u32 val)
>>>> +{
>>>> +    struct pci_config_window *cfg = bus->sysdata;
>>>> +    int dev = PCI_SLOT(devfn);
>>>> +
>>>> +    if (bus->number == cfg->busr.start) {
>>>> +        /* access only one slot on each root port */
>>>> +        if (dev > 0)
>>>> +            return PCIBIOS_DEVICE_NOT_FOUND;
>>>> +        else
>>>> +            return pci_generic_config_write32(bus, devfn, where,
>>>> +                              size, val);
>>>> +    }
>>>> +
>>>> +    return pci_generic_config_write(bus, devfn, where, size, val);
>>>> +}
>>>> +
>>>> +static void __iomem *hisi_pcie_map_bus(struct pci_bus *bus,
>>>> unsigned int devfn,
>>>> +                       int where)
>>>> +{
>>>> +    struct pci_config_window *cfg = bus->sysdata;
>>>> +    void __iomem *reg_base = cfg->priv;
>>>> +
>>>> +    if (bus->number == cfg->busr.start)
>>>> +        return reg_base + where;
>>>> +    else
>>>> +        return pci_ecam_map_bus(bus, devfn, where);
>>>> +}
>>>> +
>>>> +/*
>>>> + * Retrieve RC base and size from sub-device under the RC
>>>> + * Device (RES1)
>>>> + * {
>>>> + *    Name (_HID, "HISI0081")
>>>> + *    Name (_CID, "PNP0C02")
>>>> + *    Name (_CRS, ResourceTemplate (){
>>>> + *        Memory32Fixed (ReadWrite, 0xb0080000, 0x10000)
>>>> + *    })
>>>> + * }
>>>> + */
>>>> +static int hisi_pcie_rc_addr_get(struct acpi_device *adev,
>>>> +                 void __iomem **addr)
>>>> +{
>>>> +    struct acpi_device *child_adev;
>>>> +    struct list_head list;
>>>> +    struct resource *res;
>>>> +    struct resource_entry *entry;
>>>> +    unsigned long flags;
>>>> +    int ret;
>>>> +
>>>> +    list_for_each_entry(child_adev, &adev->children, node) {
>>>> +        ret = acpi_match_device_ids(child_adev, hisi_pcie_rc_res_ids);
>>>> +        if (ret)
>>>> +            continue;
>>>> +
>>>> +        INIT_LIST_HEAD(&list);
>>>> +        flags = IORESOURCE_MEM;
>>>> +        ret = acpi_dev_get_resources(child_adev, &list,
>>>> +                         acpi_dev_filter_resource_type_cb,
>>>> +                         (void *)flags);
>>>> +        if (ret < 0) {
>>>> +            dev_err(&child_adev->dev,
>>>> +                "failed to parse _CRS method, error code %d\n",
>>>> +                ret);
>>>> +            return ret;
>>>> +        } else if (ret == 0) {
>>>> +            dev_err(&child_adev->dev,
>>>> +                "no IO and memory resources present in _CRS\n");
>>>> +            return -EINVAL;
>>>> +        }
>>>> +
>>>> +        entry = list_first_entry(&list, struct resource_entry, node);
>>>> +        res = entry->res;
>>>> +        *addr = devm_ioremap(&child_adev->dev,
>>>> +                     res->start, resource_size(res));
>>>> +        acpi_dev_free_resource_list(&list);
>>>> +        if (IS_ERR(*addr)) {
>>>> +            dev_err(&child_adev->dev, "error with ioremap\n");
>>>> +            return -ENOMEM;
>>>> +        }
>>>> +
>>>> +        return 0;
>>>> +    }
>>>> +
>>>> +    return -EINVAL;
>>>> +}
>>>
>>> OK, so MCFG (with the possible help of quirks) tells us where *most*
>>> of this RC's ECAM space is, but it doesn't tell us where the root bus
>>> ECAM space is, which is why we need this block of ugly code.  Right?
>>
>> Tomasz is doing the same thing for ThunderX.  Can we share the code
>> somehow, e.g., add something that takes the _HID to look for and just
>> returns the register address?  Maybe in pci-acpi.c, decl in
>> drivers/pci/pci.h, under #ifdef CONFIG_ARM64?0
>
> As we have the PNP0C02 device at the root of the ACPI namespace (under
> \_SB).
> We need a way to tell which root bus the PNP0C02 resource belong to.
>
> Firstly, use _HID(HISI0081) to get the PNP0C02 devices that we need..
> Secondly, use _UID to match segment to tell which root bus the PNP0C02
> resource belong to.
>
> The implementaton is as below. What do you think about this?
> If the code is ok for other manufacturers, we can share the code.
>
> struct hisi_pcie_acpi {
>     u16 segment;
>     acpi_handle handle;
> };
>
> /*
>  * Retrieve RC base and size from PNP0C02 at the root of the ACPI namespace
>  * (under \_SB).
>  * Device (RES0)
>  * {
>  *    Name (_HID, "HISI0081")
>  *    Name (_CID, "PNP0C02")
>  *    Name (_UID, 0x0)
>  *    Name (_CRS, ResourceTemplate (){
>  *        Memory32Fixed (ReadWrite, 0xa0090000, 0x10000)
>  *    })
>  * }
>  */
> static int hisi_pcie_rc_addr_get(struct acpi_device *adev,
>                  void __iomem **addr)
> {
>     struct list_head list;
>     struct resource *res;
>     struct resource_entry *entry;
>     unsigned long flags;
>     int ret;
>
>     INIT_LIST_HEAD(&list);
>     flags = IORESOURCE_MEM;
>     ret = acpi_dev_get_resources(adev, &list,
>                      acpi_dev_filter_resource_type_cb,
>                      (void *)flags);
>     if (ret < 0) {
>         dev_err(&adev->dev,
>             "failed to parse _CRS method, error code %d\n",
>             ret);
>         return ret;
>     } else if (ret == 0) {
>         dev_err(&adev->dev,
>             "no IO and memory resources present in _CRS\n");
>         return -EINVAL;
>     }
>
>     entry = list_first_entry(&list, struct resource_entry, node);
>     res = entry->res;
>     *addr = devm_ioremap(&adev->dev,
>                  res->start, resource_size(res));
>     acpi_dev_free_resource_list(&list);
>     if (IS_ERR(*addr)) {
>         dev_err(&adev->dev, "error with ioremap\n");
>         return -ENOMEM;
>     }
>
>     return 0;
> }
>
> static acpi_status find_rc_resource(acpi_handle handle, u32 lvl,
>                     void *context, void **rv)
> {
>     struct hisi_pcie_acpi *pcie_acpi = context;
>     acpi_status status;
>     unsigned long long uid;
>
>     status = acpi_evaluate_integer(handle, "_UID", NULL, &uid);
>     if (ACPI_FAILURE(status) || uid != pcie_acpi->segment)
>         return AE_CTRL_DEPTH;
>
>     pcie_acpi->handle = handle;
>     return AE_CTRL_TERMINATE;
> }
>
> static int hisi_pcie_init(struct pci_config_window *cfg)
> {
>     struct acpi_device *adev = to_acpi_device(cfg->parent);
>     struct acpi_pci_root *root = acpi_driver_data(adev);
>     struct hisi_pcie_acpi *pcie_acpi;
>     int ret;
>     void __iomem *reg_base;
>
>     pcie_acpi = devm_kzalloc(&adev->dev, sizeof(*pcie_acpi), GFP_KERNEL);
>     if (!pcie_acpi)
>         return -ENOMEM;
>
>     pcie_acpi->segment = root->segment;
>     acpi_get_devices("HISI0081", find_rc_resource, pcie_acpi, NULL);
>
>     ret = acpi_bus_get_device(pcie_acpi->handle, &adev);
>     if (ret)
>         return ret;
>
>     ret = hisi_pcie_rc_addr_get(adev, &reg_base);
>     if (ret) {
>         dev_err(&adev->dev, "can't get rc base address");
>         return ret;
>     }
>
>     cfg->priv = pcie_acpi->reg_base;
>     return 0;
> }
>
> Thanks
> Dongdong
>>
>> .
>>
>

Re: [PATCH V4 2/2] PCI/ACPI: hisi: Add ACPI support for HiSilicon SoCs Host Controllers

[Dongdong Liu]

Hi Tomasz

在 2016/11/17 16:28, Tomasz Nowicki 写道:
> Hi Dongdong,
>
> I rewrite your code so that it could be used for ThunderX as well.

The rewrited code looks good to me.

> This assumes _UID&segment is the right way of looking up corelated RC.
> Of course acpi_get_rc_resources() and its acpi_* helpers should go to pci-acpi.c.

I am ok about this.

Thanks
Dongdong
>
> Tomasz
>
> static int acpi_get_rc_addr(struct acpi_device *adev, struct resource *res)
> {
>     struct resource_entry *entry;
>     struct list_head list;
>     unsigned long flags;
>     int ret;
>
>     INIT_LIST_HEAD(&list);
>     flags = IORESOURCE_MEM;
>     ret = acpi_dev_get_resources(adev, &list,
>             acpi_dev_filter_resource_type_cb, (void *) flags);
>     if (ret < 0) {
>         dev_err(&adev->dev,
>                 "failed to parse _CRS method, error code %d\n",
>                 ret);
>         return ret;
>     } else if (ret == 0) {
>         dev_err(&adev->dev,
>                 "no IO and memory resources present in _CRS\n");
>         return -EINVAL;
>     }
>
>     entry = list_first_entry(&list, struct resource_entry, node);
>     *res = *entry->res;
>     acpi_dev_free_resource_list(&list);
>     return 0;
> }
>
> static acpi_status acpi_match_rc(acpi_handle handle, u32 lvl, void *context,
>                  void **retval)
> {
>     u16 *segment = context;
>     unsigned long long uid;
>     acpi_status status;
>
>     status = acpi_evaluate_integer(handle, "_UID", NULL, &uid);
>     if (ACPI_FAILURE(status) || uid != *segment)
>         return AE_CTRL_DEPTH;
>
>     *(acpi_handle *)retval = handle;
>     return AE_CTRL_TERMINATE;
> }
>
> static int acpi_get_rc_resources(const char *hid, u16 segment,
>                  struct resource *res)
> {
>     struct acpi_device *adev;
>     acpi_status status;
>     acpi_handle handle;
>     int ret;
>
>     status = acpi_get_devices(hid, acpi_match_rc, &segment, &handle);
>     if (ACPI_FAILURE(status)) {
>         pr_err("Can't find _HID %s device", hid);
>         return -ENODEV;
>     }
>
>     ret = acpi_bus_get_device(handle, &adev);
>     if (ret)
>         return ret;
>
>     ret = acpi_get_rc_addr(adev, res);
>     if (ret) {
>         dev_err(&adev->dev, "can't get RC resource");
>         return ret;
>     }
>
>     return 0;
> }
>
> static int hisi_pcie_init(struct pci_config_window *cfg)
> {
>     struct acpi_device *adev = to_acpi_device(cfg->parent);
>     struct acpi_pci_root *root = acpi_driver_data(adev);
>     void __iomem *reg_base;
>     struct resource *res;
>     acpi_status status;
>     acpi_handle handle;
>     int ret;
>
>     res = devm_kzalloc(&adev->dev, sizeof(*res), GFP_KERNEL);
>     if (!res)
>         return -ENOMEM;
>
>     ret = acpi_get_rc_resources("HISI0081", root->segment, res);
>     if (ret) {
>         dev_err(&adev->dev, "can't get rc base address");
>         return ret;
>     }
>
>     reg_base = devm_ioremap(&adev->dev, res->start, resource_size(res));
>     if (!reg_base)
>         return -ENOMEM;
>
>     cfg->priv = reg_base;
>     return 0;
> }
>
> On 17.11.2016 04:02, Dongdong Liu wrote:
>> Hi Bjorn
>> 在 2016/11/17 7:00, Bjorn Helgaas 写道:
>>> On Mon, Nov 14, 2016 at 05:33:20PM -0600, Bjorn Helgaas wrote:
>>>> On Wed, Nov 09, 2016 at 05:14:57PM +0800, Dongdong Liu wrote:
>>>>> PCIe controller in Hip05/HIP06/HIP07 SoCs is not ECAM compliant.
>>>>> It is non ECAM only for the RC bus config space;for any other bus
>>>>> underneath the root bus we support ECAM access.
>>>>> Add specific quirks for PCI config space accessors.This involves:
>>>>> 1. New initialization call hisi_pcie_init() to obtain rc base
>>>>> addresses from PNP0C02 as subdevice of PNP0A03.
>>>>> 2. New entry in common quirk array.
>>>>>
>>>>> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
>>>>> Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
>>>>> ---
>>>>>  MAINTAINERS                       |   1 +
>>>>>  drivers/acpi/pci_mcfg.c           |  13 ++++
>>>>>  drivers/pci/host/Kconfig          |   8 ++
>>>>>  drivers/pci/host/Makefile         |   1 +
>>>>>  drivers/pci/host/pcie-hisi-acpi.c | 157
>>>>> ++++++++++++++++++++++++++++++++++++++
>>>>>  include/linux/pci-ecam.h          |   5 ++
>>>>>  6 files changed, 185 insertions(+)
>>>>>  create mode 100644 drivers/pci/host/pcie-hisi-acpi.c
>>>>>
>>>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>>>> index 1cd38a7..b224caa 100644
>>>>> --- a/MAINTAINERS
>>>>> +++ b/MAINTAINERS
>>>>> @@ -9358,6 +9358,7 @@ L:    linux-pci@vger.kernel.org
>>>>>  S:    Maintained
>>>>>  F:    Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
>>>>>  F:    drivers/pci/host/pcie-hisi.c
>>>>> +F:    drivers/pci/host/pcie-hisi-acpi.c
>>>>>
>>>>>  PCIE DRIVER FOR ROCKCHIP
>>>>>  M:    Shawn Lin <shawn.lin@rock-chips.com>
>>>>> diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
>>>>> index ac21db3..b1b6fc7 100644
>>>>> --- a/drivers/acpi/pci_mcfg.c
>>>>> +++ b/drivers/acpi/pci_mcfg.c
>>>>> @@ -57,6 +57,19 @@ struct mcfg_fixup {
>>>>>      { "QCOM  ", "QDF2432 ", 1, 5, MCFG_BUS_ANY, &pci_32b_ops },
>>>>>      { "QCOM  ", "QDF2432 ", 1, 6, MCFG_BUS_ANY, &pci_32b_ops },
>>>>>      { "QCOM  ", "QDF2432 ", 1, 7, MCFG_BUS_ANY, &pci_32b_ops },
>>>>> +#ifdef CONFIG_PCI_HISI_ACPI
>>>>> +    #define PCI_ACPI_QUIRK_QUAD_DOM(table_id, seg, ops) \
>>>>> +    { "HISI  ", table_id, 0, seg + 0, MCFG_BUS_ANY, ops }, \
>>>>> +    { "HISI  ", table_id, 0, seg + 1, MCFG_BUS_ANY, ops }, \
>>>>> +    { "HISI  ", table_id, 0, seg + 2, MCFG_BUS_ANY, ops }, \
>>>>> +    { "HISI  ", table_id, 0, seg + 3, MCFG_BUS_ANY, ops }
>>>>> +    PCI_ACPI_QUIRK_QUAD_DOM("HIP05   ", 0, &hisi_pcie_ops),
>>>>> +    PCI_ACPI_QUIRK_QUAD_DOM("HIP06   ", 0, &hisi_pcie_ops),
>>>>> +    PCI_ACPI_QUIRK_QUAD_DOM("HIP07   ", 0, &hisi_pcie_ops),
>>>>> +    PCI_ACPI_QUIRK_QUAD_DOM("HIP07   ", 4, &hisi_pcie_ops),
>>>>> +    PCI_ACPI_QUIRK_QUAD_DOM("HIP07   ", 8, &hisi_pcie_ops),
>>>>> +    PCI_ACPI_QUIRK_QUAD_DOM("HIP07   ", 12, &hisi_pcie_ops),
>>>>> +#endif
>>>>>  };
>>>>>
>>>>>  static char mcfg_oem_id[ACPI_OEM_ID_SIZE];
>>>>> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
>>>>> index ae98644..9ff2bcd 100644
>>>>> --- a/drivers/pci/host/Kconfig
>>>>> +++ b/drivers/pci/host/Kconfig
>>>>> @@ -227,6 +227,14 @@ config PCI_HISI
>>>>>        Say Y here if you want PCIe controller support on HiSilicon
>>>>>        Hip05 and Hip06 and Hip07 SoCs
>>>>>
>>>>> +config PCI_HISI_ACPI
>>>>> +    depends on ACPI && ARM64
>>>>> +    bool "HiSilicon Hip05 and Hip06 and Hip07 SoCs ACPI PCIe
>>>>> controllers"
>>>>> +    select PNP
>>>>> +    help
>>>>> +      Say Y here if you want ACPI PCIe controller support on HiSilicon
>>>>> +      Hip05 and Hip06 and Hip07 SoCs
>>>>> +
>>>>>  config PCIE_QCOM
>>>>>      bool "Qualcomm PCIe controller"
>>>>>      depends on ARCH_QCOM && OF
>>>>> diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
>>>>> index 084cb49..9402858 100644
>>>>> --- a/drivers/pci/host/Makefile
>>>>> +++ b/drivers/pci/host/Makefile
>>>>> @@ -26,6 +26,7 @@ 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_PCI_HISI) += pcie-hisi.o
>>>>> +obj-$(CONFIG_PCI_HISI_ACPI) += pcie-hisi-acpi.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
>>>>> diff --git a/drivers/pci/host/pcie-hisi-acpi.c
>>>>> b/drivers/pci/host/pcie-hisi-acpi.c
>>>>> new file mode 100644
>>>>> index 0000000..aade4b5
>>>>> --- /dev/null
>>>>> +++ b/drivers/pci/host/pcie-hisi-acpi.c
>>>>> @@ -0,0 +1,157 @@
>>>>> +/*
>>>>> + * PCIe host controller driver for HiSilicon HipXX SoCs
>>>>> + *
>>>>> + * Copyright (C) 2016 HiSilicon Co., Ltd. http://www.hisilicon.com
>>>>> + *
>>>>> + * Author: Dongdong Liu <liudongdong3@huawei.com>
>>>>> + *         Gabriele Paoloni <gabriele.paoloni@huawei.com>
>>>>> + *
>>>>> + * This program is free software; you can redistribute it and/or
>>>>> modify
>>>>> + * it under the terms of the GNU General Public License version 2 as
>>>>> + * published by the Free Software Foundation.
>>>>> + */
>>>>> +#include <linux/pci.h>
>>>>> +#include <linux/pci-acpi.h>
>>>>> +#include <linux/pci-ecam.h>
>>>>> +
>>>>> +#define DEBUG0                0x728
>>>>> +#define PCIE_LTSSM_LINKUP_STATE        0x11
>>>>> +#define PCIE_LTSSM_STATE_MASK        0x3F
>>>>
>>>> These are now unused.
>>>>
>>>>> +static const struct acpi_device_id hisi_pcie_rc_res_ids[] = {
>>>>> +    {"HISI0081", 0},
>>>>> +    {"", 0},
>>>>> +};
>>>>> +
>>>>> +static int hisi_pcie_acpi_rd_conf(struct pci_bus *bus, u32 devfn,
>>>>> int where,
>>>>> +                  int size, u32 *val)
>>>>> +{
>>>>> +    struct pci_config_window *cfg = bus->sysdata;
>>>>> +    int dev = PCI_SLOT(devfn);
>>>>> +
>>>>> +    if (bus->number == cfg->busr.start) {
>>>>> +        /* access only one slot on each root port */
>>>>> +        if (dev > 0)
>>>>> +            return PCIBIOS_DEVICE_NOT_FOUND;
>>>>> +        else
>>>>> +            return pci_generic_config_read32(bus, devfn, where,
>>>>> +                             size, val);
>>>>> +    }
>>>>> +
>>>>> +    return pci_generic_config_read(bus, devfn, where, size, val);
>>>>> +}
>>>>> +
>>>>> +static int hisi_pcie_acpi_wr_conf(struct pci_bus *bus, u32 devfn,
>>>>> +                  int where, int size, u32 val)
>>>>> +{
>>>>> +    struct pci_config_window *cfg = bus->sysdata;
>>>>> +    int dev = PCI_SLOT(devfn);
>>>>> +
>>>>> +    if (bus->number == cfg->busr.start) {
>>>>> +        /* access only one slot on each root port */
>>>>> +        if (dev > 0)
>>>>> +            return PCIBIOS_DEVICE_NOT_FOUND;
>>>>> +        else
>>>>> +            return pci_generic_config_write32(bus, devfn, where,
>>>>> +                              size, val);
>>>>> +    }
>>>>> +
>>>>> +    return pci_generic_config_write(bus, devfn, where, size, val);
>>>>> +}
>>>>> +
>>>>> +static void __iomem *hisi_pcie_map_bus(struct pci_bus *bus,
>>>>> unsigned int devfn,
>>>>> +                       int where)
>>>>> +{
>>>>> +    struct pci_config_window *cfg = bus->sysdata;
>>>>> +    void __iomem *reg_base = cfg->priv;
>>>>> +
>>>>> +    if (bus->number == cfg->busr.start)
>>>>> +        return reg_base + where;
>>>>> +    else
>>>>> +        return pci_ecam_map_bus(bus, devfn, where);
>>>>> +}
>>>>> +
>>>>> +/*
>>>>> + * Retrieve RC base and size from sub-device under the RC
>>>>> + * Device (RES1)
>>>>> + * {
>>>>> + *    Name (_HID, "HISI0081")
>>>>> + *    Name (_CID, "PNP0C02")
>>>>> + *    Name (_CRS, ResourceTemplate (){
>>>>> + *        Memory32Fixed (ReadWrite, 0xb0080000, 0x10000)
>>>>> + *    })
>>>>> + * }
>>>>> + */
>>>>> +static int hisi_pcie_rc_addr_get(struct acpi_device *adev,
>>>>> +                 void __iomem **addr)
>>>>> +{
>>>>> +    struct acpi_device *child_adev;
>>>>> +    struct list_head list;
>>>>> +    struct resource *res;
>>>>> +    struct resource_entry *entry;
>>>>> +    unsigned long flags;
>>>>> +    int ret;
>>>>> +
>>>>> +    list_for_each_entry(child_adev, &adev->children, node) {
>>>>> +        ret = acpi_match_device_ids(child_adev, hisi_pcie_rc_res_ids);
>>>>> +        if (ret)
>>>>> +            continue;
>>>>> +
>>>>> +        INIT_LIST_HEAD(&list);
>>>>> +        flags = IORESOURCE_MEM;
>>>>> +        ret = acpi_dev_get_resources(child_adev, &list,
>>>>> +                         acpi_dev_filter_resource_type_cb,
>>>>> +                         (void *)flags);
>>>>> +        if (ret < 0) {
>>>>> +            dev_err(&child_adev->dev,
>>>>> +                "failed to parse _CRS method, error code %d\n",
>>>>> +                ret);
>>>>> +            return ret;
>>>>> +        } else if (ret == 0) {
>>>>> +            dev_err(&child_adev->dev,
>>>>> +                "no IO and memory resources present in _CRS\n");
>>>>> +            return -EINVAL;
>>>>> +        }
>>>>> +
>>>>> +        entry = list_first_entry(&list, struct resource_entry, node);
>>>>> +        res = entry->res;
>>>>> +        *addr = devm_ioremap(&child_adev->dev,
>>>>> +                     res->start, resource_size(res));
>>>>> +        acpi_dev_free_resource_list(&list);
>>>>> +        if (IS_ERR(*addr)) {
>>>>> +            dev_err(&child_adev->dev, "error with ioremap\n");
>>>>> +            return -ENOMEM;
>>>>> +        }
>>>>> +
>>>>> +        return 0;
>>>>> +    }
>>>>> +
>>>>> +    return -EINVAL;
>>>>> +}
>>>>
>>>> OK, so MCFG (with the possible help of quirks) tells us where *most*
>>>> of this RC's ECAM space is, but it doesn't tell us where the root bus
>>>> ECAM space is, which is why we need this block of ugly code.  Right?
>>>
>>> Tomasz is doing the same thing for ThunderX.  Can we share the code
>>> somehow, e.g., add something that takes the _HID to look for and just
>>> returns the register address?  Maybe in pci-acpi.c, decl in
>>> drivers/pci/pci.h, under #ifdef CONFIG_ARM64?0
>>
>> As we have the PNP0C02 device at the root of the ACPI namespace (under
>> \_SB).
>> We need a way to tell which root bus the PNP0C02 resource belong to.
>>
>> Firstly, use _HID(HISI0081) to get the PNP0C02 devices that we need..
>> Secondly, use _UID to match segment to tell which root bus the PNP0C02
>> resource belong to.
>>
>> The implementaton is as below. What do you think about this?
>> If the code is ok for other manufacturers, we can share the code.
>>
>> struct hisi_pcie_acpi {
>>     u16 segment;
>>     acpi_handle handle;
>> };
>>
>> /*
>>  * Retrieve RC base and size from PNP0C02 at the root of the ACPI namespace
>>  * (under \_SB).
>>  * Device (RES0)
>>  * {
>>  *    Name (_HID, "HISI0081")
>>  *    Name (_CID, "PNP0C02")
>>  *    Name (_UID, 0x0)
>>  *    Name (_CRS, ResourceTemplate (){
>>  *        Memory32Fixed (ReadWrite, 0xa0090000, 0x10000)
>>  *    })
>>  * }
>>  */
>> static int hisi_pcie_rc_addr_get(struct acpi_device *adev,
>>                  void __iomem **addr)
>> {
>>     struct list_head list;
>>     struct resource *res;
>>     struct resource_entry *entry;
>>     unsigned long flags;
>>     int ret;
>>
>>     INIT_LIST_HEAD(&list);
>>     flags = IORESOURCE_MEM;
>>     ret = acpi_dev_get_resources(adev, &list,
>>                      acpi_dev_filter_resource_type_cb,
>>                      (void *)flags);
>>     if (ret < 0) {
>>         dev_err(&adev->dev,
>>             "failed to parse _CRS method, error code %d\n",
>>             ret);
>>         return ret;
>>     } else if (ret == 0) {
>>         dev_err(&adev->dev,
>>             "no IO and memory resources present in _CRS\n");
>>         return -EINVAL;
>>     }
>>
>>     entry = list_first_entry(&list, struct resource_entry, node);
>>     res = entry->res;
>>     *addr = devm_ioremap(&adev->dev,
>>                  res->start, resource_size(res));
>>     acpi_dev_free_resource_list(&list);
>>     if (IS_ERR(*addr)) {
>>         dev_err(&adev->dev, "error with ioremap\n");
>>         return -ENOMEM;
>>     }
>>
>>     return 0;
>> }
>>
>> static acpi_status find_rc_resource(acpi_handle handle, u32 lvl,
>>                     void *context, void **rv)
>> {
>>     struct hisi_pcie_acpi *pcie_acpi = context;
>>     acpi_status status;
>>     unsigned long long uid;
>>
>>     status = acpi_evaluate_integer(handle, "_UID", NULL, &uid);
>>     if (ACPI_FAILURE(status) || uid != pcie_acpi->segment)
>>         return AE_CTRL_DEPTH;
>>
>>     pcie_acpi->handle = handle;
>>     return AE_CTRL_TERMINATE;
>> }
>>
>> static int hisi_pcie_init(struct pci_config_window *cfg)
>> {
>>     struct acpi_device *adev = to_acpi_device(cfg->parent);
>>     struct acpi_pci_root *root = acpi_driver_data(adev);
>>     struct hisi_pcie_acpi *pcie_acpi;
>>     int ret;
>>     void __iomem *reg_base;
>>
>>     pcie_acpi = devm_kzalloc(&adev->dev, sizeof(*pcie_acpi), GFP_KERNEL);
>>     if (!pcie_acpi)
>>         return -ENOMEM;
>>
>>     pcie_acpi->segment = root->segment;
>>     acpi_get_devices("HISI0081", find_rc_resource, pcie_acpi, NULL);
>>
>>     ret = acpi_bus_get_device(pcie_acpi->handle, &adev);
>>     if (ret)
>>         return ret;
>>
>>     ret = hisi_pcie_rc_addr_get(adev, &reg_base);
>>     if (ret) {
>>         dev_err(&adev->dev, "can't get rc base address");
>>         return ret;
>>     }
>>
>>     cfg->priv = pcie_acpi->reg_base;
>>     return 0;
>> }
>>
>> Thanks
>> Dongdong
>>>
>>> .
>>>
>>
>
> .
>