2016-11-22 11:52:25

by Dongdong Liu

[permalink] [raw]
Subject: [PATCH V6 0/2] Add ACPI support for HiSilicon SoCs Host Controllers

This patchset adds ACPI support for the HiSilicon Hip05/Hip06/Hip07 SoC
PCIe controllers.
The two patches respectively:
- provides the common function acpi_get_rc_resources() for ARM64
platform.
- adds the HiSilicon ACPI specific quirks.

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

This patchset has been tested on HiSilicon D03 board.
The dmesg log, /proc/iomem, and ACPI table information can be found:
https://bugzilla.kernel.org/show_bug.cgi?id=187961

v5 -> v6:
- change the config option to CONFIG_PCI_ECAM_QUIRKS.
- fix some commets about acpi_get_rc_resources().

v4 -> v5:
- obtain rc base addresses from PNP0C02 at the root of the ACPI
namespace (under \_SB) instead of from sub-device under the RC.
- merge the rewrited get rc resources code by Tomasz.
- delete unused code.
- drop the PATCH V4 1/2, will rework late as a separate patch.

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

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

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

Dongdong Liu (2):
PCI/ACPI: Provide acpi_get_rc_resources() for ARM64 platform
PCI/ACPI: hisi: Add ACPI support for HiSilicon SoCs Host Controllers

MAINTAINERS | 1 +
drivers/acpi/pci_mcfg.c | 13 +++++
drivers/pci/host/Kconfig | 7 +++
drivers/pci/host/Makefile | 1 +
drivers/pci/host/pcie-hisi-acpi.c | 119 ++++++++++++++++++++++++++++++++++++++
drivers/pci/pci-acpi.c | 69 ++++++++++++++++++++++
drivers/pci/pci.h | 4 ++
include/linux/pci-ecam.h | 5 ++
8 files changed, 219 insertions(+)
create mode 100644 drivers/pci/host/pcie-hisi-acpi.c

--
1.9.1


2016-11-22 11:52:27

by Dongdong Liu

[permalink] [raw]
Subject: [PATCH V6 2/2] PCI/ACPI: hisi: Add ACPI support for HiSilicon SoCs Host Controllers

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

Signed-off-by: Dongdong Liu <[email protected]>
Signed-off-by: Gabriele Paoloni <[email protected]>
---
MAINTAINERS | 1 +
drivers/acpi/pci_mcfg.c | 13 +++++
drivers/pci/host/Kconfig | 7 +++
drivers/pci/host/Makefile | 1 +
drivers/pci/host/pcie-hisi-acpi.c | 119 ++++++++++++++++++++++++++++++++++++++
include/linux/pci-ecam.h | 5 ++
6 files changed, 146 insertions(+)
create mode 100644 drivers/pci/host/pcie-hisi-acpi.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 1cd38a7..b224caa 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9358,6 +9358,7 @@ L: [email protected]
S: Maintained
F: Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
F: drivers/pci/host/pcie-hisi.c
+F: drivers/pci/host/pcie-hisi-acpi.c

PCIE DRIVER FOR ROCKCHIP
M: Shawn Lin <[email protected]>
diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
index ac21db3..3297c5a 100644
--- a/drivers/acpi/pci_mcfg.c
+++ b/drivers/acpi/pci_mcfg.c
@@ -57,6 +57,19 @@ struct mcfg_fixup {
{ "QCOM ", "QDF2432 ", 1, 5, MCFG_BUS_ANY, &pci_32b_ops },
{ "QCOM ", "QDF2432 ", 1, 6, MCFG_BUS_ANY, &pci_32b_ops },
{ "QCOM ", "QDF2432 ", 1, 7, MCFG_BUS_ANY, &pci_32b_ops },
+#ifdef CONFIG_PCI_ECAM_QUIRKS
+ #define PCI_ACPI_QUIRK_QUAD_DOM(table_id, seg, ops) \
+ { "HISI ", table_id, 0, seg + 0, MCFG_BUS_ANY, ops }, \
+ { "HISI ", table_id, 0, seg + 1, MCFG_BUS_ANY, ops }, \
+ { "HISI ", table_id, 0, seg + 2, MCFG_BUS_ANY, ops }, \
+ { "HISI ", table_id, 0, seg + 3, MCFG_BUS_ANY, ops }
+ PCI_ACPI_QUIRK_QUAD_DOM("HIP05 ", 0, &hisi_pcie_ops),
+ PCI_ACPI_QUIRK_QUAD_DOM("HIP06 ", 0, &hisi_pcie_ops),
+ PCI_ACPI_QUIRK_QUAD_DOM("HIP07 ", 0, &hisi_pcie_ops),
+ PCI_ACPI_QUIRK_QUAD_DOM("HIP07 ", 4, &hisi_pcie_ops),
+ PCI_ACPI_QUIRK_QUAD_DOM("HIP07 ", 8, &hisi_pcie_ops),
+ PCI_ACPI_QUIRK_QUAD_DOM("HIP07 ", 12, &hisi_pcie_ops),
+#endif
};

static char mcfg_oem_id[ACPI_OEM_ID_SIZE];
diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
index ae98644..1fbade5 100644
--- a/drivers/pci/host/Kconfig
+++ b/drivers/pci/host/Kconfig
@@ -301,4 +301,11 @@ config VMD
To compile this driver as a module, choose M here: the
module will be called vmd.

+config PCI_ECAM_QUIRKS
+ bool "PCI ECAM quirks for ARM64 platform"
+ depends on PCI_ECAM && ARM64 && ACPI
+ help
+ Say y here to enable your platform-specific config access that
+ is not ECAM compliant.
+
endmenu
diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
index 084cb49..15435b4 100644
--- a/drivers/pci/host/Makefile
+++ b/drivers/pci/host/Makefile
@@ -26,6 +26,7 @@ obj-$(CONFIG_PCIE_IPROC_BCMA) += pcie-iproc-bcma.o
obj-$(CONFIG_PCIE_ALTERA) += pcie-altera.o
obj-$(CONFIG_PCIE_ALTERA_MSI) += pcie-altera-msi.o
obj-$(CONFIG_PCI_HISI) += pcie-hisi.o
+obj-$(CONFIG_PCI_ECAM_QUIRKS) += pcie-hisi-acpi.o
obj-$(CONFIG_PCIE_QCOM) += pcie-qcom.o
obj-$(CONFIG_PCI_HOST_THUNDER_ECAM) += pci-thunder-ecam.o
obj-$(CONFIG_PCI_HOST_THUNDER_PEM) += pci-thunder-pem.o
diff --git a/drivers/pci/host/pcie-hisi-acpi.c b/drivers/pci/host/pcie-hisi-acpi.c
new file mode 100644
index 0000000..8bb43b4
--- /dev/null
+++ b/drivers/pci/host/pcie-hisi-acpi.c
@@ -0,0 +1,119 @@
+/*
+ * PCIe host controller driver for HiSilicon HipXX SoCs
+ *
+ * Copyright (C) 2016 HiSilicon Co., Ltd. http://www.hisilicon.com
+ *
+ * Author: Dongdong Liu <[email protected]>
+ * Gabriele Paoloni <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#include <linux/pci.h>
+#include <linux/pci-acpi.h>
+#include <linux/pci-ecam.h>
+#include "../pci.h"
+
+static int hisi_pcie_acpi_rd_conf(struct pci_bus *bus, u32 devfn, int where,
+ int size, u32 *val)
+{
+ struct pci_config_window *cfg = bus->sysdata;
+ int dev = PCI_SLOT(devfn);
+
+ if (bus->number == cfg->busr.start) {
+ /* access only one slot on each root port */
+ if (dev > 0)
+ return PCIBIOS_DEVICE_NOT_FOUND;
+ else
+ return pci_generic_config_read32(bus, devfn, where,
+ size, val);
+ }
+
+ return pci_generic_config_read(bus, devfn, where, size, val);
+}
+
+static int hisi_pcie_acpi_wr_conf(struct pci_bus *bus, u32 devfn,
+ int where, int size, u32 val)
+{
+ struct pci_config_window *cfg = bus->sysdata;
+ int dev = PCI_SLOT(devfn);
+
+ if (bus->number == cfg->busr.start) {
+ /* access only one slot on each root port */
+ if (dev > 0)
+ return PCIBIOS_DEVICE_NOT_FOUND;
+ else
+ return pci_generic_config_write32(bus, devfn, where,
+ size, val);
+ }
+
+ return pci_generic_config_write(bus, devfn, where, size, val);
+}
+
+static void __iomem *hisi_pcie_map_bus(struct pci_bus *bus, unsigned int devfn,
+ int where)
+{
+ struct pci_config_window *cfg = bus->sysdata;
+ void __iomem *reg_base = cfg->priv;
+
+ if (bus->number == cfg->busr.start)
+ return reg_base + where;
+ else
+ return pci_ecam_map_bus(bus, devfn, where);
+}
+
+/*
+ * Retrieve RC base and size from PNP0C02 at the root of the ACPI namespace
+ * (under \_SB).
+ * Scope(_SB)
+ * {
+ * Device (PCI0)
+ * {
+ * Name(_HID, "PNP0A08")
+ * Name(_CID, "PNP0A03")
+ * Name(_SEG, 0)
+ * ......
+ * }
+ * Device (RES0)
+ * {
+ * Name(_HID, "HISI0081")
+ * Name(_CID, "PNP0C02")
+ * Name(_UID, 0x0)
+ * Name(_CRS, ResourceTemplate (){
+ * Memory32Fixed (ReadWrite, 0xa0090000, 0x10000)
+ * })
+ * }
+ * ......
+ * }
+ */
+static int hisi_pcie_init(struct pci_config_window *cfg)
+{
+ struct acpi_device *adev = to_acpi_device(cfg->parent);
+ struct acpi_pci_root *root = acpi_driver_data(adev);
+ void __iomem *reg_base;
+ struct resource *res;
+
+ res = acpi_get_rc_resources("HISI0081", root->segment);
+ if (!res) {
+ dev_err(&adev->dev, "can't get rc base address");
+ return -ENOMEM;
+ }
+
+ reg_base = devm_ioremap(&adev->dev, res->start, resource_size(res));
+ if (!reg_base)
+ return -ENOMEM;
+
+ cfg->priv = reg_base;
+ return 0;
+}
+
+struct pci_ecam_ops hisi_pcie_ops = {
+ .bus_shift = 20,
+ .init = hisi_pcie_init,
+ .pci_ops = {
+ .map_bus = hisi_pcie_map_bus,
+ .read = hisi_pcie_acpi_rd_conf,
+ .write = hisi_pcie_acpi_wr_conf,
+ }
+};
diff --git a/include/linux/pci-ecam.h b/include/linux/pci-ecam.h
index f5740b7..40fab66 100644
--- a/include/linux/pci-ecam.h
+++ b/include/linux/pci-ecam.h
@@ -67,4 +67,9 @@ void __iomem *pci_ecam_map_bus(struct pci_bus *bus, unsigned int devfn,
int pci_host_common_probe(struct platform_device *pdev,
struct pci_ecam_ops *ops);
#endif
+
+#ifdef CONFIG_PCI_ECAM_QUIRKS
+extern struct pci_ecam_ops hisi_pcie_ops;
+#endif
+
#endif
--
1.9.1

2016-11-22 11:52:42

by Dongdong Liu

[permalink] [raw]
Subject: [PATCH V6 1/2] PCI/ACPI: Provide acpi_get_rc_resources() for ARM64 platform

The acpi_get_rc_resources() is used to get the RC register address that can
not be described in MCFG. It takes the _HID&segment to look for and returns
the RC address resource. Use PNP0C02 devices to describe such RC address
resource. Use _UID to match segment to tell which root bus the PNP0C02
resource belong to.

Signed-off-by: Dongdong Liu <[email protected]>
Signed-off-by: Tomasz Nowicki <[email protected]>
---
drivers/pci/pci-acpi.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++
drivers/pci/pci.h | 4 +++
2 files changed, 73 insertions(+)

diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
index d966d47..76fd6f4 100644
--- a/drivers/pci/pci-acpi.c
+++ b/drivers/pci/pci-acpi.c
@@ -29,6 +29,75 @@
0x91, 0x17, 0xea, 0x4d, 0x19, 0xc3, 0x43, 0x4d
};

