Re: [RFC PATCH V2 1/3] PCI: hisi: re-architect Hip05/Hip06 controllers driver to preapare for ACPI

Re: [RFC PATCH V2 1/3] PCI: hisi: re-architect Hip05/Hip06 controllers driver to preapare for ACPI

[Arnd Bergmann]

On Wednesday, August 31, 2016 7:48:12 PM CEST Dongdong Liu wrote:
> +
> +/* HipXX PCIe host only supports 32-bit config access */
> +int hisi_pcie_common_cfg_read(void __iomem *reg_base, int where, int size,
> +                             u32 *val)
> +{
> +       u32 reg;
> +       u32 reg_val;
> +       void *walker = &reg_val;
> +
> +       walker += (where & 0x3);
> +       reg = where & ~0x3;
> +       reg_val = readl(reg_base + reg);
> +
> +       if (size == 1)
> +               *val = *(u8 __force *) walker;
> +       else if (size == 2)
> +               *val = *(u16 __force *) walker;

What is the __force for?

> +       else if (size == 4)
> +               *val = reg_val;
> +       else
> +               return PCIBIOS_BAD_REGISTER_NUMBER;
> +
> +       return PCIBIOS_SUCCESSFUL;

It looks like you are reimplementing pci_generic_config_read32/pci_generic_config_write32
read here, better use them directly.

	Arnd

[RFC PATCH V2 0/3] Add ACPI support for Hisilicon PCIe Host Controller

[Dongdong Liu]

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

This patchset is base on Tomasz RFC V5 quirk mechanism:
https://lkml.org/lkml/2016/8/8/273

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

Dongdong Liu (3):
  PCI: hisi: re-architect Hip05/Hip06 controllers driver to preapare for
    ACPI
  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                                        |   2 +
 drivers/pci/host/Kconfig                           |   8 +
 drivers/pci/host/Makefile                          |   3 +-
 drivers/pci/host/mcfg-quirks.c                     |   8 +
 drivers/pci/host/mcfg-quirks.h                     |  11 ++
 drivers/pci/host/pcie-designware.c                 |   3 +-
 drivers/pci/host/pcie-designware.h                 |   2 +
 drivers/pci/host/pcie-hisi-acpi.c                  | 189 +++++++++++++++++++++
 drivers/pci/host/pcie-hisi-common.c                |  66 +++++++
 drivers/pci/host/pcie-hisi.c                       | 143 ++++++++--------
 drivers/pci/host/pcie-hisi.h                       |  25 +++
 12 files changed, 392 insertions(+), 83 deletions(-)
 create mode 100644 drivers/pci/host/pcie-hisi-acpi.c
 create mode 100644 drivers/pci/host/pcie-hisi-common.c
 create mode 100644 drivers/pci/host/pcie-hisi.h

-- 
1.9.1

[RFC PATCH V2 1/3] PCI: hisi: re-architect Hip05/Hip06 controllers driver to preapare for ACPI

[Dongdong Liu]

re-architect the Hip05/Hip06 host controllers driver to prepare
for the ACPI based driver.
The common functions used also by the ACPI driver have been grouped
into a new "common" file.

Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
---
 MAINTAINERS                         |   2 +
 drivers/pci/host/Makefile           |   2 +-
 drivers/pci/host/pcie-hisi-common.c |  66 ++++++++++++++++++++++
 drivers/pci/host/pcie-hisi.c        | 110 ++++++++++--------------------------
 drivers/pci/host/pcie-hisi.h        |  23 ++++++++
 5 files changed, 123 insertions(+), 80 deletions(-)
 create mode 100644 drivers/pci/host/pcie-hisi-common.c
 create mode 100644 drivers/pci/host/pcie-hisi.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 20bb1d0..cdc1bba 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9081,7 +9081,9 @@ M:	Gabriele Paoloni <gabriele.paoloni@huawei.com>
 L:	linux-pci@vger.kernel.org
 S:	Maintained
 F:	Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
+F:	drivers/pci/host/pcie-hisi.h
 F:	drivers/pci/host/pcie-hisi.c
+F:	drivers/pci/host/pcie-hisi-common.c
 
 PCIE DRIVER FOR QUALCOMM MSM
 M:     Stanimir Varbanov <svarbanov@mm-sol.com>
diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
index 500cf78..02b498d 100644
--- a/drivers/pci/host/Makefile
+++ b/drivers/pci/host/Makefile
@@ -25,7 +25,7 @@ obj-$(CONFIG_PCIE_IPROC_PLATFORM) += pcie-iproc-platform.o
 obj-$(CONFIG_PCIE_IPROC_BCMA) += pcie-iproc-bcma.o
 obj-$(CONFIG_PCIE_ALTERA) += pcie-altera.o
 obj-$(CONFIG_PCIE_ALTERA_MSI) += pcie-altera-msi.o
-obj-$(CONFIG_PCI_HISI) += pcie-hisi.o
+obj-$(CONFIG_PCI_HISI) += pcie-hisi.o pcie-hisi-common.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-common.c b/drivers/pci/host/pcie-hisi-common.c
new file mode 100644
index 0000000..5a5f269
--- /dev/null
+++ b/drivers/pci/host/pcie-hisi-common.c
@@ -0,0 +1,66 @@
+/*
+ * PCIe host controller common functions for HiSilicon SoCs
+ *
+ * Copyright (C) 2015 HiSilicon Co., Ltd. http://www.hisilicon.com
+ *
+ * Author: Zhou Wang <wangzhou1@hisilicon.com>
+ *         Dacai Zhu <zhudacai@hisilicon.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 "pcie-hisi.h"
+
+/* HipXX PCIe host only supports 32-bit config access */
+int hisi_pcie_common_cfg_read(void __iomem *reg_base, int where, int size,
+			      u32 *val)
+{
+	u32 reg;
+	u32 reg_val;
+	void *walker = &reg_val;
+
+	walker += (where & 0x3);
+	reg = where & ~0x3;
+	reg_val = readl(reg_base + reg);
+
+	if (size == 1)
+		*val = *(u8 __force *) walker;
+	else if (size == 2)
+		*val = *(u16 __force *) walker;
+	else if (size == 4)
+		*val = reg_val;
+	else
+		return PCIBIOS_BAD_REGISTER_NUMBER;
+
+	return PCIBIOS_SUCCESSFUL;
+}
+
+/* HipXX PCIe host only supports 32-bit config access */
+int hisi_pcie_common_cfg_write(void __iomem *reg_base, int where, int  size,
+				u32 val)
+{
+	u32 reg_val;
+	u32 reg;
+	void *walker = &reg_val;
+
+	walker += (where & 0x3);
+	reg = where & ~0x3;
+	if (size == 4)
+		writel(val, reg_base + reg);
+	else if (size == 2) {
+		reg_val = readl(reg_base + reg);
+		*(u16 __force *) walker = val;
+		writel(reg_val, reg_base + reg);
+	} else if (size == 1) {
+		reg_val = readl(reg_base + reg);
+		*(u8 __force *) walker = val;
+		writel(reg_val, reg_base + reg);
+	} else
+		return PCIBIOS_BAD_REGISTER_NUMBER;
+
+	return PCIBIOS_SUCCESSFUL;
+}
diff --git a/drivers/pci/host/pcie-hisi.c b/drivers/pci/host/pcie-hisi.c
index 7ee9dfc..23d74e9 100644
--- a/drivers/pci/host/pcie-hisi.c
+++ b/drivers/pci/host/pcie-hisi.c
@@ -21,6 +21,7 @@
 #include <linux/regmap.h>
 
 #include "pcie-designware.h"
+#include "pcie-hisi.h"
 
 #define PCIE_LTSSM_LINKUP_STATE				0x11
 #define PCIE_LTSSM_STATE_MASK				0x3F
@@ -30,12 +31,6 @@
 
 #define to_hisi_pcie(x)	container_of(x, struct hisi_pcie, pp)
 
-struct hisi_pcie;
-
-struct pcie_soc_ops {
-	int (*hisi_pcie_link_up)(struct hisi_pcie *pcie);
-};
-
 struct hisi_pcie {
 	struct regmap *subctrl;
 	void __iomem *reg_base;
@@ -44,87 +39,24 @@ struct hisi_pcie {
 	struct pcie_soc_ops *soc_ops;
 };
 
-static inline void hisi_pcie_apb_writel(struct hisi_pcie *pcie,
-					u32 val, u32 reg)
-{
-	writel(val, pcie->reg_base + reg);
-}
-
-static inline u32 hisi_pcie_apb_readl(struct hisi_pcie *pcie, u32 reg)
-{
-	return readl(pcie->reg_base + reg);
-}
+struct pcie_soc_ops {
+	int (*hisi_pcie_link_up)(struct hisi_pcie *pcie);
+};
 
-/* 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)
+static inline int hisi_pcie_cfg_read(struct pcie_port *pp, int where,
+		int size, u32 *val)
 {
-	u32 reg;
-	u32 reg_val;
 	struct hisi_pcie *pcie = to_hisi_pcie(pp);
-	void *walker = &reg_val;
-
-	walker += (where & 0x3);
-	reg = where & ~0x3;
-	reg_val = hisi_pcie_apb_readl(pcie, reg);
-
-	if (size == 1)
-		*val = *(u8 __force *) walker;
-	else if (size == 2)
-		*val = *(u16 __force *) walker;
-	else if (size == 4)
-		*val = reg_val;
-	else
-		return PCIBIOS_BAD_REGISTER_NUMBER;
-
-	return PCIBIOS_SUCCESSFUL;
-}
 
-/* HipXX PCIe host only supports 32-bit config access */
-static int hisi_pcie_cfg_write(struct pcie_port *pp, int where, int  size,
-				u32 val)
-{
-	u32 reg_val;
-	u32 reg;
-	struct hisi_pcie *pcie = to_hisi_pcie(pp);
-	void *walker = &reg_val;
-
-	walker += (where & 0x3);
-	reg = where & ~0x3;
-	if (size == 4)
-		hisi_pcie_apb_writel(pcie, val, reg);
-	else if (size == 2) {
-		reg_val = hisi_pcie_apb_readl(pcie, reg);
-		*(u16 __force *) walker = val;
-		hisi_pcie_apb_writel(pcie, reg_val, reg);
-	} else if (size == 1) {
-		reg_val = hisi_pcie_apb_readl(pcie, reg);
-		*(u8 __force *) walker = val;
-		hisi_pcie_apb_writel(pcie, reg_val, reg);
-	} else
-		return PCIBIOS_BAD_REGISTER_NUMBER;
-
-	return PCIBIOS_SUCCESSFUL;
+	return hisi_pcie_common_cfg_read(pcie->reg_base, where, size, val);
 }
 
-static int hisi_pcie_link_up_hip05(struct hisi_pcie *hisi_pcie)
+static inline int hisi_pcie_cfg_write(struct pcie_port *pp, int where,
+		int size, u32 val)
 {
-	u32 val;
-
-	regmap_read(hisi_pcie->subctrl, PCIE_SUBCTRL_SYS_STATE4_REG +
-		    0x100 * hisi_pcie->port_id, &val);
-
-	return ((val & PCIE_LTSSM_STATE_MASK) == PCIE_LTSSM_LINKUP_STATE);
-}
-
-static int hisi_pcie_link_up_hip06(struct hisi_pcie *hisi_pcie)
-{
-	u32 val;
-
-	val = hisi_pcie_apb_readl(hisi_pcie, PCIE_HIP06_CTRL_OFF +
-			PCIE_SYS_STATE4);
+	struct hisi_pcie *pcie = to_hisi_pcie(pp);
 
-	return ((val & PCIE_LTSSM_STATE_MASK) == PCIE_LTSSM_LINKUP_STATE);
+	return hisi_pcie_common_cfg_write(pcie->reg_base, where, size, val);
 }
 
 static int hisi_pcie_link_up(struct pcie_port *pp)
@@ -215,6 +147,26 @@ static int hisi_pcie_probe(struct platform_device *pdev)
 	return 0;
 }
 
+static int hisi_pcie_link_up_hip05(struct hisi_pcie *hisi_pcie)
+{
+	u32 val;
+
+	regmap_read(hisi_pcie->subctrl, PCIE_SUBCTRL_SYS_STATE4_REG +
+		    0x100 * hisi_pcie->port_id, &val);
+
+	return ((val & PCIE_LTSSM_STATE_MASK) == PCIE_LTSSM_LINKUP_STATE);
+}
+
+static int hisi_pcie_link_up_hip06(struct hisi_pcie *hisi_pcie)
+{
+	u32 val;
+
+	val = readl(hisi_pcie->reg_base + PCIE_HIP06_CTRL_OFF +
+			PCIE_SYS_STATE4);
+
+	return ((val & PCIE_LTSSM_STATE_MASK) == PCIE_LTSSM_LINKUP_STATE);
+}
+
 static struct pcie_soc_ops hip05_ops = {
 		&hisi_pcie_link_up_hip05
 };
diff --git a/drivers/pci/host/pcie-hisi.h b/drivers/pci/host/pcie-hisi.h
new file mode 100644
index 0000000..44fc680
--- /dev/null
+++ b/drivers/pci/host/pcie-hisi.h
@@ -0,0 +1,23 @@
+/*
+ * PCIe host controller driver for HiSilicon SoCs
+ *
+ * Copyright (C) 2015 HiSilicon Co., Ltd. http://www.hisilicon.com
+ *
+ * Author: Zhou Wang <wangzhou1@hisilicon.com>
+ *         Dacai Zhu <zhudacai@hisilicon.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.
+ */
+#ifndef PCIE_HISI_H_
+#define PCIE_HISI_H_
+
+
+int hisi_pcie_common_cfg_read(void __iomem *reg_base, int where, int size,
+			      u32 *val);
+int hisi_pcie_common_cfg_write(void __iomem *reg_base, int where, int size,
+			       u32 val);
+
+#endif /* PCIE_HISI_H_ */
-- 
1.9.1

[RFC PATCH V2 2/3] 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/pcie-designware.c                 |  3 +-
 drivers/pci/host/pcie-designware.h                 |  2 +
 drivers/pci/host/pcie-hisi.c                       | 43 ++++++++++++++++++++++
 4 files changed, 56 insertions(+), 7 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/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index 12afce1..9f5514c 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -75,7 +75,6 @@
 #define PCIE_PHY_DEBUG_R1		(PLR_OFFSET + 0x2c)
 #define PCIE_PHY_DEBUG_R1_LINK_UP	0x00000010
 
-static struct pci_ops dw_pcie_ops;
 
 int dw_pcie_cfg_read(void __iomem *addr, int size, u32 *val)
 {
@@ -710,7 +709,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 f437f9b..234f360 100644
--- a/drivers/pci/host/pcie-designware.h
+++ b/drivers/pci/host/pcie-designware.h
@@ -86,4 +86,6 @@ int dw_pcie_link_up(struct pcie_port *pp);
 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 23d74e9..81eeaed 100644
--- a/drivers/pci/host/pcie-hisi.c
+++ b/drivers/pci/host/pcie-hisi.c
@@ -43,6 +43,18 @@ struct pcie_soc_ops {
 	int (*hisi_pcie_link_up)(struct hisi_pcie *pcie);
 };
 
+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 inline int hisi_pcie_cfg_read(struct pcie_port *pp, int where,
 		int size, u32 *val)
 {
@@ -72,6 +84,20 @@ static struct pcie_host_ops hisi_pcie_host_ops = {
 	.link_up = hisi_pcie_link_up,
 };
 
+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;
+}
+
 static int hisi_add_pcie_port(struct pcie_port *pp,
 				     struct platform_device *pdev)
 {
@@ -136,6 +162,23 @@ static int hisi_pcie_probe(struct platform_device *pdev)
 
 	hisi_pcie->pp.dbi_base = hisi_pcie->reg_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(pp, pdev);
 	if (ret)
 		return ret;
-- 
1.9.1

[RFC PATCH V2 3/3] PCI/ACPI: hisi: Add ACPI support for HiSilicon SoCs Host Controllers

[Dongdong Liu]

Add specific quirks for PCI config space accessors.This involves:
1. New initialization call hisi_pcie_acpi_init() to get RC config resource
with hardcoded range address and setup ecam mapping.
2. New entry in common quirk array.

Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
---
 drivers/pci/host/Kconfig          |   8 ++
 drivers/pci/host/Makefile         |   1 +
 drivers/pci/host/mcfg-quirks.c    |   8 ++
 drivers/pci/host/mcfg-quirks.h    |  11 +++
 drivers/pci/host/pcie-hisi-acpi.c | 189 ++++++++++++++++++++++++++++++++++++++
 drivers/pci/host/pcie-hisi.c      |   2 -
 drivers/pci/host/pcie-hisi.h      |   2 +
 7 files changed, 219 insertions(+), 2 deletions(-)
 create mode 100644 drivers/pci/host/pcie-hisi-acpi.c

diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
index 9b485d8..f940050 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 SoCs
 
+config PCI_HISI_ACPI
+	depends on ACPI
+	bool "HiSilicon Hip05 and Hip06 SoCs ACPI PCIe controllers"
+	select ACPI_PCI_HOST_GENERIC
+	help
+	  Say Y here if you want ACPI PCIe controller support on HiSilicon
+	  Hip05 and Hip06 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 02b498d..bf6bbad 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 pcie-hisi-common.o
+obj-$(CONFIG_PCI_HISI_ACPI) += pcie-hisi-acpi.o pcie-hisi-common.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/mcfg-quirks.c b/drivers/pci/host/mcfg-quirks.c
index 2993a72..772a453 100644
--- a/drivers/pci/host/mcfg-quirks.c
+++ b/drivers/pci/host/mcfg-quirks.c
@@ -51,6 +51,14 @@ static struct pci_cfg_fixup mcfg_quirks[] __initconst = {
 	{ "CAVIUM", "THUNDERX", 1, MCFG_DOM_RANGE(14, 19), MCFG_BUS_ANY, NULL,
 	  thunder_pem_cfg_init },
 #endif
+#ifdef CONFIG_PCI_HISI_ACPI
+	{ "HISI  ", "HIP05   ", 0, MCFG_DOM_RANGE(0, 3), MCFG_BUS_ANY,
+	  NULL, hisi_pcie_acpi_hip05_init},
+	{ "HISI  ", "HIP06   ", 0, MCFG_DOM_RANGE(0, 3), MCFG_BUS_ANY,
+	  NULL, hisi_pcie_acpi_hip06_init},
+	{ "HISI  ", "HIP07   ", 0, MCFG_DOM_RANGE(0, 15), MCFG_BUS_ANY,
+	  NULL, hisi_pcie_acpi_hip07_init},
+#endif
 };
 
 static bool pci_mcfg_fixup_match(struct pci_cfg_fixup *f,
diff --git a/drivers/pci/host/mcfg-quirks.h b/drivers/pci/host/mcfg-quirks.h
index 411c667..e496ddd 100644
--- a/drivers/pci/host/mcfg-quirks.h
+++ b/drivers/pci/host/mcfg-quirks.h
@@ -21,4 +21,15 @@ struct pci_config_window *
 thunder_pem_cfg_init(struct acpi_pci_root *root, struct pci_ops *ops);
 #endif
 
+#ifdef CONFIG_PCI_HISI_ACPI
+struct pci_config_window *
+hisi_pcie_acpi_hip05_init(struct acpi_pci_root *root, struct pci_ops *ops);
+
+struct pci_config_window *
+hisi_pcie_acpi_hip06_init(struct acpi_pci_root *root, struct pci_ops *ops);
+
+struct pci_config_window *
+hisi_pcie_acpi_hip07_init(struct acpi_pci_root *root, struct pci_ops *ops);
+#endif
+
 #endif /* __MCFG_QUIRKS_H__ */
diff --git a/drivers/pci/host/pcie-hisi-acpi.c b/drivers/pci/host/pcie-hisi-acpi.c
new file mode 100644
index 0000000..68cf297
--- /dev/null
+++ b/drivers/pci/host/pcie-hisi-acpi.c
@@ -0,0 +1,189 @@
+/*
+ * 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>
+
+#include "mcfg-quirks.h"
+#include "pcie-hisi.h"
+
+#define DEBUG0          0x728
+#define MAX_RC_NUM	16
+
+enum soc_type {
+	HIP05,
+	HIP06,
+	HIP07,
+};
+
+struct hisi_rc_res {
+	int soc_type;
+	struct resource res[MAX_RC_NUM];
+};
+
+static int hisi_pcie_link_up_acpi(struct pci_config_window *cfg)
+{
+	u32 val;
+	void __iomem *reg_base = cfg->priv;
+
+	val = readl(reg_base + DEBUG0);
+	return ((val & PCIE_LTSSM_STATE_MASK) == PCIE_LTSSM_LINKUP_STATE);
+
+}
+
+static int hisi_pcie_acpi_valid_config(struct pci_config_window *cfg,
+				       struct pci_bus *bus, int dev)
+{
+	/* If there is no link, then there is no device */
+	if (bus->number != cfg->busr.start) {
+		if (!hisi_pcie_link_up_acpi(cfg))
+			return 0;
+	}
+
+	/* access only one slot on each root port */
+	if (bus->number == cfg->busr.start && dev > 0)
+		return 0;
+
+	/*
+	 * do not read more than one device on the bus directly attached
+	 * to RC's (Virtual Bridge's) DS side.
+	 */
+	if (bus->primary == cfg->busr.start && dev > 0)
+		return 0;
+
+	return 1;
+}
+
+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;
+	void __iomem *reg_base = cfg->priv;
+
+	if (hisi_pcie_acpi_valid_config(cfg, bus, PCI_SLOT(devfn)) == 0)
+		return PCIBIOS_DEVICE_NOT_FOUND;
+
+	if (bus->number == cfg->busr.start)
+		return hisi_pcie_common_cfg_read(reg_base, 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;
+	void __iomem *reg_base = cfg->priv;
+
+	if (hisi_pcie_acpi_valid_config(cfg, bus, PCI_SLOT(devfn)) == 0)
+		return PCIBIOS_DEVICE_NOT_FOUND;
+
+	if (bus->number == cfg->busr.start)
+		return hisi_pcie_common_cfg_write(reg_base, where, size, val);
+
+	return pci_generic_config_write(bus, devfn, where, size, val);
+}
+
+static struct pci_ops hisi_pcie_ops = {
+	.map_bus	= pci_ecam_map_bus,
+	.read		= hisi_pcie_acpi_rd_conf,
+	.write		= hisi_pcie_acpi_wr_conf,
+};
+
+static struct hisi_rc_res rc_res[] = {
+	{
+		HIP05,
+		{
+			DEFINE_RES_MEM(0xb0070000, SZ_4K),
+			DEFINE_RES_MEM(0xb0080000, SZ_4K),
+			DEFINE_RES_MEM(0xb0090000, SZ_4K),
+			DEFINE_RES_MEM(0xb00a0000, SZ_4K)
+		}
+	},
+	{
+		HIP06,
+		{
+			DEFINE_RES_MEM(0xa0090000, SZ_4K),
+			DEFINE_RES_MEM(0xa0200000, SZ_4K),
+			DEFINE_RES_MEM(0xa00a0000, SZ_4K),
+			DEFINE_RES_MEM(0xa00b0000, SZ_4K)
+		}
+	},
+	{
+		HIP07,
+		{
+			DEFINE_RES_MEM(0xa0090000, SZ_4K),
+			DEFINE_RES_MEM(0xa0200000, SZ_4K),
+			DEFINE_RES_MEM(0xa00a0000, SZ_4K),
+			DEFINE_RES_MEM(0xa00b0000, SZ_4K),
+			DEFINE_RES_MEM(0x8a0090000UL, SZ_4K),
+			DEFINE_RES_MEM(0x8a0200000UL, SZ_4K),
+			DEFINE_RES_MEM(0x8a00a0000UL, SZ_4K),
+			DEFINE_RES_MEM(0x8a00b0000UL, SZ_4K),
+			DEFINE_RES_MEM(0x600a0090000UL, SZ_4K),
+			DEFINE_RES_MEM(0x600a0200000UL, SZ_4K),
+			DEFINE_RES_MEM(0x600a00a0000UL, SZ_4K),
+			DEFINE_RES_MEM(0x600a00b0000UL, SZ_4K),
+			DEFINE_RES_MEM(0x700a0090000UL, SZ_4K),
+			DEFINE_RES_MEM(0x700a0200000UL, SZ_4K),
+			DEFINE_RES_MEM(0x700a00a0000UL, SZ_4K),
+			DEFINE_RES_MEM(0x700a00b0000UL, SZ_4K)
+		}
+	},
+};
+
+struct pci_config_window *hisi_pcie_acpi_init(struct acpi_pci_root *root,
+					      struct pci_ops *ops,
+					      int soc_type)
+{
+	struct acpi_device *adev = root->device;
+	void __iomem *reg_base;
+	struct pci_config_window *cfg;
+	struct resource *res;
+
+	res = &rc_res[soc_type].res[root->segment];
+	reg_base = devm_ioremap_resource(&adev->dev, res);
+	if (!reg_base)
+		return ERR_PTR(-ENOMEM);
+
+	cfg = pci_acpi_setup_ecam_mapping(root, &hisi_pcie_ops);
+	if (IS_ERR(cfg)) {
+		dev_err(&adev->dev, "error %ld mapping ECAM\n", PTR_ERR(cfg));
+		return NULL;
+	}
+
+	cfg->priv = reg_base;
+
+	if (!hisi_pcie_link_up_acpi(cfg))
+		dev_warn(&adev->dev, "link status is down\n");
+
+	return cfg;
+}
+
+struct pci_config_window *hisi_pcie_acpi_hip05_init(struct acpi_pci_root *root,
+						    struct pci_ops *ops)
+{
+	return hisi_pcie_acpi_init(root, ops, HIP05);
+}
+
+struct pci_config_window *hisi_pcie_acpi_hip06_init(struct acpi_pci_root *root,
+						    struct pci_ops *ops)
+{
+	return hisi_pcie_acpi_init(root, ops, HIP06);
+}
+
+struct pci_config_window *hisi_pcie_acpi_hip07_init(struct acpi_pci_root *root,
+						    struct pci_ops *ops)
+{
+	return hisi_pcie_acpi_init(root, ops, HIP07);
+}
diff --git a/drivers/pci/host/pcie-hisi.c b/drivers/pci/host/pcie-hisi.c
index 81eeaed..0d29c0f 100644
--- a/drivers/pci/host/pcie-hisi.c
+++ b/drivers/pci/host/pcie-hisi.c
@@ -23,8 +23,6 @@
 #include "pcie-designware.h"
 #include "pcie-hisi.h"
 
-#define PCIE_LTSSM_LINKUP_STATE				0x11
-#define PCIE_LTSSM_STATE_MASK				0x3F
 #define PCIE_SUBCTRL_SYS_STATE4_REG			0x6818
 #define PCIE_SYS_STATE4						0x31c
 #define PCIE_HIP06_CTRL_OFF					0x1000
diff --git a/drivers/pci/host/pcie-hisi.h b/drivers/pci/host/pcie-hisi.h
index 44fc680..edb4977 100644
--- a/drivers/pci/host/pcie-hisi.h
+++ b/drivers/pci/host/pcie-hisi.h
@@ -14,6 +14,8 @@
 #ifndef PCIE_HISI_H_
 #define PCIE_HISI_H_
 
+#define PCIE_LTSSM_LINKUP_STATE				0x11
+#define PCIE_LTSSM_STATE_MASK				0x3F
 
 int hisi_pcie_common_cfg_read(void __iomem *reg_base, int where, int size,
 			      u32 *val);
-- 
1.9.1

Re: [RFC PATCH V2 3/3] PCI/ACPI: hisi: Add ACPI support for HiSilicon SoCs Host Controllers

[Arnd Bergmann]

On Wednesday, August 31, 2016 7:48:14 PM CEST Dongdong Liu wrote:
> +static struct hisi_rc_res rc_res[] = {
> +       {
> +               HIP05,
> +               {
> +                       DEFINE_RES_MEM(0xb0070000, SZ_4K),
> +                       DEFINE_RES_MEM(0xb0080000, SZ_4K),
> +                       DEFINE_RES_MEM(0xb0090000, SZ_4K),
> +                       DEFINE_RES_MEM(0xb00a0000, SZ_4K)
> +               }
> +       },
> +       {
> +               HIP06,
> +               {
> +                       DEFINE_RES_MEM(0xa0090000, SZ_4K),
> +                       DEFINE_RES_MEM(0xa0200000, SZ_4K),
> +                       DEFINE_RES_MEM(0xa00a0000, SZ_4K),
> +                       DEFINE_RES_MEM(0xa00b0000, SZ_4K)
> +               }
> +       },
> +       {
> +               HIP07,
> +               {
> +                       DEFINE_RES_MEM(0xa0090000, SZ_4K),
> +                       DEFINE_RES_MEM(0xa0200000, SZ_4K),
> +                       DEFINE_RES_MEM(0xa00a0000, SZ_4K),
> +                       DEFINE_RES_MEM(0xa00b0000, SZ_4K),
> +                       DEFINE_RES_MEM(0x8a0090000UL, SZ_4K),
> +                       DEFINE_RES_MEM(0x8a0200000UL, SZ_4K),
> +                       DEFINE_RES_MEM(0x8a00a0000UL, SZ_4K),
> +                       DEFINE_RES_MEM(0x8a00b0000UL, SZ_4K),
> +                       DEFINE_RES_MEM(0x600a0090000UL, SZ_4K),
> +                       DEFINE_RES_MEM(0x600a0200000UL, SZ_4K),
> +                       DEFINE_RES_MEM(0x600a00a0000UL, SZ_4K),
> +                       DEFINE_RES_MEM(0x600a00b0000UL, SZ_4K),
> +                       DEFINE_RES_MEM(0x700a0090000UL, SZ_4K),
> +                       DEFINE_RES_MEM(0x700a0200000UL, SZ_4K),
> +                       DEFINE_RES_MEM(0x700a00a0000UL, SZ_4K),
> +                       DEFINE_RES_MEM(0x700a00b0000UL, SZ_4K)
> +               }
> +       },

I don't know much about ACPI, but I'm pretty sure this is not
the normal way to find MMIO resources. Why not read them from
the ACPI tables?

	Arnd

Re: [RFC PATCH V2 3/3] PCI/ACPI: hisi: Add ACPI support for HiSilicon SoCs Host Controllers

[Rafael J. Wysocki]

On Wednesday, August 31, 2016 07:48:14 PM Dongdong Liu wrote:
> Add specific quirks for PCI config space accessors.This involves:
> 1. New initialization call hisi_pcie_acpi_init() to get RC config resource
> with hardcoded range address and setup ecam mapping.
> 2. New entry in common quirk array.
> 
> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
> Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>

Well, what exactly is the ACPI support you're adding?  Is it the ECAM part only
or is there anything more to it?

Thanks,
Rafael

Re: [RFC PATCH V2 1/3] PCI: hisi: re-architect Hip05/Hip06 controllers driver to preapare for ACPI

[Dongdong Liu]

在 2016/8/31 19:45, Arnd Bergmann 写道:
> On Wednesday, August 31, 2016 7:48:12 PM CEST Dongdong Liu wrote:
>> +
>> +/* HipXX PCIe host only supports 32-bit config access */
>> +int hisi_pcie_common_cfg_read(void __iomem *reg_base, int where, int size,
>> +                             u32 *val)
>> +{
>> +       u32 reg;
>> +       u32 reg_val;
>> +       void *walker = &reg_val;
>> +
>> +       walker += (where & 0x3);
>> +       reg = where & ~0x3;
>> +       reg_val = readl(reg_base + reg);
>> +
>> +       if (size == 1)
>> +               *val = *(u8 __force *) walker;
>> +       else if (size == 2)
>> +               *val = *(u16 __force *) walker;
>
> What is the __force for?

Hi Arnd, thanks for replying.

__force is used to, well, force a conversion, like casting from or to a bitwise type, else the Sparse checker will throw a warning.

>
>> +       else if (size == 4)
>> +               *val = reg_val;
>> +       else
>> +               return PCIBIOS_BAD_REGISTER_NUMBER;
>> +
>> +       return PCIBIOS_SUCCESSFUL;
>
> It looks like you are reimplementing pci_generic_config_read32/pci_generic_config_write32
> read here, better use them directly.
>

For our host bridge, access RC and EP config space are not the same way.
Our host bridge is non ECAM only for the RC bus config space;
for any other bus underneath the root bus we support ECAM access.

hisi_pcie_common_cfg_read is used to read RC config space, only supports 32-bit config access.
hisi_pcie_common_cfg_read/hisi_pcie_common_cfg_write may change as below will be better.

/* HipXX PCIe host only supports 32-bit config access */
int hisi_pcie_common_cfg_read(void __iomem *reg_base, int where, int size,
			      u32 *val)
{
	void __iomem *addr;

	addr = reg_base + (where & ~0x3);
	*val = readl(addr);

	if (size <= 2)
		*val = (*val >> (8 * (where & 3))) & ((1 << (size * 8)) - 1);

	return PCIBIOS_SUCCESSFUL;
}

/* HipXX PCIe host only supports 32-bit config access */
int hisi_pcie_common_cfg_write(void __iomem *reg_base, int where, int  size,
				u32 val)
{
	void __iomem *addr;
	u32 mask, tmp;

	addr = reg_base + (where & ~0x3);
	if (size == 4) {
		writel(val, addr);
		return PCIBIOS_SUCCESSFUL;
	} else {
		mask = ~(((1 << (size * 8)) - 1) << ((where & 0x3) * 8));
	}

	tmp = readl(addr) & mask;
	tmp |= val << ((where & 0x3) * 8);
	writel(tmp, addr);

	return PCIBIOS_SUCCESSFUL;
}


Thanks
Dongdong

> 	Arnd
>
> .
>

Re: [RFC PATCH V2 3/3] PCI/ACPI: hisi: Add ACPI support for HiSilicon SoCs Host Controllers

[Dongdong Liu]

在 2016/8/31 19:48, Arnd Bergmann 写道:
> On Wednesday, August 31, 2016 7:48:14 PM CEST Dongdong Liu wrote:
>> +static struct hisi_rc_res rc_res[] = {
>> +       {
>> +               HIP05,
>> +               {
>> +                       DEFINE_RES_MEM(0xb0070000, SZ_4K),
>> +                       DEFINE_RES_MEM(0xb0080000, SZ_4K),
>> +                       DEFINE_RES_MEM(0xb0090000, SZ_4K),
>> +                       DEFINE_RES_MEM(0xb00a0000, SZ_4K)
>> +               }
>> +       },
>> +       {
>> +               HIP06,
>> +               {
>> +                       DEFINE_RES_MEM(0xa0090000, SZ_4K),
>> +                       DEFINE_RES_MEM(0xa0200000, SZ_4K),
>> +                       DEFINE_RES_MEM(0xa00a0000, SZ_4K),
>> +                       DEFINE_RES_MEM(0xa00b0000, SZ_4K)
>> +               }
>> +       },
>> +       {
>> +               HIP07,
>> +               {
>> +                       DEFINE_RES_MEM(0xa0090000, SZ_4K),
>> +                       DEFINE_RES_MEM(0xa0200000, SZ_4K),
>> +                       DEFINE_RES_MEM(0xa00a0000, SZ_4K),
>> +                       DEFINE_RES_MEM(0xa00b0000, SZ_4K),
>> +                       DEFINE_RES_MEM(0x8a0090000UL, SZ_4K),
>> +                       DEFINE_RES_MEM(0x8a0200000UL, SZ_4K),
>> +                       DEFINE_RES_MEM(0x8a00a0000UL, SZ_4K),
>> +                       DEFINE_RES_MEM(0x8a00b0000UL, SZ_4K),
>> +                       DEFINE_RES_MEM(0x600a0090000UL, SZ_4K),
>> +                       DEFINE_RES_MEM(0x600a0200000UL, SZ_4K),
>> +                       DEFINE_RES_MEM(0x600a00a0000UL, SZ_4K),
>> +                       DEFINE_RES_MEM(0x600a00b0000UL, SZ_4K),
>> +                       DEFINE_RES_MEM(0x700a0090000UL, SZ_4K),
>> +                       DEFINE_RES_MEM(0x700a0200000UL, SZ_4K),
>> +                       DEFINE_RES_MEM(0x700a00a0000UL, SZ_4K),
>> +                       DEFINE_RES_MEM(0x700a00b0000UL, SZ_4K)
>> +               }
>> +       },
>
> I don't know much about ACPI, but I'm pretty sure this is not
> the normal way to find MMIO resources. Why not read them from
> the ACPI tables?
>

Hi Arnd

Our host bridge is non ECAM only for the RC bus config space;
for any other bus underneath the root bus we support ECAM access.

We have not found a comfortable ACPI way to describle RC itself config (not ECAM) resource .


Thanks
Dongdong

> 	Arnd
>
> .
>

Re: [RFC PATCH V2 3/3] PCI/ACPI: hisi: Add ACPI support for HiSilicon SoCs Host Controllers

[Dongdong Liu]

在 2016/9/1 6:56, Rafael J. Wysocki 写道:
> On Wednesday, August 31, 2016 07:48:14 PM Dongdong Liu wrote:
>> Add specific quirks for PCI config space accessors.This involves:
>> 1. New initialization call hisi_pcie_acpi_init() to get RC config resource
>> with hardcoded range address and setup ecam mapping.
>> 2. New entry in common quirk array.
>>
>> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
>> Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
>
> Well, what exactly is the ACPI support you're adding?  Is it the ECAM part only
> or is there anything more to it?
>

Hi Rafael, thanks for replying.

Our host bridge is non ECAM only for the RC bus config space;
for any other bus underneath the root bus we support ECAM access.

In our case we cannot use the standard MCFG object to pass the RC itself config space addresses.
The more discuss information can be found:
https://lkml.org/lkml/2016/2/22/1087
[...]
I have looked into this and in our case we cannot use the
standard MCFG object to pass the RC config space addresses.

The reason is that in our HW we have the config base addresses of the
root complex ports that are less than 0x100000 byte distant one from
the other as we only map the first 0x10000 bytes.
Now the MCFG acpi framework always fix the MCFG resource size to 0x100000
for each bus; therefore if we pass our RC addresses through MCFG we end
up with a resource conflict.
To give you a practical example we are in a situation where we have:

port0: [0x00000000b0080000 - 0x00000000b0080000 + 0x10000]
port1: [0x00000000b0090000 - 0x00000000b0090000 + 0x10000]
port2: [0x00000000b00A0000 - 0x00000000b00A0000 + 0x10000]
port3: [0x00000000b00B0000 - 0x00000000b00B0000 + 0x10000]
So if we pass the base addresses through MCFG the resources
will overlap as MCFG will consider 0x100000 size for each base
address of the root complex (only the RC bus uses that address)
So far I do not see many option other than using _DSD to pass
these RC config base addresses.

Thanks and Regards

Gab
[...]

and

https://patchwork.kernel.org/patch/9178791/
[...]
Furthermore, I suspect we do not even need a way to pass the
non-ECAM compliant config space resources to the OS (ie we can't
change FW anymore anyway in some platforms) so the quirks hooks
are likely to hardcode the required config space addresses for
the respective MCFG match.

......
Thanks !
Lorenzo
[...]

So we hard code with our RC itself config resource.

Thanks
Dongdong

> Thanks,
> Rafael
>
>
> .
>

Re: [RFC PATCH V2 1/3] PCI: hisi: re-architect Hip05/Hip06 controllers driver to preapare for ACPI

[Arnd Bergmann]

On Thursday, September 1, 2016 10:05:29 AM CEST Dongdong Liu wrote:
> 
> 在 2016/8/31 19:45, Arnd Bergmann 写道:
> > On Wednesday, August 31, 2016 7:48:12 PM CEST Dongdong Liu wrote:
> >> +
> >> +/* HipXX PCIe host only supports 32-bit config access */
> >> +int hisi_pcie_common_cfg_read(void __iomem *reg_base, int where, int size,
> >> +                             u32 *val)
> >> +{
> >> +       u32 reg;
> >> +       u32 reg_val;
> >> +       void *walker = &reg_val;
> >> +
> >> +       walker += (where & 0x3);
> >> +       reg = where & ~0x3;
> >> +       reg_val = readl(reg_base + reg);
> >> +
> >> +       if (size == 1)
> >> +               *val = *(u8 __force *) walker;
> >> +       else if (size == 2)
> >> +               *val = *(u16 __force *) walker;
> >
> > What is the __force for?
> 
> Hi Arnd, thanks for replying.
> 
> __force is used to, well, force a conversion, like casting from or to a bitwise type, else the Sparse checker will throw a warning.

I know what it's for in general, but in this case there is no __bitwise
or __iomem or any other annotation on either side of the assignment.

> >
> >> +       else if (size == 4)
> >> +               *val = reg_val;
> >> +       else
> >> +               return PCIBIOS_BAD_REGISTER_NUMBER;
> >> +
> >> +       return PCIBIOS_SUCCESSFUL;
> >
> > It looks like you are reimplementing pci_generic_config_read32/pci_generic_config_write32
> > read here, better use them directly.
> >
> 
> For our host bridge, access RC and EP config space are not the same way.
> Our host bridge is non ECAM only for the RC bus config space;
> for any other bus underneath the root bus we support ECAM access.
> 
> hisi_pcie_common_cfg_read is used to read RC config space, only supports 32-bit config access.
> hisi_pcie_common_cfg_read/hisi_pcie_common_cfg_write may change as below will be better.
> 
> /* HipXX PCIe host only supports 32-bit config access */
> int hisi_pcie_common_cfg_read(void __iomem *reg_base, int where, int size,
> 			      u32 *val)
> {
> 	void __iomem *addr;
> 
> 	addr = reg_base + (where & ~0x3);
> 	*val = readl(addr);
> 
> 	if (size <= 2)
> 		*val = (*val >> (8 * (where & 3))) & ((1 << (size * 8)) - 1);
> 
> 	return PCIBIOS_SUCCESSFUL;
> }

My point was: why not call pci_generic_config_read32() when accessing
the RC config space instead of duplicating the code from it?

	Arnd

Re: [RFC PATCH V2 1/3] PCI: hisi: re-architect Hip05/Hip06 controllers driver to preapare for ACPI

[Dongdong Liu]

Hi Arnd

在 2016/9/1 15:41, Arnd Bergmann 写道:
> On Thursday, September 1, 2016 10:05:29 AM CEST Dongdong Liu wrote:
>>
>> 在 2016/8/31 19:45, Arnd Bergmann 写道:
>>> On Wednesday, August 31, 2016 7:48:12 PM CEST Dongdong Liu wrote:
>>>> +
>>>> +/* HipXX PCIe host only supports 32-bit config access */
>>>> +int hisi_pcie_common_cfg_read(void __iomem *reg_base, int where, int size,
>>>> +                             u32 *val)
>>>> +{
>>>> +       u32 reg;
>>>> +       u32 reg_val;
>>>> +       void *walker = &reg_val;
>>>> +
>>>> +       walker += (where & 0x3);
>>>> +       reg = where & ~0x3;
>>>> +       reg_val = readl(reg_base + reg);
>>>> +
>>>> +       if (size == 1)
>>>> +               *val = *(u8 __force *) walker;
>>>> +       else if (size == 2)
>>>> +               *val = *(u16 __force *) walker;
>>>
>>> What is the __force for?
>>
>> Hi Arnd, thanks for replying.
>>
>> __force is used to, well, force a conversion, like casting from or to a bitwise type, else the Sparse checker will throw a warning.
>
> I know what it's for in general, but in this case there is no __bitwise
> or __iomem or any other annotation on either side of the assignment.

Thanks for you point that.

>
>>>
>>>> +       else if (size == 4)
>>>> +               *val = reg_val;
>>>> +       else
>>>> +               return PCIBIOS_BAD_REGISTER_NUMBER;
>>>> +
>>>> +       return PCIBIOS_SUCCESSFUL;
>>>
>>> It looks like you are reimplementing pci_generic_config_read32/pci_generic_config_write32
>>> read here, better use them directly.
>>>
>>
>> For our host bridge, access RC and EP config space are not the same way.
>> Our host bridge is non ECAM only for the RC bus config space;
>> for any other bus underneath the root bus we support ECAM access.
>>
>> hisi_pcie_common_cfg_read is used to read RC config space, only supports 32-bit config access.
>> hisi_pcie_common_cfg_read/hisi_pcie_common_cfg_write may change as below will be better.
>>
>> /* HipXX PCIe host only supports 32-bit config access */
>> int hisi_pcie_common_cfg_read(void __iomem *reg_base, int where, int size,
>> 			      u32 *val)
>> {
>> 	void __iomem *addr;
>>
>> 	addr = reg_base + (where & ~0x3);
>> 	*val = readl(addr);
>>
>> 	if (size <= 2)
>> 		*val = (*val >> (8 * (where & 3))) & ((1 << (size * 8)) - 1);
>>
>> 	return PCIBIOS_SUCCESSFUL;
>> }
>
> My point was: why not call pci_generic_config_read32() when accessing
> the RC config space instead of duplicating the code from it?

I know your point.

1. For our host bridge , ".map_bus = pci_ecam_map_bus" is only suitable for
accessing the EP config space.
pci_generic_config_read32() need to call "addr = bus->ops->map_bus(bus, devfn, where & ~0x3);",

drivers/pci/host/pcie-hisi-acpi.c
static struct pci_ops hisi_pcie_ops = {
	.map_bus = pci_ecam_map_bus,
	.read = hisi_pcie_acpi_rd_conf,
	.write = hisi_pcie_acpi_wr_conf,
};

Yes, we can change ".map_bus = pci_ecam_map_bus" to ".map_bus = hisi_pci_map_bus", and implentment hisi_pci_map_bus as below,
then we will not need to call hisi_pcie_common_cfg_read().

void __iomem *hisi_pci_map_bus(struct pci_bus *bus, unsigned int devfn, int where)
{
	struct pci_config_window *cfg = bus->sysdata;
	void __iomem *reg_base = cfg->priv;

	/* for RC config access*/
	if (bus->number == cfg->busr.start)
		return reg_base + (where & ~0x3);
	else
		/* for EP config access */
		return pci_ecam_map_bus(bus, devfn, where);
}

and hisi_pcie_acpi_rd_conf() need to change as below.
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;

	if (hisi_pcie_acpi_valid_config(cfg, bus, PCI_SLOT(devfn)) == 0)
		return PCIBIOS_DEVICE_NOT_FOUND;
	
	/* access RC config space */
	if (bus->number == cfg->busr.start)
		return pci_generic_config_read32(bus, devfn, where, size, val);
	
	/* access EP config space */
	return pci_generic_config_read(bus, devfn, where, size, val);
}


2. We need to backward compatible with the old dt way config access as below code,
so we have to call hisi_pcie_common_cfg_read() when accessing the RC config space.
For this, we have to call hisi_pcie_common_cfg_read().

drivers/pci/host/pcie-hisi.c
static inline int hisi_pcie_cfg_read(struct pcie_port *pp, int where,
		int size, u32 *val)
{
	struct hisi_pcie *pcie = to_hisi_pcie(pp);

	return hisi_pcie_common_cfg_read(pcie->reg_base, where, size, val);
}

static struct pcie_host_ops hisi_pcie_host_ops = {
	.rd_own_conf = hisi_pcie_cfg_read,
	.wr_own_conf = hisi_pcie_cfg_write,
	.link_up = hisi_pcie_link_up,
};

Thanks
Dongdong
>
> 	Arnd
>
> .
>

Re: [RFC PATCH V2 1/3] PCI: hisi: re-architect Hip05/Hip06 controllers driver to preapare for ACPI

[Arnd Bergmann]

On Thursday, September 1, 2016 8:44:49 PM CEST Dongdong Liu wrote:
> 在 2016/9/1 15:41, Arnd Bergmann 写道:
> > On Thursday, September 1, 2016 10:05:29 AM CEST Dongdong Liu wrote:
> >> 在 2016/8/31 19:45, Arnd Bergmann 写道:
> >>> On Wednesday, August 31, 2016 7:48:12 PM CEST Dongdong Liu wrote:
> I know your point.
> 
> 1. For our host bridge , ".map_bus = pci_ecam_map_bus" is only suitable for
> accessing the EP config space.
> pci_generic_config_read32() need to call "addr = bus->ops->map_bus(bus, devfn, where & ~0x3);",
> 
> drivers/pci/host/pcie-hisi-acpi.c
> static struct pci_ops hisi_pcie_ops = {
> 	.map_bus = pci_ecam_map_bus,
> 	.read = hisi_pcie_acpi_rd_conf,
> 	.write = hisi_pcie_acpi_wr_conf,
> };
> 
> Yes, we can change ".map_bus = pci_ecam_map_bus" to ".map_bus = hisi_pci_map_bus", and implentment hisi_pci_map_bus as below,
> then we will not need to call hisi_pcie_common_cfg_read().
> 
> void __iomem *hisi_pci_map_bus(struct pci_bus *bus, unsigned int devfn, int where)
> {
> 	struct pci_config_window *cfg = bus->sysdata;
> 	void __iomem *reg_base = cfg->priv;
> 
> 	/* for RC config access*/
> 	if (bus->number == cfg->busr.start)
> 		return reg_base + (where & ~0x3);
> 	else
> 		/* for EP config access */
> 		return pci_ecam_map_bus(bus, devfn, where);
> }
> 
> and hisi_pcie_acpi_rd_conf() need to change as below.
> 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;
> 
> 	if (hisi_pcie_acpi_valid_config(cfg, bus, PCI_SLOT(devfn)) == 0)
> 		return PCIBIOS_DEVICE_NOT_FOUND;
> 	
> 	/* access RC config space */
> 	if (bus->number == cfg->busr.start)
> 		return pci_generic_config_read32(bus, devfn, where, size, val);
> 	
> 	/* access EP config space */
> 	return pci_generic_config_read(bus, devfn, where, size, val);
> }

Right, this is what I had in mind.


> 2. We need to backward compatible with the old dt way config access as below code,
> so we have to call hisi_pcie_common_cfg_read() when accessing the RC config space.
> For this, we have to call hisi_pcie_common_cfg_read().
> 
> drivers/pci/host/pcie-hisi.c
> static inline int hisi_pcie_cfg_read(struct pcie_port *pp, int where,
> 		int size, u32 *val)
> {
> 	struct hisi_pcie *pcie = to_hisi_pcie(pp);
> 
> 	return hisi_pcie_common_cfg_read(pcie->reg_base, where, size, val);
> }
> 
> static struct pcie_host_ops hisi_pcie_host_ops = {
> 	.rd_own_conf = hisi_pcie_cfg_read,
> 	.wr_own_conf = hisi_pcie_cfg_write,
> 	.link_up = hisi_pcie_link_up,
> };

I think this would be easier if you separate the ACPI code from the
DT code and not try to have a common file used for both.

Sharing the config space accessors really isn't worth it when both
variants are fairly simple to do, but they don't fit in a common
model because one is called from the ACPI quirks and the other
is called from the dw-pcie driver with completely different calling
conventions.

	ARnd

Re: [RFC PATCH V2 3/3] PCI/ACPI: hisi: Add ACPI support for HiSilicon SoCs Host Controllers

[Rafael J. Wysocki]

On Thursday, September 01, 2016 11:23:42 AM Dongdong Liu wrote:
> 
> 在 2016/9/1 6:56, Rafael J. Wysocki 写道:
> > On Wednesday, August 31, 2016 07:48:14 PM Dongdong Liu wrote:
> >> Add specific quirks for PCI config space accessors.This involves:
> >> 1. New initialization call hisi_pcie_acpi_init() to get RC config resource
> >> with hardcoded range address and setup ecam mapping.
> >> 2. New entry in common quirk array.
> >>
> >> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
> >> Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
> >
> > Well, what exactly is the ACPI support you're adding?  Is it the ECAM part only
> > or is there anything more to it?
> >
> 
> Hi Rafael, thanks for replying.
> 
> Our host bridge is non ECAM only for the RC bus config space;
> for any other bus underneath the root bus we support ECAM access.
> 
> In our case we cannot use the standard MCFG object to pass the RC itself config space addresses.
> The more discuss information can be found:
> https://lkml.org/lkml/2016/2/22/1087
> [...]
> I have looked into this and in our case we cannot use the
> standard MCFG object to pass the RC config space addresses.
> 
> The reason is that in our HW we have the config base addresses of the
> root complex ports that are less than 0x100000 byte distant one from
> the other as we only map the first 0x10000 bytes.
> Now the MCFG acpi framework always fix the MCFG resource size to 0x100000
> for each bus; therefore if we pass our RC addresses through MCFG we end
> up with a resource conflict.
> To give you a practical example we are in a situation where we have:
> 
> port0: [0x00000000b0080000 - 0x00000000b0080000 + 0x10000]
> port1: [0x00000000b0090000 - 0x00000000b0090000 + 0x10000]
> port2: [0x00000000b00A0000 - 0x00000000b00A0000 + 0x10000]
> port3: [0x00000000b00B0000 - 0x00000000b00B0000 + 0x10000]
> So if we pass the base addresses through MCFG the resources
> will overlap as MCFG will consider 0x100000 size for each base
> address of the root complex (only the RC bus uses that address)
> So far I do not see many option other than using _DSD to pass
> these RC config base addresses.

It still is not entirely clear to me what the "ACPI support" is here.

Do you read any configuration information from the ACPI tables or similar?

If so, where is the format of it documented?

Thanks,
Rafael

Re: [RFC PATCH V2 1/3] PCI: hisi: re-architect Hip05/Hip06 controllers driver to preapare for ACPI

[Dongdong Liu]

Hi Arnd

在 2016/9/1 22:02, Arnd Bergmann 写道:
>
>> 2. We need to backward compatible with the old dt way config access as below code,
>> so we have to call hisi_pcie_common_cfg_read() when accessing the RC config space.
>> For this, we have to call hisi_pcie_common_cfg_read().
>>
>> drivers/pci/host/pcie-hisi.c
>> static inline int hisi_pcie_cfg_read(struct pcie_port *pp, int where,
>> 		int size, u32 *val)
>> {
>> 	struct hisi_pcie *pcie = to_hisi_pcie(pp);
>>
>> 	return hisi_pcie_common_cfg_read(pcie->reg_base, where, size, val);
>> }
>>
>> static struct pcie_host_ops hisi_pcie_host_ops = {
>> 	.rd_own_conf = hisi_pcie_cfg_read,
>> 	.wr_own_conf = hisi_pcie_cfg_write,
>> 	.link_up = hisi_pcie_link_up,
>> };
>
> I think this would be easier if you separate the ACPI code from the
> DT code and not try to have a common file used for both.
>
> Sharing the config space accessors really isn't worth it when both
> variants are fairly simple to do, but they don't fit in a common
> model because one is called from the ACPI quirks and the other
> is called from the dw-pcie driver with completely different calling
> conventions.

I agree, many thanks.

Thanks
Dongdong
>
> 	ARnd
> --
> 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: [RFC PATCH V2 3/3] PCI/ACPI: hisi: Add ACPI support for HiSilicon SoCs Host Controllers

[Dongdong Liu]

Hi Rafael

在 2016/9/2 7:38, Rafael J. Wysocki 写道:
> On Thursday, September 01, 2016 11:23:42 AM Dongdong Liu wrote:
>>
>> 在 2016/9/1 6:56, Rafael J. Wysocki 写道:
>>> On Wednesday, August 31, 2016 07:48:14 PM Dongdong Liu wrote:
>>>> Add specific quirks for PCI config space accessors.This involves:
>>>> 1. New initialization call hisi_pcie_acpi_init() to get RC config resource
>>>> with hardcoded range address and setup ecam mapping.
>>>> 2. New entry in common quirk array.
>>>>
>>>> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
>>>> Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
>>>
>>> Well, what exactly is the ACPI support you're adding?  Is it the ECAM part only
>>> or is there anything more to it?
>>>
>>
>> Hi Rafael, thanks for replying.
>>
>> Our host bridge is non ECAM only for the RC bus config space;
>> for any other bus underneath the root bus we support ECAM access.
>>
>> In our case we cannot use the standard MCFG object to pass the RC itself config space addresses.
>> The more discuss information can be found:
>> https://lkml.org/lkml/2016/2/22/1087
>> [...]
>> I have looked into this and in our case we cannot use the
>> standard MCFG object to pass the RC config space addresses.
>>
>> The reason is that in our HW we have the config base addresses of the
>> root complex ports that are less than 0x100000 byte distant one from
>> the other as we only map the first 0x10000 bytes.
>> Now the MCFG acpi framework always fix the MCFG resource size to 0x100000
>> for each bus; therefore if we pass our RC addresses through MCFG we end
>> up with a resource conflict.
>> To give you a practical example we are in a situation where we have:
>>
>> port0: [0x00000000b0080000 - 0x00000000b0080000 + 0x10000]
>> port1: [0x00000000b0090000 - 0x00000000b0090000 + 0x10000]
>> port2: [0x00000000b00A0000 - 0x00000000b00A0000 + 0x10000]
>> port3: [0x00000000b00B0000 - 0x00000000b00B0000 + 0x10000]
>> So if we pass the base addresses through MCFG the resources
>> will overlap as MCFG will consider 0x100000 size for each base
>> address of the root complex (only the RC bus uses that address)
>> So far I do not see many option other than using _DSD to pass
>> these RC config base addresses.
>
> It still is not entirely clear to me what the "ACPI support" is here.
>
> Do you read any configuration information from the ACPI tables or similar?
>
> If so, where is the format of it documented?
>

Since Our host bridge is non ECAM only for the RC bus config space,for any other bus underneath the root bus we support ECAM access,
we need to override these accessors prior to PCI buses enumeration.

As below is MCFG table configuration.
0x22004000000~0x220040fffff (bus 0x40) addresses are wasted, because Our host bridge is non ECAM only for the RC.
We use RC base address(0xb0080000)+offset to access our RC config space.
0x22004100000~0x22007ffffff addresses are used to aceess EP config space (bus 0x41~0x7f).This support ECAM access.

MCFG Table
...
{
       0x22000000000,                                      //Base Address
       0x0001,                                             //Segment Group Number
       0x40,                                               //Start Bus Number
       0x7f,                                               //End Bus Number
       0x00000000,                                         //Reserved
},
...

DSDT Table
//PCIe Root bus
Device (PCI1)
{
	Name (_HID, "PNP0A08") // PCI Express Root Bridge
	Name (_CID, "PNP0A03") // Compatible PCI Root Bridge
	Name(_SEG, 1) // Segment of this Root complex
	Name(_BBN, 0x40) // Base Bus Number
	Name(_CCA, 1)
	Method (_CRS, 0, Serialized) { // Root complex resources
		Name (RBUF, ResourceTemplate () {
			WordBusNumber ( // Bus numbers assigned to this root
				ResourceProducer, MinFixed, MaxFixed, PosDecode,
				0,    // AddressGranularity
				0x40, // AddressMinimum - Minimum Bus Number
				0x7f, // AddressMaximum - Maximum Bus Number
				0,    // AddressTranslation - Set to 0
				0x40  // RangeLength - Number of Busses
			)
			...
			...
		}) // Name(RBUF)
       Return (RBUF)
     } // Method(_CRS)
} // Device(PCI1)


As below hisi_pcie_acpi_rd_conf() is our config read implementation.
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;
	void __iomem *reg_base = cfg->priv;

	if (hisi_pcie_acpi_valid_config(cfg, bus, PCI_SLOT(devfn)) == 0)
		return PCIBIOS_DEVICE_NOT_FOUND;
	/* Access RC config space */
	if (bus->number == cfg->busr.start)
		return hisi_pcie_common_cfg_read(reg_base, where, size, val);

	/* Access EP config space */
	return pci_generic_config_read(bus, devfn, where, size, val);
}

Thanks
Dongdong
> Thanks,
> Rafael
>
>
> .
>

RE: [RFC PATCH V2 1/3] PCI: hisi: re-architect Hip05/Hip06 controllers driver to preapare for ACPI

[Gabriele Paoloni]

Hi Arnd, thanks for your comments

> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@arndb.de]
> Sent: 01 September 2016 15:02
> To: liudongdong (C)
> Cc: helgaas@kernel.org; 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
> Subject: Re: [RFC PATCH V2 1/3] PCI: hisi: re-architect Hip05/Hip06
> controllers driver to preapare for ACPI
> 
> On Thursday, September 1, 2016 8:44:49 PM CEST Dongdong Liu wrote:
> > 在 2016/9/1 15:41, Arnd Bergmann 写道:
> > > On Thursday, September 1, 2016 10:05:29 AM CEST Dongdong Liu wrote:
> > >> 在 2016/8/31 19:45, Arnd Bergmann 写道:
> > >>> On Wednesday, August 31, 2016 7:48:12 PM CEST Dongdong Liu wrote:
> > I know your point.
> >
> > 1. For our host bridge , ".map_bus = pci_ecam_map_bus" is only
> suitable for
> > accessing the EP config space.
> > pci_generic_config_read32() need to call "addr = bus->ops-
> >map_bus(bus, devfn, where & ~0x3);",
> >
> > drivers/pci/host/pcie-hisi-acpi.c
> > static struct pci_ops hisi_pcie_ops = {
> > 	.map_bus = pci_ecam_map_bus,
> > 	.read = hisi_pcie_acpi_rd_conf,
> > 	.write = hisi_pcie_acpi_wr_conf,
> > };
> >
> > Yes, we can change ".map_bus = pci_ecam_map_bus" to ".map_bus =
> hisi_pci_map_bus", and implentment hisi_pci_map_bus as below,
> > then we will not need to call hisi_pcie_common_cfg_read().
> >
> > void __iomem *hisi_pci_map_bus(struct pci_bus *bus, unsigned int
> devfn, int where)
> > {
> > 	struct pci_config_window *cfg = bus->sysdata;
> > 	void __iomem *reg_base = cfg->priv;
> >
> > 	/* for RC config access*/
> > 	if (bus->number == cfg->busr.start)
> > 		return reg_base + (where & ~0x3);
> > 	else
> > 		/* for EP config access */
> > 		return pci_ecam_map_bus(bus, devfn, where);
> > }
> >
> > and hisi_pcie_acpi_rd_conf() need to change as below.
> > 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;
> >
> > 	if (hisi_pcie_acpi_valid_config(cfg, bus, PCI_SLOT(devfn)) == 0)
> > 		return PCIBIOS_DEVICE_NOT_FOUND;
> >
> > 	/* access RC config space */
> > 	if (bus->number == cfg->busr.start)
> > 		return pci_generic_config_read32(bus, devfn, where, size,
> val);
> >
> > 	/* access EP config space */
> > 	return pci_generic_config_read(bus, devfn, where, size, val);
> > }
> 
> Right, this is what I had in mind.
> 
> 
> > 2. We need to backward compatible with the old dt way config access
> as below code,
> > so we have to call hisi_pcie_common_cfg_read() when accessing the RC
> config space.
> > For this, we have to call hisi_pcie_common_cfg_read().
> >
> > drivers/pci/host/pcie-hisi.c
> > static inline int hisi_pcie_cfg_read(struct pcie_port *pp, int where,
> > 		int size, u32 *val)
> > {
> > 	struct hisi_pcie *pcie = to_hisi_pcie(pp);
> >
> > 	return hisi_pcie_common_cfg_read(pcie->reg_base, where, size,
> val);
> > }
> >
> > static struct pcie_host_ops hisi_pcie_host_ops = {
> > 	.rd_own_conf = hisi_pcie_cfg_read,
> > 	.wr_own_conf = hisi_pcie_cfg_write,
> > 	.link_up = hisi_pcie_link_up,
> > };
> 
> I think this would be easier if you separate the ACPI code from the
> DT code and not try to have a common file used for both.
> 
> Sharing the config space accessors really isn't worth it when both
> variants are fairly simple to do, but they don't fit in a common
> model because one is called from the ACPI quirks and the other
> is called from the dw-pcie driver with completely different calling
> conventions.

Not sure about this...
>From my perspective having the shared code would make clear that
the two drivers (ACPI and DT) are kind of related...

For example see the reply from Bjorn to the xgene driver:
https://lkml.org/lkml/2016/9/19/749

I know in our case the duplication isn't much but as I said
I am a bit reluctant to rework this... 

Thx

Gab

> 
> 	ARnd

Re: [RFC PATCH V2 1/3] PCI: hisi: re-architect Hip05/Hip06 controllers driver to preapare for ACPI

[Arnd Bergmann]

On Tuesday, September 20, 2016 9:45:13 AM CEST Gabriele Paoloni wrote:
> > 
> > I think this would be easier if you separate the ACPI code from the
> > DT code and not try to have a common file used for both.
> > 
> > Sharing the config space accessors really isn't worth it when both
> > variants are fairly simple to do, but they don't fit in a common
> > model because one is called from the ACPI quirks and the other
> > is called from the dw-pcie driver with completely different calling
> > conventions.
> 
> Not sure about this...
> From my perspective having the shared code would make clear that
> the two drivers (ACPI and DT) are kind of related...
> 
> For example see the reply from Bjorn to the xgene driver:
> https://lkml.org/lkml/2016/9/19/749
> 
> I know in our case the duplication isn't much but as I said
> I am a bit reluctant to rework this... 

It's clearly a question of perspective: The way I see it, this
is not a driver at all, but a quirk for the acpi-pci driver,
and having hardware specific code shared between the two
complicates things.

In both cases (xgene and hisi), only a very small portion of
PCI host driver is needed as for the ACPI quirk, while all
of the hardware specific setup can be left to the firmware
here.

	Arnd

[RFC PATCH v2 3/3] PCI/ACPI: hisi: Add ACPI support for HiSilicon SoCs Host Controllers

[Gabriele Paoloni]

From: gabriele paoloni <gabriele.paoloni@huawei.com>

This patch adds specific quirks for PCI config space accessors,
it uses _HID to decide whether to hook pci_ops or not.

Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
---
 MAINTAINERS                       |   1 +
 drivers/pci/host/Kconfig          |   8 ++
 drivers/pci/host/Makefile         |   1 +
 drivers/pci/host/pcie-hisi-acpi.c | 188 ++++++++++++++++++++++++++++++++++++++
 drivers/pci/host/pcie-hisi.c      |   2 -
 drivers/pci/host/pcie-hisi.h      |   2 +
 6 files changed, 200 insertions(+), 2 deletions(-)
 create mode 100644 drivers/pci/host/pcie-hisi-acpi.c

diff --git a/MAINTAINERS b/MAINTAINERS
index d69f436..f184c3e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8412,6 +8412,7 @@ F:	Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
 F:	drivers/pci/host/pcie-hisi.h
 F:	drivers/pci/host/pcie-hisi.c
 F:	drivers/pci/host/pcie-hisi-common.c
+F:	drivers/pci/host/pcie-hisi-acpi.c
 
 PCIE DRIVER FOR QUALCOMM MSM
 M:     Stanimir Varbanov <svarbanov@mm-sol.com>
diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
index 75a6054..65b1add 100644
--- a/drivers/pci/host/Kconfig
+++ b/drivers/pci/host/Kconfig
@@ -181,6 +181,14 @@ config PCI_HISI
 	  Say Y here if you want PCIe controller support on HiSilicon
 	  Hip05 and Hip06 SoCs
 
+config PCI_HISI_ACPI
+	depends on ACPI
+	bool "HiSilicon Hip05 and Hip06 SoCs ACPI PCIe controllers"
+	select ACPI_PCI_HOST_GENERIC
+	help
+	  Say Y here if you want ACPI PCIe controller support on HiSilicon
+	  Hip05 and Hip06 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 8c93c0f..57e4379 100644
--- a/drivers/pci/host/Makefile
+++ b/drivers/pci/host/Makefile
@@ -21,4 +21,5 @@ 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 pcie-hisi-common.o
+obj-$(CONFIG_PCI_HISI_ACPI) += pcie-hisi-acpi.o pcie-hisi-common.o
 obj-$(CONFIG_PCIE_QCOM) += pcie-qcom.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..3605260
--- /dev/null
+++ b/drivers/pci/host/pcie-hisi-acpi.c
@@ -0,0 +1,188 @@
+/*
+ * 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/interrupt.h>
+#include <linux/acpi.h>
+#include <linux/ecam.h>
+#include <linux/pci.h>
+#include <linux/pci-acpi.h>
+#include "pcie-hisi.h"
+
+#define GET_PCIE_LINK_STATUS  0x0
+
+/* uuid 6d30f553-836c-408e-b6ad-45bccc957949 */
+const u8 hisi_pcie_acpi_dsm_uuid[] = {
+	0x53, 0xf5, 0x30, 0x6d, 0x6c, 0x83, 0x8e, 0x40,
+	0xb6, 0xad, 0x45, 0xbc, 0xcc, 0x95, 0x79, 0x49
+};
+
+static const struct acpi_device_id hisi_pcie_ids[] = {
+	{"HISI0080", 0},
+	{"", 0},
+};
+
+static int hisi_pcie_get_addr(struct acpi_pci_root *root, const char *name,
+				void __iomem **addr)
+{
+	struct acpi_device *device;
+	u64 base;
+	u64 size;
+	u32 buf[4];
+	int ret;
+
+	device =  root->device;
+	ret = fwnode_property_read_u32_array(&device->fwnode, name,
+					buf, ARRAY_SIZE(buf));
+	if (ret) {
+		dev_err(&device->dev, "can't get %s\n", name);
+		return ret;
+	}
+
+	base = ((u64)buf[0] << 32) | buf[1];
+	size =  ((u64)buf[2] << 32) | buf[3];
+	*addr = devm_ioremap(&device->dev, base, size);
+	if (!(*addr)) {
+		dev_err(&device->dev, "error with ioremap\n");
+		return -ENOMEM;
+	}
+
+	return 0;
+}
+
+
+static int hisi_pcie_link_up_acpi(struct acpi_pci_root *root)
+{
+	u32 val;
+	struct acpi_device *device;
+	union acpi_object *obj;
+
+	device = root->device;
+	obj = acpi_evaluate_dsm(device->handle,
+		hisi_pcie_acpi_dsm_uuid, 0,
+		GET_PCIE_LINK_STATUS, NULL);
+
+	if (!obj  ||  obj->type != ACPI_TYPE_INTEGER)  {
+		dev_err(&device->dev, "can't get link status from _DSM\n");
+		return 0;
+	}
+	val = obj->integer.value;
+
+	return ((val & PCIE_LTSSM_STATE_MASK) == PCIE_LTSSM_LINKUP_STATE);
+
+}
+
+/*
+ * Retrieve rc_dbi base and size from _DSD
+ * Name (_DSD, Package () {
+ *	ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
+ *	Package () {
+ *	Package () {"rc-dbi", Package () { 0x0, 0xb0080000, 0x0, 0x10000 }},
+ *	}
+ *	})
+ */
+static int hisi_pcie_init(struct acpi_pci_root *root)
+{
+	int ret;
+	struct acpi_device *device;
+	void __iomem *reg_base;
+
+	device =  root->device;
+	ret = hisi_pcie_get_addr(root, "rc-dbi", &reg_base);
+	if (ret) {
+		dev_err(&device->dev, "can't get rc-dbi\n");
+		return ret;
+	}
+
+	root->sysdata = reg_base;
+	return 0;
+}
+
+static int hisi_pcie_match(struct pci_mcfg_fixup *fixup,
+			struct acpi_pci_root *root)
+{
+	int ret;
+	struct acpi_device *device;
+
+	device = root->device;
+	ret = acpi_match_device_ids(device, hisi_pcie_ids);
+	if (ret)
+		return 0;
+
+	ret = hisi_pcie_init(root);
+	if (ret)
+		dev_warn(&device->dev, "hisi pcie init fail\n");
+
+	return 1;
+}
+
+static int hisi_pcie_acpi_valid_config(struct acpi_pci_root *root,
+				struct pci_bus *bus, int dev)
+{
+	/* If there is no link, then there is no device */
+	if (bus->number != root->secondary.start) {
+		if (!hisi_pcie_link_up_acpi(root))
+			return 0;
+	}
+
+	/* access only one slot on each root port */
+	if (bus->number == root->secondary.start && dev > 0)
+		return 0;
+
+	/*
+	 * do not read more than one device on the bus directly attached
+	 * to RC's (Virtual Bridge's) DS side.
+	 */
+	if (bus->primary == root->secondary.start && dev > 0)
+		return 0;
+
+	return 1;
+}
+
+static int hisi_pcie_acpi_rd_conf(struct pci_bus *bus, u32 devfn, int where,
+		int size, u32 *val)
+{
+	struct acpi_pci_root *root = bus->sysdata;
+	void __iomem *reg_base = root->sysdata;
+
+	if (hisi_pcie_acpi_valid_config(root, bus, PCI_SLOT(devfn)) == 0)
+		return PCIBIOS_DEVICE_NOT_FOUND;
+
+	if (bus->number == root->secondary.start)
+		return hisi_pcie_common_cfg_read(reg_base, 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 acpi_pci_root *root = bus->sysdata;
+	void __iomem *reg_base = root->sysdata;
+
+	if (hisi_pcie_acpi_valid_config(root, bus, PCI_SLOT(devfn)) == 0)
+		return PCIBIOS_DEVICE_NOT_FOUND;
+
+	if (bus->number == root->secondary.start)
+		return hisi_pcie_common_cfg_write(reg_base, where, size, val);
+
+	return pci_generic_config_write(bus, devfn, where, size, val);
+}
+
+struct pci_ops hisi_pcie_acpi_ops = {
+	.map_bus = pci_mcfg_dev_base,
+	.read = hisi_pcie_acpi_rd_conf,
+	.write = hisi_pcie_acpi_wr_conf,
+};
+
+
+DECLARE_ACPI_MCFG_FIXUP(NULL, hisi_pcie_match, &hisi_pcie_acpi_ops,
+	PCI_MCFG_DOMAIN_ANY, PCI_MCFG_BUS_ANY);
diff --git a/drivers/pci/host/pcie-hisi.c b/drivers/pci/host/pcie-hisi.c
index eb3201a..ed0b0b2 100644
--- a/drivers/pci/host/pcie-hisi.c
+++ b/drivers/pci/host/pcie-hisi.c
@@ -23,8 +23,6 @@
 #include "pcie-designware.h"
 #include "pcie-hisi.h"
 
-#define PCIE_LTSSM_LINKUP_STATE				0x11
-#define PCIE_LTSSM_STATE_MASK				0x3F
 #define PCIE_SUBCTRL_SYS_STATE4_REG			0x6818
 #define PCIE_SYS_STATE4						0x31c
 #define PCIE_HIP06_CTRL_OFF					0x1000
diff --git a/drivers/pci/host/pcie-hisi.h b/drivers/pci/host/pcie-hisi.h
index 29e0790..45cf0fd 100644
--- a/drivers/pci/host/pcie-hisi.h
+++ b/drivers/pci/host/pcie-hisi.h
@@ -14,6 +14,8 @@
 #ifndef PCIE_HISI_H_
 #define PCIE_HISI_H_
 
+#define PCIE_LTSSM_LINKUP_STATE				0x11
+#define PCIE_LTSSM_STATE_MASK				0x3F
 
 int hisi_pcie_common_cfg_read(void __iomem *reg_base, int where, int size,
 			      u32 *val);
-- 
1.9.1