* [PATCH v3 2/2] PCI: Rockchip: Add Rockchip PCIe controller support
@ 2016-06-16 1:50 Shawn Lin
2016-06-16 13:08 ` kbuild test robot
2016-06-23 0:29 ` Brian Norris
0 siblings, 2 replies; 6+ messages in thread
From: Shawn Lin @ 2016-06-16 1:50 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Marc Zyngier, linux-pci, Arnd Bergmann, linux-kernel,
linux-rockchip, Heiko Stuebner, Doug Anderson, Wenrui Li,
Rob Herring, devicetree, Shawn Lin
This patch adds Rockchip PCIe controller support found
on RK3399 Soc platform.
Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---
Changes in v3:
- remove header file
- remove struct msi_controller and move most of variables
of rockchip_pcie_port to become the local ones.
- allow legacy int even if enabling MSI
- drop regulator set voltage operation suggested by Doug
Changes in v2:
- remove phy related stuff and call phy API
- add new head file and define lots of macro to make
the code more readable
- remove lots msi related code suggested by Marc
- add IO window address translation
- init_port and parse_dt reconstruction suggested by Bharat
- improve wr_own_conf suggested by Arnd
- make pcie as an interrupt controller and fix wrong int handler
suggested by Marc
- remove PCI_PROBE_ONLY suggested by Lorenzo
drivers/pci/host/Kconfig | 11 +
drivers/pci/host/Makefile | 1 +
drivers/pci/host/pcie-rockchip.c | 1183 ++++++++++++++++++++++++++++++++++++++
3 files changed, 1195 insertions(+)
create mode 100644 drivers/pci/host/pcie-rockchip.c
diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
index 5d2374e..251f4ac 100644
--- a/drivers/pci/host/Kconfig
+++ b/drivers/pci/host/Kconfig
@@ -245,4 +245,15 @@ config PCIE_ARMADA_8K
Designware hardware and therefore the driver re-uses the
Designware core functions to implement the driver.
+config PCIE_ROCKCHIP
+ bool "Rockchip PCIe controller"
+ depends on ARM64 && ARCH_ROCKCHIP
+ depends on OF
+ select MFD_SYSCON
+ select PCI_MSI_IRQ_DOMAIN if PCI_MSI
+ help
+ Say Y here if you want internal PCI support on Rockchip SoC.
+ There are 1 internal PCIe port available to support GEN2 with
+ 4 slots.
+
endmenu
diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
index 9c8698e..c52985c 100644
--- a/drivers/pci/host/Makefile
+++ b/drivers/pci/host/Makefile
@@ -29,3 +29,4 @@ obj-$(CONFIG_PCIE_QCOM) += pcie-qcom.o
obj-$(CONFIG_PCI_HOST_THUNDER_ECAM) += pci-thunder-ecam.o
obj-$(CONFIG_PCI_HOST_THUNDER_PEM) += pci-thunder-pem.o
obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o
+obj-$(CONFIG_PCIE_ROCKCHIP) += pcie-rockchip.o
diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
new file mode 100644
index 0000000..661d6e0
--- /dev/null
+++ b/drivers/pci/host/pcie-rockchip.c
@@ -0,0 +1,1183 @@
+/*
+ * Rockchip AXI PCIe host controller driver
+ *
+ * Copyright (c) 2016 Rockchip, Inc.
+ *
+ * Author: Shawn Lin <shawn.lin@rock-chips.com>
+ * Wenrui Li <wenrui.li@rock-chips.com>
+ *
+ * Bits taken from Synopsys Designware Host controller driver and
+ * ARM PCI Host generic driver.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ */
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/irqchip/chained_irq.h>
+#include <linux/irqdomain.h>
+#include <linux/kernel.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/of_pci.h>
+#include <linux/of_platform.h>
+#include <linux/of_irq.h>
+#include <linux/pci.h>
+#include <linux/pci_ids.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/reset.h>
+#include <linux/regmap.h>
+
+#define PCIE_CLIENT_BASE 0x0
+#define PCIE_RC_CONFIG_NORMAL_BASE 0x800000
+#define PCIE_RC_CONFIG_BASE 0xa00000
+#define PCIE_CORE_LINK_CTRL_STATUS 0x8000d0
+#define PCIE_CORE_CTRL_MGMT_BASE 0x900000
+#define PCIE_CORE_AXI_CONF_BASE 0xc00000
+#define PCIE_CORE_AXI_INBOUND_BASE 0xc00800
+#define PCIE_CLIENT_BASIC_STATUS0 0x44
+#define PCIE_CLIENT_BASIC_STATUS1 0x48
+#define PCIE_CLIENT_INT_MASK 0x4c
+#define PCIE_CLIENT_INT_STATUS 0x50
+#define PCIE_RC_CONFIG_RID_CCR 0x8
+#define PCIR_RC_CONFIG_LCS 0xd0
+#define PCIE_RC_BAR_CONF 0x300
+#define PCIE_CORE_OB_REGION_ADDR1 0x4
+#define PCIE_CORE_OB_REGION_DESC0 0x8
+#define PCIE_CORE_OB_REGION_DESC1 0xc
+#define PCIE_RP_IB_ADDR_TRANS 0x4
+#define PCIE_CORE_INT_MASK 0x900210
+#define PCIE_CORE_INT_STATUS 0x90020c
+
+/* Size of one AXI Region (not Region 0) */
+#define AXI_REGION_SIZE BIT(20)
+/* Size of Region 0, equal to sum of sizes of other regions */
+#define AXI_REGION_0_SIZE (32 * (0x1 << 20))
+#define OB_REG_SIZE_SHIFT 5
+#define IB_ROOT_PORT_REG_SIZE_SHIFT 3
+#define AXI_WRAPPER_IO_WRITE 0x6
+#define AXI_WRAPPER_MEM_WRITE 0x2
+#define MAX_AXI_IB_ROOTPORT_REGION_NUM 3
+#define MIN_AXI_ADDR_BITS_PASSED 8
+#define ROCKCHIP_VENDOR_ID 0x1d87
+#define PCIE_ECAM_BUS(x) (((x) & 0xFF) << 20)
+#define PCIE_ECAM_DEV(x) (((x) & 0x1F) << 15)
+#define PCIE_ECAM_FUNC(x) (((x) & 0x7) << 12)
+#define PCIE_ECAM_REG(x) (((x) & 0xFFF) << 0)
+#define PCIE_ECAM_ADDR(bus, dev, func, reg) \
+ (PCIE_ECAM_BUS(bus) | PCIE_ECAM_DEV(dev) | \
+ PCIE_ECAM_FUNC(func) | PCIE_ECAM_REG(reg))
+
+/*
+ * The higher 16-bit of this register is used for write protection
+ * only if BIT(x + 16) set to 1 the BIT(x) can be written.
+ */
+#define HIWORD_UPDATE(val, mask, shift) \
+ ((val) << (shift) | (mask) << ((shift) + 16))
+
+#define RC_REGION_0_ADDR_TRANS_H 0x00000000
+#define RC_REGION_0_ADDR_TRANS_L 0x00000000
+#define RC_REGION_0_PASS_BITS (25 - 1)
+#define RC_REGION_1_ADDR_TRANS_H 0x00000000
+#define RC_REGION_1_ADDR_TRANS_L 0x00400000
+#define RC_REGION_1_PASS_BITS (20 - 1)
+#define MAX_AXI_WRAPPER_REGION_NUM 33
+#define PCIE_CORE_LCSR_RETAIN_LINK BIT(5)
+#define PCIE_CLIENT_CONF_ENABLE 1
+#define PCIE_CLIENT_LINK_TRAIN_ENABLE 1
+#define PCIE_CLIENT_ARI_ENABLE 1
+#define PCIE_CLIENT_CONF_LANE_NUM(x) (x / 2)
+#define PCIE_CLIENT_MODE_RC 1
+#define PCIE_CLIENT_GEN_SEL_2 1
+#define PCIE_CLIENT_GEN_SEL_1 0
+#define PCIE_CLIENT_CONF_ENABLE_SHIFT 0
+#define PCIE_CLIENT_CONF_ENABLE_MASK 0x1
+#define PCIE_CLIENT_LINK_TRAIN_SHIFT 1
+#define PCIE_CLIENT_LINK_TRAIN_MASK 0x1
+#define PCIE_CLIENT_ARI_ENABLE_SHIFT 3
+#define PCIE_CLIENT_ARI_ENABLE_MASK 0x1
+#define PCIE_CLIENT_CONF_LANE_NUM_SHIFT 4
+#define PCIE_CLIENT_CONF_LANE_NUM_MASK 0x3
+#define PCIE_CLIENT_MODE_SHIFT 6
+#define PCIE_CLIENT_MODE_MASK 0x1
+#define PCIE_CLIENT_GEN_SEL_SHIFT 7
+#define PCIE_CLIENT_GEN_SEL_MASK 0x1
+#define PCIE_CLIENT_LINK_STATUS_UP 0x3
+#define PCIE_CLIENT_LINK_STATUS_SHIFT 20
+#define PCIE_CLIENT_LINK_STATUS_MASK 0x3
+#define PCIE_CORE_PL_CONF_SPEED_25G 0x0
+#define PCIE_CORE_PL_CONF_SPEED_50G 0x1
+#define PCIE_CORE_PL_CONF_SPEED_80G 0x2
+#define PCIE_CORE_PL_CONF_SPEED_SHIFT 3
+#define PCIE_CORE_PL_CONF_SPEED_MASK 0x3
+#define PCIE_CORE_PL_CONF_LANE_SHIFT 1
+#define PCIE_CORE_PL_CONF_LANE_MASK 0x3
+#define PCIE_CORE_RC_CONF_SCC_SHIFT 16
+
+/* PCIE_CLIENT_INT_STATUS */
+#define PCIE_CLIENT_INT_LEGACY_DONE BIT(15)
+#define PCIE_CLIENT_INT_MSG BIT(14)
+#define PCIE_CLIENT_INT_HOT_RST BIT(13)
+#define PCIE_CLIENT_INT_DPA BIT(12)
+#define PCIE_CLIENT_INT_FATAL_ERR BIT(11)
+#define PCIE_CLIENT_INT_NFATAL_ERR BIT(10)
+#define PCIE_CLIENT_INT_CORR_ERR BIT(9)
+#define PCIE_CLIENT_INT_INTD BIT(8)
+#define PCIE_CLIENT_INT_INTC BIT(7)
+#define PCIE_CLIENT_INT_INTB BIT(6)
+#define PCIE_CLIENT_INT_INTA BIT(5)
+#define PCIE_CLIENT_INT_LOCAL BIT(4)
+#define PCIE_CLIENT_INT_UDMA BIT(3)
+#define PCIE_CLIENT_INT_PHY BIT(2)
+#define PCIE_CLIENT_INT_HOT_PLUG BIT(1)
+#define PCIE_CLIENT_INT_PWR_STCG BIT(0)
+#define PCIE_CORE_INT_PRFPE BIT(0)
+#define PCIE_CORE_INT_CRFPE BIT(1)
+#define PCIE_CORE_INT_RRPE BIT(2)
+#define PCIE_CORE_INT_PRFO BIT(3)
+#define PCIE_CORE_INT_CRFO BIT(4)
+#define PCIE_CORE_INT_RT BIT(5)
+#define PCIE_CORE_INT_RTR BIT(6)
+#define PCIE_CORE_INT_PE BIT(7)
+#define PCIE_CORE_INT_MTR BIT(8)
+#define PCIE_CORE_INT_UCR BIT(9)
+#define PCIE_CORE_INT_FCE BIT(10)
+#define PCIE_CORE_INT_CT BIT(11)
+#define PCIE_CORE_INT_UTC BIT(18)
+#define PCIE_CORE_INT_MMVC BIT(19)
+
+#define ROCKCHIP_PCIE_RPIFR1_INTR_MASK GENMASK(8, 5)
+#define ROCKCHIP_PCIE_RPIFR1_INTR_SHIFT 5
+
+#define PCIE_CORE_INT \
+ (PCIE_CORE_INT_PRFPE | PCIE_CORE_INT_CRFPE | \
+ PCIE_CORE_INT_RRPE | PCIE_CORE_INT_CRFO | \
+ PCIE_CORE_INT_RT | PCIE_CORE_INT_RTR | \
+ PCIE_CORE_INT_PE | PCIE_CORE_INT_MTR | \
+ PCIE_CORE_INT_UCR | PCIE_CORE_INT_FCE | \
+ PCIE_CORE_INT_CT | PCIE_CORE_INT_UTC | \
+ PCIE_CORE_INT_MMVC)
+
+#define PCIE_CLIENT_INT_SUBSYSTEM \
+ (PCIE_CLIENT_INT_PWR_STCG | PCIE_CLIENT_INT_HOT_PLUG | \
+ PCIE_CLIENT_INT_PHY | PCIE_CLIENT_INT_UDMA | \
+ PCIE_CLIENT_INT_LOCAL)
+
+#define PCIE_CLIENT_INT_LEGACY \
+ (PCIE_CLIENT_INT_INTA | PCIE_CLIENT_INT_INTB | \
+ PCIE_CLIENT_INT_INTC | PCIE_CLIENT_INT_INTD)
+
+#define PCIE_CLIENT_INT_CLI \
+ (PCIE_CLIENT_INT_CORR_ERR | PCIE_CLIENT_INT_NFATAL_ERR | \
+ PCIE_CLIENT_INT_FATAL_ERR | PCIE_CLIENT_INT_DPA | \
+ PCIE_CLIENT_INT_HOT_RST | PCIE_CLIENT_INT_MSG | \
+ PCIE_CLIENT_INT_LEGACY_DONE | PCIE_CLIENT_INT_LEGACY)
+
+struct rockchip_pcie_port {
+ void __iomem *reg_base;
+ void __iomem *apb_base;
+ struct phy *phy;
+ struct reset_control *core_rst;
+ struct reset_control *mgmt_rst;
+ struct reset_control *mgmt_sticky_rst;
+ struct reset_control *pipe_rst;
+ struct clk *aclk_pcie;
+ struct clk *aclk_perf_pcie;
+ struct clk *hclk_pcie;
+ struct clk *clk_pcie_pm;
+ struct regulator *vpcie3v3; /* 3.3V power supply */
+ struct regulator *vpcie1v8; /* 1.8V power supply */
+ struct regulator *vpcie0v9; /* 0.9V power supply */
+ struct gpio_desc *ep_gpio;
+ u32 lanes;
+ u8 root_bus_nr;
+ struct device *dev;
+ struct irq_domain *irq_domain;
+};
+
+static irqreturn_t rockchip_pcie_subsys_irq_handler(int irq, void *arg);
+static irqreturn_t rockchip_pcie_client_irq_handler(int irq, void *arg);
+static void rockchip_pcie_legacy_int_handler(struct irq_desc *desc);
+
+static inline u32 pcie_read(struct rockchip_pcie_port *port, u32 reg)
+{
+ return readl(port->apb_base + reg);
+}
+
+static inline void pcie_write(struct rockchip_pcie_port *port,
+ u32 val, u32 reg)
+{
+ writel(val, port->apb_base + reg);
+}
+
+static int rockchip_pcie_valid_config(struct rockchip_pcie_port *pp,
+ struct pci_bus *bus, int dev)
+{
+ /* access only one slot on each root port */
+ if (bus->number == pp->root_bus_nr && dev > 0)
+ return 0;
+
+ /*
+ * do not read more than one device on the bus directly attached
+ * to RC's (Virtual Bridge's) DS side.
+ */
+ if (bus->primary == pp->root_bus_nr && dev > 0)
+ return 0;
+
+ return 1;
+}
+
+static int rockchip_pcie_rd_own_conf(struct rockchip_pcie_port *pp,
+ int where, int size,
+ u32 *val)
+{
+ void __iomem *addr = pp->apb_base + PCIE_RC_CONFIG_BASE + where;
+
+ if (!IS_ALIGNED((uintptr_t)addr, size)) {
+ *val = 0;
+ return PCIBIOS_BAD_REGISTER_NUMBER;
+ }
+
+ if (size == 4) {
+ *val = readl(addr);
+ } else if (size == 2) {
+ *val = readw(addr);
+ } else if (size == 1) {
+ *val = readb(addr);
+ } else {
+ *val = 0;
+ return PCIBIOS_BAD_REGISTER_NUMBER;
+ }
+ return PCIBIOS_SUCCESSFUL;
+}
+
+static int rockchip_pcie_wr_own_conf(struct rockchip_pcie_port *pp,
+ int where, int size, u32 val)
+{
+ u32 mask, tmp, offset;
+
+ offset = (where & (~0x3));
+
+ if (size == 4) {
+ writel(val, pp->apb_base + PCIE_RC_CONFIG_BASE + offset);
+ return PCIBIOS_SUCCESSFUL;
+ }
+
+ mask = ~(((1 << (size * 8)) - 1) << ((where & 0x3) * 8));
+
+ tmp = readl(pp->apb_base + PCIE_RC_CONFIG_BASE + offset) & mask;
+ tmp |= val << ((where & 0x3) * 8);
+ writel(tmp, pp->apb_base + PCIE_RC_CONFIG_BASE + offset);
+
+ return PCIBIOS_SUCCESSFUL;
+}
+
+static int rockchip_pcie_rd_other_conf(struct rockchip_pcie_port *pp,
+ struct pci_bus *bus, u32 devfn,
+ int where, int size, u32 *val)
+{
+ u32 busdev;
+
+ busdev = PCIE_ECAM_ADDR(bus->number, PCI_SLOT(devfn),
+ PCI_FUNC(devfn), where);
+
+ if (!IS_ALIGNED(busdev, size)) {
+ *val = 0;
+ return PCIBIOS_BAD_REGISTER_NUMBER;
+ }
+
+ if (size == 4) {
+ *val = readl(pp->reg_base + busdev);
+ } else if (size == 2) {
+ *val = readw(pp->reg_base + busdev);
+ } else if (size == 1) {
+ *val = readb(pp->reg_base + busdev);
+ } else {
+ *val = 0;
+ return PCIBIOS_BAD_REGISTER_NUMBER;
+ }
+ return PCIBIOS_SUCCESSFUL;
+}
+
+static int rockchip_pcie_wr_other_conf(struct rockchip_pcie_port *pp,
+ struct pci_bus *bus, u32 devfn,
+ int where, int size, u32 val)
+{
+ u32 busdev;
+
+ busdev = PCIE_ECAM_ADDR(bus->number, PCI_SLOT(devfn),
+ PCI_FUNC(devfn), where);
+ if (!IS_ALIGNED(busdev, size))
+ return PCIBIOS_BAD_REGISTER_NUMBER;
+
+ if (size == 4)
+ writel(val, pp->reg_base + busdev);
+ else if (size == 2)
+ writew(val, pp->reg_base + busdev);
+ else if (size == 1)
+ writeb(val, pp->reg_base + busdev);
+ else
+ return PCIBIOS_BAD_REGISTER_NUMBER;
+
+ return PCIBIOS_SUCCESSFUL;
+}
+
+static int rockchip_pcie_rd_conf(struct pci_bus *bus, u32 devfn, int where,
+ int size, u32 *val)
+{
+ struct rockchip_pcie_port *pp = bus->sysdata;
+ int ret;
+
+ if (rockchip_pcie_valid_config(pp, bus, PCI_SLOT(devfn)) == 0) {
+ *val = 0xffffffff;
+ return PCIBIOS_DEVICE_NOT_FOUND;
+ }
+
+ if (bus->number != pp->root_bus_nr)
+ ret = rockchip_pcie_rd_other_conf(pp, bus, devfn,
+ where, size, val);
+ else
+ ret = rockchip_pcie_rd_own_conf(pp, where, size, val);
+
+ return ret;
+}
+
+static int rockchip_pcie_wr_conf(struct pci_bus *bus, u32 devfn,
+ int where, int size, u32 val)
+{
+ struct rockchip_pcie_port *pp = bus->sysdata;
+ int ret;
+
+ if (rockchip_pcie_valid_config(pp, bus, PCI_SLOT(devfn)) == 0)
+ return PCIBIOS_DEVICE_NOT_FOUND;
+
+ if (bus->number != pp->root_bus_nr)
+ ret = rockchip_pcie_wr_other_conf(pp, bus, devfn,
+ where, size, val);
+ else
+ ret = rockchip_pcie_wr_own_conf(pp, where, size, val);
+
+ return ret;
+}
+
+static struct pci_ops rockchip_pcie_ops = {
+ .read = rockchip_pcie_rd_conf,
+ .write = rockchip_pcie_wr_conf,
+};
+
+/**
+ * rockchip_pcie_init_port - Initialize hardware
+ * @port: PCIe port information
+ */
+static int rockchip_pcie_init_port(struct rockchip_pcie_port *port)
+{
+ int err;
+ u32 status;
+ unsigned long timeout;
+
+ gpiod_set_value(port->ep_gpio, 0);
+
+ err = phy_init(port->phy);
+ if (err < 0)
+ dev_err(port->dev, "fail to init phy, err %d\n", err);
+
+ err = reset_control_assert(port->core_rst);
+ if (err) {
+ dev_err(port->dev, "assert core_rst err %d\n", err);
+ return err;
+ }
+
+ err = reset_control_assert(port->mgmt_rst);
+ if (err) {
+ dev_err(port->dev, "assert mgmt_rst err %d\n", err);
+ return err;
+ }
+
+ err = reset_control_assert(port->mgmt_sticky_rst);
+ if (err) {
+ dev_err(port->dev, "assert mgmt_sticky_rst err %d\n", err);
+ return err;
+ }
+
+ err = reset_control_assert(port->pipe_rst);
+ if (err) {
+ dev_err(port->dev, "assert pipe_rst err %d\n", err);
+ return err;
+ }
+
+ pcie_write(port,
+ HIWORD_UPDATE(PCIE_CLIENT_CONF_ENABLE,
+ PCIE_CLIENT_CONF_ENABLE_MASK,
+ PCIE_CLIENT_CONF_ENABLE_SHIFT) |
+ HIWORD_UPDATE(PCIE_CLIENT_CONF_LANE_NUM(port->lanes),
+ PCIE_CLIENT_CONF_LANE_NUM_MASK,
+ PCIE_CLIENT_CONF_LANE_NUM_SHIFT) |
+ HIWORD_UPDATE(PCIE_CLIENT_MODE_RC,
+ PCIE_CLIENT_MODE_MASK,
+ PCIE_CLIENT_MODE_SHIFT) |
+ HIWORD_UPDATE(PCIE_CLIENT_ARI_ENABLE,
+ PCIE_CLIENT_ARI_ENABLE_MASK,
+ PCIE_CLIENT_ARI_ENABLE_SHIFT) |
+ HIWORD_UPDATE(PCIE_CLIENT_GEN_SEL_2,
+ PCIE_CLIENT_GEN_SEL_MASK,
+ PCIE_CLIENT_GEN_SEL_SHIFT),
+ PCIE_CLIENT_BASE);
+
+ err = phy_power_on(port->phy);
+ if (err)
+ dev_err(port->dev, "fail to power on phy, err %d\n", err);
+
+ err = reset_control_deassert(port->core_rst);
+ if (err) {
+ dev_err(port->dev, "deassert core_rst err %d\n", err);
+ return err;
+ }
+
+ err = reset_control_deassert(port->mgmt_rst);
+ if (err) {
+ dev_err(port->dev, "deassert mgmt_rst err %d\n", err);
+ return err;
+ }
+
+ err = reset_control_deassert(port->mgmt_sticky_rst);
+ if (err) {
+ dev_err(port->dev, "deassert mgmt_sticky_rst err %d\n", err);
+ return err;
+ }
+
+ err = reset_control_deassert(port->pipe_rst);
+ if (err) {
+ dev_err(port->dev, "deassert pipe_rst err %d\n", err);
+ return err;
+ }
+
+ /* Enable Gen1 trainning */
+ pcie_write(port,
+ HIWORD_UPDATE(PCIE_CLIENT_LINK_TRAIN_ENABLE,
+ PCIE_CLIENT_LINK_TRAIN_MASK,
+ PCIE_CLIENT_LINK_TRAIN_SHIFT),
+ PCIE_CLIENT_BASE);
+
+ gpiod_set_value(port->ep_gpio, 1);
+
+ /* 500ms timeout value should be enough for gen1/2 taining */
+ timeout = jiffies + msecs_to_jiffies(500);
+
+ err = -ETIMEDOUT;
+ while (time_before(jiffies, timeout)) {
+ status = pcie_read(port, PCIE_CLIENT_BASIC_STATUS1);
+ if (((status >> PCIE_CLIENT_LINK_STATUS_SHIFT) &
+ PCIE_CLIENT_LINK_STATUS_MASK) ==
+ PCIE_CLIENT_LINK_STATUS_UP) {
+ dev_dbg(port->dev, "pcie link training gen1 pass!\n");
+ err = 0;
+ break;
+ }
+ msleep(20);
+ }
+ if (err) {
+ dev_err(port->dev, "pcie link training gen1 timeout!\n");
+ return err;
+ }
+
+ /*
+ * Enable retrain for gen2. This should be configured only after
+ * gen1 finished.
+ */
+ status = pcie_read(port,
+ PCIR_RC_CONFIG_LCS + PCIE_RC_CONFIG_BASE);
+ status |= PCIE_CORE_LCSR_RETAIN_LINK;
+ pcie_write(port, status,
+ PCIR_RC_CONFIG_LCS + PCIE_RC_CONFIG_BASE);
+ err = -ETIMEDOUT;
+
+ while (time_before(jiffies, timeout)) {
+ status = pcie_read(port, PCIE_CORE_CTRL_MGMT_BASE);
+ if (((status >> PCIE_CORE_PL_CONF_SPEED_SHIFT) &
+ PCIE_CORE_PL_CONF_SPEED_MASK) ==
+ PCIE_CORE_PL_CONF_SPEED_50G) {
+ dev_dbg(port->dev, "pcie link training gen2 pass!\n");
+ err = 0;
+ break;
+ }
+ msleep(20);
+ }
+ if (err)
+ dev_dbg(port->dev, "pcie link training gen2 timeout, force to gen1!\n");
+
+ /* Check the final link with from negotiated lane counter from MGMT */
+ status = pcie_read(port, PCIE_CORE_CTRL_MGMT_BASE);
+ status = 0x1 << ((status >> PCIE_CORE_PL_CONF_LANE_SHIFT) &
+ PCIE_CORE_PL_CONF_LANE_MASK);
+ dev_dbg(port->dev, "current link width is x%d\n", status);
+
+ pcie_write(port, ROCKCHIP_VENDOR_ID, PCIE_RC_CONFIG_BASE);
+ pcie_write(port, PCI_CLASS_BRIDGE_PCI << PCIE_CORE_RC_CONF_SCC_SHIFT,
+ PCIE_RC_CONFIG_BASE + PCIE_RC_CONFIG_RID_CCR);
+ pcie_write(port, 0x0, PCIE_CORE_CTRL_MGMT_BASE + PCIE_RC_BAR_CONF);
+
+ pcie_write(port, (RC_REGION_0_ADDR_TRANS_L + RC_REGION_0_PASS_BITS),
+ PCIE_CORE_AXI_CONF_BASE);
+ pcie_write(port, RC_REGION_0_ADDR_TRANS_H,
+ PCIE_CORE_AXI_CONF_BASE + PCIE_CORE_OB_REGION_ADDR1);
+ pcie_write(port, 0x0080000a,
+ PCIE_CORE_AXI_CONF_BASE + PCIE_CORE_OB_REGION_DESC0);
+ pcie_write(port, 0x0,
+ PCIE_CORE_AXI_CONF_BASE + PCIE_CORE_OB_REGION_DESC1);
+
+ return 0;
+}
+
+/**
+ * rockchip_pcie_parse_dt - Parse Device tree
+ * @port: PCIe port information
+ *
+ * Return: '0' on success and error value on failure
+ */
+static int rockchip_pcie_parse_dt(struct rockchip_pcie_port *port)
+{
+ struct device *dev = port->dev;
+ struct platform_device *pdev = to_platform_device(dev);
+ struct device_node *node = dev->of_node;
+ struct resource *regs;
+ int irq;
+ int err = -ENODEV;
+
+ regs = platform_get_resource_byname(pdev,
+ IORESOURCE_MEM,
+ "axi-base");
+ if (!regs) {
+ dev_err(dev, "missing axi-base property\n");
+ return err;
+ }
+
+ port->reg_base = devm_ioremap_resource(dev, regs);
+ if (IS_ERR(port->reg_base))
+ return PTR_ERR(port->reg_base);
+
+ regs = platform_get_resource_byname(pdev,
+ IORESOURCE_MEM,
+ "apb-base");
+ if (!regs) {
+ dev_err(dev, "missing apb-base property\n");
+ return err;
+ }
+
+ port->apb_base = devm_ioremap_resource(dev, regs);
+ if (IS_ERR(port->apb_base))
+ return PTR_ERR(port->apb_base);
+
+ port->phy = devm_phy_get(dev, "pcie-phy");
+ if (IS_ERR_OR_NULL(port->phy)) {
+ if (PTR_ERR_OR_ZERO(port->phy) != -EPROBE_DEFER)
+ dev_err(dev, "Missing phy\n");
+ return PTR_ERR(port->phy);
+ }
+
+ port->lanes = 1;
+ err = of_property_read_u32(node, "num-lanes", &port->lanes);
+ if (!err && ((port->lanes == 0) ||
+ (port->lanes == 3) ||
+ (port->lanes > 4))) {
+ dev_warn(dev, "invalid num-lanes, default use one lane\n");
+ port->lanes = 1;
+ }
+
+ port->core_rst = devm_reset_control_get(dev, "core");
+ if (IS_ERR(port->core_rst)) {
+ if (PTR_ERR(port->core_rst) != -EPROBE_DEFER)
+ dev_err(dev, "missing core rst property in node %s\n",
+ node->name);
+ return PTR_ERR(port->core_rst);
+ }
+
+ port->mgmt_rst = devm_reset_control_get(dev, "mgmt");
+ if (IS_ERR(port->mgmt_rst)) {
+ if (PTR_ERR(port->mgmt_rst) != -EPROBE_DEFER)
+ dev_err(dev, "missing mgmt rst property in node %s\n",
+ node->name);
+ return PTR_ERR(port->mgmt_rst);
+ }
+
+ port->mgmt_sticky_rst = devm_reset_control_get(dev, "mgmt-sticky");
+ if (IS_ERR(port->mgmt_sticky_rst)) {
+ if (PTR_ERR(port->mgmt_sticky_rst) != -EPROBE_DEFER)
+ dev_err(dev, "missing mgmt-sticky rst property in node %s\n",
+ node->name);
+ return PTR_ERR(port->mgmt_sticky_rst);
+ }
+
+ port->pipe_rst = devm_reset_control_get(dev, "pipe");
+ if (IS_ERR(port->pipe_rst)) {
+ if (PTR_ERR(port->pipe_rst) != -EPROBE_DEFER)
+ dev_err(dev, "missing pipe rst property in node %s\n",
+ node->name);
+ return PTR_ERR(port->pipe_rst);
+ }
+
+ port->ep_gpio = devm_gpiod_get(dev, "ep", GPIOD_OUT_HIGH);
+ if (IS_ERR(port->ep_gpio)) {
+ dev_err(dev, "missing ep-gpios property in node %s\n",
+ node->name);
+ return PTR_ERR(port->ep_gpio);
+ }
+
+ port->aclk_pcie = devm_clk_get(dev, "aclk");
+ if (IS_ERR(port->aclk_pcie)) {
+ dev_err(dev, "aclk clock not found.\n");
+ return PTR_ERR(port->aclk_pcie);
+ }
+
+ port->aclk_perf_pcie = devm_clk_get(dev, "aclk-perf");
+ if (IS_ERR(port->aclk_perf_pcie)) {
+ dev_err(dev, "aclk_perf clock not found.\n");
+ return PTR_ERR(port->aclk_perf_pcie);
+ }
+
+ port->hclk_pcie = devm_clk_get(dev, "hclk");
+ if (IS_ERR(port->hclk_pcie)) {
+ dev_err(dev, "hclk clock not found.\n");
+ return PTR_ERR(port->hclk_pcie);
+ }
+
+ port->clk_pcie_pm = devm_clk_get(dev, "pm");
+ if (IS_ERR(port->clk_pcie_pm)) {
+ dev_err(dev, "pm clock not found.\n");
+ return PTR_ERR(port->clk_pcie_pm);
+ }
+
+ irq = platform_get_irq_byname(pdev, "sys");
+ if (irq < 0) {
+ dev_err(dev, "missing pcie_sys IRQ resource\n");
+ return -EINVAL;
+ }
+
+ err = devm_request_irq(dev, irq, rockchip_pcie_subsys_irq_handler,
+ IRQF_SHARED, "pcie-sys", port);
+ if (err) {
+ dev_err(dev, "failed to request pcie subsystem irq\n");
+ return err;
+ }
+
+ irq = platform_get_irq_byname(pdev, "legacy");
+ if (irq < 0) {
+ dev_err(dev, "missing pcie_legacy IRQ resource\n");
+ return -EINVAL;
+ }
+
+ irq_set_chained_handler_and_data(irq,
+ rockchip_pcie_legacy_int_handler,
+ port);
+
+ irq = platform_get_irq_byname(pdev, "client");
+ if (irq < 0) {
+ dev_err(dev, "missing pcie-client IRQ resource\n");
+ return -EINVAL;
+ }
+
+ err = devm_request_irq(dev, irq, rockchip_pcie_client_irq_handler,
+ IRQF_SHARED, "pcie-client", port);
+ if (err) {
+ dev_err(dev, "failed to request pcie client irq\n");
+ return err;
+ }
+
+ port->vpcie3v3 = devm_regulator_get_optional(dev, "vpcie3v3");
+ if (IS_ERR(port->vpcie3v3)) {
+ if (PTR_ERR(port->vpcie3v3) == -EPROBE_DEFER)
+ return -EPROBE_DEFER;
+ dev_info(dev, "No vpcie3v3 regulator found.\n");
+ }
+
+ port->vpcie1v8 = devm_regulator_get_optional(dev, "vpcie1v8");
+ if (IS_ERR(port->vpcie1v8)) {
+ if (PTR_ERR(port->vpcie1v8) == -EPROBE_DEFER)
+ return -EPROBE_DEFER;
+ dev_info(dev, "No vpcie1v8 regulator found.\n");
+ }
+
+ port->vpcie0v9 = devm_regulator_get_optional(dev, "vpcie0v9");
+ if (IS_ERR(port->vpcie0v9)) {
+ if (PTR_ERR(port->vpcie0v9) == -EPROBE_DEFER)
+ return -EPROBE_DEFER;
+ dev_info(dev, "No vpcie0v9 regulator found.\n");
+ }
+
+ return 0;
+}
+
+static int rockchip_pcie_set_vpcie(struct rockchip_pcie_port *port)
+{
+ int err;
+
+ if (!IS_ERR(port->vpcie3v3)) {
+ err = regulator_enable(port->vpcie3v3);
+ if (err) {
+ dev_err(port->dev, "Fail to enable vpcie3v3 regulator.\n");
+ goto err_out;
+ }
+ }
+
+ if (!IS_ERR(port->vpcie1v8)) {
+ err = regulator_enable(port->vpcie1v8);
+ if (err) {
+ dev_err(port->dev, "Fail to enable vpcie1v8 regulator.\n");
+ goto err_disable_3v3;
+ }
+ }
+
+ if (!IS_ERR(port->vpcie0v9)) {
+ err = regulator_enable(port->vpcie0v9);
+ if (err) {
+ dev_err(port->dev, "Fail to enable vpcie0v9 regulator.\n");
+ goto err_disable_1v8;
+ }
+ }
+
+ return 0;
+
+err_disable_1v8:
+ if (!IS_ERR(port->vpcie1v8))
+ regulator_disable(port->vpcie1v8);
+err_disable_3v3:
+ if (!IS_ERR(port->vpcie3v3))
+ regulator_disable(port->vpcie3v3);
+err_out:
+ return err;
+}
+
+static void rockchip_pcie_enable_interrupts(struct rockchip_pcie_port *port)
+{
+ pcie_write(port, (PCIE_CLIENT_INT_CLI << 16) &
+ (~PCIE_CLIENT_INT_CLI), PCIE_CLIENT_INT_MASK);
+ pcie_write(port, PCIE_CORE_INT, PCIE_CORE_INT_MASK);
+}
+
+static int rockchip_pcie_intx_map(struct irq_domain *domain, unsigned int irq,
+ irq_hw_number_t hwirq)
+{
+ irq_set_chip_and_handler(irq, &dummy_irq_chip, handle_simple_irq);
+ irq_set_chip_data(irq, domain->host_data);
+
+ return 0;
+}
+
+static const struct irq_domain_ops intx_domain_ops = {
+ .map = rockchip_pcie_intx_map,
+};
+
+static int rockchip_pcie_init_irq_domain(struct rockchip_pcie_port *pp)
+{
+ struct device *dev = pp->dev;
+
+ pp->irq_domain = irq_domain_add_linear(dev->of_node, 4,
+ &intx_domain_ops, pp);
+ if (!pp->irq_domain) {
+ dev_err(dev, "Failed to get a INTx IRQ domain\n");
+ return PTR_ERR(pp->irq_domain);
+ }
+
+ return 0;
+}
+
+static irqreturn_t rockchip_pcie_subsys_irq_handler(int irq, void *arg)
+{
+ struct rockchip_pcie_port *pp = arg;
+ u32 reg;
+ u32 sub_reg;
+
+ reg = pcie_read(pp, PCIE_CLIENT_INT_STATUS);
+ if (reg & PCIE_CLIENT_INT_LOCAL) {
+ dev_dbg(pp->dev, "local interrupt recived\n");
+ sub_reg = pcie_read(pp, PCIE_CORE_INT_STATUS);
+ if (sub_reg & PCIE_CORE_INT_PRFPE)
+ dev_dbg(pp->dev, "Parity error detected while reading from the PNP Receive FIFO RAM\n");
+
+ if (sub_reg & PCIE_CORE_INT_CRFPE)
+ dev_dbg(pp->dev, "Parity error detected while reading from the Completion Receive FIFO RAM\n");
+
+ if (sub_reg & PCIE_CORE_INT_RRPE)
+ dev_dbg(pp->dev, "Parity error detected while reading from Replay Buffer RAM\n");
+
+ if (sub_reg & PCIE_CORE_INT_PRFO)
+ dev_dbg(pp->dev, "Overflow occurred in the PNP Receive FIFO\n");
+
+ if (sub_reg & PCIE_CORE_INT_CRFO)
+ dev_dbg(pp->dev, "Overflow occurred in the Completion Receive FIFO\n");
+
+ if (sub_reg & PCIE_CORE_INT_RT)
+ dev_dbg(pp->dev, "Replay timer timed out\n");
+
+ if (sub_reg & PCIE_CORE_INT_RTR)
+ dev_dbg(pp->dev, "Replay timer rolled over after 4 transmissions of the same TLP\n");
+
+ if (sub_reg & PCIE_CORE_INT_PE)
+ dev_dbg(pp->dev, "Phy error detected on receive side\n");
+
+ if (sub_reg & PCIE_CORE_INT_MTR)
+ dev_dbg(pp->dev, "Malformed TLP received from the link\n");
+
+ if (sub_reg & PCIE_CORE_INT_UCR)
+ dev_dbg(pp->dev, "Malformed TLP received from the link\n");
+
+ if (sub_reg & PCIE_CORE_INT_FCE)
+ dev_dbg(pp->dev, "An error was observed in the flow control advertisements from the other side\n");
+
+ if (sub_reg & PCIE_CORE_INT_CT)
+ dev_dbg(pp->dev, "A request timed out waiting for completion\n");
+
+ if (sub_reg & PCIE_CORE_INT_UTC)
+ dev_dbg(pp->dev, "Unmapped TC error\n");
+
+ if (sub_reg & PCIE_CORE_INT_MMVC)
+ dev_dbg(pp->dev, "MSI mask register changes\n");
+
+ pcie_write(pp, sub_reg, PCIE_CORE_INT_STATUS);
+ }
+
+ pcie_write(pp, PCIE_CLIENT_INT_SUBSYSTEM | PCIE_CLIENT_INT_LOCAL,
+ PCIE_CLIENT_INT_STATUS);
+
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t rockchip_pcie_client_irq_handler(int irq, void *arg)
+{
+ struct rockchip_pcie_port *pp = arg;
+ u32 reg;
+
+ reg = pcie_read(pp, PCIE_CLIENT_INT_STATUS);
+ if (reg & PCIE_CLIENT_INT_LEGACY_DONE)
+ dev_dbg(pp->dev, "legacy done interrupt recived\n");
+
+ if (reg & PCIE_CLIENT_INT_MSG)
+ dev_dbg(pp->dev, "message done interrupt recived\n");
+
+ if (reg & PCIE_CLIENT_INT_HOT_RST)
+ dev_dbg(pp->dev, "hot reset interrupt recived\n");
+
+ if (reg & PCIE_CLIENT_INT_DPA)
+ dev_dbg(pp->dev, "dpa interrupt recived\n");
+
+ if (reg & PCIE_CLIENT_INT_FATAL_ERR)
+ dev_dbg(pp->dev, "fatal error interrupt recived\n");
+
+ if (reg & PCIE_CLIENT_INT_NFATAL_ERR)
+ dev_dbg(pp->dev, "no fatal error interrupt recived\n");
+
+ if (reg & PCIE_CLIENT_INT_CORR_ERR)
+ dev_dbg(pp->dev, "correctable error interrupt recived\n");
+
+ pcie_write(pp, reg, PCIE_CLIENT_INT_STATUS);
+
+ return IRQ_HANDLED;
+}
+
+static void rockchip_pcie_legacy_int_handler(struct irq_desc *desc)
+{
+ struct irq_chip *chip = irq_desc_get_chip(desc);
+ struct rockchip_pcie_port *port;
+ u32 reg;
+
+ chained_irq_enter(chip, desc);
+ port = irq_desc_get_handler_data(desc);
+
+ reg = pcie_read(port, PCIE_CLIENT_INT_STATUS);
+ reg = (reg & ROCKCHIP_PCIE_RPIFR1_INTR_MASK) >>
+ ROCKCHIP_PCIE_RPIFR1_INTR_SHIFT;
+ if (reg)
+ generic_handle_irq(irq_find_mapping(port->irq_domain,
+ ffs(reg)));
+ chained_irq_exit(chip, desc);
+}
+
+static int rockchip_pcie_prog_ob_atu(struct rockchip_pcie_port *pp,
+ int region_no,
+ int type, u8 num_pass_bits,
+ u32 lower_addr, u32 upper_addr)
+{
+ u32 ob_addr_0 = 0;
+ u32 ob_addr_1 = 0;
+ u32 ob_desc_0 = 0;
+ u32 ob_desc_1 = 0;
+ void __iomem *aw_base;
+
+ if (!pp)
+ return -EINVAL;
+ if (region_no >= MAX_AXI_WRAPPER_REGION_NUM)
+ return -EINVAL;
+ if ((num_pass_bits + 1) < 8)
+ return -EINVAL;
+ if (num_pass_bits > 63)
+ return -EINVAL;
+ if (region_no == 0) {
+ if (AXI_REGION_0_SIZE < (2ULL << num_pass_bits))
+ return -EINVAL;
+ }
+ if (region_no != 0) {
+ if (AXI_REGION_SIZE < (2ULL << num_pass_bits))
+ return -EINVAL;
+ }
+ aw_base = pp->apb_base + PCIE_CORE_AXI_CONF_BASE;
+ aw_base += (region_no << OB_REG_SIZE_SHIFT);
+
+ ob_addr_0 = (ob_addr_0 &
+ ~0x0000003fU) | (num_pass_bits &
+ 0x0000003fU);
+ ob_addr_0 = (ob_addr_0 &
+ ~0xffffff00U) | (lower_addr & 0xffffff00U);
+ ob_addr_1 = upper_addr;
+ ob_desc_0 = (1 << 23 | type);
+
+ writel(ob_addr_0, aw_base);
+ writel(ob_addr_1, aw_base + PCIE_CORE_OB_REGION_ADDR1);
+ writel(ob_desc_0, aw_base + PCIE_CORE_OB_REGION_DESC0);
+ writel(ob_desc_1, aw_base + PCIE_CORE_OB_REGION_DESC1);
+
+ return 0;
+}
+
+static int rockchip_pcie_prog_ib_atu(struct rockchip_pcie_port *pp,
+ int region_no,
+ u8 num_pass_bits,
+ u32 lower_addr,
+ u32 upper_addr)
+{
+ u32 ib_addr_0 = 0;
+ u32 ib_addr_1 = 0;
+ void __iomem *aw_base;
+
+ if (!pp)
+ return -EINVAL;
+ if (region_no > MAX_AXI_IB_ROOTPORT_REGION_NUM)
+ return -EINVAL;
+ if ((num_pass_bits + 1) < MIN_AXI_ADDR_BITS_PASSED)
+ return -EINVAL;
+ if (num_pass_bits > 63)
+ return -EINVAL;
+ aw_base = pp->apb_base + PCIE_CORE_AXI_INBOUND_BASE;
+ aw_base += (region_no << IB_ROOT_PORT_REG_SIZE_SHIFT);
+ ib_addr_0 = (ib_addr_0 &
+ ~0x0000003fU) | (num_pass_bits &
+ 0x0000003fU);
+
+ ib_addr_0 = (ib_addr_0 & ~0xffffff00U) |
+ ((lower_addr << 8) & 0xffffff00U);
+ ib_addr_1 = upper_addr;
+
+ writel(ib_addr_0, aw_base);
+ writel(ib_addr_1, aw_base + PCIE_RP_IB_ADDR_TRANS);
+
+ return 0;
+}
+
+static int rockchip_pcie_probe(struct platform_device *pdev)
+{
+ struct rockchip_pcie_port *port;
+ struct device *dev = &pdev->dev;
+ struct pci_bus *bus, *child;
+ struct resource_entry *win;
+ resource_size_t io_base;
+ struct resource *busn;
+ struct resource *mem;
+ struct resource *io;
+ phys_addr_t io_bus_addr;
+ u32 io_size;
+ phys_addr_t mem_bus_addr;
+ u32 mem_size;
+ int reg_no;
+ int err = 0;
+ int offset = 0;
+
+ LIST_HEAD(res);
+
+ if (!dev->of_node)
+ return -ENODEV;
+
+ port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
+ if (!port)
+ return -ENOMEM;
+
+ port->dev = dev;
+
+ err = rockchip_pcie_parse_dt(port);
+ if (err)
+ return err;
+
+ err = clk_prepare_enable(port->aclk_pcie);
+ if (err) {
+ dev_err(dev, "Unable to enable aclk_pcie clock.\n");
+ goto err_aclk_pcie;
+ }
+
+ err = clk_prepare_enable(port->aclk_perf_pcie);
+ if (err) {
+ dev_err(dev, "Unable to enable aclk_perf_pcie clock.\n");
+ goto err_aclk_perf_pcie;
+ }
+
+ err = clk_prepare_enable(port->hclk_pcie);
+ if (err) {
+ dev_err(dev, "Unable to enable hclk_pcie clock.\n");
+ goto err_hclk_pcie;
+ }
+
+ err = clk_prepare_enable(port->clk_pcie_pm);
+ if (err) {
+ dev_err(dev, "Unable to enable hclk_pcie clock.\n");
+ goto err_pcie_pm;
+ }
+
+ err = rockchip_pcie_set_vpcie(port);
+ if (err) {
+ dev_err(port->dev, "Fail to set vpcie regulator.\n");
+ goto err_set_vpcie;
+ }
+
+ err = rockchip_pcie_init_port(port);
+ if (err)
+ goto err_vpcie;
+
+ platform_set_drvdata(pdev, port);
+
+ rockchip_pcie_enable_interrupts(port);
+
+ err = rockchip_pcie_init_irq_domain(port);
+ if (err < 0)
+ goto err_vpcie;
+
+ err = of_pci_get_host_bridge_resources(dev->of_node, 0, 0xff,
+ &res, &io_base);
+ if (err)
+ goto err_vpcie;
+
+ /* Get the I/O and memory ranges from DT */
+ resource_list_for_each_entry(win, &res) {
+ switch (resource_type(win->res)) {
+ case IORESOURCE_IO:
+ io = win->res;
+ io->name = "I/O";
+ io_size = resource_size(io);
+ io_bus_addr = io->start - win->offset;
+ err = pci_remap_iospace(io, io_base);
+ if (err) {
+ dev_warn(port->dev, "error %d: failed to map resource %pR\n",
+ err, io);
+ continue;
+ }
+ break;
+ case IORESOURCE_MEM:
+ mem = win->res;
+ mem->name = "MEM";
+ mem_size = resource_size(mem);
+ mem_bus_addr = mem->start - win->offset;
+ break;
+ case 0:
+ break;
+ case IORESOURCE_BUS:
+ busn = win->res;
+ break;
+ default:
+ continue;
+ }
+ }
+
+ for (reg_no = 0; reg_no < (mem_size >> 20); reg_no++) {
+ err = rockchip_pcie_prog_ob_atu(port, reg_no + 1,
+ AXI_WRAPPER_MEM_WRITE,
+ 20 - 1,
+ mem_bus_addr +
+ (reg_no << 20),
+ 0);
+ if (err) {
+ dev_err(dev, "Program RC mem outbound atu failed\n");
+ goto err_vpcie;
+ }
+ }
+
+ err = rockchip_pcie_prog_ib_atu(port, 2, 32 - 1, 0x0, 0);
+ if (err) {
+ dev_err(dev, "Program RC mem inbound atu failed\n");
+ goto err_vpcie;
+ }
+
+ offset = mem_size >> 20;
+
+ for (reg_no = 0; reg_no < (io_size >> 20); reg_no++) {
+ err = rockchip_pcie_prog_ob_atu(port,
+ reg_no + 1 + offset,
+ AXI_WRAPPER_IO_WRITE,
+ 20 - 1,
+ io_bus_addr +
+ (reg_no << 20),
+ 0);
+ if (err) {
+ dev_err(dev, "Program RC io outbound atu failed\n");
+ goto err_vpcie;
+ }
+ }
+
+ port->root_bus_nr = busn->start;
+
+ bus = pci_scan_root_bus(&pdev->dev, 0,
+ &rockchip_pcie_ops, port, &res);
+
+ if (!bus) {
+ err = -ENOMEM;
+ goto err_vpcie;
+ }
+
+ pci_bus_size_bridges(bus);
+ pci_bus_assign_resources(bus);
+ list_for_each_entry(child, &bus->children, node)
+ pcie_bus_configure_settings(child);
+
+ pci_bus_add_devices(bus);
+
+ return err;
+
+err_vpcie:
+ if (!IS_ERR(port->vpcie3v3))
+ regulator_disable(port->vpcie3v3);
+ if (!IS_ERR(port->vpcie1v8))
+ regulator_disable(port->vpcie1v8);
+ if (!IS_ERR(port->vpcie0v9))
+ regulator_disable(port->vpcie0v9);
+err_set_vpcie:
+ clk_disable_unprepare(port->clk_pcie_pm);
+err_pcie_pm:
+ clk_disable_unprepare(port->hclk_pcie);
+err_hclk_pcie:
+ clk_disable_unprepare(port->aclk_perf_pcie);
+err_aclk_perf_pcie:
+ clk_disable_unprepare(port->aclk_pcie);
+err_aclk_pcie:
+ return err;
+}
+
+static const struct of_device_id rockchip_pcie_of_match[] = {
+ { .compatible = "rockchip,rk3399-pcie", },
+ {}
+};
+
+static struct platform_driver rockchip_pcie_driver = {
+ .driver = {
+ .name = "rockchip-pcie",
+ .of_match_table = rockchip_pcie_of_match,
+ },
+ .probe = rockchip_pcie_probe,
+
+};
+module_platform_driver(rockchip_pcie_driver);
+
+MODULE_AUTHOR("Rockchip Inc");
+MODULE_DESCRIPTION("Rockchip AXI PCIe driver");
+MODULE_LICENSE("GPL v2");
--
2.3.7
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3 2/2] PCI: Rockchip: Add Rockchip PCIe controller support
2016-06-16 1:50 [PATCH v3 2/2] PCI: Rockchip: Add Rockchip PCIe controller support Shawn Lin
@ 2016-06-16 13:08 ` kbuild test robot
2016-06-23 0:29 ` Brian Norris
1 sibling, 0 replies; 6+ messages in thread
From: kbuild test robot @ 2016-06-16 13:08 UTC (permalink / raw)
To: Shawn Lin
Cc: kbuild-all, Bjorn Helgaas, Marc Zyngier, linux-pci,
Arnd Bergmann, linux-kernel, linux-rockchip, Heiko Stuebner,
Doug Anderson, Wenrui Li, Rob Herring, devicetree, Shawn Lin
[-- Attachment #1: Type: text/plain, Size: 6260 bytes --]
Hi,
[auto build test WARNING on pci/next]
[also build test WARNING on v4.7-rc3 next-20160616]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Shawn-Lin/Documentation-bindings-add-dt-doc-for-Rockchip-PCIe-controller/20160616-095337
base: https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
config: arm64-allyesconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 5.3.1-8) 5.3.1 20160205
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm64
Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings
All warnings (new ones prefixed by >>):
drivers/pci/host/pcie-rockchip.c: In function 'rockchip_pcie_probe':
>> drivers/pci/host/pcie-rockchip.c:995:6: warning: 'mem_size' may be used uninitialized in this function [-Wmaybe-uninitialized]
u32 mem_size;
^
>> drivers/pci/host/pcie-rockchip.c:1094:9: warning: 'mem_bus_addr' may be used uninitialized in this function [-Wmaybe-uninitialized]
err = rockchip_pcie_prog_ob_atu(port, reg_no + 1,
^
>> drivers/pci/host/pcie-rockchip.c:993:6: warning: 'io_size' may be used uninitialized in this function [-Wmaybe-uninitialized]
u32 io_size;
^
>> drivers/pci/host/pcie-rockchip.c:1115:9: warning: 'io_bus_addr' may be used uninitialized in this function [-Wmaybe-uninitialized]
err = rockchip_pcie_prog_ob_atu(port,
^
>> drivers/pci/host/pcie-rockchip.c:1128:26: warning: 'busn' may be used uninitialized in this function [-Wmaybe-uninitialized]
port->root_bus_nr = busn->start;
^
vim +/mem_size +995 drivers/pci/host/pcie-rockchip.c
987 struct resource_entry *win;
988 resource_size_t io_base;
989 struct resource *busn;
990 struct resource *mem;
991 struct resource *io;
992 phys_addr_t io_bus_addr;
> 993 u32 io_size;
994 phys_addr_t mem_bus_addr;
> 995 u32 mem_size;
996 int reg_no;
997 int err = 0;
998 int offset = 0;
999
1000 LIST_HEAD(res);
1001
1002 if (!dev->of_node)
1003 return -ENODEV;
1004
1005 port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
1006 if (!port)
1007 return -ENOMEM;
1008
1009 port->dev = dev;
1010
1011 err = rockchip_pcie_parse_dt(port);
1012 if (err)
1013 return err;
1014
1015 err = clk_prepare_enable(port->aclk_pcie);
1016 if (err) {
1017 dev_err(dev, "Unable to enable aclk_pcie clock.\n");
1018 goto err_aclk_pcie;
1019 }
1020
1021 err = clk_prepare_enable(port->aclk_perf_pcie);
1022 if (err) {
1023 dev_err(dev, "Unable to enable aclk_perf_pcie clock.\n");
1024 goto err_aclk_perf_pcie;
1025 }
1026
1027 err = clk_prepare_enable(port->hclk_pcie);
1028 if (err) {
1029 dev_err(dev, "Unable to enable hclk_pcie clock.\n");
1030 goto err_hclk_pcie;
1031 }
1032
1033 err = clk_prepare_enable(port->clk_pcie_pm);
1034 if (err) {
1035 dev_err(dev, "Unable to enable hclk_pcie clock.\n");
1036 goto err_pcie_pm;
1037 }
1038
1039 err = rockchip_pcie_set_vpcie(port);
1040 if (err) {
1041 dev_err(port->dev, "Fail to set vpcie regulator.\n");
1042 goto err_set_vpcie;
1043 }
1044
1045 err = rockchip_pcie_init_port(port);
1046 if (err)
1047 goto err_vpcie;
1048
1049 platform_set_drvdata(pdev, port);
1050
1051 rockchip_pcie_enable_interrupts(port);
1052
1053 err = rockchip_pcie_init_irq_domain(port);
1054 if (err < 0)
1055 goto err_vpcie;
1056
1057 err = of_pci_get_host_bridge_resources(dev->of_node, 0, 0xff,
1058 &res, &io_base);
1059 if (err)
1060 goto err_vpcie;
1061
1062 /* Get the I/O and memory ranges from DT */
1063 resource_list_for_each_entry(win, &res) {
1064 switch (resource_type(win->res)) {
1065 case IORESOURCE_IO:
1066 io = win->res;
1067 io->name = "I/O";
1068 io_size = resource_size(io);
1069 io_bus_addr = io->start - win->offset;
1070 err = pci_remap_iospace(io, io_base);
1071 if (err) {
1072 dev_warn(port->dev, "error %d: failed to map resource %pR\n",
1073 err, io);
1074 continue;
1075 }
1076 break;
1077 case IORESOURCE_MEM:
1078 mem = win->res;
1079 mem->name = "MEM";
1080 mem_size = resource_size(mem);
1081 mem_bus_addr = mem->start - win->offset;
1082 break;
1083 case 0:
1084 break;
1085 case IORESOURCE_BUS:
1086 busn = win->res;
1087 break;
1088 default:
1089 continue;
1090 }
1091 }
1092
1093 for (reg_no = 0; reg_no < (mem_size >> 20); reg_no++) {
> 1094 err = rockchip_pcie_prog_ob_atu(port, reg_no + 1,
1095 AXI_WRAPPER_MEM_WRITE,
1096 20 - 1,
1097 mem_bus_addr +
1098 (reg_no << 20),
1099 0);
1100 if (err) {
1101 dev_err(dev, "Program RC mem outbound atu failed\n");
1102 goto err_vpcie;
1103 }
1104 }
1105
1106 err = rockchip_pcie_prog_ib_atu(port, 2, 32 - 1, 0x0, 0);
1107 if (err) {
1108 dev_err(dev, "Program RC mem inbound atu failed\n");
1109 goto err_vpcie;
1110 }
1111
1112 offset = mem_size >> 20;
1113
1114 for (reg_no = 0; reg_no < (io_size >> 20); reg_no++) {
> 1115 err = rockchip_pcie_prog_ob_atu(port,
1116 reg_no + 1 + offset,
1117 AXI_WRAPPER_IO_WRITE,
1118 20 - 1,
1119 io_bus_addr +
1120 (reg_no << 20),
1121 0);
1122 if (err) {
1123 dev_err(dev, "Program RC io outbound atu failed\n");
1124 goto err_vpcie;
1125 }
1126 }
1127
> 1128 port->root_bus_nr = busn->start;
1129
1130 bus = pci_scan_root_bus(&pdev->dev, 0,
1131 &rockchip_pcie_ops, port, &res);
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 50230 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 2/2] PCI: Rockchip: Add Rockchip PCIe controller support
2016-06-16 1:50 [PATCH v3 2/2] PCI: Rockchip: Add Rockchip PCIe controller support Shawn Lin
2016-06-16 13:08 ` kbuild test robot
@ 2016-06-23 0:29 ` Brian Norris
2016-06-23 1:09 ` Shawn Lin
1 sibling, 1 reply; 6+ messages in thread
From: Brian Norris @ 2016-06-23 0:29 UTC (permalink / raw)
To: Shawn Lin
Cc: Bjorn Helgaas, devicetree, Heiko Stuebner, Arnd Bergmann,
Marc Zyngier, linux-pci, Wenrui Li, linux-kernel, Doug Anderson,
linux-rockchip, Rob Herring
Hi Shawn,
I'm familiarizing myself a bit with your PCIe core, but I'm certainly no
expert, so I may have some dumb comments. Also a few other general
comments.
On Thu, Jun 16, 2016 at 09:50:35AM +0800, Shawn Lin wrote:
> This patch adds Rockchip PCIe controller support found
> on RK3399 Soc platform.
>
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>
> ---
>
> Changes in v3:
> - remove header file
> - remove struct msi_controller and move most of variables
> of rockchip_pcie_port to become the local ones.
> - allow legacy int even if enabling MSI
> - drop regulator set voltage operation suggested by Doug
>
> Changes in v2:
> - remove phy related stuff and call phy API
> - add new head file and define lots of macro to make
> the code more readable
> - remove lots msi related code suggested by Marc
> - add IO window address translation
> - init_port and parse_dt reconstruction suggested by Bharat
> - improve wr_own_conf suggested by Arnd
> - make pcie as an interrupt controller and fix wrong int handler
> suggested by Marc
> - remove PCI_PROBE_ONLY suggested by Lorenzo
>
> drivers/pci/host/Kconfig | 11 +
> drivers/pci/host/Makefile | 1 +
> drivers/pci/host/pcie-rockchip.c | 1183 ++++++++++++++++++++++++++++++++++++++
> 3 files changed, 1195 insertions(+)
> create mode 100644 drivers/pci/host/pcie-rockchip.c
>
> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
> index 5d2374e..251f4ac 100644
> --- a/drivers/pci/host/Kconfig
> +++ b/drivers/pci/host/Kconfig
> @@ -245,4 +245,15 @@ config PCIE_ARMADA_8K
> Designware hardware and therefore the driver re-uses the
> Designware core functions to implement the driver.
>
> +config PCIE_ROCKCHIP
> + bool "Rockchip PCIe controller"
Any good reason this can't be a module? I realize PCIe is something we'd
likely want built in most of the time, but is there a code reason?
> + depends on ARM64 && ARCH_ROCKCHIP
> + depends on OF
> + select MFD_SYSCON
> + select PCI_MSI_IRQ_DOMAIN if PCI_MSI
> + help
> + Say Y here if you want internal PCI support on Rockchip SoC.
> + There are 1 internal PCIe port available to support GEN2 with
s/are/is/
> + 4 slots.
> +
> endmenu
> diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
> index 9c8698e..c52985c 100644
> --- a/drivers/pci/host/Makefile
> +++ b/drivers/pci/host/Makefile
> @@ -29,3 +29,4 @@ obj-$(CONFIG_PCIE_QCOM) += pcie-qcom.o
> obj-$(CONFIG_PCI_HOST_THUNDER_ECAM) += pci-thunder-ecam.o
> obj-$(CONFIG_PCI_HOST_THUNDER_PEM) += pci-thunder-pem.o
> obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o
> +obj-$(CONFIG_PCIE_ROCKCHIP) += pcie-rockchip.o
> diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
> new file mode 100644
> index 0000000..661d6e0
> --- /dev/null
> +++ b/drivers/pci/host/pcie-rockchip.c
> @@ -0,0 +1,1183 @@
> +/*
> + * Rockchip AXI PCIe host controller driver
> + *
> + * Copyright (c) 2016 Rockchip, Inc.
> + *
> + * Author: Shawn Lin <shawn.lin@rock-chips.com>
> + * Wenrui Li <wenrui.li@rock-chips.com>
> + *
> + * Bits taken from Synopsys Designware Host controller driver and
> + * ARM PCI Host generic driver.
> + *
> + * This program is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 2 of the License, or
> + * (at your option) any later version.
> + */
Add a blank line here?
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/irqchip/chained_irq.h>
> +#include <linux/irqdomain.h>
> +#include <linux/kernel.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/of_device.h>
> +#include <linux/of_pci.h>
> +#include <linux/of_platform.h>
> +#include <linux/of_irq.h>
> +#include <linux/pci.h>
> +#include <linux/pci_ids.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset.h>
> +#include <linux/regmap.h>
> +
> +#define PCIE_CLIENT_BASE 0x0
> +#define PCIE_RC_CONFIG_NORMAL_BASE 0x800000
> +#define PCIE_RC_CONFIG_BASE 0xa00000
> +#define PCIE_CORE_LINK_CTRL_STATUS 0x8000d0
> +#define PCIE_CORE_CTRL_MGMT_BASE 0x900000
> +#define PCIE_CORE_AXI_CONF_BASE 0xc00000
> +#define PCIE_CORE_AXI_INBOUND_BASE 0xc00800
> +#define PCIE_CLIENT_BASIC_STATUS0 0x44
> +#define PCIE_CLIENT_BASIC_STATUS1 0x48
> +#define PCIE_CLIENT_INT_MASK 0x4c
> +#define PCIE_CLIENT_INT_STATUS 0x50
> +#define PCIE_RC_CONFIG_RID_CCR 0x8
> +#define PCIR_RC_CONFIG_LCS 0xd0
s/PCIR/PCIE/
...right?
> +#define PCIE_RC_BAR_CONF 0x300
> +#define PCIE_CORE_OB_REGION_ADDR1 0x4
> +#define PCIE_CORE_OB_REGION_DESC0 0x8
> +#define PCIE_CORE_OB_REGION_DESC1 0xc
> +#define PCIE_RP_IB_ADDR_TRANS 0x4
> +#define PCIE_CORE_INT_MASK 0x900210
> +#define PCIE_CORE_INT_STATUS 0x90020c
> +
> +/* Size of one AXI Region (not Region 0) */
> +#define AXI_REGION_SIZE BIT(20)
> +/* Size of Region 0, equal to sum of sizes of other regions */
> +#define AXI_REGION_0_SIZE (32 * (0x1 << 20))
> +#define OB_REG_SIZE_SHIFT 5
> +#define IB_ROOT_PORT_REG_SIZE_SHIFT 3
> +#define AXI_WRAPPER_IO_WRITE 0x6
> +#define AXI_WRAPPER_MEM_WRITE 0x2
> +#define MAX_AXI_IB_ROOTPORT_REGION_NUM 3
> +#define MIN_AXI_ADDR_BITS_PASSED 8
> +#define ROCKCHIP_VENDOR_ID 0x1d87
> +#define PCIE_ECAM_BUS(x) (((x) & 0xFF) << 20)
> +#define PCIE_ECAM_DEV(x) (((x) & 0x1F) << 15)
> +#define PCIE_ECAM_FUNC(x) (((x) & 0x7) << 12)
> +#define PCIE_ECAM_REG(x) (((x) & 0xFFF) << 0)
> +#define PCIE_ECAM_ADDR(bus, dev, func, reg) \
> + (PCIE_ECAM_BUS(bus) | PCIE_ECAM_DEV(dev) | \
> + PCIE_ECAM_FUNC(func) | PCIE_ECAM_REG(reg))
> +
> +/*
> + * The higher 16-bit of this register is used for write protection
> + * only if BIT(x + 16) set to 1 the BIT(x) can be written.
> + */
> +#define HIWORD_UPDATE(val, mask, shift) \
> + ((val) << (shift) | (mask) << ((shift) + 16))
> +
> +#define RC_REGION_0_ADDR_TRANS_H 0x00000000
> +#define RC_REGION_0_ADDR_TRANS_L 0x00000000
> +#define RC_REGION_0_PASS_BITS (25 - 1)
> +#define RC_REGION_1_ADDR_TRANS_H 0x00000000
> +#define RC_REGION_1_ADDR_TRANS_L 0x00400000
> +#define RC_REGION_1_PASS_BITS (20 - 1)
> +#define MAX_AXI_WRAPPER_REGION_NUM 33
> +#define PCIE_CORE_LCSR_RETAIN_LINK BIT(5)
s/RETAIN/RETRAIN/
> +#define PCIE_CLIENT_CONF_ENABLE 1
> +#define PCIE_CLIENT_LINK_TRAIN_ENABLE 1
> +#define PCIE_CLIENT_ARI_ENABLE 1
> +#define PCIE_CLIENT_CONF_LANE_NUM(x) (x / 2)
> +#define PCIE_CLIENT_MODE_RC 1
> +#define PCIE_CLIENT_GEN_SEL_2 1
> +#define PCIE_CLIENT_GEN_SEL_1 0
> +#define PCIE_CLIENT_CONF_ENABLE_SHIFT 0
> +#define PCIE_CLIENT_CONF_ENABLE_MASK 0x1
It's quite confusing (IMO) having the values (e.g.,
PCIE_CLIENT_CONF_ENABLE) separated from the bitfield SHIFT/MASK macros.
Can you reorganize so they're listed next to each other?
> +#define PCIE_CLIENT_LINK_TRAIN_SHIFT 1
> +#define PCIE_CLIENT_LINK_TRAIN_MASK 0x1
> +#define PCIE_CLIENT_ARI_ENABLE_SHIFT 3
> +#define PCIE_CLIENT_ARI_ENABLE_MASK 0x1
> +#define PCIE_CLIENT_CONF_LANE_NUM_SHIFT 4
> +#define PCIE_CLIENT_CONF_LANE_NUM_MASK 0x3
> +#define PCIE_CLIENT_MODE_SHIFT 6
> +#define PCIE_CLIENT_MODE_MASK 0x1
> +#define PCIE_CLIENT_GEN_SEL_SHIFT 7
> +#define PCIE_CLIENT_GEN_SEL_MASK 0x1
> +#define PCIE_CLIENT_LINK_STATUS_UP 0x3
> +#define PCIE_CLIENT_LINK_STATUS_SHIFT 20
> +#define PCIE_CLIENT_LINK_STATUS_MASK 0x3
> +#define PCIE_CORE_PL_CONF_SPEED_25G 0x0
> +#define PCIE_CORE_PL_CONF_SPEED_50G 0x1
> +#define PCIE_CORE_PL_CONF_SPEED_80G 0x2
These speeds are not actually 25/50/80G, they're 2.5/5.0/8.0G. Maybe you
can either add an underscore (_) where the decimal would be? And
optionally drop the 0's? Like:
#define PCIE_CORE_PL_CONF_SPEED_2_5G 0x0
#define PCIE_CORE_PL_CONF_SPEED_5G 0x1
#define PCIE_CORE_PL_CONF_SPEED_8G 0x2
I'd be ashamed to admit how much time I spent confused as to where you
were getting 25, 50, and 80 from...
> +#define PCIE_CORE_PL_CONF_SPEED_SHIFT 3
> +#define PCIE_CORE_PL_CONF_SPEED_MASK 0x3
> +#define PCIE_CORE_PL_CONF_LANE_SHIFT 1
> +#define PCIE_CORE_PL_CONF_LANE_MASK 0x3
> +#define PCIE_CORE_RC_CONF_SCC_SHIFT 16
> +
> +/* PCIE_CLIENT_INT_STATUS */
> +#define PCIE_CLIENT_INT_LEGACY_DONE BIT(15)
> +#define PCIE_CLIENT_INT_MSG BIT(14)
> +#define PCIE_CLIENT_INT_HOT_RST BIT(13)
> +#define PCIE_CLIENT_INT_DPA BIT(12)
> +#define PCIE_CLIENT_INT_FATAL_ERR BIT(11)
> +#define PCIE_CLIENT_INT_NFATAL_ERR BIT(10)
> +#define PCIE_CLIENT_INT_CORR_ERR BIT(9)
> +#define PCIE_CLIENT_INT_INTD BIT(8)
> +#define PCIE_CLIENT_INT_INTC BIT(7)
> +#define PCIE_CLIENT_INT_INTB BIT(6)
> +#define PCIE_CLIENT_INT_INTA BIT(5)
> +#define PCIE_CLIENT_INT_LOCAL BIT(4)
> +#define PCIE_CLIENT_INT_UDMA BIT(3)
> +#define PCIE_CLIENT_INT_PHY BIT(2)
> +#define PCIE_CLIENT_INT_HOT_PLUG BIT(1)
> +#define PCIE_CLIENT_INT_PWR_STCG BIT(0)
> +#define PCIE_CORE_INT_PRFPE BIT(0)
> +#define PCIE_CORE_INT_CRFPE BIT(1)
> +#define PCIE_CORE_INT_RRPE BIT(2)
> +#define PCIE_CORE_INT_PRFO BIT(3)
> +#define PCIE_CORE_INT_CRFO BIT(4)
> +#define PCIE_CORE_INT_RT BIT(5)
> +#define PCIE_CORE_INT_RTR BIT(6)
> +#define PCIE_CORE_INT_PE BIT(7)
> +#define PCIE_CORE_INT_MTR BIT(8)
> +#define PCIE_CORE_INT_UCR BIT(9)
> +#define PCIE_CORE_INT_FCE BIT(10)
> +#define PCIE_CORE_INT_CT BIT(11)
> +#define PCIE_CORE_INT_UTC BIT(18)
> +#define PCIE_CORE_INT_MMVC BIT(19)
> +
> +#define ROCKCHIP_PCIE_RPIFR1_INTR_MASK GENMASK(8, 5)
> +#define ROCKCHIP_PCIE_RPIFR1_INTR_SHIFT 5
> +
> +#define PCIE_CORE_INT \
> + (PCIE_CORE_INT_PRFPE | PCIE_CORE_INT_CRFPE | \
> + PCIE_CORE_INT_RRPE | PCIE_CORE_INT_CRFO | \
> + PCIE_CORE_INT_RT | PCIE_CORE_INT_RTR | \
> + PCIE_CORE_INT_PE | PCIE_CORE_INT_MTR | \
> + PCIE_CORE_INT_UCR | PCIE_CORE_INT_FCE | \
> + PCIE_CORE_INT_CT | PCIE_CORE_INT_UTC | \
> + PCIE_CORE_INT_MMVC)
> +
> +#define PCIE_CLIENT_INT_SUBSYSTEM \
> + (PCIE_CLIENT_INT_PWR_STCG | PCIE_CLIENT_INT_HOT_PLUG | \
> + PCIE_CLIENT_INT_PHY | PCIE_CLIENT_INT_UDMA | \
> + PCIE_CLIENT_INT_LOCAL)
> +
> +#define PCIE_CLIENT_INT_LEGACY \
> + (PCIE_CLIENT_INT_INTA | PCIE_CLIENT_INT_INTB | \
> + PCIE_CLIENT_INT_INTC | PCIE_CLIENT_INT_INTD)
> +
> +#define PCIE_CLIENT_INT_CLI \
> + (PCIE_CLIENT_INT_CORR_ERR | PCIE_CLIENT_INT_NFATAL_ERR | \
> + PCIE_CLIENT_INT_FATAL_ERR | PCIE_CLIENT_INT_DPA | \
> + PCIE_CLIENT_INT_HOT_RST | PCIE_CLIENT_INT_MSG | \
> + PCIE_CLIENT_INT_LEGACY_DONE | PCIE_CLIENT_INT_LEGACY)
> +
> +struct rockchip_pcie_port {
> + void __iomem *reg_base;
> + void __iomem *apb_base;
> + struct phy *phy;
> + struct reset_control *core_rst;
> + struct reset_control *mgmt_rst;
> + struct reset_control *mgmt_sticky_rst;
> + struct reset_control *pipe_rst;
> + struct clk *aclk_pcie;
> + struct clk *aclk_perf_pcie;
> + struct clk *hclk_pcie;
> + struct clk *clk_pcie_pm;
> + struct regulator *vpcie3v3; /* 3.3V power supply */
> + struct regulator *vpcie1v8; /* 1.8V power supply */
> + struct regulator *vpcie0v9; /* 0.9V power supply */
> + struct gpio_desc *ep_gpio;
> + u32 lanes;
> + u8 root_bus_nr;
> + struct device *dev;
> + struct irq_domain *irq_domain;
> +};
> +
> +static irqreturn_t rockchip_pcie_subsys_irq_handler(int irq, void *arg);
> +static irqreturn_t rockchip_pcie_client_irq_handler(int irq, void *arg);
> +static void rockchip_pcie_legacy_int_handler(struct irq_desc *desc);
> +
> +static inline u32 pcie_read(struct rockchip_pcie_port *port, u32 reg)
> +{
> + return readl(port->apb_base + reg);
> +}
> +
> +static inline void pcie_write(struct rockchip_pcie_port *port,
> + u32 val, u32 reg)
> +{
> + writel(val, port->apb_base + reg);
> +}
> +
> +static int rockchip_pcie_valid_config(struct rockchip_pcie_port *pp,
> + struct pci_bus *bus, int dev)
> +{
> + /* access only one slot on each root port */
> + if (bus->number == pp->root_bus_nr && dev > 0)
> + return 0;
> +
> + /*
> + * do not read more than one device on the bus directly attached
> + * to RC's (Virtual Bridge's) DS side.
> + */
> + if (bus->primary == pp->root_bus_nr && dev > 0)
> + return 0;
> +
> + return 1;
> +}
> +
> +static int rockchip_pcie_rd_own_conf(struct rockchip_pcie_port *pp,
> + int where, int size,
> + u32 *val)
> +{
> + void __iomem *addr = pp->apb_base + PCIE_RC_CONFIG_BASE + where;
> +
> + if (!IS_ALIGNED((uintptr_t)addr, size)) {
> + *val = 0;
> + return PCIBIOS_BAD_REGISTER_NUMBER;
> + }
> +
> + if (size == 4) {
> + *val = readl(addr);
> + } else if (size == 2) {
> + *val = readw(addr);
> + } else if (size == 1) {
> + *val = readb(addr);
> + } else {
> + *val = 0;
> + return PCIBIOS_BAD_REGISTER_NUMBER;
> + }
> + return PCIBIOS_SUCCESSFUL;
> +}
> +
> +static int rockchip_pcie_wr_own_conf(struct rockchip_pcie_port *pp,
> + int where, int size, u32 val)
> +{
> + u32 mask, tmp, offset;
> +
> + offset = (where & (~0x3));
> +
> + if (size == 4) {
> + writel(val, pp->apb_base + PCIE_RC_CONFIG_BASE + offset);
> + return PCIBIOS_SUCCESSFUL;
> + }
> +
> + mask = ~(((1 << (size * 8)) - 1) << ((where & 0x3) * 8));
> +
> + tmp = readl(pp->apb_base + PCIE_RC_CONFIG_BASE + offset) & mask;
> + tmp |= val << ((where & 0x3) * 8);
> + writel(tmp, pp->apb_base + PCIE_RC_CONFIG_BASE + offset);
> +
> + return PCIBIOS_SUCCESSFUL;
> +}
> +
> +static int rockchip_pcie_rd_other_conf(struct rockchip_pcie_port *pp,
> + struct pci_bus *bus, u32 devfn,
> + int where, int size, u32 *val)
> +{
> + u32 busdev;
> +
> + busdev = PCIE_ECAM_ADDR(bus->number, PCI_SLOT(devfn),
> + PCI_FUNC(devfn), where);
> +
> + if (!IS_ALIGNED(busdev, size)) {
> + *val = 0;
> + return PCIBIOS_BAD_REGISTER_NUMBER;
> + }
> +
> + if (size == 4) {
> + *val = readl(pp->reg_base + busdev);
> + } else if (size == 2) {
> + *val = readw(pp->reg_base + busdev);
> + } else if (size == 1) {
> + *val = readb(pp->reg_base + busdev);
> + } else {
> + *val = 0;
> + return PCIBIOS_BAD_REGISTER_NUMBER;
> + }
> + return PCIBIOS_SUCCESSFUL;
> +}
> +
> +static int rockchip_pcie_wr_other_conf(struct rockchip_pcie_port *pp,
> + struct pci_bus *bus, u32 devfn,
> + int where, int size, u32 val)
> +{
> + u32 busdev;
> +
> + busdev = PCIE_ECAM_ADDR(bus->number, PCI_SLOT(devfn),
> + PCI_FUNC(devfn), where);
> + if (!IS_ALIGNED(busdev, size))
> + return PCIBIOS_BAD_REGISTER_NUMBER;
> +
> + if (size == 4)
> + writel(val, pp->reg_base + busdev);
> + else if (size == 2)
> + writew(val, pp->reg_base + busdev);
> + else if (size == 1)
> + writeb(val, pp->reg_base + busdev);
> + else
> + return PCIBIOS_BAD_REGISTER_NUMBER;
> +
> + return PCIBIOS_SUCCESSFUL;
> +}
> +
> +static int rockchip_pcie_rd_conf(struct pci_bus *bus, u32 devfn, int where,
> + int size, u32 *val)
> +{
> + struct rockchip_pcie_port *pp = bus->sysdata;
> + int ret;
> +
> + if (rockchip_pcie_valid_config(pp, bus, PCI_SLOT(devfn)) == 0) {
> + *val = 0xffffffff;
> + return PCIBIOS_DEVICE_NOT_FOUND;
> + }
> +
> + if (bus->number != pp->root_bus_nr)
> + ret = rockchip_pcie_rd_other_conf(pp, bus, devfn,
> + where, size, val);
> + else
> + ret = rockchip_pcie_rd_own_conf(pp, where, size, val);
> +
> + return ret;
> +}
> +
> +static int rockchip_pcie_wr_conf(struct pci_bus *bus, u32 devfn,
> + int where, int size, u32 val)
> +{
> + struct rockchip_pcie_port *pp = bus->sysdata;
> + int ret;
> +
> + if (rockchip_pcie_valid_config(pp, bus, PCI_SLOT(devfn)) == 0)
> + return PCIBIOS_DEVICE_NOT_FOUND;
> +
> + if (bus->number != pp->root_bus_nr)
> + ret = rockchip_pcie_wr_other_conf(pp, bus, devfn,
> + where, size, val);
> + else
> + ret = rockchip_pcie_wr_own_conf(pp, where, size, val);
> +
> + return ret;
> +}
> +
> +static struct pci_ops rockchip_pcie_ops = {
> + .read = rockchip_pcie_rd_conf,
> + .write = rockchip_pcie_wr_conf,
> +};
> +
> +/**
> + * rockchip_pcie_init_port - Initialize hardware
> + * @port: PCIe port information
> + */
> +static int rockchip_pcie_init_port(struct rockchip_pcie_port *port)
> +{
> + int err;
> + u32 status;
> + unsigned long timeout;
> +
> + gpiod_set_value(port->ep_gpio, 0);
> +
> + err = phy_init(port->phy);
> + if (err < 0)
> + dev_err(port->dev, "fail to init phy, err %d\n", err);
Are you intentionally continuing, even on error? Seems like you should
'return ret'.
> +
> + err = reset_control_assert(port->core_rst);
> + if (err) {
> + dev_err(port->dev, "assert core_rst err %d\n", err);
> + return err;
> + }
> +
> + err = reset_control_assert(port->mgmt_rst);
> + if (err) {
> + dev_err(port->dev, "assert mgmt_rst err %d\n", err);
> + return err;
> + }
> +
> + err = reset_control_assert(port->mgmt_sticky_rst);
> + if (err) {
> + dev_err(port->dev, "assert mgmt_sticky_rst err %d\n", err);
> + return err;
> + }
> +
> + err = reset_control_assert(port->pipe_rst);
> + if (err) {
> + dev_err(port->dev, "assert pipe_rst err %d\n", err);
> + return err;
> + }
> +
> + pcie_write(port,
> + HIWORD_UPDATE(PCIE_CLIENT_CONF_ENABLE,
> + PCIE_CLIENT_CONF_ENABLE_MASK,
> + PCIE_CLIENT_CONF_ENABLE_SHIFT) |
> + HIWORD_UPDATE(PCIE_CLIENT_CONF_LANE_NUM(port->lanes),
> + PCIE_CLIENT_CONF_LANE_NUM_MASK,
> + PCIE_CLIENT_CONF_LANE_NUM_SHIFT) |
> + HIWORD_UPDATE(PCIE_CLIENT_MODE_RC,
> + PCIE_CLIENT_MODE_MASK,
> + PCIE_CLIENT_MODE_SHIFT) |
> + HIWORD_UPDATE(PCIE_CLIENT_ARI_ENABLE,
> + PCIE_CLIENT_ARI_ENABLE_MASK,
> + PCIE_CLIENT_ARI_ENABLE_SHIFT) |
> + HIWORD_UPDATE(PCIE_CLIENT_GEN_SEL_2,
> + PCIE_CLIENT_GEN_SEL_MASK,
> + PCIE_CLIENT_GEN_SEL_SHIFT),
> + PCIE_CLIENT_BASE);
> +
> + err = phy_power_on(port->phy);
> + if (err)
> + dev_err(port->dev, "fail to power on phy, err %d\n", err);
Same here.
> +
> + err = reset_control_deassert(port->core_rst);
> + if (err) {
> + dev_err(port->dev, "deassert core_rst err %d\n", err);
> + return err;
> + }
> +
> + err = reset_control_deassert(port->mgmt_rst);
> + if (err) {
> + dev_err(port->dev, "deassert mgmt_rst err %d\n", err);
> + return err;
> + }
> +
> + err = reset_control_deassert(port->mgmt_sticky_rst);
> + if (err) {
> + dev_err(port->dev, "deassert mgmt_sticky_rst err %d\n", err);
> + return err;
> + }
> +
> + err = reset_control_deassert(port->pipe_rst);
> + if (err) {
> + dev_err(port->dev, "deassert pipe_rst err %d\n", err);
> + return err;
> + }
> +
> + /* Enable Gen1 trainning */
s/nn/n/
> + pcie_write(port,
> + HIWORD_UPDATE(PCIE_CLIENT_LINK_TRAIN_ENABLE,
> + PCIE_CLIENT_LINK_TRAIN_MASK,
> + PCIE_CLIENT_LINK_TRAIN_SHIFT),
> + PCIE_CLIENT_BASE);
> +
> + gpiod_set_value(port->ep_gpio, 1);
> +
> + /* 500ms timeout value should be enough for gen1/2 taining */
> + timeout = jiffies + msecs_to_jiffies(500);
> +
> + err = -ETIMEDOUT;
> + while (time_before(jiffies, timeout)) {
> + status = pcie_read(port, PCIE_CLIENT_BASIC_STATUS1);
> + if (((status >> PCIE_CLIENT_LINK_STATUS_SHIFT) &
> + PCIE_CLIENT_LINK_STATUS_MASK) ==
> + PCIE_CLIENT_LINK_STATUS_UP) {
> + dev_dbg(port->dev, "pcie link training gen1 pass!\n");
> + err = 0;
> + break;
> + }
> + msleep(20);
> + }
Technically, the above timeout loop is not quite correct. Possible error
case: we can fail with a timeout after 500 ms if the training completes
between the 480-500 ms time window. This can happen because you're
doing:
(1) read register: if complete, then terminate successfully
(2) delay
(3) check for timeout: if time is up, return error
You actually need:
(1) read register: if complete, then terminate successfully
(2) delay
(3) check for timeout: if time is up, repeat (1), and then report error
You can examine the logic for readx_poll_timeout() in
include/linux/iopoll.h to see an example of a proper timeout loop. You
could even try to use one of the helpers there, except that your
pcie_read() takes 2 args.
> + if (err) {
> + dev_err(port->dev, "pcie link training gen1 timeout!\n");
> + return err;
> + }
> +
> + /*
> + * Enable retrain for gen2. This should be configured only after
> + * gen1 finished.
> + */
> + status = pcie_read(port,
> + PCIR_RC_CONFIG_LCS + PCIE_RC_CONFIG_BASE);
> + status |= PCIE_CORE_LCSR_RETAIN_LINK;
> + pcie_write(port, status,
> + PCIR_RC_CONFIG_LCS + PCIE_RC_CONFIG_BASE);
I'm not really an expert on this, but how are you actually "retraining
for gen2"? Is that just the behavior of the core, that it retries at the
higher rate on the 2nd training attempt? I'm just confused, since you
set PCIE_CLIENT_GEN_SEL_2 above, so you've already allowed either gen1
or gen2 negotiation AFAICT, and so seemingly you might already have
negotiated at gen2.
> + err = -ETIMEDOUT;
> +
> + while (time_before(jiffies, timeout)) {
You never reset 'timeout' when starting this loop. So you only have a
cumulative 500 ms to do both the gen1 and gen2 loops. Is that
intentional? (I feel like this comment was made on an earlier revision
of this driver, though I haven't read everything thoroughly.)
> + status = pcie_read(port, PCIE_CORE_CTRL_MGMT_BASE);
> + if (((status >> PCIE_CORE_PL_CONF_SPEED_SHIFT) &
> + PCIE_CORE_PL_CONF_SPEED_MASK) ==
> + PCIE_CORE_PL_CONF_SPEED_50G) {
> + dev_dbg(port->dev, "pcie link training gen2 pass!\n");
> + err = 0;
> + break;
> + }
> + msleep(20);
> + }
Similar problem with your timeout loop, as mentioned above. Although I
confused about what you do in the error case here...
> + if (err)
> + dev_dbg(port->dev, "pcie link training gen2 timeout, force to gen1!\n");
... how are you forcing gen1? I don't see any code that indicates this.
Should you at least be checking that there aren't some kind of training
errors, and that we settled back on gen1/2.5G?
> +
> + /* Check the final link with from negotiated lane counter from MGMT */
> + status = pcie_read(port, PCIE_CORE_CTRL_MGMT_BASE);
> + status = 0x1 << ((status >> PCIE_CORE_PL_CONF_LANE_SHIFT) &
> + PCIE_CORE_PL_CONF_LANE_MASK);
> + dev_dbg(port->dev, "current link width is x%d\n", status);
> +
> + pcie_write(port, ROCKCHIP_VENDOR_ID, PCIE_RC_CONFIG_BASE);
> + pcie_write(port, PCI_CLASS_BRIDGE_PCI << PCIE_CORE_RC_CONF_SCC_SHIFT,
> + PCIE_RC_CONFIG_BASE + PCIE_RC_CONFIG_RID_CCR);
> + pcie_write(port, 0x0, PCIE_CORE_CTRL_MGMT_BASE + PCIE_RC_BAR_CONF);
> +
> + pcie_write(port, (RC_REGION_0_ADDR_TRANS_L + RC_REGION_0_PASS_BITS),
> + PCIE_CORE_AXI_CONF_BASE);
> + pcie_write(port, RC_REGION_0_ADDR_TRANS_H,
> + PCIE_CORE_AXI_CONF_BASE + PCIE_CORE_OB_REGION_ADDR1);
> + pcie_write(port, 0x0080000a,
> + PCIE_CORE_AXI_CONF_BASE + PCIE_CORE_OB_REGION_DESC0);
> + pcie_write(port, 0x0,
> + PCIE_CORE_AXI_CONF_BASE + PCIE_CORE_OB_REGION_DESC1);
> +
> + return 0;
> +}
> +
> +/**
> + * rockchip_pcie_parse_dt - Parse Device tree
> + * @port: PCIe port information
> + *
> + * Return: '0' on success and error value on failure
> + */
> +static int rockchip_pcie_parse_dt(struct rockchip_pcie_port *port)
> +{
> + struct device *dev = port->dev;
> + struct platform_device *pdev = to_platform_device(dev);
> + struct device_node *node = dev->of_node;
> + struct resource *regs;
> + int irq;
> + int err = -ENODEV;
> +
> + regs = platform_get_resource_byname(pdev,
> + IORESOURCE_MEM,
> + "axi-base");
> + if (!regs) {
> + dev_err(dev, "missing axi-base property\n");
> + return err;
> + }
> +
> + port->reg_base = devm_ioremap_resource(dev, regs);
> + if (IS_ERR(port->reg_base))
> + return PTR_ERR(port->reg_base);
> +
> + regs = platform_get_resource_byname(pdev,
> + IORESOURCE_MEM,
> + "apb-base");
> + if (!regs) {
> + dev_err(dev, "missing apb-base property\n");
> + return err;
> + }
> +
> + port->apb_base = devm_ioremap_resource(dev, regs);
> + if (IS_ERR(port->apb_base))
> + return PTR_ERR(port->apb_base);
> +
> + port->phy = devm_phy_get(dev, "pcie-phy");
> + if (IS_ERR_OR_NULL(port->phy)) {
> + if (PTR_ERR_OR_ZERO(port->phy) != -EPROBE_DEFER)
> + dev_err(dev, "Missing phy\n");
> + return PTR_ERR(port->phy);
You're handling the port->phy == NULL case kind of inconsistently. On
one hand, I don't think phy_get() can return NULL, so you might not need
to handle it (i.e., the IS_ERR_OR_NULL() -> IS_ERR()) -- and the phy API
can just ignore NULL PHY. But on the other hand, if you do want to
handle NULL here, you probably shouldn't return PTR_ERR(NULL) here
(i.e., 0, which means success).
My recommendation would be:
s/IS_ERR_OR_NULL/IS_ERR/
s/PTR_ERR_OR_ZERO/PTR_ERR/
for the above 4 lines.
> + }
> +
> + port->lanes = 1;
> + err = of_property_read_u32(node, "num-lanes", &port->lanes);
> + if (!err && ((port->lanes == 0) ||
> + (port->lanes == 3) ||
> + (port->lanes > 4))) {
> + dev_warn(dev, "invalid num-lanes, default use one lane\n");
> + port->lanes = 1;
> + }
> +
> + port->core_rst = devm_reset_control_get(dev, "core");
> + if (IS_ERR(port->core_rst)) {
> + if (PTR_ERR(port->core_rst) != -EPROBE_DEFER)
> + dev_err(dev, "missing core rst property in node %s\n",
> + node->name);
> + return PTR_ERR(port->core_rst);
> + }
> +
> + port->mgmt_rst = devm_reset_control_get(dev, "mgmt");
> + if (IS_ERR(port->mgmt_rst)) {
> + if (PTR_ERR(port->mgmt_rst) != -EPROBE_DEFER)
> + dev_err(dev, "missing mgmt rst property in node %s\n",
> + node->name);
> + return PTR_ERR(port->mgmt_rst);
> + }
> +
> + port->mgmt_sticky_rst = devm_reset_control_get(dev, "mgmt-sticky");
> + if (IS_ERR(port->mgmt_sticky_rst)) {
> + if (PTR_ERR(port->mgmt_sticky_rst) != -EPROBE_DEFER)
> + dev_err(dev, "missing mgmt-sticky rst property in node %s\n",
> + node->name);
> + return PTR_ERR(port->mgmt_sticky_rst);
> + }
> +
> + port->pipe_rst = devm_reset_control_get(dev, "pipe");
> + if (IS_ERR(port->pipe_rst)) {
> + if (PTR_ERR(port->pipe_rst) != -EPROBE_DEFER)
> + dev_err(dev, "missing pipe rst property in node %s\n",
> + node->name);
> + return PTR_ERR(port->pipe_rst);
> + }
> +
> + port->ep_gpio = devm_gpiod_get(dev, "ep", GPIOD_OUT_HIGH);
> + if (IS_ERR(port->ep_gpio)) {
> + dev_err(dev, "missing ep-gpios property in node %s\n",
> + node->name);
> + return PTR_ERR(port->ep_gpio);
> + }
> +
> + port->aclk_pcie = devm_clk_get(dev, "aclk");
> + if (IS_ERR(port->aclk_pcie)) {
> + dev_err(dev, "aclk clock not found.\n");
> + return PTR_ERR(port->aclk_pcie);
> + }
> +
> + port->aclk_perf_pcie = devm_clk_get(dev, "aclk-perf");
> + if (IS_ERR(port->aclk_perf_pcie)) {
> + dev_err(dev, "aclk_perf clock not found.\n");
> + return PTR_ERR(port->aclk_perf_pcie);
> + }
> +
> + port->hclk_pcie = devm_clk_get(dev, "hclk");
> + if (IS_ERR(port->hclk_pcie)) {
> + dev_err(dev, "hclk clock not found.\n");
> + return PTR_ERR(port->hclk_pcie);
> + }
> +
> + port->clk_pcie_pm = devm_clk_get(dev, "pm");
> + if (IS_ERR(port->clk_pcie_pm)) {
> + dev_err(dev, "pm clock not found.\n");
> + return PTR_ERR(port->clk_pcie_pm);
> + }
> +
> + irq = platform_get_irq_byname(pdev, "sys");
> + if (irq < 0) {
> + dev_err(dev, "missing pcie_sys IRQ resource\n");
> + return -EINVAL;
> + }
> +
> + err = devm_request_irq(dev, irq, rockchip_pcie_subsys_irq_handler,
> + IRQF_SHARED, "pcie-sys", port);
> + if (err) {
> + dev_err(dev, "failed to request pcie subsystem irq\n");
> + return err;
> + }
> +
> + irq = platform_get_irq_byname(pdev, "legacy");
> + if (irq < 0) {
> + dev_err(dev, "missing pcie_legacy IRQ resource\n");
> + return -EINVAL;
> + }
> +
> + irq_set_chained_handler_and_data(irq,
> + rockchip_pcie_legacy_int_handler,
> + port);
> +
> + irq = platform_get_irq_byname(pdev, "client");
> + if (irq < 0) {
> + dev_err(dev, "missing pcie-client IRQ resource\n");
> + return -EINVAL;
> + }
> +
> + err = devm_request_irq(dev, irq, rockchip_pcie_client_irq_handler,
> + IRQF_SHARED, "pcie-client", port);
> + if (err) {
> + dev_err(dev, "failed to request pcie client irq\n");
> + return err;
> + }
> +
> + port->vpcie3v3 = devm_regulator_get_optional(dev, "vpcie3v3");
> + if (IS_ERR(port->vpcie3v3)) {
> + if (PTR_ERR(port->vpcie3v3) == -EPROBE_DEFER)
> + return -EPROBE_DEFER;
> + dev_info(dev, "No vpcie3v3 regulator found.\n");
> + }
> +
> + port->vpcie1v8 = devm_regulator_get_optional(dev, "vpcie1v8");
> + if (IS_ERR(port->vpcie1v8)) {
> + if (PTR_ERR(port->vpcie1v8) == -EPROBE_DEFER)
> + return -EPROBE_DEFER;
> + dev_info(dev, "No vpcie1v8 regulator found.\n");
> + }
> +
> + port->vpcie0v9 = devm_regulator_get_optional(dev, "vpcie0v9");
> + if (IS_ERR(port->vpcie0v9)) {
> + if (PTR_ERR(port->vpcie0v9) == -EPROBE_DEFER)
> + return -EPROBE_DEFER;
> + dev_info(dev, "No vpcie0v9 regulator found.\n");
> + }
> +
> + return 0;
> +}
> +
> +static int rockchip_pcie_set_vpcie(struct rockchip_pcie_port *port)
> +{
> + int err;
> +
> + if (!IS_ERR(port->vpcie3v3)) {
> + err = regulator_enable(port->vpcie3v3);
> + if (err) {
> + dev_err(port->dev, "Fail to enable vpcie3v3 regulator.\n");
> + goto err_out;
> + }
> + }
> +
> + if (!IS_ERR(port->vpcie1v8)) {
> + err = regulator_enable(port->vpcie1v8);
> + if (err) {
> + dev_err(port->dev, "Fail to enable vpcie1v8 regulator.\n");
> + goto err_disable_3v3;
> + }
> + }
> +
> + if (!IS_ERR(port->vpcie0v9)) {
> + err = regulator_enable(port->vpcie0v9);
> + if (err) {
> + dev_err(port->dev, "Fail to enable vpcie0v9 regulator.\n");
> + goto err_disable_1v8;
> + }
> + }
> +
> + return 0;
> +
> +err_disable_1v8:
> + if (!IS_ERR(port->vpcie1v8))
> + regulator_disable(port->vpcie1v8);
> +err_disable_3v3:
> + if (!IS_ERR(port->vpcie3v3))
> + regulator_disable(port->vpcie3v3);
> +err_out:
> + return err;
> +}
> +
> +static void rockchip_pcie_enable_interrupts(struct rockchip_pcie_port *port)
> +{
> + pcie_write(port, (PCIE_CLIENT_INT_CLI << 16) &
> + (~PCIE_CLIENT_INT_CLI), PCIE_CLIENT_INT_MASK);
> + pcie_write(port, PCIE_CORE_INT, PCIE_CORE_INT_MASK);
> +}
> +
> +static int rockchip_pcie_intx_map(struct irq_domain *domain, unsigned int irq,
> + irq_hw_number_t hwirq)
> +{
> + irq_set_chip_and_handler(irq, &dummy_irq_chip, handle_simple_irq);
> + irq_set_chip_data(irq, domain->host_data);
> +
> + return 0;
> +}
> +
> +static const struct irq_domain_ops intx_domain_ops = {
> + .map = rockchip_pcie_intx_map,
> +};
> +
> +static int rockchip_pcie_init_irq_domain(struct rockchip_pcie_port *pp)
> +{
> + struct device *dev = pp->dev;
> +
> + pp->irq_domain = irq_domain_add_linear(dev->of_node, 4,
> + &intx_domain_ops, pp);
> + if (!pp->irq_domain) {
> + dev_err(dev, "Failed to get a INTx IRQ domain\n");
> + return PTR_ERR(pp->irq_domain);
> + }
> +
> + return 0;
> +}
> +
> +static irqreturn_t rockchip_pcie_subsys_irq_handler(int irq, void *arg)
> +{
> + struct rockchip_pcie_port *pp = arg;
> + u32 reg;
> + u32 sub_reg;
> +
> + reg = pcie_read(pp, PCIE_CLIENT_INT_STATUS);
> + if (reg & PCIE_CLIENT_INT_LOCAL) {
> + dev_dbg(pp->dev, "local interrupt recived\n");
> + sub_reg = pcie_read(pp, PCIE_CORE_INT_STATUS);
> + if (sub_reg & PCIE_CORE_INT_PRFPE)
> + dev_dbg(pp->dev, "Parity error detected while reading from the PNP Receive FIFO RAM\n");
> +
> + if (sub_reg & PCIE_CORE_INT_CRFPE)
> + dev_dbg(pp->dev, "Parity error detected while reading from the Completion Receive FIFO RAM\n");
> +
> + if (sub_reg & PCIE_CORE_INT_RRPE)
> + dev_dbg(pp->dev, "Parity error detected while reading from Replay Buffer RAM\n");
> +
> + if (sub_reg & PCIE_CORE_INT_PRFO)
> + dev_dbg(pp->dev, "Overflow occurred in the PNP Receive FIFO\n");
> +
> + if (sub_reg & PCIE_CORE_INT_CRFO)
> + dev_dbg(pp->dev, "Overflow occurred in the Completion Receive FIFO\n");
> +
> + if (sub_reg & PCIE_CORE_INT_RT)
> + dev_dbg(pp->dev, "Replay timer timed out\n");
> +
> + if (sub_reg & PCIE_CORE_INT_RTR)
> + dev_dbg(pp->dev, "Replay timer rolled over after 4 transmissions of the same TLP\n");
> +
> + if (sub_reg & PCIE_CORE_INT_PE)
> + dev_dbg(pp->dev, "Phy error detected on receive side\n");
> +
> + if (sub_reg & PCIE_CORE_INT_MTR)
> + dev_dbg(pp->dev, "Malformed TLP received from the link\n");
> +
> + if (sub_reg & PCIE_CORE_INT_UCR)
> + dev_dbg(pp->dev, "Malformed TLP received from the link\n");
> +
> + if (sub_reg & PCIE_CORE_INT_FCE)
> + dev_dbg(pp->dev, "An error was observed in the flow control advertisements from the other side\n");
> +
> + if (sub_reg & PCIE_CORE_INT_CT)
> + dev_dbg(pp->dev, "A request timed out waiting for completion\n");
> +
> + if (sub_reg & PCIE_CORE_INT_UTC)
> + dev_dbg(pp->dev, "Unmapped TC error\n");
> +
> + if (sub_reg & PCIE_CORE_INT_MMVC)
> + dev_dbg(pp->dev, "MSI mask register changes\n");
> +
> + pcie_write(pp, sub_reg, PCIE_CORE_INT_STATUS);
> + }
> +
> + pcie_write(pp, PCIE_CLIENT_INT_SUBSYSTEM | PCIE_CLIENT_INT_LOCAL,
> + PCIE_CLIENT_INT_STATUS);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t rockchip_pcie_client_irq_handler(int irq, void *arg)
> +{
> + struct rockchip_pcie_port *pp = arg;
> + u32 reg;
> +
> + reg = pcie_read(pp, PCIE_CLIENT_INT_STATUS);
> + if (reg & PCIE_CLIENT_INT_LEGACY_DONE)
> + dev_dbg(pp->dev, "legacy done interrupt recived\n");
> +
> + if (reg & PCIE_CLIENT_INT_MSG)
> + dev_dbg(pp->dev, "message done interrupt recived\n");
> +
> + if (reg & PCIE_CLIENT_INT_HOT_RST)
> + dev_dbg(pp->dev, "hot reset interrupt recived\n");
> +
> + if (reg & PCIE_CLIENT_INT_DPA)
> + dev_dbg(pp->dev, "dpa interrupt recived\n");
> +
> + if (reg & PCIE_CLIENT_INT_FATAL_ERR)
> + dev_dbg(pp->dev, "fatal error interrupt recived\n");
> +
> + if (reg & PCIE_CLIENT_INT_NFATAL_ERR)
> + dev_dbg(pp->dev, "no fatal error interrupt recived\n");
> +
> + if (reg & PCIE_CLIENT_INT_CORR_ERR)
> + dev_dbg(pp->dev, "correctable error interrupt recived\n");
> +
> + pcie_write(pp, reg, PCIE_CLIENT_INT_STATUS);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static void rockchip_pcie_legacy_int_handler(struct irq_desc *desc)
> +{
> + struct irq_chip *chip = irq_desc_get_chip(desc);
> + struct rockchip_pcie_port *port;
> + u32 reg;
> +
> + chained_irq_enter(chip, desc);
> + port = irq_desc_get_handler_data(desc);
> +
> + reg = pcie_read(port, PCIE_CLIENT_INT_STATUS);
> + reg = (reg & ROCKCHIP_PCIE_RPIFR1_INTR_MASK) >>
> + ROCKCHIP_PCIE_RPIFR1_INTR_SHIFT;
> + if (reg)
> + generic_handle_irq(irq_find_mapping(port->irq_domain,
> + ffs(reg)));
> + chained_irq_exit(chip, desc);
> +}
> +
> +static int rockchip_pcie_prog_ob_atu(struct rockchip_pcie_port *pp,
> + int region_no,
> + int type, u8 num_pass_bits,
> + u32 lower_addr, u32 upper_addr)
> +{
> + u32 ob_addr_0 = 0;
> + u32 ob_addr_1 = 0;
> + u32 ob_desc_0 = 0;
> + u32 ob_desc_1 = 0;
> + void __iomem *aw_base;
> +
> + if (!pp)
> + return -EINVAL;
> + if (region_no >= MAX_AXI_WRAPPER_REGION_NUM)
> + return -EINVAL;
> + if ((num_pass_bits + 1) < 8)
> + return -EINVAL;
> + if (num_pass_bits > 63)
> + return -EINVAL;
> + if (region_no == 0) {
> + if (AXI_REGION_0_SIZE < (2ULL << num_pass_bits))
> + return -EINVAL;
> + }
> + if (region_no != 0) {
> + if (AXI_REGION_SIZE < (2ULL << num_pass_bits))
> + return -EINVAL;
> + }
> + aw_base = pp->apb_base + PCIE_CORE_AXI_CONF_BASE;
> + aw_base += (region_no << OB_REG_SIZE_SHIFT);
> +
> + ob_addr_0 = (ob_addr_0 &
> + ~0x0000003fU) | (num_pass_bits &
> + 0x0000003fU);
> + ob_addr_0 = (ob_addr_0 &
> + ~0xffffff00U) | (lower_addr & 0xffffff00U);
> + ob_addr_1 = upper_addr;
> + ob_desc_0 = (1 << 23 | type);
> +
> + writel(ob_addr_0, aw_base);
> + writel(ob_addr_1, aw_base + PCIE_CORE_OB_REGION_ADDR1);
> + writel(ob_desc_0, aw_base + PCIE_CORE_OB_REGION_DESC0);
> + writel(ob_desc_1, aw_base + PCIE_CORE_OB_REGION_DESC1);
> +
> + return 0;
> +}
> +
> +static int rockchip_pcie_prog_ib_atu(struct rockchip_pcie_port *pp,
> + int region_no,
> + u8 num_pass_bits,
> + u32 lower_addr,
> + u32 upper_addr)
> +{
> + u32 ib_addr_0 = 0;
> + u32 ib_addr_1 = 0;
> + void __iomem *aw_base;
> +
> + if (!pp)
> + return -EINVAL;
> + if (region_no > MAX_AXI_IB_ROOTPORT_REGION_NUM)
> + return -EINVAL;
> + if ((num_pass_bits + 1) < MIN_AXI_ADDR_BITS_PASSED)
> + return -EINVAL;
> + if (num_pass_bits > 63)
> + return -EINVAL;
> + aw_base = pp->apb_base + PCIE_CORE_AXI_INBOUND_BASE;
> + aw_base += (region_no << IB_ROOT_PORT_REG_SIZE_SHIFT);
> + ib_addr_0 = (ib_addr_0 &
> + ~0x0000003fU) | (num_pass_bits &
> + 0x0000003fU);
> +
> + ib_addr_0 = (ib_addr_0 & ~0xffffff00U) |
> + ((lower_addr << 8) & 0xffffff00U);
> + ib_addr_1 = upper_addr;
> +
> + writel(ib_addr_0, aw_base);
> + writel(ib_addr_1, aw_base + PCIE_RP_IB_ADDR_TRANS);
> +
> + return 0;
> +}
> +
> +static int rockchip_pcie_probe(struct platform_device *pdev)
> +{
> + struct rockchip_pcie_port *port;
> + struct device *dev = &pdev->dev;
> + struct pci_bus *bus, *child;
> + struct resource_entry *win;
> + resource_size_t io_base;
> + struct resource *busn;
> + struct resource *mem;
> + struct resource *io;
> + phys_addr_t io_bus_addr;
> + u32 io_size;
> + phys_addr_t mem_bus_addr;
> + u32 mem_size;
> + int reg_no;
> + int err = 0;
> + int offset = 0;
> +
> + LIST_HEAD(res);
> +
> + if (!dev->of_node)
> + return -ENODEV;
> +
> + port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
> + if (!port)
> + return -ENOMEM;
> +
> + port->dev = dev;
> +
> + err = rockchip_pcie_parse_dt(port);
> + if (err)
> + return err;
> +
> + err = clk_prepare_enable(port->aclk_pcie);
> + if (err) {
> + dev_err(dev, "Unable to enable aclk_pcie clock.\n");
> + goto err_aclk_pcie;
> + }
> +
> + err = clk_prepare_enable(port->aclk_perf_pcie);
> + if (err) {
> + dev_err(dev, "Unable to enable aclk_perf_pcie clock.\n");
> + goto err_aclk_perf_pcie;
> + }
> +
> + err = clk_prepare_enable(port->hclk_pcie);
> + if (err) {
> + dev_err(dev, "Unable to enable hclk_pcie clock.\n");
> + goto err_hclk_pcie;
> + }
> +
> + err = clk_prepare_enable(port->clk_pcie_pm);
> + if (err) {
> + dev_err(dev, "Unable to enable hclk_pcie clock.\n");
> + goto err_pcie_pm;
> + }
> +
> + err = rockchip_pcie_set_vpcie(port);
> + if (err) {
> + dev_err(port->dev, "Fail to set vpcie regulator.\n");
> + goto err_set_vpcie;
> + }
> +
> + err = rockchip_pcie_init_port(port);
> + if (err)
> + goto err_vpcie;
> +
> + platform_set_drvdata(pdev, port);
> +
> + rockchip_pcie_enable_interrupts(port);
> +
> + err = rockchip_pcie_init_irq_domain(port);
> + if (err < 0)
> + goto err_vpcie;
> +
> + err = of_pci_get_host_bridge_resources(dev->of_node, 0, 0xff,
> + &res, &io_base);
> + if (err)
> + goto err_vpcie;
> +
> + /* Get the I/O and memory ranges from DT */
> + resource_list_for_each_entry(win, &res) {
> + switch (resource_type(win->res)) {
> + case IORESOURCE_IO:
> + io = win->res;
> + io->name = "I/O";
> + io_size = resource_size(io);
> + io_bus_addr = io->start - win->offset;
> + err = pci_remap_iospace(io, io_base);
> + if (err) {
> + dev_warn(port->dev, "error %d: failed to map resource %pR\n",
> + err, io);
> + continue;
> + }
> + break;
> + case IORESOURCE_MEM:
> + mem = win->res;
> + mem->name = "MEM";
> + mem_size = resource_size(mem);
> + mem_bus_addr = mem->start - win->offset;
> + break;
> + case 0:
Why case 0? That doesn't seem right.
> + break;
> + case IORESOURCE_BUS:
> + busn = win->res;
> + break;
> + default:
> + continue;
> + }
> + }
> +
> + for (reg_no = 0; reg_no < (mem_size >> 20); reg_no++) {
kbuildbot already complained about potential unused warnings here and
elsewhere. One problem that this highlights:
what happens if the expected resources aren't included in your device
tree? Seems like you'll just keep going, with uninitialized data.
Probably want some kind of checks to make sure we found the necessary
resources.
> + err = rockchip_pcie_prog_ob_atu(port, reg_no + 1,
> + AXI_WRAPPER_MEM_WRITE,
> + 20 - 1,
> + mem_bus_addr +
> + (reg_no << 20),
> + 0);
> + if (err) {
> + dev_err(dev, "Program RC mem outbound atu failed\n");
> + goto err_vpcie;
> + }
> + }
> +
> + err = rockchip_pcie_prog_ib_atu(port, 2, 32 - 1, 0x0, 0);
> + if (err) {
> + dev_err(dev, "Program RC mem inbound atu failed\n");
> + goto err_vpcie;
> + }
> +
> + offset = mem_size >> 20;
> +
> + for (reg_no = 0; reg_no < (io_size >> 20); reg_no++) {
> + err = rockchip_pcie_prog_ob_atu(port,
> + reg_no + 1 + offset,
> + AXI_WRAPPER_IO_WRITE,
> + 20 - 1,
> + io_bus_addr +
> + (reg_no << 20),
> + 0);
> + if (err) {
> + dev_err(dev, "Program RC io outbound atu failed\n");
> + goto err_vpcie;
> + }
> + }
> +
> + port->root_bus_nr = busn->start;
> +
> + bus = pci_scan_root_bus(&pdev->dev, 0,
> + &rockchip_pcie_ops, port, &res);
> +
> + if (!bus) {
> + err = -ENOMEM;
> + goto err_vpcie;
> + }
> +
> + pci_bus_size_bridges(bus);
> + pci_bus_assign_resources(bus);
> + list_for_each_entry(child, &bus->children, node)
> + pcie_bus_configure_settings(child);
Bad indentation: the above line is part of a loop, so it should be
indented one more.
> +
> + pci_bus_add_devices(bus);
> +
> + return err;
> +
> +err_vpcie:
> + if (!IS_ERR(port->vpcie3v3))
> + regulator_disable(port->vpcie3v3);
> + if (!IS_ERR(port->vpcie1v8))
> + regulator_disable(port->vpcie1v8);
> + if (!IS_ERR(port->vpcie0v9))
> + regulator_disable(port->vpcie0v9);
> +err_set_vpcie:
> + clk_disable_unprepare(port->clk_pcie_pm);
> +err_pcie_pm:
> + clk_disable_unprepare(port->hclk_pcie);
> +err_hclk_pcie:
> + clk_disable_unprepare(port->aclk_perf_pcie);
> +err_aclk_perf_pcie:
> + clk_disable_unprepare(port->aclk_pcie);
> +err_aclk_pcie:
> + return err;
> +}
> +
> +static const struct of_device_id rockchip_pcie_of_match[] = {
> + { .compatible = "rockchip,rk3399-pcie", },
> + {}
> +};
> +
> +static struct platform_driver rockchip_pcie_driver = {
> + .driver = {
> + .name = "rockchip-pcie",
> + .of_match_table = rockchip_pcie_of_match,
> + },
> + .probe = rockchip_pcie_probe,
> +
> +};
> +module_platform_driver(rockchip_pcie_driver);
> +
> +MODULE_AUTHOR("Rockchip Inc");
> +MODULE_DESCRIPTION("Rockchip AXI PCIe driver");
> +MODULE_LICENSE("GPL v2");
> --
> 2.3.7
Brian
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 2/2] PCI: Rockchip: Add Rockchip PCIe controller support
2016-06-23 0:29 ` Brian Norris
@ 2016-06-23 1:09 ` Shawn Lin
2016-06-23 1:35 ` Brian Norris
0 siblings, 1 reply; 6+ messages in thread
From: Shawn Lin @ 2016-06-23 1:09 UTC (permalink / raw)
To: Brian Norris
Cc: shawn.lin, Bjorn Helgaas, devicetree, Heiko Stuebner,
Arnd Bergmann, Marc Zyngier, linux-pci, Wenrui Li, linux-kernel,
Doug Anderson, linux-rockchip, Rob Herring
在 2016/6/23 8:29, Brian Norris 写道:
> Hi Shawn,
>
> I'm familiarizing myself a bit with your PCIe core, but I'm certainly no
> expert, so I may have some dumb comments. Also a few other general
> comments.
>
> On Thu, Jun 16, 2016 at 09:50:35AM +0800, Shawn Lin wrote:
>> This patch adds Rockchip PCIe controller support found
>> on RK3399 Soc platform.
>>
>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>>
>> ---
>>
>> Changes in v3:
>> - remove header file
>> - remove struct msi_controller and move most of variables
>> of rockchip_pcie_port to become the local ones.
>> - allow legacy int even if enabling MSI
>> - drop regulator set voltage operation suggested by Doug
>>
>> Changes in v2:
>> - remove phy related stuff and call phy API
>> - add new head file and define lots of macro to make
>> the code more readable
>> - remove lots msi related code suggested by Marc
>> - add IO window address translation
>> - init_port and parse_dt reconstruction suggested by Bharat
>> - improve wr_own_conf suggested by Arnd
>> - make pcie as an interrupt controller and fix wrong int handler
>> suggested by Marc
>> - remove PCI_PROBE_ONLY suggested by Lorenzo
>>
>> drivers/pci/host/Kconfig | 11 +
>> drivers/pci/host/Makefile | 1 +
>> drivers/pci/host/pcie-rockchip.c | 1183 ++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 1195 insertions(+)
>> create mode 100644 drivers/pci/host/pcie-rockchip.c
>>
>> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
>> index 5d2374e..251f4ac 100644
>> --- a/drivers/pci/host/Kconfig
>> +++ b/drivers/pci/host/Kconfig
>> @@ -245,4 +245,15 @@ config PCIE_ARMADA_8K
>> Designware hardware and therefore the driver re-uses the
>> Designware core functions to implement the driver.
>>
>> +config PCIE_ROCKCHIP
>> + bool "Rockchip PCIe controller"
>
> Any good reason this can't be a module? I realize PCIe is something we'd
> likely want built in most of the time, but is there a code reason?
If you want it to be tristate, I will update it.
>
>> + depends on ARM64 && ARCH_ROCKCHIP
>> + depends on OF
>> + select MFD_SYSCON
>> + select PCI_MSI_IRQ_DOMAIN if PCI_MSI
>> + help
>> + Say Y here if you want internal PCI support on Rockchip SoC.
>> + There are 1 internal PCIe port available to support GEN2 with
>
> s/are/is/
will fix.
>
>> + 4 slots.
>> +
>> endmenu
>> diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
>> index 9c8698e..c52985c 100644
>> --- a/drivers/pci/host/Makefile
>> +++ b/drivers/pci/host/Makefile
>> @@ -29,3 +29,4 @@ obj-$(CONFIG_PCIE_QCOM) += pcie-qcom.o
>> obj-$(CONFIG_PCI_HOST_THUNDER_ECAM) += pci-thunder-ecam.o
>> obj-$(CONFIG_PCI_HOST_THUNDER_PEM) += pci-thunder-pem.o
>> obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o
>> +obj-$(CONFIG_PCIE_ROCKCHIP) += pcie-rockchip.o
>> diff --git a/drivers/pci/host/pcie-rockchip.c b/drivers/pci/host/pcie-rockchip.c
>> new file mode 100644
>> index 0000000..661d6e0
>> --- /dev/null
>> +++ b/drivers/pci/host/pcie-rockchip.c
>> @@ -0,0 +1,1183 @@
>> +/*
>> + * Rockchip AXI PCIe host controller driver
>> + *
>> + * Copyright (c) 2016 Rockchip, Inc.
>> + *
>> + * Author: Shawn Lin <shawn.lin@rock-chips.com>
>> + * Wenrui Li <wenrui.li@rock-chips.com>
>> + *
>> + * Bits taken from Synopsys Designware Host controller driver and
>> + * ARM PCI Host generic driver.
>> + *
>> + * This program is free software: you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation, either version 2 of the License, or
>> + * (at your option) any later version.
>> + */
>
> Add a blank line here?
will fix it.
>
>> +#include <linux/clk.h>
>> +#include <linux/delay.h>
>> +#include <linux/gpio/consumer.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/irq.h>
>> +#include <linux/irqchip/chained_irq.h>
>> +#include <linux/irqdomain.h>
>> +#include <linux/kernel.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/module.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of_device.h>
>> +#include <linux/of_pci.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/of_irq.h>
>> +#include <linux/pci.h>
>> +#include <linux/pci_ids.h>
>> +#include <linux/phy/phy.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/reset.h>
>> +#include <linux/regmap.h>
>> +
>> +#define PCIE_CLIENT_BASE 0x0
>> +#define PCIE_RC_CONFIG_NORMAL_BASE 0x800000
>> +#define PCIE_RC_CONFIG_BASE 0xa00000
>> +#define PCIE_CORE_LINK_CTRL_STATUS 0x8000d0
>> +#define PCIE_CORE_CTRL_MGMT_BASE 0x900000
>> +#define PCIE_CORE_AXI_CONF_BASE 0xc00000
>> +#define PCIE_CORE_AXI_INBOUND_BASE 0xc00800
>> +#define PCIE_CLIENT_BASIC_STATUS0 0x44
>> +#define PCIE_CLIENT_BASIC_STATUS1 0x48
>> +#define PCIE_CLIENT_INT_MASK 0x4c
>> +#define PCIE_CLIENT_INT_STATUS 0x50
>> +#define PCIE_RC_CONFIG_RID_CCR 0x8
>> +#define PCIR_RC_CONFIG_LCS 0xd0
>
> s/PCIR/PCIE/
>
> ...right?
Ahh... my slippy finger, I will rename it.:)
>
>> +#define PCIE_RC_BAR_CONF 0x300
>> +#define PCIE_CORE_OB_REGION_ADDR1 0x4
>> +#define PCIE_CORE_OB_REGION_DESC0 0x8
>> +#define PCIE_CORE_OB_REGION_DESC1 0xc
>> +#define PCIE_RP_IB_ADDR_TRANS 0x4
>> +#define PCIE_CORE_INT_MASK 0x900210
>> +#define PCIE_CORE_INT_STATUS 0x90020c
>> +
>> +/* Size of one AXI Region (not Region 0) */
>> +#define AXI_REGION_SIZE BIT(20)
>> +/* Size of Region 0, equal to sum of sizes of other regions */
>> +#define AXI_REGION_0_SIZE (32 * (0x1 << 20))
>> +#define OB_REG_SIZE_SHIFT 5
>> +#define IB_ROOT_PORT_REG_SIZE_SHIFT 3
>> +#define AXI_WRAPPER_IO_WRITE 0x6
>> +#define AXI_WRAPPER_MEM_WRITE 0x2
>> +#define MAX_AXI_IB_ROOTPORT_REGION_NUM 3
>> +#define MIN_AXI_ADDR_BITS_PASSED 8
>> +#define ROCKCHIP_VENDOR_ID 0x1d87
>> +#define PCIE_ECAM_BUS(x) (((x) & 0xFF) << 20)
>> +#define PCIE_ECAM_DEV(x) (((x) & 0x1F) << 15)
>> +#define PCIE_ECAM_FUNC(x) (((x) & 0x7) << 12)
>> +#define PCIE_ECAM_REG(x) (((x) & 0xFFF) << 0)
>> +#define PCIE_ECAM_ADDR(bus, dev, func, reg) \
>> + (PCIE_ECAM_BUS(bus) | PCIE_ECAM_DEV(dev) | \
>> + PCIE_ECAM_FUNC(func) | PCIE_ECAM_REG(reg))
>> +
>> +/*
>> + * The higher 16-bit of this register is used for write protection
>> + * only if BIT(x + 16) set to 1 the BIT(x) can be written.
>> + */
>> +#define HIWORD_UPDATE(val, mask, shift) \
>> + ((val) << (shift) | (mask) << ((shift) + 16))
>> +
>> +#define RC_REGION_0_ADDR_TRANS_H 0x00000000
>> +#define RC_REGION_0_ADDR_TRANS_L 0x00000000
>> +#define RC_REGION_0_PASS_BITS (25 - 1)
>> +#define RC_REGION_1_ADDR_TRANS_H 0x00000000
>> +#define RC_REGION_1_ADDR_TRANS_L 0x00400000
>> +#define RC_REGION_1_PASS_BITS (20 - 1)
>> +#define MAX_AXI_WRAPPER_REGION_NUM 33
>> +#define PCIE_CORE_LCSR_RETAIN_LINK BIT(5)
>
> s/RETAIN/RETRAIN/
will fix.
>
>> +#define PCIE_CLIENT_CONF_ENABLE 1
>> +#define PCIE_CLIENT_LINK_TRAIN_ENABLE 1
>> +#define PCIE_CLIENT_ARI_ENABLE 1
>> +#define PCIE_CLIENT_CONF_LANE_NUM(x) (x / 2)
>> +#define PCIE_CLIENT_MODE_RC 1
>> +#define PCIE_CLIENT_GEN_SEL_2 1
>> +#define PCIE_CLIENT_GEN_SEL_1 0
>> +#define PCIE_CLIENT_CONF_ENABLE_SHIFT 0
>> +#define PCIE_CLIENT_CONF_ENABLE_MASK 0x1
>
> It's quite confusing (IMO) having the values (e.g.,
> PCIE_CLIENT_CONF_ENABLE) separated from the bitfield SHIFT/MASK macros.
> Can you reorganize so they're listed next to each other?
>
okay.
>> +#define PCIE_CLIENT_LINK_TRAIN_SHIFT 1
>> +#define PCIE_CLIENT_LINK_TRAIN_MASK 0x1
>> +#define PCIE_CLIENT_ARI_ENABLE_SHIFT 3
>> +#define PCIE_CLIENT_ARI_ENABLE_MASK 0x1
>> +#define PCIE_CLIENT_CONF_LANE_NUM_SHIFT 4
>> +#define PCIE_CLIENT_CONF_LANE_NUM_MASK 0x3
>> +#define PCIE_CLIENT_MODE_SHIFT 6
>> +#define PCIE_CLIENT_MODE_MASK 0x1
>> +#define PCIE_CLIENT_GEN_SEL_SHIFT 7
>> +#define PCIE_CLIENT_GEN_SEL_MASK 0x1
>> +#define PCIE_CLIENT_LINK_STATUS_UP 0x3
>> +#define PCIE_CLIENT_LINK_STATUS_SHIFT 20
>> +#define PCIE_CLIENT_LINK_STATUS_MASK 0x3
>> +#define PCIE_CORE_PL_CONF_SPEED_25G 0x0
>> +#define PCIE_CORE_PL_CONF_SPEED_50G 0x1
>> +#define PCIE_CORE_PL_CONF_SPEED_80G 0x2
>
> These speeds are not actually 25/50/80G, they're 2.5/5.0/8.0G. Maybe you
> can either add an underscore (_) where the decimal would be? And
> optionally drop the 0's? Like:
sounds good.
>
> #define PCIE_CORE_PL_CONF_SPEED_2_5G 0x0
> #define PCIE_CORE_PL_CONF_SPEED_5G 0x1
> #define PCIE_CORE_PL_CONF_SPEED_8G 0x2
>
> I'd be ashamed to admit how much time I spent confused as to where you
> were getting 25, 50, and 80 from...
>
>> +#define PCIE_CORE_PL_CONF_SPEED_SHIFT 3
>> +#define PCIE_CORE_PL_CONF_SPEED_MASK 0x3
>> +#define PCIE_CORE_PL_CONF_LANE_SHIFT 1
>> +#define PCIE_CORE_PL_CONF_LANE_MASK 0x3
>> +#define PCIE_CORE_RC_CONF_SCC_SHIFT 16
>> +
>> +/* PCIE_CLIENT_INT_STATUS */
>> +#define PCIE_CLIENT_INT_LEGACY_DONE BIT(15)
>> +#define PCIE_CLIENT_INT_MSG BIT(14)
>> +#define PCIE_CLIENT_INT_HOT_RST BIT(13)
>> +#define PCIE_CLIENT_INT_DPA BIT(12)
>> +#define PCIE_CLIENT_INT_FATAL_ERR BIT(11)
>> +#define PCIE_CLIENT_INT_NFATAL_ERR BIT(10)
>> +#define PCIE_CLIENT_INT_CORR_ERR BIT(9)
>> +#define PCIE_CLIENT_INT_INTD BIT(8)
>> +#define PCIE_CLIENT_INT_INTC BIT(7)
>> +#define PCIE_CLIENT_INT_INTB BIT(6)
>> +#define PCIE_CLIENT_INT_INTA BIT(5)
>> +#define PCIE_CLIENT_INT_LOCAL BIT(4)
>> +#define PCIE_CLIENT_INT_UDMA BIT(3)
>> +#define PCIE_CLIENT_INT_PHY BIT(2)
>> +#define PCIE_CLIENT_INT_HOT_PLUG BIT(1)
>> +#define PCIE_CLIENT_INT_PWR_STCG BIT(0)
>> +#define PCIE_CORE_INT_PRFPE BIT(0)
>> +#define PCIE_CORE_INT_CRFPE BIT(1)
>> +#define PCIE_CORE_INT_RRPE BIT(2)
>> +#define PCIE_CORE_INT_PRFO BIT(3)
>> +#define PCIE_CORE_INT_CRFO BIT(4)
>> +#define PCIE_CORE_INT_RT BIT(5)
>> +#define PCIE_CORE_INT_RTR BIT(6)
>> +#define PCIE_CORE_INT_PE BIT(7)
>> +#define PCIE_CORE_INT_MTR BIT(8)
>> +#define PCIE_CORE_INT_UCR BIT(9)
>> +#define PCIE_CORE_INT_FCE BIT(10)
>> +#define PCIE_CORE_INT_CT BIT(11)
>> +#define PCIE_CORE_INT_UTC BIT(18)
>> +#define PCIE_CORE_INT_MMVC BIT(19)
>> +
>> +#define ROCKCHIP_PCIE_RPIFR1_INTR_MASK GENMASK(8, 5)
>> +#define ROCKCHIP_PCIE_RPIFR1_INTR_SHIFT 5
>> +
>> +#define PCIE_CORE_INT \
>> + (PCIE_CORE_INT_PRFPE | PCIE_CORE_INT_CRFPE | \
>> + PCIE_CORE_INT_RRPE | PCIE_CORE_INT_CRFO | \
>> + PCIE_CORE_INT_RT | PCIE_CORE_INT_RTR | \
>> + PCIE_CORE_INT_PE | PCIE_CORE_INT_MTR | \
>> + PCIE_CORE_INT_UCR | PCIE_CORE_INT_FCE | \
>> + PCIE_CORE_INT_CT | PCIE_CORE_INT_UTC | \
>> + PCIE_CORE_INT_MMVC)
>> +
>> +#define PCIE_CLIENT_INT_SUBSYSTEM \
>> + (PCIE_CLIENT_INT_PWR_STCG | PCIE_CLIENT_INT_HOT_PLUG | \
>> + PCIE_CLIENT_INT_PHY | PCIE_CLIENT_INT_UDMA | \
>> + PCIE_CLIENT_INT_LOCAL)
>> +
>> +#define PCIE_CLIENT_INT_LEGACY \
>> + (PCIE_CLIENT_INT_INTA | PCIE_CLIENT_INT_INTB | \
>> + PCIE_CLIENT_INT_INTC | PCIE_CLIENT_INT_INTD)
>> +
>> +#define PCIE_CLIENT_INT_CLI \
>> + (PCIE_CLIENT_INT_CORR_ERR | PCIE_CLIENT_INT_NFATAL_ERR | \
>> + PCIE_CLIENT_INT_FATAL_ERR | PCIE_CLIENT_INT_DPA | \
>> + PCIE_CLIENT_INT_HOT_RST | PCIE_CLIENT_INT_MSG | \
>> + PCIE_CLIENT_INT_LEGACY_DONE | PCIE_CLIENT_INT_LEGACY)
>> +
>> +struct rockchip_pcie_port {
>> + void __iomem *reg_base;
>> + void __iomem *apb_base;
>> + struct phy *phy;
>> + struct reset_control *core_rst;
>> + struct reset_control *mgmt_rst;
>> + struct reset_control *mgmt_sticky_rst;
>> + struct reset_control *pipe_rst;
>> + struct clk *aclk_pcie;
>> + struct clk *aclk_perf_pcie;
>> + struct clk *hclk_pcie;
>> + struct clk *clk_pcie_pm;
>> + struct regulator *vpcie3v3; /* 3.3V power supply */
>> + struct regulator *vpcie1v8; /* 1.8V power supply */
>> + struct regulator *vpcie0v9; /* 0.9V power supply */
>> + struct gpio_desc *ep_gpio;
>> + u32 lanes;
>> + u8 root_bus_nr;
>> + struct device *dev;
>> + struct irq_domain *irq_domain;
>> +};
>> +
>> +static irqreturn_t rockchip_pcie_subsys_irq_handler(int irq, void *arg);
>> +static irqreturn_t rockchip_pcie_client_irq_handler(int irq, void *arg);
>> +static void rockchip_pcie_legacy_int_handler(struct irq_desc *desc);
>> +
>> +static inline u32 pcie_read(struct rockchip_pcie_port *port, u32 reg)
>> +{
>> + return readl(port->apb_base + reg);
>> +}
>> +
>> +static inline void pcie_write(struct rockchip_pcie_port *port,
>> + u32 val, u32 reg)
>> +{
>> + writel(val, port->apb_base + reg);
>> +}
>> +
>> +static int rockchip_pcie_valid_config(struct rockchip_pcie_port *pp,
>> + struct pci_bus *bus, int dev)
>> +{
>> + /* access only one slot on each root port */
>> + if (bus->number == pp->root_bus_nr && dev > 0)
>> + return 0;
>> +
>> + /*
>> + * do not read more than one device on the bus directly attached
>> + * to RC's (Virtual Bridge's) DS side.
>> + */
>> + if (bus->primary == pp->root_bus_nr && dev > 0)
>> + return 0;
>> +
>> + return 1;
>> +}
>> +
>> +static int rockchip_pcie_rd_own_conf(struct rockchip_pcie_port *pp,
>> + int where, int size,
>> + u32 *val)
>> +{
>> + void __iomem *addr = pp->apb_base + PCIE_RC_CONFIG_BASE + where;
>> +
>> + if (!IS_ALIGNED((uintptr_t)addr, size)) {
>> + *val = 0;
>> + return PCIBIOS_BAD_REGISTER_NUMBER;
>> + }
>> +
>> + if (size == 4) {
>> + *val = readl(addr);
>> + } else if (size == 2) {
>> + *val = readw(addr);
>> + } else if (size == 1) {
>> + *val = readb(addr);
>> + } else {
>> + *val = 0;
>> + return PCIBIOS_BAD_REGISTER_NUMBER;
>> + }
>> + return PCIBIOS_SUCCESSFUL;
>> +}
>> +
>> +static int rockchip_pcie_wr_own_conf(struct rockchip_pcie_port *pp,
>> + int where, int size, u32 val)
>> +{
>> + u32 mask, tmp, offset;
>> +
>> + offset = (where & (~0x3));
>> +
>> + if (size == 4) {
>> + writel(val, pp->apb_base + PCIE_RC_CONFIG_BASE + offset);
>> + return PCIBIOS_SUCCESSFUL;
>> + }
>> +
>> + mask = ~(((1 << (size * 8)) - 1) << ((where & 0x3) * 8));
>> +
>> + tmp = readl(pp->apb_base + PCIE_RC_CONFIG_BASE + offset) & mask;
>> + tmp |= val << ((where & 0x3) * 8);
>> + writel(tmp, pp->apb_base + PCIE_RC_CONFIG_BASE + offset);
>> +
>> + return PCIBIOS_SUCCESSFUL;
>> +}
>> +
>> +static int rockchip_pcie_rd_other_conf(struct rockchip_pcie_port *pp,
>> + struct pci_bus *bus, u32 devfn,
>> + int where, int size, u32 *val)
>> +{
>> + u32 busdev;
>> +
>> + busdev = PCIE_ECAM_ADDR(bus->number, PCI_SLOT(devfn),
>> + PCI_FUNC(devfn), where);
>> +
>> + if (!IS_ALIGNED(busdev, size)) {
>> + *val = 0;
>> + return PCIBIOS_BAD_REGISTER_NUMBER;
>> + }
>> +
>> + if (size == 4) {
>> + *val = readl(pp->reg_base + busdev);
>> + } else if (size == 2) {
>> + *val = readw(pp->reg_base + busdev);
>> + } else if (size == 1) {
>> + *val = readb(pp->reg_base + busdev);
>> + } else {
>> + *val = 0;
>> + return PCIBIOS_BAD_REGISTER_NUMBER;
>> + }
>> + return PCIBIOS_SUCCESSFUL;
>> +}
>> +
>> +static int rockchip_pcie_wr_other_conf(struct rockchip_pcie_port *pp,
>> + struct pci_bus *bus, u32 devfn,
>> + int where, int size, u32 val)
>> +{
>> + u32 busdev;
>> +
>> + busdev = PCIE_ECAM_ADDR(bus->number, PCI_SLOT(devfn),
>> + PCI_FUNC(devfn), where);
>> + if (!IS_ALIGNED(busdev, size))
>> + return PCIBIOS_BAD_REGISTER_NUMBER;
>> +
>> + if (size == 4)
>> + writel(val, pp->reg_base + busdev);
>> + else if (size == 2)
>> + writew(val, pp->reg_base + busdev);
>> + else if (size == 1)
>> + writeb(val, pp->reg_base + busdev);
>> + else
>> + return PCIBIOS_BAD_REGISTER_NUMBER;
>> +
>> + return PCIBIOS_SUCCESSFUL;
>> +}
>> +
>> +static int rockchip_pcie_rd_conf(struct pci_bus *bus, u32 devfn, int where,
>> + int size, u32 *val)
>> +{
>> + struct rockchip_pcie_port *pp = bus->sysdata;
>> + int ret;
>> +
>> + if (rockchip_pcie_valid_config(pp, bus, PCI_SLOT(devfn)) == 0) {
>> + *val = 0xffffffff;
>> + return PCIBIOS_DEVICE_NOT_FOUND;
>> + }
>> +
>> + if (bus->number != pp->root_bus_nr)
>> + ret = rockchip_pcie_rd_other_conf(pp, bus, devfn,
>> + where, size, val);
>> + else
>> + ret = rockchip_pcie_rd_own_conf(pp, where, size, val);
>> +
>> + return ret;
>> +}
>> +
>> +static int rockchip_pcie_wr_conf(struct pci_bus *bus, u32 devfn,
>> + int where, int size, u32 val)
>> +{
>> + struct rockchip_pcie_port *pp = bus->sysdata;
>> + int ret;
>> +
>> + if (rockchip_pcie_valid_config(pp, bus, PCI_SLOT(devfn)) == 0)
>> + return PCIBIOS_DEVICE_NOT_FOUND;
>> +
>> + if (bus->number != pp->root_bus_nr)
>> + ret = rockchip_pcie_wr_other_conf(pp, bus, devfn,
>> + where, size, val);
>> + else
>> + ret = rockchip_pcie_wr_own_conf(pp, where, size, val);
>> +
>> + return ret;
>> +}
>> +
>> +static struct pci_ops rockchip_pcie_ops = {
>> + .read = rockchip_pcie_rd_conf,
>> + .write = rockchip_pcie_wr_conf,
>> +};
>> +
>> +/**
>> + * rockchip_pcie_init_port - Initialize hardware
>> + * @port: PCIe port information
>> + */
>> +static int rockchip_pcie_init_port(struct rockchip_pcie_port *port)
>> +{
>> + int err;
>> + u32 status;
>> + unsigned long timeout;
>> +
>> + gpiod_set_value(port->ep_gpio, 0);
>> +
>> + err = phy_init(port->phy);
>> + if (err < 0)
>> + dev_err(port->dev, "fail to init phy, err %d\n", err);
>
> Are you intentionally continuing, even on error? Seems like you should
> 'return ret'.
Good catch, thanks.
>
>> +
>> + err = reset_control_assert(port->core_rst);
>> + if (err) {
>> + dev_err(port->dev, "assert core_rst err %d\n", err);
>> + return err;
>> + }
>> +
>> + err = reset_control_assert(port->mgmt_rst);
>> + if (err) {
>> + dev_err(port->dev, "assert mgmt_rst err %d\n", err);
>> + return err;
>> + }
>> +
>> + err = reset_control_assert(port->mgmt_sticky_rst);
>> + if (err) {
>> + dev_err(port->dev, "assert mgmt_sticky_rst err %d\n", err);
>> + return err;
>> + }
>> +
>> + err = reset_control_assert(port->pipe_rst);
>> + if (err) {
>> + dev_err(port->dev, "assert pipe_rst err %d\n", err);
>> + return err;
>> + }
>> +
>> + pcie_write(port,
>> + HIWORD_UPDATE(PCIE_CLIENT_CONF_ENABLE,
>> + PCIE_CLIENT_CONF_ENABLE_MASK,
>> + PCIE_CLIENT_CONF_ENABLE_SHIFT) |
>> + HIWORD_UPDATE(PCIE_CLIENT_CONF_LANE_NUM(port->lanes),
>> + PCIE_CLIENT_CONF_LANE_NUM_MASK,
>> + PCIE_CLIENT_CONF_LANE_NUM_SHIFT) |
>> + HIWORD_UPDATE(PCIE_CLIENT_MODE_RC,
>> + PCIE_CLIENT_MODE_MASK,
>> + PCIE_CLIENT_MODE_SHIFT) |
>> + HIWORD_UPDATE(PCIE_CLIENT_ARI_ENABLE,
>> + PCIE_CLIENT_ARI_ENABLE_MASK,
>> + PCIE_CLIENT_ARI_ENABLE_SHIFT) |
>> + HIWORD_UPDATE(PCIE_CLIENT_GEN_SEL_2,
>> + PCIE_CLIENT_GEN_SEL_MASK,
>> + PCIE_CLIENT_GEN_SEL_SHIFT),
>> + PCIE_CLIENT_BASE);
>> +
>> + err = phy_power_on(port->phy);
>> + if (err)
>> + dev_err(port->dev, "fail to power on phy, err %d\n", err);
>
> Same here.
>
>> +
>> + err = reset_control_deassert(port->core_rst);
>> + if (err) {
>> + dev_err(port->dev, "deassert core_rst err %d\n", err);
>> + return err;
>> + }
>> +
>> + err = reset_control_deassert(port->mgmt_rst);
>> + if (err) {
>> + dev_err(port->dev, "deassert mgmt_rst err %d\n", err);
>> + return err;
>> + }
>> +
>> + err = reset_control_deassert(port->mgmt_sticky_rst);
>> + if (err) {
>> + dev_err(port->dev, "deassert mgmt_sticky_rst err %d\n", err);
>> + return err;
>> + }
>> +
>> + err = reset_control_deassert(port->pipe_rst);
>> + if (err) {
>> + dev_err(port->dev, "deassert pipe_rst err %d\n", err);
>> + return err;
>> + }
>> +
>> + /* Enable Gen1 trainning */
>
> s/nn/n/
>
>> + pcie_write(port,
>> + HIWORD_UPDATE(PCIE_CLIENT_LINK_TRAIN_ENABLE,
>> + PCIE_CLIENT_LINK_TRAIN_MASK,
>> + PCIE_CLIENT_LINK_TRAIN_SHIFT),
>> + PCIE_CLIENT_BASE);
>> +
>> + gpiod_set_value(port->ep_gpio, 1);
>> +
>> + /* 500ms timeout value should be enough for gen1/2 taining */
>> + timeout = jiffies + msecs_to_jiffies(500);
>> +
>> + err = -ETIMEDOUT;
>> + while (time_before(jiffies, timeout)) {
>> + status = pcie_read(port, PCIE_CLIENT_BASIC_STATUS1);
>> + if (((status >> PCIE_CLIENT_LINK_STATUS_SHIFT) &
>> + PCIE_CLIENT_LINK_STATUS_MASK) ==
>> + PCIE_CLIENT_LINK_STATUS_UP) {
>> + dev_dbg(port->dev, "pcie link training gen1 pass!\n");
>> + err = 0;
>> + break;
>> + }
>> + msleep(20);
>> + }
>
> Technically, the above timeout loop is not quite correct. Possible error
> case: we can fail with a timeout after 500 ms if the training completes
> between the 480-500 ms time window. This can happen because you're
> doing:
>
> (1) read register: if complete, then terminate successfully
> (2) delay
> (3) check for timeout: if time is up, return error
>
> You actually need:
>
> (1) read register: if complete, then terminate successfully
> (2) delay
> (3) check for timeout: if time is up, repeat (1), and then report error
>
> You can examine the logic for readx_poll_timeout() in
> include/linux/iopoll.h to see an example of a proper timeout loop. You
> could even try to use one of the helpers there, except that your
> pcie_read() takes 2 args.
I see, thanks.
>
>> + if (err) {
>> + dev_err(port->dev, "pcie link training gen1 timeout!\n");
>> + return err;
>> + }
>> +
>> + /*
>> + * Enable retrain for gen2. This should be configured only after
>> + * gen1 finished.
>> + */
>> + status = pcie_read(port,
>> + PCIR_RC_CONFIG_LCS + PCIE_RC_CONFIG_BASE);
>> + status |= PCIE_CORE_LCSR_RETAIN_LINK;
>> + pcie_write(port, status,
>> + PCIR_RC_CONFIG_LCS + PCIE_RC_CONFIG_BASE);
>
> I'm not really an expert on this, but how are you actually "retraining
> for gen2"? Is that just the behavior of the core, that it retries at the
> higher rate on the 2nd training attempt? I'm just confused, since you
> set PCIE_CLIENT_GEN_SEL_2 above, so you've already allowed either gen1
> or gen2 negotiation AFAICT, and so seemingly you might already have
> negotiated at gen2.
Not really. I allow the core to enable gen2, but it needs a extra
setting of retrain after finishing gen1. It's not so strange as it
depends on the vendor's design. So I have to say it fits the
designer's expectation.
>
>> + err = -ETIMEDOUT;
>> +
>> + while (time_before(jiffies, timeout)) {
>
> You never reset 'timeout' when starting this loop. So you only have a
> cumulative 500 ms to do both the gen1 and gen2 loops. Is that
> intentional? (I feel like this comment was made on an earlier revision
> of this driver, though I haven't read everything thoroughly.)
yes, I don't have any docs to let me know how long should I wait for
gen1/2 to be finished. Maybe someday it will be augmented to a larger
value if finding a device actually need a longer time. But the only
thing I can say is that it's from my test for many pcie devices
currently.
Do you agree?
>
>> + status = pcie_read(port, PCIE_CORE_CTRL_MGMT_BASE);
>> + if (((status >> PCIE_CORE_PL_CONF_SPEED_SHIFT) &
>> + PCIE_CORE_PL_CONF_SPEED_MASK) ==
>> + PCIE_CORE_PL_CONF_SPEED_50G) {
>> + dev_dbg(port->dev, "pcie link training gen2 pass!\n");
>> + err = 0;
>> + break;
>> + }
>> + msleep(20);
>> + }
>
> Similar problem with your timeout loop, as mentioned above. Although I
> confused about what you do in the error case here...
>
>> + if (err)
>> + dev_dbg(port->dev, "pcie link training gen2 timeout, force to gen1!\n");
>
> ... how are you forcing gen1? I don't see any code that indicates this.
> Should you at least be checking that there aren't some kind of training
> errors, and that we settled back on gen1/2.5G?
yes, when failing gen2, my pcie controller will fallback to gen1
automatically.
if we pass the gen1 then fail to train gen2, a dbg msg here is enough
here to let we know that we should check the HW signal. Of course we
should make sure that this device supports gen2.
>
>> +
>> + /* Check the final link with from negotiated lane counter from MGMT */
>> + status = pcie_read(port, PCIE_CORE_CTRL_MGMT_BASE);
>> + status = 0x1 << ((status >> PCIE_CORE_PL_CONF_LANE_SHIFT) &
>> + PCIE_CORE_PL_CONF_LANE_MASK);
>> + dev_dbg(port->dev, "current link width is x%d\n", status);
>> +
>> + pcie_write(port, ROCKCHIP_VENDOR_ID, PCIE_RC_CONFIG_BASE);
>> + pcie_write(port, PCI_CLASS_BRIDGE_PCI << PCIE_CORE_RC_CONF_SCC_SHIFT,
>> + PCIE_RC_CONFIG_BASE + PCIE_RC_CONFIG_RID_CCR);
>> + pcie_write(port, 0x0, PCIE_CORE_CTRL_MGMT_BASE + PCIE_RC_BAR_CONF);
>> +
>> + pcie_write(port, (RC_REGION_0_ADDR_TRANS_L + RC_REGION_0_PASS_BITS),
>> + PCIE_CORE_AXI_CONF_BASE);
>> + pcie_write(port, RC_REGION_0_ADDR_TRANS_H,
>> + PCIE_CORE_AXI_CONF_BASE + PCIE_CORE_OB_REGION_ADDR1);
>> + pcie_write(port, 0x0080000a,
>> + PCIE_CORE_AXI_CONF_BASE + PCIE_CORE_OB_REGION_DESC0);
>> + pcie_write(port, 0x0,
>> + PCIE_CORE_AXI_CONF_BASE + PCIE_CORE_OB_REGION_DESC1);
>> +
>> + return 0;
>> +}
>> +
>> +/**
>> + * rockchip_pcie_parse_dt - Parse Device tree
>> + * @port: PCIe port information
>> + *
>> + * Return: '0' on success and error value on failure
>> + */
>> +static int rockchip_pcie_parse_dt(struct rockchip_pcie_port *port)
>> +{
>> + struct device *dev = port->dev;
>> + struct platform_device *pdev = to_platform_device(dev);
>> + struct device_node *node = dev->of_node;
>> + struct resource *regs;
>> + int irq;
>> + int err = -ENODEV;
>> +
>> + regs = platform_get_resource_byname(pdev,
>> + IORESOURCE_MEM,
>> + "axi-base");
>> + if (!regs) {
>> + dev_err(dev, "missing axi-base property\n");
>> + return err;
>> + }
>> +
>> + port->reg_base = devm_ioremap_resource(dev, regs);
>> + if (IS_ERR(port->reg_base))
>> + return PTR_ERR(port->reg_base);
>> +
>> + regs = platform_get_resource_byname(pdev,
>> + IORESOURCE_MEM,
>> + "apb-base");
>> + if (!regs) {
>> + dev_err(dev, "missing apb-base property\n");
>> + return err;
>> + }
>> +
>> + port->apb_base = devm_ioremap_resource(dev, regs);
>> + if (IS_ERR(port->apb_base))
>> + return PTR_ERR(port->apb_base);
>> +
>> + port->phy = devm_phy_get(dev, "pcie-phy");
>> + if (IS_ERR_OR_NULL(port->phy)) {
>> + if (PTR_ERR_OR_ZERO(port->phy) != -EPROBE_DEFER)
>> + dev_err(dev, "Missing phy\n");
>> + return PTR_ERR(port->phy);
>
> You're handling the port->phy == NULL case kind of inconsistently. On
> one hand, I don't think phy_get() can return NULL, so you might not need
> to handle it (i.e., the IS_ERR_OR_NULL() -> IS_ERR()) -- and the phy API
> can just ignore NULL PHY. But on the other hand, if you do want to
> handle NULL here, you probably shouldn't return PTR_ERR(NULL) here
> (i.e., 0, which means success).
>
Brilliant.. I will fix it.
> My recommendation would be:
>
> s/IS_ERR_OR_NULL/IS_ERR/
> s/PTR_ERR_OR_ZERO/PTR_ERR/
>
> for the above 4 lines.
>
>> + }
>> +
>> + port->lanes = 1;
>> + err = of_property_read_u32(node, "num-lanes", &port->lanes);
>> + if (!err && ((port->lanes == 0) ||
>> + (port->lanes == 3) ||
>> + (port->lanes > 4))) {
>> + dev_warn(dev, "invalid num-lanes, default use one lane\n");
>> + port->lanes = 1;
>> + }
>> +
>> + port->core_rst = devm_reset_control_get(dev, "core");
>> + if (IS_ERR(port->core_rst)) {
>> + if (PTR_ERR(port->core_rst) != -EPROBE_DEFER)
>> + dev_err(dev, "missing core rst property in node %s\n",
>> + node->name);
>> + return PTR_ERR(port->core_rst);
>> + }
>> +
>> + port->mgmt_rst = devm_reset_control_get(dev, "mgmt");
>> + if (IS_ERR(port->mgmt_rst)) {
>> + if (PTR_ERR(port->mgmt_rst) != -EPROBE_DEFER)
>> + dev_err(dev, "missing mgmt rst property in node %s\n",
>> + node->name);
>> + return PTR_ERR(port->mgmt_rst);
>> + }
>> +
>> + port->mgmt_sticky_rst = devm_reset_control_get(dev, "mgmt-sticky");
>> + if (IS_ERR(port->mgmt_sticky_rst)) {
>> + if (PTR_ERR(port->mgmt_sticky_rst) != -EPROBE_DEFER)
>> + dev_err(dev, "missing mgmt-sticky rst property in node %s\n",
>> + node->name);
>> + return PTR_ERR(port->mgmt_sticky_rst);
>> + }
>> +
>> + port->pipe_rst = devm_reset_control_get(dev, "pipe");
>> + if (IS_ERR(port->pipe_rst)) {
>> + if (PTR_ERR(port->pipe_rst) != -EPROBE_DEFER)
>> + dev_err(dev, "missing pipe rst property in node %s\n",
>> + node->name);
>> + return PTR_ERR(port->pipe_rst);
>> + }
>> +
>> + port->ep_gpio = devm_gpiod_get(dev, "ep", GPIOD_OUT_HIGH);
>> + if (IS_ERR(port->ep_gpio)) {
>> + dev_err(dev, "missing ep-gpios property in node %s\n",
>> + node->name);
>> + return PTR_ERR(port->ep_gpio);
>> + }
>> +
>> + port->aclk_pcie = devm_clk_get(dev, "aclk");
>> + if (IS_ERR(port->aclk_pcie)) {
>> + dev_err(dev, "aclk clock not found.\n");
>> + return PTR_ERR(port->aclk_pcie);
>> + }
>> +
>> + port->aclk_perf_pcie = devm_clk_get(dev, "aclk-perf");
>> + if (IS_ERR(port->aclk_perf_pcie)) {
>> + dev_err(dev, "aclk_perf clock not found.\n");
>> + return PTR_ERR(port->aclk_perf_pcie);
>> + }
>> +
>> + port->hclk_pcie = devm_clk_get(dev, "hclk");
>> + if (IS_ERR(port->hclk_pcie)) {
>> + dev_err(dev, "hclk clock not found.\n");
>> + return PTR_ERR(port->hclk_pcie);
>> + }
>> +
>> + port->clk_pcie_pm = devm_clk_get(dev, "pm");
>> + if (IS_ERR(port->clk_pcie_pm)) {
>> + dev_err(dev, "pm clock not found.\n");
>> + return PTR_ERR(port->clk_pcie_pm);
>> + }
>> +
>> + irq = platform_get_irq_byname(pdev, "sys");
>> + if (irq < 0) {
>> + dev_err(dev, "missing pcie_sys IRQ resource\n");
>> + return -EINVAL;
>> + }
>> +
>> + err = devm_request_irq(dev, irq, rockchip_pcie_subsys_irq_handler,
>> + IRQF_SHARED, "pcie-sys", port);
>> + if (err) {
>> + dev_err(dev, "failed to request pcie subsystem irq\n");
>> + return err;
>> + }
>> +
>> + irq = platform_get_irq_byname(pdev, "legacy");
>> + if (irq < 0) {
>> + dev_err(dev, "missing pcie_legacy IRQ resource\n");
>> + return -EINVAL;
>> + }
>> +
>> + irq_set_chained_handler_and_data(irq,
>> + rockchip_pcie_legacy_int_handler,
>> + port);
>> +
>> + irq = platform_get_irq_byname(pdev, "client");
>> + if (irq < 0) {
>> + dev_err(dev, "missing pcie-client IRQ resource\n");
>> + return -EINVAL;
>> + }
>> +
>> + err = devm_request_irq(dev, irq, rockchip_pcie_client_irq_handler,
>> + IRQF_SHARED, "pcie-client", port);
>> + if (err) {
>> + dev_err(dev, "failed to request pcie client irq\n");
>> + return err;
>> + }
>> +
>> + port->vpcie3v3 = devm_regulator_get_optional(dev, "vpcie3v3");
>> + if (IS_ERR(port->vpcie3v3)) {
>> + if (PTR_ERR(port->vpcie3v3) == -EPROBE_DEFER)
>> + return -EPROBE_DEFER;
>> + dev_info(dev, "No vpcie3v3 regulator found.\n");
>> + }
>> +
>> + port->vpcie1v8 = devm_regulator_get_optional(dev, "vpcie1v8");
>> + if (IS_ERR(port->vpcie1v8)) {
>> + if (PTR_ERR(port->vpcie1v8) == -EPROBE_DEFER)
>> + return -EPROBE_DEFER;
>> + dev_info(dev, "No vpcie1v8 regulator found.\n");
>> + }
>> +
>> + port->vpcie0v9 = devm_regulator_get_optional(dev, "vpcie0v9");
>> + if (IS_ERR(port->vpcie0v9)) {
>> + if (PTR_ERR(port->vpcie0v9) == -EPROBE_DEFER)
>> + return -EPROBE_DEFER;
>> + dev_info(dev, "No vpcie0v9 regulator found.\n");
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int rockchip_pcie_set_vpcie(struct rockchip_pcie_port *port)
>> +{
>> + int err;
>> +
>> + if (!IS_ERR(port->vpcie3v3)) {
>> + err = regulator_enable(port->vpcie3v3);
>> + if (err) {
>> + dev_err(port->dev, "Fail to enable vpcie3v3 regulator.\n");
>> + goto err_out;
>> + }
>> + }
>> +
>> + if (!IS_ERR(port->vpcie1v8)) {
>> + err = regulator_enable(port->vpcie1v8);
>> + if (err) {
>> + dev_err(port->dev, "Fail to enable vpcie1v8 regulator.\n");
>> + goto err_disable_3v3;
>> + }
>> + }
>> +
>> + if (!IS_ERR(port->vpcie0v9)) {
>> + err = regulator_enable(port->vpcie0v9);
>> + if (err) {
>> + dev_err(port->dev, "Fail to enable vpcie0v9 regulator.\n");
>> + goto err_disable_1v8;
>> + }
>> + }
>> +
>> + return 0;
>> +
>> +err_disable_1v8:
>> + if (!IS_ERR(port->vpcie1v8))
>> + regulator_disable(port->vpcie1v8);
>> +err_disable_3v3:
>> + if (!IS_ERR(port->vpcie3v3))
>> + regulator_disable(port->vpcie3v3);
>> +err_out:
>> + return err;
>> +}
>> +
>> +static void rockchip_pcie_enable_interrupts(struct rockchip_pcie_port *port)
>> +{
>> + pcie_write(port, (PCIE_CLIENT_INT_CLI << 16) &
>> + (~PCIE_CLIENT_INT_CLI), PCIE_CLIENT_INT_MASK);
>> + pcie_write(port, PCIE_CORE_INT, PCIE_CORE_INT_MASK);
>> +}
>> +
>> +static int rockchip_pcie_intx_map(struct irq_domain *domain, unsigned int irq,
>> + irq_hw_number_t hwirq)
>> +{
>> + irq_set_chip_and_handler(irq, &dummy_irq_chip, handle_simple_irq);
>> + irq_set_chip_data(irq, domain->host_data);
>> +
>> + return 0;
>> +}
>> +
>> +static const struct irq_domain_ops intx_domain_ops = {
>> + .map = rockchip_pcie_intx_map,
>> +};
>> +
>> +static int rockchip_pcie_init_irq_domain(struct rockchip_pcie_port *pp)
>> +{
>> + struct device *dev = pp->dev;
>> +
>> + pp->irq_domain = irq_domain_add_linear(dev->of_node, 4,
>> + &intx_domain_ops, pp);
>> + if (!pp->irq_domain) {
>> + dev_err(dev, "Failed to get a INTx IRQ domain\n");
>> + return PTR_ERR(pp->irq_domain);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static irqreturn_t rockchip_pcie_subsys_irq_handler(int irq, void *arg)
>> +{
>> + struct rockchip_pcie_port *pp = arg;
>> + u32 reg;
>> + u32 sub_reg;
>> +
>> + reg = pcie_read(pp, PCIE_CLIENT_INT_STATUS);
>> + if (reg & PCIE_CLIENT_INT_LOCAL) {
>> + dev_dbg(pp->dev, "local interrupt recived\n");
>> + sub_reg = pcie_read(pp, PCIE_CORE_INT_STATUS);
>> + if (sub_reg & PCIE_CORE_INT_PRFPE)
>> + dev_dbg(pp->dev, "Parity error detected while reading from the PNP Receive FIFO RAM\n");
>> +
>> + if (sub_reg & PCIE_CORE_INT_CRFPE)
>> + dev_dbg(pp->dev, "Parity error detected while reading from the Completion Receive FIFO RAM\n");
>> +
>> + if (sub_reg & PCIE_CORE_INT_RRPE)
>> + dev_dbg(pp->dev, "Parity error detected while reading from Replay Buffer RAM\n");
>> +
>> + if (sub_reg & PCIE_CORE_INT_PRFO)
>> + dev_dbg(pp->dev, "Overflow occurred in the PNP Receive FIFO\n");
>> +
>> + if (sub_reg & PCIE_CORE_INT_CRFO)
>> + dev_dbg(pp->dev, "Overflow occurred in the Completion Receive FIFO\n");
>> +
>> + if (sub_reg & PCIE_CORE_INT_RT)
>> + dev_dbg(pp->dev, "Replay timer timed out\n");
>> +
>> + if (sub_reg & PCIE_CORE_INT_RTR)
>> + dev_dbg(pp->dev, "Replay timer rolled over after 4 transmissions of the same TLP\n");
>> +
>> + if (sub_reg & PCIE_CORE_INT_PE)
>> + dev_dbg(pp->dev, "Phy error detected on receive side\n");
>> +
>> + if (sub_reg & PCIE_CORE_INT_MTR)
>> + dev_dbg(pp->dev, "Malformed TLP received from the link\n");
>> +
>> + if (sub_reg & PCIE_CORE_INT_UCR)
>> + dev_dbg(pp->dev, "Malformed TLP received from the link\n");
>> +
>> + if (sub_reg & PCIE_CORE_INT_FCE)
>> + dev_dbg(pp->dev, "An error was observed in the flow control advertisements from the other side\n");
>> +
>> + if (sub_reg & PCIE_CORE_INT_CT)
>> + dev_dbg(pp->dev, "A request timed out waiting for completion\n");
>> +
>> + if (sub_reg & PCIE_CORE_INT_UTC)
>> + dev_dbg(pp->dev, "Unmapped TC error\n");
>> +
>> + if (sub_reg & PCIE_CORE_INT_MMVC)
>> + dev_dbg(pp->dev, "MSI mask register changes\n");
>> +
>> + pcie_write(pp, sub_reg, PCIE_CORE_INT_STATUS);
>> + }
>> +
>> + pcie_write(pp, PCIE_CLIENT_INT_SUBSYSTEM | PCIE_CLIENT_INT_LOCAL,
>> + PCIE_CLIENT_INT_STATUS);
>> +
>> + return IRQ_HANDLED;
>> +}
>> +
>> +static irqreturn_t rockchip_pcie_client_irq_handler(int irq, void *arg)
>> +{
>> + struct rockchip_pcie_port *pp = arg;
>> + u32 reg;
>> +
>> + reg = pcie_read(pp, PCIE_CLIENT_INT_STATUS);
>> + if (reg & PCIE_CLIENT_INT_LEGACY_DONE)
>> + dev_dbg(pp->dev, "legacy done interrupt recived\n");
>> +
>> + if (reg & PCIE_CLIENT_INT_MSG)
>> + dev_dbg(pp->dev, "message done interrupt recived\n");
>> +
>> + if (reg & PCIE_CLIENT_INT_HOT_RST)
>> + dev_dbg(pp->dev, "hot reset interrupt recived\n");
>> +
>> + if (reg & PCIE_CLIENT_INT_DPA)
>> + dev_dbg(pp->dev, "dpa interrupt recived\n");
>> +
>> + if (reg & PCIE_CLIENT_INT_FATAL_ERR)
>> + dev_dbg(pp->dev, "fatal error interrupt recived\n");
>> +
>> + if (reg & PCIE_CLIENT_INT_NFATAL_ERR)
>> + dev_dbg(pp->dev, "no fatal error interrupt recived\n");
>> +
>> + if (reg & PCIE_CLIENT_INT_CORR_ERR)
>> + dev_dbg(pp->dev, "correctable error interrupt recived\n");
>> +
>> + pcie_write(pp, reg, PCIE_CLIENT_INT_STATUS);
>> +
>> + return IRQ_HANDLED;
>> +}
>> +
>> +static void rockchip_pcie_legacy_int_handler(struct irq_desc *desc)
>> +{
>> + struct irq_chip *chip = irq_desc_get_chip(desc);
>> + struct rockchip_pcie_port *port;
>> + u32 reg;
>> +
>> + chained_irq_enter(chip, desc);
>> + port = irq_desc_get_handler_data(desc);
>> +
>> + reg = pcie_read(port, PCIE_CLIENT_INT_STATUS);
>> + reg = (reg & ROCKCHIP_PCIE_RPIFR1_INTR_MASK) >>
>> + ROCKCHIP_PCIE_RPIFR1_INTR_SHIFT;
>> + if (reg)
>> + generic_handle_irq(irq_find_mapping(port->irq_domain,
>> + ffs(reg)));
>> + chained_irq_exit(chip, desc);
>> +}
>> +
>> +static int rockchip_pcie_prog_ob_atu(struct rockchip_pcie_port *pp,
>> + int region_no,
>> + int type, u8 num_pass_bits,
>> + u32 lower_addr, u32 upper_addr)
>> +{
>> + u32 ob_addr_0 = 0;
>> + u32 ob_addr_1 = 0;
>> + u32 ob_desc_0 = 0;
>> + u32 ob_desc_1 = 0;
>> + void __iomem *aw_base;
>> +
>> + if (!pp)
>> + return -EINVAL;
>> + if (region_no >= MAX_AXI_WRAPPER_REGION_NUM)
>> + return -EINVAL;
>> + if ((num_pass_bits + 1) < 8)
>> + return -EINVAL;
>> + if (num_pass_bits > 63)
>> + return -EINVAL;
>> + if (region_no == 0) {
>> + if (AXI_REGION_0_SIZE < (2ULL << num_pass_bits))
>> + return -EINVAL;
>> + }
>> + if (region_no != 0) {
>> + if (AXI_REGION_SIZE < (2ULL << num_pass_bits))
>> + return -EINVAL;
>> + }
>> + aw_base = pp->apb_base + PCIE_CORE_AXI_CONF_BASE;
>> + aw_base += (region_no << OB_REG_SIZE_SHIFT);
>> +
>> + ob_addr_0 = (ob_addr_0 &
>> + ~0x0000003fU) | (num_pass_bits &
>> + 0x0000003fU);
>> + ob_addr_0 = (ob_addr_0 &
>> + ~0xffffff00U) | (lower_addr & 0xffffff00U);
>> + ob_addr_1 = upper_addr;
>> + ob_desc_0 = (1 << 23 | type);
>> +
>> + writel(ob_addr_0, aw_base);
>> + writel(ob_addr_1, aw_base + PCIE_CORE_OB_REGION_ADDR1);
>> + writel(ob_desc_0, aw_base + PCIE_CORE_OB_REGION_DESC0);
>> + writel(ob_desc_1, aw_base + PCIE_CORE_OB_REGION_DESC1);
>> +
>> + return 0;
>> +}
>> +
>> +static int rockchip_pcie_prog_ib_atu(struct rockchip_pcie_port *pp,
>> + int region_no,
>> + u8 num_pass_bits,
>> + u32 lower_addr,
>> + u32 upper_addr)
>> +{
>> + u32 ib_addr_0 = 0;
>> + u32 ib_addr_1 = 0;
>> + void __iomem *aw_base;
>> +
>> + if (!pp)
>> + return -EINVAL;
>> + if (region_no > MAX_AXI_IB_ROOTPORT_REGION_NUM)
>> + return -EINVAL;
>> + if ((num_pass_bits + 1) < MIN_AXI_ADDR_BITS_PASSED)
>> + return -EINVAL;
>> + if (num_pass_bits > 63)
>> + return -EINVAL;
>> + aw_base = pp->apb_base + PCIE_CORE_AXI_INBOUND_BASE;
>> + aw_base += (region_no << IB_ROOT_PORT_REG_SIZE_SHIFT);
>> + ib_addr_0 = (ib_addr_0 &
>> + ~0x0000003fU) | (num_pass_bits &
>> + 0x0000003fU);
>> +
>> + ib_addr_0 = (ib_addr_0 & ~0xffffff00U) |
>> + ((lower_addr << 8) & 0xffffff00U);
>> + ib_addr_1 = upper_addr;
>> +
>> + writel(ib_addr_0, aw_base);
>> + writel(ib_addr_1, aw_base + PCIE_RP_IB_ADDR_TRANS);
>> +
>> + return 0;
>> +}
>> +
>> +static int rockchip_pcie_probe(struct platform_device *pdev)
>> +{
>> + struct rockchip_pcie_port *port;
>> + struct device *dev = &pdev->dev;
>> + struct pci_bus *bus, *child;
>> + struct resource_entry *win;
>> + resource_size_t io_base;
>> + struct resource *busn;
>> + struct resource *mem;
>> + struct resource *io;
>> + phys_addr_t io_bus_addr;
>> + u32 io_size;
>> + phys_addr_t mem_bus_addr;
>> + u32 mem_size;
>> + int reg_no;
>> + int err = 0;
>> + int offset = 0;
>> +
>> + LIST_HEAD(res);
>> +
>> + if (!dev->of_node)
>> + return -ENODEV;
>> +
>> + port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
>> + if (!port)
>> + return -ENOMEM;
>> +
>> + port->dev = dev;
>> +
>> + err = rockchip_pcie_parse_dt(port);
>> + if (err)
>> + return err;
>> +
>> + err = clk_prepare_enable(port->aclk_pcie);
>> + if (err) {
>> + dev_err(dev, "Unable to enable aclk_pcie clock.\n");
>> + goto err_aclk_pcie;
>> + }
>> +
>> + err = clk_prepare_enable(port->aclk_perf_pcie);
>> + if (err) {
>> + dev_err(dev, "Unable to enable aclk_perf_pcie clock.\n");
>> + goto err_aclk_perf_pcie;
>> + }
>> +
>> + err = clk_prepare_enable(port->hclk_pcie);
>> + if (err) {
>> + dev_err(dev, "Unable to enable hclk_pcie clock.\n");
>> + goto err_hclk_pcie;
>> + }
>> +
>> + err = clk_prepare_enable(port->clk_pcie_pm);
>> + if (err) {
>> + dev_err(dev, "Unable to enable hclk_pcie clock.\n");
>> + goto err_pcie_pm;
>> + }
>> +
>> + err = rockchip_pcie_set_vpcie(port);
>> + if (err) {
>> + dev_err(port->dev, "Fail to set vpcie regulator.\n");
>> + goto err_set_vpcie;
>> + }
>> +
>> + err = rockchip_pcie_init_port(port);
>> + if (err)
>> + goto err_vpcie;
>> +
>> + platform_set_drvdata(pdev, port);
>> +
>> + rockchip_pcie_enable_interrupts(port);
>> +
>> + err = rockchip_pcie_init_irq_domain(port);
>> + if (err < 0)
>> + goto err_vpcie;
>> +
>> + err = of_pci_get_host_bridge_resources(dev->of_node, 0, 0xff,
>> + &res, &io_base);
>> + if (err)
>> + goto err_vpcie;
>> +
>> + /* Get the I/O and memory ranges from DT */
>> + resource_list_for_each_entry(win, &res) {
>> + switch (resource_type(win->res)) {
>> + case IORESOURCE_IO:
>> + io = win->res;
>> + io->name = "I/O";
>> + io_size = resource_size(io);
>> + io_bus_addr = io->start - win->offset;
>> + err = pci_remap_iospace(io, io_base);
>> + if (err) {
>> + dev_warn(port->dev, "error %d: failed to map resource %pR\n",
>> + err, io);
>> + continue;
>> + }
>> + break;
>> + case IORESOURCE_MEM:
>> + mem = win->res;
>> + mem->name = "MEM";
>> + mem_size = resource_size(mem);
>> + mem_bus_addr = mem->start - win->offset;
>> + break;
>> + case 0:
>
> Why case 0? That doesn't seem right.
I will remove it as it actually means configure space. But my controller
has the fixed cfg space inside the core, so we never need to get it
from parsing DT.
>
>> + break;
>> + case IORESOURCE_BUS:
>> + busn = win->res;
>> + break;
>> + default:
>> + continue;
>> + }
>> + }
>> +
>> + for (reg_no = 0; reg_no < (mem_size >> 20); reg_no++) {
>
> kbuildbot already complained about potential unused warnings here and
> elsewhere. One problem that this highlights:
>
> what happens if the expected resources aren't included in your device
> tree? Seems like you'll just keep going, with uninitialized data.
> Probably want some kind of checks to make sure we found the necessary
> resources.
yes, I saw the report from kbuildbot and I will fix it.
>
>> + err = rockchip_pcie_prog_ob_atu(port, reg_no + 1,
>> + AXI_WRAPPER_MEM_WRITE,
>> + 20 - 1,
>> + mem_bus_addr +
>> + (reg_no << 20),
>> + 0);
>> + if (err) {
>> + dev_err(dev, "Program RC mem outbound atu failed\n");
>> + goto err_vpcie;
>> + }
>> + }
>> +
>> + err = rockchip_pcie_prog_ib_atu(port, 2, 32 - 1, 0x0, 0);
>> + if (err) {
>> + dev_err(dev, "Program RC mem inbound atu failed\n");
>> + goto err_vpcie;
>> + }
>> +
>> + offset = mem_size >> 20;
>> +
>> + for (reg_no = 0; reg_no < (io_size >> 20); reg_no++) {
>> + err = rockchip_pcie_prog_ob_atu(port,
>> + reg_no + 1 + offset,
>> + AXI_WRAPPER_IO_WRITE,
>> + 20 - 1,
>> + io_bus_addr +
>> + (reg_no << 20),
>> + 0);
>> + if (err) {
>> + dev_err(dev, "Program RC io outbound atu failed\n");
>> + goto err_vpcie;
>> + }
>> + }
>> +
>> + port->root_bus_nr = busn->start;
>> +
>> + bus = pci_scan_root_bus(&pdev->dev, 0,
>> + &rockchip_pcie_ops, port, &res);
>> +
>> + if (!bus) {
>> + err = -ENOMEM;
>> + goto err_vpcie;
>> + }
>> +
>> + pci_bus_size_bridges(bus);
>> + pci_bus_assign_resources(bus);
>> + list_for_each_entry(child, &bus->children, node)
>> + pcie_bus_configure_settings(child);
>
> Bad indentation: the above line is part of a loop, so it should be
> indented one more.
>
My bad, will fix it.
>> +
>> + pci_bus_add_devices(bus);
>> +
>> + return err;
>> +
>> +err_vpcie:
>> + if (!IS_ERR(port->vpcie3v3))
>> + regulator_disable(port->vpcie3v3);
>> + if (!IS_ERR(port->vpcie1v8))
>> + regulator_disable(port->vpcie1v8);
>> + if (!IS_ERR(port->vpcie0v9))
>> + regulator_disable(port->vpcie0v9);
>> +err_set_vpcie:
>> + clk_disable_unprepare(port->clk_pcie_pm);
>> +err_pcie_pm:
>> + clk_disable_unprepare(port->hclk_pcie);
>> +err_hclk_pcie:
>> + clk_disable_unprepare(port->aclk_perf_pcie);
>> +err_aclk_perf_pcie:
>> + clk_disable_unprepare(port->aclk_pcie);
>> +err_aclk_pcie:
>> + return err;
>> +}
>> +
>> +static const struct of_device_id rockchip_pcie_of_match[] = {
>> + { .compatible = "rockchip,rk3399-pcie", },
>> + {}
>> +};
>> +
>> +static struct platform_driver rockchip_pcie_driver = {
>> + .driver = {
>> + .name = "rockchip-pcie",
>> + .of_match_table = rockchip_pcie_of_match,
>> + },
>> + .probe = rockchip_pcie_probe,
>> +
>> +};
>> +module_platform_driver(rockchip_pcie_driver);
>> +
>> +MODULE_AUTHOR("Rockchip Inc");
>> +MODULE_DESCRIPTION("Rockchip AXI PCIe driver");
>> +MODULE_LICENSE("GPL v2");
>> --
>> 2.3.7
>
> Brian
>
>
>
--
Best Regards
Shawn Lin
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 2/2] PCI: Rockchip: Add Rockchip PCIe controller support
2016-06-23 1:09 ` Shawn Lin
@ 2016-06-23 1:35 ` Brian Norris
2016-06-23 2:15 ` Shawn Lin
0 siblings, 1 reply; 6+ messages in thread
From: Brian Norris @ 2016-06-23 1:35 UTC (permalink / raw)
To: Shawn Lin
Cc: Bjorn Helgaas, devicetree, Heiko Stuebner, Arnd Bergmann,
Marc Zyngier, linux-pci, Wenrui Li, linux-kernel, Doug Anderson,
linux-rockchip, Rob Herring
Hi,
On Thu, Jun 23, 2016 at 09:09:46AM +0800, Shawn Lin wrote:
> 在 2016/6/23 8:29, Brian Norris 写道:
> >On Thu, Jun 16, 2016 at 09:50:35AM +0800, Shawn Lin wrote:
[...]
> >>+ /* 500ms timeout value should be enough for gen1/2 taining */
> >>+ timeout = jiffies + msecs_to_jiffies(500);
> >>+
> >>+ err = -ETIMEDOUT;
> >>+ while (time_before(jiffies, timeout)) {
> >>+ status = pcie_read(port, PCIE_CLIENT_BASIC_STATUS1);
> >>+ if (((status >> PCIE_CLIENT_LINK_STATUS_SHIFT) &
> >>+ PCIE_CLIENT_LINK_STATUS_MASK) ==
> >>+ PCIE_CLIENT_LINK_STATUS_UP) {
> >>+ dev_dbg(port->dev, "pcie link training gen1 pass!\n");
> >>+ err = 0;
> >>+ break;
> >>+ }
> >>+ msleep(20);
> >>+ }
> >
> >Technically, the above timeout loop is not quite correct. Possible error
> >case: we can fail with a timeout after 500 ms if the training completes
> >between the 480-500 ms time window. This can happen because you're
> >doing:
> >
> >(1) read register: if complete, then terminate successfully
> >(2) delay
> >(3) check for timeout: if time is up, return error
> >
> >You actually need:
> >
> >(1) read register: if complete, then terminate successfully
> >(2) delay
> >(3) check for timeout: if time is up, repeat (1), and then report error
> >
> >You can examine the logic for readx_poll_timeout() in
> >include/linux/iopoll.h to see an example of a proper timeout loop. You
> >could even try to use one of the helpers there, except that your
> >pcie_read() takes 2 args.
>
> I see, thanks.
>
> >
> >>+ if (err) {
> >>+ dev_err(port->dev, "pcie link training gen1 timeout!\n");
> >>+ return err;
> >>+ }
> >>+
> >>+ /*
> >>+ * Enable retrain for gen2. This should be configured only after
> >>+ * gen1 finished.
> >>+ */
> >>+ status = pcie_read(port,
> >>+ PCIR_RC_CONFIG_LCS + PCIE_RC_CONFIG_BASE);
> >>+ status |= PCIE_CORE_LCSR_RETAIN_LINK;
> >>+ pcie_write(port, status,
> >>+ PCIR_RC_CONFIG_LCS + PCIE_RC_CONFIG_BASE);
> >
> >I'm not really an expert on this, but how are you actually "retraining
> >for gen2"? Is that just the behavior of the core, that it retries at the
> >higher rate on the 2nd training attempt? I'm just confused, since you
> >set PCIE_CLIENT_GEN_SEL_2 above, so you've already allowed either gen1
> >or gen2 negotiation AFAICT, and so seemingly you might already have
> >negotiated at gen2.
>
>
> Not really. I allow the core to enable gen2, but it needs a extra
> setting of retrain after finishing gen1. It's not so strange as it
> depends on the vendor's design. So I have to say it fits the
> designer's expectation.
OK.
> >>+ err = -ETIMEDOUT;
> >>+
> >>+ while (time_before(jiffies, timeout)) {
> >
> >You never reset 'timeout' when starting this loop. So you only have a
> >cumulative 500 ms to do both the gen1 and gen2 loops. Is that
> >intentional? (I feel like this comment was made on an earlier revision
> >of this driver, though I haven't read everything thoroughly.)
>
> yes, I don't have any docs to let me know how long should I wait for
> gen1/2 to be finished. Maybe someday it will be augmented to a larger
> value if finding a device actually need a longer time. But the only
> thing I can say is that it's from my test for many pcie devices
> currently.
>
>
> Do you agree?
I'm not suggesting increasing the timeout, exactly; I'm suggesting that
you should set some minimum timeout for each training loop, instead of
reusing the same deadline. i.e., something like this before the second
loop:
/* Reset the deadline for gen2 */
timeout = jiffies + msecs_to_jiffies(500);
As it stands, if the first loop takes 490 ms, that leaves you only 10 ms
for the second loop. And I think it'd be confusing if we ever see the
first loop take a "long" time, and then we time out on the second --
we'd be blaming the gen2 training for gen1's slowness.
> >>+ status = pcie_read(port, PCIE_CORE_CTRL_MGMT_BASE);
> >>+ if (((status >> PCIE_CORE_PL_CONF_SPEED_SHIFT) &
> >>+ PCIE_CORE_PL_CONF_SPEED_MASK) ==
> >>+ PCIE_CORE_PL_CONF_SPEED_50G) {
> >>+ dev_dbg(port->dev, "pcie link training gen2 pass!\n");
> >>+ err = 0;
> >>+ break;
> >>+ }
> >>+ msleep(20);
> >>+ }
> >
> >Similar problem with your timeout loop, as mentioned above. Although I
> >confused about what you do in the error case here...
> >
> >>+ if (err)
> >>+ dev_dbg(port->dev, "pcie link training gen2 timeout, force to gen1!\n");
> >
> >... how are you forcing gen1? I don't see any code that indicates this.
> >Should you at least be checking that there aren't some kind of training
> >errors, and that we settled back on gen1/2.5G?
>
> yes, when failing gen2, my pcie controller will fallback to gen1
> automatically.
OK. Well maybe the text should say something like "falling back" instead
of "force"?
> if we pass the gen1 then fail to train gen2, a dbg msg here is enough
> here to let we know that we should check the HW signal. Of course we
> should make sure that this device supports gen2.
OK.
[...]
Brian
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 2/2] PCI: Rockchip: Add Rockchip PCIe controller support
2016-06-23 1:35 ` Brian Norris
@ 2016-06-23 2:15 ` Shawn Lin
0 siblings, 0 replies; 6+ messages in thread
From: Shawn Lin @ 2016-06-23 2:15 UTC (permalink / raw)
To: Brian Norris
Cc: shawn.lin, Bjorn Helgaas, devicetree, Heiko Stuebner,
Arnd Bergmann, Marc Zyngier, linux-pci, Wenrui Li, linux-kernel,
Doug Anderson, linux-rockchip, Rob Herring
在 2016/6/23 9:35, Brian Norris 写道:
> Hi,
>
> On Thu, Jun 23, 2016 at 09:09:46AM +0800, Shawn Lin wrote:
>> 在 2016/6/23 8:29, Brian Norris 写道:
>>> On Thu, Jun 16, 2016 at 09:50:35AM +0800, Shawn Lin wrote:
>
> [...]
>
>>>> + /* 500ms timeout value should be enough for gen1/2 taining */
>>>> + timeout = jiffies + msecs_to_jiffies(500);
>>>> +
>>>> + err = -ETIMEDOUT;
>>>> + while (time_before(jiffies, timeout)) {
>>>> + status = pcie_read(port, PCIE_CLIENT_BASIC_STATUS1);
>>>> + if (((status >> PCIE_CLIENT_LINK_STATUS_SHIFT) &
>>>> + PCIE_CLIENT_LINK_STATUS_MASK) ==
>>>> + PCIE_CLIENT_LINK_STATUS_UP) {
>>>> + dev_dbg(port->dev, "pcie link training gen1 pass!\n");
>>>> + err = 0;
>>>> + break;
>>>> + }
>>>> + msleep(20);
>>>> + }
>>>
>>> Technically, the above timeout loop is not quite correct. Possible error
>>> case: we can fail with a timeout after 500 ms if the training completes
>>> between the 480-500 ms time window. This can happen because you're
>>> doing:
>>>
>>> (1) read register: if complete, then terminate successfully
>>> (2) delay
>>> (3) check for timeout: if time is up, return error
>>>
>>> You actually need:
>>>
>>> (1) read register: if complete, then terminate successfully
>>> (2) delay
>>> (3) check for timeout: if time is up, repeat (1), and then report error
>>>
>>> You can examine the logic for readx_poll_timeout() in
>>> include/linux/iopoll.h to see an example of a proper timeout loop. You
>>> could even try to use one of the helpers there, except that your
>>> pcie_read() takes 2 args.
>>
>> I see, thanks.
>>
>>>
>>>> + if (err) {
>>>> + dev_err(port->dev, "pcie link training gen1 timeout!\n");
>>>> + return err;
>>>> + }
>>>> +
>>>> + /*
>>>> + * Enable retrain for gen2. This should be configured only after
>>>> + * gen1 finished.
>>>> + */
>>>> + status = pcie_read(port,
>>>> + PCIR_RC_CONFIG_LCS + PCIE_RC_CONFIG_BASE);
>>>> + status |= PCIE_CORE_LCSR_RETAIN_LINK;
>>>> + pcie_write(port, status,
>>>> + PCIR_RC_CONFIG_LCS + PCIE_RC_CONFIG_BASE);
>>>
>>> I'm not really an expert on this, but how are you actually "retraining
>>> for gen2"? Is that just the behavior of the core, that it retries at the
>>> higher rate on the 2nd training attempt? I'm just confused, since you
>>> set PCIE_CLIENT_GEN_SEL_2 above, so you've already allowed either gen1
>>> or gen2 negotiation AFAICT, and so seemingly you might already have
>>> negotiated at gen2.
>>
>>
>> Not really. I allow the core to enable gen2, but it needs a extra
>> setting of retrain after finishing gen1. It's not so strange as it
>> depends on the vendor's design. So I have to say it fits the
>> designer's expectation.
>
> OK.
>
>>>> + err = -ETIMEDOUT;
>>>> +
>>>> + while (time_before(jiffies, timeout)) {
>>>
>>> You never reset 'timeout' when starting this loop. So you only have a
>>> cumulative 500 ms to do both the gen1 and gen2 loops. Is that
>>> intentional? (I feel like this comment was made on an earlier revision
>>> of this driver, though I haven't read everything thoroughly.)
>>
>> yes, I don't have any docs to let me know how long should I wait for
>> gen1/2 to be finished. Maybe someday it will be augmented to a larger
>> value if finding a device actually need a longer time. But the only
>> thing I can say is that it's from my test for many pcie devices
>> currently.
>>
>>
>> Do you agree?
>
> I'm not suggesting increasing the timeout, exactly; I'm suggesting that
> you should set some minimum timeout for each training loop, instead of
> reusing the same deadline. i.e., something like this before the second
> loop:
>
> /* Reset the deadline for gen2 */
> timeout = jiffies + msecs_to_jiffies(500);
ok, I will update it.
>
> As it stands, if the first loop takes 490 ms, that leaves you only 10 ms
> for the second loop. And I think it'd be confusing if we ever see the
> first loop take a "long" time, and then we time out on the second --
> we'd be blaming the gen2 training for gen1's slowness.
>
>>>> + status = pcie_read(port, PCIE_CORE_CTRL_MGMT_BASE);
>>>> + if (((status >> PCIE_CORE_PL_CONF_SPEED_SHIFT) &
>>>> + PCIE_CORE_PL_CONF_SPEED_MASK) ==
>>>> + PCIE_CORE_PL_CONF_SPEED_50G) {
>>>> + dev_dbg(port->dev, "pcie link training gen2 pass!\n");
>>>> + err = 0;
>>>> + break;
>>>> + }
>>>> + msleep(20);
>>>> + }
>>>
>>> Similar problem with your timeout loop, as mentioned above. Although I
>>> confused about what you do in the error case here...
>>>
>>>> + if (err)
>>>> + dev_dbg(port->dev, "pcie link training gen2 timeout, force to gen1!\n");
>>>
>>> ... how are you forcing gen1? I don't see any code that indicates this.
>>> Should you at least be checking that there aren't some kind of training
>>> errors, and that we settled back on gen1/2.5G?
>>
>> yes, when failing gen2, my pcie controller will fallback to gen1
>> automatically.
>
> OK. Well maybe the text should say something like "falling back" instead
> of "force"?
>
Sounds good, will fix. Thanks.
>> if we pass the gen1 then fail to train gen2, a dbg msg here is enough
>> here to let we know that we should check the HW signal. Of course we
>> should make sure that this device supports gen2.
>
> OK.
>
> [...]
>
> Brian
>
>
>
--
Best Regards
Shawn Lin
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-06-23 2:15 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-16 1:50 [PATCH v3 2/2] PCI: Rockchip: Add Rockchip PCIe controller support Shawn Lin
2016-06-16 13:08 ` kbuild test robot
2016-06-23 0:29 ` Brian Norris
2016-06-23 1:09 ` Shawn Lin
2016-06-23 1:35 ` Brian Norris
2016-06-23 2:15 ` Shawn Lin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).