+#ifdef CONFIG_ARM64
+static struct resource *acpi_get_rc_addr(struct acpi_device *adev)
+{
+ struct resource_entry *entry;
+ struct list_head list;
+ unsigned long flags;
+ int ret;
+ struct resource *res;
+
+ INIT_LIST_HEAD(&list);
+ flags = IORESOURCE_MEM;
+ ret = acpi_dev_get_resources(adev, &list,
+ acpi_dev_filter_resource_type_cb,
+ (void *) flags);
+ if (ret <= 0)
+ return NULL;
+
+ entry = list_first_entry(&list, struct resource_entry, node);
+ res = entry->res;
+ acpi_dev_free_resource_list(&list);
+ return res;
+}
+
+static acpi_status acpi_match_rc(acpi_handle handle, u32 lvl, void *context,
+ void **retval)
+{
+ u16 *segment = context;
+ unsigned long long uid;
+ acpi_status status;
+
+ status = acpi_evaluate_integer(handle, "_UID", NULL, &uid);
+ if (ACPI_FAILURE(status) || uid != *segment)
+ return AE_CTRL_DEPTH;
+
+ *(acpi_handle *)retval = handle;
+ return AE_CTRL_TERMINATE;
+}
+
+/**
+ * acpi_get_rc_resources() - get the RC address resource.
+ * @hid: HID to search for.
+ * @segment: PCI Segment number.
+ *
+ * Get the RC address resource that can not be described in MCFG. It takes
+ * the _HID&segment to look for and returns the RC address resource. Use
+ * _CRS of PNP0C02 devices to describe such RC address resource. Use _UID
+ * to match segment to tell which root bus the PNP0C02 resource belong to.
+ *
+ * Return: RC address resource.
+ */
+struct resource *acpi_get_rc_resources(const char *hid, u16 segment)
+{
+ struct acpi_device *adev;
+ acpi_status status;
+ acpi_handle handle;
+ int ret;
+
+ status = acpi_get_devices(hid, acpi_match_rc, &segment, &handle);
+ if (ACPI_FAILURE(status))
+ return -ENODEV;
+
+ ret = acpi_bus_get_device(handle, &adev);
+ if (ret)
+ return ret;
+
+ return acpi_get_rc_addr(adev);
+}
+#endif
+
phys_addr_t acpi_pci_root_get_mcfg_addr(acpi_handle handle)
{
acpi_status status = AE_NOT_EXIST;
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 4518562..bf1dbfe 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -356,4 +356,8 @@ static inline int pci_dev_specific_reset(struct pci_dev *dev, int probe)
}
#endif

+#if defined(CONFIG_ARM64) && defined(CONFIG_ACPI)
+struct resource *acpi_get_rc_resources(const char *hid, u16 segment);
+#endif
+
#endif /* DRIVERS_PCI_H */
--
1.9.1

2016-11-22 12:32:18

by Tomasz Nowicki

[permalink] [raw]
Subject: Re: [PATCH V6 1/2] PCI/ACPI: Provide acpi_get_rc_resources() for ARM64 platform

Hi Dongdong,

On 22.11.2016 13:08, Dongdong Liu wrote:
> The acpi_get_rc_resources() is used to get the RC register address that can
> not be described in MCFG. It takes the _HID&segment to look for and returns
> the RC address resource. Use PNP0C02 devices to describe such RC address
> resource. Use _UID to match segment to tell which root bus the PNP0C02
> resource belong to.
>
> Signed-off-by: Dongdong Liu <[email protected]>
> Signed-off-by: Tomasz Nowicki <[email protected]>
> ---
> drivers/pci/pci-acpi.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++
> drivers/pci/pci.h | 4 +++
> 2 files changed, 73 insertions(+)
>
> diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
> index d966d47..76fd6f4 100644
> --- a/drivers/pci/pci-acpi.c
> +++ b/drivers/pci/pci-acpi.c
> @@ -29,6 +29,75 @@
> 0x91, 0x17, 0xea, 0x4d, 0x19, 0xc3, 0x43, 0x4d
> };
>
> +#ifdef CONFIG_ARM64
> +static struct resource *acpi_get_rc_addr(struct acpi_device *adev)
> +{
> + struct resource_entry *entry;
> + struct list_head list;
> + unsigned long flags;
> + int ret;
> + struct resource *res;
> +
> + INIT_LIST_HEAD(&list);
> + flags = IORESOURCE_MEM;
> + ret = acpi_dev_get_resources(adev, &list,
> + acpi_dev_filter_resource_type_cb,
> + (void *) flags);
> + if (ret <= 0)
> + return NULL;
> +
> + entry = list_first_entry(&list, struct resource_entry, node);
> + res = entry->res;

You return "res" memory pointer and...

> + acpi_dev_free_resource_list(&list);

free it here.

> + return res;
> +}


We either allocate memory for res here or get it from the caller.

Tomasz

2016-11-22 13:58:19

by Tomasz Nowicki

[permalink] [raw]
Subject: Re: [PATCH V6 2/2] PCI/ACPI: hisi: Add ACPI support for HiSilicon SoCs Host Controllers

On 22.11.2016 13:08, Dongdong Liu wrote:
> PCIe controller in Hip05/HIP06/HIP07 SoCs is not ECAM compliant.
> It is non ECAM only for the RC bus config space;for any other bus
> underneath the root bus we support ECAM access.
> Add specific quirks for PCI config space accessors.This involves:
> 1. New initialization call hisi_pcie_init() to obtain rc base
> addresses from PNP0C02 at the root of the ACPI namespace (under \_SB).
> 2. New entry in common quirk array.
>
> Signed-off-by: Dongdong Liu <[email protected]>
> Signed-off-by: Gabriele Paoloni <[email protected]>
> ---
> MAINTAINERS | 1 +
> drivers/acpi/pci_mcfg.c | 13 +++++
> drivers/pci/host/Kconfig | 7 +++
> drivers/pci/host/Makefile | 1 +
> drivers/pci/host/pcie-hisi-acpi.c | 119 ++++++++++++++++++++++++++++++++++++++
> include/linux/pci-ecam.h | 5 ++
> 6 files changed, 146 insertions(+)
> create mode 100644 drivers/pci/host/pcie-hisi-acpi.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 1cd38a7..b224caa 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -9358,6 +9358,7 @@ L: [email protected]
> S: Maintained
> F: Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
> F: drivers/pci/host/pcie-hisi.c
> +F: drivers/pci/host/pcie-hisi-acpi.c
>
> PCIE DRIVER FOR ROCKCHIP
> M: Shawn Lin <[email protected]>
> diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
> index ac21db3..3297c5a 100644
> --- a/drivers/acpi/pci_mcfg.c
> +++ b/drivers/acpi/pci_mcfg.c
> @@ -57,6 +57,19 @@ struct mcfg_fixup {
> { "QCOM ", "QDF2432 ", 1, 5, MCFG_BUS_ANY, &pci_32b_ops },
> { "QCOM ", "QDF2432 ", 1, 6, MCFG_BUS_ANY, &pci_32b_ops },
> { "QCOM ", "QDF2432 ", 1, 7, MCFG_BUS_ANY, &pci_32b_ops },
> +#ifdef CONFIG_PCI_ECAM_QUIRKS
> + #define PCI_ACPI_QUIRK_QUAD_DOM(table_id, seg, ops) \
> + { "HISI ", table_id, 0, seg + 0, MCFG_BUS_ANY, ops }, \
> + { "HISI ", table_id, 0, seg + 1, MCFG_BUS_ANY, ops }, \
> + { "HISI ", table_id, 0, seg + 2, MCFG_BUS_ANY, ops }, \
> + { "HISI ", table_id, 0, seg + 3, MCFG_BUS_ANY, ops }
> + PCI_ACPI_QUIRK_QUAD_DOM("HIP05 ", 0, &hisi_pcie_ops),
> + PCI_ACPI_QUIRK_QUAD_DOM("HIP06 ", 0, &hisi_pcie_ops),
> + PCI_ACPI_QUIRK_QUAD_DOM("HIP07 ", 0, &hisi_pcie_ops),
> + PCI_ACPI_QUIRK_QUAD_DOM("HIP07 ", 4, &hisi_pcie_ops),
> + PCI_ACPI_QUIRK_QUAD_DOM("HIP07 ", 8, &hisi_pcie_ops),
> + PCI_ACPI_QUIRK_QUAD_DOM("HIP07 ", 12, &hisi_pcie_ops),
> +#endif
> };
>
> static char mcfg_oem_id[ACPI_OEM_ID_SIZE];
> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
> index ae98644..1fbade5 100644
> --- a/drivers/pci/host/Kconfig
> +++ b/drivers/pci/host/Kconfig
> @@ -301,4 +301,11 @@ config VMD
> To compile this driver as a module, choose M here: the
> module will be called vmd.
>
> +config PCI_ECAM_QUIRKS
> + bool "PCI ECAM quirks for ARM64 platform"
> + depends on PCI_ECAM && ARM64 && ACPI
> + help
> + Say y here to enable your platform-specific config access that
> + is not ECAM compliant.
> +

