* [RFC PATCH v2 0/3] Add ACPI support for HiSilicon PCIe Host Controllers
@ 2016-02-08 12:41 ` Gabriele Paoloni
0 siblings, 0 replies; 63+ messages in thread
From: Gabriele Paoloni @ 2016-02-08 12:41 UTC (permalink / raw)
To: linux-arm-kernel
From: gabriele paoloni <gabriele.paoloni@huawei.com>
This patchset adds ACPI support for the HiSilicon Hip05/Hip06 SoC PCIe controllers
The four patches respectively:
- re-architect the current HiSilicon driver to make it scalable to the
new ACPI quirks
- rework the current HiSilicon driver to make it ECAM compliant
- adds the HiSilicon ACPI specific quirks.
This patchset is based on
https://github.com/semihalf-nowicki-tomasz/linux.git
branch pci-acpi-v4
Changes v1 -> v2: removed the ACPI quirks dependency on Designware framework
gabriele paoloni (3):
PCI: hisi: re-architect Hip05/Hip06 controllers driver to preapare for
ACPI
PCI: hisi: Make the HiSilicon PCIe host controller ECAM compliant
PCI/ACPI: hisi: Add ACPI support for HiSilicon SoCs Host Controllers
MAINTAINERS | 3 +
drivers/pci/host/Kconfig | 8 ++
drivers/pci/host/Makefile | 3 +-
drivers/pci/host/pcie-designware.c | 4 +-
drivers/pci/host/pcie-designware.h | 2 +
drivers/pci/host/pcie-hisi-acpi.c | 188 ++++++++++++++++++++++++++++++++++++
drivers/pci/host/pcie-hisi-common.c | 73 ++++++++++++++
drivers/pci/host/pcie-hisi.c | 144 +++++++++++++--------------
drivers/pci/host/pcie-hisi.h | 25 +++++
9 files changed, 367 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
^ permalink raw reply [flat|nested] 63+ messages in thread* [RFC PATCH v2 0/3] Add ACPI support for HiSilicon PCIe Host Controllers
@ 2016-02-08 12:41 ` Gabriele Paoloni
0 siblings, 0 replies; 63+ messages in thread
From: Gabriele Paoloni @ 2016-02-08 12:41 UTC (permalink / raw)
To: guohanjun, wangzhou1, liudongdong3, linuxarm, qiujiang, bhelgaas,
arnd, Lorenzo.Pieralisi, tn
Cc: gabriele.paoloni, zhangjukuo, xuwei5, liguozhu, linux-pci,
linux-arm-kernel, linux-acpi, linux-kernel, jcm
From: gabriele paoloni <gabriele.paoloni@huawei.com>
This patchset adds ACPI support for the HiSilicon Hip05/Hip06 SoC PCIe controllers
The four patches respectively:
- re-architect the current HiSilicon driver to make it scalable to the
new ACPI quirks
- rework the current HiSilicon driver to make it ECAM compliant
- adds the HiSilicon ACPI specific quirks.
This patchset is based on
https://github.com/semihalf-nowicki-tomasz/linux.git
branch pci-acpi-v4
Changes v1 -> v2: removed the ACPI quirks dependency on Designware framework
gabriele paoloni (3):
PCI: hisi: re-architect Hip05/Hip06 controllers driver to preapare for
ACPI
PCI: hisi: Make the HiSilicon PCIe host controller ECAM compliant
PCI/ACPI: hisi: Add ACPI support for HiSilicon SoCs Host Controllers
MAINTAINERS | 3 +
drivers/pci/host/Kconfig | 8 ++
drivers/pci/host/Makefile | 3 +-
drivers/pci/host/pcie-designware.c | 4 +-
drivers/pci/host/pcie-designware.h | 2 +
drivers/pci/host/pcie-hisi-acpi.c | 188 ++++++++++++++++++++++++++++++++++++
drivers/pci/host/pcie-hisi-common.c | 73 ++++++++++++++
drivers/pci/host/pcie-hisi.c | 144 +++++++++++++--------------
drivers/pci/host/pcie-hisi.h | 25 +++++
9 files changed, 367 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
^ permalink raw reply [flat|nested] 63+ messages in thread* [RFC PATCH v2 1/3] PCI: hisi: re-architect Hip05/Hip06 controllers driver to preapare for ACPI
2016-02-08 12:41 ` Gabriele Paoloni
(?)
@ 2016-02-08 12:41 ` Gabriele Paoloni
-1 siblings, 0 replies; 63+ messages in thread
From: Gabriele Paoloni @ 2016-02-08 12:41 UTC (permalink / raw)
To: guohanjun, wangzhou1, liudongdong3, linuxarm, qiujiang, bhelgaas,
arnd, Lorenzo.Pieralisi, tn
Cc: gabriele.paoloni, zhangjukuo, xuwei5, liguozhu, linux-pci,
linux-arm-kernel, linux-acpi, linux-kernel, jcm
From: gabriele paoloni <gabriele.paoloni@huawei.com>
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>
---
MAINTAINERS | 2 +
drivers/pci/host/Makefile | 2 +-
drivers/pci/host/pcie-hisi-common.c | 73 ++++++++++++++++++++++
drivers/pci/host/pcie-hisi.c | 119 +++++++++++-------------------------
drivers/pci/host/pcie-hisi.h | 23 +++++++
5 files changed, 135 insertions(+), 84 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 30aca4a..d69f436 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8409,7 +8409,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 7b2f20c..8c93c0f 100644
--- a/drivers/pci/host/Makefile
+++ b/drivers/pci/host/Makefile
@@ -20,5 +20,5 @@ 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
diff --git a/drivers/pci/host/pcie-hisi-common.c b/drivers/pci/host/pcie-hisi-common.c
new file mode 100644
index 0000000..6dfb4c3
--- /dev/null
+++ b/drivers/pci/host/pcie-hisi-common.c
@@ -0,0 +1,73 @@
+/*
+ * 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/acpi.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/mfd/syscon.h>
+#include <linux/of_pci.h>
+#include <linux/regmap.h>
+
+#include "pcie-designware.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 = ®_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 = ®_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(val, reg_base + reg);
+ } else if (size == 1) {
+ reg_val = readl(reg_base + reg);
+ *(u8 __force *) walker = val;
+ writel(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 3e98d4e..458d0f8 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);
-}
-
-/* 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)
-{
- u32 reg;
- u32 reg_val;
- struct hisi_pcie *pcie = to_hisi_pcie(pp);
- void *walker = ®_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;
-}
+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_write(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_val;
- u32 reg;
struct hisi_pcie *pcie = to_hisi_pcie(pp);
- void *walker = ®_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;
-}
-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);
+ return hisi_pcie_common_cfg_read(pcie->reg_base, where, size, val);
}
-static int hisi_pcie_link_up_hip06(struct hisi_pcie *hisi_pcie)
+static inline int hisi_pcie_cfg_write(struct pcie_port *pp, int where,
+ int size, u32 val)
{
- 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)
@@ -134,12 +66,13 @@ static int hisi_pcie_link_up(struct pcie_port *pp)
return hisi_pcie->soc_ops->hisi_pcie_link_up(hisi_pcie);
}
-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,
+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,
};
+
static int hisi_add_pcie_port(struct pcie_port *pp,
struct platform_device *pdev)
{
@@ -215,6 +148,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..29e0790
--- /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
^ permalink raw reply related [flat|nested] 63+ messages in thread* [RFC PATCH v2 1/3] PCI: hisi: re-architect Hip05/Hip06 controllers driver to preapare for ACPI
@ 2016-02-08 12:41 ` Gabriele Paoloni
0 siblings, 0 replies; 63+ messages in thread
From: Gabriele Paoloni @ 2016-02-08 12:41 UTC (permalink / raw)
To: linux-arm-kernel
From: gabriele paoloni <gabriele.paoloni@huawei.com>
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>
---
MAINTAINERS | 2 +
drivers/pci/host/Makefile | 2 +-
drivers/pci/host/pcie-hisi-common.c | 73 ++++++++++++++++++++++
drivers/pci/host/pcie-hisi.c | 119 +++++++++++-------------------------
drivers/pci/host/pcie-hisi.h | 23 +++++++
5 files changed, 135 insertions(+), 84 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 30aca4a..d69f436 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8409,7 +8409,9 @@ M: Gabriele Paoloni <gabriele.paoloni@huawei.com>
L: linux-pci at 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 7b2f20c..8c93c0f 100644
--- a/drivers/pci/host/Makefile
+++ b/drivers/pci/host/Makefile
@@ -20,5 +20,5 @@ 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
diff --git a/drivers/pci/host/pcie-hisi-common.c b/drivers/pci/host/pcie-hisi-common.c
new file mode 100644
index 0000000..6dfb4c3
--- /dev/null
+++ b/drivers/pci/host/pcie-hisi-common.c
@@ -0,0 +1,73 @@
+/*
+ * 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/acpi.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/mfd/syscon.h>
+#include <linux/of_pci.h>
+#include <linux/regmap.h>
+
+#include "pcie-designware.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 = ®_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 = ®_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(val, reg_base + reg);
+ } else if (size == 1) {
+ reg_val = readl(reg_base + reg);
+ *(u8 __force *) walker = val;
+ writel(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 3e98d4e..458d0f8 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);
-}
-
-/* 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)
-{
- u32 reg;
- u32 reg_val;
- struct hisi_pcie *pcie = to_hisi_pcie(pp);
- void *walker = ®_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;
-}
+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_write(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_val;
- u32 reg;
struct hisi_pcie *pcie = to_hisi_pcie(pp);
- void *walker = ®_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;
-}
-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);
+ return hisi_pcie_common_cfg_read(pcie->reg_base, where, size, val);
}
-static int hisi_pcie_link_up_hip06(struct hisi_pcie *hisi_pcie)
+static inline int hisi_pcie_cfg_write(struct pcie_port *pp, int where,
+ int size, u32 val)
{
- 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)
@@ -134,12 +66,13 @@ static int hisi_pcie_link_up(struct pcie_port *pp)
return hisi_pcie->soc_ops->hisi_pcie_link_up(hisi_pcie);
}
-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,
+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,
};
+
static int hisi_add_pcie_port(struct pcie_port *pp,
struct platform_device *pdev)
{
@@ -215,6 +148,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..29e0790
--- /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
^ permalink raw reply related [flat|nested] 63+ messages in thread* [RFC PATCH v2 1/3] PCI: hisi: re-architect Hip05/Hip06 controllers driver to preapare for ACPI
@ 2016-02-08 12:41 ` Gabriele Paoloni
0 siblings, 0 replies; 63+ messages in thread
From: Gabriele Paoloni @ 2016-02-08 12:41 UTC (permalink / raw)
To: guohanjun, wangzhou1, liudongdong3, linuxarm, qiujiang, bhelgaas,
arnd, Lorenzo.Pieralisi, tn
Cc: gabriele.paoloni, zhangjukuo, xuwei5, liguozhu, linux-pci,
linux-arm-kernel, linux-acpi, linux-kernel, jcm
From: gabriele paoloni <gabriele.paoloni@huawei.com>
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>
---
MAINTAINERS | 2 +
drivers/pci/host/Makefile | 2 +-
drivers/pci/host/pcie-hisi-common.c | 73 ++++++++++++++++++++++
drivers/pci/host/pcie-hisi.c | 119 +++++++++++-------------------------
drivers/pci/host/pcie-hisi.h | 23 +++++++
5 files changed, 135 insertions(+), 84 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 30aca4a..d69f436 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8409,7 +8409,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 7b2f20c..8c93c0f 100644
--- a/drivers/pci/host/Makefile
+++ b/drivers/pci/host/Makefile
@@ -20,5 +20,5 @@ 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
diff --git a/drivers/pci/host/pcie-hisi-common.c b/drivers/pci/host/pcie-hisi-common.c
new file mode 100644
index 0000000..6dfb4c3
--- /dev/null
+++ b/drivers/pci/host/pcie-hisi-common.c
@@ -0,0 +1,73 @@
+/*
+ * 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/acpi.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/mfd/syscon.h>
+#include <linux/of_pci.h>
+#include <linux/regmap.h>
+
+#include "pcie-designware.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 = ®_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 = ®_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(val, reg_base + reg);
+ } else if (size == 1) {
+ reg_val = readl(reg_base + reg);
+ *(u8 __force *) walker = val;
+ writel(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 3e98d4e..458d0f8 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);
-}
-
-/* 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)
-{
- u32 reg;
- u32 reg_val;
- struct hisi_pcie *pcie = to_hisi_pcie(pp);
- void *walker = ®_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;
-}
+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_write(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_val;
- u32 reg;
struct hisi_pcie *pcie = to_hisi_pcie(pp);
- void *walker = ®_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;
-}
-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);
+ return hisi_pcie_common_cfg_read(pcie->reg_base, where, size, val);
}
-static int hisi_pcie_link_up_hip06(struct hisi_pcie *hisi_pcie)
+static inline int hisi_pcie_cfg_write(struct pcie_port *pp, int where,
+ int size, u32 val)
{
- 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)
@@ -134,12 +66,13 @@ static int hisi_pcie_link_up(struct pcie_port *pp)
return hisi_pcie->soc_ops->hisi_pcie_link_up(hisi_pcie);
}
-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,
+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,
};
+
static int hisi_add_pcie_port(struct pcie_port *pp,
struct platform_device *pdev)
{
@@ -215,6 +148,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..29e0790
--- /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
^ permalink raw reply related [flat|nested] 63+ messages in thread* Re: [RFC PATCH v2 1/3] PCI: hisi: re-architect Hip05/Hip06 controllers driver to preapare for ACPI
2016-02-08 12:41 ` Gabriele Paoloni
@ 2016-02-08 13:50 ` Arnd Bergmann
-1 siblings, 0 replies; 63+ messages in thread
From: Arnd Bergmann @ 2016-02-08 13:50 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Gabriele Paoloni, guohanjun, wangzhou1, liudongdong3, linuxarm,
qiujiang, bhelgaas, Lorenzo.Pieralisi, tn, linux-pci,
linux-kernel, xuwei5, linux-acpi, jcm, zhangjukuo, liguozhu
On Monday 08 February 2016 12:41:02 Gabriele Paoloni 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 = ®_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;
> +}
Isn't this the same hack that Qualcomm are using?
Arnd
^ permalink raw reply [flat|nested] 63+ messages in thread* [RFC PATCH v2 1/3] PCI: hisi: re-architect Hip05/Hip06 controllers driver to preapare for ACPI
@ 2016-02-08 13:50 ` Arnd Bergmann
0 siblings, 0 replies; 63+ messages in thread
From: Arnd Bergmann @ 2016-02-08 13:50 UTC (permalink / raw)
To: linux-arm-kernel
On Monday 08 February 2016 12:41:02 Gabriele Paoloni 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 = ®_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;
> +}
Isn't this the same hack that Qualcomm are using?
Arnd
^ permalink raw reply [flat|nested] 63+ messages in thread* RE: [RFC PATCH v2 1/3] PCI: hisi: re-architect Hip05/Hip06 controllers driver to preapare for ACPI
2016-02-08 13:50 ` Arnd Bergmann
(?)
@ 2016-02-08 16:06 ` Gabriele Paoloni
-1 siblings, 0 replies; 63+ messages in thread
From: Gabriele Paoloni @ 2016-02-08 16:06 UTC (permalink / raw)
To: Arnd Bergmann, linux-arm-kernel
Cc: Guohanjun (Hanjun Guo), Wangzhou (B), liudongdong (C),
Linuxarm, qiujiang, bhelgaas, Lorenzo.Pieralisi, tn, linux-pci,
linux-kernel, xuwei (O),
linux-acpi, jcm, zhangjukuo, Liguozhu (Kenneth)
Hi Arnd
> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@arndb.de]
> Sent: 08 February 2016 13:50
> To: linux-arm-kernel@lists.infradead.org
> Cc: Gabriele Paoloni; Guohanjun (Hanjun Guo); Wangzhou (B); liudongdong
> (C); Linuxarm; qiujiang; bhelgaas@google.com;
> Lorenzo.Pieralisi@arm.com; tn@semihalf.com; linux-pci@vger.kernel.org;
> linux-kernel@vger.kernel.org; xuwei (O); linux-acpi@vger.kernel.org;
> jcm@redhat.com; zhangjukuo; Liguozhu (Kenneth)
> Subject: Re: [RFC PATCH v2 1/3] PCI: hisi: re-architect Hip05/Hip06
> controllers driver to preapare for ACPI
>
> On Monday 08 February 2016 12:41:02 Gabriele Paoloni 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 = ®_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;
> > +}
>
> Isn't this the same hack that Qualcomm are using?
As far as I can see Qualcomm defines its own config access
mechanism only for RC config read and also it seems they're
having problems with reporting the device class...
https://github.com/torvalds/linux/blob/master/drivers/pci/host/pcie-qcom.c#L474
Our problem is that our HW can only perform 32b rd/wr accesses
So we can't use readw/readb/writew/writeb...
Thanks
Gab
>
> Arnd
^ permalink raw reply [flat|nested] 63+ messages in thread* [RFC PATCH v2 1/3] PCI: hisi: re-architect Hip05/Hip06 controllers driver to preapare for ACPI
@ 2016-02-08 16:06 ` Gabriele Paoloni
0 siblings, 0 replies; 63+ messages in thread
From: Gabriele Paoloni @ 2016-02-08 16:06 UTC (permalink / raw)
To: linux-arm-kernel
Hi Arnd
> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd at arndb.de]
> Sent: 08 February 2016 13:50
> To: linux-arm-kernel at lists.infradead.org
> Cc: Gabriele Paoloni; Guohanjun (Hanjun Guo); Wangzhou (B); liudongdong
> (C); Linuxarm; qiujiang; bhelgaas at google.com;
> Lorenzo.Pieralisi at arm.com; tn at semihalf.com; linux-pci at vger.kernel.org;
> linux-kernel at vger.kernel.org; xuwei (O); linux-acpi at vger.kernel.org;
> jcm at redhat.com; zhangjukuo; Liguozhu (Kenneth)
> Subject: Re: [RFC PATCH v2 1/3] PCI: hisi: re-architect Hip05/Hip06
> controllers driver to preapare for ACPI
>
> On Monday 08 February 2016 12:41:02 Gabriele Paoloni 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 = ®_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;
> > +}
>
> Isn't this the same hack that Qualcomm are using?
As far as I can see Qualcomm defines its own config access
mechanism only for RC config read and also it seems they're
having problems with reporting the device class...
https://github.com/torvalds/linux/blob/master/drivers/pci/host/pcie-qcom.c#L474
Our problem is that our HW can only perform 32b rd/wr accesses
So we can't use readw/readb/writew/writeb...
Thanks
Gab
>
> Arnd
^ permalink raw reply [flat|nested] 63+ messages in thread* RE: [RFC PATCH v2 1/3] PCI: hisi: re-architect Hip05/Hip06 controllers driver to preapare for ACPI
@ 2016-02-08 16:06 ` Gabriele Paoloni
0 siblings, 0 replies; 63+ messages in thread
From: Gabriele Paoloni @ 2016-02-08 16:06 UTC (permalink / raw)
To: Arnd Bergmann, linux-arm-kernel
Cc: Guohanjun (Hanjun Guo), Wangzhou (B), liudongdong (C),
Linuxarm, qiujiang, bhelgaas, Lorenzo.Pieralisi, tn, linux-pci,
linux-kernel, xuwei (O),
linux-acpi, jcm, zhangjukuo, Liguozhu (Kenneth)
Hi Arnd
> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@arndb.de]
> Sent: 08 February 2016 13:50
> To: linux-arm-kernel@lists.infradead.org
> Cc: Gabriele Paoloni; Guohanjun (Hanjun Guo); Wangzhou (B); liudongdong
> (C); Linuxarm; qiujiang; bhelgaas@google.com;
> Lorenzo.Pieralisi@arm.com; tn@semihalf.com; linux-pci@vger.kernel.org;
> linux-kernel@vger.kernel.org; xuwei (O); linux-acpi@vger.kernel.org;
> jcm@redhat.com; zhangjukuo; Liguozhu (Kenneth)
> Subject: Re: [RFC PATCH v2 1/3] PCI: hisi: re-architect Hip05/Hip06
> controllers driver to preapare for ACPI
>
> On Monday 08 February 2016 12:41:02 Gabriele Paoloni 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 = ®_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;
> > +}
>
> Isn't this the same hack that Qualcomm are using?
As far as I can see Qualcomm defines its own config access
mechanism only for RC config read and also it seems they're
having problems with reporting the device class...
https://github.com/torvalds/linux/blob/master/drivers/pci/host/pcie-qcom.c#L474
Our problem is that our HW can only perform 32b rd/wr accesses
So we can't use readw/readb/writew/writeb...
Thanks
Gab
>
> Arnd
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [RFC PATCH v2 1/3] PCI: hisi: re-architect Hip05/Hip06 controllers driver to preapare for ACPI
2016-02-08 16:06 ` Gabriele Paoloni
(?)
@ 2016-02-08 16:32 ` Arnd Bergmann
-1 siblings, 0 replies; 63+ messages in thread
From: Arnd Bergmann @ 2016-02-08 16:32 UTC (permalink / raw)
To: Gabriele Paoloni
Cc: linux-arm-kernel, Guohanjun (Hanjun Guo), Wangzhou (B),
liudongdong (C),
Linuxarm, qiujiang, bhelgaas, Lorenzo.Pieralisi, tn, linux-pci,
linux-kernel, xuwei (O),
linux-acpi, jcm, zhangjukuo, Liguozhu (Kenneth)
On Monday 08 February 2016 16:06:54 Gabriele Paoloni wrote:
> >
> > On Monday 08 February 2016 12:41:02 Gabriele Paoloni 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 = ®_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;
> > > +}
> >
> > Isn't this the same hack that Qualcomm are using?
>
> As far as I can see Qualcomm defines its own config access
> mechanism only for RC config read and also it seems they're
> having problems with reporting the device class...
>
> https://github.com/torvalds/linux/blob/master/drivers/pci/host/pcie-qcom.c#L474
>
> Our problem is that our HW can only perform 32b rd/wr accesses
> So we can't use readw/readb/writew/writeb...
>
>
Sorry, my mistake, I meant Cavium not Qualcomm.
See https://lkml.org/lkml/2016/2/5/689 for the patches.
Arnd
^ permalink raw reply [flat|nested] 63+ messages in thread* [RFC PATCH v2 1/3] PCI: hisi: re-architect Hip05/Hip06 controllers driver to preapare for ACPI
@ 2016-02-08 16:32 ` Arnd Bergmann
0 siblings, 0 replies; 63+ messages in thread
From: Arnd Bergmann @ 2016-02-08 16:32 UTC (permalink / raw)
To: linux-arm-kernel
On Monday 08 February 2016 16:06:54 Gabriele Paoloni wrote:
> >
> > On Monday 08 February 2016 12:41:02 Gabriele Paoloni 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 = ®_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;
> > > +}
> >
> > Isn't this the same hack that Qualcomm are using?
>
> As far as I can see Qualcomm defines its own config access
> mechanism only for RC config read and also it seems they're
> having problems with reporting the device class...
>
> https://github.com/torvalds/linux/blob/master/drivers/pci/host/pcie-qcom.c#L474
>
> Our problem is that our HW can only perform 32b rd/wr accesses
> So we can't use readw/readb/writew/writeb...
>
>
Sorry, my mistake, I meant Cavium not Qualcomm.
See https://lkml.org/lkml/2016/2/5/689 for the patches.
Arnd
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [RFC PATCH v2 1/3] PCI: hisi: re-architect Hip05/Hip06 controllers driver to preapare for ACPI
@ 2016-02-08 16:32 ` Arnd Bergmann
0 siblings, 0 replies; 63+ messages in thread
From: Arnd Bergmann @ 2016-02-08 16:32 UTC (permalink / raw)
To: Gabriele Paoloni
Cc: linux-arm-kernel, Guohanjun (Hanjun Guo), Wangzhou (B),
liudongdong (C),
Linuxarm, qiujiang, bhelgaas, Lorenzo.Pieralisi, tn, linux-pci,
linux-kernel, xuwei (O),
linux-acpi, jcm, zhangjukuo, Liguozhu (Kenneth)
On Monday 08 February 2016 16:06:54 Gabriele Paoloni wrote:
> >
> > On Monday 08 February 2016 12:41:02 Gabriele Paoloni 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 = ®_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;
> > > +}
> >
> > Isn't this the same hack that Qualcomm are using?
>
> As far as I can see Qualcomm defines its own config access
> mechanism only for RC config read and also it seems they're
> having problems with reporting the device class...
>
> https://github.com/torvalds/linux/blob/master/drivers/pci/host/pcie-qcom.c#L474
>
> Our problem is that our HW can only perform 32b rd/wr accesses
> So we can't use readw/readb/writew/writeb...
>
>
Sorry, my mistake, I meant Cavium not Qualcomm.
See https://lkml.org/lkml/2016/2/5/689 for the patches.
Arnd
^ permalink raw reply [flat|nested] 63+ messages in thread* RE: [RFC PATCH v2 1/3] PCI: hisi: re-architect Hip05/Hip06 controllers driver to preapare for ACPI
2016-02-08 16:32 ` Arnd Bergmann
(?)
@ 2016-02-08 16:51 ` Gabriele Paoloni
-1 siblings, 0 replies; 63+ messages in thread
From: Gabriele Paoloni @ 2016-02-08 16:51 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-arm-kernel, Guohanjun (Hanjun Guo), Wangzhou (B),
liudongdong (C),
Linuxarm, qiujiang, bhelgaas, Lorenzo.Pieralisi, tn, linux-pci,
linux-kernel, xuwei (O),
linux-acpi, jcm, zhangjukuo, Liguozhu (Kenneth)
> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@arndb.de]
> Sent: 08 February 2016 16:33
> To: Gabriele Paoloni
> Cc: linux-arm-kernel@lists.infradead.org; Guohanjun (Hanjun Guo);
> Wangzhou (B); liudongdong (C); Linuxarm; qiujiang; bhelgaas@google.com;
> Lorenzo.Pieralisi@arm.com; tn@semihalf.com; linux-pci@vger.kernel.org;
> linux-kernel@vger.kernel.org; xuwei (O); linux-acpi@vger.kernel.org;
> jcm@redhat.com; zhangjukuo; Liguozhu (Kenneth)
> Subject: Re: [RFC PATCH v2 1/3] PCI: hisi: re-architect Hip05/Hip06
> controllers driver to preapare for ACPI
>
> On Monday 08 February 2016 16:06:54 Gabriele Paoloni wrote:
> > >
> > > On Monday 08 February 2016 12:41:02 Gabriele Paoloni 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 = ®_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;
> > > > +}
> > >
> > > Isn't this the same hack that Qualcomm are using?
> >
> > As far as I can see Qualcomm defines its own config access
> > mechanism only for RC config read and also it seems they're
> > having problems with reporting the device class...
> >
> > https://github.com/torvalds/linux/blob/master/drivers/pci/host/pcie-
> qcom.c#L474
> >
> > Our problem is that our HW can only perform 32b rd/wr accesses
> > So we can't use readw/readb/writew/writeb...
> >
> >
>
> Sorry, my mistake, I meant Cavium not Qualcomm.
> See https://lkml.org/lkml/2016/2/5/689 for the patches.
Well, looking at it Cavium seems quite different,
On read they need to trigger the retrieval of the
config space info writing to the lower 32b of a 64b register,
then they need to read data back on the upper 64b of the
same register and adjust the content to remove the garbage...
We just use 32b accesses and adjust grab the appropriate
bytes depending on the read/write sizes...
Thanks
Gab
>
> Arnd
^ permalink raw reply [flat|nested] 63+ messages in thread* [RFC PATCH v2 1/3] PCI: hisi: re-architect Hip05/Hip06 controllers driver to preapare for ACPI
@ 2016-02-08 16:51 ` Gabriele Paoloni
0 siblings, 0 replies; 63+ messages in thread
From: Gabriele Paoloni @ 2016-02-08 16:51 UTC (permalink / raw)
To: linux-arm-kernel
> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd at arndb.de]
> Sent: 08 February 2016 16:33
> To: Gabriele Paoloni
> Cc: linux-arm-kernel at lists.infradead.org; Guohanjun (Hanjun Guo);
> Wangzhou (B); liudongdong (C); Linuxarm; qiujiang; bhelgaas at google.com;
> Lorenzo.Pieralisi at arm.com; tn at semihalf.com; linux-pci at vger.kernel.org;
> linux-kernel at vger.kernel.org; xuwei (O); linux-acpi at vger.kernel.org;
> jcm at redhat.com; zhangjukuo; Liguozhu (Kenneth)
> Subject: Re: [RFC PATCH v2 1/3] PCI: hisi: re-architect Hip05/Hip06
> controllers driver to preapare for ACPI
>
> On Monday 08 February 2016 16:06:54 Gabriele Paoloni wrote:
> > >
> > > On Monday 08 February 2016 12:41:02 Gabriele Paoloni 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 = ®_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;
> > > > +}
> > >
> > > Isn't this the same hack that Qualcomm are using?
> >
> > As far as I can see Qualcomm defines its own config access
> > mechanism only for RC config read and also it seems they're
> > having problems with reporting the device class...
> >
> > https://github.com/torvalds/linux/blob/master/drivers/pci/host/pcie-
> qcom.c#L474
> >
> > Our problem is that our HW can only perform 32b rd/wr accesses
> > So we can't use readw/readb/writew/writeb...
> >
> >
>
> Sorry, my mistake, I meant Cavium not Qualcomm.
> See https://lkml.org/lkml/2016/2/5/689 for the patches.
Well, looking at it Cavium seems quite different,
On read they need to trigger the retrieval of the
config space info writing to the lower 32b of a 64b register,
then they need to read data back on the upper 64b of the
same register and adjust the content to remove the garbage...
We just use 32b accesses and adjust grab the appropriate
bytes depending on the read/write sizes...
Thanks
Gab
>
> Arnd
^ permalink raw reply [flat|nested] 63+ messages in thread* RE: [RFC PATCH v2 1/3] PCI: hisi: re-architect Hip05/Hip06 controllers driver to preapare for ACPI
@ 2016-02-08 16:51 ` Gabriele Paoloni
0 siblings, 0 replies; 63+ messages in thread
From: Gabriele Paoloni @ 2016-02-08 16:51 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-arm-kernel, Guohanjun (Hanjun Guo), Wangzhou (B),
liudongdong (C),
Linuxarm, qiujiang, bhelgaas, Lorenzo.Pieralisi, tn, linux-pci,
linux-kernel, xuwei (O),
linux-acpi, jcm, zhangjukuo, Liguozhu (Kenneth)
> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@arndb.de]
> Sent: 08 February 2016 16:33
> To: Gabriele Paoloni
> Cc: linux-arm-kernel@lists.infradead.org; Guohanjun (Hanjun Guo);
> Wangzhou (B); liudongdong (C); Linuxarm; qiujiang; bhelgaas@google.com;
> Lorenzo.Pieralisi@arm.com; tn@semihalf.com; linux-pci@vger.kernel.org;
> linux-kernel@vger.kernel.org; xuwei (O); linux-acpi@vger.kernel.org;
> jcm@redhat.com; zhangjukuo; Liguozhu (Kenneth)
> Subject: Re: [RFC PATCH v2 1/3] PCI: hisi: re-architect Hip05/Hip06
> controllers driver to preapare for ACPI
>
> On Monday 08 February 2016 16:06:54 Gabriele Paoloni wrote:
> > >
> > > On Monday 08 February 2016 12:41:02 Gabriele Paoloni 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 = ®_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;
> > > > +}
> > >
> > > Isn't this the same hack that Qualcomm are using?
> >
> > As far as I can see Qualcomm defines its own config access
> > mechanism only for RC config read and also it seems they're
> > having problems with reporting the device class...
> >
> > https://github.com/torvalds/linux/blob/master/drivers/pci/host/pcie-
> qcom.c#L474
> >
> > Our problem is that our HW can only perform 32b rd/wr accesses
> > So we can't use readw/readb/writew/writeb...
> >
> >
>
> Sorry, my mistake, I meant Cavium not Qualcomm.
> See https://lkml.org/lkml/2016/2/5/689 for the patches.
Well, looking at it Cavium seems quite different,
On read they need to trigger the retrieval of the
config space info writing to the lower 32b of a 64b register,
then they need to read data back on the upper 64b of the
same register and adjust the content to remove the garbage...
We just use 32b accesses and adjust grab the appropriate
bytes depending on the read/write sizes...
Thanks
Gab
>
> Arnd
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [RFC PATCH v2 1/3] PCI: hisi: re-architect Hip05/Hip06 controllers driver to preapare for ACPI
2016-02-08 16:51 ` Gabriele Paoloni
(?)
@ 2016-02-09 16:27 ` Arnd Bergmann
-1 siblings, 0 replies; 63+ messages in thread
From: Arnd Bergmann @ 2016-02-09 16:27 UTC (permalink / raw)
To: Gabriele Paoloni
Cc: linux-arm-kernel, Guohanjun (Hanjun Guo), Wangzhou (B),
liudongdong (C),
Linuxarm, qiujiang, bhelgaas, Lorenzo.Pieralisi, tn, linux-pci,
linux-kernel, xuwei (O),
linux-acpi, jcm, zhangjukuo, Liguozhu (Kenneth)
On Monday 08 February 2016 16:51:19 Gabriele Paoloni wrote:
> > From: Arnd Bergmann [mailto:arnd@arndb.de]
> > On Monday 08 February 2016 16:06:54 Gabriele Paoloni wrote:
> > > > On Monday 08 February 2016 12:41:02 Gabriele Paoloni wrote:
> > int
> > > > size,
> > > > > + u32 *val)
> > > > > +{
> > > > > + u32 reg;
> > > > > + u32 reg_val;
> > > > > + void *walker = ®_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;
> > > > > +}
> > > >
> > > > Isn't this the same hack that Qualcomm are using?
> > >
> > > As far as I can see Qualcomm defines its own config access
> > > mechanism only for RC config read and also it seems they're
> > > having problems with reporting the device class...
> > >
> > > https://github.com/torvalds/linux/blob/master/drivers/pci/host/pcie-
> > qcom.c#L474
> > >
> > > Our problem is that our HW can only perform 32b rd/wr accesses
> > > So we can't use readw/readb/writew/writeb...
> > >
> > >
> >
> > Sorry, my mistake, I meant Cavium not Qualcomm.
> > See https://lkml.org/lkml/2016/2/5/689 for the patches.
>
> Well, looking at it Cavium seems quite different,
>
> On read they need to trigger the retrieval of the
> config space info writing to the lower 32b of a 64b register,
> then they need to read data back on the upper 64b of the
> same register and adjust the content to remove the garbage...
>
> We just use 32b accesses and adjust grab the appropriate
> bytes depending on the read/write sizes...
Hmm, I must have misremembered that too then, let me try once more ;-)
The above appears to reimplement pci_generic_config_read32(). Can you
just use that instead?
Arnd
^ permalink raw reply [flat|nested] 63+ messages in thread* [RFC PATCH v2 1/3] PCI: hisi: re-architect Hip05/Hip06 controllers driver to preapare for ACPI
@ 2016-02-09 16:27 ` Arnd Bergmann
0 siblings, 0 replies; 63+ messages in thread
From: Arnd Bergmann @ 2016-02-09 16:27 UTC (permalink / raw)
To: linux-arm-kernel
On Monday 08 February 2016 16:51:19 Gabriele Paoloni wrote:
> > From: Arnd Bergmann [mailto:arnd at arndb.de]
> > On Monday 08 February 2016 16:06:54 Gabriele Paoloni wrote:
> > > > On Monday 08 February 2016 12:41:02 Gabriele Paoloni wrote:
> > int
> > > > size,
> > > > > + u32 *val)
> > > > > +{
> > > > > + u32 reg;
> > > > > + u32 reg_val;
> > > > > + void *walker = ®_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;
> > > > > +}
> > > >
> > > > Isn't this the same hack that Qualcomm are using?
> > >
> > > As far as I can see Qualcomm defines its own config access
> > > mechanism only for RC config read and also it seems they're
> > > having problems with reporting the device class...
> > >
> > > https://github.com/torvalds/linux/blob/master/drivers/pci/host/pcie-
> > qcom.c#L474
> > >
> > > Our problem is that our HW can only perform 32b rd/wr accesses
> > > So we can't use readw/readb/writew/writeb...
> > >
> > >
> >
> > Sorry, my mistake, I meant Cavium not Qualcomm.
> > See https://lkml.org/lkml/2016/2/5/689 for the patches.
>
> Well, looking at it Cavium seems quite different,
>
> On read they need to trigger the retrieval of the
> config space info writing to the lower 32b of a 64b register,
> then they need to read data back on the upper 64b of the
> same register and adjust the content to remove the garbage...
>
> We just use 32b accesses and adjust grab the appropriate
> bytes depending on the read/write sizes...
Hmm, I must have misremembered that too then, let me try once more ;-)
The above appears to reimplement pci_generic_config_read32(). Can you
just use that instead?
Arnd
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [RFC PATCH v2 1/3] PCI: hisi: re-architect Hip05/Hip06 controllers driver to preapare for ACPI
@ 2016-02-09 16:27 ` Arnd Bergmann
0 siblings, 0 replies; 63+ messages in thread
From: Arnd Bergmann @ 2016-02-09 16:27 UTC (permalink / raw)
To: Gabriele Paoloni
Cc: linux-arm-kernel, Guohanjun (Hanjun Guo), Wangzhou (B),
liudongdong (C),
Linuxarm, qiujiang, bhelgaas, Lorenzo.Pieralisi, tn, linux-pci,
linux-kernel, xuwei (O),
linux-acpi, jcm, zhangjukuo, Liguozhu (Kenneth)
On Monday 08 February 2016 16:51:19 Gabriele Paoloni wrote:
> > From: Arnd Bergmann [mailto:arnd@arndb.de]
> > On Monday 08 February 2016 16:06:54 Gabriele Paoloni wrote:
> > > > On Monday 08 February 2016 12:41:02 Gabriele Paoloni wrote:
> > int
> > > > size,
> > > > > + u32 *val)
> > > > > +{
> > > > > + u32 reg;
> > > > > + u32 reg_val;
> > > > > + void *walker = ®_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;
> > > > > +}
> > > >
> > > > Isn't this the same hack that Qualcomm are using?
> > >
> > > As far as I can see Qualcomm defines its own config access
> > > mechanism only for RC config read and also it seems they're
> > > having problems with reporting the device class...
> > >
> > > https://github.com/torvalds/linux/blob/master/drivers/pci/host/pcie-
> > qcom.c#L474
> > >
> > > Our problem is that our HW can only perform 32b rd/wr accesses
> > > So we can't use readw/readb/writew/writeb...
> > >
> > >
> >
> > Sorry, my mistake, I meant Cavium not Qualcomm.
> > See https://lkml.org/lkml/2016/2/5/689 for the patches.
>
> Well, looking at it Cavium seems quite different,
>
> On read they need to trigger the retrieval of the
> config space info writing to the lower 32b of a 64b register,
> then they need to read data back on the upper 64b of the
> same register and adjust the content to remove the garbage...
>
> We just use 32b accesses and adjust grab the appropriate
> bytes depending on the read/write sizes...
Hmm, I must have misremembered that too then, let me try once more ;-)
The above appears to reimplement pci_generic_config_read32(). Can you
just use that instead?
Arnd
^ permalink raw reply [flat|nested] 63+ messages in thread* RE: [RFC PATCH v2 1/3] PCI: hisi: re-architect Hip05/Hip06 controllers driver to preapare for ACPI
2016-02-09 16:27 ` Arnd Bergmann
(?)
@ 2016-02-09 16:52 ` Gabriele Paoloni
-1 siblings, 0 replies; 63+ messages in thread
From: Gabriele Paoloni @ 2016-02-09 16:52 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-arm-kernel, Guohanjun (Hanjun Guo), Wangzhou (B),
liudongdong (C),
Linuxarm, qiujiang, bhelgaas, Lorenzo.Pieralisi, tn, linux-pci,
linux-kernel, xuwei (O),
linux-acpi, jcm, zhangjukuo, Liguozhu (Kenneth)
> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@arndb.de]
> Sent: 09 February 2016 16:27
> To: Gabriele Paoloni
> Cc: linux-arm-kernel@lists.infradead.org; Guohanjun (Hanjun Guo);
> Wangzhou (B); liudongdong (C); Linuxarm; qiujiang; bhelgaas@google.com;
> Lorenzo.Pieralisi@arm.com; tn@semihalf.com; linux-pci@vger.kernel.org;
> linux-kernel@vger.kernel.org; xuwei (O); linux-acpi@vger.kernel.org;
> jcm@redhat.com; zhangjukuo; Liguozhu (Kenneth)
> Subject: Re: [RFC PATCH v2 1/3] PCI: hisi: re-architect Hip05/Hip06
> controllers driver to preapare for ACPI
>
> On Monday 08 February 2016 16:51:19 Gabriele Paoloni wrote:
> > > From: Arnd Bergmann [mailto:arnd@arndb.de]
> > > On Monday 08 February 2016 16:06:54 Gabriele Paoloni wrote:
> > > > > On Monday 08 February 2016 12:41:02 Gabriele Paoloni wrote:
> > > int
> > > > > size,
> > > > > > + u32 *val)
> > > > > > +{
> > > > > > + u32 reg;
> > > > > > + u32 reg_val;
> > > > > > + void *walker = ®_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;
> > > > > > +}
> > > > >
> > > > > Isn't this the same hack that Qualcomm are using?
> > > >
> > > > As far as I can see Qualcomm defines its own config access
> > > > mechanism only for RC config read and also it seems they're
> > > > having problems with reporting the device class...
> > > >
> > > >
> https://github.com/torvalds/linux/blob/master/drivers/pci/host/pcie-
> > > qcom.c#L474
> > > >
> > > > Our problem is that our HW can only perform 32b rd/wr accesses
> > > > So we can't use readw/readb/writew/writeb...
> > > >
> > > >
> > >
> > > Sorry, my mistake, I meant Cavium not Qualcomm.
> > > See https://lkml.org/lkml/2016/2/5/689 for the patches.
> >
> > Well, looking at it Cavium seems quite different,
> >
> > On read they need to trigger the retrieval of the
> > config space info writing to the lower 32b of a 64b register,
> > then they need to read data back on the upper 64b of the
> > same register and adjust the content to remove the garbage...
> >
> > We just use 32b accesses and adjust grab the appropriate
> > bytes depending on the read/write sizes...
>
> Hmm, I must have misremembered that too then, let me try once more ;-)
>
> The above appears to reimplement pci_generic_config_read32(). Can you
> just use that instead?
Nope I don't think so,
When we read the root complex config space we need to use a configuration
address space that is different from the one used to map the rest of the
hierarchy; I think this is something to do with Designware itself. It
is clear if you look at
http://lxr.free-electrons.com/source/drivers/pci/host/pcie-designware.c#L657
Here you can see that in calling "dw_pcie_wr_own_conf" Designware does not
pass "bus" and "devfn" that are actually required by pci_generic_config_read32()
to map the addr...
Many Thanks
Gab
>
> Arnd
^ permalink raw reply [flat|nested] 63+ messages in thread* [RFC PATCH v2 1/3] PCI: hisi: re-architect Hip05/Hip06 controllers driver to preapare for ACPI
@ 2016-02-09 16:52 ` Gabriele Paoloni
0 siblings, 0 replies; 63+ messages in thread
From: Gabriele Paoloni @ 2016-02-09 16:52 UTC (permalink / raw)
To: linux-arm-kernel
> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd at arndb.de]
> Sent: 09 February 2016 16:27
> To: Gabriele Paoloni
> Cc: linux-arm-kernel at lists.infradead.org; Guohanjun (Hanjun Guo);
> Wangzhou (B); liudongdong (C); Linuxarm; qiujiang; bhelgaas at google.com;
> Lorenzo.Pieralisi at arm.com; tn at semihalf.com; linux-pci at vger.kernel.org;
> linux-kernel at vger.kernel.org; xuwei (O); linux-acpi at vger.kernel.org;
> jcm at redhat.com; zhangjukuo; Liguozhu (Kenneth)
> Subject: Re: [RFC PATCH v2 1/3] PCI: hisi: re-architect Hip05/Hip06
> controllers driver to preapare for ACPI
>
> On Monday 08 February 2016 16:51:19 Gabriele Paoloni wrote:
> > > From: Arnd Bergmann [mailto:arnd at arndb.de]
> > > On Monday 08 February 2016 16:06:54 Gabriele Paoloni wrote:
> > > > > On Monday 08 February 2016 12:41:02 Gabriele Paoloni wrote:
> > > int
> > > > > size,
> > > > > > + u32 *val)
> > > > > > +{
> > > > > > + u32 reg;
> > > > > > + u32 reg_val;
> > > > > > + void *walker = ®_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;
> > > > > > +}
> > > > >
> > > > > Isn't this the same hack that Qualcomm are using?
> > > >
> > > > As far as I can see Qualcomm defines its own config access
> > > > mechanism only for RC config read and also it seems they're
> > > > having problems with reporting the device class...
> > > >
> > > >
> https://github.com/torvalds/linux/blob/master/drivers/pci/host/pcie-
> > > qcom.c#L474
> > > >
> > > > Our problem is that our HW can only perform 32b rd/wr accesses
> > > > So we can't use readw/readb/writew/writeb...
> > > >
> > > >
> > >
> > > Sorry, my mistake, I meant Cavium not Qualcomm.
> > > See https://lkml.org/lkml/2016/2/5/689 for the patches.
> >
> > Well, looking at it Cavium seems quite different,
> >
> > On read they need to trigger the retrieval of the
> > config space info writing to the lower 32b of a 64b register,
> > then they need to read data back on the upper 64b of the
> > same register and adjust the content to remove the garbage...
> >
> > We just use 32b accesses and adjust grab the appropriate
> > bytes depending on the read/write sizes...
>
> Hmm, I must have misremembered that too then, let me try once more ;-)
>
> The above appears to reimplement pci_generic_config_read32(). Can you
> just use that instead?
Nope I don't think so,
When we read the root complex config space we need to use a configuration
address space that is different from the one used to map the rest of the
hierarchy; I think this is something to do with Designware itself. It
is clear if you look at
http://lxr.free-electrons.com/source/drivers/pci/host/pcie-designware.c#L657
Here you can see that in calling "dw_pcie_wr_own_conf" Designware does not
pass "bus" and "devfn" that are actually required by pci_generic_config_read32()
to map the addr...
Many Thanks
Gab
>
> Arnd
^ permalink raw reply [flat|nested] 63+ messages in thread* RE: [RFC PATCH v2 1/3] PCI: hisi: re-architect Hip05/Hip06 controllers driver to preapare for ACPI
@ 2016-02-09 16:52 ` Gabriele Paoloni
0 siblings, 0 replies; 63+ messages in thread
From: Gabriele Paoloni @ 2016-02-09 16:52 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-arm-kernel, Guohanjun (Hanjun Guo), Wangzhou (B),
liudongdong (C),
Linuxarm, qiujiang, bhelgaas, Lorenzo.Pieralisi, tn, linux-pci,
linux-kernel, xuwei (O),
linux-acpi, jcm, zhangjukuo, Liguozhu (Kenneth)
> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@arndb.de]
> Sent: 09 February 2016 16:27
> To: Gabriele Paoloni
> Cc: linux-arm-kernel@lists.infradead.org; Guohanjun (Hanjun Guo);
> Wangzhou (B); liudongdong (C); Linuxarm; qiujiang; bhelgaas@google.com;
> Lorenzo.Pieralisi@arm.com; tn@semihalf.com; linux-pci@vger.kernel.org;
> linux-kernel@vger.kernel.org; xuwei (O); linux-acpi@vger.kernel.org;
> jcm@redhat.com; zhangjukuo; Liguozhu (Kenneth)
> Subject: Re: [RFC PATCH v2 1/3] PCI: hisi: re-architect Hip05/Hip06
> controllers driver to preapare for ACPI
>
> On Monday 08 February 2016 16:51:19 Gabriele Paoloni wrote:
> > > From: Arnd Bergmann [mailto:arnd@arndb.de]
> > > On Monday 08 February 2016 16:06:54 Gabriele Paoloni wrote:
> > > > > On Monday 08 February 2016 12:41:02 Gabriele Paoloni wrote:
> > > int
> > > > > size,
> > > > > > + u32 *val)
> > > > > > +{
> > > > > > + u32 reg;
> > > > > > + u32 reg_val;
> > > > > > + void *walker = ®_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;
> > > > > > +}
> > > > >
> > > > > Isn't this the same hack that Qualcomm are using?
> > > >
> > > > As far as I can see Qualcomm defines its own config access
> > > > mechanism only for RC config read and also it seems they're
> > > > having problems with reporting the device class...
> > > >
> > > >
> https://github.com/torvalds/linux/blob/master/drivers/pci/host/pcie-
> > > qcom.c#L474
> > > >
> > > > Our problem is that our HW can only perform 32b rd/wr accesses
> > > > So we can't use readw/readb/writew/writeb...
> > > >
> > > >
> > >
> > > Sorry, my mistake, I meant Cavium not Qualcomm.
> > > See https://lkml.org/lkml/2016/2/5/689 for the patches.
> >
> > Well, looking at it Cavium seems quite different,
> >
> > On read they need to trigger the retrieval of the
> > config space info writing to the lower 32b of a 64b register,
> > then they need to read data back on the upper 64b of the
> > same register and adjust the content to remove the garbage...
> >
> > We just use 32b accesses and adjust grab the appropriate
> > bytes depending on the read/write sizes...
>
> Hmm, I must have misremembered that too then, let me try once more ;-)
>
> The above appears to reimplement pci_generic_config_read32(). Can you
> just use that instead?
Nope I don't think so,
When we read the root complex config space we need to use a configuration
address space that is different from the one used to map the rest of the
hierarchy; I think this is something to do with Designware itself. It
is clear if you look at
http://lxr.free-electrons.com/source/drivers/pci/host/pcie-designware.c#L657
Here you can see that in calling "dw_pcie_wr_own_conf" Designware does not
pass "bus" and "devfn" that are actually required by pci_generic_config_read32()
to map the addr...
Many Thanks
Gab
>
> Arnd
^ permalink raw reply [flat|nested] 63+ messages in thread
* [RFC PATCH v2 2/3] PCI: hisi: Make the HiSilicon PCIe host controller ECAM compliant
2016-02-08 12:41 ` Gabriele Paoloni
(?)
@ 2016-02-08 12:41 ` Gabriele Paoloni
-1 siblings, 0 replies; 63+ messages in thread
From: Gabriele Paoloni @ 2016-02-08 12:41 UTC (permalink / raw)
To: guohanjun, wangzhou1, liudongdong3, linuxarm, qiujiang, bhelgaas,
arnd, Lorenzo.Pieralisi, tn
Cc: gabriele.paoloni, zhangjukuo, xuwei5, liguozhu, linux-pci,
linux-arm-kernel, linux-acpi, linux-kernel, jcm
From: gabriele paoloni <gabriele.paoloni@huawei.com>
This patch modifies the current Hip05/Hip06 PCIe host controller
driver to make it ECAM compliant. 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).
Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
---
drivers/pci/host/pcie-designware.c | 4 +---
drivers/pci/host/pcie-designware.h | 2 ++
drivers/pci/host/pcie-hisi.c | 35 +++++++++++++++++++++++++++++++++++
3 files changed, 38 insertions(+), 3 deletions(-)
diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index 2171682..5f19ea4 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -69,8 +69,6 @@
#define PCIE_ATU_FUNC(x) (((x) & 0x7) << 16)
#define PCIE_ATU_UPPER_TARGET 0x91C
-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)) {
@@ -690,7 +688,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 2356d29..3f42655 100644
--- a/drivers/pci/host/pcie-designware.h
+++ b/drivers/pci/host/pcie-designware.h
@@ -80,4 +80,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 458d0f8..eb3201a 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_other_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_other_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)
{
@@ -67,11 +79,24 @@ static int hisi_pcie_link_up(struct pcie_port *pp)
}
struct pcie_host_ops hisi_pcie_host_ops = {
+ .rd_other_conf = hisi_rd_other_conf,
+ .wr_other_conf = hisi_wr_other_conf,
.rd_own_conf = hisi_pcie_cfg_read,
.wr_own_conf = hisi_pcie_cfg_write,
.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 + where;
+
+ return addr;
+}
static int hisi_add_pcie_port(struct pcie_port *pp,
struct platform_device *pdev)
@@ -92,6 +117,8 @@ static int hisi_add_pcie_port(struct pcie_port *pp,
pp->ops = &hisi_pcie_host_ops;
+ dw_pcie_ops.map_bus = hisi_pci_map_cfg_bus_cam;
+
ret = dw_pcie_host_init(pp);
if (ret) {
dev_err(&pdev->dev, "failed to initialize host\n");
@@ -137,6 +164,14 @@ 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");
+ 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;
+
ret = hisi_add_pcie_port(pp, pdev);
if (ret)
return ret;
--
1.9.1
^ permalink raw reply related [flat|nested] 63+ messages in thread* [RFC PATCH v2 2/3] PCI: hisi: Make the HiSilicon PCIe host controller ECAM compliant
@ 2016-02-08 12:41 ` Gabriele Paoloni
0 siblings, 0 replies; 63+ messages in thread
From: Gabriele Paoloni @ 2016-02-08 12:41 UTC (permalink / raw)
To: linux-arm-kernel
From: gabriele paoloni <gabriele.paoloni@huawei.com>
This patch modifies the current Hip05/Hip06 PCIe host controller
driver to make it ECAM compliant. 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).
Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
---
drivers/pci/host/pcie-designware.c | 4 +---
drivers/pci/host/pcie-designware.h | 2 ++
drivers/pci/host/pcie-hisi.c | 35 +++++++++++++++++++++++++++++++++++
3 files changed, 38 insertions(+), 3 deletions(-)
diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index 2171682..5f19ea4 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -69,8 +69,6 @@
#define PCIE_ATU_FUNC(x) (((x) & 0x7) << 16)
#define PCIE_ATU_UPPER_TARGET 0x91C
-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)) {
@@ -690,7 +688,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 2356d29..3f42655 100644
--- a/drivers/pci/host/pcie-designware.h
+++ b/drivers/pci/host/pcie-designware.h
@@ -80,4 +80,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 458d0f8..eb3201a 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_other_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_other_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)
{
@@ -67,11 +79,24 @@ static int hisi_pcie_link_up(struct pcie_port *pp)
}
struct pcie_host_ops hisi_pcie_host_ops = {
+ .rd_other_conf = hisi_rd_other_conf,
+ .wr_other_conf = hisi_wr_other_conf,
.rd_own_conf = hisi_pcie_cfg_read,
.wr_own_conf = hisi_pcie_cfg_write,
.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 + where;
+
+ return addr;
+}
static int hisi_add_pcie_port(struct pcie_port *pp,
struct platform_device *pdev)
@@ -92,6 +117,8 @@ static int hisi_add_pcie_port(struct pcie_port *pp,
pp->ops = &hisi_pcie_host_ops;
+ dw_pcie_ops.map_bus = hisi_pci_map_cfg_bus_cam;
+
ret = dw_pcie_host_init(pp);
if (ret) {
dev_err(&pdev->dev, "failed to initialize host\n");
@@ -137,6 +164,14 @@ 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");
+ 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;
+
ret = hisi_add_pcie_port(pp, pdev);
if (ret)
return ret;
--
1.9.1
^ permalink raw reply related [flat|nested] 63+ messages in thread* [RFC PATCH v2 2/3] PCI: hisi: Make the HiSilicon PCIe host controller ECAM compliant
@ 2016-02-08 12:41 ` Gabriele Paoloni
0 siblings, 0 replies; 63+ messages in thread
From: Gabriele Paoloni @ 2016-02-08 12:41 UTC (permalink / raw)
To: guohanjun, wangzhou1, liudongdong3, linuxarm, qiujiang, bhelgaas,
arnd, Lorenzo.Pieralisi, tn
Cc: gabriele.paoloni, zhangjukuo, xuwei5, liguozhu, linux-pci,
linux-arm-kernel, linux-acpi, linux-kernel, jcm
From: gabriele paoloni <gabriele.paoloni@huawei.com>
This patch modifies the current Hip05/Hip06 PCIe host controller
driver to make it ECAM compliant. 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).
Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
---
drivers/pci/host/pcie-designware.c | 4 +---
drivers/pci/host/pcie-designware.h | 2 ++
drivers/pci/host/pcie-hisi.c | 35 +++++++++++++++++++++++++++++++++++
3 files changed, 38 insertions(+), 3 deletions(-)
diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index 2171682..5f19ea4 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -69,8 +69,6 @@
#define PCIE_ATU_FUNC(x) (((x) & 0x7) << 16)
#define PCIE_ATU_UPPER_TARGET 0x91C
-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)) {
@@ -690,7 +688,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 2356d29..3f42655 100644
--- a/drivers/pci/host/pcie-designware.h
+++ b/drivers/pci/host/pcie-designware.h
@@ -80,4 +80,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 458d0f8..eb3201a 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_other_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_other_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)
{
@@ -67,11 +79,24 @@ static int hisi_pcie_link_up(struct pcie_port *pp)
}
struct pcie_host_ops hisi_pcie_host_ops = {
+ .rd_other_conf = hisi_rd_other_conf,
+ .wr_other_conf = hisi_wr_other_conf,
.rd_own_conf = hisi_pcie_cfg_read,
.wr_own_conf = hisi_pcie_cfg_write,
.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 + where;
+
+ return addr;
+}
static int hisi_add_pcie_port(struct pcie_port *pp,
struct platform_device *pdev)
@@ -92,6 +117,8 @@ static int hisi_add_pcie_port(struct pcie_port *pp,
pp->ops = &hisi_pcie_host_ops;
+ dw_pcie_ops.map_bus = hisi_pci_map_cfg_bus_cam;
+
ret = dw_pcie_host_init(pp);
if (ret) {
dev_err(&pdev->dev, "failed to initialize host\n");
@@ -137,6 +164,14 @@ 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");
+ 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;
+
ret = hisi_add_pcie_port(pp, pdev);
if (ret)
return ret;
--
1.9.1
^ permalink raw reply related [flat|nested] 63+ messages in thread* Re: [RFC PATCH v2 2/3] PCI: hisi: Make the HiSilicon PCIe host controller ECAM compliant
2016-02-08 12:41 ` Gabriele Paoloni
@ 2016-02-08 13:48 ` Arnd Bergmann
-1 siblings, 0 replies; 63+ messages in thread
From: Arnd Bergmann @ 2016-02-08 13:48 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Gabriele Paoloni, guohanjun, wangzhou1, liudongdong3, linuxarm,
qiujiang, bhelgaas, Lorenzo.Pieralisi, tn, linux-pci,
linux-kernel, xuwei5, linux-acpi, jcm, zhangjukuo, liguozhu
On Monday 08 February 2016 12:41:03 Gabriele Paoloni wrote:
> From: gabriele paoloni <gabriele.paoloni@huawei.com>
>
> This patch modifies the current Hip05/Hip06 PCIe host controller
> driver to make it ECAM compliant. 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).
>
> Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
>
Doesn't this break backwards compatibility?
I think you need to use a new compatible string in the firmware
if you change the register layout, and then change the driver
to support both the old and the new layout.
Arnd
^ permalink raw reply [flat|nested] 63+ messages in thread
* [RFC PATCH v2 2/3] PCI: hisi: Make the HiSilicon PCIe host controller ECAM compliant
@ 2016-02-08 13:48 ` Arnd Bergmann
0 siblings, 0 replies; 63+ messages in thread
From: Arnd Bergmann @ 2016-02-08 13:48 UTC (permalink / raw)
To: linux-arm-kernel
On Monday 08 February 2016 12:41:03 Gabriele Paoloni wrote:
> From: gabriele paoloni <gabriele.paoloni@huawei.com>
>
> This patch modifies the current Hip05/Hip06 PCIe host controller
> driver to make it ECAM compliant. 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).
>
> Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
>
Doesn't this break backwards compatibility?
I think you need to use a new compatible string in the firmware
if you change the register layout, and then change the driver
to support both the old and the new layout.
Arnd
^ permalink raw reply [flat|nested] 63+ messages in thread
* RE: [RFC PATCH v2 2/3] PCI: hisi: Make the HiSilicon PCIe host controller ECAM compliant
2016-02-08 13:48 ` Arnd Bergmann
(?)
@ 2016-02-08 15:55 ` Gabriele Paoloni
-1 siblings, 0 replies; 63+ messages in thread
From: Gabriele Paoloni @ 2016-02-08 15:55 UTC (permalink / raw)
To: Arnd Bergmann, linux-arm-kernel
Cc: Guohanjun (Hanjun Guo), Wangzhou (B), liudongdong (C),
Linuxarm, qiujiang, bhelgaas, Lorenzo.Pieralisi, tn, linux-pci,
linux-kernel, xuwei (O),
linux-acpi, jcm, zhangjukuo, Liguozhu (Kenneth)
> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@arndb.de]
> Sent: 08 February 2016 13:48
> To: linux-arm-kernel@lists.infradead.org
> Cc: Gabriele Paoloni; Guohanjun (Hanjun Guo); Wangzhou (B); liudongdong
> (C); Linuxarm; qiujiang; bhelgaas@google.com;
> Lorenzo.Pieralisi@arm.com; tn@semihalf.com; linux-pci@vger.kernel.org;
> linux-kernel@vger.kernel.org; xuwei (O); linux-acpi@vger.kernel.org;
> jcm@redhat.com; zhangjukuo; Liguozhu (Kenneth)
> Subject: Re: [RFC PATCH v2 2/3] PCI: hisi: Make the HiSilicon PCIe host
> controller ECAM compliant
>
> On Monday 08 February 2016 12:41:03 Gabriele Paoloni wrote:
> > From: gabriele paoloni <gabriele.paoloni@huawei.com>
> >
> > This patch modifies the current Hip05/Hip06 PCIe host controller
> > driver to make it ECAM compliant. 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).
> >
> > Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
> > Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
> >
>
> Doesn't this break backwards compatibility?
Well Hip05/Hip06 SoCs are used into evaluation boards
For the Estuary project.
https://github.com/hisilicon/estuary
As this new driver gets upstream we'll merge this new driver into estuary
and release a new version of the firmware to support it.
>
> I think you need to use a new compatible string in the firmware
> if you change the register layout, and then change the driver
> to support both the old and the new layout.
You are right, for some reason in this patchset I missed the Documentation
update that I posted in the previous one, i.e.:
--- a/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
@@ -23,8 +23,8 @@ 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 0x00100000 0 0x0f00000>;
+ reg-names = "rc_dbi", "ecam-cfg";
I will fix this in v3
Thanks
Gab
>
> Arnd
^ permalink raw reply [flat|nested] 63+ messages in thread* [RFC PATCH v2 2/3] PCI: hisi: Make the HiSilicon PCIe host controller ECAM compliant
@ 2016-02-08 15:55 ` Gabriele Paoloni
0 siblings, 0 replies; 63+ messages in thread
From: Gabriele Paoloni @ 2016-02-08 15:55 UTC (permalink / raw)
To: linux-arm-kernel
> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd at arndb.de]
> Sent: 08 February 2016 13:48
> To: linux-arm-kernel at lists.infradead.org
> Cc: Gabriele Paoloni; Guohanjun (Hanjun Guo); Wangzhou (B); liudongdong
> (C); Linuxarm; qiujiang; bhelgaas at google.com;
> Lorenzo.Pieralisi at arm.com; tn at semihalf.com; linux-pci at vger.kernel.org;
> linux-kernel at vger.kernel.org; xuwei (O); linux-acpi at vger.kernel.org;
> jcm at redhat.com; zhangjukuo; Liguozhu (Kenneth)
> Subject: Re: [RFC PATCH v2 2/3] PCI: hisi: Make the HiSilicon PCIe host
> controller ECAM compliant
>
> On Monday 08 February 2016 12:41:03 Gabriele Paoloni wrote:
> > From: gabriele paoloni <gabriele.paoloni@huawei.com>
> >
> > This patch modifies the current Hip05/Hip06 PCIe host controller
> > driver to make it ECAM compliant. 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).
> >
> > Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
> > Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
> >
>
> Doesn't this break backwards compatibility?
Well Hip05/Hip06 SoCs are used into evaluation boards
For the Estuary project.
https://github.com/hisilicon/estuary
As this new driver gets upstream we'll merge this new driver into estuary
and release a new version of the firmware to support it.
>
> I think you need to use a new compatible string in the firmware
> if you change the register layout, and then change the driver
> to support both the old and the new layout.
You are right, for some reason in this patchset I missed the Documentation
update that I posted in the previous one, i.e.:
--- a/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
@@ -23,8 +23,8 @@ Optional properties:
Hip05 Example (note that Hip06 is the same except compatible):
pcie at 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 0x00100000 0 0x0f00000>;
+ reg-names = "rc_dbi", "ecam-cfg";
I will fix this in v3
Thanks
Gab
>
> Arnd
^ permalink raw reply [flat|nested] 63+ messages in thread* RE: [RFC PATCH v2 2/3] PCI: hisi: Make the HiSilicon PCIe host controller ECAM compliant
@ 2016-02-08 15:55 ` Gabriele Paoloni
0 siblings, 0 replies; 63+ messages in thread
From: Gabriele Paoloni @ 2016-02-08 15:55 UTC (permalink / raw)
To: Arnd Bergmann, linux-arm-kernel
Cc: Guohanjun (Hanjun Guo), Wangzhou (B), liudongdong (C),
Linuxarm, qiujiang, bhelgaas, Lorenzo.Pieralisi, tn, linux-pci,
linux-kernel, xuwei (O),
linux-acpi, jcm, zhangjukuo, Liguozhu (Kenneth)
> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@arndb.de]
> Sent: 08 February 2016 13:48
> To: linux-arm-kernel@lists.infradead.org
> Cc: Gabriele Paoloni; Guohanjun (Hanjun Guo); Wangzhou (B); liudongdong
> (C); Linuxarm; qiujiang; bhelgaas@google.com;
> Lorenzo.Pieralisi@arm.com; tn@semihalf.com; linux-pci@vger.kernel.org;
> linux-kernel@vger.kernel.org; xuwei (O); linux-acpi@vger.kernel.org;
> jcm@redhat.com; zhangjukuo; Liguozhu (Kenneth)
> Subject: Re: [RFC PATCH v2 2/3] PCI: hisi: Make the HiSilicon PCIe host
> controller ECAM compliant
>
> On Monday 08 February 2016 12:41:03 Gabriele Paoloni wrote:
> > From: gabriele paoloni <gabriele.paoloni@huawei.com>
> >
> > This patch modifies the current Hip05/Hip06 PCIe host controller
> > driver to make it ECAM compliant. 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).
> >
> > Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
> > Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
> >
>
> Doesn't this break backwards compatibility?
Well Hip05/Hip06 SoCs are used into evaluation boards
For the Estuary project.
https://github.com/hisilicon/estuary
As this new driver gets upstream we'll merge this new driver into estuary
and release a new version of the firmware to support it.
>
> I think you need to use a new compatible string in the firmware
> if you change the register layout, and then change the driver
> to support both the old and the new layout.
You are right, for some reason in this patchset I missed the Documentation
update that I posted in the previous one, i.e.:
--- a/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
@@ -23,8 +23,8 @@ 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 0x00100000 0 0x0f00000>;
+ reg-names = "rc_dbi", "ecam-cfg";
I will fix this in v3
Thanks
Gab
>
> Arnd
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [RFC PATCH v2 2/3] PCI: hisi: Make the HiSilicon PCIe host controller ECAM compliant
2016-02-08 15:55 ` Gabriele Paoloni
(?)
(?)
@ 2016-02-08 16:29 ` Arnd Bergmann
-1 siblings, 0 replies; 63+ messages in thread
From: Arnd Bergmann @ 2016-02-08 16:29 UTC (permalink / raw)
To: Gabriele Paoloni
Cc: Lorenzo.Pieralisi, jcm, tn, linux-pci, Linuxarm, xuwei (O),
linux-kernel, linux-acpi, Wangzhou (B), liudongdong (C),
Guohanjun (Hanjun Guo), bhelgaas, zhangjukuo, Liguozhu (Kenneth),
qiujiang, linux-arm-kernel
On Monday 08 February 2016 15:55:35 Gabriele Paoloni wrote:
> > Doesn't this break backwards compatibility?
>
> Well Hip05/Hip06 SoCs are used into evaluation boards
> For the Estuary project.
>
> https://github.com/hisilicon/estuary
>
> As this new driver gets upstream we'll merge this new driver into estuary
> and release a new version of the firmware to support it.
So what happens to folks running the old firmware then?
> > I think you need to use a new compatible string in the firmware
> > if you change the register layout, and then change the driver
> > to support both the old and the new layout.
>
> You are right, for some reason in this patchset I missed the Documentation
> update that I posted in the previous one, i.e.:
>
> --- a/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
> @@ -23,8 +23,8 @@ 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 0x00100000 0 0x0f00000>;
> + reg-names = "rc_dbi", "ecam-cfg";
>
That is not the compatible string, it's an undocumented register set.
You can either define the a new compatible string that gives the "config"
registers a new meaning, or you change the binding to allow two either
a "config" or an "ecam-cfg" register set, and let the driver handle both.
Arnd
^ permalink raw reply [flat|nested] 63+ messages in thread* [RFC PATCH v2 2/3] PCI: hisi: Make the HiSilicon PCIe host controller ECAM compliant
@ 2016-02-08 16:29 ` Arnd Bergmann
0 siblings, 0 replies; 63+ messages in thread
From: Arnd Bergmann @ 2016-02-08 16:29 UTC (permalink / raw)
To: linux-arm-kernel
On Monday 08 February 2016 15:55:35 Gabriele Paoloni wrote:
> > Doesn't this break backwards compatibility?
>
> Well Hip05/Hip06 SoCs are used into evaluation boards
> For the Estuary project.
>
> https://github.com/hisilicon/estuary
>
> As this new driver gets upstream we'll merge this new driver into estuary
> and release a new version of the firmware to support it.
So what happens to folks running the old firmware then?
> > I think you need to use a new compatible string in the firmware
> > if you change the register layout, and then change the driver
> > to support both the old and the new layout.
>
> You are right, for some reason in this patchset I missed the Documentation
> update that I posted in the previous one, i.e.:
>
> --- a/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
> @@ -23,8 +23,8 @@ Optional properties:
> Hip05 Example (note that Hip06 is the same except compatible):
> pcie at 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 0x00100000 0 0x0f00000>;
> + reg-names = "rc_dbi", "ecam-cfg";
>
That is not the compatible string, it's an undocumented register set.
You can either define the a new compatible string that gives the "config"
registers a new meaning, or you change the binding to allow two either
a "config" or an "ecam-cfg" register set, and let the driver handle both.
Arnd
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [RFC PATCH v2 2/3] PCI: hisi: Make the HiSilicon PCIe host controller ECAM compliant
@ 2016-02-08 16:29 ` Arnd Bergmann
0 siblings, 0 replies; 63+ messages in thread
From: Arnd Bergmann @ 2016-02-08 16:29 UTC (permalink / raw)
To: Gabriele Paoloni
Cc: linux-arm-kernel, Guohanjun (Hanjun Guo), Wangzhou (B),
liudongdong (C),
Linuxarm, qiujiang, bhelgaas, Lorenzo.Pieralisi, tn, linux-pci,
linux-kernel, xuwei (O),
linux-acpi, jcm, zhangjukuo, Liguozhu (Kenneth)
On Monday 08 February 2016 15:55:35 Gabriele Paoloni wrote:
> > Doesn't this break backwards compatibility?
>
> Well Hip05/Hip06 SoCs are used into evaluation boards
> For the Estuary project.
>
> https://github.com/hisilicon/estuary
>
> As this new driver gets upstream we'll merge this new driver into estuary
> and release a new version of the firmware to support it.
So what happens to folks running the old firmware then?
> > I think you need to use a new compatible string in the firmware
> > if you change the register layout, and then change the driver
> > to support both the old and the new layout.
>
> You are right, for some reason in this patchset I missed the Documentation
> update that I posted in the previous one, i.e.:
>
> --- a/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
> @@ -23,8 +23,8 @@ 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 0x00100000 0 0x0f00000>;
> + reg-names = "rc_dbi", "ecam-cfg";
>
That is not the compatible string, it's an undocumented register set.
You can either define the a new compatible string that gives the "config"
registers a new meaning, or you change the binding to allow two either
a "config" or an "ecam-cfg" register set, and let the driver handle both.
Arnd
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [RFC PATCH v2 2/3] PCI: hisi: Make the HiSilicon PCIe host controller ECAM compliant
@ 2016-02-08 16:29 ` Arnd Bergmann
0 siblings, 0 replies; 63+ messages in thread
From: Arnd Bergmann @ 2016-02-08 16:29 UTC (permalink / raw)
To: Gabriele Paoloni
Cc: linux-arm-kernel, Guohanjun (Hanjun Guo), Wangzhou (B),
liudongdong (C),
Linuxarm, qiujiang, bhelgaas, Lorenzo.Pieralisi, tn, linux-pci,
linux-kernel, xuwei (O),
linux-acpi, jcm, zhangjukuo, Liguozhu (Kenneth)
On Monday 08 February 2016 15:55:35 Gabriele Paoloni wrote:
> > Doesn't this break backwards compatibility?
>
> Well Hip05/Hip06 SoCs are used into evaluation boards
> For the Estuary project.
>
> https://github.com/hisilicon/estuary
>
> As this new driver gets upstream we'll merge this new driver into estuary
> and release a new version of the firmware to support it.
So what happens to folks running the old firmware then?
> > I think you need to use a new compatible string in the firmware
> > if you change the register layout, and then change the driver
> > to support both the old and the new layout.
>
> You are right, for some reason in this patchset I missed the Documentation
> update that I posted in the previous one, i.e.:
>
> --- a/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
> @@ -23,8 +23,8 @@ 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 0x00100000 0 0x0f00000>;
> + reg-names = "rc_dbi", "ecam-cfg";
>
That is not the compatible string, it's an undocumented register set.
You can either define the a new compatible string that gives the "config"
registers a new meaning, or you change the binding to allow two either
a "config" or an "ecam-cfg" register set, and let the driver handle both.
Arnd
^ permalink raw reply [flat|nested] 63+ messages in thread* RE: [RFC PATCH v2 2/3] PCI: hisi: Make the HiSilicon PCIe host controller ECAM compliant
2016-02-08 16:29 ` Arnd Bergmann
(?)
@ 2016-02-08 17:21 ` Gabriele Paoloni
-1 siblings, 0 replies; 63+ messages in thread
From: Gabriele Paoloni @ 2016-02-08 17:21 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-arm-kernel, Guohanjun (Hanjun Guo), Wangzhou (B),
liudongdong (C),
Linuxarm, qiujiang, bhelgaas, Lorenzo.Pieralisi, tn, linux-pci,
linux-kernel, xuwei (O),
linux-acpi, jcm, zhangjukuo, Liguozhu (Kenneth)
Hi Arnd
> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@arndb.de]
> Sent: 08 February 2016 16:30
> To: Gabriele Paoloni
> Cc: linux-arm-kernel@lists.infradead.org; Guohanjun (Hanjun Guo);
> Wangzhou (B); liudongdong (C); Linuxarm; qiujiang; bhelgaas@google.com;
> Lorenzo.Pieralisi@arm.com; tn@semihalf.com; linux-pci@vger.kernel.org;
> linux-kernel@vger.kernel.org; xuwei (O); linux-acpi@vger.kernel.org;
> jcm@redhat.com; zhangjukuo; Liguozhu (Kenneth)
> Subject: Re: [RFC PATCH v2 2/3] PCI: hisi: Make the HiSilicon PCIe host
> controller ECAM compliant
>
> On Monday 08 February 2016 15:55:35 Gabriele Paoloni wrote:
> > > Doesn't this break backwards compatibility?
> >
> > Well Hip05/Hip06 SoCs are used into evaluation boards
> > For the Estuary project.
> >
> > https://github.com/hisilicon/estuary
> >
> > As this new driver gets upstream we'll merge this new driver into
> estuary
> > and release a new version of the firmware to support it.
>
> So what happens to folks running the old firmware then?
So far we haven't released the PCIe nodes dtsi upstream and we think
we can handle the firmware upgrade for our current users either
by pointing them to the updated Estuary branch or by internal
channels
>
> > > I think you need to use a new compatible string in the firmware
> > > if you change the register layout, and then change the driver
> > > to support both the old and the new layout.
> >
> > You are right, for some reason in this patchset I missed the
> Documentation
> > update that I posted in the previous one, i.e.:
> >
> > --- a/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
> > +++ b/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
> > @@ -23,8 +23,8 @@ 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 0x00100000 0
> 0x0f00000>;
> > + reg-names = "rc_dbi", "ecam-cfg";
> >
>
> That is not the compatible string, it's an undocumented register set.
Sorry, I misunderstood here, got it now :)
> You can either define the a new compatible string that gives the
> "config"
> registers a new meaning, or you change the binding to allow two either
> a "config" or an "ecam-cfg" register set, and let the driver handle
> both.
As per reply above I think that it would be quite easy for us to
let out current users update the BIOS so I don't think there is any
value in maintaining two version of the drivers one of which will
not be used.
What do you think?
Thanks
Gab
>
> Arnd
^ permalink raw reply [flat|nested] 63+ messages in thread* [RFC PATCH v2 2/3] PCI: hisi: Make the HiSilicon PCIe host controller ECAM compliant
@ 2016-02-08 17:21 ` Gabriele Paoloni
0 siblings, 0 replies; 63+ messages in thread
From: Gabriele Paoloni @ 2016-02-08 17:21 UTC (permalink / raw)
To: linux-arm-kernel
Hi Arnd
> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd at arndb.de]
> Sent: 08 February 2016 16:30
> To: Gabriele Paoloni
> Cc: linux-arm-kernel at lists.infradead.org; Guohanjun (Hanjun Guo);
> Wangzhou (B); liudongdong (C); Linuxarm; qiujiang; bhelgaas at google.com;
> Lorenzo.Pieralisi at arm.com; tn at semihalf.com; linux-pci at vger.kernel.org;
> linux-kernel at vger.kernel.org; xuwei (O); linux-acpi at vger.kernel.org;
> jcm at redhat.com; zhangjukuo; Liguozhu (Kenneth)
> Subject: Re: [RFC PATCH v2 2/3] PCI: hisi: Make the HiSilicon PCIe host
> controller ECAM compliant
>
> On Monday 08 February 2016 15:55:35 Gabriele Paoloni wrote:
> > > Doesn't this break backwards compatibility?
> >
> > Well Hip05/Hip06 SoCs are used into evaluation boards
> > For the Estuary project.
> >
> > https://github.com/hisilicon/estuary
> >
> > As this new driver gets upstream we'll merge this new driver into
> estuary
> > and release a new version of the firmware to support it.
>
> So what happens to folks running the old firmware then?
So far we haven't released the PCIe nodes dtsi upstream and we think
we can handle the firmware upgrade for our current users either
by pointing them to the updated Estuary branch or by internal
channels
>
> > > I think you need to use a new compatible string in the firmware
> > > if you change the register layout, and then change the driver
> > > to support both the old and the new layout.
> >
> > You are right, for some reason in this patchset I missed the
> Documentation
> > update that I posted in the previous one, i.e.:
> >
> > --- a/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
> > +++ b/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
> > @@ -23,8 +23,8 @@ Optional properties:
> > Hip05 Example (note that Hip06 is the same except compatible):
> > pcie at 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 0x00100000 0
> 0x0f00000>;
> > + reg-names = "rc_dbi", "ecam-cfg";
> >
>
> That is not the compatible string, it's an undocumented register set.
Sorry, I misunderstood here, got it now :)
> You can either define the a new compatible string that gives the
> "config"
> registers a new meaning, or you change the binding to allow two either
> a "config" or an "ecam-cfg" register set, and let the driver handle
> both.
As per reply above I think that it would be quite easy for us to
let out current users update the BIOS so I don't think there is any
value in maintaining two version of the drivers one of which will
not be used.
What do you think?
Thanks
Gab
>
> Arnd
^ permalink raw reply [flat|nested] 63+ messages in thread* RE: [RFC PATCH v2 2/3] PCI: hisi: Make the HiSilicon PCIe host controller ECAM compliant
@ 2016-02-08 17:21 ` Gabriele Paoloni
0 siblings, 0 replies; 63+ messages in thread
From: Gabriele Paoloni @ 2016-02-08 17:21 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-arm-kernel, Guohanjun (Hanjun Guo), Wangzhou (B),
liudongdong (C),
Linuxarm, qiujiang, bhelgaas, Lorenzo.Pieralisi, tn, linux-pci,
linux-kernel, xuwei (O),
linux-acpi, jcm, zhangjukuo, Liguozhu (Kenneth)
Hi Arnd
> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@arndb.de]
> Sent: 08 February 2016 16:30
> To: Gabriele Paoloni
> Cc: linux-arm-kernel@lists.infradead.org; Guohanjun (Hanjun Guo);
> Wangzhou (B); liudongdong (C); Linuxarm; qiujiang; bhelgaas@google.com;
> Lorenzo.Pieralisi@arm.com; tn@semihalf.com; linux-pci@vger.kernel.org;
> linux-kernel@vger.kernel.org; xuwei (O); linux-acpi@vger.kernel.org;
> jcm@redhat.com; zhangjukuo; Liguozhu (Kenneth)
> Subject: Re: [RFC PATCH v2 2/3] PCI: hisi: Make the HiSilicon PCIe host
> controller ECAM compliant
>
> On Monday 08 February 2016 15:55:35 Gabriele Paoloni wrote:
> > > Doesn't this break backwards compatibility?
> >
> > Well Hip05/Hip06 SoCs are used into evaluation boards
> > For the Estuary project.
> >
> > https://github.com/hisilicon/estuary
> >
> > As this new driver gets upstream we'll merge this new driver into
> estuary
> > and release a new version of the firmware to support it.
>
> So what happens to folks running the old firmware then?
So far we haven't released the PCIe nodes dtsi upstream and we think
we can handle the firmware upgrade for our current users either
by pointing them to the updated Estuary branch or by internal
channels
>
> > > I think you need to use a new compatible string in the firmware
> > > if you change the register layout, and then change the driver
> > > to support both the old and the new layout.
> >
> > You are right, for some reason in this patchset I missed the
> Documentation
> > update that I posted in the previous one, i.e.:
> >
> > --- a/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
> > +++ b/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
> > @@ -23,8 +23,8 @@ 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 0x00100000 0
> 0x0f00000>;
> > + reg-names = "rc_dbi", "ecam-cfg";
> >
>
> That is not the compatible string, it's an undocumented register set.
Sorry, I misunderstood here, got it now :)
> You can either define the a new compatible string that gives the
> "config"
> registers a new meaning, or you change the binding to allow two either
> a "config" or an "ecam-cfg" register set, and let the driver handle
> both.
As per reply above I think that it would be quite easy for us to
let out current users update the BIOS so I don't think there is any
value in maintaining two version of the drivers one of which will
not be used.
What do you think?
Thanks
Gab
>
> Arnd
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [RFC PATCH v2 2/3] PCI: hisi: Make the HiSilicon PCIe host controller ECAM compliant
2016-02-08 17:21 ` Gabriele Paoloni
@ 2016-02-09 15:32 ` Arnd Bergmann
-1 siblings, 0 replies; 63+ messages in thread
From: Arnd Bergmann @ 2016-02-09 15:32 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Gabriele Paoloni, Lorenzo.Pieralisi, jcm, tn, linux-pci,
Linuxarm, xuwei (O), linux-kernel, linux-acpi, Wangzhou (B),
liudongdong (C), Guohanjun (Hanjun Guo),
bhelgaas, zhangjukuo, Liguozhu (Kenneth),
qiujiang
On Monday 08 February 2016 17:21:27 Gabriele Paoloni wrote:
> Hi Arnd
>
> > -----Original Message-----
> > From: Arnd Bergmann [mailto:arnd@arndb.de]
> > Sent: 08 February 2016 16:30
> > To: Gabriele Paoloni
> > Cc: linux-arm-kernel@lists.infradead.org; Guohanjun (Hanjun Guo);
> > Wangzhou (B); liudongdong (C); Linuxarm; qiujiang; bhelgaas@google.com;
> > Lorenzo.Pieralisi@arm.com; tn@semihalf.com; linux-pci@vger.kernel.org;
> > linux-kernel@vger.kernel.org; xuwei (O); linux-acpi@vger.kernel.org;
> > jcm@redhat.com; zhangjukuo; Liguozhu (Kenneth)
> > Subject: Re: [RFC PATCH v2 2/3] PCI: hisi: Make the HiSilicon PCIe host
> > controller ECAM compliant
> >
> > On Monday 08 February 2016 15:55:35 Gabriele Paoloni wrote:
> > > > Doesn't this break backwards compatibility?
> > >
> > > Well Hip05/Hip06 SoCs are used into evaluation boards
> > > For the Estuary project.
> > >
> > > https://github.com/hisilicon/estuary
> > >
> > > As this new driver gets upstream we'll merge this new driver into
> > estuary
> > > and release a new version of the firmware to support it.
> >
> > So what happens to folks running the old firmware then?
>
> So far we haven't released the PCIe nodes dtsi upstream and we think
> we can handle the firmware upgrade for our current users either
> by pointing them to the updated Estuary branch or by internal
> channels
The dts files in the kernel tree are not really that important,
the question is whether you are breaking things for real users.
If the upstream drivers work on some released firmware version,
they should keep working.
> >
> > > > I think you need to use a new compatible string in the firmware
> > > > if you change the register layout, and then change the driver
> > > > to support both the old and the new layout.
> > >
> > > You are right, for some reason in this patchset I missed the
> > Documentation
> > > update that I posted in the previous one, i.e.:
> > >
> > > --- a/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
> > > +++ b/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
> > > @@ -23,8 +23,8 @@ 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 0x00100000 0
> > 0x0f00000>;
> > > + reg-names = "rc_dbi", "ecam-cfg";
> > >
> >
> > That is not the compatible string, it's an undocumented register set.
>
> Sorry, I misunderstood here, got it now :)
>
> > You can either define the a new compatible string that gives the
> > "config"
> > registers a new meaning, or you change the binding to allow two either
> > a "config" or an "ecam-cfg" register set, and let the driver handle
> > both.
>
> As per reply above I think that it would be quite easy for us to
> let out current users update the BIOS so I don't think there is any
> value in maintaining two version of the drivers one of which will
> not be used.
>
> What do you think?
Requiring a BIOS update for a kernel update is really bad, even more
so when the same BIOS update breaks older kernels.
Arnd
^ permalink raw reply [flat|nested] 63+ messages in thread* [RFC PATCH v2 2/3] PCI: hisi: Make the HiSilicon PCIe host controller ECAM compliant
@ 2016-02-09 15:32 ` Arnd Bergmann
0 siblings, 0 replies; 63+ messages in thread
From: Arnd Bergmann @ 2016-02-09 15:32 UTC (permalink / raw)
To: linux-arm-kernel
On Monday 08 February 2016 17:21:27 Gabriele Paoloni wrote:
> Hi Arnd
>
> > -----Original Message-----
> > From: Arnd Bergmann [mailto:arnd at arndb.de]
> > Sent: 08 February 2016 16:30
> > To: Gabriele Paoloni
> > Cc: linux-arm-kernel at lists.infradead.org; Guohanjun (Hanjun Guo);
> > Wangzhou (B); liudongdong (C); Linuxarm; qiujiang; bhelgaas at google.com;
> > Lorenzo.Pieralisi at arm.com; tn at semihalf.com; linux-pci at vger.kernel.org;
> > linux-kernel at vger.kernel.org; xuwei (O); linux-acpi at vger.kernel.org;
> > jcm at redhat.com; zhangjukuo; Liguozhu (Kenneth)
> > Subject: Re: [RFC PATCH v2 2/3] PCI: hisi: Make the HiSilicon PCIe host
> > controller ECAM compliant
> >
> > On Monday 08 February 2016 15:55:35 Gabriele Paoloni wrote:
> > > > Doesn't this break backwards compatibility?
> > >
> > > Well Hip05/Hip06 SoCs are used into evaluation boards
> > > For the Estuary project.
> > >
> > > https://github.com/hisilicon/estuary
> > >
> > > As this new driver gets upstream we'll merge this new driver into
> > estuary
> > > and release a new version of the firmware to support it.
> >
> > So what happens to folks running the old firmware then?
>
> So far we haven't released the PCIe nodes dtsi upstream and we think
> we can handle the firmware upgrade for our current users either
> by pointing them to the updated Estuary branch or by internal
> channels
The dts files in the kernel tree are not really that important,
the question is whether you are breaking things for real users.
If the upstream drivers work on some released firmware version,
they should keep working.
> >
> > > > I think you need to use a new compatible string in the firmware
> > > > if you change the register layout, and then change the driver
> > > > to support both the old and the new layout.
> > >
> > > You are right, for some reason in this patchset I missed the
> > Documentation
> > > update that I posted in the previous one, i.e.:
> > >
> > > --- a/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
> > > +++ b/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
> > > @@ -23,8 +23,8 @@ Optional properties:
> > > Hip05 Example (note that Hip06 is the same except compatible):
> > > pcie at 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 0x00100000 0
> > 0x0f00000>;
> > > + reg-names = "rc_dbi", "ecam-cfg";
> > >
> >
> > That is not the compatible string, it's an undocumented register set.
>
> Sorry, I misunderstood here, got it now :)
>
> > You can either define the a new compatible string that gives the
> > "config"
> > registers a new meaning, or you change the binding to allow two either
> > a "config" or an "ecam-cfg" register set, and let the driver handle
> > both.
>
> As per reply above I think that it would be quite easy for us to
> let out current users update the BIOS so I don't think there is any
> value in maintaining two version of the drivers one of which will
> not be used.
>
> What do you think?
Requiring a BIOS update for a kernel update is really bad, even more
so when the same BIOS update breaks older kernels.
Arnd
^ permalink raw reply [flat|nested] 63+ messages in thread* RE: [RFC PATCH v2 2/3] PCI: hisi: Make the HiSilicon PCIe host controller ECAM compliant
2016-02-09 15:32 ` Arnd Bergmann
(?)
@ 2016-02-09 15:56 ` Gabriele Paoloni
-1 siblings, 0 replies; 63+ messages in thread
From: Gabriele Paoloni @ 2016-02-09 15:56 UTC (permalink / raw)
To: Arnd Bergmann, linux-arm-kernel
Cc: Lorenzo.Pieralisi, jcm, tn, linux-pci, Linuxarm, xuwei (O),
linux-kernel, linux-acpi, Wangzhou (B), liudongdong (C),
Guohanjun (Hanjun Guo), bhelgaas, zhangjukuo, Liguozhu (Kenneth),
qiujiang
> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@arndb.de]
> Sent: 09 February 2016 15:32
> To: linux-arm-kernel@lists.infradead.org
> Cc: Gabriele Paoloni; Lorenzo.Pieralisi@arm.com; jcm@redhat.com;
> tn@semihalf.com; linux-pci@vger.kernel.org; Linuxarm; xuwei (O); linux-
> kernel@vger.kernel.org; linux-acpi@vger.kernel.org; Wangzhou (B);
> liudongdong (C); Guohanjun (Hanjun Guo); bhelgaas@google.com;
> zhangjukuo; Liguozhu (Kenneth); qiujiang
> Subject: Re: [RFC PATCH v2 2/3] PCI: hisi: Make the HiSilicon PCIe host
> controller ECAM compliant
>
> On Monday 08 February 2016 17:21:27 Gabriele Paoloni wrote:
> > Hi Arnd
> >
> > > -----Original Message-----
> > > From: Arnd Bergmann [mailto:arnd@arndb.de]
> > > Sent: 08 February 2016 16:30
> > > To: Gabriele Paoloni
> > > Cc: linux-arm-kernel@lists.infradead.org; Guohanjun (Hanjun Guo);
> > > Wangzhou (B); liudongdong (C); Linuxarm; qiujiang;
> bhelgaas@google.com;
> > > Lorenzo.Pieralisi@arm.com; tn@semihalf.com; linux-
> pci@vger.kernel.org;
> > > linux-kernel@vger.kernel.org; xuwei (O); linux-acpi@vger.kernel.org;
> > > jcm@redhat.com; zhangjukuo; Liguozhu (Kenneth)
> > > Subject: Re: [RFC PATCH v2 2/3] PCI: hisi: Make the HiSilicon PCIe
> host
> > > controller ECAM compliant
> > >
> > > On Monday 08 February 2016 15:55:35 Gabriele Paoloni wrote:
> > > > > Doesn't this break backwards compatibility?
> > > >
> > > > Well Hip05/Hip06 SoCs are used into evaluation boards
> > > > For the Estuary project.
> > > >
> > > > https://github.com/hisilicon/estuary
> > > >
> > > > As this new driver gets upstream we'll merge this new driver into
> > > estuary
> > > > and release a new version of the firmware to support it.
> > >
> > > So what happens to folks running the old firmware then?
> >
> > So far we haven't released the PCIe nodes dtsi upstream and we think
> > we can handle the firmware upgrade for our current users either
> > by pointing them to the updated Estuary branch or by internal
> > channels
>
> The dts files in the kernel tree are not really that important,
> the question is whether you are breaking things for real users.
>
> If the upstream drivers work on some released firmware version,
> they should keep working.
>
> > >
> > > > > I think you need to use a new compatible string in the firmware
> > > > > if you change the register layout, and then change the driver
> > > > > to support both the old and the new layout.
> > > >
> > > > You are right, for some reason in this patchset I missed the
> > > Documentation
> > > > update that I posted in the previous one, i.e.:
> > > >
> > > > --- a/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
> > > > +++ b/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
> > > > @@ -23,8 +23,8 @@ 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 0x00100000
> 0
> > > 0x0f00000>;
> > > > + reg-names = "rc_dbi", "ecam-cfg";
> > > >
> > >
> > > That is not the compatible string, it's an undocumented register
> set.
> >
> > Sorry, I misunderstood here, got it now :)
> >
> > > You can either define the a new compatible string that gives the
> > > "config"
> > > registers a new meaning, or you change the binding to allow two
> either
> > > a "config" or an "ecam-cfg" register set, and let the driver handle
> > > both.
> >
> > As per reply above I think that it would be quite easy for us to
> > let out current users update the BIOS so I don't think there is any
> > value in maintaining two version of the drivers one of which will
> > not be used.
> >
> > What do you think?
>
> Requiring a BIOS update for a kernel update is really bad, even more
> so when the same BIOS update breaks older kernels.
Well I see your point.
To be honest I won't expect many users of the old BIOS version since
as I said we plan to send out the BIOS update to pretty much every user,
however on the other side it is not too much effort for me to add code
to support both ECAM/non ECAM,
so ok I'll fix this in v3,
Many Thanks
Gab
>
> Arnd
^ permalink raw reply [flat|nested] 63+ messages in thread* [RFC PATCH v2 2/3] PCI: hisi: Make the HiSilicon PCIe host controller ECAM compliant
@ 2016-02-09 15:56 ` Gabriele Paoloni
0 siblings, 0 replies; 63+ messages in thread
From: Gabriele Paoloni @ 2016-02-09 15:56 UTC (permalink / raw)
To: linux-arm-kernel
> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd at arndb.de]
> Sent: 09 February 2016 15:32
> To: linux-arm-kernel at lists.infradead.org
> Cc: Gabriele Paoloni; Lorenzo.Pieralisi at arm.com; jcm at redhat.com;
> tn at semihalf.com; linux-pci at vger.kernel.org; Linuxarm; xuwei (O); linux-
> kernel at vger.kernel.org; linux-acpi at vger.kernel.org; Wangzhou (B);
> liudongdong (C); Guohanjun (Hanjun Guo); bhelgaas at google.com;
> zhangjukuo; Liguozhu (Kenneth); qiujiang
> Subject: Re: [RFC PATCH v2 2/3] PCI: hisi: Make the HiSilicon PCIe host
> controller ECAM compliant
>
> On Monday 08 February 2016 17:21:27 Gabriele Paoloni wrote:
> > Hi Arnd
> >
> > > -----Original Message-----
> > > From: Arnd Bergmann [mailto:arnd at arndb.de]
> > > Sent: 08 February 2016 16:30
> > > To: Gabriele Paoloni
> > > Cc: linux-arm-kernel at lists.infradead.org; Guohanjun (Hanjun Guo);
> > > Wangzhou (B); liudongdong (C); Linuxarm; qiujiang;
> bhelgaas at google.com;
> > > Lorenzo.Pieralisi at arm.com; tn at semihalf.com; linux-
> pci at vger.kernel.org;
> > > linux-kernel at vger.kernel.org; xuwei (O); linux-acpi at vger.kernel.org;
> > > jcm at redhat.com; zhangjukuo; Liguozhu (Kenneth)
> > > Subject: Re: [RFC PATCH v2 2/3] PCI: hisi: Make the HiSilicon PCIe
> host
> > > controller ECAM compliant
> > >
> > > On Monday 08 February 2016 15:55:35 Gabriele Paoloni wrote:
> > > > > Doesn't this break backwards compatibility?
> > > >
> > > > Well Hip05/Hip06 SoCs are used into evaluation boards
> > > > For the Estuary project.
> > > >
> > > > https://github.com/hisilicon/estuary
> > > >
> > > > As this new driver gets upstream we'll merge this new driver into
> > > estuary
> > > > and release a new version of the firmware to support it.
> > >
> > > So what happens to folks running the old firmware then?
> >
> > So far we haven't released the PCIe nodes dtsi upstream and we think
> > we can handle the firmware upgrade for our current users either
> > by pointing them to the updated Estuary branch or by internal
> > channels
>
> The dts files in the kernel tree are not really that important,
> the question is whether you are breaking things for real users.
>
> If the upstream drivers work on some released firmware version,
> they should keep working.
>
> > >
> > > > > I think you need to use a new compatible string in the firmware
> > > > > if you change the register layout, and then change the driver
> > > > > to support both the old and the new layout.
> > > >
> > > > You are right, for some reason in this patchset I missed the
> > > Documentation
> > > > update that I posted in the previous one, i.e.:
> > > >
> > > > --- a/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
> > > > +++ b/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
> > > > @@ -23,8 +23,8 @@ Optional properties:
> > > > Hip05 Example (note that Hip06 is the same except compatible):
> > > > pcie at 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 0x00100000
> 0
> > > 0x0f00000>;
> > > > + reg-names = "rc_dbi", "ecam-cfg";
> > > >
> > >
> > > That is not the compatible string, it's an undocumented register
> set.
> >
> > Sorry, I misunderstood here, got it now :)
> >
> > > You can either define the a new compatible string that gives the
> > > "config"
> > > registers a new meaning, or you change the binding to allow two
> either
> > > a "config" or an "ecam-cfg" register set, and let the driver handle
> > > both.
> >
> > As per reply above I think that it would be quite easy for us to
> > let out current users update the BIOS so I don't think there is any
> > value in maintaining two version of the drivers one of which will
> > not be used.
> >
> > What do you think?
>
> Requiring a BIOS update for a kernel update is really bad, even more
> so when the same BIOS update breaks older kernels.
Well I see your point.
To be honest I won't expect many users of the old BIOS version since
as I said we plan to send out the BIOS update to pretty much every user,
however on the other side it is not too much effort for me to add code
to support both ECAM/non ECAM,
so ok I'll fix this in v3,
Many Thanks
Gab
>
> Arnd
^ permalink raw reply [flat|nested] 63+ messages in thread* RE: [RFC PATCH v2 2/3] PCI: hisi: Make the HiSilicon PCIe host controller ECAM compliant
@ 2016-02-09 15:56 ` Gabriele Paoloni
0 siblings, 0 replies; 63+ messages in thread
From: Gabriele Paoloni @ 2016-02-09 15:56 UTC (permalink / raw)
To: Arnd Bergmann, linux-arm-kernel
Cc: Lorenzo.Pieralisi, jcm, tn, linux-pci, Linuxarm, xuwei (O),
linux-kernel, linux-acpi, Wangzhou (B), liudongdong (C),
Guohanjun (Hanjun Guo), bhelgaas, zhangjukuo, Liguozhu (Kenneth),
qiujiang
> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@arndb.de]
> Sent: 09 February 2016 15:32
> To: linux-arm-kernel@lists.infradead.org
> Cc: Gabriele Paoloni; Lorenzo.Pieralisi@arm.com; jcm@redhat.com;
> tn@semihalf.com; linux-pci@vger.kernel.org; Linuxarm; xuwei (O); linux-
> kernel@vger.kernel.org; linux-acpi@vger.kernel.org; Wangzhou (B);
> liudongdong (C); Guohanjun (Hanjun Guo); bhelgaas@google.com;
> zhangjukuo; Liguozhu (Kenneth); qiujiang
> Subject: Re: [RFC PATCH v2 2/3] PCI: hisi: Make the HiSilicon PCIe host
> controller ECAM compliant
>
> On Monday 08 February 2016 17:21:27 Gabriele Paoloni wrote:
> > Hi Arnd
> >
> > > -----Original Message-----
> > > From: Arnd Bergmann [mailto:arnd@arndb.de]
> > > Sent: 08 February 2016 16:30
> > > To: Gabriele Paoloni
> > > Cc: linux-arm-kernel@lists.infradead.org; Guohanjun (Hanjun Guo);
> > > Wangzhou (B); liudongdong (C); Linuxarm; qiujiang;
> bhelgaas@google.com;
> > > Lorenzo.Pieralisi@arm.com; tn@semihalf.com; linux-
> pci@vger.kernel.org;
> > > linux-kernel@vger.kernel.org; xuwei (O); linux-acpi@vger.kernel.org;
> > > jcm@redhat.com; zhangjukuo; Liguozhu (Kenneth)
> > > Subject: Re: [RFC PATCH v2 2/3] PCI: hisi: Make the HiSilicon PCIe
> host
> > > controller ECAM compliant
> > >
> > > On Monday 08 February 2016 15:55:35 Gabriele Paoloni wrote:
> > > > > Doesn't this break backwards compatibility?
> > > >
> > > > Well Hip05/Hip06 SoCs are used into evaluation boards
> > > > For the Estuary project.
> > > >
> > > > https://github.com/hisilicon/estuary
> > > >
> > > > As this new driver gets upstream we'll merge this new driver into
> > > estuary
> > > > and release a new version of the firmware to support it.
> > >
> > > So what happens to folks running the old firmware then?
> >
> > So far we haven't released the PCIe nodes dtsi upstream and we think
> > we can handle the firmware upgrade for our current users either
> > by pointing them to the updated Estuary branch or by internal
> > channels
>
> The dts files in the kernel tree are not really that important,
> the question is whether you are breaking things for real users.
>
> If the upstream drivers work on some released firmware version,
> they should keep working.
>
> > >
> > > > > I think you need to use a new compatible string in the firmware
> > > > > if you change the register layout, and then change the driver
> > > > > to support both the old and the new layout.
> > > >
> > > > You are right, for some reason in this patchset I missed the
> > > Documentation
> > > > update that I posted in the previous one, i.e.:
> > > >
> > > > --- a/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
> > > > +++ b/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
> > > > @@ -23,8 +23,8 @@ 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 0x00100000
> 0
> > > 0x0f00000>;
> > > > + reg-names = "rc_dbi", "ecam-cfg";
> > > >
> > >
> > > That is not the compatible string, it's an undocumented register
> set.
> >
> > Sorry, I misunderstood here, got it now :)
> >
> > > You can either define the a new compatible string that gives the
> > > "config"
> > > registers a new meaning, or you change the binding to allow two
> either
> > > a "config" or an "ecam-cfg" register set, and let the driver handle
> > > both.
> >
> > As per reply above I think that it would be quite easy for us to
> > let out current users update the BIOS so I don't think there is any
> > value in maintaining two version of the drivers one of which will
> > not be used.
> >
> > What do you think?
>
> Requiring a BIOS update for a kernel update is really bad, even more
> so when the same BIOS update breaks older kernels.
Well I see your point.
To be honest I won't expect many users of the old BIOS version since
as I said we plan to send out the BIOS update to pretty much every user,
however on the other side it is not too much effort for me to add code
to support both ECAM/non ECAM,
so ok I'll fix this in v3,
Many Thanks
Gab
>
> Arnd
^ permalink raw reply [flat|nested] 63+ messages in thread
* [RFC PATCH v2 3/3] PCI/ACPI: hisi: Add ACPI support for HiSilicon SoCs Host Controllers
2016-02-08 12:41 ` Gabriele Paoloni
(?)
@ 2016-02-08 12:41 ` Gabriele Paoloni
-1 siblings, 0 replies; 63+ messages in thread
From: Gabriele Paoloni @ 2016-02-08 12:41 UTC (permalink / raw)
To: guohanjun, wangzhou1, liudongdong3, linuxarm, qiujiang, bhelgaas,
arnd, Lorenzo.Pieralisi, tn
Cc: gabriele.paoloni, zhangjukuo, xuwei5, liguozhu, linux-pci,
linux-arm-kernel, linux-acpi, linux-kernel, jcm
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", ®_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
^ permalink raw reply related [flat|nested] 63+ messages in thread* [RFC PATCH v2 3/3] PCI/ACPI: hisi: Add ACPI support for HiSilicon SoCs Host Controllers
@ 2016-02-08 12:41 ` Gabriele Paoloni
0 siblings, 0 replies; 63+ messages in thread
From: Gabriele Paoloni @ 2016-02-08 12:41 UTC (permalink / raw)
To: linux-arm-kernel
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", ®_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
^ permalink raw reply related [flat|nested] 63+ messages in thread* [RFC PATCH v2 3/3] PCI/ACPI: hisi: Add ACPI support for HiSilicon SoCs Host Controllers
@ 2016-02-08 12:41 ` Gabriele Paoloni
0 siblings, 0 replies; 63+ messages in thread
From: Gabriele Paoloni @ 2016-02-08 12:41 UTC (permalink / raw)
To: guohanjun, wangzhou1, liudongdong3, linuxarm, qiujiang, bhelgaas,
arnd, Lorenzo.Pieralisi, tn
Cc: gabriele.paoloni, zhangjukuo, xuwei5, liguozhu, linux-pci,
linux-arm-kernel, linux-acpi, linux-kernel, jcm
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", ®_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
^ permalink raw reply related [flat|nested] 63+ messages in thread
* [RFC PATCH V2 1/3] PCI: hisi: re-architect Hip05/Hip06 controllers driver to preapare for ACPI
2016-08-31 11:48 [RFC PATCH V2 0/3] Add ACPI support for Hisilicon PCIe Host Controller Dongdong Liu
@ 2016-08-31 11:48 ` Dongdong Liu
0 siblings, 0 replies; 63+ messages in thread
From: Dongdong Liu @ 2016-08-31 11:48 UTC (permalink / raw)
To: helgaas, arnd, rafael, Lorenzo.Pieralisi, tn, wangzhou1, pratyush.anand
Cc: linux-pci, linux-acpi, linux-kernel, jcm, liudongdong3,
gabriele.paoloni, charles.chenxin, hanjun.guo, linuxarm
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 = ®_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 = ®_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 = ®_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 = ®_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
^ permalink raw reply related [flat|nested] 63+ messages in thread* [RFC PATCH V2 1/3] PCI: hisi: re-architect Hip05/Hip06 controllers driver to preapare for ACPI
@ 2016-08-31 11:48 ` Dongdong Liu
0 siblings, 0 replies; 63+ messages in thread
From: Dongdong Liu @ 2016-08-31 11:48 UTC (permalink / raw)
To: helgaas, arnd, rafael, Lorenzo.Pieralisi, tn, wangzhou1, pratyush.anand
Cc: linux-pci, linux-acpi, linux-kernel, jcm, liudongdong3,
gabriele.paoloni, charles.chenxin, hanjun.guo, linuxarm
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 = ®_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 = ®_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 = ®_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 = ®_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
^ permalink raw reply related [flat|nested] 63+ messages in thread* Re: [RFC PATCH V2 1/3] PCI: hisi: re-architect Hip05/Hip06 controllers driver to preapare for ACPI
2016-08-31 11:48 ` Dongdong Liu
(?)
@ 2016-08-31 11:45 ` Arnd Bergmann
2016-09-01 2:05 ` Dongdong Liu
-1 siblings, 1 reply; 63+ messages in thread
From: Arnd Bergmann @ 2016-08-31 11:45 UTC (permalink / raw)
To: Dongdong Liu
Cc: helgaas, rafael, Lorenzo.Pieralisi, tn, wangzhou1,
pratyush.anand, linux-pci, linux-acpi, linux-kernel, jcm,
gabriele.paoloni, charles.chenxin, hanjun.guo, linuxarm
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 = ®_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
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [RFC PATCH V2 1/3] PCI: hisi: re-architect Hip05/Hip06 controllers driver to preapare for ACPI
2016-08-31 11:45 ` Arnd Bergmann
@ 2016-09-01 2:05 ` Dongdong Liu
0 siblings, 0 replies; 63+ messages in thread
From: Dongdong Liu @ 2016-09-01 2:05 UTC (permalink / raw)
To: Arnd Bergmann
Cc: helgaas, rafael, Lorenzo.Pieralisi, tn, wangzhou1,
pratyush.anand, linux-pci, linux-acpi, linux-kernel, jcm,
gabriele.paoloni, charles.chenxin, hanjun.guo, linuxarm
在 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 = ®_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
>
> .
>
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [RFC PATCH V2 1/3] PCI: hisi: re-architect Hip05/Hip06 controllers driver to preapare for ACPI
@ 2016-09-01 2:05 ` Dongdong Liu
0 siblings, 0 replies; 63+ messages in thread
From: Dongdong Liu @ 2016-09-01 2:05 UTC (permalink / raw)
To: Arnd Bergmann
Cc: helgaas, rafael, Lorenzo.Pieralisi, tn, wangzhou1,
pratyush.anand, linux-pci, linux-acpi, linux-kernel, jcm,
gabriele.paoloni, charles.chenxin, hanjun.guo, linuxarm
在 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 = ®_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
>
> .
>
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [RFC PATCH V2 1/3] PCI: hisi: re-architect Hip05/Hip06 controllers driver to preapare for ACPI
2016-09-01 2:05 ` Dongdong Liu
@ 2016-09-01 7:41 ` Arnd Bergmann
-1 siblings, 0 replies; 63+ messages in thread
From: Arnd Bergmann @ 2016-09-01 7:41 UTC (permalink / raw)
To: Dongdong Liu
Cc: helgaas, rafael, Lorenzo.Pieralisi, tn, wangzhou1,
pratyush.anand, linux-pci, linux-acpi, linux-kernel, jcm,
gabriele.paoloni, charles.chenxin, hanjun.guo, linuxarm
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 = ®_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
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [RFC PATCH V2 1/3] PCI: hisi: re-architect Hip05/Hip06 controllers driver to preapare for ACPI
@ 2016-09-01 7:41 ` Arnd Bergmann
0 siblings, 0 replies; 63+ messages in thread
From: Arnd Bergmann @ 2016-09-01 7:41 UTC (permalink / raw)
To: Dongdong Liu
Cc: helgaas, rafael, Lorenzo.Pieralisi, tn, wangzhou1,
pratyush.anand, linux-pci, linux-acpi, linux-kernel, jcm,
gabriele.paoloni, charles.chenxin, hanjun.guo, linuxarm
On Thursday, September 1, 2016 10:05:29 AM CEST Dongdong Liu wrote:
>=20
> =E5=9C=A8 2016/8/31 19:45, Arnd Bergmann =E5=86=99=E9=81=93:
> > 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 =3D ®_val;
> >> +
> >> + walker +=3D (where & 0x3);
> >> + reg =3D where & ~0x3;
> >> + reg_val =3D readl(reg_base + reg);
> >> +
> >> + if (size =3D=3D 1)
> >> + *val =3D *(u8 __force *) walker;
> >> + else if (size =3D=3D 2)
> >> + *val =3D *(u16 __force *) walker;
> >
> > What is the __force for?
>=20
> Hi Arnd, thanks for replying.
>=20
> __force is used to, well, force a conversion, like casting from or to a b=
itwise 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 =3D=3D 4)
> >> + *val =3D reg_val;
> >> + else
> >> + return PCIBIOS_BAD_REGISTER_NUMBER;
> >> +
> >> + return PCIBIOS_SUCCESSFUL;
> >
> > It looks like you are reimplementing pci_generic_config_read32/pci_gene=
ric_config_write32
> > read here, better use them directly.
> >
>=20
> 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.
>=20
> 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.
>=20
> /* 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;
>=20
> addr =3D reg_base + (where & ~0x3);
> *val =3D readl(addr);
>=20
> if (size <=3D 2)
> *val =3D (*val >> (8 * (where & 3))) & ((1 << (size * 8)) - 1);
>=20
> 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
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [RFC PATCH V2 1/3] PCI: hisi: re-architect Hip05/Hip06 controllers driver to preapare for ACPI
2016-09-01 7:41 ` Arnd Bergmann
@ 2016-09-01 12:44 ` Dongdong Liu
-1 siblings, 0 replies; 63+ messages in thread
From: Dongdong Liu @ 2016-09-01 12:44 UTC (permalink / raw)
To: Arnd Bergmann
Cc: helgaas, rafael, Lorenzo.Pieralisi, tn, wangzhou1,
pratyush.anand, linux-pci, linux-acpi, linux-kernel, jcm,
gabriele.paoloni, charles.chenxin, hanjun.guo, linuxarm
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 = ®_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
>
> .
>
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [RFC PATCH V2 1/3] PCI: hisi: re-architect Hip05/Hip06 controllers driver to preapare for ACPI
@ 2016-09-01 12:44 ` Dongdong Liu
0 siblings, 0 replies; 63+ messages in thread
From: Dongdong Liu @ 2016-09-01 12:44 UTC (permalink / raw)
To: Arnd Bergmann
Cc: helgaas, rafael, Lorenzo.Pieralisi, tn, wangzhou1,
pratyush.anand, linux-pci, linux-acpi, linux-kernel, jcm,
gabriele.paoloni, charles.chenxin, hanjun.guo, linuxarm
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 = ®_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
>
> .
>
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [RFC PATCH V2 1/3] PCI: hisi: re-architect Hip05/Hip06 controllers driver to preapare for ACPI
2016-09-01 12:44 ` Dongdong Liu
@ 2016-09-01 14:02 ` Arnd Bergmann
-1 siblings, 0 replies; 63+ messages in thread
From: Arnd Bergmann @ 2016-09-01 14:02 UTC (permalink / raw)
To: Dongdong Liu
Cc: helgaas, rafael, Lorenzo.Pieralisi, tn, wangzhou1,
pratyush.anand, linux-pci, linux-acpi, linux-kernel, jcm,
gabriele.paoloni, charles.chenxin, hanjun.guo, linuxarm
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
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [RFC PATCH V2 1/3] PCI: hisi: re-architect Hip05/Hip06 controllers driver to preapare for ACPI
@ 2016-09-01 14:02 ` Arnd Bergmann
0 siblings, 0 replies; 63+ messages in thread
From: Arnd Bergmann @ 2016-09-01 14:02 UTC (permalink / raw)
To: Dongdong Liu
Cc: helgaas, rafael, Lorenzo.Pieralisi, tn, wangzhou1,
pratyush.anand, linux-pci, linux-acpi, linux-kernel, jcm,
gabriele.paoloni, charles.chenxin, hanjun.guo, linuxarm
On Thursday, September 1, 2016 8:44:49 PM CEST Dongdong Liu wrote:
> =E5=9C=A8 2016/9/1 15:41, Arnd Bergmann =E5=86=99=E9=81=93:
> > On Thursday, September 1, 2016 10:05:29 AM CEST Dongdong Liu wrote:
> >> =E5=9C=A8 2016/8/31 19:45, Arnd Bergmann =E5=86=99=E9=81=93:
> >>> On Wednesday, August 31, 2016 7:48:12 PM CEST Dongdong Liu wrote:
> I know your point.
>=20
> 1. For our host bridge , ".map_bus =3D pci_ecam_map_bus" is only suitable=
for
> accessing the EP config space.
> pci_generic_config_read32() need to call "addr =3D bus->ops->map_bus(bus,=
devfn, where & ~0x3);",
>=20
> drivers/pci/host/pcie-hisi-acpi.c
> static struct pci_ops hisi_pcie_ops =3D {
> .map_bus =3D pci_ecam_map_bus,
> .read =3D hisi_pcie_acpi_rd_conf,
> .write =3D hisi_pcie_acpi_wr_conf,
> };
>=20
> Yes, we can change ".map_bus =3D pci_ecam_map_bus" to ".map_bus =3D hisi_=
pci_map_bus", and implentment hisi_pci_map_bus as below,
> then we will not need to call hisi_pcie_common_cfg_read().
>=20
> void __iomem *hisi_pci_map_bus(struct pci_bus *bus, unsigned int devfn, i=
nt where)
> {
> struct pci_config_window *cfg =3D bus->sysdata;
> void __iomem *reg_base =3D cfg->priv;
>=20
> /* for RC config access*/
> if (bus->number =3D=3D cfg->busr.start)
> return reg_base + (where & ~0x3);
> else
> /* for EP config access */
> return pci_ecam_map_bus(bus, devfn, where);
> }
>=20
> 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 whe=
re,
> int size, u32 *val)
> {
> struct pci_config_window *cfg =3D bus->sysdata;
>=20
> if (hisi_pcie_acpi_valid_config(cfg, bus, PCI_SLOT(devfn)) =3D=3D 0)
> return PCIBIOS_DEVICE_NOT_FOUND;
> =09
> /* access RC config space */
> if (bus->number =3D=3D cfg->busr.start)
> return pci_generic_config_read32(bus, devfn, where, size, val);
> =09
> /* 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 be=
low code,
> so we have to call hisi_pcie_common_cfg_read() when accessing the RC conf=
ig space.
> For this, we have to call hisi_pcie_common_cfg_read().
>=20
> 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 =3D to_hisi_pcie(pp);
>=20
> return hisi_pcie_common_cfg_read(pcie->reg_base, where, size, val);
> }
>=20
> static struct pcie_host_ops hisi_pcie_host_ops =3D {
> .rd_own_conf =3D hisi_pcie_cfg_read,
> .wr_own_conf =3D hisi_pcie_cfg_write,
> .link_up =3D 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
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [RFC PATCH V2 1/3] PCI: hisi: re-architect Hip05/Hip06 controllers driver to preapare for ACPI
2016-09-01 14:02 ` Arnd Bergmann
@ 2016-09-02 2:02 ` Dongdong Liu
-1 siblings, 0 replies; 63+ messages in thread
From: Dongdong Liu @ 2016-09-02 2:02 UTC (permalink / raw)
To: Arnd Bergmann
Cc: helgaas, rafael, Lorenzo.Pieralisi, tn, wangzhou1,
pratyush.anand, linux-pci, linux-acpi, linux-kernel, jcm,
gabriele.paoloni, charles.chenxin, hanjun.guo, linuxarm
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
>
> .
>
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [RFC PATCH V2 1/3] PCI: hisi: re-architect Hip05/Hip06 controllers driver to preapare for ACPI
@ 2016-09-02 2:02 ` Dongdong Liu
0 siblings, 0 replies; 63+ messages in thread
From: Dongdong Liu @ 2016-09-02 2:02 UTC (permalink / raw)
To: Arnd Bergmann
Cc: helgaas, rafael, Lorenzo.Pieralisi, tn, wangzhou1,
pratyush.anand, linux-pci, linux-acpi, linux-kernel, jcm,
gabriele.paoloni, charles.chenxin, hanjun.guo, linuxarm
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
>
> .
>
^ permalink raw reply [flat|nested] 63+ messages in thread
* RE: [RFC PATCH V2 1/3] PCI: hisi: re-architect Hip05/Hip06 controllers driver to preapare for ACPI
2016-09-01 14:02 ` Arnd Bergmann
(?)
@ 2016-09-20 9:45 ` Gabriele Paoloni
-1 siblings, 0 replies; 63+ messages in thread
From: Gabriele Paoloni @ 2016-09-20 9:45 UTC (permalink / raw)
To: Arnd Bergmann, liudongdong (C)
Cc: helgaas, rafael, Lorenzo.Pieralisi, tn, Wangzhou (B),
pratyush.anand, linux-pci, linux-acpi, linux-kernel, jcm,
Chenxin (Charles),
hanjun.guo, Linuxarm
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
^ permalink raw reply [flat|nested] 63+ messages in thread* RE: [RFC PATCH V2 1/3] PCI: hisi: re-architect Hip05/Hip06 controllers driver to preapare for ACPI
@ 2016-09-20 9:45 ` Gabriele Paoloni
0 siblings, 0 replies; 63+ messages in thread
From: Gabriele Paoloni @ 2016-09-20 9:45 UTC (permalink / raw)
To: Arnd Bergmann, liudongdong (C)
Cc: helgaas, rafael, Lorenzo.Pieralisi, tn, Wangzhou (B),
pratyush.anand, linux-pci, linux-acpi, linux-kernel, jcm,
Chenxin (Charles),
hanjun.guo, Linuxarm
SGkgQXJuZCwgdGhhbmtzIGZvciB5b3VyIGNvbW1lbnRzDQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNz
YWdlLS0tLS0NCj4gRnJvbTogQXJuZCBCZXJnbWFubiBbbWFpbHRvOmFybmRAYXJuZGIuZGVdDQo+
IFNlbnQ6IDAxIFNlcHRlbWJlciAyMDE2IDE1OjAyDQo+IFRvOiBsaXVkb25nZG9uZyAoQykNCj4g
Q2M6IGhlbGdhYXNAa2VybmVsLm9yZzsgcmFmYWVsQGtlcm5lbC5vcmc7IExvcmVuem8uUGllcmFs
aXNpQGFybS5jb207DQo+IHRuQHNlbWloYWxmLmNvbTsgV2FuZ3pob3UgKEIpOyBwcmF0eXVzaC5h
bmFuZEBnbWFpbC5jb207IGxpbnV4LQ0KPiBwY2lAdmdlci5rZXJuZWwub3JnOyBsaW51eC1hY3Bp
QHZnZXIua2VybmVsLm9yZzsgbGludXgtDQo+IGtlcm5lbEB2Z2VyLmtlcm5lbC5vcmc7IGpjbUBy
ZWRoYXQuY29tOyBHYWJyaWVsZSBQYW9sb25pOyBDaGVueGluDQo+IChDaGFybGVzKTsgaGFuanVu
Lmd1b0BsaW5hcm8ub3JnOyBMaW51eGFybQ0KPiBTdWJqZWN0OiBSZTogW1JGQyBQQVRDSCBWMiAx
LzNdIFBDSTogaGlzaTogcmUtYXJjaGl0ZWN0IEhpcDA1L0hpcDA2DQo+IGNvbnRyb2xsZXJzIGRy
aXZlciB0byBwcmVhcGFyZSBmb3IgQUNQSQ0KPiANCj4gT24gVGh1cnNkYXksIFNlcHRlbWJlciAx
LCAyMDE2IDg6NDQ6NDkgUE0gQ0VTVCBEb25nZG9uZyBMaXUgd3JvdGU6DQo+ID4g5ZyoIDIwMTYv
OS8xIDE1OjQxLCBBcm5kIEJlcmdtYW5uIOWGmemBkzoNCj4gPiA+IE9uIFRodXJzZGF5LCBTZXB0
ZW1iZXIgMSwgMjAxNiAxMDowNToyOSBBTSBDRVNUIERvbmdkb25nIExpdSB3cm90ZToNCj4gPiA+
PiDlnKggMjAxNi84LzMxIDE5OjQ1LCBBcm5kIEJlcmdtYW5uIOWGmemBkzoNCj4gPiA+Pj4gT24g
V2VkbmVzZGF5LCBBdWd1c3QgMzEsIDIwMTYgNzo0ODoxMiBQTSBDRVNUIERvbmdkb25nIExpdSB3
cm90ZToNCj4gPiBJIGtub3cgeW91ciBwb2ludC4NCj4gPg0KPiA+IDEuIEZvciBvdXIgaG9zdCBi
cmlkZ2UgLCAiLm1hcF9idXMgPSBwY2lfZWNhbV9tYXBfYnVzIiBpcyBvbmx5DQo+IHN1aXRhYmxl
IGZvcg0KPiA+IGFjY2Vzc2luZyB0aGUgRVAgY29uZmlnIHNwYWNlLg0KPiA+IHBjaV9nZW5lcmlj
X2NvbmZpZ19yZWFkMzIoKSBuZWVkIHRvIGNhbGwgImFkZHIgPSBidXMtPm9wcy0NCj4gPm1hcF9i
dXMoYnVzLCBkZXZmbiwgd2hlcmUgJiB+MHgzKTsiLA0KPiA+DQo+ID4gZHJpdmVycy9wY2kvaG9z
dC9wY2llLWhpc2ktYWNwaS5jDQo+ID4gc3RhdGljIHN0cnVjdCBwY2lfb3BzIGhpc2lfcGNpZV9v
cHMgPSB7DQo+ID4gCS5tYXBfYnVzID0gcGNpX2VjYW1fbWFwX2J1cywNCj4gPiAJLnJlYWQgPSBo
aXNpX3BjaWVfYWNwaV9yZF9jb25mLA0KPiA+IAkud3JpdGUgPSBoaXNpX3BjaWVfYWNwaV93cl9j
b25mLA0KPiA+IH07DQo+ID4NCj4gPiBZZXMsIHdlIGNhbiBjaGFuZ2UgIi5tYXBfYnVzID0gcGNp
X2VjYW1fbWFwX2J1cyIgdG8gIi5tYXBfYnVzID0NCj4gaGlzaV9wY2lfbWFwX2J1cyIsIGFuZCBp
bXBsZW50bWVudCBoaXNpX3BjaV9tYXBfYnVzIGFzIGJlbG93LA0KPiA+IHRoZW4gd2Ugd2lsbCBu
b3QgbmVlZCB0byBjYWxsIGhpc2lfcGNpZV9jb21tb25fY2ZnX3JlYWQoKS4NCj4gPg0KPiA+IHZv
aWQgX19pb21lbSAqaGlzaV9wY2lfbWFwX2J1cyhzdHJ1Y3QgcGNpX2J1cyAqYnVzLCB1bnNpZ25l
ZCBpbnQNCj4gZGV2Zm4sIGludCB3aGVyZSkNCj4gPiB7DQo+ID4gCXN0cnVjdCBwY2lfY29uZmln
X3dpbmRvdyAqY2ZnID0gYnVzLT5zeXNkYXRhOw0KPiA+IAl2b2lkIF9faW9tZW0gKnJlZ19iYXNl
ID0gY2ZnLT5wcml2Ow0KPiA+DQo+ID4gCS8qIGZvciBSQyBjb25maWcgYWNjZXNzKi8NCj4gPiAJ
aWYgKGJ1cy0+bnVtYmVyID09IGNmZy0+YnVzci5zdGFydCkNCj4gPiAJCXJldHVybiByZWdfYmFz
ZSArICh3aGVyZSAmIH4weDMpOw0KPiA+IAllbHNlDQo+ID4gCQkvKiBmb3IgRVAgY29uZmlnIGFj
Y2VzcyAqLw0KPiA+IAkJcmV0dXJuIHBjaV9lY2FtX21hcF9idXMoYnVzLCBkZXZmbiwgd2hlcmUp
Ow0KPiA+IH0NCj4gPg0KPiA+IGFuZCBoaXNpX3BjaWVfYWNwaV9yZF9jb25mKCkgbmVlZCB0byBj
aGFuZ2UgYXMgYmVsb3cuDQo+ID4gc3RhdGljIGludCBoaXNpX3BjaWVfYWNwaV9yZF9jb25mKHN0
cnVjdCBwY2lfYnVzICpidXMsIHUzMiBkZXZmbiwgaW50DQo+IHdoZXJlLA0KPiA+IAkJCQkgIGlu
dCBzaXplLCB1MzIgKnZhbCkNCj4gPiB7DQo+ID4gCXN0cnVjdCBwY2lfY29uZmlnX3dpbmRvdyAq
Y2ZnID0gYnVzLT5zeXNkYXRhOw0KPiA+DQo+ID4gCWlmIChoaXNpX3BjaWVfYWNwaV92YWxpZF9j
b25maWcoY2ZnLCBidXMsIFBDSV9TTE9UKGRldmZuKSkgPT0gMCkNCj4gPiAJCXJldHVybiBQQ0lC
SU9TX0RFVklDRV9OT1RfRk9VTkQ7DQo+ID4NCj4gPiAJLyogYWNjZXNzIFJDIGNvbmZpZyBzcGFj
ZSAqLw0KPiA+IAlpZiAoYnVzLT5udW1iZXIgPT0gY2ZnLT5idXNyLnN0YXJ0KQ0KPiA+IAkJcmV0
dXJuIHBjaV9nZW5lcmljX2NvbmZpZ19yZWFkMzIoYnVzLCBkZXZmbiwgd2hlcmUsIHNpemUsDQo+
IHZhbCk7DQo+ID4NCj4gPiAJLyogYWNjZXNzIEVQIGNvbmZpZyBzcGFjZSAqLw0KPiA+IAlyZXR1
cm4gcGNpX2dlbmVyaWNfY29uZmlnX3JlYWQoYnVzLCBkZXZmbiwgd2hlcmUsIHNpemUsIHZhbCk7
DQo+ID4gfQ0KPiANCj4gUmlnaHQsIHRoaXMgaXMgd2hhdCBJIGhhZCBpbiBtaW5kLg0KPiANCj4g
DQo+ID4gMi4gV2UgbmVlZCB0byBiYWNrd2FyZCBjb21wYXRpYmxlIHdpdGggdGhlIG9sZCBkdCB3
YXkgY29uZmlnIGFjY2Vzcw0KPiBhcyBiZWxvdyBjb2RlLA0KPiA+IHNvIHdlIGhhdmUgdG8gY2Fs
bCBoaXNpX3BjaWVfY29tbW9uX2NmZ19yZWFkKCkgd2hlbiBhY2Nlc3NpbmcgdGhlIFJDDQo+IGNv
bmZpZyBzcGFjZS4NCj4gPiBGb3IgdGhpcywgd2UgaGF2ZSB0byBjYWxsIGhpc2lfcGNpZV9jb21t
b25fY2ZnX3JlYWQoKS4NCj4gPg0KPiA+IGRyaXZlcnMvcGNpL2hvc3QvcGNpZS1oaXNpLmMNCj4g
PiBzdGF0aWMgaW5saW5lIGludCBoaXNpX3BjaWVfY2ZnX3JlYWQoc3RydWN0IHBjaWVfcG9ydCAq
cHAsIGludCB3aGVyZSwNCj4gPiAJCWludCBzaXplLCB1MzIgKnZhbCkNCj4gPiB7DQo+ID4gCXN0
cnVjdCBoaXNpX3BjaWUgKnBjaWUgPSB0b19oaXNpX3BjaWUocHApOw0KPiA+DQo+ID4gCXJldHVy
biBoaXNpX3BjaWVfY29tbW9uX2NmZ19yZWFkKHBjaWUtPnJlZ19iYXNlLCB3aGVyZSwgc2l6ZSwN
Cj4gdmFsKTsNCj4gPiB9DQo+ID4NCj4gPiBzdGF0aWMgc3RydWN0IHBjaWVfaG9zdF9vcHMgaGlz
aV9wY2llX2hvc3Rfb3BzID0gew0KPiA+IAkucmRfb3duX2NvbmYgPSBoaXNpX3BjaWVfY2ZnX3Jl
YWQsDQo+ID4gCS53cl9vd25fY29uZiA9IGhpc2lfcGNpZV9jZmdfd3JpdGUsDQo+ID4gCS5saW5r
X3VwID0gaGlzaV9wY2llX2xpbmtfdXAsDQo+ID4gfTsNCj4gDQo+IEkgdGhpbmsgdGhpcyB3b3Vs
ZCBiZSBlYXNpZXIgaWYgeW91IHNlcGFyYXRlIHRoZSBBQ1BJIGNvZGUgZnJvbSB0aGUNCj4gRFQg
Y29kZSBhbmQgbm90IHRyeSB0byBoYXZlIGEgY29tbW9uIGZpbGUgdXNlZCBmb3IgYm90aC4NCj4g
DQo+IFNoYXJpbmcgdGhlIGNvbmZpZyBzcGFjZSBhY2Nlc3NvcnMgcmVhbGx5IGlzbid0IHdvcnRo
IGl0IHdoZW4gYm90aA0KPiB2YXJpYW50cyBhcmUgZmFpcmx5IHNpbXBsZSB0byBkbywgYnV0IHRo
ZXkgZG9uJ3QgZml0IGluIGEgY29tbW9uDQo+IG1vZGVsIGJlY2F1c2Ugb25lIGlzIGNhbGxlZCBm
cm9tIHRoZSBBQ1BJIHF1aXJrcyBhbmQgdGhlIG90aGVyDQo+IGlzIGNhbGxlZCBmcm9tIHRoZSBk
dy1wY2llIGRyaXZlciB3aXRoIGNvbXBsZXRlbHkgZGlmZmVyZW50IGNhbGxpbmcNCj4gY29udmVu
dGlvbnMuDQoNCk5vdCBzdXJlIGFib3V0IHRoaXMuLi4NCkZyb20gbXkgcGVyc3BlY3RpdmUgaGF2
aW5nIHRoZSBzaGFyZWQgY29kZSB3b3VsZCBtYWtlIGNsZWFyIHRoYXQNCnRoZSB0d28gZHJpdmVy
cyAoQUNQSSBhbmQgRFQpIGFyZSBraW5kIG9mIHJlbGF0ZWQuLi4NCg0KRm9yIGV4YW1wbGUgc2Vl
IHRoZSByZXBseSBmcm9tIEJqb3JuIHRvIHRoZSB4Z2VuZSBkcml2ZXI6DQpodHRwczovL2xrbWwu
b3JnL2xrbWwvMjAxNi85LzE5Lzc0OQ0KDQpJIGtub3cgaW4gb3VyIGNhc2UgdGhlIGR1cGxpY2F0
aW9uIGlzbid0IG11Y2ggYnV0IGFzIEkgc2FpZA0KSSBhbSBhIGJpdCByZWx1Y3RhbnQgdG8gcmV3
b3JrIHRoaXMuLi4gDQoNClRoeA0KDQpHYWINCg0KPiANCj4gCUFSbmQNCg==
^ permalink raw reply [flat|nested] 63+ messages in thread
* RE: [RFC PATCH V2 1/3] PCI: hisi: re-architect Hip05/Hip06 controllers driver to preapare for ACPI
@ 2016-09-20 9:45 ` Gabriele Paoloni
0 siblings, 0 replies; 63+ messages in thread
From: Gabriele Paoloni @ 2016-09-20 9:45 UTC (permalink / raw)
To: Arnd Bergmann, liudongdong (C)
Cc: helgaas, rafael, Lorenzo.Pieralisi, tn, Wangzhou (B),
pratyush.anand, linux-pci, linux-acpi, linux-kernel, jcm,
Chenxin (Charles),
hanjun.guo, Linuxarm
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
^ permalink raw reply [flat|nested] 63+ messages in thread* Re: [RFC PATCH V2 1/3] PCI: hisi: re-architect Hip05/Hip06 controllers driver to preapare for ACPI
2016-09-20 9:45 ` Gabriele Paoloni
(?)
(?)
@ 2016-09-20 13:22 ` Arnd Bergmann
-1 siblings, 0 replies; 63+ messages in thread
From: Arnd Bergmann @ 2016-09-20 13:22 UTC (permalink / raw)
To: Gabriele Paoloni
Cc: liudongdong (C),
helgaas, rafael, Lorenzo.Pieralisi, tn, Wangzhou (B),
pratyush.anand, linux-pci, linux-acpi, linux-kernel, jcm,
Chenxin (Charles),
hanjun.guo, Linuxarm
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
^ permalink raw reply [flat|nested] 63+ messages in thread