[RFC,8/9] pci: Add driver for Broadcom STB PCIe controller

This patch adds basic driver for the Broadcom STB PCIe host controller.
The code is based on Linux upstream driver (pcie-brcmtsb.c) with MSI
handling removed. The inbound access memory region is not currently
parsed from dma-ranges DT property and is fixed as a 1:1 mapping of
whole RAM.
The patch has been tested on rpi4 board, i.e. on BCM2711 SoC with VL805
USB Host Controller.

Signed-off-by: Sylwester Nawrocki <s.nawrocki at samsung.com>
---
 drivers/pci/Kconfig        |   5 +
 drivers/pci/Makefile       |   1 +
 drivers/pci/pcie_brcmstb.c | 844 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 850 insertions(+)
 create mode 100644 drivers/pci/pcie_brcmstb.c

Hi Sylwester,

On Wed, Apr 22, 2020 at 12:51 AM Sylwester Nawrocki
<s.nawrocki at samsung.com> wrote:
>
> This patch adds basic driver for the Broadcom STB PCIe host controller.
> The code is based on Linux upstream driver (pcie-brcmtsb.c) with MSI

pcie-brcmstb.c

> handling removed. The inbound access memory region is not currently
> parsed from dma-ranges DT property and is fixed as a 1:1 mapping of
> whole RAM.
> The patch has been tested on rpi4 board, i.e. on BCM2711 SoC with VL805
> USB Host Controller.
>
> Signed-off-by: Sylwester Nawrocki <s.nawrocki at samsung.com>
> ---
>  drivers/pci/Kconfig        |   5 +
>  drivers/pci/Makefile       |   1 +
>  drivers/pci/pcie_brcmstb.c | 844 +++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 850 insertions(+)
>  create mode 100644 drivers/pci/pcie_brcmstb.c
>
> diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
> index 437cd9a..02dcc57 100644
> --- a/drivers/pci/Kconfig
> +++ b/drivers/pci/Kconfig
> @@ -197,4 +197,9 @@ config PCIE_MEDIATEK
>           Say Y here if you want to enable Gen2 PCIe controller,
>           which could be found on MT7623 SoC family.
>
> +config PCI_BRCMSTB
> +       bool "Broadcom STB PCIe controller"
> +       depends on DM_PCI
> +       help
> +         Say Y here if you want to enable PCI controller support on BCM2711 SoC.
>  endif
> diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
> index c051ecc..3e53b1f 100644
> --- a/drivers/pci/Makefile
> +++ b/drivers/pci/Makefile
> @@ -43,3 +43,4 @@ obj-$(CONFIG_PCI_PHYTIUM) += pcie_phytium.o
>  obj-$(CONFIG_PCIE_INTEL_FPGA) += pcie_intel_fpga.o
>  obj-$(CONFIG_PCI_KEYSTONE) += pcie_dw_ti.o
>  obj-$(CONFIG_PCIE_MEDIATEK) += pcie_mediatek.o
> +obj-$(CONFIG_PCI_BRCMSTB) += pcie_brcmstb.o
> diff --git a/drivers/pci/pcie_brcmstb.c b/drivers/pci/pcie_brcmstb.c
> new file mode 100644
> index 0000000..e96e163
> --- /dev/null
> +++ b/drivers/pci/pcie_brcmstb.c
> @@ -0,0 +1,844 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Broadcom STB PCIe controller driver
> + *
> + * Copyright (C) 2020 Samsung Electronics Co., Ltd.
> + * Author: Sylwester Nawrocki <s.nawrocki at samsung.com>
> + *
> + * Based on upstream Linux kernel driver:
> + *
> + *  drivers/pci/controller/pcie-brcmstb.c
> + *  Copyright (C) 2009 - 2017 Broadcom
> + */
> +
> +#include <asm/io.h>
> +#include <common.h>
> +#include <dm.h>
> +#include <dm/device-internal.h>
> +#include <dm/lists.h>
> +#include <dm/of_access.h>
> +#include <dm/of.h>
> +#include <linux/errno.h>
> +#include <linux/ioport.h>
> +#include <linux/log2.h>
> +#include <linux/mbus.h>
> +#include <pci.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;

This seems not be used by this driver