ARM64 selects PCI_ECAM for ACPI anyway so we can skip it here.

How about selecting PCI_ECAM_QUIRKS from ARM64 Kconfig:

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 30398db..9743186 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -105,6 +105,7 @@ config ARM64
select OF_EARLY_FLATTREE
select OF_RESERVED_MEM
select PCI_ECAM if ACPI
+ select PCI_ECAM_QUIRKS if ACPI
select POWER_RESET
select POWER_SUPPLY
select SPARSE_IRQ

I agree with Bjorn, we should not have to turn on platform-specific
config options for ARM64 & ACPI.

Thanks,
Tomasz

2016-11-22 15:06:05

by Gabriele Paoloni

[permalink] [raw]
Subject: RE: [PATCH V6 2/2] PCI/ACPI: hisi: Add ACPI support for HiSilicon SoCs Host Controllers

Hi Tomasz

> -----Original Message-----
> From: Tomasz Nowicki [mailto:[email protected]]
> Sent: 22 November 2016 13:58
> To: liudongdong (C); [email protected]; [email protected];
> [email protected]; [email protected]; Wangzhou (B);
> [email protected]
> Cc: [email protected]; [email protected]; linux-
> [email protected]; [email protected]; Gabriele Paoloni; Chenxin
> (Charles); [email protected]; Linuxarm
> Subject: Re: [PATCH V6 2/2] PCI/ACPI: hisi: Add ACPI support for
> HiSilicon SoCs Host Controllers
>
> On 22.11.2016 13:08, Dongdong Liu wrote:
> > PCIe controller in Hip05/HIP06/HIP07 SoCs is not ECAM compliant.
> > It is non ECAM only for the RC bus config space;for any other bus
> > underneath the root bus we support ECAM access.
> > Add specific quirks for PCI config space accessors.This involves:
> > 1. New initialization call hisi_pcie_init() to obtain rc base
> > addresses from PNP0C02 at the root of the ACPI namespace (under
> \_SB).
> > 2. New entry in common quirk array.
> >
> > Signed-off-by: Dongdong Liu <[email protected]>
> > Signed-off-by: Gabriele Paoloni <[email protected]>
> > ---
> > MAINTAINERS | 1 +
> > drivers/acpi/pci_mcfg.c | 13 +++++
> > drivers/pci/host/Kconfig | 7 +++
> > drivers/pci/host/Makefile | 1 +
> > drivers/pci/host/pcie-hisi-acpi.c | 119
> ++++++++++++++++++++++++++++++++++++++
> > include/linux/pci-ecam.h | 5 ++
> > 6 files changed, 146 insertions(+)
> > create mode 100644 drivers/pci/host/pcie-hisi-acpi.c
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 1cd38a7..b224caa 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -9358,6 +9358,7 @@ L: [email protected]
> > S: Maintained
> > F: Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
> > F: drivers/pci/host/pcie-hisi.c
> > +F: drivers/pci/host/pcie-hisi-acpi.c
> >
> > PCIE DRIVER FOR ROCKCHIP
> > M: Shawn Lin <[email protected]>
> > diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
> > index ac21db3..3297c5a 100644
> > --- a/drivers/acpi/pci_mcfg.c
> > +++ b/drivers/acpi/pci_mcfg.c
> > @@ -57,6 +57,19 @@ struct mcfg_fixup {
> > { "QCOM ", "QDF2432 ", 1, 5, MCFG_BUS_ANY, &pci_32b_ops },
> > { "QCOM ", "QDF2432 ", 1, 6, MCFG_BUS_ANY, &pci_32b_ops },
> > { "QCOM ", "QDF2432 ", 1, 7, MCFG_BUS_ANY, &pci_32b_ops },
> > +#ifdef CONFIG_PCI_ECAM_QUIRKS
> > + #define PCI_ACPI_QUIRK_QUAD_DOM(table_id, seg, ops) \
> > + { "HISI ", table_id, 0, seg + 0, MCFG_BUS_ANY, ops }, \
> > + { "HISI ", table_id, 0, seg + 1, MCFG_BUS_ANY, ops }, \
> > + { "HISI ", table_id, 0, seg + 2, MCFG_BUS_ANY, ops }, \
> > + { "HISI ", table_id, 0, seg + 3, MCFG_BUS_ANY, ops }
> > + PCI_ACPI_QUIRK_QUAD_DOM("HIP05 ", 0, &hisi_pcie_ops),
> > + PCI_ACPI_QUIRK_QUAD_DOM("HIP06 ", 0, &hisi_pcie_ops),
> > + PCI_ACPI_QUIRK_QUAD_DOM("HIP07 ", 0, &hisi_pcie_ops),
> > + PCI_ACPI_QUIRK_QUAD_DOM("HIP07 ", 4, &hisi_pcie_ops),
> > + PCI_ACPI_QUIRK_QUAD_DOM("HIP07 ", 8, &hisi_pcie_ops),
> > + PCI_ACPI_QUIRK_QUAD_DOM("HIP07 ", 12, &hisi_pcie_ops),
> > +#endif
> > };
> >
> > static char mcfg_oem_id[ACPI_OEM_ID_SIZE];
> > diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
> > index ae98644..1fbade5 100644
> > --- a/drivers/pci/host/Kconfig
> > +++ b/drivers/pci/host/Kconfig
> > @@ -301,4 +301,11 @@ config VMD
> > To compile this driver as a module, choose M here: the
> > module will be called vmd.
> >
> > +config PCI_ECAM_QUIRKS
> > + bool "PCI ECAM quirks for ARM64 platform"
> > + depends on PCI_ECAM && ARM64 && ACPI
> > + help
> > + Say y here to enable your platform-specific config access that
> > + is not ECAM compliant.
> > +
>
> ARM64 selects PCI_ECAM for ACPI anyway so we can skip it here.

Yes sure

>
> How about selecting PCI_ECAM_QUIRKS from ARM64 Kconfig:

I don't think we should assume that ARM64 platforms should
support quirks by default...
i.e. from my perspective the quirk module should go in only
if strictly required by the platform you're working on.

Thoughts?

Thanks

Gab

>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 30398db..9743186 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -105,6 +105,7 @@ config ARM64
> select OF_EARLY_FLATTREE
> select OF_RESERVED_MEM
> select PCI_ECAM if ACPI
> + select PCI_ECAM_QUIRKS if ACPI
> select POWER_RESET
> select POWER_SUPPLY
> select SPARSE_IRQ
>
> I agree with Bjorn, we should not have to turn on platform-specific
> config options for ARM64 & ACPI.
>
> Thanks,
> Tomasz

2016-11-22 15:55:44

by Lorenzo Pieralisi

[permalink] [raw]
Subject: Re: [PATCH V6 1/2] PCI/ACPI: Provide acpi_get_rc_resources() for ARM64 platform

On Tue, Nov 22, 2016 at 08:08:48PM +0800, Dongdong Liu wrote:
> The acpi_get_rc_resources() is used to get the RC register address that can
> not be described in MCFG. It takes the _HID&segment to look for and returns
> the RC address resource. Use PNP0C02 devices to describe such RC address
> resource. Use _UID to match segment to tell which root bus the PNP0C02
> resource belong to.

Here (ie interpreting the _UID as a segment identifier) is where we
start interpreting the specs instead of relying on them, if you want to
go ahead with this feel free but I do not like this attitude at all.

> Signed-off-by: Dongdong Liu <[email protected]>
> Signed-off-by: Tomasz Nowicki <[email protected]>
> ---
> drivers/pci/pci-acpi.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++
> drivers/pci/pci.h | 4 +++
> 2 files changed, 73 insertions(+)
>
> diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
> index d966d47..76fd6f4 100644
> --- a/drivers/pci/pci-acpi.c
> +++ b/drivers/pci/pci-acpi.c
> @@ -29,6 +29,75 @@
> 0x91, 0x17, 0xea, 0x4d, 0x19, 0xc3, 0x43, 0x4d
> };
>
> +#ifdef CONFIG_ARM64

CONFIG_PCI_ECAM_QUIRKS (that you will introduce earlier in your series).

