Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752731AbcKQLjT (ORCPT ); Thu, 17 Nov 2016 06:39:19 -0500 Received: from szxga01-in.huawei.com ([58.251.152.64]:9652 "EHLO szxga01-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751166AbcKQLjP (ORCPT ); Thu, 17 Nov 2016 06:39:15 -0500 Subject: Re: [PATCH V4 2/2] PCI/ACPI: hisi: Add ACPI support for HiSilicon SoCs Host Controllers To: Tomasz Nowicki , Bjorn Helgaas References: <1478682897-119874-1-git-send-email-liudongdong3@huawei.com> <1478682897-119874-3-git-send-email-liudongdong3@huawei.com> <20161114233320.GB30262@bhelgaas-glaptop.roam.corp.google.com> <20161116230040.GA19913@bhelgaas-glaptop.roam.corp.google.com> <350379a5-733e-a91d-a3c1-ca932e8bde72@huawei.com> <326c0cd0-2a93-92b0-1293-4b8a0f99db8e@semihalf.com> CC: , , , , , , , , , , , , From: Dongdong Liu Message-ID: <5ae66256-db18-1fd4-60e2-9cec82d5e8d1@huawei.com> Date: Thu, 17 Nov 2016 19:29:57 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <326c0cd0-2a93-92b0-1293-4b8a0f99db8e@semihalf.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.63.141.93] X-CFilter-Loop: Reflected Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 16522 Lines: 483 Hi Tomasz 在 2016/11/17 16:28, Tomasz Nowicki 写道: > Hi Dongdong, > > I rewrite your code so that it could be used for ThunderX as well. The rewrited code looks good to me. > This assumes _UID&segment is the right way of looking up corelated RC. > Of course acpi_get_rc_resources() and its acpi_* helpers should go to pci-acpi.c. I am ok about this. Thanks Dongdong > > Tomasz > > static int acpi_get_rc_addr(struct acpi_device *adev, struct resource *res) > { > struct resource_entry *entry; > struct list_head list; > unsigned long flags; > int ret; > > INIT_LIST_HEAD(&list); > flags = IORESOURCE_MEM; > ret = acpi_dev_get_resources(adev, &list, > acpi_dev_filter_resource_type_cb, (void *) flags); > if (ret < 0) { > dev_err(&adev->dev, > "failed to parse _CRS method, error code %d\n", > ret); > return ret; > } else if (ret == 0) { > dev_err(&adev->dev, > "no IO and memory resources present in _CRS\n"); > return -EINVAL; > } > > entry = list_first_entry(&list, struct resource_entry, node); > *res = *entry->res; > acpi_dev_free_resource_list(&list); > return 0; > } > > static acpi_status acpi_match_rc(acpi_handle handle, u32 lvl, void *context, > void **retval) > { > u16 *segment = context; > unsigned long long uid; > acpi_status status; > > status = acpi_evaluate_integer(handle, "_UID", NULL, &uid); > if (ACPI_FAILURE(status) || uid != *segment) > return AE_CTRL_DEPTH; > > *(acpi_handle *)retval = handle; > return AE_CTRL_TERMINATE; > } > > static int acpi_get_rc_resources(const char *hid, u16 segment, > struct resource *res) > { > struct acpi_device *adev; > acpi_status status; > acpi_handle handle; > int ret; > > status = acpi_get_devices(hid, acpi_match_rc, &segment, &handle); > if (ACPI_FAILURE(status)) { > pr_err("Can't find _HID %s device", hid); > return -ENODEV; > } > > ret = acpi_bus_get_device(handle, &adev); > if (ret) > return ret; > > ret = acpi_get_rc_addr(adev, res); > if (ret) { > dev_err(&adev->dev, "can't get RC resource"); > return ret; > } > > return 0; > } > > static int hisi_pcie_init(struct pci_config_window *cfg) > { > struct acpi_device *adev = to_acpi_device(cfg->parent); > struct acpi_pci_root *root = acpi_driver_data(adev); > void __iomem *reg_base; > struct resource *res; > acpi_status status; > acpi_handle handle; > int ret; > > res = devm_kzalloc(&adev->dev, sizeof(*res), GFP_KERNEL); > if (!res) > return -ENOMEM; > > ret = acpi_get_rc_resources("HISI0081", root->segment, res); > if (ret) { > dev_err(&adev->dev, "can't get rc base address"); > return ret; > } > > reg_base = devm_ioremap(&adev->dev, res->start, resource_size(res)); > if (!reg_base) > return -ENOMEM; > > cfg->priv = reg_base; > return 0; > } > > On 17.11.2016 04:02, Dongdong Liu wrote: >> Hi Bjorn >> 在 2016/11/17 7:00, Bjorn Helgaas 写道: >>> On Mon, Nov 14, 2016 at 05:33:20PM -0600, Bjorn Helgaas wrote: >>>> On Wed, Nov 09, 2016 at 05:14:57PM +0800, Dongdong Liu wrote: >>>>> PCIe controller in Hip05/HIP06/HIP07 SoCs is not ECAM compliant. >>>>> It is non ECAM only for the RC bus config space;for any other bus >>>>> underneath the root bus we support ECAM access. >>>>> Add specific quirks for PCI config space accessors.This involves: >>>>> 1. New initialization call hisi_pcie_init() to obtain rc base >>>>> addresses from PNP0C02 as subdevice of PNP0A03. >>>>> 2. New entry in common quirk array. >>>>> >>>>> Signed-off-by: Dongdong Liu >>>>> Signed-off-by: Gabriele Paoloni >>>>> --- >>>>> MAINTAINERS | 1 + >>>>> drivers/acpi/pci_mcfg.c | 13 ++++ >>>>> drivers/pci/host/Kconfig | 8 ++ >>>>> drivers/pci/host/Makefile | 1 + >>>>> drivers/pci/host/pcie-hisi-acpi.c | 157 >>>>> ++++++++++++++++++++++++++++++++++++++ >>>>> include/linux/pci-ecam.h | 5 ++ >>>>> 6 files changed, 185 insertions(+) >>>>> create mode 100644 drivers/pci/host/pcie-hisi-acpi.c >>>>> >>>>> diff --git a/MAINTAINERS b/MAINTAINERS >>>>> index 1cd38a7..b224caa 100644 >>>>> --- a/MAINTAINERS >>>>> +++ b/MAINTAINERS >>>>> @@ -9358,6 +9358,7 @@ L: linux-pci@vger.kernel.org >>>>> S: Maintained >>>>> F: Documentation/devicetree/bindings/pci/hisilicon-pcie.txt >>>>> F: drivers/pci/host/pcie-hisi.c >>>>> +F: drivers/pci/host/pcie-hisi-acpi.c >>>>> >>>>> PCIE DRIVER FOR ROCKCHIP >>>>> M: Shawn Lin >>>>> diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c >>>>> index ac21db3..b1b6fc7 100644 >>>>> --- a/drivers/acpi/pci_mcfg.c >>>>> +++ b/drivers/acpi/pci_mcfg.c >>>>> @@ -57,6 +57,19 @@ struct mcfg_fixup { >>>>> { "QCOM ", "QDF2432 ", 1, 5, MCFG_BUS_ANY, &pci_32b_ops }, >>>>> { "QCOM ", "QDF2432 ", 1, 6, MCFG_BUS_ANY, &pci_32b_ops }, >>>>> { "QCOM ", "QDF2432 ", 1, 7, MCFG_BUS_ANY, &pci_32b_ops }, >>>>> +#ifdef CONFIG_PCI_HISI_ACPI >>>>> + #define PCI_ACPI_QUIRK_QUAD_DOM(table_id, seg, ops) \ >>>>> + { "HISI ", table_id, 0, seg + 0, MCFG_BUS_ANY, ops }, \ >>>>> + { "HISI ", table_id, 0, seg + 1, MCFG_BUS_ANY, ops }, \ >>>>> + { "HISI ", table_id, 0, seg + 2, MCFG_BUS_ANY, ops }, \ >>>>> + { "HISI ", table_id, 0, seg + 3, MCFG_BUS_ANY, ops } >>>>> + PCI_ACPI_QUIRK_QUAD_DOM("HIP05 ", 0, &hisi_pcie_ops), >>>>> + PCI_ACPI_QUIRK_QUAD_DOM("HIP06 ", 0, &hisi_pcie_ops), >>>>> + PCI_ACPI_QUIRK_QUAD_DOM("HIP07 ", 0, &hisi_pcie_ops), >>>>> + PCI_ACPI_QUIRK_QUAD_DOM("HIP07 ", 4, &hisi_pcie_ops), >>>>> + PCI_ACPI_QUIRK_QUAD_DOM("HIP07 ", 8, &hisi_pcie_ops), >>>>> + PCI_ACPI_QUIRK_QUAD_DOM("HIP07 ", 12, &hisi_pcie_ops), >>>>> +#endif >>>>> }; >>>>> >>>>> static char mcfg_oem_id[ACPI_OEM_ID_SIZE]; >>>>> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig >>>>> index ae98644..9ff2bcd 100644 >>>>> --- a/drivers/pci/host/Kconfig >>>>> +++ b/drivers/pci/host/Kconfig >>>>> @@ -227,6 +227,14 @@ config PCI_HISI >>>>> Say Y here if you want PCIe controller support on HiSilicon >>>>> Hip05 and Hip06 and Hip07 SoCs >>>>> >>>>> +config PCI_HISI_ACPI >>>>> + depends on ACPI && ARM64 >>>>> + bool "HiSilicon Hip05 and Hip06 and Hip07 SoCs ACPI PCIe >>>>> controllers" >>>>> + select PNP >>>>> + help >>>>> + Say Y here if you want ACPI PCIe controller support on HiSilicon >>>>> + Hip05 and Hip06 and Hip07 SoCs >>>>> + >>>>> config PCIE_QCOM >>>>> bool "Qualcomm PCIe controller" >>>>> depends on ARCH_QCOM && OF >>>>> diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile >>>>> index 084cb49..9402858 100644 >>>>> --- a/drivers/pci/host/Makefile >>>>> +++ b/drivers/pci/host/Makefile >>>>> @@ -26,6 +26,7 @@ obj-$(CONFIG_PCIE_IPROC_BCMA) += pcie-iproc-bcma.o >>>>> obj-$(CONFIG_PCIE_ALTERA) += pcie-altera.o >>>>> obj-$(CONFIG_PCIE_ALTERA_MSI) += pcie-altera-msi.o >>>>> obj-$(CONFIG_PCI_HISI) += pcie-hisi.o >>>>> +obj-$(CONFIG_PCI_HISI_ACPI) += pcie-hisi-acpi.o >>>>> obj-$(CONFIG_PCIE_QCOM) += pcie-qcom.o >>>>> obj-$(CONFIG_PCI_HOST_THUNDER_ECAM) += pci-thunder-ecam.o >>>>> obj-$(CONFIG_PCI_HOST_THUNDER_PEM) += pci-thunder-pem.o >>>>> diff --git a/drivers/pci/host/pcie-hisi-acpi.c >>>>> b/drivers/pci/host/pcie-hisi-acpi.c >>>>> new file mode 100644 >>>>> index 0000000..aade4b5 >>>>> --- /dev/null >>>>> +++ b/drivers/pci/host/pcie-hisi-acpi.c >>>>> @@ -0,0 +1,157 @@ >>>>> +/* >>>>> + * PCIe host controller driver for HiSilicon HipXX SoCs >>>>> + * >>>>> + * Copyright (C) 2016 HiSilicon Co., Ltd. http://www.hisilicon.com >>>>> + * >>>>> + * Author: Dongdong Liu >>>>> + * Gabriele Paoloni >>>>> + * >>>>> + * 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 >>>>> +#include >>>>> +#include >>>>> + >>>>> +#define DEBUG0 0x728 >>>>> +#define PCIE_LTSSM_LINKUP_STATE 0x11 >>>>> +#define PCIE_LTSSM_STATE_MASK 0x3F >>>> >>>> These are now unused. >>>> >>>>> +static const struct acpi_device_id hisi_pcie_rc_res_ids[] = { >>>>> + {"HISI0081", 0}, >>>>> + {"", 0}, >>>>> +}; >>>>> + >>>>> +static int hisi_pcie_acpi_rd_conf(struct pci_bus *bus, u32 devfn, >>>>> int where, >>>>> + int size, u32 *val) >>>>> +{ >>>>> + struct pci_config_window *cfg = bus->sysdata; >>>>> + int dev = PCI_SLOT(devfn); >>>>> + >>>>> + if (bus->number == cfg->busr.start) { >>>>> + /* access only one slot on each root port */ >>>>> + if (dev > 0) >>>>> + return PCIBIOS_DEVICE_NOT_FOUND; >>>>> + else >>>>> + return pci_generic_config_read32(bus, devfn, where, >>>>> + size, val); >>>>> + } >>>>> + >>>>> + return pci_generic_config_read(bus, devfn, where, size, val); >>>>> +} >>>>> + >>>>> +static int hisi_pcie_acpi_wr_conf(struct pci_bus *bus, u32 devfn, >>>>> + int where, int size, u32 val) >>>>> +{ >>>>> + struct pci_config_window *cfg = bus->sysdata; >>>>> + int dev = PCI_SLOT(devfn); >>>>> + >>>>> + if (bus->number == cfg->busr.start) { >>>>> + /* access only one slot on each root port */ >>>>> + if (dev > 0) >>>>> + return PCIBIOS_DEVICE_NOT_FOUND; >>>>> + else >>>>> + return pci_generic_config_write32(bus, devfn, where, >>>>> + size, val); >>>>> + } >>>>> + >>>>> + return pci_generic_config_write(bus, devfn, where, size, val); >>>>> +} >>>>> + >>>>> +static void __iomem *hisi_pcie_map_bus(struct pci_bus *bus, >>>>> unsigned int devfn, >>>>> + int where) >>>>> +{ >>>>> + struct pci_config_window *cfg = bus->sysdata; >>>>> + void __iomem *reg_base = cfg->priv; >>>>> + >>>>> + if (bus->number == cfg->busr.start) >>>>> + return reg_base + where; >>>>> + else >>>>> + return pci_ecam_map_bus(bus, devfn, where); >>>>> +} >>>>> + >>>>> +/* >>>>> + * Retrieve RC base and size from sub-device under the RC >>>>> + * Device (RES1) >>>>> + * { >>>>> + * Name (_HID, "HISI0081") >>>>> + * Name (_CID, "PNP0C02") >>>>> + * Name (_CRS, ResourceTemplate (){ >>>>> + * Memory32Fixed (ReadWrite, 0xb0080000, 0x10000) >>>>> + * }) >>>>> + * } >>>>> + */ >>>>> +static int hisi_pcie_rc_addr_get(struct acpi_device *adev, >>>>> + void __iomem **addr) >>>>> +{ >>>>> + struct acpi_device *child_adev; >>>>> + struct list_head list; >>>>> + struct resource *res; >>>>> + struct resource_entry *entry; >>>>> + unsigned long flags; >>>>> + int ret; >>>>> + >>>>> + list_for_each_entry(child_adev, &adev->children, node) { >>>>> + ret = acpi_match_device_ids(child_adev, hisi_pcie_rc_res_ids); >>>>> + if (ret) >>>>> + continue; >>>>> + >>>>> + INIT_LIST_HEAD(&list); >>>>> + flags = IORESOURCE_MEM; >>>>> + ret = acpi_dev_get_resources(child_adev, &list, >>>>> + acpi_dev_filter_resource_type_cb, >>>>> + (void *)flags); >>>>> + if (ret < 0) { >>>>> + dev_err(&child_adev->dev, >>>>> + "failed to parse _CRS method, error code %d\n", >>>>> + ret); >>>>> + return ret; >>>>> + } else if (ret == 0) { >>>>> + dev_err(&child_adev->dev, >>>>> + "no IO and memory resources present in _CRS\n"); >>>>> + return -EINVAL; >>>>> + } >>>>> + >>>>> + entry = list_first_entry(&list, struct resource_entry, node); >>>>> + res = entry->res; >>>>> + *addr = devm_ioremap(&child_adev->dev, >>>>> + res->start, resource_size(res)); >>>>> + acpi_dev_free_resource_list(&list); >>>>> + if (IS_ERR(*addr)) { >>>>> + dev_err(&child_adev->dev, "error with ioremap\n"); >>>>> + return -ENOMEM; >>>>> + } >>>>> + >>>>> + return 0; >>>>> + } >>>>> + >>>>> + return -EINVAL; >>>>> +} >>>> >>>> OK, so MCFG (with the possible help of quirks) tells us where *most* >>>> of this RC's ECAM space is, but it doesn't tell us where the root bus >>>> ECAM space is, which is why we need this block of ugly code. Right? >>> >>> Tomasz is doing the same thing for ThunderX. Can we share the code >>> somehow, e.g., add something that takes the _HID to look for and just >>> returns the register address? Maybe in pci-acpi.c, decl in >>> drivers/pci/pci.h, under #ifdef CONFIG_ARM64?0 >> >> As we have the PNP0C02 device at the root of the ACPI namespace (under >> \_SB). >> We need a way to tell which root bus the PNP0C02 resource belong to. >> >> Firstly, use _HID(HISI0081) to get the PNP0C02 devices that we need.. >> Secondly, use _UID to match segment to tell which root bus the PNP0C02 >> resource belong to. >> >> The implementaton is as below. What do you think about this? >> If the code is ok for other manufacturers, we can share the code. >> >> struct hisi_pcie_acpi { >> u16 segment; >> acpi_handle handle; >> }; >> >> /* >> * Retrieve RC base and size from PNP0C02 at the root of the ACPI namespace >> * (under \_SB). >> * Device (RES0) >> * { >> * Name (_HID, "HISI0081") >> * Name (_CID, "PNP0C02") >> * Name (_UID, 0x0) >> * Name (_CRS, ResourceTemplate (){ >> * Memory32Fixed (ReadWrite, 0xa0090000, 0x10000) >> * }) >> * } >> */ >> static int hisi_pcie_rc_addr_get(struct acpi_device *adev, >> void __iomem **addr) >> { >> struct list_head list; >> struct resource *res; >> struct resource_entry *entry; >> unsigned long flags; >> int ret; >> >> INIT_LIST_HEAD(&list); >> flags = IORESOURCE_MEM; >> ret = acpi_dev_get_resources(adev, &list, >> acpi_dev_filter_resource_type_cb, >> (void *)flags); >> if (ret < 0) { >> dev_err(&adev->dev, >> "failed to parse _CRS method, error code %d\n", >> ret); >> return ret; >> } else if (ret == 0) { >> dev_err(&adev->dev, >> "no IO and memory resources present in _CRS\n"); >> return -EINVAL; >> } >> >> entry = list_first_entry(&list, struct resource_entry, node); >> res = entry->res; >> *addr = devm_ioremap(&adev->dev, >> res->start, resource_size(res)); >> acpi_dev_free_resource_list(&list); >> if (IS_ERR(*addr)) { >> dev_err(&adev->dev, "error with ioremap\n"); >> return -ENOMEM; >> } >> >> return 0; >> } >> >> static acpi_status find_rc_resource(acpi_handle handle, u32 lvl, >> void *context, void **rv) >> { >> struct hisi_pcie_acpi *pcie_acpi = context; >> acpi_status status; >> unsigned long long uid; >> >> status = acpi_evaluate_integer(handle, "_UID", NULL, &uid); >> if (ACPI_FAILURE(status) || uid != pcie_acpi->segment) >> return AE_CTRL_DEPTH; >> >> pcie_acpi->handle = handle; >> return AE_CTRL_TERMINATE; >> } >> >> static int hisi_pcie_init(struct pci_config_window *cfg) >> { >> struct acpi_device *adev = to_acpi_device(cfg->parent); >> struct acpi_pci_root *root = acpi_driver_data(adev); >> struct hisi_pcie_acpi *pcie_acpi; >> int ret; >> void __iomem *reg_base; >> >> pcie_acpi = devm_kzalloc(&adev->dev, sizeof(*pcie_acpi), GFP_KERNEL); >> if (!pcie_acpi) >> return -ENOMEM; >> >> pcie_acpi->segment = root->segment; >> acpi_get_devices("HISI0081", find_rc_resource, pcie_acpi, NULL); >> >> ret = acpi_bus_get_device(pcie_acpi->handle, &adev); >> if (ret) >> return ret; >> >> ret = hisi_pcie_rc_addr_get(adev, ®_base); >> if (ret) { >> dev_err(&adev->dev, "can't get rc base address"); >> return ret; >> } >> >> cfg->priv = pcie_acpi->reg_base; >> return 0; >> } >> >> Thanks >> Dongdong >>> >>> . >>> >> > > . >