> +
> +/* BRCM_PCIE_CAP_REGS - Offset for the mandatory capability config regs */
> +#define BRCM_PCIE_CAP_REGS                             0x00ac
> +
> +/*
> + * Broadcom Settop Box PCIe Register Offsets. The names are from
> + * the chip's RDB and we use them here so that a script can correlate
> + * this code and the RDB to prevent discrepancies.
> + */
> +#define PCIE_RC_CFG_VENDOR_VENDOR_SPECIFIC_REG1                0x0188
> +#define PCIE_RC_CFG_PRIV1_ID_VAL3                      0x043c
> +#define PCIE_RC_DL_MDIO_ADDR                           0x1100
> +#define PCIE_RC_DL_MDIO_WR_DATA                                0x1104
> +#define PCIE_RC_DL_MDIO_RD_DATA                                0x1108
> +#define PCIE_MISC_MISC_CTRL                            0x4008
> +#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_LO               0x400c
> +#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_HI               0x4010
> +#define PCIE_MISC_RC_BAR1_CONFIG_LO                    0x402c
> +#define PCIE_MISC_RC_BAR2_CONFIG_LO                    0x4034
> +#define PCIE_MISC_RC_BAR2_CONFIG_HI                    0x4038
> +#define PCIE_MISC_RC_BAR3_CONFIG_LO                    0x403c
> +#define PCIE_MISC_MSI_BAR_CONFIG_LO                    0x4044
> +#define PCIE_MISC_MSI_BAR_CONFIG_HI                    0x4048
> +#define PCIE_MISC_MSI_DATA_CONFIG                      0x404c
> +#define PCIE_MISC_EOI_CTRL                             0x4060
> +#define PCIE_MISC_PCIE_CTRL                            0x4064
> +#define PCIE_MISC_PCIE_STATUS                          0x4068
> +#define PCIE_MISC_REVISION                             0x406c
> +#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_LIMIT       0x4070
> +#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_HI          0x4080
> +#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_LIMIT_HI         0x4084
> +#define PCIE_MISC_HARD_PCIE_HARD_DEBUG                 0x4204
> +#define PCIE_INTR2_CPU_BASE                            0x4300
> +#define PCIE_MSI_INTR2_BASE                            0x4500
> +
> +/*
> + * Broadcom Settop Box PCIe Register Field shift and mask info. The
> + * names are from the chip's RDB and we use them here so that a script
> + * can correlate this code and the RDB to prevent discrepancies.
> + */
> +#define PCIE_RC_CFG_VENDOR_VENDOR_SPECIFIC_REG1_ENDIAN_MODE_BAR2_MASK  0xc
> +#define PCIE_RC_CFG_VENDOR_VENDOR_SPECIFIC_REG1_ENDIAN_MODE_BAR2_SHIFT 0x2
> +#define PCIE_RC_CFG_PRIV1_ID_VAL3_CLASS_CODE_MASK              0xffffff
> +#define PCIE_RC_CFG_PRIV1_ID_VAL3_CLASS_CODE_SHIFT             0x0
> +#define PCIE_MISC_MISC_CTRL_SCB_ACCESS_EN_MASK                 0x1000
> +#define PCIE_MISC_MISC_CTRL_SCB_ACCESS_EN_SHIFT                        0xc
> +#define PCIE_MISC_MISC_CTRL_CFG_READ_UR_MODE_MASK              0x2000
> +#define PCIE_MISC_MISC_CTRL_CFG_READ_UR_MODE_SHIFT             0xd
> +#define PCIE_MISC_MISC_CTRL_MAX_BURST_SIZE_MASK                        0x300000
> +#define PCIE_MISC_MISC_CTRL_MAX_BURST_SIZE_SHIFT               0x14
> +#define PCIE_MISC_MISC_CTRL_SCB0_SIZE_MASK                     0xf8000000
> +#define PCIE_MISC_MISC_CTRL_SCB0_SIZE_SHIFT                    0x1b
> +#define PCIE_MISC_MISC_CTRL_SCB1_SIZE_MASK                     0x7c00000
> +#define PCIE_MISC_MISC_CTRL_SCB1_SIZE_SHIFT                    0x16
> +#define PCIE_MISC_MISC_CTRL_SCB2_SIZE_MASK                     0x1f
> +#define PCIE_MISC_MISC_CTRL_SCB2_SIZE_SHIFT                    0x0
> +#define PCIE_MISC_RC_BAR1_CONFIG_LO_SIZE_MASK                  0x1f
> +#define PCIE_MISC_RC_BAR1_CONFIG_LO_SIZE_SHIFT                 0x0
> +#define PCIE_MISC_RC_BAR2_CONFIG_LO_SIZE_MASK                  0x1f
> +#define PCIE_MISC_RC_BAR2_CONFIG_LO_SIZE_SHIFT                 0x0
> +#define PCIE_MISC_RC_BAR3_CONFIG_LO_SIZE_MASK                  0x1f
> +#define PCIE_MISC_RC_BAR3_CONFIG_LO_SIZE_SHIFT                 0x0
> +#define PCIE_MISC_PCIE_CTRL_PCIE_PERSTB_MASK                   0x4
> +#define PCIE_MISC_PCIE_CTRL_PCIE_PERSTB_SHIFT                  0x2
> +#define PCIE_MISC_PCIE_CTRL_PCIE_L23_REQUEST_MASK              0x1
> +#define PCIE_MISC_PCIE_CTRL_PCIE_L23_REQUEST_SHIFT             0x0
> +#define PCIE_MISC_PCIE_STATUS_PCIE_PORT_MASK                   0x80
> +#define PCIE_MISC_PCIE_STATUS_PCIE_PORT_SHIFT                  0x7
> +#define PCIE_MISC_PCIE_STATUS_PCIE_DL_ACTIVE_MASK              0x20
> +#define PCIE_MISC_PCIE_STATUS_PCIE_DL_ACTIVE_SHIFT             0x5
> +#define PCIE_MISC_PCIE_STATUS_PCIE_PHYLINKUP_MASK              0x10
> +#define PCIE_MISC_PCIE_STATUS_PCIE_PHYLINKUP_SHIFT             0x4
> +#define PCIE_MISC_PCIE_STATUS_PCIE_LINK_IN_L23_MASK            0x40
> +#define PCIE_MISC_PCIE_STATUS_PCIE_LINK_IN_L23_SHIFT           0x6
> +#define PCIE_MISC_REVISION_MAJMIN_MASK                         0xffff
> +#define PCIE_MISC_REVISION_MAJMIN_SHIFT                                0
> +#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_LIMIT_LIMIT_MASK    0xfff00000
> +#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_LIMIT_LIMIT_SHIFT   0x14
> +#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_LIMIT_BASE_MASK     0xfff0
> +#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_LIMIT_BASE_SHIFT    0x4
> +#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_LIMIT_NUM_MASK_BITS 0xc
> +#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_HI_BASE_MASK                0xff
> +#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_HI_BASE_SHIFT       0x0
> +#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_LIMIT_HI_LIMIT_MASK      0xff
> +#define PCIE_MISC_CPU_2_PCIE_MEM_WIN0_LIMIT_HI_LIMIT_SHIFT     0x0
> +#define PCIE_MISC_HARD_PCIE_HARD_DEBUG_CLKREQ_DEBUG_ENABLE_MASK        0x2
> +#define PCIE_MISC_HARD_PCIE_HARD_DEBUG_CLKREQ_DEBUG_ENABLE_SHIFT 0x1
> +#define PCIE_MISC_HARD_PCIE_HARD_DEBUG_SERDES_IDDQ_MASK                0x08000000
> +#define PCIE_MISC_HARD_PCIE_HARD_DEBUG_SERDES_IDDQ_SHIFT       0x1b
> +#define PCIE_RGR1_SW_INIT_1_PERST_MASK                         0x1
> +#define PCIE_RGR1_SW_INIT_1_PERST_SHIFT                                0x0
> +
> +/* Offsets from PCIE_INTR2_CPU_BASE */
> +#define STATUS                         0x0
> +#define SET                            0x4
> +#define CLR                            0x8
> +#define MASK_STATUS                    0xc
> +#define MASK_SET                       0x10
> +#define MASK_CLR                       0x14
> +
> +#define BRCM_NUM_PCIE_OUT_WINS         0x4
> +#define BRCM_MAX_SCB                   0x4
> +
> +#define BURST_SIZE_128                 0
> +
> +#define PCIE_BUSNUM_SHIFT              20
> +#define PCIE_SLOT_SHIFT                        15
> +#define PCIE_FUNC_SHIFT                        12
> +
> +#if defined(__BIG_ENDIAN)
> +#define        DATA_ENDIAN                     2       /* PCIe->DDR inbound traffic */
> +#define MMIO_ENDIAN                    2       /* CPU->PCIe outbound traffic */
> +#else
> +#define        DATA_ENDIAN                     0
> +#define MMIO_ENDIAN                    0
> +#endif
> +
> +#define MDIO_PORT0                     0x0
> +#define MDIO_DATA_MASK                 0x7fffffff
> +#define MDIO_DATA_SHIFT                        0x0
> +#define MDIO_PORT_MASK                 0xf0000
> +#define MDIO_PORT_SHIFT                        0x16
> +#define MDIO_REGAD_MASK                        0xffff
> +#define MDIO_REGAD_SHIFT               0x0
> +#define MDIO_CMD_MASK                  0xfff00000
> +#define MDIO_CMD_SHIFT                 0x14
> +#define MDIO_CMD_READ                  0x1
> +#define MDIO_CMD_WRITE                 0x0
> +#define MDIO_DATA_DONE_MASK            0x80000000
> +#define MDIO_RD_DONE(x)                        (((x) & MDIO_DATA_DONE_MASK) ? 1 : 0)
> +#define MDIO_WT_DONE(x)                        (((x) & MDIO_DATA_DONE_MASK) ? 0 : 1)
> +#define SSC_REGS_ADDR                  0x1100
> +#define SET_ADDR_OFFSET                        0x1f
> +#define SSC_CNTL_OFFSET                        0x2
> +#define SSC_CNTL_OVRD_EN_MASK          0x8000
> +#define SSC_CNTL_OVRD_EN_SHIFT         0xf
> +#define SSC_CNTL_OVRD_VAL_MASK         0x4000
> +#define SSC_CNTL_OVRD_VAL_SHIFT                0xe
> +#define SSC_STATUS_OFFSET              0x1
> +#define SSC_STATUS_SSC_MASK            0x400
> +#define SSC_STATUS_SSC_SHIFT           0xa
> +#define SSC_STATUS_PLL_LOCK_MASK       0x800
> +#define SSC_STATUS_PLL_LOCK_SHIFT      0xb
> +
> +#define IDX_ADDR(pcie) ((pcie)->reg_offsets[EXT_CFG_INDEX])
> +#define DATA_ADDR(pcie)        ((pcie)->reg_offsets[EXT_CFG_DATA])
> +#define PCIE_RGR1_SW_INIT_1(pcie) ((pcie)->reg_offsets[RGR1_SW_INIT_1])
> +
> +enum {
> +       RGR1_SW_INIT_1,
> +       EXT_CFG_INDEX,
> +       EXT_CFG_DATA,
> +};
> +
> +enum {
> +       RGR1_SW_INIT_1_INIT_MASK,
> +       RGR1_SW_INIT_1_INIT_SHIFT,
> +       RGR1_SW_INIT_1_PERST_MASK,
> +       RGR1_SW_INIT_1_PERST_SHIFT,
> +};
> +
> +enum pcie_type {
> +       BCM7425,
> +       BCM7435,
> +       GENERIC,
> +       BCM7278,
> +       BCM2711,
> +};
> +
> +struct brcm_window {
> +       dma_addr_t pci_addr;
> +       phys_addr_t phys_addr;
> +       dma_addr_t size;
> +};
> +
> +struct brcm_pcie {
> +       struct resource         mem;
> +       bool                    ssc;
> +
> +       struct device_node      *dn;
> +       void __iomem            *base;
> +
> +       int                     num_out_wins;
> +       struct brcm_window      out_wins[BRCM_NUM_PCIE_OUT_WINS];
> +
> +       int                     gen;
> +       unsigned int            rev;
> +
> +       const int               *reg_offsets;
> +       const int               *reg_field_info;
> +       u32                     max_burst_size;
> +       enum pcie_type          type;
> +};
> +
> +struct pcie_cfg_data {
> +       const int               *reg_field_info;
> +       const int               *offsets;
> +       const u32               max_burst_size;
> +       const enum pcie_type    type;
> +};
> +
> +static struct brcm_window dma_ranges[1];
> +static int num_dma_ranges;
> +
> +static const int pcie_reg_field_info[] = {
> +       [RGR1_SW_INIT_1_INIT_MASK] = 0x2,
> +       [RGR1_SW_INIT_1_INIT_SHIFT] = 0x1,
> +};
> +
> +static const int pcie_offsets[] = {
> +       [RGR1_SW_INIT_1] = 0x9210,
> +       [EXT_CFG_INDEX]  = 0x9000,
> +       [EXT_CFG_DATA]   = 0x8000,
> +};
> +
> +static const struct pcie_cfg_data generic_cfg = {
> +       .reg_field_info = pcie_reg_field_info,
> +       .offsets        = pcie_offsets,
> +       .max_burst_size = BURST_SIZE_128,
> +       .type           = GENERIC,
> +};
> +
> +static const struct pcie_cfg_data bcm2711_cfg = {
> +       .reg_field_info = pcie_reg_field_info,
> +       .offsets        = pcie_offsets,
> +       .max_burst_size = BURST_SIZE_128,
> +       .type           = BCM2711,
> +};
> +
> +/* These macros extract/insert fields to host controller's register set */
> +#define WR_FLD(base, reg, field, val) \
> +       wr_fld(base + reg, reg##_##field##_MASK, reg##_##field##_SHIFT, val)
> +#define WR_FLD_RB(base, reg, field, val) \
> +       wr_fld_rb(base + reg, reg##_##field##_MASK, reg##_##field##_SHIFT, val)
> +#define WR_FLD_WITH_OFFSET(base, off, reg, field, val) \
> +       wr_fld(base + reg + off, reg##_##field##_MASK, \
> +              reg##_##field##_SHIFT, val)
> +#define EXTRACT_FIELD(val, reg, field) \
> +       ((val & reg##_##field##_MASK) >> reg##_##field##_SHIFT)
> +#define INSERT_FIELD(val, reg, field, field_val) \
> +       ((val & ~reg##_##field##_MASK) | \
> +        (reg##_##field##_MASK & (field_val << reg##_##field##_SHIFT)))
> +
> +#define msleep(a) udelay((a) * 1000)
> +
> +/*
> + * The roundup_pow_of_two() from log2.h invokes
> + * __roundup_pow_of_two(unsigned long), but we really need
> + * such a function to take a native u64 since unsigned long
> + * is 32 bits on some configurations.  So we provide this helper
> + * function below.
> + */
> +static u64 roundup_pow_of_two_64(u64 n)
> +{
> +       return 1ULL << fls64(n - 1);
> +}
> +
> +/*
> + * This is to convert the size of the inbound "BAR" region to the
> + * non-linear values of PCIE_X_MISC_RC_BAR[123]_CONFIG_LO.SIZE
> + */
> +int encode_ibar_size(u64 size)

This should be static

> +{
> +       int log2_in = ilog2(size);
> +
> +       if (log2_in >= 12 && log2_in <= 15)
> +               /* Covers 4KB to 32KB (inclusive) */
> +               return (log2_in - 12) + 0x1c;
> +       else if (log2_in >= 16 && log2_in <= 37)
> +               /* Covers 64KB to 32GB, (inclusive) */
> +               return log2_in - 15;
> +       /* Something is awry so disable */
> +       return 0;
> +}
> +
> +/* Configuration space read/write support */
> +static int cfg_index(int busnr, int devfn, int reg)
> +{
> +       return (PCI_DEV(devfn) << PCIE_SLOT_SHIFT)
> +               | (PCI_FUNC(devfn) << PCIE_FUNC_SHIFT)
> +               | (busnr << PCIE_BUSNUM_SHIFT)
> +               | (reg & ~3);
> +}
> +
> +/* The controller is capable of serving in both RC and EP roles */
> +static bool brcm_pcie_rc_mode(struct brcm_pcie *pcie)
> +{
> +       void __iomem *base = pcie->base;
> +       u32 val = readl(base + PCIE_MISC_PCIE_STATUS);
> +
> +       return !!EXTRACT_FIELD(val, PCIE_MISC_PCIE_STATUS, PCIE_PORT);
> +}
> +
> +static bool brcm_pcie_link_up(struct brcm_pcie *pcie)
> +{
> +       void __iomem *base = pcie->base;
> +       u32 val = readl(base + PCIE_MISC_PCIE_STATUS);
> +       u32 dla = EXTRACT_FIELD(val, PCIE_MISC_PCIE_STATUS, PCIE_DL_ACTIVE);
> +       u32 plu = EXTRACT_FIELD(val, PCIE_MISC_PCIE_STATUS, PCIE_PHYLINKUP);
> +
> +       return  (dla && plu) ? true : false;
> +}
> +
> +static int brcm_pcie_config_address(const struct udevice *udev, pci_dev_t bdf,
> +                                   uint offset, void **paddress)
> +{
> +       struct brcm_pcie *pcie = dev_get_priv(udev);
> +       unsigned int bus = PCI_BUS(bdf);
> +       unsigned int dev = PCI_DEV(bdf);
> +       int idx;
> +
> +       if (bus > 0 && !brcm_pcie_link_up(pcie))
> +               return -ENODEV;
> +
> +       /*
> +        * Busses 0 (host PCIe bridge) and 1 (its immediate child)
> +        * are limited to a single device each
> +        */
> +       if ((bus == (udev->seq + 1)) && dev > 0)
> +               return -ENODEV;
> +
> +       /* Accesses to the RC go right to the RC registers if PCI device == 0 */
> +       if (bus == udev->seq) {
> +               if (PCI_DEV(bdf))
> +                       return -ENODEV;
> +
> +               *paddress = pcie->base + offset;
> +               return 0;
> +       }
> +
> +       /* For devices, write to the config space index register */
> +       idx = cfg_index(bus, bdf, 0);
> +
> +       writel(idx, pcie->base + IDX_ADDR(pcie));
> +       *paddress = pcie->base + DATA_ADDR(pcie) + offset;
> +
> +       return 0;
> +}
> +
> +static int brcm_pcie_read_config(const struct udevice *bus, pci_dev_t bdf,
> +                                uint offset, ulong *valuep,
> +                                enum pci_size_t size)
> +{
> +       return pci_generic_mmap_read_config(bus, brcm_pcie_config_address,
> +                                           bdf, offset, valuep, size);
> +}
> +
> +static int brcm_pcie_write_config(struct udevice *bus, pci_dev_t bdf,
> +                                 uint offset, ulong value,
> +                                 enum pci_size_t size)
> +{
> +       return pci_generic_mmap_write_config(bus, brcm_pcie_config_address,
> +                                            bdf, offset, value, size);
> +}
> +
> +static void wr_fld(void __iomem *p, u32 mask, int shift, u32 val)
> +{
> +       u32 reg = readl(p);
> +
> +       reg = (reg & ~mask) | ((val << shift) & mask);
> +       writel(reg, p);
> +}
> +
> +static void wr_fld_rb(void __iomem *p, u32 mask, int shift, u32 val)
> +{
> +       wr_fld(p, mask, shift, val);
> +       (void)readl(p);
> +}
> +
> +static const char *link_speed_to_str(int s)
> +{
> +       switch (s) {
> +       case 1:
> +               return "2.5";
> +       case 2:
> +               return "5.0";
> +       case 3:
> +               return "8.0";
> +       default:
> +               break;
> +       }
> +       return "???";
> +}
> +
> +static inline void brcm_pcie_bridge_sw_init_set(struct brcm_pcie *pcie,
> +                                               unsigned int val)
> +{
> +       unsigned int shift = pcie->reg_field_info[RGR1_SW_INIT_1_INIT_SHIFT];
> +       u32 mask =  pcie->reg_field_info[RGR1_SW_INIT_1_INIT_MASK];
> +
> +       wr_fld_rb(pcie->base + PCIE_RGR1_SW_INIT_1(pcie), mask, shift, val);
> +}
> +
> +static inline void brcm_pcie_perst_set(struct brcm_pcie *pcie,
> +                                      unsigned int val)
> +{
> +       if (pcie->type != BCM7278)
> +               wr_fld_rb(pcie->base + PCIE_RGR1_SW_INIT_1(pcie),
> +                         PCIE_RGR1_SW_INIT_1_PERST_MASK,
> +                         PCIE_RGR1_SW_INIT_1_PERST_SHIFT, val);
> +       else
> +               /* Assert = 0, de-assert = 1 on 7278 */
> +               WR_FLD_RB(pcie->base, PCIE_MISC_PCIE_CTRL, PCIE_PERSTB, !val);
> +}
> +
> +static u32 mdio_form_pkt(int port, int regad, int cmd)
> +{
> +       u32 pkt = 0;
> +
> +       pkt |= (port << MDIO_PORT_SHIFT) & MDIO_PORT_MASK;
> +       pkt |= (regad << MDIO_REGAD_SHIFT) & MDIO_REGAD_MASK;
> +       pkt |= (cmd << MDIO_CMD_SHIFT) & MDIO_CMD_MASK;
> +
> +       return pkt;
> +}
> +
> +/* Negative return value indicates error */
> +static int mdio_read(void __iomem *base, u8 port, u8 regad)
> +{
> +       int tries;
> +       u32 data;
> +
> +       writel(mdio_form_pkt(port, regad, MDIO_CMD_READ),
> +              base + PCIE_RC_DL_MDIO_ADDR);
> +       readl(base + PCIE_RC_DL_MDIO_ADDR);
> +
> +       data = readl(base + PCIE_RC_DL_MDIO_RD_DATA);
> +       for (tries = 0; !MDIO_RD_DONE(data) && tries < 10; tries++) {
> +               udelay(10);
> +               data = readl(base + PCIE_RC_DL_MDIO_RD_DATA);
> +       }
> +
> +       return MDIO_RD_DONE(data)
> +               ? (data & MDIO_DATA_MASK) >> MDIO_DATA_SHIFT
> +               : -EIO;
> +}
> +
> +/* Negative return value indicates error */
> +static int mdio_write(void __iomem *base, u8 port, u8 regad, u16 wrdata)
> +{
> +       int tries;
> +       u32 data;
> +
> +       writel(mdio_form_pkt(port, regad, MDIO_CMD_WRITE),
> +              base + PCIE_RC_DL_MDIO_ADDR);
> +       readl(base + PCIE_RC_DL_MDIO_ADDR);
> +       writel(MDIO_DATA_DONE_MASK | wrdata,
> +              base + PCIE_RC_DL_MDIO_WR_DATA);
> +
> +       data = readl(base + PCIE_RC_DL_MDIO_WR_DATA);
> +       for (tries = 0; !MDIO_WT_DONE(data) && tries < 10; tries++) {
> +               udelay(10);
> +               data = readl(base + PCIE_RC_DL_MDIO_WR_DATA);
> +       }
> +
> +       return MDIO_WT_DONE(data) ? 0 : -EIO;
> +}
> +
> +/*
> + * Configures device for Spread Spectrum Clocking (SSC) mode; negative
> + * return value indicates error.
> + */
> +static int set_ssc(void __iomem *base)
> +{
> +       int tmp;
> +       u16 wrdata;
> +       int pll, ssc;
> +
> +       tmp = mdio_write(base, MDIO_PORT0, SET_ADDR_OFFSET, SSC_REGS_ADDR);
> +       if (tmp < 0)
> +               return tmp;
> +
> +       tmp = mdio_read(base, MDIO_PORT0, SSC_CNTL_OFFSET);
> +       if (tmp < 0)
> +               return tmp;
> +
> +       wrdata = INSERT_FIELD(tmp, SSC_CNTL_OVRD, EN, 1);
> +       wrdata = INSERT_FIELD(wrdata, SSC_CNTL_OVRD, VAL, 1);
> +       tmp = mdio_write(base, MDIO_PORT0, SSC_CNTL_OFFSET, wrdata);
> +       if (tmp < 0)
> +               return tmp;
> +
> +       udelay(1500);
> +       tmp = mdio_read(base, MDIO_PORT0, SSC_STATUS_OFFSET);
> +       if (tmp < 0)
> +               return tmp;
> +
> +       ssc = EXTRACT_FIELD(tmp, SSC_STATUS, SSC);
> +       pll = EXTRACT_FIELD(tmp, SSC_STATUS, PLL_LOCK);
> +
> +       return (ssc && pll) ? 0 : -EIO;
> +}
> +
> +/* Limits operation to a specific generation (1, 2, or 3) */
> +static void set_gen(void __iomem *base, int gen)
> +{
> +       u32 lnkcap = readl(base + BRCM_PCIE_CAP_REGS + PCI_EXP_LNKCAP);
> +       u16 lnkctl2 = readw(base + BRCM_PCIE_CAP_REGS + PCI_EXP_LNKCTL2);
> +
> +       lnkcap = (lnkcap & ~PCI_EXP_LNKCAP_SLS) | gen;
> +       writel(lnkcap, base + BRCM_PCIE_CAP_REGS + PCI_EXP_LNKCAP);
> +
> +       lnkctl2 = (lnkctl2 & ~0xf) | gen;
> +       writew(lnkctl2, base + BRCM_PCIE_CAP_REGS + PCI_EXP_LNKCTL2);
> +}
> +
> +static void brcm_pcie_set_outbound_win(struct brcm_pcie *pcie,
> +                                      unsigned int win, phys_addr_t phys_addr,
> +                                      dma_addr_t pcie_addr, dma_addr_t size)
> +{
> +       void __iomem *base = pcie->base;
> +       phys_addr_t phys_addr_mb, limit_addr_mb;
> +       u32 tmp;
> +
> +       /* Set the base of the pcie_addr window */
> +       writel(lower_32_bits(pcie_addr) + MMIO_ENDIAN,
> +              base + PCIE_MISC_CPU_2_PCIE_MEM_WIN0_LO + (win * 8));
> +       writel(upper_32_bits(pcie_addr),
> +              base + PCIE_MISC_CPU_2_PCIE_MEM_WIN0_HI + (win * 8));
> +
> +       phys_addr_mb = phys_addr >> 20;
> +       limit_addr_mb = (phys_addr + size - 1) >> 20;
> +
> +       /* Write the addr base low register */
> +       WR_FLD_WITH_OFFSET(base, (win * 4),
> +                          PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_LIMIT,
> +                          BASE, phys_addr_mb);
> +       /* Write the addr limit low register */
> +       WR_FLD_WITH_OFFSET(base, (win * 4),
> +                          PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_LIMIT,
> +                          LIMIT, limit_addr_mb);
> +
> +       if (pcie->type != BCM7435 && pcie->type != BCM7425) {
> +               /* Write the cpu addr high register */
> +               tmp = (u32)(phys_addr_mb >>
> +                       PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_LIMIT_NUM_MASK_BITS);
> +               WR_FLD_WITH_OFFSET(base, (win * 8),
> +                                  PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_HI,
> +                                  BASE, tmp);
> +               /* Write the cpu limit high register */
> +               tmp = (u32)(limit_addr_mb >>
> +                       PCIE_MISC_CPU_2_PCIE_MEM_WIN0_BASE_LIMIT_NUM_MASK_BITS);
> +               WR_FLD_WITH_OFFSET(base, (win * 8),
> +                                  PCIE_MISC_CPU_2_PCIE_MEM_WIN0_LIMIT_HI,
> +                                  LIMIT, tmp);
> +       }
> +}
> +
> +static int brcm_pcie_setup(struct brcm_pcie *pcie)
> +{
> +       phys_addr_t scb_size[BRCM_MAX_SCB] = {0};
> +       void __iomem *base = pcie->base;
> +       unsigned int scb_size_val;
> +       u64 rc_bar2_offset, rc_bar2_size;
> +       u64 total_mem_size = 0;
> +       int i, j, ret, limit, num_memc;
> +       u16 nlw, cls, lnksta;
> +       bool ssc_good = false;
> +       u32 tmp;
> +
> +       /* Reset the bridge */
> +       brcm_pcie_bridge_sw_init_set(pcie, 1);
> +
> +       /*
> +        * Ensure that the fundamental reset is asserted, except for 7278,
> +        * which fails if we do this.
> +        */
> +       if (pcie->type != BCM7278)
> +               brcm_pcie_perst_set(pcie, 1);
> +
> +       udelay(150);
> +
> +       /* Take the bridge out of reset */
> +       brcm_pcie_bridge_sw_init_set(pcie, 0);
> +
> +       WR_FLD_RB(base, PCIE_MISC_HARD_PCIE_HARD_DEBUG, SERDES_IDDQ, 0);
> +       /* Wait for SerDes to be stable */
> +       udelay(150);
> +
> +       /* Grab the PCIe hw revision number */
> +       tmp = readl(base + PCIE_MISC_REVISION);
> +       pcie->rev = EXTRACT_FIELD(tmp, PCIE_MISC_REVISION, MAJMIN);
> +
> +       /* Set SCB_MAX_BURST_SIZE, CFG_READ_UR_MODE, SCB_ACCESS_EN */
> +       tmp = INSERT_FIELD(0, PCIE_MISC_MISC_CTRL, SCB_ACCESS_EN, 1);
> +       tmp = INSERT_FIELD(tmp, PCIE_MISC_MISC_CTRL, CFG_READ_UR_MODE, 1);
> +       tmp = INSERT_FIELD(tmp, PCIE_MISC_MISC_CTRL, MAX_BURST_SIZE,
> +                          pcie->max_burst_size);
> +       writel(tmp, base + PCIE_MISC_MISC_CTRL);
> +
> +       /*
> +        * Set up inbound memory view for the EP (called RC_BAR2, not to be
> +        * confused with the BARs that are advertised by the EP).
> +        *
> +        * The PCIe host controller by design must set the inbound viewport
> +        * to be a contiguous arrangement of all of the system's memory.
> +        * In addition, its size must be a power of two.  Further, the MSI
> +        * target address must NOT be placed inside this region, as the
> +        * decoding logic will consider its address to be inbound memory
> +        * traffic.  To further complicate matters, the viewport must start
> +        * on a pcie-address that is aligned on a multiple of its size.
> +        * If a portion of the viewport does not represent system memory
> +        * -- e.g. 3GB of memory requires a 4GB viewport -- we can map
> +        * the outbound memory in or after 3GB and even though the viewport
> +        * will overlap the outbound memory the controller will know to send
> +        * outbound memory downstream and everything else upstream.
> +        */
> +
> +       if (num_dma_ranges) {
> +               /*
> +                * Use the base address and size(s) provided in the dma-ranges
> +                * property.
> +                */
> +               for (i = 0; i < num_dma_ranges; i++)
> +                       scb_size[i] = roundup_pow_of_two_64(dma_ranges[i].size);
> +
> +               num_memc = num_dma_ranges;
> +               rc_bar2_offset = dma_ranges[0].pci_addr;
> +       } else {
> +               return -EINVAL;
> +       }
> +
> +       for (i = 0; i < num_memc; i++)
> +               total_mem_size += scb_size[i];
> +
> +       rc_bar2_size = roundup_pow_of_two_64(total_mem_size);
> +
> +       /* Verify the alignment is correct */
> +       if (rc_bar2_offset & (rc_bar2_size - 1)) {
> +               printf("PCIe BRCM: inbound window is misaligned\n");
> +               return -EINVAL;
> +       }
> +
> +       tmp = lower_32_bits(rc_bar2_offset);
> +       tmp = INSERT_FIELD(tmp, PCIE_MISC_RC_BAR2_CONFIG_LO, SIZE,
> +                          encode_ibar_size(rc_bar2_size));
> +       writel(tmp, base + PCIE_MISC_RC_BAR2_CONFIG_LO);
> +       writel(upper_32_bits(rc_bar2_offset),
> +              base + PCIE_MISC_RC_BAR2_CONFIG_HI);
> +
> +       scb_size_val = scb_size[0]
> +               ? ilog2(scb_size[0]) - 15 : 0xf; /* 0xf is 1GB */
> +       WR_FLD(base, PCIE_MISC_MISC_CTRL, SCB0_SIZE, scb_size_val);
> +
> +       if (num_memc > 1) {
> +               scb_size_val = scb_size[1]
> +                       ? ilog2(scb_size[1]) - 15 : 0xf; /* 0xf is 1GB */
> +               WR_FLD(base, PCIE_MISC_MISC_CTRL, SCB1_SIZE, scb_size_val);
> +       }
> +
> +       if (num_memc > 2) {
> +               scb_size_val = scb_size[2]
> +                       ? ilog2(scb_size[2]) - 15 : 0xf; /* 0xf is 1GB */
> +               WR_FLD(base, PCIE_MISC_MISC_CTRL, SCB2_SIZE, scb_size_val);
> +       }
> +
> +       /* Disable the PCIe->GISB memory window (RC_BAR1) */
> +       WR_FLD(base, PCIE_MISC_RC_BAR1_CONFIG_LO, SIZE, 0);
> +
> +       /* Disable the PCIe->SCB memory window (RC_BAR3) */
> +       WR_FLD(base, PCIE_MISC_RC_BAR3_CONFIG_LO, SIZE, 0);
> +
> +       /* Clear any interrupts we find on boot */
> +       writel(0xffffffff, base + PCIE_INTR2_CPU_BASE + CLR);
> +       (void)readl(base + PCIE_INTR2_CPU_BASE + CLR);
> +
> +       /* Mask all interrupts since we are not handling any yet */
> +       writel(0xffffffff, base + PCIE_INTR2_CPU_BASE + MASK_SET);
> +       (void)readl(base + PCIE_INTR2_CPU_BASE + MASK_SET);
> +
> +       if (pcie->gen)
> +               set_gen(base, pcie->gen);
> +
> +       /* Unassert the fundamental reset */
> +       brcm_pcie_perst_set(pcie, 0);
> +
> +       /* Give the RC/EP time to wake up, before trying to configure RC. */
> +       limit = 100;
> +       for (i = 1, j = 0; j < limit && !brcm_pcie_link_up(pcie);
> +            j += i, i = i * 2)
> +               msleep(i + j > limit ? limit - j : i);
> +
> +       if (!brcm_pcie_link_up(pcie)) {
> +               printf("PCIe BRCM: link down\n");
> +               return -ENODEV;
> +       }
> +
> +       if (!brcm_pcie_rc_mode(pcie)) {
> +               printf("PCIe misconfigured; is in EP mode\n");
> +               return -EINVAL;
> +       }
> +
> +       for (i = 0; i < pcie->num_out_wins; i++)
> +               brcm_pcie_set_outbound_win(pcie, i, pcie->out_wins[i].phys_addr,
> +                                          pcie->out_wins[i].pci_addr,
> +                                          pcie->out_wins[i].size);
> +
> +       /*
> +        * For config space accesses on the RC, show the right class for
> +        * a PCIe-PCIe bridge (the default setting is to be EP mode).
> +        */
> +       WR_FLD_RB(base, PCIE_RC_CFG_PRIV1_ID_VAL3, CLASS_CODE, 0x060400);
> +
> +       if (pcie->ssc) {
> +               ret = set_ssc(base);
> +               if (ret == 0)
> +                       ssc_good = true;
> +               else
> +                       printf("PCIe BRCM: failed attempt to enter ssc mode\n");
> +       }
> +
> +       lnksta = readw(base + BRCM_PCIE_CAP_REGS + PCI_EXP_LNKSTA);
> +       cls = lnksta & PCI_EXP_LNKSTA_CLS;
> +       nlw = (lnksta & PCI_EXP_LNKSTA_NLW) >> PCI_EXP_LNKSTA_NLW_SHIFT;
> +
> +       printf("PCIe BRCM: link up, %s Gbps x%u %s\n", link_speed_to_str(cls),
> +              nlw, ssc_good ? "(SSC)" : "(!SSC)");
> +
> +       /* PCIe->SCB endian mode for BAR */
> +       /* field ENDIAN_MODE_BAR2 = DATA_ENDIAN */
> +       WR_FLD_RB(base, PCIE_RC_CFG_VENDOR_VENDOR_SPECIFIC_REG1,
> +                 ENDIAN_MODE_BAR2, DATA_ENDIAN);
> +
> +       /*
> +        * Refclk from RC should be gated with CLKREQ# input when ASPM L0s,L1
> +        * is enabled =>  setting the CLKREQ_DEBUG_ENABLE field to 1.
> +        */
> +       WR_FLD_RB(base, PCIE_MISC_HARD_PCIE_HARD_DEBUG, CLKREQ_DEBUG_ENABLE, 1);
> +
> +       return 0;
> +}
> +
> +static const struct udevice_id brcm_pcie_ids[] = {
> +       { .compatible = "brcm,bcm7445-pcie", .data = (ulong)&generic_cfg },
> +       { .compatible = "brcm,bcm2711-pcie", .data = (ulong)&bcm2711_cfg },
> +       { }
> +};
> +
> +static int brcm_pcie_probe(struct udevice *dev)
> +{
> +       struct udevice *ctlr = pci_get_controller(dev);
> +       struct pci_controller *hose = dev_get_uclass_priv(ctlr);
> +       struct brcm_pcie *pcie = dev_get_priv(dev);
> +       struct pcie_cfg_data *data = (struct pcie_cfg_data *)dev_get_driver_data(dev);
> +       ofnode dn = dev_ofnode(dev);
> +       u32 max_link_speed;
> +       int i;
> +
> +       if (hose->region_count < 2) {
> +               pr_err("PCIe BRCM: Missing PCI regions definition");
> +               return -EINVAL;
> +       }
> +
> +       for (i = 0; i < hose->region_count; i++) {
> +               if (hose->regions[i].flags == PCI_REGION_SYS_MEMORY)
> +                       continue;
> +
> +               pcie->out_wins[i].phys_addr = hose->regions[i].phys_start;
> +               pcie->out_wins[i].pci_addr = hose->regions[i].bus_start;
> +               pcie->out_wins[i].size = hose->regions[i].size;
> +               pcie->num_out_wins++;
> +       }
> +
> +       /* TODO: Parse inbound access mapping from dma-ranges DT property */
> +       num_dma_ranges = 1;
> +       for (i = 0; i < hose->region_count; i++) {
> +               if (hose->regions[i].flags != PCI_REGION_SYS_MEMORY)
> +                       continue;
> +
> +               dma_ranges[0].pci_addr = hose->regions[i].bus_start;
> +               dma_ranges[0].phys_addr = hose->regions[i].phys_start;
> +               dma_ranges[0].size = hose->regions[i].size;
> +               break;
> +       }
> +
> +       pcie->reg_offsets = data->offsets;
> +       pcie->reg_field_info = data->reg_field_info;
> +       pcie->max_burst_size = data->max_burst_size;
> +       pcie->type = data->type;
> +
> +       if (ofnode_read_u32(dn, "max-link-speed", &max_link_speed) ||
> +           max_link_speed > 4)
> +               pcie->gen = 0;
> +       else
> +               pcie->gen = max_link_speed;
> +
> +       pcie->ssc = ofnode_read_bool(dn, "brcm,enable-ssc");
> +
> +       return brcm_pcie_setup(pcie);
> +}
> +
> +static int brcm_pcie_ofdata_to_platdata(struct udevice *dev)
> +{
> +       struct brcm_pcie *pcie = dev_get_priv(dev);
> +
> +       /* Get the controller base address */
> +       pcie->base = dev_read_addr_ptr(dev);
> +       if (!pcie->base)
> +               return -EINVAL;
> +
> +       return 0;
> +}
> +
> +static const struct dm_pci_ops brcm_pcie_ops = {
> +       .read_config    = brcm_pcie_read_config,
> +       .write_config   = brcm_pcie_write_config,
> +};
> +
> +U_BOOT_DRIVER(pcie_brcm_base) = {
> +       .name                   = "pcie_brcm",
> +       .id                     = UCLASS_PCI,
> +       .ops                    = &brcm_pcie_ops,
> +       .of_match               = brcm_pcie_ids,
> +       .probe                  = brcm_pcie_probe,
> +       .ofdata_to_platdata     = brcm_pcie_ofdata_to_platdata,
> +       .priv_auto_alloc_size   = sizeof(struct brcm_pcie),
> +};
> --

Regards,
Bin

Hi Sylwester,

On Tue, 2020-04-21 at 18:50 +0200, Sylwester Nawrocki wrote:
> This patch adds basic driver for the Broadcom STB PCIe host controller.
> The code is based on Linux upstream driver (pcie-brcmtsb.c) with MSI
> handling removed. The inbound access memory region is not currently
> parsed from dma-ranges DT property and is fixed as a 1:1 mapping of
> whole RAM.
> The patch has been tested on rpi4 board, i.e. on BCM2711 SoC with VL805
> USB Host Controller.
> 
> Signed-off-by: Sylwester Nawrocki <s.nawrocki at samsung.com>
> ---

I see that you based your code on the downstream PCIe driver (the one available
in the Raspberry Pi foundation kernel repos). There's a real upstream version
of it[1], It's properly reviewed and we made a point to remove all non bcm2711
specific stuff from it, as support for other platforms (like bcm7445) isn't
there yet (it depends on supporting multiple dma-ranges in Linux, which we
don't). Also note that the Raspberry Pi foundation is moving towards using this
driver, see for example their rpi-5.6.y branch, and I belive Broadcom is doing
the same.

So I strongly suggest you base this driver on the upstream version. As I was
working towards the same goal as you I already have an implementation for
u-boot myself[2]. Feel free to use that if you find it useful. Ultimately,
whether you base it on my version or the upstream Linux version it's half the
size and easier to follow.

Regards,
Nicolas

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pci/controller/pcie-brcmstb.c?h=v5.7-rc2
[2] https://github.com/vianpl/u-boot/commits/brcm-pcie

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: This is a digitally signed message part
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200422/d53649cb/attachment.sig>

Hi Nicolas,

On 22.04.2020 11:16, Nicolas Saenz Julienne wrote:
 
> I see that you based your code on the downstream PCIe driver (the one available
> in the Raspberry Pi foundation kernel repos). There's a real upstream version
> of it[1], It's properly reviewed and we made a point to remove all non bcm2711
> specific stuff from it, as support for other platforms (like bcm7445) isn't
> there yet (it depends on supporting multiple dma-ranges in Linux, which we
> don't). Also note that the Raspberry Pi foundation is moving towards using this
> driver, see for example their rpi-5.6.y branch, and I belive Broadcom is doing
> the same.
> 
> So I strongly suggest you base this driver on the upstream version. As I was
> working towards the same goal as you I already have an implementation for
> u-boot myself[2]. Feel free to use that if you find it useful. Ultimately,
> whether you base it on my version or the upstream Linux version it's half the
> size and easier to follow.

Thanks, I'm going to reuse you code, it looks much cleaner.
I started working on this PCIe driver back at end of 2019, then that task was
postponed for few months. I should have been following better related Linux
mainline works. Now, when it all more or less works I will try reuse your code.
> [1] 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pci/controller/pcie-brcmstb.c?h=v5.7-rc2
> [2] https://protect2.fireeye.com/url?k=14361ea6-49a8854c-143795e9-0cc47a6cba04-df90fb3bfec30a2d&q=1&u=https%3A%2F%2Fgithub.com%2Fvianpl%2Fu-boot%2Fcommits%2Fbrcm-pcie
>

On Wed, 2020-04-22 at 18:42 +0200, Sylwester Nawrocki wrote:
> Hi Nicolas,
> 
> On 22.04.2020 11:16, Nicolas Saenz Julienne wrote:
>  
> > I see that you based your code on the downstream PCIe driver (the one
> > available
> > in the Raspberry Pi foundation kernel repos). There's a real upstream
> > version
> > of it[1], It's properly reviewed and we made a point to remove all non
> > bcm2711
> > specific stuff from it, as support for other platforms (like bcm7445) isn't
> > there yet (it depends on supporting multiple dma-ranges in Linux, which we
> > don't). Also note that the Raspberry Pi foundation is moving towards using
> > this
> > driver, see for example their rpi-5.6.y branch, and I belive Broadcom is
> > doing
> > the same.
> > 
> > So I strongly suggest you base this driver on the upstream version. As I was
> > working towards the same goal as you I already have an implementation for
> > u-boot myself[2]. Feel free to use that if you find it useful. Ultimately,
> > whether you base it on my version or the upstream Linux version it's half
> > the
> > size and easier to follow.
> 
> Thanks, I'm going to reuse you code, it looks much cleaner.
> I started working on this PCIe driver back at end of 2019, then that task was
> postponed for few months. I should have been following better related Linux
> mainline works. Now, when it all more or less works I will try reuse your
> code.

Nice, FYI I replaced the driver as is and it worked out of the box, so it
should be relatively painless.

Regards,
Nicolas

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: This is a digitally signed message part
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200422/585d2a1f/attachment.sig>