[U-Boot] [PATCH 3/4] pci: mediatek: Add pci-driver for mt2701
Ryder Lee
ryder.lee at mediatek.com
Wed Jul 31 14:35:32 UTC 2019
On Wed, 2019-07-31 at 17:13 +0300, Aleksandr Rybalko wrote:
> Hello Ryder.
>
>
> ср, 31 лип. 2019 о 15:45 Ryder Lee <ryder.lee at mediatek.com> пише:
>
> + GSS_MTK_Uboot_upstream <GSS_MTK_Uboot_upstream at mediatek.com>
>
> On Wed, 2019-07-31 at 13:51 +0200, Frank Wunderlich wrote:
> > From: Oleksandr Rybalko <ray at ddteam.net>
> >
> > this chip is used in MT7623 and some other Mediatek SoCs for
> pcie
> >
> > Tested-by: Frank Wunderlich <frank-w at public-files.de>
> > Signed-off-by: Frank Wunderlich <frank-w at public-files.de>
> > Signed-off-by: Oleksandr Rybalko <ray at ddteam.net>
> > ---
> > drivers/pci/Kconfig | 6 +
> > drivers/pci/Makefile | 1 +
> > drivers/pci/pci-mt2701.c | 490
> +++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 497 insertions(+)
> > create mode 100644 drivers/pci/pci-mt2701.c
>
> Rename 'pci-mt2701.c' to 'pcie-mediatek.c' and then change the
> subject.
>
>
> So you promise us, that Mediatek will never produce PCI-E controller
> which will be totally different from that one? :)
imho we can use single driver for different IP generation and that is
what we did in linux now.
MT7623/MT2701 - mtk_pcie_soc_v1
MT7622/MT7629 - v2
gen3 IP - TBD
>
>
> Obviously, this is an intermediate version of Linux patch, so
> I suggest
> to take a look at the latest version of vanilla Kernel:
> https://github.com/torvalds/linux/blob/master/drivers/pci/controller/pcie-mediatek.c
>
> Please see my comments inline:
>
> > diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
> > index 3fe38f7315..cfe8ba5e52 100644
> > --- a/drivers/pci/Kconfig
> > +++ b/drivers/pci/Kconfig
> > @@ -145,4 +145,10 @@ config PCI_MVEBU
> > Say Y here if you want to enable PCIe controller
> support on
> > Armada XP/38x SoCs.
> >
> > +config PCIE_MT2701
> > + bool "Mediatek 2701 PCI-E"
> > + help
> > + Say Y here if you want to enable PCIe controller
> support on
> > + Mediatek MT7623
> > +
> > endif
> > diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
> > index b5ebd50c85..a4c4002b9c 100644
> > --- a/drivers/pci/Makefile
> > +++ b/drivers/pci/Makefile
> > @@ -38,3 +38,4 @@ obj-$(CONFIG_PCIE_LAYERSCAPE_GEN4) +=
> pcie_layerscape_gen4.o \
> > pcie_layerscape_gen4_fixup.o
> > obj-$(CONFIG_PCI_XILINX) += pcie_xilinx.o
> > obj-$(CONFIG_PCIE_INTEL_FPGA) += pcie_intel_fpga.o
> > +obj-$(CONFIG_PCIE_MT2701) += pci-mt2701.o
> > diff --git a/drivers/pci/pci-mt2701.c
> b/drivers/pci/pci-mt2701.c
> > new file mode 100644
> > index 0000000000..5904f15330
> > --- /dev/null
> > +++ b/drivers/pci/pci-mt2701.c
> > @@ -0,0 +1,490 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Mediatek MT7623 SoC PCIE support
> > + *
> > + * Copyright (C) 2015 Mediatek
> > + * Copyright (C) 2015 John Crispin <blogic at openwrt.org>
> > + * Copyright (C) 2015 Ziv Huang <ziv.huang at mediatek.com>
> > + * Copyright (C) 2019 Oleksandr Rybalko <ray at ddteam.net>
> > + */
> > +
> > +#include <common.h>
> > +#include <malloc.h>
> > +
> > +#include <pci.h>
> > +#include <asm/io.h>
> > +
> > +#include <dm.h>
> > +#include <fdtdec.h>
> > +#include <linux/libfdt.h>
> > +#include <dm/lists.h>
> > +
> > +#define iowrite32(v, a) writel(v, a)
> > +#define iowrite16(v, a) writew(v, a)
> > +#define iowrite8(v, a) writeb(v, a)
> > +#define ioread32(a) readl(a)
> > +#define ioread16(a) readw(a)
> > +#define ioread8(a) readb(a)
>
> Remove these defines.
>
> > +#define RT_HIFSYS_BASE 0x1a000000
> > +#define RT_PCIE_BASE 0x1a140000
> > +#define RT_PCIE_IOWIN_BASE 0x1a160000
> > +#define RT_PCIE_IOWIN_SIZE 0x00010000
> > +#define RT_PCIE_MEMWIN_BASE 0x60000000
> > +#define RT_PCIE_MEMWIN_SIZE 0x10000000
>
> Move these base to dts.
>
>
> Already there (ranges), just not used yet.
so it's better remove unused parts, right?
>
> > +#define RD(x) readl(RT_PCIE_BASE | (x))
> > +#define WR(x, v) writel(v, RT_PCIE_BASE | (x))
> > +
> > +#define SYSCFG1 0x14
> > +#define RSTCTL 0x34
> > +#define RSTSTAT 0x38
> > +#define PCICFG 0x00
> > +#define PCIINT 0x08
> > +#define PCIENA 0x0c
> > +#define CFGADDR 0x20
> > +#define CFGDATA 0x24
> > +#define MEMBASE 0x28
> > +#define IOBASE 0x2c
> > +
> > +#define BAR0SETUP 0x10
> > +#define IMBASEBAR0 0x18
> > +#define PCIE_CLASS 0x34
> > +#define PCIE_SISTAT 0x50
> > +
> > +#define MTK_PCIE_HIGH_PERF BIT(14)
> > +#define PCIEP0_BASE 0x2000
> > +#define PCIEP1_BASE 0x3000
> > +#define PCIEP2_BASE 0x4000
> > +
> > +#define PHY_P0_CTL 0x9000
> > +#define PHY_P1_CTL 0xa000
> > +#define PHY_P2_CTL 0x4000 /* in USB space */
> > +
> > +#define RSTCTL_PCIE0_RST BIT(24)
> > +#define RSTCTL_PCIE1_RST BIT(25)
> > +#define RSTCTL_PCIE2_RST BIT(26)
> > +#define MAX_PORT_NUM 3
> > +
> > +struct resource {
> > + char *name;
> > + u32 start;
> > + u32 end;
> > +};
> > +
> > +struct mt_pcie {
> > + char name[16];
> > +};
> > +
> > +static struct mtk_pcie_port {
> > + int id;
> > + int enable;
> > + u32 base;
> > + u32 phy_base;
> > + u32 perst_n;
> > + u32 reset;
> > + u32 interrupt_en;
> > + int irq;
> > + u32 link;
> > +} mtk_pcie_port[] = {
> > + { 0, 1, PCIEP0_BASE, PHY_P0_CTL, BIT(1),
> RSTCTL_PCIE0_RST, BIT(20) },
> > + { 1, 1, PCIEP1_BASE, PHY_P1_CTL, BIT(2),
> RSTCTL_PCIE1_RST, BIT(21) },
> > + { 2, 0, PCIEP2_BASE, PHY_P2_CTL, BIT(3),
> RSTCTL_PCIE2_RST, BIT(22) },
> > +};
>
> move some of mtk_pcie_port[] to dts.
>
>
> Main point, at the end use orginal linux kernel DTS files. So it can
> be passed into kernel, instead of hardcode it inside kernel.
>
> Just like we have ACPI blob on PC.
>
>From what I see "[PATCH 4/4] arm: dts: Add PCI-E controller for mt7623"
has enough information about "base/offset/port/phy" (that dts is same as
linux one), but this driver does not use them.
>
> > +struct mtk_pcie {
> > + struct device *dev;
> > + void __iomem *sys_base; /* HIF SYSCTL
> registers */
> > + void __iomem *pcie_base; /* PCIE registers */
> > + void __iomem *usb_base; /* USB registers */
> > +
> > + struct resource io;
> > + struct resource pio;
> > + struct resource mem;
> > + struct resource prefetch;
> > + struct resource busn;
> > +
> > + u32 io_bus_addr;
> > + u32 mem_bus_addr;
> > +
> > + struct clk *clk;
> > + int pcie_card_link;
> > +};
> > +
> > +static const struct mtk_phy_init {
> > + u32 reg;
> > + u32 mask;
> > + u32 val;
> > +} mtk_phy_init[] = {
> > + { 0xc00, 0x33000, 0x22000 },
> > + { 0xb04, 0xe0000000, 0x40000000 },
> > + { 0xb00, 0xe, 0x4 },
> > + { 0xc3C, 0xffff0000, 0x3c0000 },
> > + { 0xc48, 0xffff, 0x36 },
> > + { 0xc0c, 0x30000000, 0x10000000 },
> > + { 0xc08, 0x3800c0, 0xc0 },
> > + { 0xc10, 0xf0000, 0x20000 },
> > + { 0xc0c, 0xf000, 0x1000 },
> > + { 0xc14, 0xf0000, 0xa0000 },
> > + { 0xa28, 0x3fe00, 0x2000 },
> > + { 0xa2c, 0x1ff, 0x10 },
> > +};
>
> We should add another PHY driver (drivers/phy/) for these
> settings.
> Like what we did for Linux:
> https://github.com/torvalds/linux/blob/master/drivers/phy/mediatek/phy-mtk-tphy.c
>
>
>
> Yes, but it more complex way for future releases.
>
phy driver is used for configuring basic settings, there is no complex
logic in it. we can even copy and paste the entire block from that
driver.
.version = MTK_PHY_V1
case PHY_TYPE_PCIE:
pcie_phy_instance_init(tphy, instance);
> > +/*
> > + * Globals.
> > + */
> > +
> > +struct mtk_pcie pcie;
> > +struct mtk_pcie *pcie0;
> > +
> > +static int mtk_pcie_probe(void);
> > +static int mt_pcie_read_config(struct udevice *, pci_dev_t,
> uint, ulong *,
> > + enum pci_size_t);
> > +static int mt_pcie_write_config(struct udevice *,
> pci_dev_t, uint, ulong,
> > + enum pci_size_t);
> > +
> > +#define mtk_foreach_port(p) \
> > + for ((p) = mtk_pcie_port; \
> > + (p) !=
> &mtk_pcie_port[ARRAY_SIZE(mtk_pcie_port)]; (p)++)
>
> Use dts for each port.
>
> > +#define BITS(m, n) (~(BIT(m) - 1) & ((BIT(n) - 1) |
> BIT(n)))
> > +
> > +static void
> > +mt7623_pcie_pinmux_set(void)
> > +{
> > + u32 regValue;
> > +
> > + /* Pin208: PCIE0_PERST_N (3) */
> > + regValue = le32_to_cpu(ioread32(0x100059f0));
> > + regValue &= ~(BITS(9, 11));
> > + regValue |= 3 << 9;
> > + iowrite32(regValue, 0x100059f0);
> > +
> > + /* Pin208: PCIE1_PERST_N (3) */
> > + regValue = le32_to_cpu(ioread32(0x100059f0));
> > + regValue &= ~(BITS(12, 14));
> > + regValue |= 3 << 12;
> > + iowrite32(regValue, 0x100059f0);
> > +}
>
> Configure pinmux in dts.
> Will be, when we reach point of full tree.
now we have mt7623 pinctrl driver that should be enough.
> > +static void
> > +sys_w32(struct mtk_pcie *pcie, u32 val, unsigned int reg)
> > +{
> > + iowrite32(val, pcie->sys_base + reg);
> > +}
> > +
> > +static u32
> > +sys_r32(struct mtk_pcie *pcie, unsigned int reg)
> > +{
> > + return ioread32(pcie->sys_base + reg);
> > +}
> > +
> > +static void
> > +pcie_w32(struct mtk_pcie *pcie, u32 val, unsigned int reg)
> > +{
> > + iowrite32(val, pcie->pcie_base + reg);
> > +}
> > +
> > +static void
> > +pcie_w16(struct mtk_pcie *pcie, u16 val, unsigned int reg)
> > +{
> > + iowrite16(val, pcie->pcie_base + reg);
> > +}
> > +
> > +static void
> > +pcie_w8(struct mtk_pcie *pcie, u8 val, unsigned int reg)
> > +{
> > + iowrite8(val, pcie->pcie_base + reg);
> > +}
> > +
> > +static u32
> > +pcie_r32(struct mtk_pcie *pcie, unsigned int reg)
> > +{
> > + return ioread32(pcie->pcie_base + reg);
> > +}
> > +
> > +static u32
> > +pcie_r16(struct mtk_pcie *pcie, unsigned int reg)
> > +{
> > + return ioread16(pcie->pcie_base + reg);
> > +}
> > +
> > +static u32
> > +pcie_r8(struct mtk_pcie *pcie, unsigned int reg)
> > +{
> > + return ioread8(pcie->pcie_base + reg);
> > +}
> > +
> > +static void
> > +pcie_m32(struct mtk_pcie *pcie, u32 mask, u32 val, unsigned
> int reg)
> > +{
> > + u32 v;
> > +
> > + v = pcie_r32(pcie, reg);
> > + v &= mask;
> > + v |= val;
> > + pcie_w32(pcie, v, reg);
> > +}
> > +
> > +static void
> > +mtk_pcie_configure_phy(struct mtk_pcie_port *port)
> > +{
> > + int i;
> > +
> > + for (i = 0; i < ARRAY_SIZE(mtk_phy_init); i++) {
> > + void __iomem *phy_addr = (void __iomem
> *)port->phy_base +
> > + mtk_phy_init[i].reg;
> > + u32 val = ioread32(phy_addr);
> > +
> > + val &= ~mtk_phy_init[i].mask;
> > + val |= mtk_phy_init[i].val;
> > + iowrite32(val, phy_addr);
> > + udelay(100);
> > + }
> > + mdelay(16);
> > +}
>
> Use PHY driver.
>
> > +static void
> > +mtk_pcie_configure_rc(struct mtk_pcie *pcie, struct
> mtk_pcie_port *port)
> > +{
> > + ulong val = 0;
> > +
> > + mt_pcie_write_config(NULL, (port->id) << 3,
> PCI_BASE_ADDRESS_0,
> > + 0x80000000, PCI_SIZE_32);
> > + mt_pcie_read_config(NULL, (port->id) << 3,
> PCI_BASE_ADDRESS_0, &val,
> > + PCI_SIZE_32);
> > +
> > + /* Configre RC Credit */
> > + val = 0;
> > + mt_pcie_read_config(NULL, (port->id) << 3, 0x73c,
> &val, PCI_SIZE_32);
> > + val &= ~(0x9fffUL) << 16;
> > + val |= 0x806c << 16;
> > + mt_pcie_write_config(NULL, (port->id) << 3, 0x73c,
> val, PCI_SIZE_32);
> > + mt_pcie_read_config(NULL, (port->id) << 3, 0x73c,
> &val, PCI_SIZE_32);
> > +
> > + /* Configre RC FTS number */
> > + mt_pcie_read_config(NULL, (port->id) << 3, 0x70c,
> &val, PCI_SIZE_32);
> > + val &= ~(0xff3) << 8;
> > + val |= 0x50 << 8;
> > + mt_pcie_write_config(NULL, (port->id) << 3, 0x70c,
> val, PCI_SIZE_32);
> > + mt_pcie_read_config(NULL, (port->id) << 3, 0x70c,
> &val, PCI_SIZE_32);
> > +}
> > +
> > +static void
> > +mtk_pcie_preinit(struct mtk_pcie *pcie)
> > +{
> > + struct mtk_pcie_port *port;
> > + u32 val = 0;
> > + int i;
> > +
> > + mt7623_pcie_pinmux_set();
> > +
> > + /* PCIe RC Reset */
> > + val = 0;
> > + mtk_foreach_port(port)
> > + if (port->enable)
> > + val |= port->reset;
> > + sys_w32(pcie, sys_r32(pcie, RSTCTL) | val, RSTCTL);
> > + mdelay(12);
> > + sys_w32(pcie, sys_r32(pcie, RSTCTL) & ~val, RSTCTL);
> > + mdelay(12);
> > +
> > + i = 100000;
> > + while (i--) {
> > + if (sys_r32(pcie, RSTSTAT) == 0)
> > + break;
> > + udelay(10);
> > + }
>
> We should avoid this while loop.
>
> > + /* Configure PCIe PHY */
> > +
> > + mtk_foreach_port(port)
> > + if (port->enable)
> > + mtk_pcie_configure_phy(port);
> > +
> > + /* PCIe EP reset */
> > + val = 0;
> > + mtk_foreach_port(port)
> > + if (port->enable)
> > + val |= port->perst_n;
> > + pcie_w32(pcie, pcie_r32(pcie, PCICFG) | val, PCICFG);
> > + mdelay(12);
> > + pcie_w32(pcie, pcie_r32(pcie, PCICFG) & ~val, PCICFG);
> > + mdelay(200);
> > +
> > + /* check the link status */
> > + val = 0;
> > + mtk_foreach_port(port) {
> > + if (port->enable) {
> > + if ((pcie_r32(pcie, port->base +
> PCIE_SISTAT) & 0x1))
> > + port->link = 1;
> > + else
> > + val |= port->reset;
> > + }
> > + }
> > + sys_w32(pcie, sys_r32(pcie, RSTCTL) | val, RSTCTL);
> > + mdelay(200);
> > +
> > + i = 100000;
> > + while (i--) {
> > + if (sys_r32(pcie, RSTSTAT) == 0)
> > + break;
> > + udelay(10);
> > + }
> > +
> > + mtk_foreach_port(port) {
> > + if (port->link)
> > + pcie->pcie_card_link++;
> > + }
> > + printf("%s: PCIe Link count=%d\n", __func__,
> pcie->pcie_card_link);
> > + if (!pcie->pcie_card_link)
> > + return;
> > +
> > + pcie_w32(pcie, pcie->mem_bus_addr, MEMBASE);
> > + pcie_w32(pcie, pcie->io_bus_addr, IOBASE);
> > +
> > + mtk_foreach_port(port) {
> > + if (port->link) {
> > + pcie_m32(pcie, 0xffffffff,
> port->interrupt_en, PCIENA);
> > + pcie_w32(pcie, 0x7fff0001, port->base
> + BAR0SETUP);
> > + pcie_w32(pcie, 0x80000000, port->base
> + IMBASEBAR0);
> > + pcie_w32(pcie, 0x06040001, port->base
> + PCIE_CLASS);
> magic number.
> Descriptive by reg names.
we can directly replace these chunks with mtk_pcie_startup_port()
(pcie-mediatek.c) and there are many defines for the registers/bit
field.
>
>
>
>
More information about the U-Boot
mailing list