This patchset includes SiFive FU740 PCIe host controller driver. We also
add pcie_aux clock and pcie_power_on_reset controller to prci driver for
PCIe driver to use it.
This is tested with e1000e: Intel(R) PRO/1000 Network Card and SP M.2 PCIe
Gen 3 SSD in SiFive Unmatched.
Greentime Hu (5):
clk: sifive: Add pcie_aux clock in prci driver for PCIe driver
clk: sifive: Use reset-simple in prci driver for PCIe driver
MAINTAINERS: Add maintainers for SiFive FU740 PCIe driver
dt-bindings: PCI: Add SiFive FU740 PCIe host controller
riscv: dts: Add PCIe support for the SiFive FU740-C000 SoC
Paul Walmsley (1):
PCI: designware: Add SiFive FU740 PCIe host controller driver
.../bindings/pci/sifive,fu740-pcie.yaml | 119 +++++
MAINTAINERS | 8 +
arch/riscv/boot/dts/sifive/fu740-c000.dtsi | 34 ++
drivers/clk/sifive/Kconfig | 2 +
drivers/clk/sifive/fu740-prci.c | 11 +
drivers/clk/sifive/fu740-prci.h | 2 +-
drivers/clk/sifive/sifive-prci.c | 55 +++
drivers/clk/sifive/sifive-prci.h | 13 +
drivers/pci/controller/dwc/Kconfig | 9 +
drivers/pci/controller/dwc/Makefile | 1 +
drivers/pci/controller/dwc/pcie-fu740.c | 455 ++++++++++++++++++
drivers/reset/Kconfig | 3 +-
include/dt-bindings/clock/sifive-fu740-prci.h | 1 +
13 files changed, 711 insertions(+), 2 deletions(-)
create mode 100644 Documentation/devicetree/bindings/pci/sifive,fu740-pcie.yaml
create mode 100644 drivers/pci/controller/dwc/pcie-fu740.c
--
2.30.0
We add pcie_aux clock in this patch so that pcie driver can use
clk_prepare_enable() and clk_disable_unprepare() to enable and disable
pcie_aux clock.
Signed-off-by: Greentime Hu <[email protected]>
---
drivers/clk/sifive/fu740-prci.c | 11 +++++
drivers/clk/sifive/fu740-prci.h | 2 +-
drivers/clk/sifive/sifive-prci.c | 41 +++++++++++++++++++
drivers/clk/sifive/sifive-prci.h | 9 ++++
include/dt-bindings/clock/sifive-fu740-prci.h | 1 +
5 files changed, 63 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/sifive/fu740-prci.c b/drivers/clk/sifive/fu740-prci.c
index 764d1097aa51..53f6e00a03b9 100644
--- a/drivers/clk/sifive/fu740-prci.c
+++ b/drivers/clk/sifive/fu740-prci.c
@@ -72,6 +72,12 @@ static const struct clk_ops sifive_fu740_prci_hfpclkplldiv_clk_ops = {
.recalc_rate = sifive_prci_hfpclkplldiv_recalc_rate,
};
+static const struct clk_ops sifive_fu740_prci_pcie_aux_clk_ops = {
+ .enable = sifive_prci_pcie_aux_clock_enable,
+ .disable = sifive_prci_pcie_aux_clock_disable,
+ .is_enabled = sifive_prci_pcie_aux_clock_is_enabled,
+};
+
/* List of clock controls provided by the PRCI */
struct __prci_clock __prci_init_clocks_fu740[] = {
[PRCI_CLK_COREPLL] = {
@@ -120,4 +126,9 @@ struct __prci_clock __prci_init_clocks_fu740[] = {
.parent_name = "hfpclkpll",
.ops = &sifive_fu740_prci_hfpclkplldiv_clk_ops,
},
+ [PRCI_CLK_PCIE_AUX] = {
+ .name = "pcie_aux",
+ .parent_name = "hfclk",
+ .ops = &sifive_fu740_prci_pcie_aux_clk_ops,
+ },
};
diff --git a/drivers/clk/sifive/fu740-prci.h b/drivers/clk/sifive/fu740-prci.h
index 13ef971f7764..511a0bf7ba2b 100644
--- a/drivers/clk/sifive/fu740-prci.h
+++ b/drivers/clk/sifive/fu740-prci.h
@@ -9,7 +9,7 @@
#include "sifive-prci.h"
-#define NUM_CLOCK_FU740 8
+#define NUM_CLOCK_FU740 9
extern struct __prci_clock __prci_init_clocks_fu740[NUM_CLOCK_FU740];
diff --git a/drivers/clk/sifive/sifive-prci.c b/drivers/clk/sifive/sifive-prci.c
index c78b042750e2..baf7313dac92 100644
--- a/drivers/clk/sifive/sifive-prci.c
+++ b/drivers/clk/sifive/sifive-prci.c
@@ -448,6 +448,47 @@ void sifive_prci_hfpclkpllsel_use_hfpclkpll(struct __prci_data *pd)
r = __prci_readl(pd, PRCI_HFPCLKPLLSEL_OFFSET); /* barrier */
}
+/* PCIE AUX clock APIs for enable, disable. */
+int sifive_prci_pcie_aux_clock_is_enabled(struct clk_hw *hw)
+{
+ struct __prci_clock *pc = clk_hw_to_prci_clock(hw);
+ struct __prci_data *pd = pc->pd;
+ u32 r;
+
+ r = __prci_readl(pd, PRCI_PCIE_AUX_OFFSET);
+
+ if (r & PRCI_PCIE_AUX_EN_MASK)
+ return 1;
+ else
+ return 0;
+}
+
+int sifive_prci_pcie_aux_clock_enable(struct clk_hw *hw)
+{
+ struct __prci_clock *pc = clk_hw_to_prci_clock(hw);
+ struct __prci_data *pd = pc->pd;
+ u32 r;
+
+ if (sifive_prci_pcie_aux_clock_is_enabled(hw))
+ return 0;
+
+ __prci_writel(1, PRCI_PCIE_AUX_OFFSET, pd);
+ r = __prci_readl(pd, PRCI_PCIE_AUX_OFFSET); /* barrier */
+
+ return 0;
+}
+
+void sifive_prci_pcie_aux_clock_disable(struct clk_hw *hw)
+{
+ struct __prci_clock *pc = clk_hw_to_prci_clock(hw);
+ struct __prci_data *pd = pc->pd;
+ u32 r;
+
+ __prci_writel(0, PRCI_PCIE_AUX_OFFSET, pd);
+ r = __prci_readl(pd, PRCI_PCIE_AUX_OFFSET); /* barrier */
+
+}
+
/**
* __prci_register_clocks() - register clock controls in the PRCI
* @dev: Linux struct device
diff --git a/drivers/clk/sifive/sifive-prci.h b/drivers/clk/sifive/sifive-prci.h
index dbdbd1722688..022c67cf053c 100644
--- a/drivers/clk/sifive/sifive-prci.h
+++ b/drivers/clk/sifive/sifive-prci.h
@@ -67,6 +67,11 @@
#define PRCI_DDRPLLCFG1_CKE_SHIFT 31
#define PRCI_DDRPLLCFG1_CKE_MASK (0x1 << PRCI_DDRPLLCFG1_CKE_SHIFT)
+/* PCIEAUX */
+#define PRCI_PCIE_AUX_OFFSET 0x14
+#define PRCI_PCIE_AUX_EN_SHIFT 0
+#define PRCI_PCIE_AUX_EN_MASK (0x1 << PRCI_PCIE_AUX_EN_SHIFT)
+
/* GEMGXLPLLCFG0 */
#define PRCI_GEMGXLPLLCFG0_OFFSET 0x1c
#define PRCI_GEMGXLPLLCFG0_DIVR_SHIFT 0
@@ -296,4 +301,8 @@ unsigned long sifive_prci_tlclksel_recalc_rate(struct clk_hw *hw,
unsigned long sifive_prci_hfpclkplldiv_recalc_rate(struct clk_hw *hw,
unsigned long parent_rate);
+int sifive_prci_pcie_aux_clock_is_enabled(struct clk_hw *hw);
+int sifive_prci_pcie_aux_clock_enable(struct clk_hw *hw);
+void sifive_prci_pcie_aux_clock_disable(struct clk_hw *hw);
+
#endif /* __SIFIVE_CLK_SIFIVE_PRCI_H */
diff --git a/include/dt-bindings/clock/sifive-fu740-prci.h b/include/dt-bindings/clock/sifive-fu740-prci.h
index cd7706ea5677..7899b7fee7db 100644
--- a/include/dt-bindings/clock/sifive-fu740-prci.h
+++ b/include/dt-bindings/clock/sifive-fu740-prci.h
@@ -19,5 +19,6 @@
#define PRCI_CLK_CLTXPLL 5
#define PRCI_CLK_TLCLK 6
#define PRCI_CLK_PCLK 7
+#define PRCI_CLK_PCIE_AUX 8
#endif /* __DT_BINDINGS_CLOCK_SIFIVE_FU740_PRCI_H */
--
2.30.0
From: Paul Walmsley <[email protected]>
Add driver for the SiFive FU740 PCIe host controller.
This controller is based on the DesignWare PCIe core.
Co-developed-by: Henry Styles <[email protected]>
Signed-off-by: Henry Styles <[email protected]>
Co-developed-by: Erik Danie <[email protected]>
Signed-off-by: Erik Danie <[email protected]>
Co-developed-by: Greentime Hu <[email protected]>
Signed-off-by: Greentime Hu <[email protected]>
Signed-off-by: Paul Walmsley <[email protected]>
---
drivers/pci/controller/dwc/Kconfig | 9 +
drivers/pci/controller/dwc/Makefile | 1 +
drivers/pci/controller/dwc/pcie-fu740.c | 455 ++++++++++++++++++++++++
3 files changed, 465 insertions(+)
create mode 100644 drivers/pci/controller/dwc/pcie-fu740.c
diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
index 22c5529e9a65..0a37d21ed64e 100644
--- a/drivers/pci/controller/dwc/Kconfig
+++ b/drivers/pci/controller/dwc/Kconfig
@@ -318,4 +318,13 @@ config PCIE_AL
required only for DT-based platforms. ACPI platforms with the
Annapurna Labs PCIe controller don't need to enable this.
+config PCIE_FU740
+ bool "SiFive FU740 PCIe host controller"
+ depends on PCI_MSI_IRQ_DOMAIN
+ depends on SOC_SIFIVE || COMPILE_TEST
+ select PCIE_DW_HOST
+ help
+ Say Y here if you want PCIe controller support for the SiFive
+ FU740.
+
endmenu
diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
index a751553fa0db..625f6aaeb5b8 100644
--- a/drivers/pci/controller/dwc/Makefile
+++ b/drivers/pci/controller/dwc/Makefile
@@ -5,6 +5,7 @@ obj-$(CONFIG_PCIE_DW_EP) += pcie-designware-ep.o
obj-$(CONFIG_PCIE_DW_PLAT) += pcie-designware-plat.o
obj-$(CONFIG_PCI_DRA7XX) += pci-dra7xx.o
obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
+obj-$(CONFIG_PCIE_FU740) += pcie-fu740.o
obj-$(CONFIG_PCI_IMX6) += pci-imx6.o
obj-$(CONFIG_PCIE_SPEAR13XX) += pcie-spear13xx.o
obj-$(CONFIG_PCI_KEYSTONE) += pci-keystone.o
diff --git a/drivers/pci/controller/dwc/pcie-fu740.c b/drivers/pci/controller/dwc/pcie-fu740.c
new file mode 100644
index 000000000000..6916eea40ea5
--- /dev/null
+++ b/drivers/pci/controller/dwc/pcie-fu740.c
@@ -0,0 +1,455 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * FU740 DesignWare PCIe Controller integration
+ * Copyright (C) 2019-2021 SiFive, Inc.
+ * Paul Walmsley
+ * Greentime Hu
+ *
+ * Based in part on the i.MX6 PCIe host controller shim which is:
+ *
+ * Copyright (C) 2013 Kosagi
+ * https://www.kosagi.com
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/gpio.h>
+#include <linux/kernel.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of_gpio.h>
+#include <linux/of_device.h>
+#include <linux/pci.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/resource.h>
+#include <linux/signal.h>
+#include <linux/types.h>
+#include <linux/interrupt.h>
+#include <linux/reset.h>
+#include <linux/pm_domain.h>
+#include <linux/pm_runtime.h>
+
+#include "pcie-designware.h"
+
+#define to_fu740_pcie(x) dev_get_drvdata((x)->dev)
+
+struct fu740_pcie {
+ struct dw_pcie *pci;
+ void __iomem *mgmt_base;
+ int perstn_gpio;
+ int pwren_gpio;
+ struct clk *pcie_aux;
+ struct reset_control *rst;
+};
+
+#define SIFIVE_DEVICESRESETREG 0x28
+
+#define PCIEX8MGMT_PERST_N 0x0
+#define PCIEX8MGMT_APP_LTSSM_ENABLE 0x10
+#define PCIEX8MGMT_APP_HOLD_PHY_RST 0x18
+#define PCIEX8MGMT_DEVICE_TYPE 0x708
+#define PCIEX8MGMT_PHY0_CR_PARA_ADDR 0x860
+#define PCIEX8MGMT_PHY0_CR_PARA_RD_EN 0x870
+#define PCIEX8MGMT_PHY0_CR_PARA_RD_DATA 0x878
+#define PCIEX8MGMT_PHY0_CR_PARA_SEL 0x880
+#define PCIEX8MGMT_PHY0_CR_PARA_WR_DATA 0x888
+#define PCIEX8MGMT_PHY0_CR_PARA_WR_EN 0x890
+#define PCIEX8MGMT_PHY0_CR_PARA_ACK 0x898
+#define PCIEX8MGMT_PHY1_CR_PARA_ADDR 0x8a0
+#define PCIEX8MGMT_PHY1_CR_PARA_RD_EN 0x8b0
+#define PCIEX8MGMT_PHY1_CR_PARA_RD_DATA 0x8b8
+#define PCIEX8MGMT_PHY1_CR_PARA_SEL 0x8c0
+#define PCIEX8MGMT_PHY1_CR_PARA_WR_DATA 0x8c8
+#define PCIEX8MGMT_PHY1_CR_PARA_WR_EN 0x8d0
+#define PCIEX8MGMT_PHY1_CR_PARA_ACK 0x8d8
+
+/* PCIe Port Logic registers (memory-mapped) */
+#define PL_OFFSET 0x700
+#define PCIE_PL_GEN2_CTRL_OFF (PL_OFFSET + 0x10c)
+#define PCIE_PL_DIRECTED_SPEED_CHANGE_OFF 0x20000
+
+#define PCIE_PHY_MAX_RETRY_CNT 1000
+
+static void fu740_pcie_assert_perstn(struct fu740_pcie *afp)
+{
+ /* PERST_N GPIO */
+ if (gpio_is_valid(afp->perstn_gpio))
+ gpio_direction_output(afp->perstn_gpio, 0);
+
+ /* Controller PERST_N */
+ __raw_writel(0x0, afp->mgmt_base + PCIEX8MGMT_PERST_N);
+}
+
+static void fu740_pcie_deassert_perstn(struct fu740_pcie *afp)
+{
+ /* Controller PERST_N */
+ __raw_writel(0x1, afp->mgmt_base + PCIEX8MGMT_PERST_N);
+ /* PERST_N GPIO */
+ if (gpio_is_valid(afp->perstn_gpio))
+ gpio_direction_output(afp->perstn_gpio, 1);
+}
+
+static void fu740_pcie_power_on(struct fu740_pcie *afp)
+{
+ if (gpio_is_valid(afp->pwren_gpio)) {
+ gpio_direction_output(afp->pwren_gpio, 1);
+ mdelay(100);
+ }
+}
+
+static void fu740_pcie_drive_perstn(struct fu740_pcie *afp)
+{
+ fu740_pcie_assert_perstn(afp);
+ fu740_pcie_power_on(afp);
+ fu740_pcie_deassert_perstn(afp);
+}
+
+static void fu740_phyregreadwrite(const uint8_t phy, const uint8_t write,
+ const uint16_t addr,
+ const uint16_t wrdata, uint16_t *rddata,
+ struct fu740_pcie *afp)
+{
+ unsigned char ack = 0;
+ unsigned int cnt = 0;
+ struct device *dev = afp->pci->dev;
+
+ /* setup */
+ __raw_writel(addr,
+ afp->mgmt_base +
+ (phy ? PCIEX8MGMT_PHY1_CR_PARA_ADDR :
+ PCIEX8MGMT_PHY0_CR_PARA_ADDR));
+ if (write)
+ __raw_writel(wrdata,
+ afp->mgmt_base +
+ (phy ? PCIEX8MGMT_PHY1_CR_PARA_WR_DATA :
+ PCIEX8MGMT_PHY0_CR_PARA_WR_DATA));
+ if (write)
+ __raw_writel(1,
+ afp->mgmt_base +
+ (phy ? PCIEX8MGMT_PHY1_CR_PARA_WR_EN :
+ PCIEX8MGMT_PHY0_CR_PARA_WR_EN));
+ else
+ __raw_writel(1,
+ afp->mgmt_base +
+ (phy ? PCIEX8MGMT_PHY1_CR_PARA_RD_EN :
+ PCIEX8MGMT_PHY0_CR_PARA_RD_EN));
+
+ /* wait for wait_idle */
+ do {
+ if (__raw_readl
+ (afp->mgmt_base +
+ (phy ? PCIEX8MGMT_PHY1_CR_PARA_ACK :
+ PCIEX8MGMT_PHY0_CR_PARA_ACK))) {
+ ack = 1;
+ if (!write)
+ __raw_readl(afp->mgmt_base +
+ (phy ?
+ PCIEX8MGMT_PHY1_CR_PARA_RD_DATA :
+ PCIEX8MGMT_PHY0_CR_PARA_RD_DATA));
+ }
+ } while (!ack);
+
+ /* clear */
+ if (write)
+ __raw_writel(0,
+ afp->mgmt_base +
+ (phy ? PCIEX8MGMT_PHY1_CR_PARA_WR_EN :
+ PCIEX8MGMT_PHY0_CR_PARA_WR_EN));
+ else
+ __raw_writel(0,
+ afp->mgmt_base +
+ (phy ? PCIEX8MGMT_PHY1_CR_PARA_RD_EN :
+ PCIEX8MGMT_PHY0_CR_PARA_RD_EN));
+
+ /* wait for ~wait_idle */
+ while (__raw_readl
+ (afp->mgmt_base +
+ (phy ? PCIEX8MGMT_PHY1_CR_PARA_ACK :
+ PCIEX8MGMT_PHY0_CR_PARA_ACK))) {
+ cpu_relax();
+ cnt++;
+ if (cnt > PCIE_PHY_MAX_RETRY_CNT) {
+ dev_err(dev, "PCIE phy doesn't enter idle state.\n");
+ break;
+ }
+ }
+}
+
+static void fu740_pcie_init_phy(struct fu740_pcie *afp)
+{
+ int lane;
+
+ /* enable phy cr_para_sel interfaces */
+ __raw_writel(0x1, afp->mgmt_base + PCIEX8MGMT_PHY0_CR_PARA_SEL);
+ __raw_writel(0x1, afp->mgmt_base + PCIEX8MGMT_PHY1_CR_PARA_SEL);
+
+ /* wait 10 cr_para cycles */
+ msleep(1);
+
+ /* set PHY AC termination mode */
+ for (lane = 0; lane < 4; lane++) {
+ fu740_phyregreadwrite(0, 1,
+ 0x1008 + (0x100 * lane),
+ 0x0e21, NULL, afp);
+ fu740_phyregreadwrite(1, 1,
+ 0x1008 + (0x100 * lane),
+ 0x0e21, NULL, afp);
+ }
+
+}
+
+static void fu740_pcie_ltssm_enable(struct device *dev)
+{
+ struct fu740_pcie *afp = dev_get_drvdata(dev);
+
+ /* Enable LTSSM */
+ __raw_writel(0x1, afp->mgmt_base + PCIEX8MGMT_APP_LTSSM_ENABLE);
+}
+
+static int fu740_pcie_start_link(struct dw_pcie *pci)
+{
+ struct device *dev = pci->dev;
+ u8 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
+ u32 tmp;
+ int ret;
+
+ /*
+ * Force Gen1 operation when starting the link. In case the link is
+ * started in Gen2 mode, there is a possibility the devices on the
+ * bus will not be detected at all. This happens with PCIe switches.
+ */
+ dw_pcie_dbi_ro_wr_en(pci);
+ tmp = dw_pcie_readl_dbi(pci, offset + PCI_EXP_LNKCAP);
+ tmp &= ~PCI_EXP_LNKCAP_SLS;
+ tmp |= PCI_EXP_LNKCAP_SLS_2_5GB;
+ dw_pcie_writel_dbi(pci, offset + PCI_EXP_LNKCAP, tmp);
+ dw_pcie_dbi_ro_wr_dis(pci);
+
+ /* Start LTSSM. */
+ fu740_pcie_ltssm_enable(dev);
+
+ ret = dw_pcie_wait_for_link(pci);
+ if (ret)
+ goto err_reset_phy;
+
+ /* Now set it to operate in Gen3 */
+ dw_pcie_dbi_ro_wr_en(pci);
+ tmp = dw_pcie_readl_dbi(pci, offset + PCI_EXP_LNKCAP);
+ tmp &= ~PCI_EXP_LNKCAP_SLS;
+ tmp |= PCI_EXP_LNKCAP_SLS_8_0GB;
+ dw_pcie_writel_dbi(pci, offset + PCI_EXP_LNKCAP, tmp);
+ /* Enable DIRECTED SPEED CHANGE bit of GEN2_CTRL_OFF register */
+ tmp = dw_pcie_readl_dbi(pci, PCIE_PL_GEN2_CTRL_OFF);
+ tmp |= PCIE_PL_DIRECTED_SPEED_CHANGE_OFF;
+ dw_pcie_writel_dbi(pci, PCIE_PL_GEN2_CTRL_OFF, tmp);
+ dw_pcie_dbi_ro_wr_dis(pci);
+
+ ret = dw_pcie_wait_for_link(pci);
+ if (ret)
+ goto err_reset_phy;
+
+ /*
+ * Reenable DIRECTED SPEED CHANGE.
+ *
+ * You need to set this bit after each speed change, but after
+ * reaching G1, setting it once doesn't seem to work (it reaches G3
+ * equalization states and then times out, falls back to G1). But
+ * If after that, you set it again, it then reaches G3 perfectly
+ * fine.
+ */
+ dw_pcie_dbi_ro_wr_en(pci);
+ tmp = dw_pcie_readl_dbi(pci, PCIE_PL_GEN2_CTRL_OFF);
+ tmp |= PCIE_PL_DIRECTED_SPEED_CHANGE_OFF;
+ dw_pcie_writel_dbi(pci, PCIE_PL_GEN2_CTRL_OFF, tmp);
+ dw_pcie_dbi_ro_wr_dis(pci);
+
+ ret = dw_pcie_wait_for_link(pci);
+ if (ret)
+ goto err_reset_phy;
+
+ tmp = dw_pcie_readw_dbi(pci, offset + PCI_EXP_LNKSTA);
+ dev_info(dev, "Link up, Gen%i\n", tmp & PCI_EXP_LNKSTA_CLS);
+ return 0;
+
+ err_reset_phy:
+ dev_err(dev, "Failed to bring link up!\n"
+ "PHY DEBUG_R0=0x%08x DEBUG_R1=0x%08x\n",
+ dw_pcie_readl_dbi(pci, PCIE_PORT_DEBUG0),
+ dw_pcie_readl_dbi(pci, PCIE_PORT_DEBUG1));
+ return ret;
+}
+
+static int fu740_pcie_host_init(struct pcie_port *pp)
+{
+ struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+ struct fu740_pcie *afp = to_fu740_pcie(pci);
+ struct device *dev = pci->dev;
+ int ret = 0;
+
+ /* power on reset */
+ fu740_pcie_drive_perstn(afp);
+
+ /* enable pcieauxclk */
+ ret = clk_prepare_enable(afp->pcie_aux);
+ if (ret)
+ dev_err(dev, "unable to enable pcie_aux clock\n");
+
+ /*
+ * assert hold_phy_rst (hold the controller LTSSM in reset after
+ * power_up_rst_n
+ * for register programming with cr_para)
+ */
+ __raw_writel(0x1, afp->mgmt_base + PCIEX8MGMT_APP_HOLD_PHY_RST);
+
+ /* deassert power_up_rst_n */
+ ret = reset_control_deassert(afp->rst);
+ if (ret)
+ dev_err(dev, "unable to deassert pcie_power_up_rst_n\n");
+
+ fu740_pcie_init_phy(afp);
+
+ /* disable pcieauxclk */
+ clk_disable_unprepare(afp->pcie_aux);
+ /* clear hold_phy_rst */
+ __raw_writel(0x0, afp->mgmt_base + PCIEX8MGMT_APP_HOLD_PHY_RST);
+ /* enable pcieauxclk */
+ ret = clk_prepare_enable(afp->pcie_aux);
+ /* set RC mode */
+ __raw_writel(0x4, afp->mgmt_base + PCIEX8MGMT_DEVICE_TYPE);
+
+ return 0;
+}
+
+static const struct dw_pcie_host_ops fu740_pcie_host_ops = {
+ .host_init = fu740_pcie_host_init,
+};
+
+static const struct dw_pcie_ops dw_pcie_ops = {
+ .start_link = fu740_pcie_start_link,
+};
+
+static const struct dev_pm_ops fu740_pcie_pm_ops = {
+ SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(fu740_pcie_suspend_noirq,
+ fu740_pcie_resume_noirq)
+};
+
+static int fu740_pcie_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct dw_pcie *pci;
+ struct fu740_pcie *afp;
+ struct resource *mgmt_res;
+ struct device_node *node = dev->of_node;
+ int ret;
+ u16 val;
+
+ afp = devm_kzalloc(dev, sizeof(*afp), GFP_KERNEL);
+ if (!afp)
+ return -ENOMEM;
+
+ pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL);
+ if (!pci)
+ return -ENOMEM;
+
+ pci->dev = dev;
+ pci->ops = &dw_pcie_ops;
+ pci->pp.ops = &fu740_pcie_host_ops;
+
+ afp->pci = pci;
+
+ mgmt_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "mgmt");
+ if (!mgmt_res) {
+ dev_warn(dev, "missing required mgmt address range");
+ return -ENOENT;
+ }
+ afp->mgmt_base = devm_ioremap_resource(dev, mgmt_res);
+ if (IS_ERR(afp->mgmt_base))
+ return PTR_ERR(afp->mgmt_base);
+
+ /* Fetch GPIOs */
+ afp->perstn_gpio = of_get_named_gpio(node, "perstn-gpios", 0);
+ if (gpio_is_valid(afp->perstn_gpio)) {
+ ret = devm_gpio_request_one(dev, afp->perstn_gpio,
+ GPIOF_OUT_INIT_LOW, "perstn-gpios");
+ if (ret) {
+ dev_err(dev, "unable to get perstn gpio\n");
+ return ret;
+ }
+ } else if (afp->perstn_gpio == -EPROBE_DEFER) {
+ dev_err(dev, "perst-gpios EPROBE_DEFER\n");
+ return afp->perstn_gpio;
+ }
+
+ afp->pwren_gpio = of_get_named_gpio(node, "pwren-gpios", 0);
+ if (gpio_is_valid(afp->pwren_gpio)) {
+ ret = devm_gpio_request_one(dev, afp->pwren_gpio,
+ GPIOF_OUT_INIT_LOW, "pwren-gpios");
+ if (ret) {
+ dev_err(dev, "unable to get pwren gpio\n");
+ return ret;
+ }
+ } else if (afp->pwren_gpio == -EPROBE_DEFER) {
+ dev_err(dev, "pwren-gpios EPROBE_DEFER\n");
+ return afp->pwren_gpio;
+ }
+
+ /* Fetch clocks */
+ afp->pcie_aux = devm_clk_get(dev, "pcie_aux");
+ if (IS_ERR(afp->pcie_aux))
+ return dev_err_probe(dev, PTR_ERR(afp->pcie_aux),
+ "pcie_aux clock source missing or invalid\n");
+
+ /* Fetch reset */
+ afp->rst = devm_reset_control_get(dev, NULL);
+ if (IS_ERR(afp->rst))
+ return dev_err_probe(dev, PTR_ERR(afp->rst), "unable to get reset\n");
+
+ platform_set_drvdata(pdev, afp);
+
+ ret = dw_pcie_host_init(&pci->pp);
+ if (ret < 0)
+ return ret;
+
+ if (pci_msi_enabled()) {
+ u8 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_MSI);
+
+ val = dw_pcie_readw_dbi(pci, offset + PCI_MSI_FLAGS);
+ val |= PCI_MSI_FLAGS_ENABLE;
+ dw_pcie_writew_dbi(pci, offset + PCI_MSI_FLAGS, val);
+ }
+
+ return 0;
+}
+
+static void fu740_pcie_shutdown(struct platform_device *pdev)
+{
+ struct fu740_pcie *afp = platform_get_drvdata(pdev);
+
+ /* bring down link, so bootloader gets clean state in case of reboot */
+ fu740_pcie_assert_perstn(afp);
+}
+
+static const struct of_device_id fu740_pcie_of_match[] = {
+ {.compatible = "sifive,fu740-pcie"},
+ {},
+};
+
+static struct platform_driver fu740_pcie_driver = {
+ .driver = {
+ .name = "fu740-pcie",
+ .of_match_table = fu740_pcie_of_match,
+ .suppress_bind_attrs = true,
+ .pm = &fu740_pcie_pm_ops,
+ },
+ .probe = fu740_pcie_probe,
+ .shutdown = fu740_pcie_shutdown,
+};
+
+static int __init fu740_pcie_init(void)
+{
+ return platform_driver_register(&fu740_pcie_driver);
+}
+
+device_initcall(fu740_pcie_init);
--
2.30.0
Signed-off-by: Greentime Hu <[email protected]>
---
arch/riscv/boot/dts/sifive/fu740-c000.dtsi | 34 ++++++++++++++++++++++
1 file changed, 34 insertions(+)
diff --git a/arch/riscv/boot/dts/sifive/fu740-c000.dtsi b/arch/riscv/boot/dts/sifive/fu740-c000.dtsi
index d1bb22b11920..d0839739b425 100644
--- a/arch/riscv/boot/dts/sifive/fu740-c000.dtsi
+++ b/arch/riscv/boot/dts/sifive/fu740-c000.dtsi
@@ -158,6 +158,7 @@ prci: clock-controller@10000000 {
reg = <0x0 0x10000000 0x0 0x1000>;
clocks = <&hfclk>, <&rtcclk>;
#clock-cells = <1>;
+ #reset-cells = <1>;
};
uart0: serial@10010000 {
compatible = "sifive,fu740-c000-uart", "sifive,uart0";
@@ -288,5 +289,38 @@ gpio: gpio@10060000 {
clocks = <&prci PRCI_CLK_PCLK>;
status = "disabled";
};
+ pcie@e00000000 {
+ #address-cells = <3>;
+ #interrupt-cells = <1>;
+ #num-lanes = <8>;
+ #size-cells = <2>;
+ compatible = "sifive,fu740-pcie";
+ reg = <0xe 0x00000000 0x1 0x0
+ 0xd 0xf0000000 0x0 0x10000000
+ 0x0 0x100d0000 0x0 0x1000>;
+ reg-names = "dbi", "config", "mgmt";
+ device_type = "pci";
+ dma-coherent;
+ bus-range = <0x0 0xff>;
+ ranges = <0x81000000 0x0 0x60080000 0x0 0x60080000 0x0 0x10000 /* I/O */
+ 0x82000000 0x0 0x60090000 0x0 0x60090000 0x0 0xff70000 /* mem */
+ 0x82000000 0x0 0x70000000 0x0 0x70000000 0x0 0x1000000 /* mem */
+ 0xc3000000 0x20 0x00000000 0x20 0x00000000 0x20 0x00000000>; /* mem prefetchable */
+ num-lanes = <0x8>;
+ interrupts = <56 57 58 59 60 61 62 63 64>;
+ interrupt-names = "msi", "inta", "intb", "intc", "intd";
+ interrupt-parent = <&plic0>;
+ interrupt-map-mask = <0x0 0x0 0x0 0x7>;
+ interrupt-map = <0x0 0x0 0x0 0x1 &plic0 57>,
+ <0x0 0x0 0x0 0x2 &plic0 58>,
+ <0x0 0x0 0x0 0x3 &plic0 59>,
+ <0x0 0x0 0x0 0x4 &plic0 60>;
+ clock-names = "pcie_aux";
+ clocks = <&prci PRCI_CLK_PCIE_AUX>;
+ pwren-gpios = <&gpio 5 0>;
+ perstn-gpios = <&gpio 8 0>;
+ resets = <&prci 4>;
+ status = "okay";
+ };
};
};
--
2.30.0
Add PCIe host controller DT bindings of SiFive FU740.
Signed-off-by: Greentime Hu <[email protected]>
---
.../bindings/pci/sifive,fu740-pcie.yaml | 119 ++++++++++++++++++
1 file changed, 119 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pci/sifive,fu740-pcie.yaml
diff --git a/Documentation/devicetree/bindings/pci/sifive,fu740-pcie.yaml b/Documentation/devicetree/bindings/pci/sifive,fu740-pcie.yaml
new file mode 100644
index 000000000000..879ab4f80456
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/sifive,fu740-pcie.yaml
@@ -0,0 +1,119 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/sifive,fu740-pcie.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: SiFive fu740 PCIe host controller
+
+description: |
+ SiFive fu740 PCIe host controller is based on the Synopsys DesignWare
+ PCI core. It shares common features with the PCIe DesignWare core and
+ inherits common properties defined in
+ Documentation/devicetree/bindings/pci/designware-pcie.txt.
+
+maintainers:
+ - Paul Walmsley <[email protected]>
+ - Greentime Hu <[email protected]>
+
+allOf:
+ - $ref: /schemas/pci/pci-bus.yaml#
+
+properties:
+ compatible:
+ const: sifive,fu740-pcie
+
+ reg:
+ maxItems: 4
+
+ reg-names:
+ items:
+ - const: dbi
+ - const: config
+ - const: mgmt
+
+ device_type:
+ const: pci
+
+ dma-coherent:
+ description: Indicates that the PCIe IP block can ensure the coherency
+
+ bus-range:
+ description: Range of bus numbers associated with this controller.
+
+ num-lanes: true
+
+ msi-parent: true
+
+ interrupt-names:
+ items:
+ - const: msi
+ - const: inta
+ - const: intb
+ - const: intc
+ - const: intd
+
+ resets:
+ description: A phandle to the PCIe power up reset line
+
+ pwren-gpios:
+ description: Should specify the GPIO for controlling the PCI bus device power on
+
+ perstn-gpios:
+ description: Should specify the GPIO for controlling the PCI bus device reset
+
+required:
+ - compatible
+ - reg
+ - reg-names
+ - device_type
+ - dma-coherent
+ - bus-range
+ - ranges
+ - num-lanes
+ - interrupts
+ - interrupt-names
+ - interrupt-parent
+ - interrupt-map-mask
+ - interrupt-map
+ - clock-names
+ - clocks
+ - resets
+ - pwren-gpios
+ - perstn-gpios
+
+additionalProperties: false
+
+examples:
+ - |
+ pcie@e00000000 {
+ #address-cells = <3>;
+ #interrupt-cells = <1>;
+ #size-cells = <2>;
+ compatible = "sifive,fu740-pcie";
+ reg = <0xe 0x00000000 0x1 0x0
+ 0xd 0xf0000000 0x0 0x10000000
+ 0x0 0x100d0000 0x0 0x1000>;
+ reg-names = "dbi", "config", "mgmt";
+ device_type = "pci";
+ dma-coherent;
+ bus-range = <0x0 0xff>;
+ ranges = <0x81000000 0x0 0x60080000 0x0 0x60080000 0x0 0x10000 /* I/O */
+ 0x82000000 0x0 0x60090000 0x0 0x60090000 0x0 0xff70000 /* mem */
+ 0x82000000 0x0 0x70000000 0x0 0x70000000 0x0 0x1000000 /* mem */
+ 0xc3000000 0x20 0x00000000 0x20 0x00000000 0x20 0x00000000>; /* mem prefetchable */
+ num-lanes = <0x8>;
+ interrupts = <56 57 58 59 60 61 62 63 64>;
+ interrupt-names = "msi", "inta", "intb", "intc", "intd";
+ interrupt-parent = <&plic0>;
+ interrupt-map-mask = <0x0 0x0 0x0 0x7>;
+ interrupt-map = <0x0 0x0 0x0 0x1 &plic0 57>,
+ <0x0 0x0 0x0 0x2 &plic0 58>,
+ <0x0 0x0 0x0 0x3 &plic0 59>,
+ <0x0 0x0 0x0 0x4 &plic0 60>;
+ clock-names = "pcie_aux";
+ clocks = <&prci PRCI_CLK_PCIE_AUX>;
+ resets = <&prci 4>;
+ pwren-gpios = <&gpio 5 0>;
+ perstn-gpios = <&gpio 8 0>;
+ };
--
2.30.0
Here add maintainer information for SiFive FU740 PCIe driver.
Signed-off-by: Greentime Hu <[email protected]>
---
MAINTAINERS | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index bfc1b86e3e73..4da888be6e80 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13592,6 +13592,14 @@ S: Maintained
F: Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
F: drivers/pci/controller/dwc/*imx6*
+PCI DRIVER FOR FU740
+M: Paul Walmsley <[email protected]>
+M: Greentime Hu <[email protected]>
+L: [email protected]
+S: Maintained
+F: Documentation/devicetree/bindings/pci/sifive,fu740-pcie.yaml
+F: drivers/pci/controller/dwc/pcie-fu740.c
+
PCI DRIVER FOR INTEL VOLUME MANAGEMENT DEVICE (VMD)
M: Jonathan Derrick <[email protected]>
L: [email protected]
--
2.30.0
On Tue, 02 Mar 2021 18:59:15 +0800, Greentime Hu wrote:
> Add PCIe host controller DT bindings of SiFive FU740.
>
> Signed-off-by: Greentime Hu <[email protected]>
> ---
> .../bindings/pci/sifive,fu740-pcie.yaml | 119 ++++++++++++++++++
> 1 file changed, 119 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/pci/sifive,fu740-pcie.yaml
>
My bot found errors running 'make dt_binding_check' on your patch:
yamllint warnings/errors:
./Documentation/devicetree/bindings/pci/sifive,fu740-pcie.yaml:114:1: [error] syntax error: found character '\t' that cannot start any token (syntax)
dtschema/dtc warnings/errors:
make[1]: *** Deleting file 'Documentation/devicetree/bindings/pci/sifive,fu740-pcie.example.dts'
Traceback (most recent call last):
File "/usr/local/bin/dt-extract-example", line 45, in <module>
binding = yaml.load(open(args.yamlfile, encoding='utf-8').read())
File "/usr/local/lib/python3.8/dist-packages/ruamel/yaml/main.py", line 343, in load
return constructor.get_single_data()
File "/usr/local/lib/python3.8/dist-packages/ruamel/yaml/constructor.py", line 111, in get_single_data
node = self.composer.get_single_node()
File "_ruamel_yaml.pyx", line 706, in _ruamel_yaml.CParser.get_single_node
File "_ruamel_yaml.pyx", line 724, in _ruamel_yaml.CParser._compose_document
File "_ruamel_yaml.pyx", line 775, in _ruamel_yaml.CParser._compose_node
File "_ruamel_yaml.pyx", line 889, in _ruamel_yaml.CParser._compose_mapping_node
File "_ruamel_yaml.pyx", line 773, in _ruamel_yaml.CParser._compose_node
File "_ruamel_yaml.pyx", line 848, in _ruamel_yaml.CParser._compose_sequence_node
File "_ruamel_yaml.pyx", line 904, in _ruamel_yaml.CParser._parse_next_event
ruamel.yaml.scanner.ScannerError: while scanning a block scalar
in "<unicode string>", line 88, column 5
found a tab character where an indentation space is expected
in "<unicode string>", line 114, column 1
make[1]: *** [Documentation/devicetree/bindings/Makefile:20: Documentation/devicetree/bindings/pci/sifive,fu740-pcie.example.dts] Error 1
make[1]: *** Waiting for unfinished jobs....
./Documentation/devicetree/bindings/pci/sifive,fu740-pcie.yaml: while scanning a block scalar
in "<unicode string>", line 88, column 5
found a tab character where an indentation space is expected
in "<unicode string>", line 114, column 1
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/pci/sifive,fu740-pcie.yaml: ignoring, error parsing file
warning: no schema found in file: ./Documentation/devicetree/bindings/pci/sifive,fu740-pcie.yaml
make: *** [Makefile:1380: dt_binding_check] Error 2
See https://patchwork.ozlabs.org/patch/1446288
This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit.
On Tue, Mar 2, 2021 at 4:59 AM Greentime Hu <[email protected]> wrote:
>
> From: Paul Walmsley <[email protected]>
>
> Add driver for the SiFive FU740 PCIe host controller.
> This controller is based on the DesignWare PCIe core.
>
> Co-developed-by: Henry Styles <[email protected]>
> Signed-off-by: Henry Styles <[email protected]>
> Co-developed-by: Erik Danie <[email protected]>
> Signed-off-by: Erik Danie <[email protected]>
> Co-developed-by: Greentime Hu <[email protected]>
> Signed-off-by: Greentime Hu <[email protected]>
Your Sob should be last since you are sending the patch.
> Signed-off-by: Paul Walmsley <[email protected]>
And Paul's either before yours or first in the list depending on the history.
> ---
> drivers/pci/controller/dwc/Kconfig | 9 +
> drivers/pci/controller/dwc/Makefile | 1 +
> drivers/pci/controller/dwc/pcie-fu740.c | 455 ++++++++++++++++++++++++
> 3 files changed, 465 insertions(+)
> create mode 100644 drivers/pci/controller/dwc/pcie-fu740.c
>
> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> index 22c5529e9a65..0a37d21ed64e 100644
> --- a/drivers/pci/controller/dwc/Kconfig
> +++ b/drivers/pci/controller/dwc/Kconfig
> @@ -318,4 +318,13 @@ config PCIE_AL
> required only for DT-based platforms. ACPI platforms with the
> Annapurna Labs PCIe controller don't need to enable this.
>
> +config PCIE_FU740
> + bool "SiFive FU740 PCIe host controller"
> + depends on PCI_MSI_IRQ_DOMAIN
> + depends on SOC_SIFIVE || COMPILE_TEST
> + select PCIE_DW_HOST
> + help
> + Say Y here if you want PCIe controller support for the SiFive
> + FU740.
> +
> endmenu
> diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
> index a751553fa0db..625f6aaeb5b8 100644
> --- a/drivers/pci/controller/dwc/Makefile
> +++ b/drivers/pci/controller/dwc/Makefile
> @@ -5,6 +5,7 @@ obj-$(CONFIG_PCIE_DW_EP) += pcie-designware-ep.o
> obj-$(CONFIG_PCIE_DW_PLAT) += pcie-designware-plat.o
> obj-$(CONFIG_PCI_DRA7XX) += pci-dra7xx.o
> obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
> +obj-$(CONFIG_PCIE_FU740) += pcie-fu740.o
> obj-$(CONFIG_PCI_IMX6) += pci-imx6.o
> obj-$(CONFIG_PCIE_SPEAR13XX) += pcie-spear13xx.o
> obj-$(CONFIG_PCI_KEYSTONE) += pci-keystone.o
> diff --git a/drivers/pci/controller/dwc/pcie-fu740.c b/drivers/pci/controller/dwc/pcie-fu740.c
> new file mode 100644
> index 000000000000..6916eea40ea5
> --- /dev/null
> +++ b/drivers/pci/controller/dwc/pcie-fu740.c
> @@ -0,0 +1,455 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * FU740 DesignWare PCIe Controller integration
> + * Copyright (C) 2019-2021 SiFive, Inc.
> + * Paul Walmsley
> + * Greentime Hu
> + *
> + * Based in part on the i.MX6 PCIe host controller shim which is:
> + *
> + * Copyright (C) 2013 Kosagi
> + * https://www.kosagi.com
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/gpio.h>
> +#include <linux/kernel.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/of_gpio.h>
> +#include <linux/of_device.h>
> +#include <linux/pci.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/resource.h>
> +#include <linux/signal.h>
> +#include <linux/types.h>
> +#include <linux/interrupt.h>
> +#include <linux/reset.h>
> +#include <linux/pm_domain.h>
> +#include <linux/pm_runtime.h>
> +
> +#include "pcie-designware.h"
> +
> +#define to_fu740_pcie(x) dev_get_drvdata((x)->dev)
> +
> +struct fu740_pcie {
> + struct dw_pcie *pci;
> + void __iomem *mgmt_base;
> + int perstn_gpio;
> + int pwren_gpio;
Use gpio descriptors and the gpiod_* api.
> + struct clk *pcie_aux;
> + struct reset_control *rst;
> +};
> +
> +#define SIFIVE_DEVICESRESETREG 0x28
> +
> +#define PCIEX8MGMT_PERST_N 0x0
> +#define PCIEX8MGMT_APP_LTSSM_ENABLE 0x10
> +#define PCIEX8MGMT_APP_HOLD_PHY_RST 0x18
> +#define PCIEX8MGMT_DEVICE_TYPE 0x708
> +#define PCIEX8MGMT_PHY0_CR_PARA_ADDR 0x860
> +#define PCIEX8MGMT_PHY0_CR_PARA_RD_EN 0x870
> +#define PCIEX8MGMT_PHY0_CR_PARA_RD_DATA 0x878
> +#define PCIEX8MGMT_PHY0_CR_PARA_SEL 0x880
> +#define PCIEX8MGMT_PHY0_CR_PARA_WR_DATA 0x888
> +#define PCIEX8MGMT_PHY0_CR_PARA_WR_EN 0x890
> +#define PCIEX8MGMT_PHY0_CR_PARA_ACK 0x898
> +#define PCIEX8MGMT_PHY1_CR_PARA_ADDR 0x8a0
> +#define PCIEX8MGMT_PHY1_CR_PARA_RD_EN 0x8b0
> +#define PCIEX8MGMT_PHY1_CR_PARA_RD_DATA 0x8b8
> +#define PCIEX8MGMT_PHY1_CR_PARA_SEL 0x8c0
> +#define PCIEX8MGMT_PHY1_CR_PARA_WR_DATA 0x8c8
> +#define PCIEX8MGMT_PHY1_CR_PARA_WR_EN 0x8d0
> +#define PCIEX8MGMT_PHY1_CR_PARA_ACK 0x8d8
I tend to think we should split the phy part to a proper phy driver.
Just because the registers happen to be in the same register space
isn't really a good reason to combine them.
> +
> +/* PCIe Port Logic registers (memory-mapped) */
> +#define PL_OFFSET 0x700
That's all DWC controllers.
> +#define PCIE_PL_GEN2_CTRL_OFF (PL_OFFSET + 0x10c)
Also common. Pretty sure there's a define already.
> +#define PCIE_PL_DIRECTED_SPEED_CHANGE_OFF 0x20000
> +
> +#define PCIE_PHY_MAX_RETRY_CNT 1000
> +
> +static void fu740_pcie_assert_perstn(struct fu740_pcie *afp)
> +{
> + /* PERST_N GPIO */
> + if (gpio_is_valid(afp->perstn_gpio))
> + gpio_direction_output(afp->perstn_gpio, 0);
> +
> + /* Controller PERST_N */
> + __raw_writel(0x0, afp->mgmt_base + PCIEX8MGMT_PERST_N);
writel_relaxed is the preferred variant.
> +}
> +
> +static void fu740_pcie_deassert_perstn(struct fu740_pcie *afp)
> +{
> + /* Controller PERST_N */
> + __raw_writel(0x1, afp->mgmt_base + PCIEX8MGMT_PERST_N);
> + /* PERST_N GPIO */
> + if (gpio_is_valid(afp->perstn_gpio))
> + gpio_direction_output(afp->perstn_gpio, 1);
> +}
> +
> +static void fu740_pcie_power_on(struct fu740_pcie *afp)
> +{
> + if (gpio_is_valid(afp->pwren_gpio)) {
> + gpio_direction_output(afp->pwren_gpio, 1);
> + mdelay(100);
> + }
> +}
> +
> +static void fu740_pcie_drive_perstn(struct fu740_pcie *afp)
> +{
> + fu740_pcie_assert_perstn(afp);
> + fu740_pcie_power_on(afp);
> + fu740_pcie_deassert_perstn(afp);
> +}
> +
> +static void fu740_phyregreadwrite(const uint8_t phy, const uint8_t write,
> + const uint16_t addr,
> + const uint16_t wrdata, uint16_t *rddata,
> + struct fu740_pcie *afp)
I think you should split this into separate read and write functions.
> +{
> + unsigned char ack = 0;
> + unsigned int cnt = 0;
> + struct device *dev = afp->pci->dev;
> +
> + /* setup */
> + __raw_writel(addr,
> + afp->mgmt_base +
> + (phy ? PCIEX8MGMT_PHY1_CR_PARA_ADDR :
> + PCIEX8MGMT_PHY0_CR_PARA_ADDR));
> + if (write)
> + __raw_writel(wrdata,
> + afp->mgmt_base +
> + (phy ? PCIEX8MGMT_PHY1_CR_PARA_WR_DATA :
> + PCIEX8MGMT_PHY0_CR_PARA_WR_DATA));
> + if (write)
> + __raw_writel(1,
> + afp->mgmt_base +
> + (phy ? PCIEX8MGMT_PHY1_CR_PARA_WR_EN :
> + PCIEX8MGMT_PHY0_CR_PARA_WR_EN));
> + else
> + __raw_writel(1,
> + afp->mgmt_base +
> + (phy ? PCIEX8MGMT_PHY1_CR_PARA_RD_EN :
> + PCIEX8MGMT_PHY0_CR_PARA_RD_EN));
> +
> + /* wait for wait_idle */
> + do {
> + if (__raw_readl
> + (afp->mgmt_base +
> + (phy ? PCIEX8MGMT_PHY1_CR_PARA_ACK :
> + PCIEX8MGMT_PHY0_CR_PARA_ACK))) {
> + ack = 1;
> + if (!write)
> + __raw_readl(afp->mgmt_base +
> + (phy ?
> + PCIEX8MGMT_PHY1_CR_PARA_RD_DATA :
> + PCIEX8MGMT_PHY0_CR_PARA_RD_DATA));
> + }
> + } while (!ack);
> +
> + /* clear */
> + if (write)
> + __raw_writel(0,
> + afp->mgmt_base +
> + (phy ? PCIEX8MGMT_PHY1_CR_PARA_WR_EN :
> + PCIEX8MGMT_PHY0_CR_PARA_WR_EN));
> + else
> + __raw_writel(0,
> + afp->mgmt_base +
> + (phy ? PCIEX8MGMT_PHY1_CR_PARA_RD_EN :
> + PCIEX8MGMT_PHY0_CR_PARA_RD_EN));
> +
> + /* wait for ~wait_idle */
> + while (__raw_readl
> + (afp->mgmt_base +
> + (phy ? PCIEX8MGMT_PHY1_CR_PARA_ACK :
> + PCIEX8MGMT_PHY0_CR_PARA_ACK))) {
> + cpu_relax();
> + cnt++;
> + if (cnt > PCIE_PHY_MAX_RETRY_CNT) {
> + dev_err(dev, "PCIE phy doesn't enter idle state.\n");
> + break;
> + }
> + }
We have helpers for this kind of loop.
> +}
> +
> +static void fu740_pcie_init_phy(struct fu740_pcie *afp)
> +{
> + int lane;
> +
> + /* enable phy cr_para_sel interfaces */
> + __raw_writel(0x1, afp->mgmt_base + PCIEX8MGMT_PHY0_CR_PARA_SEL);
> + __raw_writel(0x1, afp->mgmt_base + PCIEX8MGMT_PHY1_CR_PARA_SEL);
> +
> + /* wait 10 cr_para cycles */
> + msleep(1);
> +
> + /* set PHY AC termination mode */
> + for (lane = 0; lane < 4; lane++) {
> + fu740_phyregreadwrite(0, 1,
> + 0x1008 + (0x100 * lane),
> + 0x0e21, NULL, afp);
> + fu740_phyregreadwrite(1, 1,
> + 0x1008 + (0x100 * lane),
> + 0x0e21, NULL, afp);
> + }
> +
> +}
> +
> +static void fu740_pcie_ltssm_enable(struct device *dev)
> +{
> + struct fu740_pcie *afp = dev_get_drvdata(dev);
> +
> + /* Enable LTSSM */
> + __raw_writel(0x1, afp->mgmt_base + PCIEX8MGMT_APP_LTSSM_ENABLE);
> +}
> +
> +static int fu740_pcie_start_link(struct dw_pcie *pci)
> +{
> + struct device *dev = pci->dev;
> + u8 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> + u32 tmp;
> + int ret;
> +
> + /*
> + * Force Gen1 operation when starting the link. In case the link is
> + * started in Gen2 mode, there is a possibility the devices on the
> + * bus will not be detected at all. This happens with PCIe switches.
> + */
> + dw_pcie_dbi_ro_wr_en(pci);
> + tmp = dw_pcie_readl_dbi(pci, offset + PCI_EXP_LNKCAP);
> + tmp &= ~PCI_EXP_LNKCAP_SLS;
> + tmp |= PCI_EXP_LNKCAP_SLS_2_5GB;
> + dw_pcie_writel_dbi(pci, offset + PCI_EXP_LNKCAP, tmp);
> + dw_pcie_dbi_ro_wr_dis(pci);
> +
> + /* Start LTSSM. */
> + fu740_pcie_ltssm_enable(dev);
> +
> + ret = dw_pcie_wait_for_link(pci);
> + if (ret)
> + goto err_reset_phy;
> +
> + /* Now set it to operate in Gen3 */
> + dw_pcie_dbi_ro_wr_en(pci);
> + tmp = dw_pcie_readl_dbi(pci, offset + PCI_EXP_LNKCAP);
> + tmp &= ~PCI_EXP_LNKCAP_SLS;
> + tmp |= PCI_EXP_LNKCAP_SLS_8_0GB;
You don't support slower speeds?
We have some common DWC handling of this too, that doesn't work?
> + dw_pcie_writel_dbi(pci, offset + PCI_EXP_LNKCAP, tmp);
> + /* Enable DIRECTED SPEED CHANGE bit of GEN2_CTRL_OFF register */
> + tmp = dw_pcie_readl_dbi(pci, PCIE_PL_GEN2_CTRL_OFF);
> + tmp |= PCIE_PL_DIRECTED_SPEED_CHANGE_OFF;
> + dw_pcie_writel_dbi(pci, PCIE_PL_GEN2_CTRL_OFF, tmp);
> + dw_pcie_dbi_ro_wr_dis(pci);
> +
> + ret = dw_pcie_wait_for_link(pci);
> + if (ret)
> + goto err_reset_phy;
> +
> + /*
> + * Reenable DIRECTED SPEED CHANGE.
> + *
> + * You need to set this bit after each speed change, but after
> + * reaching G1, setting it once doesn't seem to work (it reaches G3
> + * equalization states and then times out, falls back to G1). But
> + * If after that, you set it again, it then reaches G3 perfectly
> + * fine.
> + */
> + dw_pcie_dbi_ro_wr_en(pci);
> + tmp = dw_pcie_readl_dbi(pci, PCIE_PL_GEN2_CTRL_OFF);
> + tmp |= PCIE_PL_DIRECTED_SPEED_CHANGE_OFF;
> + dw_pcie_writel_dbi(pci, PCIE_PL_GEN2_CTRL_OFF, tmp);
> + dw_pcie_dbi_ro_wr_dis(pci);
> +
> + ret = dw_pcie_wait_for_link(pci);
The DWC core does this after start_link() now.
> + if (ret)
> + goto err_reset_phy;
> +
> + tmp = dw_pcie_readw_dbi(pci, offset + PCI_EXP_LNKSTA);
> + dev_info(dev, "Link up, Gen%i\n", tmp & PCI_EXP_LNKSTA_CLS);
If we want this, it should be common.
> + return 0;
> +
> + err_reset_phy:
> + dev_err(dev, "Failed to bring link up!\n"
> + "PHY DEBUG_R0=0x%08x DEBUG_R1=0x%08x\n",
> + dw_pcie_readl_dbi(pci, PCIE_PORT_DEBUG0),
> + dw_pcie_readl_dbi(pci, PCIE_PORT_DEBUG1));
> + return ret;
> +}
> +
> +static int fu740_pcie_host_init(struct pcie_port *pp)
> +{
> + struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> + struct fu740_pcie *afp = to_fu740_pcie(pci);
> + struct device *dev = pci->dev;
> + int ret = 0;
> +
> + /* power on reset */
> + fu740_pcie_drive_perstn(afp);
> +
> + /* enable pcieauxclk */
> + ret = clk_prepare_enable(afp->pcie_aux);
> + if (ret)
> + dev_err(dev, "unable to enable pcie_aux clock\n");
> +
> + /*
> + * assert hold_phy_rst (hold the controller LTSSM in reset after
> + * power_up_rst_n
> + * for register programming with cr_para)
> + */
> + __raw_writel(0x1, afp->mgmt_base + PCIEX8MGMT_APP_HOLD_PHY_RST);
> +
> + /* deassert power_up_rst_n */
> + ret = reset_control_deassert(afp->rst);
> + if (ret)
> + dev_err(dev, "unable to deassert pcie_power_up_rst_n\n");
> +
> + fu740_pcie_init_phy(afp);
> +
> + /* disable pcieauxclk */
> + clk_disable_unprepare(afp->pcie_aux);
> + /* clear hold_phy_rst */
> + __raw_writel(0x0, afp->mgmt_base + PCIEX8MGMT_APP_HOLD_PHY_RST);
> + /* enable pcieauxclk */
> + ret = clk_prepare_enable(afp->pcie_aux);
> + /* set RC mode */
> + __raw_writel(0x4, afp->mgmt_base + PCIEX8MGMT_DEVICE_TYPE);
> +
> + return 0;
> +}
> +
> +static const struct dw_pcie_host_ops fu740_pcie_host_ops = {
> + .host_init = fu740_pcie_host_init,
> +};
> +
> +static const struct dw_pcie_ops dw_pcie_ops = {
> + .start_link = fu740_pcie_start_link,
> +};
> +
> +static const struct dev_pm_ops fu740_pcie_pm_ops = {
> + SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(fu740_pcie_suspend_noirq,
> + fu740_pcie_resume_noirq)
> +};
> +
> +static int fu740_pcie_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct dw_pcie *pci;
> + struct fu740_pcie *afp;
> + struct resource *mgmt_res;
> + struct device_node *node = dev->of_node;
> + int ret;
> + u16 val;
> +
> + afp = devm_kzalloc(dev, sizeof(*afp), GFP_KERNEL);
> + if (!afp)
> + return -ENOMEM;
> +
> + pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL);
Embed dw_pcie in fu740_pcie and do 1 alloc.
> + if (!pci)
> + return -ENOMEM;
> +
> + pci->dev = dev;
> + pci->ops = &dw_pcie_ops;
> + pci->pp.ops = &fu740_pcie_host_ops;
> +
> + afp->pci = pci;
> +
> + mgmt_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "mgmt");
> + if (!mgmt_res) {
> + dev_warn(dev, "missing required mgmt address range");
> + return -ENOENT;
> + }
> + afp->mgmt_base = devm_ioremap_resource(dev, mgmt_res);
Use devm_platform_ioremap_resource_byname
> + if (IS_ERR(afp->mgmt_base))
> + return PTR_ERR(afp->mgmt_base);
> +
> + /* Fetch GPIOs */
> + afp->perstn_gpio = of_get_named_gpio(node, "perstn-gpios", 0);
> + if (gpio_is_valid(afp->perstn_gpio)) {
> + ret = devm_gpio_request_one(dev, afp->perstn_gpio,
> + GPIOF_OUT_INIT_LOW, "perstn-gpios");
> + if (ret) {
> + dev_err(dev, "unable to get perstn gpio\n");
> + return ret;
> + }
> + } else if (afp->perstn_gpio == -EPROBE_DEFER) {
> + dev_err(dev, "perst-gpios EPROBE_DEFER\n");
> + return afp->perstn_gpio;
> + }
GPIOs are optional? That's not what the binding says.
> +
> + afp->pwren_gpio = of_get_named_gpio(node, "pwren-gpios", 0);
> + if (gpio_is_valid(afp->pwren_gpio)) {
> + ret = devm_gpio_request_one(dev, afp->pwren_gpio,
> + GPIOF_OUT_INIT_LOW, "pwren-gpios");
> + if (ret) {
> + dev_err(dev, "unable to get pwren gpio\n");
> + return ret;
> + }
> + } else if (afp->pwren_gpio == -EPROBE_DEFER) {
> + dev_err(dev, "pwren-gpios EPROBE_DEFER\n");
> + return afp->pwren_gpio;
> + }
> +
> + /* Fetch clocks */
> + afp->pcie_aux = devm_clk_get(dev, "pcie_aux");
> + if (IS_ERR(afp->pcie_aux))
> + return dev_err_probe(dev, PTR_ERR(afp->pcie_aux),
> + "pcie_aux clock source missing or invalid\n");
> +
> + /* Fetch reset */
> + afp->rst = devm_reset_control_get(dev, NULL);
> + if (IS_ERR(afp->rst))
> + return dev_err_probe(dev, PTR_ERR(afp->rst), "unable to get reset\n");
> +
> + platform_set_drvdata(pdev, afp);
> +
> + ret = dw_pcie_host_init(&pci->pp);
> + if (ret < 0)
> + return ret;
> +
> + if (pci_msi_enabled()) {
> + u8 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_MSI);
> +
> + val = dw_pcie_readw_dbi(pci, offset + PCI_MSI_FLAGS);
> + val |= PCI_MSI_FLAGS_ENABLE;
> + dw_pcie_writew_dbi(pci, offset + PCI_MSI_FLAGS, val);
Are you sure this is needed? It's suspect that only i.MX6 needs it.
> + }
> +
> + return 0;
> +}
> +
> +static void fu740_pcie_shutdown(struct platform_device *pdev)
> +{
> + struct fu740_pcie *afp = platform_get_drvdata(pdev);
> +
> + /* bring down link, so bootloader gets clean state in case of reboot */
> + fu740_pcie_assert_perstn(afp);
> +}
> +
> +static const struct of_device_id fu740_pcie_of_match[] = {
> + {.compatible = "sifive,fu740-pcie"},
> + {},
> +};
> +
> +static struct platform_driver fu740_pcie_driver = {
> + .driver = {
> + .name = "fu740-pcie",
> + .of_match_table = fu740_pcie_of_match,
> + .suppress_bind_attrs = true,
> + .pm = &fu740_pcie_pm_ops,
> + },
> + .probe = fu740_pcie_probe,
> + .shutdown = fu740_pcie_shutdown,
> +};
> +
> +static int __init fu740_pcie_init(void)
> +{
> + return platform_driver_register(&fu740_pcie_driver);
> +}
> +
> +device_initcall(fu740_pcie_init);
Needs to be early? Why?
Make the subject like this:
PCI: fu740: Add SiFive FU740 PCIe host controller driver
since you're adding a "fu740" driver, not a "designware" driver.
Future commits will then look like:
PCI: fu740: ...
On Tue, Mar 02, 2021 at 06:59:16PM +0800, Greentime Hu wrote:
> From: Paul Walmsley <[email protected]>
>
> Add driver for the SiFive FU740 PCIe host controller.
> This controller is based on the DesignWare PCIe core.
>
> Co-developed-by: Henry Styles <[email protected]>
> Signed-off-by: Henry Styles <[email protected]>
> Co-developed-by: Erik Danie <[email protected]>
> Signed-off-by: Erik Danie <[email protected]>
> Co-developed-by: Greentime Hu <[email protected]>
> Signed-off-by: Greentime Hu <[email protected]>
> Signed-off-by: Paul Walmsley <[email protected]>
> ---
> drivers/pci/controller/dwc/Kconfig | 9 +
> drivers/pci/controller/dwc/Makefile | 1 +
> drivers/pci/controller/dwc/pcie-fu740.c | 455 ++++++++++++++++++++++++
> 3 files changed, 465 insertions(+)
> create mode 100644 drivers/pci/controller/dwc/pcie-fu740.c
>
> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> index 22c5529e9a65..0a37d21ed64e 100644
> --- a/drivers/pci/controller/dwc/Kconfig
> +++ b/drivers/pci/controller/dwc/Kconfig
> @@ -318,4 +318,13 @@ config PCIE_AL
> required only for DT-based platforms. ACPI platforms with the
> Annapurna Labs PCIe controller don't need to enable this.
>
> +config PCIE_FU740
> + bool "SiFive FU740 PCIe host controller"
> + depends on PCI_MSI_IRQ_DOMAIN
> + depends on SOC_SIFIVE || COMPILE_TEST
> + select PCIE_DW_HOST
> + help
> + Say Y here if you want PCIe controller support for the SiFive
> + FU740.
> +
> endmenu
> diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
> index a751553fa0db..625f6aaeb5b8 100644
> --- a/drivers/pci/controller/dwc/Makefile
> +++ b/drivers/pci/controller/dwc/Makefile
> @@ -5,6 +5,7 @@ obj-$(CONFIG_PCIE_DW_EP) += pcie-designware-ep.o
> obj-$(CONFIG_PCIE_DW_PLAT) += pcie-designware-plat.o
> obj-$(CONFIG_PCI_DRA7XX) += pci-dra7xx.o
> obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
> +obj-$(CONFIG_PCIE_FU740) += pcie-fu740.o
> obj-$(CONFIG_PCI_IMX6) += pci-imx6.o
> obj-$(CONFIG_PCIE_SPEAR13XX) += pcie-spear13xx.o
> obj-$(CONFIG_PCI_KEYSTONE) += pci-keystone.o
> diff --git a/drivers/pci/controller/dwc/pcie-fu740.c b/drivers/pci/controller/dwc/pcie-fu740.c
> new file mode 100644
> index 000000000000..6916eea40ea5
> --- /dev/null
> +++ b/drivers/pci/controller/dwc/pcie-fu740.c
> @@ -0,0 +1,455 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * FU740 DesignWare PCIe Controller integration
> + * Copyright (C) 2019-2021 SiFive, Inc.
> + * Paul Walmsley
> + * Greentime Hu
> + *
> + * Based in part on the i.MX6 PCIe host controller shim which is:
> + *
> + * Copyright (C) 2013 Kosagi
> + * https://www.kosagi.com
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/gpio.h>
> +#include <linux/kernel.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/of_gpio.h>
> +#include <linux/of_device.h>
> +#include <linux/pci.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/resource.h>
> +#include <linux/signal.h>
> +#include <linux/types.h>
> +#include <linux/interrupt.h>
> +#include <linux/reset.h>
> +#include <linux/pm_domain.h>
> +#include <linux/pm_runtime.h>
> +
> +#include "pcie-designware.h"
> +
> +#define to_fu740_pcie(x) dev_get_drvdata((x)->dev)
> +
> +struct fu740_pcie {
> + struct dw_pcie *pci;
> + void __iomem *mgmt_base;
> + int perstn_gpio;
> + int pwren_gpio;
> + struct clk *pcie_aux;
> + struct reset_control *rst;
> +};
> +
> +#define SIFIVE_DEVICESRESETREG 0x28
> +
> +#define PCIEX8MGMT_PERST_N 0x0
> +#define PCIEX8MGMT_APP_LTSSM_ENABLE 0x10
> +#define PCIEX8MGMT_APP_HOLD_PHY_RST 0x18
> +#define PCIEX8MGMT_DEVICE_TYPE 0x708
> +#define PCIEX8MGMT_PHY0_CR_PARA_ADDR 0x860
> +#define PCIEX8MGMT_PHY0_CR_PARA_RD_EN 0x870
> +#define PCIEX8MGMT_PHY0_CR_PARA_RD_DATA 0x878
> +#define PCIEX8MGMT_PHY0_CR_PARA_SEL 0x880
> +#define PCIEX8MGMT_PHY0_CR_PARA_WR_DATA 0x888
> +#define PCIEX8MGMT_PHY0_CR_PARA_WR_EN 0x890
> +#define PCIEX8MGMT_PHY0_CR_PARA_ACK 0x898
> +#define PCIEX8MGMT_PHY1_CR_PARA_ADDR 0x8a0
> +#define PCIEX8MGMT_PHY1_CR_PARA_RD_EN 0x8b0
> +#define PCIEX8MGMT_PHY1_CR_PARA_RD_DATA 0x8b8
> +#define PCIEX8MGMT_PHY1_CR_PARA_SEL 0x8c0
> +#define PCIEX8MGMT_PHY1_CR_PARA_WR_DATA 0x8c8
> +#define PCIEX8MGMT_PHY1_CR_PARA_WR_EN 0x8d0
> +#define PCIEX8MGMT_PHY1_CR_PARA_ACK 0x8d8
> +
> +/* PCIe Port Logic registers (memory-mapped) */
> +#define PL_OFFSET 0x700
> +#define PCIE_PL_GEN2_CTRL_OFF (PL_OFFSET + 0x10c)
> +#define PCIE_PL_DIRECTED_SPEED_CHANGE_OFF 0x20000
> +
> +#define PCIE_PHY_MAX_RETRY_CNT 1000
> +
> +static void fu740_pcie_assert_perstn(struct fu740_pcie *afp)
> +{
> + /* PERST_N GPIO */
> + if (gpio_is_valid(afp->perstn_gpio))
> + gpio_direction_output(afp->perstn_gpio, 0);
> +
> + /* Controller PERST_N */
> + __raw_writel(0x0, afp->mgmt_base + PCIEX8MGMT_PERST_N);
> +}
> +
> +static void fu740_pcie_deassert_perstn(struct fu740_pcie *afp)
> +{
> + /* Controller PERST_N */
> + __raw_writel(0x1, afp->mgmt_base + PCIEX8MGMT_PERST_N);
> + /* PERST_N GPIO */
> + if (gpio_is_valid(afp->perstn_gpio))
> + gpio_direction_output(afp->perstn_gpio, 1);
> +}
> +
> +static void fu740_pcie_power_on(struct fu740_pcie *afp)
> +{
> + if (gpio_is_valid(afp->pwren_gpio)) {
> + gpio_direction_output(afp->pwren_gpio, 1);
> + mdelay(100);
Can you add a note about where the 100ms value comes from?
> + }
> +}
> +
> +static void fu740_pcie_drive_perstn(struct fu740_pcie *afp)
> +{
> + fu740_pcie_assert_perstn(afp);
> + fu740_pcie_power_on(afp);
> + fu740_pcie_deassert_perstn(afp);
> +}
> +
> +static void fu740_phyregreadwrite(const uint8_t phy, const uint8_t write,
> + const uint16_t addr,
> + const uint16_t wrdata, uint16_t *rddata,
> + struct fu740_pcie *afp)
> +{
> + unsigned char ack = 0;
> + unsigned int cnt = 0;
> + struct device *dev = afp->pci->dev;
> +
> + /* setup */
Most of your single-line comments start with a capital letter. It's
nice if they all use the same style.
> + __raw_writel(addr,
> + afp->mgmt_base +
> + (phy ? PCIEX8MGMT_PHY1_CR_PARA_ADDR :
> + PCIEX8MGMT_PHY0_CR_PARA_ADDR));
All these "phy ? x : y" expressions are really ugly and maybe could be
factored out ahead of time.
> + if (write)
> + __raw_writel(wrdata,
> + afp->mgmt_base +
> + (phy ? PCIEX8MGMT_PHY1_CR_PARA_WR_DATA :
> + PCIEX8MGMT_PHY0_CR_PARA_WR_DATA));
> + if (write)
> + __raw_writel(1,
> + afp->mgmt_base +
> + (phy ? PCIEX8MGMT_PHY1_CR_PARA_WR_EN :
> + PCIEX8MGMT_PHY0_CR_PARA_WR_EN));
> + else
> + __raw_writel(1,
> + afp->mgmt_base +
> + (phy ? PCIEX8MGMT_PHY1_CR_PARA_RD_EN :
> + PCIEX8MGMT_PHY0_CR_PARA_RD_EN));
> +
> + /* wait for wait_idle */
> + do {
> + if (__raw_readl
> + (afp->mgmt_base +
> + (phy ? PCIEX8MGMT_PHY1_CR_PARA_ACK :
> + PCIEX8MGMT_PHY0_CR_PARA_ACK))) {
> + ack = 1;
> + if (!write)
> + __raw_readl(afp->mgmt_base +
> + (phy ?
> + PCIEX8MGMT_PHY1_CR_PARA_RD_DATA :
> + PCIEX8MGMT_PHY0_CR_PARA_RD_DATA));
> + }
> + } while (!ack);
Possible infinite loop. We should try to avoid depending on hardware
doing the right thing. If the hardware breaks or is removed, we don't
want to hang.
> + /* clear */
> + if (write)
> + __raw_writel(0,
> + afp->mgmt_base +
> + (phy ? PCIEX8MGMT_PHY1_CR_PARA_WR_EN :
> + PCIEX8MGMT_PHY0_CR_PARA_WR_EN));
> + else
> + __raw_writel(0,
> + afp->mgmt_base +
> + (phy ? PCIEX8MGMT_PHY1_CR_PARA_RD_EN :
> + PCIEX8MGMT_PHY0_CR_PARA_RD_EN));
> +
> + /* wait for ~wait_idle */
> + while (__raw_readl
> + (afp->mgmt_base +
> + (phy ? PCIEX8MGMT_PHY1_CR_PARA_ACK :
> + PCIEX8MGMT_PHY0_CR_PARA_ACK))) {
> + cpu_relax();
> + cnt++;
> + if (cnt > PCIE_PHY_MAX_RETRY_CNT) {
> + dev_err(dev, "PCIE phy doesn't enter idle state.\n");
> + break;
> + }
> + }
> +}
> +
> +static void fu740_pcie_init_phy(struct fu740_pcie *afp)
> +{
> + int lane;
> +
> + /* enable phy cr_para_sel interfaces */
> + __raw_writel(0x1, afp->mgmt_base + PCIEX8MGMT_PHY0_CR_PARA_SEL);
> + __raw_writel(0x1, afp->mgmt_base + PCIEX8MGMT_PHY1_CR_PARA_SEL);
> +
> + /* wait 10 cr_para cycles */
> + msleep(1);
A reference to the spec section that requires the 1ms would be
helpful.
> + /* set PHY AC termination mode */
> + for (lane = 0; lane < 4; lane++) {
> + fu740_phyregreadwrite(0, 1,
> + 0x1008 + (0x100 * lane),
> + 0x0e21, NULL, afp);
> + fu740_phyregreadwrite(1, 1,
> + 0x1008 + (0x100 * lane),
> + 0x0e21, NULL, afp);
Rewrap to use more of the 80-column line. Each will fit on two
lines.
Maybe add #defines for the 0x1008, 0x100, 0x0e21 magic numbers? Could
compute the "0x1008 + (0x100 * lane)" expression once instead of
repeating it.
> + }
> +
> +}
> +
> +static void fu740_pcie_ltssm_enable(struct device *dev)
> +{
> + struct fu740_pcie *afp = dev_get_drvdata(dev);
> +
> + /* Enable LTSSM */
> + __raw_writel(0x1, afp->mgmt_base + PCIEX8MGMT_APP_LTSSM_ENABLE);
> +}
> +
> +static int fu740_pcie_start_link(struct dw_pcie *pci)
> +{
> + struct device *dev = pci->dev;
> + u8 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> + u32 tmp;
> + int ret;
> +
> + /*
> + * Force Gen1 operation when starting the link. In case the link is
> + * started in Gen2 mode, there is a possibility the devices on the
> + * bus will not be detected at all. This happens with PCIe switches.
> + */
> + dw_pcie_dbi_ro_wr_en(pci);
> + tmp = dw_pcie_readl_dbi(pci, offset + PCI_EXP_LNKCAP);
> + tmp &= ~PCI_EXP_LNKCAP_SLS;
> + tmp |= PCI_EXP_LNKCAP_SLS_2_5GB;
> + dw_pcie_writel_dbi(pci, offset + PCI_EXP_LNKCAP, tmp);
> + dw_pcie_dbi_ro_wr_dis(pci);
> +
> + /* Start LTSSM. */
> + fu740_pcie_ltssm_enable(dev);
> +
> + ret = dw_pcie_wait_for_link(pci);
> + if (ret)
> + goto err_reset_phy;
> +
> + /* Now set it to operate in Gen3 */
> + dw_pcie_dbi_ro_wr_en(pci);
> + tmp = dw_pcie_readl_dbi(pci, offset + PCI_EXP_LNKCAP);
> + tmp &= ~PCI_EXP_LNKCAP_SLS;
> + tmp |= PCI_EXP_LNKCAP_SLS_8_0GB;
> + dw_pcie_writel_dbi(pci, offset + PCI_EXP_LNKCAP, tmp);
> + /* Enable DIRECTED SPEED CHANGE bit of GEN2_CTRL_OFF register */
> + tmp = dw_pcie_readl_dbi(pci, PCIE_PL_GEN2_CTRL_OFF);
> + tmp |= PCIE_PL_DIRECTED_SPEED_CHANGE_OFF;
> + dw_pcie_writel_dbi(pci, PCIE_PL_GEN2_CTRL_OFF, tmp);
> + dw_pcie_dbi_ro_wr_dis(pci);
> +
> + ret = dw_pcie_wait_for_link(pci);
> + if (ret)
> + goto err_reset_phy;
> +
> + /*
> + * Reenable DIRECTED SPEED CHANGE.
> + *
> + * You need to set this bit after each speed change, but after
> + * reaching G1, setting it once doesn't seem to work (it reaches G3
> + * equalization states and then times out, falls back to G1). But
> + * If after that, you set it again, it then reaches G3 perfectly
s/If after/if after/
> + * fine.
> + */
> + dw_pcie_dbi_ro_wr_en(pci);
> + tmp = dw_pcie_readl_dbi(pci, PCIE_PL_GEN2_CTRL_OFF);
> + tmp |= PCIE_PL_DIRECTED_SPEED_CHANGE_OFF;
> + dw_pcie_writel_dbi(pci, PCIE_PL_GEN2_CTRL_OFF, tmp);
> + dw_pcie_dbi_ro_wr_dis(pci);
> +
> + ret = dw_pcie_wait_for_link(pci);
> + if (ret)
> + goto err_reset_phy;
> +
> + tmp = dw_pcie_readw_dbi(pci, offset + PCI_EXP_LNKSTA);
> + dev_info(dev, "Link up, Gen%i\n", tmp & PCI_EXP_LNKSTA_CLS);
> + return 0;
> +
> + err_reset_phy:
> + dev_err(dev, "Failed to bring link up!\n"
> + "PHY DEBUG_R0=0x%08x DEBUG_R1=0x%08x\n",
> + dw_pcie_readl_dbi(pci, PCIE_PORT_DEBUG0),
> + dw_pcie_readl_dbi(pci, PCIE_PORT_DEBUG1));
> + return ret;
> +}
> +
> +static int fu740_pcie_host_init(struct pcie_port *pp)
> +{
> + struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> + struct fu740_pcie *afp = to_fu740_pcie(pci);
> + struct device *dev = pci->dev;
> + int ret = 0;
Unnecessary init of "ret".
> +
> + /* power on reset */
> + fu740_pcie_drive_perstn(afp);
> +
> + /* enable pcieauxclk */
> + ret = clk_prepare_enable(afp->pcie_aux);
> + if (ret)
> + dev_err(dev, "unable to enable pcie_aux clock\n");
> +
> + /*
> + * assert hold_phy_rst (hold the controller LTSSM in reset after
> + * power_up_rst_n
> + * for register programming with cr_para)
Rewrap text to fill lines.
> + */
> + __raw_writel(0x1, afp->mgmt_base + PCIEX8MGMT_APP_HOLD_PHY_RST);
> +
> + /* deassert power_up_rst_n */
> + ret = reset_control_deassert(afp->rst);
> + if (ret)
> + dev_err(dev, "unable to deassert pcie_power_up_rst_n\n");
> +
> + fu740_pcie_init_phy(afp);
> +
> + /* disable pcieauxclk */
> + clk_disable_unprepare(afp->pcie_aux);
> + /* clear hold_phy_rst */
> + __raw_writel(0x0, afp->mgmt_base + PCIEX8MGMT_APP_HOLD_PHY_RST);
> + /* enable pcieauxclk */
> + ret = clk_prepare_enable(afp->pcie_aux);
> + /* set RC mode */
> + __raw_writel(0x4, afp->mgmt_base + PCIEX8MGMT_DEVICE_TYPE);
> +
> + return 0;
> +}
> +
> +static const struct dw_pcie_host_ops fu740_pcie_host_ops = {
> + .host_init = fu740_pcie_host_init,
> +};
> +
> +static const struct dw_pcie_ops dw_pcie_ops = {
> + .start_link = fu740_pcie_start_link,
> +};
> +
> +static const struct dev_pm_ops fu740_pcie_pm_ops = {
> + SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(fu740_pcie_suspend_noirq,
> + fu740_pcie_resume_noirq)
fu740_pcie_suspend_noirq() and fu740_pcie_resume_noirq() don't exist.
Am I missing something?
> +};
> +
> +static int fu740_pcie_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct dw_pcie *pci;
> + struct fu740_pcie *afp;
> + struct resource *mgmt_res;
> + struct device_node *node = dev->of_node;
> + int ret;
> + u16 val;
> +
> + afp = devm_kzalloc(dev, sizeof(*afp), GFP_KERNEL);
> + if (!afp)
> + return -ENOMEM;
> +
> + pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL);
> + if (!pci)
> + return -ENOMEM;
> +
> + pci->dev = dev;
> + pci->ops = &dw_pcie_ops;
> + pci->pp.ops = &fu740_pcie_host_ops;
> +
> + afp->pci = pci;
> +
> + mgmt_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "mgmt");
Does every driver make up a new name for this register space? I see
"mem", "cfg", "app", "dbics", "regs", "config", "ctrl", "dbi", "dbi2",
"addr_space", "atu", "cs", "csr", breg", "cfg", "pcireg", etc. I know
there *are* different kinds of registers that need different names,
but I don't believe there are *this many* different kinds. It'd be
nice if different drivers could use the same name for the same
registers.
> + if (!mgmt_res) {
> + dev_warn(dev, "missing required mgmt address range");
> + return -ENOENT;
> + }
> + afp->mgmt_base = devm_ioremap_resource(dev, mgmt_res);
> + if (IS_ERR(afp->mgmt_base))
> + return PTR_ERR(afp->mgmt_base);
> +
> + /* Fetch GPIOs */
> + afp->perstn_gpio = of_get_named_gpio(node, "perstn-gpios", 0);
> + if (gpio_is_valid(afp->perstn_gpio)) {
> + ret = devm_gpio_request_one(dev, afp->perstn_gpio,
> + GPIOF_OUT_INIT_LOW, "perstn-gpios");
> + if (ret) {
> + dev_err(dev, "unable to get perstn gpio\n");
Maybe make the message match the DT property, i.e., "perstn-gpios"?
> + return ret;
> + }
> + } else if (afp->perstn_gpio == -EPROBE_DEFER) {
> + dev_err(dev, "perst-gpios EPROBE_DEFER\n");
> + return afp->perstn_gpio;
> + }
This structure is a little awkward, partly because it makes the implicit
assumption that -EPROBE_DEFER is not a valid GPIO number. I think
this structure would be better:
afp->perstn_gpio = of_get_named_gpio(...);
if (afp->perstn_gpio == -EPROBE_DEFER)
return perstn_gpio;
if (gpio_is_valid(afp->perstn_gpio)) {
ret = devm_gpio_request_one(...);
if (ret)
return ret;
}
Also, similar code in mvebu_pcie_parse_port() suggests that
devm_gpio_request_one() itself can return -EPROBE_DEFER, which seems
to be true. I have no idea whether you want to do anything special
with -EPROBE_DEFER in that case here.
> + afp->pwren_gpio = of_get_named_gpio(node, "pwren-gpios", 0);
> + if (gpio_is_valid(afp->pwren_gpio)) {
> + ret = devm_gpio_request_one(dev, afp->pwren_gpio,
> + GPIOF_OUT_INIT_LOW, "pwren-gpios");
> + if (ret) {
> + dev_err(dev, "unable to get pwren gpio\n");
> + return ret;
> + }
> + } else if (afp->pwren_gpio == -EPROBE_DEFER) {
> + dev_err(dev, "pwren-gpios EPROBE_DEFER\n");
> + return afp->pwren_gpio;
> + }
> +
> + /* Fetch clocks */
> + afp->pcie_aux = devm_clk_get(dev, "pcie_aux");
> + if (IS_ERR(afp->pcie_aux))
> + return dev_err_probe(dev, PTR_ERR(afp->pcie_aux),
> + "pcie_aux clock source missing or invalid\n");
> +
> + /* Fetch reset */
> + afp->rst = devm_reset_control_get(dev, NULL);
> + if (IS_ERR(afp->rst))
> + return dev_err_probe(dev, PTR_ERR(afp->rst), "unable to get reset\n");
> +
> + platform_set_drvdata(pdev, afp);
> +
> + ret = dw_pcie_host_init(&pci->pp);
> + if (ret < 0)
> + return ret;
> +
> + if (pci_msi_enabled()) {
> + u8 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_MSI);
> +
> + val = dw_pcie_readw_dbi(pci, offset + PCI_MSI_FLAGS);
> + val |= PCI_MSI_FLAGS_ENABLE;
> + dw_pcie_writew_dbi(pci, offset + PCI_MSI_FLAGS, val);
> + }
> +
> + return 0;
> +}
> +
> +static void fu740_pcie_shutdown(struct platform_device *pdev)
> +{
> + struct fu740_pcie *afp = platform_get_drvdata(pdev);
> +
> + /* bring down link, so bootloader gets clean state in case of reboot */
> + fu740_pcie_assert_perstn(afp);
> +}
> +
> +static const struct of_device_id fu740_pcie_of_match[] = {
> + {.compatible = "sifive,fu740-pcie"},
Typically there are a couple spaces inserted here, i.e.,
{ .compatible = "sifive,fu740-pcie", },
> + {},
> +};
> +
> +static struct platform_driver fu740_pcie_driver = {
> + .driver = {
> + .name = "fu740-pcie",
> + .of_match_table = fu740_pcie_of_match,
> + .suppress_bind_attrs = true,
> + .pm = &fu740_pcie_pm_ops,
> + },
Typical indentation would remove a tab before the "}". Match other
drivers.
> + .probe = fu740_pcie_probe,
> + .shutdown = fu740_pcie_shutdown,
> +};
> +
> +static int __init fu740_pcie_init(void)
> +{
> + return platform_driver_register(&fu740_pcie_driver);
> +}
> +
> +device_initcall(fu740_pcie_init);
pci-imx6.c is the only other driver that uses device_initcall().
Use builtin_platform_driver() or similar if possible, as other drivers
do.
On Tue, 2021-03-02 at 18:59 +0800, Greentime Hu wrote:
[...]
> +static int fu740_pcie_probe(struct platform_device *pdev)
> +{
[...]
> + /* Fetch reset */
> + afp->rst = devm_reset_control_get(dev, NULL);
Please use
afp->rst = devm_reset_control_get_exclusive(dev, NULL);
regards
Philipp