The way you did it seems to imply that on ARM64 to get RC address we
need to (and are allowed to) use this function (and related, *Linux
specific*, ACPI bindings) and that's a generic way of getting config
space. It is not and it must not be and I consider it a bit of a stretch
to have it in generic code but I can live with that as long as it comes
with its own config option that describes that this stuff is
non-compliant code used by non-compliant platform drivers.

Thanks,
Lorenzo

> +static struct resource *acpi_get_rc_addr(struct acpi_device *adev)
> +{
> + struct resource_entry *entry;
> + struct list_head list;
> + unsigned long flags;
> + int ret;
> + struct resource *res;
> +
> + INIT_LIST_HEAD(&list);
> + flags = IORESOURCE_MEM;
> + ret = acpi_dev_get_resources(adev, &list,
> + acpi_dev_filter_resource_type_cb,
> + (void *) flags);
> + if (ret <= 0)
> + return NULL;
> +
> + entry = list_first_entry(&list, struct resource_entry, node);
> + res = entry->res;
> + acpi_dev_free_resource_list(&list);
> + return res;
> +}
> +
> +static acpi_status acpi_match_rc(acpi_handle handle, u32 lvl, void *context,
> + void **retval)
> +{
> + u16 *segment = context;
> + unsigned long long uid;
> + acpi_status status;
> +
> + status = acpi_evaluate_integer(handle, "_UID", NULL, &uid);
> + if (ACPI_FAILURE(status) || uid != *segment)
> + return AE_CTRL_DEPTH;
> +
> + *(acpi_handle *)retval = handle;
> + return AE_CTRL_TERMINATE;
> +}
> +
> +/**
> + * acpi_get_rc_resources() - get the RC address resource.
> + * @hid: HID to search for.
> + * @segment: PCI Segment number.
> + *
> + * Get the RC address resource that can not be described in MCFG. It takes
> + * the _HID&segment to look for and returns the RC address resource. Use
> + * _CRS of PNP0C02 devices to describe such RC address resource. Use _UID
> + * to match segment to tell which root bus the PNP0C02 resource belong to.
> + *
> + * Return: RC address resource.
> + */
> +struct resource *acpi_get_rc_resources(const char *hid, u16 segment)
> +{
> + struct acpi_device *adev;
> + acpi_status status;
> + acpi_handle handle;
> + int ret;
> +
> + status = acpi_get_devices(hid, acpi_match_rc, &segment, &handle);
> + if (ACPI_FAILURE(status))
> + return -ENODEV;
> +
> + ret = acpi_bus_get_device(handle, &adev);
> + if (ret)
> + return ret;
> +
> + return acpi_get_rc_addr(adev);
> +}
> +#endif
> +
> phys_addr_t acpi_pci_root_get_mcfg_addr(acpi_handle handle)
> {
> acpi_status status = AE_NOT_EXIST;
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 4518562..bf1dbfe 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -356,4 +356,8 @@ static inline int pci_dev_specific_reset(struct pci_dev *dev, int probe)
> }
> #endif
>
> +#if defined(CONFIG_ARM64) && defined(CONFIG_ACPI)
> +struct resource *acpi_get_rc_resources(const char *hid, u16 segment);
> +#endif
> +
> #endif /* DRIVERS_PCI_H */
> --
> 1.9.1
>

2016-11-22 16:10:28

by Gabriele Paoloni

[permalink] [raw]
Subject: RE: [PATCH V6 1/2] PCI/ACPI: Provide acpi_get_rc_resources() for ARM64 platform

Hi Lorenzo

Thanks for reviewing

> -----Original Message-----
> From: Lorenzo Pieralisi [mailto:[email protected]]
> Sent: 22 November 2016 15:57
> To: liudongdong (C)
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; Wangzhou (B); [email protected]; linux-
> [email protected]; [email protected]; linux-
> [email protected]; [email protected]; Gabriele Paoloni; Chenxin
> (Charles); [email protected]; Linuxarm
> Subject: Re: [PATCH V6 1/2] PCI/ACPI: Provide acpi_get_rc_resources()
> for ARM64 platform
>
> On Tue, Nov 22, 2016 at 08:08:48PM +0800, Dongdong Liu wrote:
> > The acpi_get_rc_resources() is used to get the RC register address
> that can
> > not be described in MCFG. It takes the _HID&segment to look for and
> returns
> > the RC address resource. Use PNP0C02 devices to describe such RC
> address
> > resource. Use _UID to match segment to tell which root bus the
> PNP0C02
> > resource belong to.
>
> Here (ie interpreting the _UID as a segment identifier) is where we
> start interpreting the specs instead of relying on them, if you want to
> go ahead with this feel free but I do not like this attitude at all.

Obviously there are no specs to specify the segment id of a quirk.
Looking at the ACPI specs it seems to me that the _UID is no worse
than other device identification objects...

Maybe we can use the _SUN as an alternative solution?

>
> > Signed-off-by: Dongdong Liu <[email protected]>
> > Signed-off-by: Tomasz Nowicki <[email protected]>
> > ---
> > drivers/pci/pci-acpi.c | 69
> ++++++++++++++++++++++++++++++++++++++++++++++++++
> > drivers/pci/pci.h | 4 +++
> > 2 files changed, 73 insertions(+)
> >
> > diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
> > index d966d47..76fd6f4 100644
> > --- a/drivers/pci/pci-acpi.c
> > +++ b/drivers/pci/pci-acpi.c
> > @@ -29,6 +29,75 @@
> > 0x91, 0x17, 0xea, 0x4d, 0x19, 0xc3, 0x43, 0x4d
> > };
> >
> > +#ifdef CONFIG_ARM64
>
> CONFIG_PCI_ECAM_QUIRKS (that you will introduce earlier in your
> series).
>
> The way you did it seems to imply that on ARM64 to get RC address we
> need to (and are allowed to) use this function (and related, *Linux
> specific*, ACPI bindings) and that's a generic way of getting config
> space. It is not and it must not be and I consider it a bit of a
> stretch
> to have it in generic code but I can live with that as long as it comes
> with its own config option that describes that this stuff is
> non-compliant code used by non-compliant platform drivers.

Yes probably better to have

#ifdef CONFIG_PCI_ECAM_QUIRKS

Thanks

Gab

>
> Thanks,
> Lorenzo
>
> > +static struct resource *acpi_get_rc_addr(struct acpi_device *adev)
> > +{
> > + struct resource_entry *entry;
> > + struct list_head list;
> > + unsigned long flags;
> > + int ret;
> > + struct resource *res;
> > +
> > + INIT_LIST_HEAD(&list);
> > + flags = IORESOURCE_MEM;
> > + ret = acpi_dev_get_resources(adev, &list,
> > + acpi_dev_filter_resource_type_cb,
> > + (void *) flags);
> > + if (ret <= 0)
> > + return NULL;
> > +
> > + entry = list_first_entry(&list, struct resource_entry, node);
> > + res = entry->res;
> > + acpi_dev_free_resource_list(&list);
> > + return res;
> > +}
> > +
> > +static acpi_status acpi_match_rc(acpi_handle handle, u32 lvl, void
> *context,
> > + void **retval)
> > +{
> > + u16 *segment = context;
> > + unsigned long long uid;
> > + acpi_status status;
> > +
> > + status = acpi_evaluate_integer(handle, "_UID", NULL, &uid);
> > + if (ACPI_FAILURE(status) || uid != *segment)
> > + return AE_CTRL_DEPTH;
> > +
> > + *(acpi_handle *)retval = handle;
> > + return AE_CTRL_TERMINATE;
> > +}
> > +
> > +/**
> > + * acpi_get_rc_resources() - get the RC address resource.
> > + * @hid: HID to search for.
> > + * @segment: PCI Segment number.
> > + *
> > + * Get the RC address resource that can not be described in MCFG. It
> takes
> > + * the _HID&segment to look for and returns the RC address resource.
> Use
> > + * _CRS of PNP0C02 devices to describe such RC address resource. Use
> _UID
> > + * to match segment to tell which root bus the PNP0C02 resource
> belong to.
> > + *
> > + * Return: RC address resource.
> > + */
> > +struct resource *acpi_get_rc_resources(const char *hid, u16 segment)
> > +{
> > + struct acpi_device *adev;
> > + acpi_status status;
> > + acpi_handle handle;
> > + int ret;
> > +
> > + status = acpi_get_devices(hid, acpi_match_rc, &segment, &handle);
> > + if (ACPI_FAILURE(status))
> > + return -ENODEV;
> > +
> > + ret = acpi_bus_get_device(handle, &adev);
> > + if (ret)
> > + return ret;
> > +
> > + return acpi_get_rc_addr(adev);
> > +}
> > +#endif
> > +
> > phys_addr_t acpi_pci_root_get_mcfg_addr(acpi_handle handle)
> > {
> > acpi_status status = AE_NOT_EXIST;
> > diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> > index 4518562..bf1dbfe 100644
> > --- a/drivers/pci/pci.h
> > +++ b/drivers/pci/pci.h
> > @@ -356,4 +356,8 @@ static inline int pci_dev_specific_reset(struct
> pci_dev *dev, int probe)
> > }
> > #endif
> >
> > +#if defined(CONFIG_ARM64) && defined(CONFIG_ACPI)
> > +struct resource *acpi_get_rc_resources(const char *hid, u16
> segment);
> > +#endif
> > +
> > #endif /* DRIVERS_PCI_H */
> > --
> > 1.9.1
> >

2016-11-22 17:12:39

by Lorenzo Pieralisi

[permalink] [raw]
Subject: Re: [PATCH V6 1/2] PCI/ACPI: Provide acpi_get_rc_resources() for ARM64 platform

On Tue, Nov 22, 2016 at 04:09:57PM +0000, Gabriele Paoloni wrote:

[...]

