| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20160209182429.GD4348@leverpostej>
Date: Tue, 9 Feb 2016 18:24:29 +0000
From: Mark Rutland <mark.rutland@....com>
To: Gabriele Paoloni <gabriele.paoloni@...wei.com>
Cc: guohanjun@...wei.com, wangzhou1@...ilicon.com,
liudongdong3@...wei.com, linuxarm@...wei.com, qiujiang@...wei.com,
bhelgaas@...gle.com, arnd@...db.de, Lorenzo.Pieralisi@....com,
tn@...ihalf.com, linux-pci@...r.kernel.org,
linux-kernel@...r.kernel.org, xuwei5@...ilicon.com,
linux-acpi@...r.kernel.org, jcm@...hat.com, zhangjukuo@...wei.com,
liguozhu@...ilicon.com, linux-arm-kernel@...ts.infradead.org
Subject: Re: [RFC PATCH v3 3/3] PCI/ACPI: hisi: Add ACPI support for
HiSilicon SoCs Host Controllers
On Tue, Feb 09, 2016 at 05:34:20PM +0000, Gabriele Paoloni wrote:
> From: gabriele paoloni <gabriele.paoloni@...wei.com>
>
> This patch adds specific quirks for PCI config space accessors,
> it uses _HID to decide whether to hook pci_ops or not.
If I understand correctly, this would mean that it's not actually ECAM
compliant, then?
> Signed-off-by: Dongdong Liu <liudongdong3@...wei.com>
> Signed-off-by: Gabriele Paoloni <gabriele.paoloni@...wei.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@...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@...wei.com>
> + * Gabriele Paoloni <gabriele.paoloni@...wei.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;
> + }
This does not seem like the correct way of describing an addres in ACPI.
> +
> + 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 }},
> + * }
> + * })
> + */
As above, this does not look right. ACPI has standard mechanisms for
describing addresses. Making something up like this is not a good idea.
Mark.
Powered by blists - more mailing lists