| 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: <20171016234326.GM25517@bhelgaas-glaptop.roam.corp.google.com>
Date: Mon, 16 Oct 2017 18:43:26 -0500
From: Bjorn Helgaas <helgaas@...nel.org>
To: Niklas Cassel <niklas.cassel@...s.com>
Cc: Niklas Cassel <niklass@...s.com>,
Jesper Nilsson <jespern@...s.com>,
Bjorn Helgaas <bhelgaas@...gle.com>,
Rob Herring <robh+dt@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Kishon Vijay Abraham I <kishon@...com>,
Jingoo Han <jingoohan1@...il.com>,
Xiaowei Song <songxiaowei@...ilicon.com>,
Peter Robinson <pbrobinson@...il.com>,
Tomasz Nowicki <tn@...ihalf.com>,
Gabriele Paoloni <gabriele.paoloni@...wei.com>,
Duc Dang <dhdang@....com>, linux-arm-kernel@...s.com,
linux-pci@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 08/10] PCI: dwc: artpec6: add support for endpoint mode
On Fri, Oct 13, 2017 at 06:09:11PM +0200, Niklas Cassel wrote:
> Signed-off-by: Niklas Cassel <niklas.cassel@...s.com>
> ---
> .../devicetree/bindings/pci/axis,artpec6-pcie.txt | 3 +-
> drivers/pci/dwc/Kconfig | 41 +++--
> drivers/pci/dwc/Makefile | 4 +-
> drivers/pci/dwc/pcie-artpec6.c | 202 ++++++++++++++++++++-
> 4 files changed, 233 insertions(+), 17 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/pci/axis,artpec6-pcie.txt b/Documentation/devicetree/bindings/pci/axis,artpec6-pcie.txt
> index 4e4aee4439ea..33eef7ae5a23 100644
> --- a/Documentation/devicetree/bindings/pci/axis,artpec6-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/axis,artpec6-pcie.txt
> @@ -4,7 +4,8 @@ This PCIe host controller is based on the Synopsys DesignWare PCIe IP
> and thus inherits all the common properties defined in designware-pcie.txt.
>
> Required properties:
> -- compatible: "axis,artpec6-pcie", "snps,dw-pcie"
> +- compatible: "axis,artpec6-pcie", "snps,dw-pcie" for ARTPEC-6 in RC mode;
> + "axis,artpec6-pcie-ep", "snps,dw-pcie" for ARTPEC-6 in EP mode;
> - reg: base addresses and lengths of the PCIe controller (DBI),
> the PHY controller, and configuration address space.
> - reg-names: Must include the following entries:
> diff --git a/drivers/pci/dwc/Kconfig b/drivers/pci/dwc/Kconfig
> index 22ec82fcdea2..e333283fb1ed 100644
> --- a/drivers/pci/dwc/Kconfig
> +++ b/drivers/pci/dwc/Kconfig
> @@ -14,6 +14,36 @@ config PCIE_DW_EP
> depends on PCI_ENDPOINT
> select PCIE_DW
>
> +config PCIE_ARTPEC6
> + bool "Axis ARTPEC-6 PCIe controller"
> + depends on (PCI && PCI_MSI_IRQ_DOMAIN) || PCI_ENDPOINT
> + depends on MACH_ARTPEC6
> + help
> + Say Y here to enable PCIe controller support on Axis ARTPEC-6
> + SoCs. This PCIe controller uses the DesignWare core.
> +
> +if PCIE_ARTPEC6
> +
> +config PCIE_ARTPEC6_HOST
> + bool "Axis ARTPEC-6 Host Mode"
> + depends on PCI
> + depends on PCI_MSI_IRQ_DOMAIN
> + select PCIEPORTBUS
> + select PCIE_DW_HOST
> + help
> + Enables support for the PCIe controller in the ARTPEC-6 SoC to work in
> + host mode.
> +
> +config PCIE_ARTPEC6_EP
> + bool "Axis ARTPEC-6 Endpoint Mode"
> + depends on PCI_ENDPOINT
> + select PCIE_DW_EP
> + help
> + Enables support for the PCIe controller in the ARTPEC-6 SoC to work in
> + endpoint mode.
> +
> +endif
> +
> config PCI_DRA7XX
> bool "TI DRA7xx PCIe controller"
> depends on SOC_DRA7XX || COMPILE_TEST
> @@ -148,17 +178,6 @@ config PCIE_ARMADA_8K
> DesignWare hardware and therefore the driver re-uses the
> DesignWare core functions to implement the driver.
>
> -config PCIE_ARTPEC6
> - bool "Axis ARTPEC-6 PCIe controller"
> - depends on PCI
> - depends on MACH_ARTPEC6
> - depends on PCI_MSI_IRQ_DOMAIN
> - select PCIEPORTBUS
> - select PCIE_DW_HOST
> - help
> - Say Y here to enable PCIe controller support on Axis ARTPEC-6
> - SoCs. This PCIe controller uses the DesignWare core.
> -
> config PCIE_KIRIN
> depends on OF && ARM64
> bool "HiSilicon Kirin series SoCs PCIe controllers"
> diff --git a/drivers/pci/dwc/Makefile b/drivers/pci/dwc/Makefile
> index c61be9738cce..ac98242b83a9 100644
> --- a/drivers/pci/dwc/Makefile
> +++ b/drivers/pci/dwc/Makefile
> @@ -2,6 +2,9 @@ obj-$(CONFIG_PCIE_DW) += pcie-designware.o
> obj-$(CONFIG_PCIE_DW_HOST) += pcie-designware-host.o
> obj-$(CONFIG_PCIE_DW_EP) += pcie-designware-ep.o
> obj-$(CONFIG_PCIE_DW_PLAT) += pcie-designware-plat.o
> +ifneq ($(filter y,$(CONFIG_PCIE_ARTPEC6_HOST) $(CONFIG_PCIE_ARTPEC6_EP)),)
> + obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o
> +endif
I see you're copying the DRA7XX style here, but I don't really
understand it. I guess the idea is to build pcie-artpec6.o if either
CONFIG_PCIE_ARTPEC6_HOST or CONFIG_PCIE_ARTPEC6_EP is set (or both).
Is this really the simplest way to express that in Kconfig? Both the
"if PCIE_ARTPEC6" and this ifneq thing are complicated and relatively
unusual.
> ifneq ($(filter y,$(CONFIG_PCI_DRA7XX_HOST) $(CONFIG_PCI_DRA7XX_EP)),)
> obj-$(CONFIG_PCI_DRA7XX) += pci-dra7xx.o
> endif
> +static void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar)
Looks funny to have this dw_*() function in pcie-artpec6.c. Intended?
Or is this something other endpoint drivers could/should share?
> +{
> + u32 reg;
> +
> + reg = PCI_BASE_ADDRESS_0 + (4 * bar);
> + dw_pcie_dbi_ro_wr_en(pci);
> + dw_pcie_writel_dbi2(pci, reg, 0x0);
> + dw_pcie_writel_dbi(pci, reg, 0x0);
> + dw_pcie_dbi_ro_wr_dis(pci);
> +}
> +
> +static void artpec6_pcie_ep_init(struct dw_pcie_ep *ep)
> +{
> + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> + struct artpec6_pcie *artpec6_pcie = to_artpec6_pcie(pci);
> + enum pci_barno bar;
> +
> + artpec6_pcie_assert_core_reset(artpec6_pcie);
> + artpec6_pcie_init_phy(artpec6_pcie);
> + artpec6_pcie_deassert_core_reset(artpec6_pcie);
> +
> + for (bar = BAR_0; bar <= BAR_5; bar++)
> + dw_pcie_ep_reset_bar(pci, bar);
> +}
I naively expected some of this new code to be #ifdef PCIE_ARTPEC6_EP.
But I haven't really internalized the endpoint support, so maybe that
doesn't make sense?
Bjorn
Powered by blists - more mailing lists