[U-Boot] [PATCH 12/15] pci: mvebu: Add PCIe driver for Armada-8K
Simon Glass
sjg at chromium.org
Fri Nov 18 02:14:19 CET 2016
Hi,
On 15 November 2016 at 02:08, Stefan Roese <sr at denx.de> wrote:
> From: Shadi Ammouri <shadi at marvell.com>
>
> This patch adds a driver for the PCIe controller integrated in the
> Marvell Armada-8K SoC. This controller is based on the DesignWare
> IP core.
>
> The original version was written by Shadi and Yehuda. I ported this
> driver to the latest mainline U-Boot version with DM support.
>
> Tested on the Marvell DB-88F8040 Armada-8K eval board.
>
> Signed-off-by: Shadi Ammouri <shadi at marvell.com>
> Signed-off-by: Yehuda Yitschak <yehuday at marvell.com>
> Signed-off-by: Stefan Roese <sr at denx.de>
> Cc: Simon Glass <sjg at chromium.org>
> Cc: Nadav Haklai <nadavh at marvell.com>
> Cc: Neta Zur Hershkovits <neta at marvell.com>
> Cc: Kostya Porotchkin <kostap at marvell.com>
> Cc: Omri Itach <omrii at marvell.com>
> Cc: Igal Liberman <igall at marvell.com>
> Cc: Haim Boot <hayim at marvell.com>
> Cc: Hanna Hawa <hannah at marvell.com>
> ---
> drivers/pci/Kconfig | 10 ++
> drivers/pci/Makefile | 1 +
> drivers/pci/pcie_dw_mvebu.c | 415 ++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 426 insertions(+)
> create mode 100644 drivers/pci/pcie_dw_mvebu.c
>
> diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
> index b8376b4..ff2c370 100644
> --- a/drivers/pci/Kconfig
> +++ b/drivers/pci/Kconfig
> @@ -33,6 +33,16 @@ config PCI_PNP
> help
> Enable PCI memory and I/O space resource allocation and assignment.
>
> +config PCIE_DW_MVEBU
> + bool "Enable Armada-8K PCIe driver (DesignWare core)"
> + default n
> + depends on DM_PCI
> + depends on ARMADA_8K
> + help
> + Say Y here if you want to enable PCIe controller support on
> + Armada-8K SoCs. The PCIe controller on Armada-8K is based on
> + DesignWare hardware.
> +
> config PCI_SANDBOX
> bool "Sandbox PCI support"
> depends on SANDBOX && DM_PCI
> diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
> index 9583e91..86717a4 100644
> --- a/drivers/pci/Makefile
> +++ b/drivers/pci/Makefile
> @@ -30,5 +30,6 @@ obj-$(CONFIG_SH7780_PCI) +=pci_sh7780.o
> obj-$(CONFIG_PCI_TEGRA) += pci_tegra.o
> obj-$(CONFIG_TSI108_PCI) += tsi108_pci.o
> obj-$(CONFIG_WINBOND_83C553) += w83c553f.o
> +obj-$(CONFIG_PCIE_DW_MVEBU) += pcie_dw_mvebu.o
> obj-$(CONFIG_PCIE_LAYERSCAPE) += pcie_layerscape.o
> obj-$(CONFIG_PCI_XILINX) += pcie_xilinx.o
> diff --git a/drivers/pci/pcie_dw_mvebu.c b/drivers/pci/pcie_dw_mvebu.c
> new file mode 100644
> index 0000000..dd5ba34
> --- /dev/null
> +++ b/drivers/pci/pcie_dw_mvebu.c
> @@ -0,0 +1,415 @@
> +/*
> + * Copyright (C) 2015 Marvell International Ltd.
> + *
> + * Copyright (C) 2016 Stefan Roese <sr at denx.de>
> + *
> + * Based on:
> + * - drivers/pci/pcie_imx.c
> + * - drivers/pci/pci_mvebu.c
> + * - drivers/pci/pcie_xilinx.c
> + *
> + * SPDX-License-Identifier: GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <dm.h>
> +#include <pci.h>
> +#include <asm/io.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +/* PCI Config space registers */
> +#define PCIE_CONFIG_BAR0 0x10
> +#define PCIE_LINK_STATUS_REG 0x80
> +#define PCIE_LINK_STATUS_SPEED_OFF 16
> +#define PCIE_LINK_STATUS_SPEED_MASK 0xf
> +#define PCIE_LINK_STATUS_WIDTH_OFF 20
> +#define PCIE_LINK_STATUS_WIDTH_MASK 0xf
> +
> +/* Resizable bar capability registers */
> +#define RESIZABLE_BAR_CAP 0x250
> +#define RESIZABLE_BAR_CTL0 0x254
> +#define RESIZABLE_BAR_CTL1 0x258
> +
> +/* iATU registers */
> +#define PCIE_ATU_VIEWPORT 0x900
> +#define PCIE_ATU_REGION_INBOUND (0x1 << 31)
> +#define PCIE_ATU_REGION_OUTBOUND (0x0 << 31)
> +#define PCIE_ATU_REGION_INDEX1 (0x1 << 0)
> +#define PCIE_ATU_REGION_INDEX0 (0x0 << 0)
> +#define PCIE_ATU_CR1 0x904
> +#define PCIE_ATU_TYPE_MEM (0x0 << 0)
> +#define PCIE_ATU_TYPE_IO (0x2 << 0)
> +#define PCIE_ATU_TYPE_CFG0 (0x4 << 0)
> +#define PCIE_ATU_TYPE_CFG1 (0x5 << 0)
> +#define PCIE_ATU_CR2 0x908
> +#define PCIE_ATU_ENABLE (0x1 << 31)
> +#define PCIE_ATU_BAR_MODE_ENABLE (0x1 << 30)
> +#define PCIE_ATU_LOWER_BASE 0x90C
> +#define PCIE_ATU_UPPER_BASE 0x910
> +#define PCIE_ATU_LIMIT 0x914
> +#define PCIE_ATU_LOWER_TARGET 0x918
> +#define PCIE_ATU_BUS(x) (((x) & 0xff) << 24)
> +#define PCIE_ATU_DEV(x) (((x) & 0x1f) << 19)
> +#define PCIE_ATU_FUNC(x) (((x) & 0x7) << 16)
> +#define PCIE_ATU_UPPER_TARGET 0x91C
> +
> +#define PCIE_LINK_CAPABILITY 0x7C
> +#define PCIE_LINK_CTL_2 0xA0
> +#define TARGET_LINK_SPEED_MASK 0xF
> +#define LINK_SPEED_GEN_1 0x1
> +#define LINK_SPEED_GEN_2 0x2
> +#define LINK_SPEED_GEN_3 0x3
> +
> +#define PCIE_GEN3_RELATED 0x890
> +#define GEN3_EQU_DISABLE (1 << 16)
> +#define GEN3_ZRXDC_NON_COMP (1 << 0)
> +
> +#define PCIE_GEN3_EQU_CTRL 0x8A8
> +#define GEN3_EQU_EVAL_2MS_DISABLE (1 << 5)
> +
> +#define PCIE_ROOT_COMPLEX_MODE_MASK (0xF << 4)
> +
> +#define PCIE_LINK_UP_TIMEOUT_MS 100
> +
> +#define PCIE_GLOBAL_CONTROL 0x8000
> +#define PCIE_APP_LTSSM_EN (1 << 2)
> +#define PCIE_DEVICE_TYPE_OFFSET (4)
> +#define PCIE_DEVICE_TYPE_MASK (0xF)
> +#define PCIE_DEVICE_TYPE_EP (0x0) /* Endpoint */
> +#define PCIE_DEVICE_TYPE_LEP (0x1) /* Legacy endpoint */
> +#define PCIE_DEVICE_TYPE_RC (0x4) /* Root complex */
> +
> +#define PCIE_GLOBAL_STATUS 0x8008
> +#define PCIE_GLB_STS_RDLH_LINK_UP (1 << 1)
> +#define PCIE_GLB_STS_PHY_LINK_UP (1 << 9)
> +
> +#define PCIE_ARCACHE_TRC 0x8050
> +#define PCIE_AWCACHE_TRC 0x8054
> +#define ARCACHE_SHAREABLE_CACHEABLE 0x3511
> +#define AWCACHE_SHAREABLE_CACHEABLE 0x5311
> +
> +#define LINK_SPEED_GEN_1 0x1
> +#define LINK_SPEED_GEN_2 0x2
> +#define LINK_SPEED_GEN_3 0x3
> +
> +struct pcie_dw_mvebu {
> + struct pci_controller hose;
Why is this here? It should be accessible with dev_get_uclass_priv(bus).
> + void *ctrl_base;
It would be nice if this were a C struct rather than offsets.
> + void *cfg_base;
> + fdt_size_t cfg_size;
> + int first_busno;
What are these two for? Needs comments.
> +};
> +
> +static int pcie_dw_get_link_speed(void *regs_base)
> +{
> + return ((readl(regs_base + PCIE_LINK_STATUS_REG)) >>
> + PCIE_LINK_STATUS_SPEED_OFF) & PCIE_LINK_STATUS_SPEED_MASK;
I suggest making the MASK a shifted value, so that you mask and then shift.
> +}
> +
> +static int pcie_dw_get_link_width(void *regs_base)
> +{
> + return ((readl(regs_base + PCIE_LINK_STATUS_REG)) >>
> + PCIE_LINK_STATUS_WIDTH_OFF) & PCIE_LINK_STATUS_WIDTH_MASK;
> +}
> +
> +static uintptr_t set_cfg_address(struct pcie_dw_mvebu *pcie,
> + pci_dev_t d, int where)
Function comment? Much of this file could do with function comments.
> +{
> + uintptr_t va_address;
> +
> + /*
> + * Region #0 is used for Outbound CFG space access.
> + * Direction = Outbound
> + * Region Index = 0
> + */
> + writel(0, pcie->ctrl_base + PCIE_ATU_VIEWPORT);
> +
> + if (PCI_BUS(d) == (pcie->first_busno + 1))
> + /* For local bus, change TLP Type field to 4. */
> + writel(PCIE_ATU_TYPE_CFG0, pcie->ctrl_base + PCIE_ATU_CR1);
> + else
> + /* Otherwise, change TLP Type field to 5. */
> + writel(PCIE_ATU_TYPE_CFG1, pcie->ctrl_base + PCIE_ATU_CR1);
> +
> + if (PCI_BUS(d) == pcie->first_busno) {
> + /* Accessing root port configuration space. */
> + va_address = (uintptr_t)pcie->ctrl_base;
> + } else {
> + writel(d << 8, pcie->ctrl_base + PCIE_ATU_LOWER_TARGET);
> + va_address = (uintptr_t)pcie->cfg_base;
> + }
> +
> + va_address += (where & ~0x3);
> +
> + return va_address;
> +}
> +
> +static int pcie_dw_addr_valid(pci_dev_t d, int first_busno)
> +{
> + if ((PCI_BUS(d) == first_busno) && (PCI_DEV(d) > 0))
> + return 0;
> + if ((PCI_BUS(d) == first_busno + 1) && (PCI_DEV(d) > 0))
> + return 0;
> +
> + return 1;
> +}
> +
> +static int pcie_dw_mvebu_read_config(struct udevice *bus, pci_dev_t bdf,
> + uint offset, ulong *valuep,
> + enum pci_size_t size)
> +{
> + struct pcie_dw_mvebu *pcie = dev_get_priv(bus);
> + uintptr_t va_address;
> + ulong value;
> +
> + debug("PCIE CFG read: (b,d,f)=(%2d,%2d,%2d) ",
> + PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf));
> +
> + if (!pcie_dw_addr_valid(bdf, pcie->first_busno)) {
> + debug("- out of range\n");
> + *valuep = pci_get_ff(size);
> + return 0;
> + }
> +
> + va_address = set_cfg_address(pcie, bdf, offset);
> +
> + value = readl(va_address);
> +
> + debug("(addr,val)=(0x%04x, 0x%08lx)\n", offset, value);
> + *valuep = pci_conv_32_to_size(value, offset, size);
> +
> + return 0;
> +}
> +
> +static int pcie_dw_mvebu_write_config(struct udevice *bus, pci_dev_t bdf,
> + uint offset, ulong value,
> + enum pci_size_t size)
> +{
> + struct pcie_dw_mvebu *pcie = dev_get_priv(bus);
> + uintptr_t va_address;
> + ulong old;
> +
> + debug("PCIE CFG write: (b,d,f)=(%2d,%2d,%2d) ",
> + PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf));
> + debug("(addr,val)=(0x%04x, 0x%08lx)\n", offset, value);
> +
> + if (!pcie_dw_addr_valid(bdf, pcie->first_busno)) {
> + debug("- out of range\n");
> + return 0;
> + }
> +
> + va_address = set_cfg_address(pcie, bdf, offset);
> +
> + old = readl(va_address);
> + value = pci_conv_size_to_32(old, value, offset, size);
> + writel(value, va_address);
> +
> + return 0;
> +}
> +
> +static void pcie_dw_configure(void *regs_base, u32 cap_speed)
> +{
> + u32 reg;
> +
> + /* TODO: need to read the serdes speed from the dts
> + and according to it configure the PCIe gen */
/*
* TODO(email): ..
* ...
*/
> +
> + /* Set link to GEN 3 */
> + reg = readl(regs_base + PCIE_LINK_CTL_2);
> + reg &= ~TARGET_LINK_SPEED_MASK;
> + reg |= cap_speed;
> + writel(reg, regs_base + PCIE_LINK_CTL_2);
clrsetbits_le32()
> +
> + reg = readl(regs_base + PCIE_LINK_CAPABILITY);
> + reg &= ~TARGET_LINK_SPEED_MASK;
> + reg |= cap_speed;
> + writel(reg, regs_base + PCIE_LINK_CAPABILITY);
> +
> + reg = readl(regs_base + PCIE_GEN3_EQU_CTRL);
> + reg |= GEN3_EQU_EVAL_2MS_DISABLE;
> + writel(reg, regs_base + PCIE_GEN3_EQU_CTRL);
setbits_le32()
> +}
> +
> +static int is_link_up(void *regs_base)
const void? It would be better as a struct though
> +{
> + u32 mask = PCIE_GLB_STS_RDLH_LINK_UP | PCIE_GLB_STS_PHY_LINK_UP;
> + u32 reg;
> +
> + reg = readl(regs_base + PCIE_GLOBAL_STATUS);
> + if ((reg & mask) == mask)
> + return 1;
> +
> + return 0;
> +}
> +
> +static int wait_link_up(void *regs_base)
Better to return 0 if OK, -ETIMEDOUT if not.
> +{
> + unsigned long timeout;
> +
> + timeout = get_timer(0) + PCIE_LINK_UP_TIMEOUT_MS;
> + while (!is_link_up(regs_base)) {
> + if (get_timer(0) > timeout)
> + return 0;
> + };
> +
> + return 1;
> +}
> +
> +static int pcie_dw_mvebu_pcie_link_up(void *regs_base, u32 cap_speed)
Better to return 0 if OK, -ve error if not
> +{
> + u32 reg;
> +
> + if (!is_link_up(regs_base)) {
> + /* Disable LTSSM state machine to enable configuration */
> + reg = readl(regs_base + PCIE_GLOBAL_CONTROL);
> + reg &= ~(PCIE_APP_LTSSM_EN);
> + writel(reg, regs_base + PCIE_GLOBAL_CONTROL);
> + }
> +
> + reg = readl(regs_base + PCIE_GLOBAL_CONTROL);
> + reg &= ~(PCIE_DEVICE_TYPE_MASK << PCIE_DEVICE_TYPE_OFFSET);
> + reg |= (PCIE_DEVICE_TYPE_RC << PCIE_DEVICE_TYPE_OFFSET);
> + writel(reg, regs_base + PCIE_GLOBAL_CONTROL);
> +
> + /* Set the PCIe master AXI attributes */
> + writel(ARCACHE_SHAREABLE_CACHEABLE, regs_base + PCIE_ARCACHE_TRC);
> + writel(AWCACHE_SHAREABLE_CACHEABLE, regs_base + PCIE_AWCACHE_TRC);
> +
> + /* DW pre link configurations */
> +
(regs_base, cap_speed);
> +
> + if (!is_link_up(regs_base)) {
> + /* Configuration done. Start LTSSM */
> + reg = readl(regs_base + PCIE_GLOBAL_CONTROL);
> + reg |= PCIE_APP_LTSSM_EN;
> + writel(reg, regs_base + PCIE_GLOBAL_CONTROL);
> + }
> +
> + /* Check that link was established */
> + if (!wait_link_up(regs_base))
> + return 0;
> +
> + /*
> + * Link can be established in Gen 1. still need to wait
> + * till MAC nagaotiation is completed
> + */
> + udelay(100);
> +
> + return 1;
> +}
> +
> +/*
> + * iATU region setup
> + */
> +static int pcie_dw_regions_setup(struct pcie_dw_mvebu *pcie)
> +{
> + /*
> + * Region #0 is used for Outbound CFG space access.
> + * Direction = Outbound
> + * Region Index = 0
> + */
> + writel(0, pcie->ctrl_base + PCIE_ATU_VIEWPORT);
> +
> + writel((u32)(uintptr_t)pcie->cfg_base, pcie->ctrl_base
> + + PCIE_ATU_LOWER_BASE);
> + writel(0, pcie->ctrl_base + PCIE_ATU_UPPER_BASE);
> + writel((u32)(uintptr_t)pcie->cfg_base + pcie->cfg_size,
> + pcie->ctrl_base + PCIE_ATU_LIMIT);
> +
> + writel(0, pcie->ctrl_base + PCIE_ATU_LOWER_TARGET);
> + writel(0, pcie->ctrl_base + PCIE_ATU_UPPER_TARGET);
> + writel(PCIE_ATU_TYPE_CFG0, pcie->ctrl_base + PCIE_ATU_CR1);
> + writel(PCIE_ATU_ENABLE, pcie->ctrl_base + PCIE_ATU_CR2);
> +
> + return 0;
> +}
> +
> +static void pcie_dw_set_host_bars(void *regs_base)
> +{
> + u32 size = gd->ram_size;
> + u64 max_size;
> + u32 reg;
> + u32 bar0;
> +
> + /* Verify the maximal BAR size */
> + reg = readl(regs_base + RESIZABLE_BAR_CAP);
> + max_size = 1ULL << (5 + (reg + (1 << 4)));
> +
> + if (size > max_size) {
> + size = max_size;
> + printf("Warning: PCIe BARs can't map all DRAM space\n");
Isn't this an error? If so I suggest returning it.
> + }
> +
> + /* Set the BAR base and size towards DDR */
> + bar0 = CONFIG_SYS_SDRAM_BASE & ~0xf;
> + bar0 |= PCI_BASE_ADDRESS_MEM_TYPE_32;
> + writel(CONFIG_SYS_SDRAM_BASE, regs_base + PCIE_CONFIG_BAR0);
> +
> + reg = ((size >> 20) - 1) << 12;
> + writel(size, regs_base + RESIZABLE_BAR_CTL0);
> +}
> +
> +static int pcie_dw_mvebu_probe(struct udevice *dev)
> +{
> + struct pcie_dw_mvebu *pcie = dev_get_priv(dev);
> +
> + pcie->first_busno = dev->seq;
OK I see what that variable is now. But please comment it at the top.
> +
> + /* Don't register host if link is down */
> + if (!pcie_dw_mvebu_pcie_link_up(pcie->ctrl_base, LINK_SPEED_GEN_3)) {
> + printf("PCIE-%d: Link down\n", dev->seq);
> + return -ENODEV;
> + }
> +
> + printf("PCIE-%d: Link up (Gen%d-x%d, Bus%d)\n", dev->seq,
> + pcie_dw_get_link_speed(pcie->ctrl_base),
> + pcie_dw_get_link_width(pcie->ctrl_base), pcie->hose.first_busno);
> +
> + pcie_dw_regions_setup(pcie);
> + pcie_dw_set_host_bars(pcie->ctrl_base);
> +
> + return 0;
> +}
> +
> +static int pcie_dw_mvebu_ofdata_to_platdata(struct udevice *dev)
> +{
> + struct pcie_dw_mvebu *pcie = dev_get_priv(dev);
> +
> + pcie->ctrl_base = (void *)dev_get_addr_index(dev, 0);
> + if (IS_ERR(pcie->ctrl_base))
> + return PTR_ERR(pcie->ctrl_base);
> +
> + /*
> + * We need to know the size for the cfg space as well from the
> + * DT "reg" property. dev_get_addr_index() doesn't provide this
> + * size, so we need to use a different method to acquire the
> + * address and size here.
> + */
> + pcie->cfg_base = (void *)fdtdec_get_addr_size_auto_noparent(
> + gd->fdt_blob, dev->of_offset, "reg", 1, &pcie->cfg_size, true);
This function returns FDT_ADDR_T_NONE on failure. Do we need to add a
dev_get_addr_size()?
> + if (pcie->cfg_base == NULL)
> + return -ENODEV;
> +
> + return 0;
> +}
> +
> +static const struct dm_pci_ops pcie_dw_mvebu_ops = {
> + .read_config = pcie_dw_mvebu_read_config,
> + .write_config = pcie_dw_mvebu_write_config,
> +};
> +
> +static const struct udevice_id pcie_dw_mvebu_ids[] = {
> + { .compatible = "marvell,armada8k-pcie" },
> + { }
> +};
> +
> +U_BOOT_DRIVER(pcie_dw_mvebu) = {
> + .name = "pcie_dw_mvebu",
> + .id = UCLASS_PCI,
> + .of_match = pcie_dw_mvebu_ids,
> + .ops = &pcie_dw_mvebu_ops,
> + .ofdata_to_platdata = pcie_dw_mvebu_ofdata_to_platdata,
> + .probe = pcie_dw_mvebu_probe,
> + .priv_auto_alloc_size = sizeof(struct pcie_dw_mvebu),
> +};
> --
> 2.10.2
>
Regards,
Simon
More information about the U-Boot
mailing list