> > On Tue, Nov 22, 2016 at 08:08:48PM +0800, Dongdong Liu wrote:
> > > The acpi_get_rc_resources() is used to get the RC register address
> > that can
> > > not be described in MCFG. It takes the _HID&segment to look for and
> > returns
> > > the RC address resource. Use PNP0C02 devices to describe such RC
> > address
> > > resource. Use _UID to match segment to tell which root bus the
> > PNP0C02
> > > resource belong to.
> >
> > Here (ie interpreting the _UID as a segment identifier) is where we
> > start interpreting the specs instead of relying on them, if you want to
> > go ahead with this feel free but I do not like this attitude at all.
>
> Obviously there are no specs to specify the segment id of a quirk.

That's the reason why _UID (or any other standard object) should not be
(ab)used to do that, again, that's just my opinion.

> Looking at the ACPI specs it seems to me that the _UID is no worse
> than other device identification objects...

There are no bindings in the specs describing what you need. If you want
to say "for this _HID, _UID means something completely different
- ie root bridge segment identifier - go ahead if that's fine with Bjorn
and Rafael; another solution would be adding a _DSM/DSD to the PNP0c02
device to retrieve the segment number, other than that I have run out
of ideas.

Lorenzo

>
> >
> > > Signed-off-by: Dongdong Liu <[email protected]>
> > > Signed-off-by: Tomasz Nowicki <[email protected]>
> > > ---
> > > drivers/pci/pci-acpi.c | 69
> > ++++++++++++++++++++++++++++++++++++++++++++++++++
> > > drivers/pci/pci.h | 4 +++
> > > 2 files changed, 73 insertions(+)
> > >
> > > diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
> > > index d966d47..76fd6f4 100644
> > > --- a/drivers/pci/pci-acpi.c
> > > +++ b/drivers/pci/pci-acpi.c
> > > @@ -29,6 +29,75 @@
> > > 0x91, 0x17, 0xea, 0x4d, 0x19, 0xc3, 0x43, 0x4d
> > > };
> > >
> > > +#ifdef CONFIG_ARM64
> >
> > CONFIG_PCI_ECAM_QUIRKS (that you will introduce earlier in your
> > series).
> >
> > The way you did it seems to imply that on ARM64 to get RC address we
> > need to (and are allowed to) use this function (and related, *Linux
> > specific*, ACPI bindings) and that's a generic way of getting config
> > space. It is not and it must not be and I consider it a bit of a
> > stretch
> > to have it in generic code but I can live with that as long as it comes
> > with its own config option that describes that this stuff is
> > non-compliant code used by non-compliant platform drivers.
>
> Yes probably better to have
>
> #ifdef CONFIG_PCI_ECAM_QUIRKS
>
> Thanks
>
> Gab
>
> >
> > Thanks,
> > Lorenzo
> >
> > > +static struct resource *acpi_get_rc_addr(struct acpi_device *adev)
> > > +{
> > > + struct resource_entry *entry;
> > > + struct list_head list;
> > > + unsigned long flags;
> > > + int ret;
> > > + struct resource *res;
> > > +
> > > + INIT_LIST_HEAD(&list);
> > > + flags = IORESOURCE_MEM;
> > > + ret = acpi_dev_get_resources(adev, &list,
> > > + acpi_dev_filter_resource_type_cb,
> > > + (void *) flags);
> > > + if (ret <= 0)
> > > + return NULL;
> > > +
> > > + entry = list_first_entry(&list, struct resource_entry, node);
> > > + res = entry->res;
> > > + acpi_dev_free_resource_list(&list);
> > > + return res;
> > > +}
> > > +
> > > +static acpi_status acpi_match_rc(acpi_handle handle, u32 lvl, void
> > *context,
> > > + void **retval)
> > > +{
> > > + u16 *segment = context;
> > > + unsigned long long uid;
> > > + acpi_status status;
> > > +
> > > + status = acpi_evaluate_integer(handle, "_UID", NULL, &uid);
> > > + if (ACPI_FAILURE(status) || uid != *segment)
> > > + return AE_CTRL_DEPTH;
> > > +
> > > + *(acpi_handle *)retval = handle;
> > > + return AE_CTRL_TERMINATE;
> > > +}
> > > +
> > > +/**
> > > + * acpi_get_rc_resources() - get the RC address resource.
> > > + * @hid: HID to search for.
> > > + * @segment: PCI Segment number.
> > > + *
> > > + * Get the RC address resource that can not be described in MCFG. It
> > takes
> > > + * the _HID&segment to look for and returns the RC address resource.
> > Use
> > > + * _CRS of PNP0C02 devices to describe such RC address resource. Use
> > _UID
> > > + * to match segment to tell which root bus the PNP0C02 resource
> > belong to.
> > > + *
> > > + * Return: RC address resource.
> > > + */
> > > +struct resource *acpi_get_rc_resources(const char *hid, u16 segment)
> > > +{
> > > + struct acpi_device *adev;
> > > + acpi_status status;
> > > + acpi_handle handle;
> > > + int ret;
> > > +
> > > + status = acpi_get_devices(hid, acpi_match_rc, &segment, &handle);
> > > + if (ACPI_FAILURE(status))
> > > + return -ENODEV;
> > > +
> > > + ret = acpi_bus_get_device(handle, &adev);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + return acpi_get_rc_addr(adev);
> > > +}
> > > +#endif
> > > +
> > > phys_addr_t acpi_pci_root_get_mcfg_addr(acpi_handle handle)
> > > {
> > > acpi_status status = AE_NOT_EXIST;
> > > diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> > > index 4518562..bf1dbfe 100644
> > > --- a/drivers/pci/pci.h
> > > +++ b/drivers/pci/pci.h
> > > @@ -356,4 +356,8 @@ static inline int pci_dev_specific_reset(struct
> > pci_dev *dev, int probe)
> > > }
> > > #endif
> > >
> > > +#if defined(CONFIG_ARM64) && defined(CONFIG_ACPI)
> > > +struct resource *acpi_get_rc_resources(const char *hid, u16
> > segment);
> > > +#endif
> > > +
> > > #endif /* DRIVERS_PCI_H */
> > > --
> > > 1.9.1
> > >

2016-11-23 01:45:07

by Dongdong Liu

[permalink] [raw]
Subject: Re: [PATCH V6 1/2] PCI/ACPI: Provide acpi_get_rc_resources() for ARM64 platform

Hi Tomasz

在 2016/11/22 20:32, Tomasz Nowicki 写道:
> Hi Dongdong,
>
> On 22.11.2016 13:08, Dongdong Liu wrote:
>> The acpi_get_rc_resources() is used to get the RC register address that can
>> not be described in MCFG. It takes the _HID&segment to look for and returns
>> the RC address resource. Use PNP0C02 devices to describe such RC address
>> resource. Use _UID to match segment to tell which root bus the PNP0C02
>> resource belong to.
>>
>> Signed-off-by: Dongdong Liu <[email protected]>
>> Signed-off-by: Tomasz Nowicki <[email protected]>
>> ---
>> drivers/pci/pci-acpi.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++
>> drivers/pci/pci.h | 4 +++
>> 2 files changed, 73 insertions(+)
>>
>> diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
>> index d966d47..76fd6f4 100644
>> --- a/drivers/pci/pci-acpi.c
>> +++ b/drivers/pci/pci-acpi.c
>> @@ -29,6 +29,75 @@
>> 0x91, 0x17, 0xea, 0x4d, 0x19, 0xc3, 0x43, 0x4d
>> };
>>
>> +#ifdef CONFIG_ARM64
>> +static struct resource *acpi_get_rc_addr(struct acpi_device *adev)
>> +{
>> + struct resource_entry *entry;
>> + struct list_head list;
>> + unsigned long flags;
>> + int ret;
>> + struct resource *res;
>> +
>> + INIT_LIST_HEAD(&list);
>> + flags = IORESOURCE_MEM;
>> + ret = acpi_dev_get_resources(adev, &list,
>> + acpi_dev_filter_resource_type_cb,
>> + (void *) flags);
>> + if (ret <= 0)
>> + return NULL;
>> +
>> + entry = list_first_entry(&list, struct resource_entry, node);
>> + res = entry->res;
>
> You return "res" memory pointer and...
>
>> + acpi_dev_free_resource_list(&list);
>
> free it here.

acpi_dev_free_resource_list
--->resource_list_free
--->resource_list_destroy_entry
--->resource_list_free_entry
--->kfree(entry)
only free entry not free entry->res, so this is ok.

Thanks
Dongdong
>
>> + return res;
>> +}
>
>
> We either allocate memory for res here or get it from the caller.
>
> Tomasz
>
> .
>

2016-11-23 02:25:16

by Dongdong Liu

[permalink] [raw]
Subject: Re: [PATCH V6 1/2] PCI/ACPI: Provide acpi_get_rc_resources() for ARM64 platform

Hi Tomasz

在 2016/11/23 9:44, Dongdong Liu 写道:
> Hi Tomasz
>
> 在 2016/11/22 20:32, Tomasz Nowicki 写道:
>> Hi Dongdong,
>>
>> On 22.11.2016 13:08, Dongdong Liu wrote:
>>> The acpi_get_rc_resources() is used to get the RC register address that can
>>> not be described in MCFG. It takes the _HID&segment to look for and returns
>>> the RC address resource. Use PNP0C02 devices to describe such RC address
>>> resource. Use _UID to match segment to tell which root bus the PNP0C02
>>> resource belong to.
>>>
>>> Signed-off-by: Dongdong Liu <[email protected]>
>>> Signed-off-by: Tomasz Nowicki <[email protected]>
>>> ---
>>> drivers/pci/pci-acpi.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>> drivers/pci/pci.h | 4 +++
>>> 2 files changed, 73 insertions(+)
>>>
>>> diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
>>> index d966d47..76fd6f4 100644
>>> --- a/drivers/pci/pci-acpi.c
>>> +++ b/drivers/pci/pci-acpi.c
>>> @@ -29,6 +29,75 @@
>>> 0x91, 0x17, 0xea, 0x4d, 0x19, 0xc3, 0x43, 0x4d
>>> };
>>>
>>> +#ifdef CONFIG_ARM64
>>> +static struct resource *acpi_get_rc_addr(struct acpi_device *adev)
>>> +{
>>> + struct resource_entry *entry;
>>> + struct list_head list;
>>> + unsigned long flags;
>>> + int ret;
>>> + struct resource *res;
>>> +
>>> + INIT_LIST_HEAD(&list);
>>> + flags = IORESOURCE_MEM;
>>> + ret = acpi_dev_get_resources(adev, &list,
>>> + acpi_dev_filter_resource_type_cb,
>>> + (void *) flags);
>>> + if (ret <= 0)
>>> + return NULL;
>>> +
>>> + entry = list_first_entry(&list, struct resource_entry, node);
>>> + res = entry->res;
>>
>> You return "res" memory pointer and...
>>
>>> + acpi_dev_free_resource_list(&list);
>>
>> free it here.
>
> acpi_dev_free_resource_list
> --->resource_list_free
> --->resource_list_destroy_entry
> --->resource_list_free_entry
> --->kfree(entry)
> only free entry not free entry->res, so this is ok.

Sorry I am wrong, ignore this.
>
> Thanks
> Dongdong
>>
>>> + return res;
>>> +}
>>
>>
>> We either allocate memory for res here or get it from the caller.

Yes, you are right.I prefer to get if from the caller as PATCH V5 shows.

Thanks
Dongdong
>>
>> Tomasz
>>
>> .
>>
>
> _______________________________________________
> linuxarm mailing list
> [email protected]
> http://rnd-openeuler.huawei.com/mailman/listinfo/linuxarm

2016-11-23 09:44:43

by Graeme Gregory

[permalink] [raw]
Subject: Re: [PATCH V6 2/2] PCI/ACPI: hisi: Add ACPI support for HiSilicon SoCs Host Controllers

On Tue, Nov 22, 2016 at 03:05:29PM +0000, Gabriele Paoloni wrote:
> Hi Tomasz
>
> > -----Original Message-----
> > From: Tomasz Nowicki [mailto:[email protected]]
> > Sent: 22 November 2016 13:58
> > To: liudongdong (C); [email protected]; [email protected];
> > [email protected]; [email protected]; Wangzhou (B);
> > [email protected]
> > Cc: [email protected]; [email protected]; linux-
> > [email protected]; [email protected]; Gabriele Paoloni; Chenxin
> > (Charles); [email protected]; Linuxarm
> > Subject: Re: [PATCH V6 2/2] PCI/ACPI: hisi: Add ACPI support for
> > HiSilicon SoCs Host Controllers
> >
> > On 22.11.2016 13:08, Dongdong Liu wrote:
> > > PCIe controller in Hip05/HIP06/HIP07 SoCs is not ECAM compliant.
> > > It is non ECAM only for the RC bus config space;for any other bus
> > > underneath the root bus we support ECAM access.
> > > Add specific quirks for PCI config space accessors.This involves:
> > > 1. New initialization call hisi_pcie_init() to obtain rc base
> > > addresses from PNP0C02 at the root of the ACPI namespace (under
> > \_SB).
> > > 2. New entry in common quirk array.
> > >
> > > Signed-off-by: Dongdong Liu <[email protected]>
> > > Signed-off-by: Gabriele Paoloni <[email protected]>
> > > ---
> > > MAINTAINERS | 1 +
> > > drivers/acpi/pci_mcfg.c | 13 +++++
> > > drivers/pci/host/Kconfig | 7 +++
> > > drivers/pci/host/Makefile | 1 +
> > > drivers/pci/host/pcie-hisi-acpi.c | 119
> > ++++++++++++++++++++++++++++++++++++++
> > > include/linux/pci-ecam.h | 5 ++
> > > 6 files changed, 146 insertions(+)
> > > create mode 100644 drivers/pci/host/pcie-hisi-acpi.c
> > >
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > index 1cd38a7..b224caa 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -9358,6 +9358,7 @@ L: [email protected]
> > > S: Maintained
> > > F: Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
> > > F: drivers/pci/host/pcie-hisi.c
> > > +F: drivers/pci/host/pcie-hisi-acpi.c
> > >
> > > PCIE DRIVER FOR ROCKCHIP
> > > M: Shawn Lin <[email protected]>
> > > diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
> > > index ac21db3..3297c5a 100644
> > > --- a/drivers/acpi/pci_mcfg.c
> > > +++ b/drivers/acpi/pci_mcfg.c
> > > @@ -57,6 +57,19 @@ struct mcfg_fixup {
> > > { "QCOM ", "QDF2432 ", 1, 5, MCFG_BUS_ANY, &pci_32b_ops },
> > > { "QCOM ", "QDF2432 ", 1, 6, MCFG_BUS_ANY, &pci_32b_ops },
> > > { "QCOM ", "QDF2432 ", 1, 7, MCFG_BUS_ANY, &pci_32b_ops },
> > > +#ifdef CONFIG_PCI_ECAM_QUIRKS
> > > + #define PCI_ACPI_QUIRK_QUAD_DOM(table_id, seg, ops) \
> > > + { "HISI ", table_id, 0, seg + 0, MCFG_BUS_ANY, ops }, \
> > > + { "HISI ", table_id, 0, seg + 1, MCFG_BUS_ANY, ops }, \
> > > + { "HISI ", table_id, 0, seg + 2, MCFG_BUS_ANY, ops }, \
> > > + { "HISI ", table_id, 0, seg + 3, MCFG_BUS_ANY, ops }
> > > + PCI_ACPI_QUIRK_QUAD_DOM("HIP05 ", 0, &hisi_pcie_ops),
> > > + PCI_ACPI_QUIRK_QUAD_DOM("HIP06 ", 0, &hisi_pcie_ops),
> > > + PCI_ACPI_QUIRK_QUAD_DOM("HIP07 ", 0, &hisi_pcie_ops),
> > > + PCI_ACPI_QUIRK_QUAD_DOM("HIP07 ", 4, &hisi_pcie_ops),
> > > + PCI_ACPI_QUIRK_QUAD_DOM("HIP07 ", 8, &hisi_pcie_ops),
> > > + PCI_ACPI_QUIRK_QUAD_DOM("HIP07 ", 12, &hisi_pcie_ops),
> > > +#endif
> > > };
> > >
> > > static char mcfg_oem_id[ACPI_OEM_ID_SIZE];
> > > diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
> > > index ae98644..1fbade5 100644
> > > --- a/drivers/pci/host/Kconfig
> > > +++ b/drivers/pci/host/Kconfig
> > > @@ -301,4 +301,11 @@ config VMD
> > > To compile this driver as a module, choose M here: the
> > > module will be called vmd.
> > >
> > > +config PCI_ECAM_QUIRKS
> > > + bool "PCI ECAM quirks for ARM64 platform"
> > > + depends on PCI_ECAM && ARM64 && ACPI
> > > + help
> > > + Say y here to enable your platform-specific config access that
> > > + is not ECAM compliant.
> > > +
> >
> > ARM64 selects PCI_ECAM for ACPI anyway so we can skip it here.
>
> Yes sure
>
> >
> > How about selecting PCI_ECAM_QUIRKS from ARM64 Kconfig:
>
> I don't think we should assume that ARM64 platforms should
> support quirks by default...
> i.e. from my perspective the quirk module should go in only
> if strictly required by the platform you're working on.
>
> Thoughts?
>
> Thanks
>
No, same kernel multiple machines is the model, code should go in
unconditional.

Graeme

2016-11-23 14:09:36

by Gabriele Paoloni

[permalink] [raw]
Subject: RE: [PATCH V6 2/2] PCI/ACPI: hisi: Add ACPI support for HiSilicon SoCs Host Controllers

Hi Graeme

> -----Original Message-----
> From: Graeme Gregory [mailto:[email protected]]
> Sent: 23 November 2016 09:44
> To: Gabriele Paoloni
> Cc: Tomasz Nowicki; liudongdong (C); [email protected]; [email protected];
> [email protected]; [email protected]; Wangzhou (B);
> [email protected]; [email protected]; linux-
> [email protected]; [email protected]; [email protected];
> Chenxin (Charles); [email protected]; Linuxarm
> Subject: Re: [PATCH V6 2/2] PCI/ACPI: hisi: Add ACPI support for
> HiSilicon SoCs Host Controllers
>
> On Tue, Nov 22, 2016 at 03:05:29PM +0000, Gabriele Paoloni wrote:
> > Hi Tomasz
> >
> > > -----Original Message-----
> > > From: Tomasz Nowicki [mailto:[email protected]]
> > > Sent: 22 November 2016 13:58
> > > To: liudongdong (C); [email protected]; [email protected];
> > > [email protected]; [email protected]; Wangzhou (B);
> > > [email protected]
> > > Cc: [email protected]; [email protected]; linux-
> > > [email protected]; [email protected]; Gabriele Paoloni; Chenxin
> > > (Charles); [email protected]; Linuxarm
> > > Subject: Re: [PATCH V6 2/2] PCI/ACPI: hisi: Add ACPI support for
> > > HiSilicon SoCs Host Controllers
> > >
> > > On 22.11.2016 13:08, Dongdong Liu wrote:
> > > > PCIe controller in Hip05/HIP06/HIP07 SoCs is not ECAM compliant.
> > > > It is non ECAM only for the RC bus config space;for any other bus
> > > > underneath the root bus we support ECAM access.
> > > > Add specific quirks for PCI config space accessors.This involves:
> > > > 1. New initialization call hisi_pcie_init() to obtain rc base
> > > > addresses from PNP0C02 at the root of the ACPI namespace (under
> > > \_SB).
> > > > 2. New entry in common quirk array.
> > > >
> > > > Signed-off-by: Dongdong Liu <[email protected]>
> > > > Signed-off-by: Gabriele Paoloni <[email protected]>
> > > > ---
> > > > MAINTAINERS | 1 +
> > > > drivers/acpi/pci_mcfg.c | 13 +++++
> > > > drivers/pci/host/Kconfig | 7 +++
> > > > drivers/pci/host/Makefile | 1 +
> > > > drivers/pci/host/pcie-hisi-acpi.c | 119
> > > ++++++++++++++++++++++++++++++++++++++
> > > > include/linux/pci-ecam.h | 5 ++
> > > > 6 files changed, 146 insertions(+)
> > > > create mode 100644 drivers/pci/host/pcie-hisi-acpi.c
> > > >
> > > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > > index 1cd38a7..b224caa 100644
> > > > --- a/MAINTAINERS
> > > > +++ b/MAINTAINERS
> > > > @@ -9358,6 +9358,7 @@ L: [email protected]
> > > > S: Maintained
> > > > F: Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
> > > > F: drivers/pci/host/pcie-hisi.c
> > > > +F: drivers/pci/host/pcie-hisi-acpi.c
> > > >
> > > > PCIE DRIVER FOR ROCKCHIP
> > > > M: Shawn Lin <[email protected]>
> > > > diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
> > > > index ac21db3..3297c5a 100644
> > > > --- a/drivers/acpi/pci_mcfg.c
> > > > +++ b/drivers/acpi/pci_mcfg.c
> > > > @@ -57,6 +57,19 @@ struct mcfg_fixup {
> > > > { "QCOM ", "QDF2432 ", 1, 5, MCFG_BUS_ANY, &pci_32b_ops },
> > > > { "QCOM ", "QDF2432 ", 1, 6, MCFG_BUS_ANY, &pci_32b_ops },
> > > > { "QCOM ", "QDF2432 ", 1, 7, MCFG_BUS_ANY, &pci_32b_ops },
> > > > +#ifdef CONFIG_PCI_ECAM_QUIRKS
> > > > + #define PCI_ACPI_QUIRK_QUAD_DOM(table_id, seg, ops) \
> > > > + { "HISI ", table_id, 0, seg + 0, MCFG_BUS_ANY, ops }, \
> > > > + { "HISI ", table_id, 0, seg + 1, MCFG_BUS_ANY, ops }, \
> > > > + { "HISI ", table_id, 0, seg + 2, MCFG_BUS_ANY, ops }, \
> > > > + { "HISI ", table_id, 0, seg + 3, MCFG_BUS_ANY, ops }
> > > > + PCI_ACPI_QUIRK_QUAD_DOM("HIP05 ", 0, &hisi_pcie_ops),
> > > > + PCI_ACPI_QUIRK_QUAD_DOM("HIP06 ", 0, &hisi_pcie_ops),
> > > > + PCI_ACPI_QUIRK_QUAD_DOM("HIP07 ", 0, &hisi_pcie_ops),
> > > > + PCI_ACPI_QUIRK_QUAD_DOM("HIP07 ", 4, &hisi_pcie_ops),
> > > > + PCI_ACPI_QUIRK_QUAD_DOM("HIP07 ", 8, &hisi_pcie_ops),
> > > > + PCI_ACPI_QUIRK_QUAD_DOM("HIP07 ", 12, &hisi_pcie_ops),
> > > > +#endif
> > > > };
> > > >
> > > > static char mcfg_oem_id[ACPI_OEM_ID_SIZE];
> > > > diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
> > > > index ae98644..1fbade5 100644
> > > > --- a/drivers/pci/host/Kconfig
> > > > +++ b/drivers/pci/host/Kconfig
> > > > @@ -301,4 +301,11 @@ config VMD
> > > > To compile this driver as a module, choose M here: the
> > > > module will be called vmd.
> > > >
> > > > +config PCI_ECAM_QUIRKS
> > > > + bool "PCI ECAM quirks for ARM64 platform"
> > > > + depends on PCI_ECAM && ARM64 && ACPI
> > > > + help
> > > > + Say y here to enable your platform-specific config access
> that
> > > > + is not ECAM compliant.
> > > > +
> > >
> > > ARM64 selects PCI_ECAM for ACPI anyway so we can skip it here.
> >
> > Yes sure
> >
> > >
> > > How about selecting PCI_ECAM_QUIRKS from ARM64 Kconfig:
> >
> > I don't think we should assume that ARM64 platforms should
> > support quirks by default...
> > i.e. from my perspective the quirk module should go in only
> > if strictly required by the platform you're working on.
> >
> > Thoughts?
> >
> > Thanks
> >
> No, same kernel multiple machines is the model, code should go in
> unconditional.

What about selecting it in "arch/arm64/Kconfig.platforms" ?

So it seems more logical (as the quirk is not selected directly by
the ARM64 architecture) and would solve the "same kernel multiple
machine" issue...

Thanks

Gab

>
> Graeme

2016-12-22 08:31:50

by Ming Lei

[permalink] [raw]
Subject: Re: [PATCH V6 0/2] Add ACPI support for HiSilicon SoCs Host Controllers

Hi Dongdong,

On Tue, Nov 22, 2016 at 8:08 PM, Dongdong Liu <[email protected]> wrote:
> This patchset adds ACPI support for the HiSilicon Hip05/Hip06/Hip07 SoC
> PCIe controllers.
> The two patches respectively:
> - provides the common function acpi_get_rc_resources() for ARM64
> platform.
> - adds the HiSilicon ACPI specific quirks.
>
> This patchset is based on branch pci/ecam-v6
> It can be found here:
> https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git(pci/ecam-v6)
>
> This patchset has been tested on HiSilicon D03 board.
> The dmesg log, /proc/iomem, and ACPI table information can be found:
> https://bugzilla.kernel.org/show_bug.cgi?id=187961
>
> v5 -> v6:
> - change the config option to CONFIG_PCI_ECAM_QUIRKS.
> - fix some commets about acpi_get_rc_resources().

Could you post out v7 for fixing conflicts against current linus tree?

BTW, I tried to fix the conflicts by myself, but still caues the following
build failure:

[[email protected]]$make -j4 CROSS_COMPILE=aarch64-linux-gnu-
ARCH=arm64 drivers/pci/host/pcie-hisi-acpi.o
CHK include/config/kernel.release
CHK include/generated/uapi/linux/version.h
CHK include/generated/utsrelease.h
CHK include/generated/timeconst.h
CHK include/generated/bounds.h
CHK include/generated/asm-offsets.h
CALL scripts/checksyscalls.sh
CC drivers/pci/host/pcie-hisi-acpi.o
In file included from drivers/pci/host/pcie-hisi-acpi.c:16:0:
drivers/pci/host/../pci.h:357:18: error: conflicting types for
‘acpi_get_rc_resources’
struct resource *acpi_get_rc_resources(const char *hid, u16 segment);
^
drivers/pci/host/../pci.h:352:5: note: previous declaration of
‘acpi_get_rc_resources’ was here
int acpi_get_rc_resources(struct device *dev, const char *hid, u16 segment,
^
scripts/Makefile.build:293: recipe for target
'drivers/pci/host/pcie-hisi-acpi.o' failed
make[1]: *** [drivers/pci/host/pcie-hisi-acpi.o] Error 1
Makefile:1640: recipe for target 'drivers/pci/host/pcie-hisi-acpi.o' failed
make: *** [drivers/pci/host/pcie-hisi-acpi.o] Error 2


Thanks,
Ming

>
> v4 -> v5:
> - obtain rc base addresses from PNP0C02 at the root of the ACPI
> namespace (under \_SB) instead of from sub-device under the RC.
> - merge the rewrited get rc resources code by Tomasz.
> - delete unused code.
> - drop the PATCH V4 1/2, will rework late as a separate patch.
>
> v3 -> v4:
> - rebase on pci/ecam-v6.
> - delete the unnecessary link_up check code.
>
> v2 -> v3:
> - rebase against 4.9-rc1 and add Tomasz quirks V6 pathcset.
> - obtain rc base addresses from PNP0C02 as subdevice of PNP0A03 instead of
> hardcode the addresses.
> - modify hisi_pcie_acpi_rd_conf/hisi_pcie_acpi_wr_conf() according to
> Arnd comments.
>
> v1 -> v2:
> - rebase against Tomasz RFC V5 quirk mechanism
> - add ACPI support for the HiSilicon Hip07 SoC PCIe controllers.
>
> Dongdong Liu (2):
> PCI/ACPI: Provide acpi_get_rc_resources() for ARM64 platform
> PCI/ACPI: hisi: Add ACPI support for HiSilicon SoCs Host Controllers
>
> MAINTAINERS | 1 +
> drivers/acpi/pci_mcfg.c | 13 +++++
> drivers/pci/host/Kconfig | 7 +++
> drivers/pci/host/Makefile | 1 +
> drivers/pci/host/pcie-hisi-acpi.c | 119 ++++++++++++++++++++++++++++++++++++++
> drivers/pci/pci-acpi.c | 69 ++++++++++++++++++++++
> drivers/pci/pci.h | 4 ++
> include/linux/pci-ecam.h | 5 ++
> 8 files changed, 219 insertions(+)
> create mode 100644 drivers/pci/host/pcie-hisi-acpi.c
>
> --
> 1.9.1
>



--
Ming Lei

2016-12-22 12:32:27

by Dongdong Liu

[permalink] [raw]
Subject: Re: [PATCH V6 0/2] Add ACPI support for HiSilicon SoCs Host Controllers

Hi Ming

The latest patchset is [PATCH v11 00/15] PCI: ARM64 ECAM quirks
You can get them from https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git (pci/ecam)

Thanks
Dongdong
在 2016/12/22 16:31, Ming Lei 写道:
> Hi Dongdong,
>
> On Tue, Nov 22, 2016 at 8:08 PM, Dongdong Liu <[email protected]> wrote:
>> This patchset adds ACPI support for the HiSilicon Hip05/Hip06/Hip07 SoC
>> PCIe controllers.
>> The two patches respectively:
>> - provides the common function acpi_get_rc_resources() for ARM64
>> platform.
>> - adds the HiSilicon ACPI specific quirks.
>>
>> This patchset is based on branch pci/ecam-v6
>> It can be found here:
>> https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git(pci/ecam-v6)
>>
>> This patchset has been tested on HiSilicon D03 board.
>> The dmesg log, /proc/iomem, and ACPI table information can be found:
>> https://bugzilla.kernel.org/show_bug.cgi?id=187961
>>
>> v5 -> v6:
>> - change the config option to CONFIG_PCI_ECAM_QUIRKS.
>> - fix some commets about acpi_get_rc_resources().
>
> Could you post out v7 for fixing conflicts against current linus tree?
>
> BTW, I tried to fix the conflicts by myself, but still caues the following
> build failure:
>
> [[email protected]]$make -j4 CROSS_COMPILE=aarch64-linux-gnu-
> ARCH=arm64 drivers/pci/host/pcie-hisi-acpi.o
> CHK include/config/kernel.release
> CHK include/generated/uapi/linux/version.h
> CHK include/generated/utsrelease.h
> CHK include/generated/timeconst.h
> CHK include/generated/bounds.h
> CHK include/generated/asm-offsets.h
> CALL scripts/checksyscalls.sh
> CC drivers/pci/host/pcie-hisi-acpi.o
> In file included from drivers/pci/host/pcie-hisi-acpi.c:16:0:
> drivers/pci/host/../pci.h:357:18: error: conflicting types for
> ‘acpi_get_rc_resources’
> struct resource *acpi_get_rc_resources(const char *hid, u16 segment);
> ^
> drivers/pci/host/../pci.h:352:5: note: previous declaration of
> ‘acpi_get_rc_resources’ was here
> int acpi_get_rc_resources(struct device *dev, const char *hid, u16 segment,
> ^
> scripts/Makefile.build:293: recipe for target
> 'drivers/pci/host/pcie-hisi-acpi.o' failed
> make[1]: *** [drivers/pci/host/pcie-hisi-acpi.o] Error 1
> Makefile:1640: recipe for target 'drivers/pci/host/pcie-hisi-acpi.o' failed
> make: *** [drivers/pci/host/pcie-hisi-acpi.o] Error 2
>
>
> Thanks,
> Ming
>
>>
>> v4 -> v5:
>> - obtain rc base addresses from PNP0C02 at the root of the ACPI
>> namespace (under \_SB) instead of from sub-device under the RC.
>> - merge the rewrited get rc resources code by Tomasz.
>> - delete unused code.
>> - drop the PATCH V4 1/2, will rework late as a separate patch.
>>
>> v3 -> v4:
>> - rebase on pci/ecam-v6.
>> - delete the unnecessary link_up check code.
>>
>> v2 -> v3:
>> - rebase against 4.9-rc1 and add Tomasz quirks V6 pathcset.
>> - obtain rc base addresses from PNP0C02 as subdevice of PNP0A03 instead of
>> hardcode the addresses.
>> - modify hisi_pcie_acpi_rd_conf/hisi_pcie_acpi_wr_conf() according to
>> Arnd comments.
>>
>> v1 -> v2:
>> - rebase against Tomasz RFC V5 quirk mechanism
>> - add ACPI support for the HiSilicon Hip07 SoC PCIe controllers.
>>
>> Dongdong Liu (2):
>> PCI/ACPI: Provide acpi_get_rc_resources() for ARM64 platform
>> PCI/ACPI: hisi: Add ACPI support for HiSilicon SoCs Host Controllers
>>
>> MAINTAINERS | 1 +
>> drivers/acpi/pci_mcfg.c | 13 +++++
>> drivers/pci/host/Kconfig | 7 +++
>> drivers/pci/host/Makefile | 1 +
>> drivers/pci/host/pcie-hisi-acpi.c | 119 ++++++++++++++++++++++++++++++++++++++
>> drivers/pci/pci-acpi.c | 69 ++++++++++++++++++++++
>> drivers/pci/pci.h | 4 ++
>> include/linux/pci-ecam.h | 5 ++
>> 8 files changed, 219 insertions(+)
>> create mode 100644 drivers/pci/host/pcie-hisi-acpi.c
>>
>> --
>> 1.9.1
>>
>
>
>

2016-12-22 12:40:18

by Ming Lei

[permalink] [raw]
Subject: Re: [PATCH V6 0/2] Add ACPI support for HiSilicon SoCs Host Controllers

Hi Dongdong,

On Thu, Dec 22, 2016 at 8:30 PM, Dongdong Liu <[email protected]> wrote:
> Hi Ming
>
> The latest patchset is [PATCH v11 00/15] PCI: ARM64 ECAM quirks
> You can get them from
> https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git (pci/ecam)

OK, thanks, looks it has been in linus tree.

Thanks,
Ming

>
> Thanks
> Dongdong
>
> 在 2016/12/22 16:31, Ming Lei 写道:
>>
>> Hi Dongdong,
>>
>> On Tue, Nov 22, 2016 at 8:08 PM, Dongdong Liu <[email protected]>
>> wrote:
>>>
>>> This patchset adds ACPI support for the HiSilicon Hip05/Hip06/Hip07 SoC
>>> PCIe controllers.
>>> The two patches respectively:
>>> - provides the common function acpi_get_rc_resources() for ARM64
>>> platform.
>>> - adds the HiSilicon ACPI specific quirks.
>>>
>>> This patchset is based on branch pci/ecam-v6
>>> It can be found here:
>>>
>>> https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git(pci/ecam-v6)
>>>
>>> This patchset has been tested on HiSilicon D03 board.
>>> The dmesg log, /proc/iomem, and ACPI table information can be found:
>>> https://bugzilla.kernel.org/show_bug.cgi?id=187961
>>>
>>> v5 -> v6:
>>> - change the config option to CONFIG_PCI_ECAM_QUIRKS.
>>> - fix some commets about acpi_get_rc_resources().
>>
>>
>> Could you post out v7 for fixing conflicts against current linus tree?
>>
>> BTW, I tried to fix the conflicts by myself, but still caues the following
>> build failure:
>>
>> [[email protected]]$make -j4 CROSS_COMPILE=aarch64-linux-gnu-
>> ARCH=arm64 drivers/pci/host/pcie-hisi-acpi.o
>> CHK include/config/kernel.release
>> CHK include/generated/uapi/linux/version.h
>> CHK include/generated/utsrelease.h
>> CHK include/generated/timeconst.h
>> CHK include/generated/bounds.h
>> CHK include/generated/asm-offsets.h
>> CALL scripts/checksyscalls.sh
>> CC drivers/pci/host/pcie-hisi-acpi.o
>> In file included from drivers/pci/host/pcie-hisi-acpi.c:16:0:
>> drivers/pci/host/../pci.h:357:18: error: conflicting types for
>> ‘acpi_get_rc_resources’
>> struct resource *acpi_get_rc_resources(const char *hid, u16 segment);
>> ^
>> drivers/pci/host/../pci.h:352:5: note: previous declaration of
>> ‘acpi_get_rc_resources’ was here
>> int acpi_get_rc_resources(struct device *dev, const char *hid, u16
>> segment,
>> ^
>> scripts/Makefile.build:293: recipe for target
>> 'drivers/pci/host/pcie-hisi-acpi.o' failed
>> make[1]: *** [drivers/pci/host/pcie-hisi-acpi.o] Error 1
>> Makefile:1640: recipe for target 'drivers/pci/host/pcie-hisi-acpi.o'
>> failed
>> make: *** [drivers/pci/host/pcie-hisi-acpi.o] Error 2
>>
>>
>> Thanks,
>> Ming
>>
>>>
>>> v4 -> v5:
>>> - obtain rc base addresses from PNP0C02 at the root of the ACPI
>>> namespace (under \_SB) instead of from sub-device under the RC.
>>> - merge the rewrited get rc resources code by Tomasz.
>>> - delete unused code.
>>> - drop the PATCH V4 1/2, will rework late as a separate patch.
>>>
>>> v3 -> v4:
>>> - rebase on pci/ecam-v6.
>>> - delete the unnecessary link_up check code.
>>>
>>> v2 -> v3:
>>> - rebase against 4.9-rc1 and add Tomasz quirks V6 pathcset.
>>> - obtain rc base addresses from PNP0C02 as subdevice of PNP0A03 instead
>>> of
>>> hardcode the addresses.
>>> - modify hisi_pcie_acpi_rd_conf/hisi_pcie_acpi_wr_conf() according to
>>> Arnd comments.
>>>
>>> v1 -> v2:
>>> - rebase against Tomasz RFC V5 quirk mechanism
>>> - add ACPI support for the HiSilicon Hip07 SoC PCIe controllers.
>>>
>>> Dongdong Liu (2):
>>> PCI/ACPI: Provide acpi_get_rc_resources() for ARM64 platform
>>> PCI/ACPI: hisi: Add ACPI support for HiSilicon SoCs Host Controllers
>>>
>>> MAINTAINERS | 1 +
>>> drivers/acpi/pci_mcfg.c | 13 +++++
>>> drivers/pci/host/Kconfig | 7 +++
>>> drivers/pci/host/Makefile | 1 +
>>> drivers/pci/host/pcie-hisi-acpi.c | 119
>>> ++++++++++++++++++++++++++++++++++++++
>>> drivers/pci/pci-acpi.c | 69 ++++++++++++++++++++++
>>> drivers/pci/pci.h | 4 ++
>>> include/linux/pci-ecam.h | 5 ++
>>> 8 files changed, 219 insertions(+)
>>> create mode 100644 drivers/pci/host/pcie-hisi-acpi.c
>>>
>>> --
>>> 1.9.1
>>>
>>
>>
>>
>



--
Ming Lei