2018-09-21 06:03:52

by Hanjie Lin

[permalink] [raw]
Subject: [RESEND PATCH v3 0/2] add the Amlogic Meson PCIe controller driver

This is v3 post RESEND: [2]
- add Rob's Reviewed-by to patch 1/2
- add more to&cc list suggested by get_maintainer.pl

The Amlogic Meson PCIe host controller is based on the Synopsys DesignWare
PCI core. This patchset add the driver and dt-bindings of the controller.

Till patch v2, we have a dedicated phy driver which only process reset job.
It's too overkill so we integrate phy driver into the controller.

Changes since v2: [1]
- abandon phy driver, move reset to the controller
- use devm_add_action_or_reset() to use clock res
- format correcting

Changes since v1: [0]
- use gpio lib instead open code
- move 'apb' and 'port' reset from phy driver
- format correcting

[0] : https://lkml.kernel.org/r/[email protected]
[1] : https://lkml.kernel.org/r/[email protected]
[2] : https://lkml.kernel.org/r/[email protected]

Yue Wang (2):
dt-bindings: PCI: meson: add DT bindings for Amlogic Meson PCIe
controller
PCI: meson: add the Amlogic Meson PCIe controller driver

.../devicetree/bindings/pci/amlogic,meson-pcie.txt | 70 +++
drivers/pci/controller/dwc/Kconfig | 12 +
drivers/pci/controller/dwc/Makefile | 1 +
drivers/pci/controller/dwc/pci-meson.c | 617 +++++++++++++++++++++
4 files changed, 700 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pci/amlogic,meson-pcie.txt
create mode 100644 drivers/pci/controller/dwc/pci-meson.c

--
2.7.4



2018-09-21 06:03:54

by Hanjie Lin

[permalink] [raw]
Subject: [RESEND PATCH v3 1/2] dt-bindings: PCI: meson: add DT bindings for Amlogic Meson PCIe controller

From: Yue Wang <[email protected]>

The Amlogic Meson PCIe host controller is based on the Synopsys DesignWare
PCI core. This patch adds documentation for the DT bindings in Meson PCIe
controller.

Signed-off-by: Yue Wang <[email protected]>
Signed-off-by: Hanjie Lin <[email protected]>
Reviewed-by: Rob Herring <[email protected]>

---
.../devicetree/bindings/pci/amlogic,meson-pcie.txt | 70 ++++++++++++++++++++++
1 file changed, 70 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pci/amlogic,meson-pcie.txt

diff --git a/Documentation/devicetree/bindings/pci/amlogic,meson-pcie.txt b/Documentation/devicetree/bindings/pci/amlogic,meson-pcie.txt
new file mode 100644
index 0000000..12b18f8
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/amlogic,meson-pcie.txt
@@ -0,0 +1,70 @@
+Amlogic Meson AXG DWC PCIE SoC controller
+
+Amlogic Meson PCIe host controller is based on the Synopsys DesignWare PCI core.
+It shares common functions with the PCIe DesignWare core driver and
+inherits common properties defined in
+Documentation/devicetree/bindings/pci/designware-pci.txt.
+
+Additional properties are described here:
+
+Required properties:
+- compatible:
+ should contain "amlogic,axg-pcie" to identify the core.
+- reg:
+ should contain the configuration address space.
+- reg-names: Must be
+ - "elbi" External local bus interface registers
+ - "cfg" Meson specific registers
+ - "phy" Meson PCIE PHY registers
+ - "config" PCIe configuration space
+- reset-gpios: The GPIO to generate PCIe PERST# assert and deassert signal.
+- clocks: Must contain an entry for each entry in clock-names.
+- clock-names: Must include the following entries:
+ - "pclk" PCIe GEN 100M PLL clock
+ - "port" PCIe_x(A or B) RC clock gate
+ - "general" PCIe Phy clock
+ - "mipi" PCIe_x(A or B) 100M ref clock gate
+- resets: phandle to the reset lines.
+- reset-names: must contain "phy" "port" and "apb"
+ - "phy" Share PHY reset
+ - "port" Port A or B reset
+ - "apb" Share APB reset
+- device_type:
+ should be "pci". As specified in designware-pcie.txt
+
+
+Example configuration:
+
+ pcie: pcie@f9800000 {
+ compatible = "amlogic,axg-pcie", "snps,dw-pcie";
+ reg = <0x0 0xf9800000 0x0 0x400000
+ 0x0 0xff646000 0x0 0x2000
+ 0x0 0xff644000 0x0 0x2000
+ 0x0 0xf9f00000 0x0 0x100000>;
+ reg-names = "elbi", "cfg", "phy", "config";
+ reset-gpios = <&gpio GPIOX_19 GPIO_ACTIVE_HIGH>;
+ interrupts = <GIC_SPI 177 IRQ_TYPE_EDGE_RISING>;
+ #interrupt-cells = <1>;
+ interrupt-map-mask = <0 0 0 0>;
+ interrupt-map = <0 0 0 0 &gic GIC_SPI 179 IRQ_TYPE_EDGE_RISING>;
+ bus-range = <0x0 0xff>;
+ #address-cells = <3>;
+ #size-cells = <2>;
+ device_type = "pci";
+ ranges = <0x82000000 0 0 0x0 0xf9c00000 0 0x00300000>;
+
+ clocks = <&clkc CLKID_USB
+ &clkc CLKID_MIPI_ENABLE
+ &clkc CLKID_PCIE_A
+ &clkc CLKID_PCIE_CML_EN0>;
+ clock-names = "general",
+ "mipi",
+ "pclk",
+ "port";
+ resets = <&reset RESET_PCIE_PHY>,
+ <&reset RESET_PCIE_A>,
+ <&reset RESET_PCIE_APB>;
+ reset-names = "phy",
+ "port",
+ "apb";
+ };
--
2.7.4


2018-09-21 06:05:21

by Hanjie Lin

[permalink] [raw]
Subject: [RESEND PATCH v3 2/2] PCI: meson: add the Amlogic Meson PCIe controller driver

From: Yue Wang <[email protected]>

The Amlogic Meson PCIe host controller is based on the Synopsys DesignWare
PCI core. This patch adds the driver support for Meson PCIe controller.

Signed-off-by: Yue Wang <[email protected]>
Signed-off-by: Hanjie Lin <[email protected]>
---
drivers/pci/controller/dwc/Kconfig | 12 +
drivers/pci/controller/dwc/Makefile | 1 +
drivers/pci/controller/dwc/pci-meson.c | 617 +++++++++++++++++++++++++++++++++
3 files changed, 630 insertions(+)
create mode 100644 drivers/pci/controller/dwc/pci-meson.c

diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
index 91b0194..6cb36f6 100644
--- a/drivers/pci/controller/dwc/Kconfig
+++ b/drivers/pci/controller/dwc/Kconfig
@@ -193,4 +193,16 @@ config PCIE_HISI_STB
help
Say Y here if you want PCIe controller support on HiSilicon STB SoCs

+config PCI_MESON
+ bool "MESON PCIe controller"
+ depends on PCI
+ depends on PCI_MSI_IRQ_DOMAIN
+ select PCIEPORTBUS
+ select PCIE_DW_HOST
+ help
+ Say Y here if you want to enable PCI controller support on Amlogic
+ SoCs. The PCI controller on Amlogic is based on DesignWare hardware
+ and therefore the driver re-uses the DesignWare core functions to
+ implement the driver.
+
endmenu
diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
index 5d2ce72..cf676bd 100644
--- a/drivers/pci/controller/dwc/Makefile
+++ b/drivers/pci/controller/dwc/Makefile
@@ -14,6 +14,7 @@ obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o
obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o
obj-$(CONFIG_PCIE_KIRIN) += pcie-kirin.o
obj-$(CONFIG_PCIE_HISI_STB) += pcie-histb.o
+obj-$(CONFIG_PCI_MESON) += pci-meson.o

# The following drivers are for devices that use the generic ACPI
# pci_root.c driver but don't support standard ECAM config access.
diff --git a/drivers/pci/controller/dwc/pci-meson.c b/drivers/pci/controller/dwc/pci-meson.c
new file mode 100644
index 0000000..9c92a89
--- /dev/null
+++ b/drivers/pci/controller/dwc/pci-meson.c
@@ -0,0 +1,617 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * PCIe host controller driver for Amlogic MESON SoCs
+ *
+ * Copyright (c) 2018 Amlogic, inc.
+ * Author: Yue Wang <[email protected]>
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/of_device.h>
+#include <linux/of_gpio.h>
+#include <linux/pci.h>
+#include <linux/platform_device.h>
+#include <linux/resource.h>
+#include <linux/types.h>
+#include <linux/reset.h>
+
+#include "pcie-designware.h"
+
+#define to_meson_pcie(x) dev_get_drvdata((x)->dev)
+
+/* External local bus interface registers */
+#define PLR_OFFSET 0x700
+#define PCIE_PORT_LINK_CTRL_OFF (PLR_OFFSET + 0x10)
+#define FAST_LINK_MODE BIT(7)
+#define LINK_CAPABLE_MASK GENMASK(21, 16)
+#define LINK_CAPABLE_X1 BIT(16)
+
+#define PCIE_GEN2_CTRL_OFF (PLR_OFFSET + 0x10c)
+#define NUM_OF_LANES_MASK GENMASK(12, 8)
+#define NUM_OF_LANES_X1 BIT(8)
+#define DIRECT_SPEED_CHANGE BIT(17)
+
+#define TYPE1_HDR_OFFSET 0x0
+#define PCIE_STATUS_COMMAND (TYPE1_HDR_OFFSET + 0x04)
+#define PCI_IO_EN BIT(0)
+#define PCI_MEM_SPACE_EN BIT(1)
+#define PCI_BUS_MASTER_EN BIT(2)
+
+#define PCIE_BASE_ADDR0 (TYPE1_HDR_OFFSET + 0x10)
+#define PCIE_BASE_ADDR1 (TYPE1_HDR_OFFSET + 0x14)
+
+#define PCIE_CAP_OFFSET 0x70
+#define PCIE_DEV_CTRL_DEV_STUS (PCIE_CAP_OFFSET + 0x08)
+#define PCIE_CAP_MAX_PAYLOAD_MASK GENMASK(7, 5)
+#define PCIE_CAP_MAX_PAYLOAD_SIZE(x) ((x) << 5)
+#define PCIE_CAP_MAX_READ_REQ_MASK GENMASK(14, 12)
+#define PCIE_CAP_MAX_READ_REQ_SIZE(x) ((x) << 12)
+
+#define PCI_CLASS_REVISION_MASK GENMASK(7, 0)
+
+/* PCIe specific config registers */
+#define PCIE_CFG0 0x0
+#define APP_LTSSM_ENABLE BIT(7)
+
+#define PCIE_CFG_STATUS12 0x30
+#define IS_SMLH_LINK_UP(x) ((x) & (1 << 6))
+#define IS_RDLH_LINK_UP(x) ((x) & (1 << 16))
+#define IS_LTSSM_UP(x) ((((x) >> 10) & 0x1f) == 0x11)
+
+#define PCIE_CFG_STATUS17 0x44
+#define PM_CURRENT_STATE(x) (((x) >> 7) & 0x1)
+
+#define WAIT_LINKUP_TIMEOUT 2000
+#define PORT_CLK_RATE 100000000UL
+#define MAX_PAYLOAD_SIZE 256
+#define MAX_READ_REQ_SIZE 256
+#define MESON_PCIE_PHY_POWERUP 0x1c
+#define PCIE_RESET_DELAY 500
+#define PCIE_SHARED_RESET 1
+#define PCIE_NORMAL_RESET 0
+
+enum pcie_data_rate {
+ PCIE_GEN1,
+ PCIE_GEN2,
+ PCIE_GEN3,
+ PCIE_GEN4
+};
+
+struct meson_pcie_mem_res {
+ void __iomem *elbi_base; /* DT 0th resource */
+ void __iomem *cfg_base; /* DT 1nd resource */
+ void __iomem *phy_base; /* DT 2nd resource */
+};
+
+struct meson_pcie_clk_res {
+ struct clk *clk;
+ struct clk *mipi_gate;
+ struct clk *port_clk;
+ struct clk *general_clk;
+};
+
+struct meson_pcie_rc_reset {
+ struct reset_control *phy;
+ struct reset_control *port;
+ struct reset_control *apb;
+};
+
+struct meson_pcie {
+ struct dw_pcie pci;
+ struct meson_pcie_mem_res mem_res;
+ struct meson_pcie_clk_res clk_res;
+ struct meson_pcie_rc_reset mrst;
+ struct gpio_desc *reset_gpio;
+
+ enum of_gpio_flags gpio_flag;
+ int pcie_num;
+ u32 port_num;
+ u32 device_attch;
+};
+
+static struct reset_control *meson_pcie_get_reset(struct meson_pcie *mp,
+ const char *id,
+ u32 reset_type)
+{
+ struct device *dev = mp->pci.dev;
+ struct reset_control *reset;
+
+ if (reset_type == PCIE_SHARED_RESET)
+ reset = devm_reset_control_get_shared(dev, id);
+ else
+ reset = devm_reset_control_get(dev, id);
+
+ return reset;
+}
+
+static int meson_pcie_get_resets(struct meson_pcie *mp)
+{
+ struct meson_pcie_rc_reset *mrst = &mp->mrst;
+
+ mrst->phy = meson_pcie_get_reset(mp, "phy", PCIE_SHARED_RESET);
+ if (IS_ERR(mrst->phy))
+ return PTR_ERR(mrst->phy);
+ reset_control_deassert(mrst->phy);
+
+ mrst->port = meson_pcie_get_reset(mp, "port", PCIE_NORMAL_RESET);
+ if (IS_ERR(mrst->port))
+ return PTR_ERR(mrst->port);
+ reset_control_deassert(mrst->port);
+
+ mrst->apb = meson_pcie_get_reset(mp, "apb", PCIE_SHARED_RESET);
+ if (IS_ERR(mrst->apb))
+ return PTR_ERR(mrst->apb);
+ reset_control_deassert(mrst->apb);
+
+ return 0;
+}
+
+static void __iomem *meson_pcie_get_mem_shared(struct platform_device *pdev,
+ struct meson_pcie *mp,
+ const char *id)
+{
+ struct resource *res;
+
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, id);
+
+ return ioremap(res->start, resource_size(res));
+}
+
+static void __iomem *meson_pcie_get_mem(struct platform_device *pdev,
+ struct meson_pcie *mp,
+ const char *id)
+{
+ struct device *dev = mp->pci.dev;
+ struct resource *res;
+
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, id);
+
+ return devm_ioremap_resource(dev, res);
+}
+
+static int meson_pcie_get_mems(struct platform_device *pdev,
+ struct meson_pcie *mp)
+{
+ mp->mem_res.elbi_base = meson_pcie_get_mem(pdev, mp, "elbi");
+ if (IS_ERR(mp->mem_res.elbi_base))
+ return PTR_ERR(mp->mem_res.elbi_base);
+
+ mp->mem_res.cfg_base = meson_pcie_get_mem(pdev, mp, "cfg");
+ if (IS_ERR(mp->mem_res.cfg_base))
+ return PTR_ERR(mp->mem_res.cfg_base);
+
+ mp->mem_res.phy_base = meson_pcie_get_mem_shared(pdev, mp, "phy");
+ if (IS_ERR(mp->mem_res.phy_base))
+ return PTR_ERR(mp->mem_res.phy_base);
+
+ return 0;
+}
+
+static void meson_pcie_power_on(struct meson_pcie *mp)
+{
+ writel(MESON_PCIE_PHY_POWERUP, mp->mem_res.phy_base);
+}
+
+static void meson_pcie_reset(struct meson_pcie *mp)
+{
+ struct meson_pcie_rc_reset *mrst = &mp->mrst;
+
+ reset_control_assert(mrst->phy);
+ udelay(PCIE_RESET_DELAY);
+ reset_control_deassert(mrst->phy);
+ udelay(PCIE_RESET_DELAY);
+
+ reset_control_assert(mrst->port);
+ reset_control_assert(mrst->apb);
+ udelay(PCIE_RESET_DELAY);
+ reset_control_deassert(mrst->port);
+ reset_control_deassert(mrst->apb);
+ udelay(PCIE_RESET_DELAY);
+}
+
+static inline struct clk *meson_pcie_probe_clock(struct device *dev,
+ const char *id, u64 rate)
+{
+ struct clk *clk = NULL;
+ int ret;
+
+ clk = devm_clk_get(dev, id);
+ if (IS_ERR(clk))
+ return clk;
+
+ if (rate) {
+ ret = clk_set_rate(clk, rate);
+ if (ret) {
+ dev_err(dev, "set clk rate failed, ret = %d\n", ret);
+ return ERR_PTR(ret);
+ }
+ }
+
+ ret = clk_prepare_enable(clk);
+ if (ret) {
+ dev_err(dev, "couldn't enable clk\n");
+ return ERR_PTR(ret);
+ }
+
+ devm_add_action_or_reset(dev,
+ (void (*) (void *))clk_disable_unprepare,
+ clk);
+
+ return clk;
+}
+
+static int meson_pcie_probe_clocks(struct meson_pcie *mp)
+{
+ struct device *dev = mp->pci.dev;
+ struct meson_pcie_clk_res *res = &mp->clk_res;
+
+ res->port_clk = meson_pcie_probe_clock(dev, "port", PORT_CLK_RATE);
+ if (IS_ERR(res->port_clk))
+ return PTR_ERR(res->port_clk);
+
+ res->mipi_gate = meson_pcie_probe_clock(dev, "pcie_mipi_en", 0);
+ if (IS_ERR(res->mipi_gate))
+ return PTR_ERR(res->mipi_gate);
+
+ res->general_clk = meson_pcie_probe_clock(dev, "pcie_general", 0);
+ if (IS_ERR(res->general_clk))
+ return PTR_ERR(res->general_clk);
+
+ res->clk = meson_pcie_probe_clock(dev, "pcie", 0);
+ if (IS_ERR(res->clk))
+ return PTR_ERR(res->clk);
+
+ return 0;
+}
+
+static inline void meson_elb_writel(struct meson_pcie *mp, u32 val, u32 reg)
+{
+ writel(val, mp->mem_res.elbi_base + reg);
+}
+
+static inline u32 meson_elb_readl(struct meson_pcie *mp, u32 reg)
+{
+ return readl(mp->mem_res.elbi_base + reg);
+}
+
+static inline u32 meson_cfg_readl(struct meson_pcie *mp, u32 reg)
+{
+ return readl(mp->mem_res.cfg_base + reg);
+}
+
+static inline void meson_cfg_writel(struct meson_pcie *mp, u32 val, u32 reg)
+{
+ writel(val, mp->mem_res.cfg_base + reg);
+}
+
+static void meson_pcie_assert_reset(struct meson_pcie *mp)
+{
+ gpiod_set_value_cansleep(mp->reset_gpio, 0);
+ udelay(500);
+ gpiod_set_value_cansleep(mp->reset_gpio, 1);
+}
+
+static void meson_pcie_init_dw(struct meson_pcie *mp)
+{
+ u32 val = 0;
+
+ val = meson_cfg_readl(mp, PCIE_CFG0);
+ val |= APP_LTSSM_ENABLE;
+ meson_cfg_writel(mp, val, PCIE_CFG0);
+
+ val = meson_elb_readl(mp, PCIE_PORT_LINK_CTRL_OFF);
+ val &= ~LINK_CAPABLE_MASK;
+ meson_elb_writel(mp, val, PCIE_PORT_LINK_CTRL_OFF);
+
+ val = meson_elb_readl(mp, PCIE_PORT_LINK_CTRL_OFF);
+ val |= LINK_CAPABLE_X1 | FAST_LINK_MODE;
+ meson_elb_writel(mp, val, PCIE_PORT_LINK_CTRL_OFF);
+
+ val = meson_elb_readl(mp, PCIE_GEN2_CTRL_OFF);
+ val &= ~NUM_OF_LANES_MASK;
+ meson_elb_writel(mp, val, PCIE_GEN2_CTRL_OFF);
+
+ val = meson_elb_readl(mp, PCIE_GEN2_CTRL_OFF);
+ val |= NUM_OF_LANES_X1 | DIRECT_SPEED_CHANGE;
+ meson_elb_writel(mp, val, PCIE_GEN2_CTRL_OFF);
+
+ meson_elb_writel(mp, 0x0, PCIE_BASE_ADDR0);
+ meson_elb_writel(mp, 0x0, PCIE_BASE_ADDR1);
+}
+
+static int meson_size_to_payload(int size)
+{
+ if (size & (size - 1) || size < 128 || size > 4096)
+ return 1;
+
+ return fls(size) - 8;
+}
+
+static void meson_set_max_payload(struct meson_pcie *mp, int size)
+{
+ u32 val = 0;
+ int max_payload_size = meson_size_to_payload(size);
+
+ val = meson_elb_readl(mp, PCIE_DEV_CTRL_DEV_STUS);
+ val &= ~PCIE_CAP_MAX_PAYLOAD_MASK;
+ meson_elb_writel(mp, val, PCIE_DEV_CTRL_DEV_STUS);
+
+ val = meson_elb_readl(mp, PCIE_DEV_CTRL_DEV_STUS);
+ val |= PCIE_CAP_MAX_PAYLOAD_SIZE(max_payload_size);
+ meson_elb_writel(mp, val, PCIE_DEV_CTRL_DEV_STUS);
+}
+
+static void meson_set_max_rd_req_size(struct meson_pcie *mp, int size)
+{
+ u32 val = 0;
+ int max_rd_req_size = meson_size_to_payload(size);
+
+ val = meson_elb_readl(mp, PCIE_DEV_CTRL_DEV_STUS);
+ val &= ~PCIE_CAP_MAX_READ_REQ_MASK;
+ meson_elb_writel(mp, val, PCIE_DEV_CTRL_DEV_STUS);
+
+ val = meson_elb_readl(mp, PCIE_DEV_CTRL_DEV_STUS);
+ val |= PCIE_CAP_MAX_READ_REQ_SIZE(max_rd_req_size);
+ meson_elb_writel(mp, val, PCIE_DEV_CTRL_DEV_STUS);
+}
+
+static inline void meson_enable_memory_space(struct meson_pcie *mp)
+{
+ /* Set the RC Bus Master, Memory Space and I/O Space enables */
+ meson_elb_writel(mp, PCI_IO_EN | PCI_MEM_SPACE_EN | PCI_BUS_MASTER_EN,
+ PCIE_STATUS_COMMAND);
+}
+
+static int meson_pcie_establish_link(struct meson_pcie *mp)
+{
+ struct dw_pcie *pci = &mp->pci;
+ struct pcie_port *pp = &pci->pp;
+
+ meson_pcie_init_dw(mp);
+ meson_set_max_payload(mp, MAX_PAYLOAD_SIZE);
+ meson_set_max_rd_req_size(mp, MAX_READ_REQ_SIZE);
+
+ dw_pcie_setup_rc(pp);
+ meson_enable_memory_space(mp);
+
+ meson_pcie_assert_reset(mp);
+
+ /* check if the link is up or not */
+ if (!dw_pcie_wait_for_link(pci))
+ return 0;
+
+ return -ETIMEDOUT;
+}
+
+static void meson_pcie_msi_init(struct meson_pcie *mp)
+{
+ struct pcie_port *pp = &mp->pci.pp;
+
+ dw_pcie_msi_init(pp);
+}
+
+static void meson_pcie_enable_interrupts(struct meson_pcie *mp)
+{
+ if (IS_ENABLED(CONFIG_PCI_MSI))
+ meson_pcie_msi_init(mp);
+}
+
+static u32 meson_pcie_read_dbi(struct dw_pcie *pci, void __iomem *base,
+ u32 reg, size_t size)
+{
+ u32 val;
+
+ dw_pcie_read(base + reg, size, &val);
+
+ return val;
+}
+
+static void meson_pcie_write_dbi(struct dw_pcie *pci, void __iomem *base,
+ u32 reg, size_t size, u32 val)
+{
+ dw_pcie_write(base + reg, size, val);
+}
+
+static int meson_pcie_rd_own_conf(struct pcie_port *pp, int where, int size,
+ u32 *val)
+{
+ struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+ struct meson_pcie *mp = to_meson_pcie(pci);
+
+ if (!mp->device_attch)
+ return 0;
+
+ /* the device class is not reported correctly from the register */
+ if (where == PCI_CLASS_REVISION) {
+ *val = readl(pci->dbi_base + PCI_CLASS_REVISION);
+ /* keep revision id */
+ *val &= PCI_CLASS_REVISION_MASK;
+ *val |= PCI_CLASS_BRIDGE_PCI << 16;
+ return PCIBIOS_SUCCESSFUL;
+ }
+
+ return dw_pcie_read(pci->dbi_base + where, size, val);
+}
+
+static int meson_pcie_wr_own_conf(struct pcie_port *pp, int where,
+ int size, u32 val)
+{
+ struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+ struct meson_pcie *mp = to_meson_pcie(pci);
+
+ if (!mp->device_attch)
+ return 0;
+
+ return dw_pcie_write(pci->dbi_base + where, size, val);
+}
+
+static int meson_pcie_link_up(struct dw_pcie *pci)
+{
+ struct meson_pcie *mp = to_meson_pcie(pci);
+ struct device *dev = pci->dev;
+ u32 smlh_up = 0;
+ u32 ltssm_up = 0;
+ u32 rdlh_up = 0;
+ u32 speed_okay = 0;
+ u32 cnt = 0;
+ u32 state12, state17;
+
+ while (smlh_up == 0 || rdlh_up == 0 || ltssm_up == 0 ||
+ speed_okay == 0) {
+ udelay(20);
+
+ state12 = meson_cfg_readl(mp, PCIE_CFG_STATUS12);
+ state17 = meson_cfg_readl(mp, PCIE_CFG_STATUS17);
+ smlh_up = IS_SMLH_LINK_UP(state12);
+ rdlh_up = IS_RDLH_LINK_UP(state12);
+ ltssm_up = IS_LTSSM_UP(state12);
+
+ if (PM_CURRENT_STATE(state17) < PCIE_GEN3)
+ speed_okay = 1;
+
+ if (smlh_up)
+ dev_dbg(dev, "smlh_link_up is on\n");
+ if (rdlh_up)
+ dev_dbg(dev, "rdlh_link_up is on\n");
+ if (ltssm_up)
+ dev_dbg(dev, "ltssm_up is on\n");
+ if (speed_okay)
+ dev_dbg(dev, "speed_okay\n");
+
+ cnt++;
+
+ if (cnt >= WAIT_LINKUP_TIMEOUT) {
+ dev_err(dev, "Error: Wait linkup timeout.\n");
+ return 0;
+ }
+ }
+
+ return 1;
+}
+
+static int meson_pcie_host_init(struct pcie_port *pp)
+{
+ struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+ struct meson_pcie *mp = to_meson_pcie(pci);
+ int ret;
+
+ ret = meson_pcie_establish_link(mp);
+
+ if (ret)
+ return ret;
+
+ mp->device_attch = 1;
+ meson_pcie_enable_interrupts(mp);
+
+ return 0;
+}
+
+static const struct dw_pcie_host_ops meson_pcie_host_ops = {
+ .rd_own_conf = meson_pcie_rd_own_conf,
+ .wr_own_conf = meson_pcie_wr_own_conf,
+ .host_init = meson_pcie_host_init,
+};
+
+static int meson_add_pcie_port(struct meson_pcie *mp,
+ struct platform_device *pdev)
+{
+ struct dw_pcie *pci = &mp->pci;
+ struct pcie_port *pp = &pci->pp;
+ struct device *dev = &pdev->dev;
+ int ret;
+
+ if (IS_ENABLED(CONFIG_PCI_MSI)) {
+ pp->msi_irq = platform_get_irq(pdev, 0);
+ if (pp->msi_irq < 0) {
+ dev_err(dev, "failed to get msi irq\n");
+ return pp->msi_irq;
+ }
+ }
+
+ pp->root_bus_nr = -1;
+ pp->ops = &meson_pcie_host_ops;
+ pci->dbi_base = mp->mem_res.elbi_base;
+
+ ret = dw_pcie_host_init(pp);
+ if (ret) {
+ dev_err(dev, "failed to initialize host\n");
+ return ret;
+ }
+
+ return 0;
+}
+
+static const struct dw_pcie_ops dw_pcie_ops = {
+ .read_dbi = meson_pcie_read_dbi,
+ .write_dbi = meson_pcie_write_dbi,
+ .link_up = meson_pcie_link_up,
+};
+
+static int meson_pcie_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct dw_pcie *pci;
+ struct meson_pcie *mp;
+ int ret;
+
+ mp = devm_kzalloc(dev, sizeof(*mp), GFP_KERNEL);
+ if (!mp)
+ return -ENOMEM;
+
+ pci = &mp->pci;
+ pci->dev = dev;
+ pci->ops = &dw_pcie_ops;
+
+ mp->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
+ if (IS_ERR(mp->reset_gpio)) {
+ dev_err(dev, "Get reset gpio failed\n");
+ return PTR_ERR(mp->reset_gpio);
+ }
+
+ ret = meson_pcie_get_resets(mp);
+ if (ret) {
+ dev_err(dev, "Get reset resource failed, %d\n", ret);
+ return ret;
+ }
+
+ ret = meson_pcie_get_mems(pdev, mp);
+ if (ret) {
+ dev_err(dev, "Get memory resource failed, %d\n", ret);
+ return ret;
+ }
+
+ meson_pcie_power_on(mp);
+ meson_pcie_reset(mp);
+
+ ret = meson_pcie_probe_clocks(mp);
+ if (ret) {
+ dev_err(dev, "Init clock resources failed, %d\n", ret);
+ return ret;
+ }
+
+ platform_set_drvdata(pdev, mp);
+
+ ret = meson_add_pcie_port(mp, pdev);
+ if (ret < 0)
+ dev_err(dev, "Add PCIE port failed, %d\n", ret);
+
+ return ret;
+}
+
+static const struct of_device_id meson_pcie_of_match[] = {
+ {
+ .compatible = "amlogic,axg-pcie",
+ },
+ {},
+};
+
+static struct platform_driver meson_pcie_driver = {
+ .probe = meson_pcie_probe,
+ .driver = {
+ .name = "meson-pcie",
+ .of_match_table = meson_pcie_of_match,
+ },
+};
+
+builtin_platform_driver(meson_pcie_driver);
--
2.7.4


2018-09-21 14:48:45

by Lorenzo Pieralisi

[permalink] [raw]
Subject: Re: [RESEND PATCH v3 2/2] PCI: meson: add the Amlogic Meson PCIe controller driver

On Fri, Sep 21, 2018 at 02:03:40PM +0800, Hanjie Lin wrote:
> From: Yue Wang <[email protected]>
>
> The Amlogic Meson PCIe host controller is based on the Synopsys DesignWare
> PCI core. This patch adds the driver support for Meson PCIe controller.

Please read:

https://marc.info/?l=linux-pci&m=150905742808166&w=2

update the $SUBJECT accordingly, I can do it but you will repost anyway
so please do it yourself while at it.

> Signed-off-by: Yue Wang <[email protected]>
> Signed-off-by: Hanjie Lin <[email protected]>
> ---
> drivers/pci/controller/dwc/Kconfig | 12 +
> drivers/pci/controller/dwc/Makefile | 1 +
> drivers/pci/controller/dwc/pci-meson.c | 617 +++++++++++++++++++++++++++++++++
> 3 files changed, 630 insertions(+)
> create mode 100644 drivers/pci/controller/dwc/pci-meson.c

I would request a MAINTAINERS update too.

> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> index 91b0194..6cb36f6 100644
> --- a/drivers/pci/controller/dwc/Kconfig
> +++ b/drivers/pci/controller/dwc/Kconfig
> @@ -193,4 +193,16 @@ config PCIE_HISI_STB
> help
> Say Y here if you want PCIe controller support on HiSilicon STB SoCs
>
> +config PCI_MESON
> + bool "MESON PCIe controller"
> + depends on PCI

This is redundant, remove it.

> + depends on PCI_MSI_IRQ_DOMAIN
> + select PCIEPORTBUS

There is no real dependency on it, this should be left to kernel
configuration, remove it.

> + select PCIE_DW_HOST
> + help
> + Say Y here if you want to enable PCI controller support on Amlogic
> + SoCs. The PCI controller on Amlogic is based on DesignWare hardware
> + and therefore the driver re-uses the DesignWare core functions to
> + implement the driver.
> +
> endmenu
> diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
> index 5d2ce72..cf676bd 100644
> --- a/drivers/pci/controller/dwc/Makefile
> +++ b/drivers/pci/controller/dwc/Makefile
> @@ -14,6 +14,7 @@ obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o
> obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o
> obj-$(CONFIG_PCIE_KIRIN) += pcie-kirin.o
> obj-$(CONFIG_PCIE_HISI_STB) += pcie-histb.o
> +obj-$(CONFIG_PCI_MESON) += pci-meson.o
>
> # The following drivers are for devices that use the generic ACPI
> # pci_root.c driver but don't support standard ECAM config access.
> diff --git a/drivers/pci/controller/dwc/pci-meson.c b/drivers/pci/controller/dwc/pci-meson.c
> new file mode 100644
> index 0000000..9c92a89
> --- /dev/null
> +++ b/drivers/pci/controller/dwc/pci-meson.c
> @@ -0,0 +1,617 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * PCIe host controller driver for Amlogic MESON SoCs
> + *
> + * Copyright (c) 2018 Amlogic, inc.
> + * Author: Yue Wang <[email protected]>
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/of_device.h>
> +#include <linux/of_gpio.h>
> +#include <linux/pci.h>
> +#include <linux/platform_device.h>
> +#include <linux/resource.h>
> +#include <linux/types.h>
> +#include <linux/reset.h>
> +
> +#include "pcie-designware.h"
> +
> +#define to_meson_pcie(x) dev_get_drvdata((x)->dev)

Side note: this macro can be made dwc global since it is the same for all
dwc based host bridges.

> +/* External local bus interface registers */
> +#define PLR_OFFSET 0x700
> +#define PCIE_PORT_LINK_CTRL_OFF (PLR_OFFSET + 0x10)
> +#define FAST_LINK_MODE BIT(7)
> +#define LINK_CAPABLE_MASK GENMASK(21, 16)
> +#define LINK_CAPABLE_X1 BIT(16)
> +
> +#define PCIE_GEN2_CTRL_OFF (PLR_OFFSET + 0x10c)
> +#define NUM_OF_LANES_MASK GENMASK(12, 8)
> +#define NUM_OF_LANES_X1 BIT(8)
> +#define DIRECT_SPEED_CHANGE BIT(17)
> +
> +#define TYPE1_HDR_OFFSET 0x0
> +#define PCIE_STATUS_COMMAND (TYPE1_HDR_OFFSET + 0x04)
> +#define PCI_IO_EN BIT(0)
> +#define PCI_MEM_SPACE_EN BIT(1)
> +#define PCI_BUS_MASTER_EN BIT(2)
> +
> +#define PCIE_BASE_ADDR0 (TYPE1_HDR_OFFSET + 0x10)
> +#define PCIE_BASE_ADDR1 (TYPE1_HDR_OFFSET + 0x14)
> +
> +#define PCIE_CAP_OFFSET 0x70
> +#define PCIE_DEV_CTRL_DEV_STUS (PCIE_CAP_OFFSET + 0x08)
> +#define PCIE_CAP_MAX_PAYLOAD_MASK GENMASK(7, 5)
> +#define PCIE_CAP_MAX_PAYLOAD_SIZE(x) ((x) << 5)
> +#define PCIE_CAP_MAX_READ_REQ_MASK GENMASK(14, 12)
> +#define PCIE_CAP_MAX_READ_REQ_SIZE(x) ((x) << 12)
> +
> +#define PCI_CLASS_REVISION_MASK GENMASK(7, 0)
> +
> +/* PCIe specific config registers */
> +#define PCIE_CFG0 0x0
> +#define APP_LTSSM_ENABLE BIT(7)
> +
> +#define PCIE_CFG_STATUS12 0x30
> +#define IS_SMLH_LINK_UP(x) ((x) & (1 << 6))
> +#define IS_RDLH_LINK_UP(x) ((x) & (1 << 16))
> +#define IS_LTSSM_UP(x) ((((x) >> 10) & 0x1f) == 0x11)
> +
> +#define PCIE_CFG_STATUS17 0x44
> +#define PM_CURRENT_STATE(x) (((x) >> 7) & 0x1)
> +
> +#define WAIT_LINKUP_TIMEOUT 2000
> +#define PORT_CLK_RATE 100000000UL
> +#define MAX_PAYLOAD_SIZE 256
> +#define MAX_READ_REQ_SIZE 256
> +#define MESON_PCIE_PHY_POWERUP 0x1c
> +#define PCIE_RESET_DELAY 500
> +#define PCIE_SHARED_RESET 1
> +#define PCIE_NORMAL_RESET 0
> +
> +enum pcie_data_rate {
> + PCIE_GEN1,
> + PCIE_GEN2,
> + PCIE_GEN3,
> + PCIE_GEN4
> +};
> +
> +struct meson_pcie_mem_res {
> + void __iomem *elbi_base; /* DT 0th resource */
> + void __iomem *cfg_base; /* DT 1nd resource */
> + void __iomem *phy_base; /* DT 2nd resource */
> +};
> +
> +struct meson_pcie_clk_res {
> + struct clk *clk;
> + struct clk *mipi_gate;
> + struct clk *port_clk;
> + struct clk *general_clk;
> +};
> +
> +struct meson_pcie_rc_reset {
> + struct reset_control *phy;
> + struct reset_control *port;
> + struct reset_control *apb;
> +};
> +
> +struct meson_pcie {
> + struct dw_pcie pci;
> + struct meson_pcie_mem_res mem_res;
> + struct meson_pcie_clk_res clk_res;
> + struct meson_pcie_rc_reset mrst;
> + struct gpio_desc *reset_gpio;
> +
> + enum of_gpio_flags gpio_flag;
> + int pcie_num;
> + u32 port_num;
> + u32 device_attch;

What's this variable for and is it really needed ?

> +};
> +
> +static struct reset_control *meson_pcie_get_reset(struct meson_pcie *mp,
> + const char *id,
> + u32 reset_type)
> +{
> + struct device *dev = mp->pci.dev;
> + struct reset_control *reset;
> +
> + if (reset_type == PCIE_SHARED_RESET)
> + reset = devm_reset_control_get_shared(dev, id);
> + else
> + reset = devm_reset_control_get(dev, id);
> +
> + return reset;
> +}
> +
> +static int meson_pcie_get_resets(struct meson_pcie *mp)
> +{
> + struct meson_pcie_rc_reset *mrst = &mp->mrst;
> +
> + mrst->phy = meson_pcie_get_reset(mp, "phy", PCIE_SHARED_RESET);
> + if (IS_ERR(mrst->phy))
> + return PTR_ERR(mrst->phy);
> + reset_control_deassert(mrst->phy);
> +
> + mrst->port = meson_pcie_get_reset(mp, "port", PCIE_NORMAL_RESET);
> + if (IS_ERR(mrst->port))
> + return PTR_ERR(mrst->port);
> + reset_control_deassert(mrst->port);
> +
> + mrst->apb = meson_pcie_get_reset(mp, "apb", PCIE_SHARED_RESET);
> + if (IS_ERR(mrst->apb))
> + return PTR_ERR(mrst->apb);
> + reset_control_deassert(mrst->apb);
> +
> + return 0;
> +}
> +
> +static void __iomem *meson_pcie_get_mem_shared(struct platform_device *pdev,
> + struct meson_pcie *mp,
> + const char *id)
> +{
> + struct resource *res;
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, id);
> +
> + return ioremap(res->start, resource_size(res));
> +}
> +
> +static void __iomem *meson_pcie_get_mem(struct platform_device *pdev,
> + struct meson_pcie *mp,
> + const char *id)
> +{
> + struct device *dev = mp->pci.dev;
> + struct resource *res;
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, id);
> +
> + return devm_ioremap_resource(dev, res);
> +}
> +
> +static int meson_pcie_get_mems(struct platform_device *pdev,
> + struct meson_pcie *mp)
> +{
> + mp->mem_res.elbi_base = meson_pcie_get_mem(pdev, mp, "elbi");
> + if (IS_ERR(mp->mem_res.elbi_base))
> + return PTR_ERR(mp->mem_res.elbi_base);
> +
> + mp->mem_res.cfg_base = meson_pcie_get_mem(pdev, mp, "cfg");
> + if (IS_ERR(mp->mem_res.cfg_base))
> + return PTR_ERR(mp->mem_res.cfg_base);
> +
> + mp->mem_res.phy_base = meson_pcie_get_mem_shared(pdev, mp, "phy");
> + if (IS_ERR(mp->mem_res.phy_base))
> + return PTR_ERR(mp->mem_res.phy_base);

I do no understand the logic behind this _shared() extension and how
it would work in the failure path.

> + return 0;
> +}
> +
> +static void meson_pcie_power_on(struct meson_pcie *mp)
> +{
> + writel(MESON_PCIE_PHY_POWERUP, mp->mem_res.phy_base);
> +}
> +
> +static void meson_pcie_reset(struct meson_pcie *mp)
> +{
> + struct meson_pcie_rc_reset *mrst = &mp->mrst;
> +
> + reset_control_assert(mrst->phy);
> + udelay(PCIE_RESET_DELAY);
> + reset_control_deassert(mrst->phy);
> + udelay(PCIE_RESET_DELAY);
> +
> + reset_control_assert(mrst->port);
> + reset_control_assert(mrst->apb);
> + udelay(PCIE_RESET_DELAY);
> + reset_control_deassert(mrst->port);
> + reset_control_deassert(mrst->apb);
> + udelay(PCIE_RESET_DELAY);
> +}
> +
> +static inline struct clk *meson_pcie_probe_clock(struct device *dev,
> + const char *id, u64 rate)
> +{
> + struct clk *clk = NULL;

Nit: useless initialization.

> + int ret;
> +
> + clk = devm_clk_get(dev, id);
> + if (IS_ERR(clk))
> + return clk;
> +
> + if (rate) {
> + ret = clk_set_rate(clk, rate);
> + if (ret) {
> + dev_err(dev, "set clk rate failed, ret = %d\n", ret);
> + return ERR_PTR(ret);
> + }
> + }
> +
> + ret = clk_prepare_enable(clk);
> + if (ret) {
> + dev_err(dev, "couldn't enable clk\n");
> + return ERR_PTR(ret);
> + }
> +
> + devm_add_action_or_reset(dev,
> + (void (*) (void *))clk_disable_unprepare,
> + clk);
> +
> + return clk;
> +}
> +
> +static int meson_pcie_probe_clocks(struct meson_pcie *mp)
> +{
> + struct device *dev = mp->pci.dev;
> + struct meson_pcie_clk_res *res = &mp->clk_res;
> +
> + res->port_clk = meson_pcie_probe_clock(dev, "port", PORT_CLK_RATE);
> + if (IS_ERR(res->port_clk))
> + return PTR_ERR(res->port_clk);
> +
> + res->mipi_gate = meson_pcie_probe_clock(dev, "pcie_mipi_en", 0);
> + if (IS_ERR(res->mipi_gate))
> + return PTR_ERR(res->mipi_gate);
> +
> + res->general_clk = meson_pcie_probe_clock(dev, "pcie_general", 0);
> + if (IS_ERR(res->general_clk))
> + return PTR_ERR(res->general_clk);
> +
> + res->clk = meson_pcie_probe_clock(dev, "pcie", 0);
> + if (IS_ERR(res->clk))
> + return PTR_ERR(res->clk);
> +
> + return 0;
> +}
> +
> +static inline void meson_elb_writel(struct meson_pcie *mp, u32 val, u32 reg)
> +{
> + writel(val, mp->mem_res.elbi_base + reg);
> +}
> +
> +static inline u32 meson_elb_readl(struct meson_pcie *mp, u32 reg)
> +{
> + return readl(mp->mem_res.elbi_base + reg);
> +}
> +
> +static inline u32 meson_cfg_readl(struct meson_pcie *mp, u32 reg)
> +{
> + return readl(mp->mem_res.cfg_base + reg);
> +}
> +
> +static inline void meson_cfg_writel(struct meson_pcie *mp, u32 val, u32 reg)
> +{
> + writel(val, mp->mem_res.cfg_base + reg);
> +}
> +
> +static void meson_pcie_assert_reset(struct meson_pcie *mp)
> +{
> + gpiod_set_value_cansleep(mp->reset_gpio, 0);
> + udelay(500);
> + gpiod_set_value_cansleep(mp->reset_gpio, 1);
> +}
> +
> +static void meson_pcie_init_dw(struct meson_pcie *mp)
> +{
> + u32 val = 0;

Useless initialization.

> + val = meson_cfg_readl(mp, PCIE_CFG0);
> + val |= APP_LTSSM_ENABLE;
> + meson_cfg_writel(mp, val, PCIE_CFG0);
> +
> + val = meson_elb_readl(mp, PCIE_PORT_LINK_CTRL_OFF);
> + val &= ~LINK_CAPABLE_MASK;
> + meson_elb_writel(mp, val, PCIE_PORT_LINK_CTRL_OFF);
> +
> + val = meson_elb_readl(mp, PCIE_PORT_LINK_CTRL_OFF);
> + val |= LINK_CAPABLE_X1 | FAST_LINK_MODE;
> + meson_elb_writel(mp, val, PCIE_PORT_LINK_CTRL_OFF);
> +
> + val = meson_elb_readl(mp, PCIE_GEN2_CTRL_OFF);
> + val &= ~NUM_OF_LANES_MASK;
> + meson_elb_writel(mp, val, PCIE_GEN2_CTRL_OFF);
> +
> + val = meson_elb_readl(mp, PCIE_GEN2_CTRL_OFF);
> + val |= NUM_OF_LANES_X1 | DIRECT_SPEED_CHANGE;
> + meson_elb_writel(mp, val, PCIE_GEN2_CTRL_OFF);
> +
> + meson_elb_writel(mp, 0x0, PCIE_BASE_ADDR0);
> + meson_elb_writel(mp, 0x0, PCIE_BASE_ADDR1);
> +}
> +
> +static int meson_size_to_payload(int size)
> +{
> + if (size & (size - 1) || size < 128 || size > 4096)
> + return 1;

Please expain this statement. It deserves a comment at least.

> +
> + return fls(size) - 8;
> +}
> +
> +static void meson_set_max_payload(struct meson_pcie *mp, int size)
> +{
> + u32 val = 0;
> + int max_payload_size = meson_size_to_payload(size);
> +
> + val = meson_elb_readl(mp, PCIE_DEV_CTRL_DEV_STUS);
> + val &= ~PCIE_CAP_MAX_PAYLOAD_MASK;
> + meson_elb_writel(mp, val, PCIE_DEV_CTRL_DEV_STUS);
> +
> + val = meson_elb_readl(mp, PCIE_DEV_CTRL_DEV_STUS);
> + val |= PCIE_CAP_MAX_PAYLOAD_SIZE(max_payload_size);
> + meson_elb_writel(mp, val, PCIE_DEV_CTRL_DEV_STUS);
> +}
> +
> +static void meson_set_max_rd_req_size(struct meson_pcie *mp, int size)
> +{
> + u32 val = 0;

Useless initialization.

> + int max_rd_req_size = meson_size_to_payload(size);
> +
> + val = meson_elb_readl(mp, PCIE_DEV_CTRL_DEV_STUS);
> + val &= ~PCIE_CAP_MAX_READ_REQ_MASK;
> + meson_elb_writel(mp, val, PCIE_DEV_CTRL_DEV_STUS);
> +
> + val = meson_elb_readl(mp, PCIE_DEV_CTRL_DEV_STUS);
> + val |= PCIE_CAP_MAX_READ_REQ_SIZE(max_rd_req_size);
> + meson_elb_writel(mp, val, PCIE_DEV_CTRL_DEV_STUS);
> +}
> +
> +static inline void meson_enable_memory_space(struct meson_pcie *mp)
> +{
> + /* Set the RC Bus Master, Memory Space and I/O Space enables */
> + meson_elb_writel(mp, PCI_IO_EN | PCI_MEM_SPACE_EN | PCI_BUS_MASTER_EN,
> + PCIE_STATUS_COMMAND);
> +}
> +
> +static int meson_pcie_establish_link(struct meson_pcie *mp)
> +{
> + struct dw_pcie *pci = &mp->pci;
> + struct pcie_port *pp = &pci->pp;
> +
> + meson_pcie_init_dw(mp);
> + meson_set_max_payload(mp, MAX_PAYLOAD_SIZE);
> + meson_set_max_rd_req_size(mp, MAX_READ_REQ_SIZE);
> +
> + dw_pcie_setup_rc(pp);
> + meson_enable_memory_space(mp);
> +
> + meson_pcie_assert_reset(mp);
> +
> + /* check if the link is up or not */
> + if (!dw_pcie_wait_for_link(pci))
> + return 0;
> +
> + return -ETIMEDOUT;

return dw_pcie_wait_for_link(pci));

?

> +}
> +
> +static void meson_pcie_msi_init(struct meson_pcie *mp)
> +{
> + struct pcie_port *pp = &mp->pci.pp;
> +
> + dw_pcie_msi_init(pp);
> +}
> +
> +static void meson_pcie_enable_interrupts(struct meson_pcie *mp)
> +{
> + if (IS_ENABLED(CONFIG_PCI_MSI))
> + meson_pcie_msi_init(mp);
> +}

You do not need two functions to do what can be done in one.

All you need is struct pcie_port instead of calling this function
with struct meson_pci.

> +
> +static u32 meson_pcie_read_dbi(struct dw_pcie *pci, void __iomem *base,
> + u32 reg, size_t size)
> +{
> + u32 val;
> +
> + dw_pcie_read(base + reg, size, &val);
> +
> + return val;
> +}
> +
> +static void meson_pcie_write_dbi(struct dw_pcie *pci, void __iomem *base,
> + u32 reg, size_t size, u32 val)
> +{
> + dw_pcie_write(base + reg, size, val);
> +}

Nit: there is nothing dbi specific in these two functions.

> +
> +static int meson_pcie_rd_own_conf(struct pcie_port *pp, int where, int size,
> + u32 *val)
> +{
> + struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> + struct meson_pcie *mp = to_meson_pcie(pci);
> +
> + if (!mp->device_attch)
> + return 0;

Why do you need this guard ?

> +
> + /* the device class is not reported correctly from the register */
> + if (where == PCI_CLASS_REVISION) {
> + *val = readl(pci->dbi_base + PCI_CLASS_REVISION);
> + /* keep revision id */
> + *val &= PCI_CLASS_REVISION_MASK;
> + *val |= PCI_CLASS_BRIDGE_PCI << 16;
> + return PCIBIOS_SUCCESSFUL;
> + }
> +
> + return dw_pcie_read(pci->dbi_base + where, size, val);
> +}
> +
> +static int meson_pcie_wr_own_conf(struct pcie_port *pp, int where,
> + int size, u32 val)
> +{
> + struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> + struct meson_pcie *mp = to_meson_pcie(pci);
> +
> + if (!mp->device_attch)
> + return 0;

Same question.

> +
> + return dw_pcie_write(pci->dbi_base + where, size, val);
> +}
> +
> +static int meson_pcie_link_up(struct dw_pcie *pci)
> +{
> + struct meson_pcie *mp = to_meson_pcie(pci);
> + struct device *dev = pci->dev;
> + u32 smlh_up = 0;
> + u32 ltssm_up = 0;
> + u32 rdlh_up = 0;
> + u32 speed_okay = 0;
> + u32 cnt = 0;
> + u32 state12, state17;
> +
> + while (smlh_up == 0 || rdlh_up == 0 || ltssm_up == 0 ||
> + speed_okay == 0) {
> + udelay(20);
> +
> + state12 = meson_cfg_readl(mp, PCIE_CFG_STATUS12);
> + state17 = meson_cfg_readl(mp, PCIE_CFG_STATUS17);
> + smlh_up = IS_SMLH_LINK_UP(state12);
> + rdlh_up = IS_RDLH_LINK_UP(state12);
> + ltssm_up = IS_LTSSM_UP(state12);
> +
> + if (PM_CURRENT_STATE(state17) < PCIE_GEN3)
> + speed_okay = 1;
> +
> + if (smlh_up)
> + dev_dbg(dev, "smlh_link_up is on\n");
> + if (rdlh_up)
> + dev_dbg(dev, "rdlh_link_up is on\n");
> + if (ltssm_up)
> + dev_dbg(dev, "ltssm_up is on\n");
> + if (speed_okay)
> + dev_dbg(dev, "speed_okay\n");
> +
> + cnt++;
> +
> + if (cnt >= WAIT_LINKUP_TIMEOUT) {
> + dev_err(dev, "Error: Wait linkup timeout.\n");
> + return 0;
> + }
> + }
> +
> + return 1;
> +}
> +
> +static int meson_pcie_host_init(struct pcie_port *pp)
> +{
> + struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> + struct meson_pcie *mp = to_meson_pcie(pci);
> + int ret;
> +
> + ret = meson_pcie_establish_link(mp);
> +

Remove this empty line.

> + if (ret)
> + return ret;
> +
> + mp->device_attch = 1;

I want to understand why you need this variable at all (and why it is
an u32).

> + meson_pcie_enable_interrupts(mp);
> +
> + return 0;
> +}
> +
> +static const struct dw_pcie_host_ops meson_pcie_host_ops = {
> + .rd_own_conf = meson_pcie_rd_own_conf,
> + .wr_own_conf = meson_pcie_wr_own_conf,
> + .host_init = meson_pcie_host_init,
> +};
> +
> +static int meson_add_pcie_port(struct meson_pcie *mp,
> + struct platform_device *pdev)
> +{
> + struct dw_pcie *pci = &mp->pci;
> + struct pcie_port *pp = &pci->pp;
> + struct device *dev = &pdev->dev;
> + int ret;
> +
> + if (IS_ENABLED(CONFIG_PCI_MSI)) {
> + pp->msi_irq = platform_get_irq(pdev, 0);
> + if (pp->msi_irq < 0) {
> + dev_err(dev, "failed to get msi irq\n");
> + return pp->msi_irq;
> + }
> + }
> +
> + pp->root_bus_nr = -1;

This is useless, since it is initialized in dw_pcie_host_init(), remove
it.

> + pp->ops = &meson_pcie_host_ops;
> + pci->dbi_base = mp->mem_res.elbi_base;
> +
> + ret = dw_pcie_host_init(pp);
> + if (ret) {
> + dev_err(dev, "failed to initialize host\n");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static const struct dw_pcie_ops dw_pcie_ops = {
> + .read_dbi = meson_pcie_read_dbi,
> + .write_dbi = meson_pcie_write_dbi,
> + .link_up = meson_pcie_link_up,
> +};
> +
> +static int meson_pcie_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct dw_pcie *pci;
> + struct meson_pcie *mp;
> + int ret;
> +
> + mp = devm_kzalloc(dev, sizeof(*mp), GFP_KERNEL);
> + if (!mp)
> + return -ENOMEM;
> +
> + pci = &mp->pci;
> + pci->dev = dev;
> + pci->ops = &dw_pcie_ops;
> +
> + mp->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
> + if (IS_ERR(mp->reset_gpio)) {
> + dev_err(dev, "Get reset gpio failed\n");
> + return PTR_ERR(mp->reset_gpio);
> + }
> +
> + ret = meson_pcie_get_resets(mp);
> + if (ret) {
> + dev_err(dev, "Get reset resource failed, %d\n", ret);
> + return ret;
> + }
> +
> + ret = meson_pcie_get_mems(pdev, mp);
> + if (ret) {
> + dev_err(dev, "Get memory resource failed, %d\n", ret);
> + return ret;
> + }
> +
> + meson_pcie_power_on(mp);
> + meson_pcie_reset(mp);
> +
> + ret = meson_pcie_probe_clocks(mp);
> + if (ret) {
> + dev_err(dev, "Init clock resources failed, %d\n", ret);
> + return ret;
> + }
> +
> + platform_set_drvdata(pdev, mp);
> +
> + ret = meson_add_pcie_port(mp, pdev);
> + if (ret < 0)
> + dev_err(dev, "Add PCIE port failed, %d\n", ret);

Isn't this fatal ?

Thanks,
Lorenzo

> + return ret;
> +}
> +
> +static const struct of_device_id meson_pcie_of_match[] = {
> + {
> + .compatible = "amlogic,axg-pcie",
> + },
> + {},
> +};
> +
> +static struct platform_driver meson_pcie_driver = {
> + .probe = meson_pcie_probe,
> + .driver = {
> + .name = "meson-pcie",
> + .of_match_table = meson_pcie_of_match,
> + },
> +};
> +
> +builtin_platform_driver(meson_pcie_driver);
> --
> 2.7.4
>

2018-09-24 08:38:56

by Gustavo Pimentel

[permalink] [raw]
Subject: Re: [RESEND PATCH v3 2/2] PCI: meson: add the Amlogic Meson PCIe controller driver

Hi Hanjie,

On 21/09/2018 07:03, Hanjie Lin wrote:
> From: Yue Wang <[email protected]>
>
> The Amlogic Meson PCIe host controller is based on the Synopsys DesignWare
> PCI core. This patch adds the driver support for Meson PCIe controller.
>
> Signed-off-by: Yue Wang <[email protected]>
> Signed-off-by: Hanjie Lin <[email protected]>
> ---
> drivers/pci/controller/dwc/Kconfig | 12 +
> drivers/pci/controller/dwc/Makefile | 1 +
> drivers/pci/controller/dwc/pci-meson.c | 617 +++++++++++++++++++++++++++++++++
> 3 files changed, 630 insertions(+)
> create mode 100644 drivers/pci/controller/dwc/pci-meson.c
>
> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> index 91b0194..6cb36f6 100644
> --- a/drivers/pci/controller/dwc/Kconfig
> +++ b/drivers/pci/controller/dwc/Kconfig
> @@ -193,4 +193,16 @@ config PCIE_HISI_STB
> help
> Say Y here if you want PCIe controller support on HiSilicon STB SoCs
>
> +config PCI_MESON
> + bool "MESON PCIe controller"
> + depends on PCI
> + depends on PCI_MSI_IRQ_DOMAIN

I would suggest to compress the previous 2 line into in just one, like:
depends on PCI && PCI_MSI_IRQ_DOMAIN

Regards,
Gustavo

> + select PCIEPORTBUS
> + select PCIE_DW_HOST
> + help
> + Say Y here if you want to enable PCI controller support on Amlogic
> + SoCs. The PCI controller on Amlogic is based on DesignWare hardware
> + and therefore the driver re-uses the DesignWare core functions to
> + implement the driver.
> +
> endmenu
> diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
> index 5d2ce72..cf676bd 100644
> --- a/drivers/pci/controller/dwc/Makefile
> +++ b/drivers/pci/controller/dwc/Makefile
> @@ -14,6 +14,7 @@ obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o
> obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o
> obj-$(CONFIG_PCIE_KIRIN) += pcie-kirin.o
> obj-$(CONFIG_PCIE_HISI_STB) += pcie-histb.o
> +obj-$(CONFIG_PCI_MESON) += pci-meson.o
>
> # The following drivers are for devices that use the generic ACPI
> # pci_root.c driver but don't support standard ECAM config access.
> diff --git a/drivers/pci/controller/dwc/pci-meson.c b/drivers/pci/controller/dwc/pci-meson.c
> new file mode 100644
> index 0000000..9c92a89
> --- /dev/null
> +++ b/drivers/pci/controller/dwc/pci-meson.c
> @@ -0,0 +1,617 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * PCIe host controller driver for Amlogic MESON SoCs
> + *
> + * Copyright (c) 2018 Amlogic, inc.
> + * Author: Yue Wang <[email protected]>
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/of_device.h>
> +#include <linux/of_gpio.h>
> +#include <linux/pci.h>
> +#include <linux/platform_device.h>
> +#include <linux/resource.h>
> +#include <linux/types.h>
> +#include <linux/reset.h>
> +
> +#include "pcie-designware.h"
> +
> +#define to_meson_pcie(x) dev_get_drvdata((x)->dev)
> +
> +/* External local bus interface registers */
> +#define PLR_OFFSET 0x700
> +#define PCIE_PORT_LINK_CTRL_OFF (PLR_OFFSET + 0x10)
> +#define FAST_LINK_MODE BIT(7)
> +#define LINK_CAPABLE_MASK GENMASK(21, 16)
> +#define LINK_CAPABLE_X1 BIT(16)
> +
> +#define PCIE_GEN2_CTRL_OFF (PLR_OFFSET + 0x10c)
> +#define NUM_OF_LANES_MASK GENMASK(12, 8)
> +#define NUM_OF_LANES_X1 BIT(8)
> +#define DIRECT_SPEED_CHANGE BIT(17)
> +
> +#define TYPE1_HDR_OFFSET 0x0
> +#define PCIE_STATUS_COMMAND (TYPE1_HDR_OFFSET + 0x04)
> +#define PCI_IO_EN BIT(0)
> +#define PCI_MEM_SPACE_EN BIT(1)
> +#define PCI_BUS_MASTER_EN BIT(2)
> +
> +#define PCIE_BASE_ADDR0 (TYPE1_HDR_OFFSET + 0x10)
> +#define PCIE_BASE_ADDR1 (TYPE1_HDR_OFFSET + 0x14)
> +
> +#define PCIE_CAP_OFFSET 0x70
> +#define PCIE_DEV_CTRL_DEV_STUS (PCIE_CAP_OFFSET + 0x08)
> +#define PCIE_CAP_MAX_PAYLOAD_MASK GENMASK(7, 5)
> +#define PCIE_CAP_MAX_PAYLOAD_SIZE(x) ((x) << 5)
> +#define PCIE_CAP_MAX_READ_REQ_MASK GENMASK(14, 12)
> +#define PCIE_CAP_MAX_READ_REQ_SIZE(x) ((x) << 12)
> +
> +#define PCI_CLASS_REVISION_MASK GENMASK(7, 0)
> +
> +/* PCIe specific config registers */
> +#define PCIE_CFG0 0x0
> +#define APP_LTSSM_ENABLE BIT(7)
> +
> +#define PCIE_CFG_STATUS12 0x30
> +#define IS_SMLH_LINK_UP(x) ((x) & (1 << 6))
> +#define IS_RDLH_LINK_UP(x) ((x) & (1 << 16))
> +#define IS_LTSSM_UP(x) ((((x) >> 10) & 0x1f) == 0x11)
> +
> +#define PCIE_CFG_STATUS17 0x44
> +#define PM_CURRENT_STATE(x) (((x) >> 7) & 0x1)
> +
> +#define WAIT_LINKUP_TIMEOUT 2000
> +#define PORT_CLK_RATE 100000000UL
> +#define MAX_PAYLOAD_SIZE 256
> +#define MAX_READ_REQ_SIZE 256
> +#define MESON_PCIE_PHY_POWERUP 0x1c
> +#define PCIE_RESET_DELAY 500
> +#define PCIE_SHARED_RESET 1
> +#define PCIE_NORMAL_RESET 0
> +
> +enum pcie_data_rate {
> + PCIE_GEN1,
> + PCIE_GEN2,
> + PCIE_GEN3,
> + PCIE_GEN4
> +};
> +
> +struct meson_pcie_mem_res {
> + void __iomem *elbi_base; /* DT 0th resource */
> + void __iomem *cfg_base; /* DT 1nd resource */
> + void __iomem *phy_base; /* DT 2nd resource */
> +};
> +
> +struct meson_pcie_clk_res {
> + struct clk *clk;
> + struct clk *mipi_gate;
> + struct clk *port_clk;
> + struct clk *general_clk;
> +};
> +
> +struct meson_pcie_rc_reset {
> + struct reset_control *phy;
> + struct reset_control *port;
> + struct reset_control *apb;
> +};
> +
> +struct meson_pcie {
> + struct dw_pcie pci;
> + struct meson_pcie_mem_res mem_res;
> + struct meson_pcie_clk_res clk_res;
> + struct meson_pcie_rc_reset mrst;
> + struct gpio_desc *reset_gpio;
> +
> + enum of_gpio_flags gpio_flag;
> + int pcie_num;
> + u32 port_num;
> + u32 device_attch;
> +};
> +
> +static struct reset_control *meson_pcie_get_reset(struct meson_pcie *mp,
> + const char *id,
> + u32 reset_type)
> +{
> + struct device *dev = mp->pci.dev;
> + struct reset_control *reset;
> +
> + if (reset_type == PCIE_SHARED_RESET)
> + reset = devm_reset_control_get_shared(dev, id);
> + else
> + reset = devm_reset_control_get(dev, id);
> +
> + return reset;
> +}
> +
> +static int meson_pcie_get_resets(struct meson_pcie *mp)
> +{
> + struct meson_pcie_rc_reset *mrst = &mp->mrst;
> +
> + mrst->phy = meson_pcie_get_reset(mp, "phy", PCIE_SHARED_RESET);
> + if (IS_ERR(mrst->phy))
> + return PTR_ERR(mrst->phy);
> + reset_control_deassert(mrst->phy);
> +
> + mrst->port = meson_pcie_get_reset(mp, "port", PCIE_NORMAL_RESET);
> + if (IS_ERR(mrst->port))
> + return PTR_ERR(mrst->port);
> + reset_control_deassert(mrst->port);
> +
> + mrst->apb = meson_pcie_get_reset(mp, "apb", PCIE_SHARED_RESET);
> + if (IS_ERR(mrst->apb))
> + return PTR_ERR(mrst->apb);
> + reset_control_deassert(mrst->apb);
> +
> + return 0;
> +}
> +
> +static void __iomem *meson_pcie_get_mem_shared(struct platform_device *pdev,
> + struct meson_pcie *mp,
> + const char *id)
> +{
> + struct resource *res;
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, id);
> +
> + return ioremap(res->start, resource_size(res));
> +}
> +
> +static void __iomem *meson_pcie_get_mem(struct platform_device *pdev,
> + struct meson_pcie *mp,
> + const char *id)
> +{
> + struct device *dev = mp->pci.dev;
> + struct resource *res;
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, id);
> +
> + return devm_ioremap_resource(dev, res);
> +}
> +
> +static int meson_pcie_get_mems(struct platform_device *pdev,
> + struct meson_pcie *mp)
> +{
> + mp->mem_res.elbi_base = meson_pcie_get_mem(pdev, mp, "elbi");
> + if (IS_ERR(mp->mem_res.elbi_base))
> + return PTR_ERR(mp->mem_res.elbi_base);
> +
> + mp->mem_res.cfg_base = meson_pcie_get_mem(pdev, mp, "cfg");
> + if (IS_ERR(mp->mem_res.cfg_base))
> + return PTR_ERR(mp->mem_res.cfg_base);
> +
> + mp->mem_res.phy_base = meson_pcie_get_mem_shared(pdev, mp, "phy");
> + if (IS_ERR(mp->mem_res.phy_base))
> + return PTR_ERR(mp->mem_res.phy_base);
> +
> + return 0;
> +}
> +
> +static void meson_pcie_power_on(struct meson_pcie *mp)
> +{
> + writel(MESON_PCIE_PHY_POWERUP, mp->mem_res.phy_base);
> +}
> +
> +static void meson_pcie_reset(struct meson_pcie *mp)
> +{
> + struct meson_pcie_rc_reset *mrst = &mp->mrst;
> +
> + reset_control_assert(mrst->phy);
> + udelay(PCIE_RESET_DELAY);
> + reset_control_deassert(mrst->phy);
> + udelay(PCIE_RESET_DELAY);
> +
> + reset_control_assert(mrst->port);
> + reset_control_assert(mrst->apb);
> + udelay(PCIE_RESET_DELAY);
> + reset_control_deassert(mrst->port);
> + reset_control_deassert(mrst->apb);
> + udelay(PCIE_RESET_DELAY);
> +}
> +
> +static inline struct clk *meson_pcie_probe_clock(struct device *dev,
> + const char *id, u64 rate)
> +{
> + struct clk *clk = NULL;
> + int ret;
> +
> + clk = devm_clk_get(dev, id);
> + if (IS_ERR(clk))
> + return clk;
> +
> + if (rate) {
> + ret = clk_set_rate(clk, rate);
> + if (ret) {
> + dev_err(dev, "set clk rate failed, ret = %d\n", ret);
> + return ERR_PTR(ret);
> + }
> + }
> +
> + ret = clk_prepare_enable(clk);
> + if (ret) {
> + dev_err(dev, "couldn't enable clk\n");
> + return ERR_PTR(ret);
> + }
> +
> + devm_add_action_or_reset(dev,
> + (void (*) (void *))clk_disable_unprepare,
> + clk);
> +
> + return clk;
> +}
> +
> +static int meson_pcie_probe_clocks(struct meson_pcie *mp)
> +{
> + struct device *dev = mp->pci.dev;
> + struct meson_pcie_clk_res *res = &mp->clk_res;
> +
> + res->port_clk = meson_pcie_probe_clock(dev, "port", PORT_CLK_RATE);
> + if (IS_ERR(res->port_clk))
> + return PTR_ERR(res->port_clk);
> +
> + res->mipi_gate = meson_pcie_probe_clock(dev, "pcie_mipi_en", 0);
> + if (IS_ERR(res->mipi_gate))
> + return PTR_ERR(res->mipi_gate);
> +
> + res->general_clk = meson_pcie_probe_clock(dev, "pcie_general", 0);
> + if (IS_ERR(res->general_clk))
> + return PTR_ERR(res->general_clk);
> +
> + res->clk = meson_pcie_probe_clock(dev, "pcie", 0);
> + if (IS_ERR(res->clk))
> + return PTR_ERR(res->clk);
> +
> + return 0;
> +}
> +
> +static inline void meson_elb_writel(struct meson_pcie *mp, u32 val, u32 reg)
> +{
> + writel(val, mp->mem_res.elbi_base + reg);
> +}
> +
> +static inline u32 meson_elb_readl(struct meson_pcie *mp, u32 reg)
> +{
> + return readl(mp->mem_res.elbi_base + reg);
> +}
> +
> +static inline u32 meson_cfg_readl(struct meson_pcie *mp, u32 reg)
> +{
> + return readl(mp->mem_res.cfg_base + reg);
> +}
> +
> +static inline void meson_cfg_writel(struct meson_pcie *mp, u32 val, u32 reg)
> +{
> + writel(val, mp->mem_res.cfg_base + reg);
> +}
> +
> +static void meson_pcie_assert_reset(struct meson_pcie *mp)
> +{
> + gpiod_set_value_cansleep(mp->reset_gpio, 0);
> + udelay(500);
> + gpiod_set_value_cansleep(mp->reset_gpio, 1);
> +}
> +
> +static void meson_pcie_init_dw(struct meson_pcie *mp)
> +{
> + u32 val = 0;
> +
> + val = meson_cfg_readl(mp, PCIE_CFG0);
> + val |= APP_LTSSM_ENABLE;
> + meson_cfg_writel(mp, val, PCIE_CFG0);
> +
> + val = meson_elb_readl(mp, PCIE_PORT_LINK_CTRL_OFF);
> + val &= ~LINK_CAPABLE_MASK;
> + meson_elb_writel(mp, val, PCIE_PORT_LINK_CTRL_OFF);
> +
> + val = meson_elb_readl(mp, PCIE_PORT_LINK_CTRL_OFF);
> + val |= LINK_CAPABLE_X1 | FAST_LINK_MODE;
> + meson_elb_writel(mp, val, PCIE_PORT_LINK_CTRL_OFF);
> +
> + val = meson_elb_readl(mp, PCIE_GEN2_CTRL_OFF);
> + val &= ~NUM_OF_LANES_MASK;
> + meson_elb_writel(mp, val, PCIE_GEN2_CTRL_OFF);
> +
> + val = meson_elb_readl(mp, PCIE_GEN2_CTRL_OFF);
> + val |= NUM_OF_LANES_X1 | DIRECT_SPEED_CHANGE;
> + meson_elb_writel(mp, val, PCIE_GEN2_CTRL_OFF);
> +
> + meson_elb_writel(mp, 0x0, PCIE_BASE_ADDR0);
> + meson_elb_writel(mp, 0x0, PCIE_BASE_ADDR1);
> +}
> +
> +static int meson_size_to_payload(int size)
> +{
> + if (size & (size - 1) || size < 128 || size > 4096)
> + return 1;
> +
> + return fls(size) - 8;
> +}
> +
> +static void meson_set_max_payload(struct meson_pcie *mp, int size)
> +{
> + u32 val = 0;
> + int max_payload_size = meson_size_to_payload(size);
> +
> + val = meson_elb_readl(mp, PCIE_DEV_CTRL_DEV_STUS);
> + val &= ~PCIE_CAP_MAX_PAYLOAD_MASK;
> + meson_elb_writel(mp, val, PCIE_DEV_CTRL_DEV_STUS);
> +
> + val = meson_elb_readl(mp, PCIE_DEV_CTRL_DEV_STUS);
> + val |= PCIE_CAP_MAX_PAYLOAD_SIZE(max_payload_size);
> + meson_elb_writel(mp, val, PCIE_DEV_CTRL_DEV_STUS);
> +}
> +
> +static void meson_set_max_rd_req_size(struct meson_pcie *mp, int size)
> +{
> + u32 val = 0;
> + int max_rd_req_size = meson_size_to_payload(size);
> +
> + val = meson_elb_readl(mp, PCIE_DEV_CTRL_DEV_STUS);
> + val &= ~PCIE_CAP_MAX_READ_REQ_MASK;
> + meson_elb_writel(mp, val, PCIE_DEV_CTRL_DEV_STUS);
> +
> + val = meson_elb_readl(mp, PCIE_DEV_CTRL_DEV_STUS);
> + val |= PCIE_CAP_MAX_READ_REQ_SIZE(max_rd_req_size);
> + meson_elb_writel(mp, val, PCIE_DEV_CTRL_DEV_STUS);
> +}
> +
> +static inline void meson_enable_memory_space(struct meson_pcie *mp)
> +{
> + /* Set the RC Bus Master, Memory Space and I/O Space enables */
> + meson_elb_writel(mp, PCI_IO_EN | PCI_MEM_SPACE_EN | PCI_BUS_MASTER_EN,
> + PCIE_STATUS_COMMAND);
> +}
> +
> +static int meson_pcie_establish_link(struct meson_pcie *mp)
> +{
> + struct dw_pcie *pci = &mp->pci;
> + struct pcie_port *pp = &pci->pp;
> +
> + meson_pcie_init_dw(mp);
> + meson_set_max_payload(mp, MAX_PAYLOAD_SIZE);
> + meson_set_max_rd_req_size(mp, MAX_READ_REQ_SIZE);
> +
> + dw_pcie_setup_rc(pp);
> + meson_enable_memory_space(mp);
> +
> + meson_pcie_assert_reset(mp);
> +
> + /* check if the link is up or not */
> + if (!dw_pcie_wait_for_link(pci))
> + return 0;
> +
> + return -ETIMEDOUT;
> +}
> +
> +static void meson_pcie_msi_init(struct meson_pcie *mp)
> +{
> + struct pcie_port *pp = &mp->pci.pp;
> +
> + dw_pcie_msi_init(pp);
> +}
> +
> +static void meson_pcie_enable_interrupts(struct meson_pcie *mp)
> +{
> + if (IS_ENABLED(CONFIG_PCI_MSI))
> + meson_pcie_msi_init(mp);
> +}
> +
> +static u32 meson_pcie_read_dbi(struct dw_pcie *pci, void __iomem *base,
> + u32 reg, size_t size)
> +{
> + u32 val;
> +
> + dw_pcie_read(base + reg, size, &val);
> +
> + return val;
> +}
> +
> +static void meson_pcie_write_dbi(struct dw_pcie *pci, void __iomem *base,
> + u32 reg, size_t size, u32 val)
> +{
> + dw_pcie_write(base + reg, size, val);
> +}
> +
> +static int meson_pcie_rd_own_conf(struct pcie_port *pp, int where, int size,
> + u32 *val)
> +{
> + struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> + struct meson_pcie *mp = to_meson_pcie(pci);
> +
> + if (!mp->device_attch)
> + return 0;
> +
> + /* the device class is not reported correctly from the register */
> + if (where == PCI_CLASS_REVISION) {
> + *val = readl(pci->dbi_base + PCI_CLASS_REVISION);
> + /* keep revision id */
> + *val &= PCI_CLASS_REVISION_MASK;
> + *val |= PCI_CLASS_BRIDGE_PCI << 16;
> + return PCIBIOS_SUCCESSFUL;
> + }
> +
> + return dw_pcie_read(pci->dbi_base + where, size, val);
> +}
> +
> +static int meson_pcie_wr_own_conf(struct pcie_port *pp, int where,
> + int size, u32 val)
> +{
> + struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> + struct meson_pcie *mp = to_meson_pcie(pci);
> +
> + if (!mp->device_attch)
> + return 0;
> +
> + return dw_pcie_write(pci->dbi_base + where, size, val);
> +}
> +
> +static int meson_pcie_link_up(struct dw_pcie *pci)
> +{
> + struct meson_pcie *mp = to_meson_pcie(pci);
> + struct device *dev = pci->dev;
> + u32 smlh_up = 0;
> + u32 ltssm_up = 0;
> + u32 rdlh_up = 0;
> + u32 speed_okay = 0;
> + u32 cnt = 0;
> + u32 state12, state17;
> +
> + while (smlh_up == 0 || rdlh_up == 0 || ltssm_up == 0 ||
> + speed_okay == 0) {
> + udelay(20);
> +
> + state12 = meson_cfg_readl(mp, PCIE_CFG_STATUS12);
> + state17 = meson_cfg_readl(mp, PCIE_CFG_STATUS17);
> + smlh_up = IS_SMLH_LINK_UP(state12);
> + rdlh_up = IS_RDLH_LINK_UP(state12);
> + ltssm_up = IS_LTSSM_UP(state12);
> +
> + if (PM_CURRENT_STATE(state17) < PCIE_GEN3)
> + speed_okay = 1;
> +
> + if (smlh_up)
> + dev_dbg(dev, "smlh_link_up is on\n");
> + if (rdlh_up)
> + dev_dbg(dev, "rdlh_link_up is on\n");
> + if (ltssm_up)
> + dev_dbg(dev, "ltssm_up is on\n");
> + if (speed_okay)
> + dev_dbg(dev, "speed_okay\n");
> +
> + cnt++;
> +
> + if (cnt >= WAIT_LINKUP_TIMEOUT) {
> + dev_err(dev, "Error: Wait linkup timeout.\n");
> + return 0;
> + }
> + }
> +
> + return 1;
> +}
> +
> +static int meson_pcie_host_init(struct pcie_port *pp)
> +{
> + struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> + struct meson_pcie *mp = to_meson_pcie(pci);
> + int ret;
> +
> + ret = meson_pcie_establish_link(mp);
> +
> + if (ret)
> + return ret;
> +
> + mp->device_attch = 1;
> + meson_pcie_enable_interrupts(mp);
> +
> + return 0;
> +}
> +
> +static const struct dw_pcie_host_ops meson_pcie_host_ops = {
> + .rd_own_conf = meson_pcie_rd_own_conf,
> + .wr_own_conf = meson_pcie_wr_own_conf,
> + .host_init = meson_pcie_host_init,
> +};
> +
> +static int meson_add_pcie_port(struct meson_pcie *mp,
> + struct platform_device *pdev)
> +{
> + struct dw_pcie *pci = &mp->pci;
> + struct pcie_port *pp = &pci->pp;
> + struct device *dev = &pdev->dev;
> + int ret;
> +
> + if (IS_ENABLED(CONFIG_PCI_MSI)) {
> + pp->msi_irq = platform_get_irq(pdev, 0);
> + if (pp->msi_irq < 0) {
> + dev_err(dev, "failed to get msi irq\n");
> + return pp->msi_irq;
> + }
> + }
> +
> + pp->root_bus_nr = -1;
> + pp->ops = &meson_pcie_host_ops;
> + pci->dbi_base = mp->mem_res.elbi_base;
> +
> + ret = dw_pcie_host_init(pp);
> + if (ret) {
> + dev_err(dev, "failed to initialize host\n");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static const struct dw_pcie_ops dw_pcie_ops = {
> + .read_dbi = meson_pcie_read_dbi,
> + .write_dbi = meson_pcie_write_dbi,
> + .link_up = meson_pcie_link_up,
> +};
> +
> +static int meson_pcie_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct dw_pcie *pci;
> + struct meson_pcie *mp;
> + int ret;
> +
> + mp = devm_kzalloc(dev, sizeof(*mp), GFP_KERNEL);
> + if (!mp)
> + return -ENOMEM;
> +
> + pci = &mp->pci;
> + pci->dev = dev;
> + pci->ops = &dw_pcie_ops;
> +
> + mp->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
> + if (IS_ERR(mp->reset_gpio)) {
> + dev_err(dev, "Get reset gpio failed\n");
> + return PTR_ERR(mp->reset_gpio);
> + }
> +
> + ret = meson_pcie_get_resets(mp);
> + if (ret) {
> + dev_err(dev, "Get reset resource failed, %d\n", ret);
> + return ret;
> + }
> +
> + ret = meson_pcie_get_mems(pdev, mp);
> + if (ret) {
> + dev_err(dev, "Get memory resource failed, %d\n", ret);
> + return ret;
> + }
> +
> + meson_pcie_power_on(mp);
> + meson_pcie_reset(mp);
> +
> + ret = meson_pcie_probe_clocks(mp);
> + if (ret) {
> + dev_err(dev, "Init clock resources failed, %d\n", ret);
> + return ret;
> + }
> +
> + platform_set_drvdata(pdev, mp);
> +
> + ret = meson_add_pcie_port(mp, pdev);
> + if (ret < 0)
> + dev_err(dev, "Add PCIE port failed, %d\n", ret);
> +
> + return ret;
> +}
> +
> +static const struct of_device_id meson_pcie_of_match[] = {
> + {
> + .compatible = "amlogic,axg-pcie",
> + },
> + {},
> +};
> +
> +static struct platform_driver meson_pcie_driver = {
> + .probe = meson_pcie_probe,
> + .driver = {
> + .name = "meson-pcie",
> + .of_match_table = meson_pcie_of_match,
> + },
> +};
> +
> +builtin_platform_driver(meson_pcie_driver);
>


2018-09-26 06:45:57

by Hanjie Lin

[permalink] [raw]
Subject: Re: [RESEND PATCH v3 2/2] PCI: meson: add the Amlogic Meson PCIe controller driver



On 2018/9/21 22:47, Lorenzo Pieralisi wrote:
> On Fri, Sep 21, 2018 at 02:03:40PM +0800, Hanjie Lin wrote:
>> From: Yue Wang <[email protected]>
>>
>> The Amlogic Meson PCIe host controller is based on the Synopsys DesignWare
>> PCI core. This patch adds the driver support for Meson PCIe controller.
>
> Please read:
>
> https://marc.info/?l=linux-pci&m=150905742808166&w=2
>
> update the $SUBJECT accordingly, I can do it but you will repost anyway
> so please do it yourself while at it.
>

OK, I will follow the list and check the patch.

>> Signed-off-by: Yue Wang <[email protected]>
>> Signed-off-by: Hanjie Lin <[email protected]>
>> ---
>> drivers/pci/controller/dwc/Kconfig | 12 +
>> drivers/pci/controller/dwc/Makefile | 1 +
>> drivers/pci/controller/dwc/pci-meson.c | 617 +++++++++++++++++++++++++++++++++
>> 3 files changed, 630 insertions(+)
>> create mode 100644 drivers/pci/controller/dwc/pci-meson.c
>
> I would request a MAINTAINERS update too.
>

I will update MAINTAINERS in this patch of next-version.

>> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
>> index 91b0194..6cb36f6 100644
>> --- a/drivers/pci/controller/dwc/Kconfig
>> +++ b/drivers/pci/controller/dwc/Kconfig
>> @@ -193,4 +193,16 @@ config PCIE_HISI_STB
>> help
>> Say Y here if you want PCIe controller support on HiSilicon STB SoCs
>>
>> +config PCI_MESON
>> + bool "MESON PCIe controller"
>> + depends on PCI
>
> This is redundant, remove it.
>

Yes, really redundant I will remove it.

>> + depends on PCI_MSI_IRQ_DOMAIN
>> + select PCIEPORTBUS
>
> There is no real dependency on it, this should be left to kernel
> configuration, remove it.
>

Yes, I will remove it.

>> + select PCIE_DW_HOST
>> + help
>> + Say Y here if you want to enable PCI controller support on Amlogic
>> + SoCs. The PCI controller on Amlogic is based on DesignWare hardware
>> + and therefore the driver re-uses the DesignWare core functions to
>> + implement the driver.
>> +
>> endmenu
>> diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
>> index 5d2ce72..cf676bd 100644
>> --- a/drivers/pci/controller/dwc/Makefile
>> +++ b/drivers/pci/controller/dwc/Makefile
>> @@ -14,6 +14,7 @@ obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o
>> obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o
>> obj-$(CONFIG_PCIE_KIRIN) += pcie-kirin.o
>> obj-$(CONFIG_PCIE_HISI_STB) += pcie-histb.o
>> +obj-$(CONFIG_PCI_MESON) += pci-meson.o
>>
>> # The following drivers are for devices that use the generic ACPI
>> # pci_root.c driver but don't support standard ECAM config access.
>> diff --git a/drivers/pci/controller/dwc/pci-meson.c b/drivers/pci/controller/dwc/pci-meson.c
>> new file mode 100644
>> index 0000000..9c92a89
>> --- /dev/null
>> +++ b/drivers/pci/controller/dwc/pci-meson.c
>> @@ -0,0 +1,617 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * PCIe host controller driver for Amlogic MESON SoCs
>> + *
>> + * Copyright (c) 2018 Amlogic, inc.
>> + * Author: Yue Wang <[email protected]>
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/delay.h>
>> +#include <linux/of_device.h>
>> +#include <linux/of_gpio.h>
>> +#include <linux/pci.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/resource.h>
>> +#include <linux/types.h>
>> +#include <linux/reset.h>
>> +
>> +#include "pcie-designware.h"
>> +
>> +#define to_meson_pcie(x) dev_get_drvdata((x)->dev)
>
> Side note: this macro can be made dwc global since it is the same for all
> dwc based host bridges.
>

Yes, actually this patch refers to the other drivers in dwc directory.

>> +/* External local bus interface registers */
>> +#define PLR_OFFSET 0x700
>> +#define PCIE_PORT_LINK_CTRL_OFF (PLR_OFFSET + 0x10)
>> +#define FAST_LINK_MODE BIT(7)
>> +#define LINK_CAPABLE_MASK GENMASK(21, 16)
>> +#define LINK_CAPABLE_X1 BIT(16)
>> +
>> +#define PCIE_GEN2_CTRL_OFF (PLR_OFFSET + 0x10c)
>> +#define NUM_OF_LANES_MASK GENMASK(12, 8)
>> +#define NUM_OF_LANES_X1 BIT(8)
>> +#define DIRECT_SPEED_CHANGE BIT(17)
>> +
>> +#define TYPE1_HDR_OFFSET 0x0
>> +#define PCIE_STATUS_COMMAND (TYPE1_HDR_OFFSET + 0x04)
>> +#define PCI_IO_EN BIT(0)
>> +#define PCI_MEM_SPACE_EN BIT(1)
>> +#define PCI_BUS_MASTER_EN BIT(2)
>> +
>> +#define PCIE_BASE_ADDR0 (TYPE1_HDR_OFFSET + 0x10)
>> +#define PCIE_BASE_ADDR1 (TYPE1_HDR_OFFSET + 0x14)
>> +
>> +#define PCIE_CAP_OFFSET 0x70
>> +#define PCIE_DEV_CTRL_DEV_STUS (PCIE_CAP_OFFSET + 0x08)
>> +#define PCIE_CAP_MAX_PAYLOAD_MASK GENMASK(7, 5)
>> +#define PCIE_CAP_MAX_PAYLOAD_SIZE(x) ((x) << 5)
>> +#define PCIE_CAP_MAX_READ_REQ_MASK GENMASK(14, 12)
>> +#define PCIE_CAP_MAX_READ_REQ_SIZE(x) ((x) << 12)
>> +
>> +#define PCI_CLASS_REVISION_MASK GENMASK(7, 0)
>> +
>> +/* PCIe specific config registers */
>> +#define PCIE_CFG0 0x0
>> +#define APP_LTSSM_ENABLE BIT(7)
>> +
>> +#define PCIE_CFG_STATUS12 0x30
>> +#define IS_SMLH_LINK_UP(x) ((x) & (1 << 6))
>> +#define IS_RDLH_LINK_UP(x) ((x) & (1 << 16))
>> +#define IS_LTSSM_UP(x) ((((x) >> 10) & 0x1f) == 0x11)
>> +
>> +#define PCIE_CFG_STATUS17 0x44
>> +#define PM_CURRENT_STATE(x) (((x) >> 7) & 0x1)
>> +
>> +#define WAIT_LINKUP_TIMEOUT 2000
>> +#define PORT_CLK_RATE 100000000UL
>> +#define MAX_PAYLOAD_SIZE 256
>> +#define MAX_READ_REQ_SIZE 256
>> +#define MESON_PCIE_PHY_POWERUP 0x1c
>> +#define PCIE_RESET_DELAY 500
>> +#define PCIE_SHARED_RESET 1
>> +#define PCIE_NORMAL_RESET 0
>> +
>> +enum pcie_data_rate {
>> + PCIE_GEN1,
>> + PCIE_GEN2,
>> + PCIE_GEN3,
>> + PCIE_GEN4
>> +};
>> +
>> +struct meson_pcie_mem_res {
>> + void __iomem *elbi_base; /* DT 0th resource */
>> + void __iomem *cfg_base; /* DT 1nd resource */
>> + void __iomem *phy_base; /* DT 2nd resource */
>> +};
>> +
>> +struct meson_pcie_clk_res {
>> + struct clk *clk;
>> + struct clk *mipi_gate;
>> + struct clk *port_clk;
>> + struct clk *general_clk;
>> +};
>> +
>> +struct meson_pcie_rc_reset {
>> + struct reset_control *phy;
>> + struct reset_control *port;
>> + struct reset_control *apb;
>> +};
>> +
>> +struct meson_pcie {
>> + struct dw_pcie pci;
>> + struct meson_pcie_mem_res mem_res;
>> + struct meson_pcie_clk_res clk_res;
>> + struct meson_pcie_rc_reset mrst;
>> + struct gpio_desc *reset_gpio;
>> +
>> + enum of_gpio_flags gpio_flag;
>> + int pcie_num;
>> + u32 port_num;
>> + u32 device_attch;
>
> What's this variable for and is it really needed ?
>

It was designed for dw_pcie_host_ops operations protection.
It seems to be unnecessary when we looking into the code.
We will continue to confirm and remove it if necessary.

>> +};
>> +
>> +static struct reset_control *meson_pcie_get_reset(struct meson_pcie *mp,
>> + const char *id,
>> + u32 reset_type)
>> +{
>> + struct device *dev = mp->pci.dev;
>> + struct reset_control *reset;
>> +
>> + if (reset_type == PCIE_SHARED_RESET)
>> + reset = devm_reset_control_get_shared(dev, id);
>> + else
>> + reset = devm_reset_control_get(dev, id);
>> +
>> + return reset;
>> +}
>> +
>> +static int meson_pcie_get_resets(struct meson_pcie *mp)
>> +{
>> + struct meson_pcie_rc_reset *mrst = &mp->mrst;
>> +
>> + mrst->phy = meson_pcie_get_reset(mp, "phy", PCIE_SHARED_RESET);
>> + if (IS_ERR(mrst->phy))
>> + return PTR_ERR(mrst->phy);
>> + reset_control_deassert(mrst->phy);
>> +
>> + mrst->port = meson_pcie_get_reset(mp, "port", PCIE_NORMAL_RESET);
>> + if (IS_ERR(mrst->port))
>> + return PTR_ERR(mrst->port);
>> + reset_control_deassert(mrst->port);
>> +
>> + mrst->apb = meson_pcie_get_reset(mp, "apb", PCIE_SHARED_RESET);
>> + if (IS_ERR(mrst->apb))
>> + return PTR_ERR(mrst->apb);
>> + reset_control_deassert(mrst->apb);
>> +
>> + return 0;
>> +}
>> +
>> +static void __iomem *meson_pcie_get_mem_shared(struct platform_device *pdev,
>> + struct meson_pcie *mp,
>> + const char *id)
>> +{
>> + struct resource *res;
>> +
>> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, id);
>> +
>> + return ioremap(res->start, resource_size(res));
>> +}
>> +
>> +static void __iomem *meson_pcie_get_mem(struct platform_device *pdev,
>> + struct meson_pcie *mp,
>> + const char *id)
>> +{
>> + struct device *dev = mp->pci.dev;
>> + struct resource *res;
>> +
>> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, id);
>> +
>> + return devm_ioremap_resource(dev, res);
>> +}
>> +
>> +static int meson_pcie_get_mems(struct platform_device *pdev,
>> + struct meson_pcie *mp)
>> +{
>> + mp->mem_res.elbi_base = meson_pcie_get_mem(pdev, mp, "elbi");
>> + if (IS_ERR(mp->mem_res.elbi_base))
>> + return PTR_ERR(mp->mem_res.elbi_base);
>> +
>> + mp->mem_res.cfg_base = meson_pcie_get_mem(pdev, mp, "cfg");
>> + if (IS_ERR(mp->mem_res.cfg_base))
>> + return PTR_ERR(mp->mem_res.cfg_base);
>> +
>> + mp->mem_res.phy_base = meson_pcie_get_mem_shared(pdev, mp, "phy");
>> + if (IS_ERR(mp->mem_res.phy_base))
>> + return PTR_ERR(mp->mem_res.phy_base);
>
> I do no understand the logic behind this _shared() extension and how
> it would work in the failure path.
>

Yes, this 'shared' likely to leads to some confusion.
Because we have two PCI controllers in meson SoC and they share one phy register
bank to handle power-on thing, so it's just a prompt.
Currently there is no error-handler and does not check the res before ioremap().
I think it's better to use devm_ioremap_resource() instead and give a comment for
'phy' register.

>> + return 0;
>> +}
>> +
>> +static void meson_pcie_power_on(struct meson_pcie *mp)
>> +{
>> + writel(MESON_PCIE_PHY_POWERUP, mp->mem_res.phy_base);
>> +}
>> +
>> +static void meson_pcie_reset(struct meson_pcie *mp)
>> +{
>> + struct meson_pcie_rc_reset *mrst = &mp->mrst;
>> +
>> + reset_control_assert(mrst->phy);
>> + udelay(PCIE_RESET_DELAY);
>> + reset_control_deassert(mrst->phy);
>> + udelay(PCIE_RESET_DELAY);
>> +
>> + reset_control_assert(mrst->port);
>> + reset_control_assert(mrst->apb);
>> + udelay(PCIE_RESET_DELAY);
>> + reset_control_deassert(mrst->port);
>> + reset_control_deassert(mrst->apb);
>> + udelay(PCIE_RESET_DELAY);
>> +}
>> +
>> +static inline struct clk *meson_pcie_probe_clock(struct device *dev,
>> + const char *id, u64 rate)
>> +{
>> + struct clk *clk = NULL;
>
> Nit: useless initialization.
>

OK, I will remove it.

>> + int ret;
>> +
>> + clk = devm_clk_get(dev, id);
>> + if (IS_ERR(clk))
>> + return clk;
>> +
>> + if (rate) {
>> + ret = clk_set_rate(clk, rate);
>> + if (ret) {
>> + dev_err(dev, "set clk rate failed, ret = %d\n", ret);
>> + return ERR_PTR(ret);
>> + }
>> + }
>> +
>> + ret = clk_prepare_enable(clk);
>> + if (ret) {
>> + dev_err(dev, "couldn't enable clk\n");
>> + return ERR_PTR(ret);
>> + }
>> +
>> + devm_add_action_or_reset(dev,
>> + (void (*) (void *))clk_disable_unprepare,
>> + clk);
>> +
>> + return clk;
>> +}
>> +
>> +static int meson_pcie_probe_clocks(struct meson_pcie *mp)
>> +{
>> + struct device *dev = mp->pci.dev;
>> + struct meson_pcie_clk_res *res = &mp->clk_res;
>> +
>> + res->port_clk = meson_pcie_probe_clock(dev, "port", PORT_CLK_RATE);
>> + if (IS_ERR(res->port_clk))
>> + return PTR_ERR(res->port_clk);
>> +
>> + res->mipi_gate = meson_pcie_probe_clock(dev, "pcie_mipi_en", 0);
>> + if (IS_ERR(res->mipi_gate))
>> + return PTR_ERR(res->mipi_gate);
>> +
>> + res->general_clk = meson_pcie_probe_clock(dev, "pcie_general", 0);
>> + if (IS_ERR(res->general_clk))
>> + return PTR_ERR(res->general_clk);
>> +
>> + res->clk = meson_pcie_probe_clock(dev, "pcie", 0);
>> + if (IS_ERR(res->clk))
>> + return PTR_ERR(res->clk);
>> +
>> + return 0;
>> +}
>> +
>> +static inline void meson_elb_writel(struct meson_pcie *mp, u32 val, u32 reg)
>> +{
>> + writel(val, mp->mem_res.elbi_base + reg);
>> +}
>> +
>> +static inline u32 meson_elb_readl(struct meson_pcie *mp, u32 reg)
>> +{
>> + return readl(mp->mem_res.elbi_base + reg);
>> +}
>> +
>> +static inline u32 meson_cfg_readl(struct meson_pcie *mp, u32 reg)
>> +{
>> + return readl(mp->mem_res.cfg_base + reg);
>> +}
>> +
>> +static inline void meson_cfg_writel(struct meson_pcie *mp, u32 val, u32 reg)
>> +{
>> + writel(val, mp->mem_res.cfg_base + reg);
>> +}
>> +
>> +static void meson_pcie_assert_reset(struct meson_pcie *mp)
>> +{
>> + gpiod_set_value_cansleep(mp->reset_gpio, 0);
>> + udelay(500);
>> + gpiod_set_value_cansleep(mp->reset_gpio, 1);
>> +}
>> +
>> +static void meson_pcie_init_dw(struct meson_pcie *mp)
>> +{
>> + u32 val = 0;
>
> Useless initialization.
>

OK, I will remove it.

>> + val = meson_cfg_readl(mp, PCIE_CFG0);
>> + val |= APP_LTSSM_ENABLE;
>> + meson_cfg_writel(mp, val, PCIE_CFG0);
>> +
>> + val = meson_elb_readl(mp, PCIE_PORT_LINK_CTRL_OFF);
>> + val &= ~LINK_CAPABLE_MASK;
>> + meson_elb_writel(mp, val, PCIE_PORT_LINK_CTRL_OFF);
>> +
>> + val = meson_elb_readl(mp, PCIE_PORT_LINK_CTRL_OFF);
>> + val |= LINK_CAPABLE_X1 | FAST_LINK_MODE;
>> + meson_elb_writel(mp, val, PCIE_PORT_LINK_CTRL_OFF);
>> +
>> + val = meson_elb_readl(mp, PCIE_GEN2_CTRL_OFF);
>> + val &= ~NUM_OF_LANES_MASK;
>> + meson_elb_writel(mp, val, PCIE_GEN2_CTRL_OFF);
>> +
>> + val = meson_elb_readl(mp, PCIE_GEN2_CTRL_OFF);
>> + val |= NUM_OF_LANES_X1 | DIRECT_SPEED_CHANGE;
>> + meson_elb_writel(mp, val, PCIE_GEN2_CTRL_OFF);
>> +
>> + meson_elb_writel(mp, 0x0, PCIE_BASE_ADDR0);
>> + meson_elb_writel(mp, 0x0, PCIE_BASE_ADDR1);
>> +}
>> +
>> +static int meson_size_to_payload(int size)
>> +{
>> + if (size & (size - 1) || size < 128 || size > 4096)
>> + return 1;
>
> Please expain this statement. It deserves a comment at least.
>

Yes, I will add the comment in next-version.

>> +
>> + return fls(size) - 8;
>> +}
>> +
>> +static void meson_set_max_payload(struct meson_pcie *mp, int size)
>> +{
>> + u32 val = 0;
>> + int max_payload_size = meson_size_to_payload(size);
>> +
>> + val = meson_elb_readl(mp, PCIE_DEV_CTRL_DEV_STUS);
>> + val &= ~PCIE_CAP_MAX_PAYLOAD_MASK;
>> + meson_elb_writel(mp, val, PCIE_DEV_CTRL_DEV_STUS);
>> +
>> + val = meson_elb_readl(mp, PCIE_DEV_CTRL_DEV_STUS);
>> + val |= PCIE_CAP_MAX_PAYLOAD_SIZE(max_payload_size);
>> + meson_elb_writel(mp, val, PCIE_DEV_CTRL_DEV_STUS);
>> +}
>> +
>> +static void meson_set_max_rd_req_size(struct meson_pcie *mp, int size)
>> +{
>> + u32 val = 0;
>
> Useless initialization.
>

OK, I will remove it.

>> + int max_rd_req_size = meson_size_to_payload(size);
>> +
>> + val = meson_elb_readl(mp, PCIE_DEV_CTRL_DEV_STUS);
>> + val &= ~PCIE_CAP_MAX_READ_REQ_MASK;
>> + meson_elb_writel(mp, val, PCIE_DEV_CTRL_DEV_STUS);
>> +
>> + val = meson_elb_readl(mp, PCIE_DEV_CTRL_DEV_STUS);
>> + val |= PCIE_CAP_MAX_READ_REQ_SIZE(max_rd_req_size);
>> + meson_elb_writel(mp, val, PCIE_DEV_CTRL_DEV_STUS);
>> +}
>> +
>> +static inline void meson_enable_memory_space(struct meson_pcie *mp)
>> +{
>> + /* Set the RC Bus Master, Memory Space and I/O Space enables */
>> + meson_elb_writel(mp, PCI_IO_EN | PCI_MEM_SPACE_EN | PCI_BUS_MASTER_EN,
>> + PCIE_STATUS_COMMAND);
>> +}
>> +
>> +static int meson_pcie_establish_link(struct meson_pcie *mp)
>> +{
>> + struct dw_pcie *pci = &mp->pci;
>> + struct pcie_port *pp = &pci->pp;
>> +
>> + meson_pcie_init_dw(mp);
>> + meson_set_max_payload(mp, MAX_PAYLOAD_SIZE);
>> + meson_set_max_rd_req_size(mp, MAX_READ_REQ_SIZE);
>> +
>> + dw_pcie_setup_rc(pp);
>> + meson_enable_memory_space(mp);
>> +
>> + meson_pcie_assert_reset(mp);
>> +
>> + /* check if the link is up or not */
>> + if (!dw_pcie_wait_for_link(pci))
>> + return 0;
>> +
>> + return -ETIMEDOUT;
>
> return dw_pcie_wait_for_link(pci));
>
> ?
>

Yes, it's more clear.

>> +}
>> +
>> +static void meson_pcie_msi_init(struct meson_pcie *mp)
>> +{
>> + struct pcie_port *pp = &mp->pci.pp;
>> +
>> + dw_pcie_msi_init(pp);
>> +}
>> +
>> +static void meson_pcie_enable_interrupts(struct meson_pcie *mp)
>> +{
>> + if (IS_ENABLED(CONFIG_PCI_MSI))
>> + meson_pcie_msi_init(mp);
>> +}
>
> You do not need two functions to do what can be done in one.
>
> All you need is struct pcie_port instead of calling this function
> with struct meson_pci.
>

Yes, these two functions are too redundant.
I will remove function meson_pcie_msi_init().

>> +
>> +static u32 meson_pcie_read_dbi(struct dw_pcie *pci, void __iomem *base,
>> + u32 reg, size_t size)
>> +{
>> + u32 val;
>> +
>> + dw_pcie_read(base + reg, size, &val);
>> +
>> + return val;
>> +}
>> +
>> +static void meson_pcie_write_dbi(struct dw_pcie *pci, void __iomem *base,
>> + u32 reg, size_t size, u32 val)
>> +{
>> + dw_pcie_write(base + reg, size, val);
>> +}
>
> Nit: there is nothing dbi specific in these two functions.
>

We will check this code.

>> +
>> +static int meson_pcie_rd_own_conf(struct pcie_port *pp, int where, int size,
>> + u32 *val)
>> +{
>> + struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>> + struct meson_pcie *mp = to_meson_pcie(pci);
>> +
>> + if (!mp->device_attch)
>> + return 0;
>
> Why do you need this guard ?
>

We'll continue to check this guard and remove it if necessary.

>> +
>> + /* the device class is not reported correctly from the register */
>> + if (where == PCI_CLASS_REVISION) {
>> + *val = readl(pci->dbi_base + PCI_CLASS_REVISION);
>> + /* keep revision id */
>> + *val &= PCI_CLASS_REVISION_MASK;
>> + *val |= PCI_CLASS_BRIDGE_PCI << 16;
>> + return PCIBIOS_SUCCESSFUL;
>> + }
>> +
>> + return dw_pcie_read(pci->dbi_base + where, size, val);
>> +}
>> +
>> +static int meson_pcie_wr_own_conf(struct pcie_port *pp, int where,
>> + int size, u32 val)
>> +{
>> + struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>> + struct meson_pcie *mp = to_meson_pcie(pci);
>> +
>> + if (!mp->device_attch)
>> + return 0;
>
> Same question.
>
>> +
>> + return dw_pcie_write(pci->dbi_base + where, size, val);
>> +}
>> +
>> +static int meson_pcie_link_up(struct dw_pcie *pci)
>> +{
>> + struct meson_pcie *mp = to_meson_pcie(pci);
>> + struct device *dev = pci->dev;
>> + u32 smlh_up = 0;
>> + u32 ltssm_up = 0;
>> + u32 rdlh_up = 0;
>> + u32 speed_okay = 0;
>> + u32 cnt = 0;
>> + u32 state12, state17;
>> +
>> + while (smlh_up == 0 || rdlh_up == 0 || ltssm_up == 0 ||
>> + speed_okay == 0) {
>> + udelay(20);
>> +
>> + state12 = meson_cfg_readl(mp, PCIE_CFG_STATUS12);
>> + state17 = meson_cfg_readl(mp, PCIE_CFG_STATUS17);
>> + smlh_up = IS_SMLH_LINK_UP(state12);
>> + rdlh_up = IS_RDLH_LINK_UP(state12);
>> + ltssm_up = IS_LTSSM_UP(state12);
>> +
>> + if (PM_CURRENT_STATE(state17) < PCIE_GEN3)
>> + speed_okay = 1;
>> +
>> + if (smlh_up)
>> + dev_dbg(dev, "smlh_link_up is on\n");
>> + if (rdlh_up)
>> + dev_dbg(dev, "rdlh_link_up is on\n");
>> + if (ltssm_up)
>> + dev_dbg(dev, "ltssm_up is on\n");
>> + if (speed_okay)
>> + dev_dbg(dev, "speed_okay\n");
>> +
>> + cnt++;
>> +
>> + if (cnt >= WAIT_LINKUP_TIMEOUT) {
>> + dev_err(dev, "Error: Wait linkup timeout.\n");
>> + return 0;
>> + }
>> + }
>> +
>> + return 1;
>> +}
>> +
>> +static int meson_pcie_host_init(struct pcie_port *pp)
>> +{
>> + struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>> + struct meson_pcie *mp = to_meson_pcie(pci);
>> + int ret;
>> +
>> + ret = meson_pcie_establish_link(mp);
>> +
>
> Remove this empty line.
>

OK.

>> + if (ret)
>> + return ret;
>> +
>> + mp->device_attch = 1;
>
> I want to understand why you need this variable at all (and why it is
> an u32).
>
>> + meson_pcie_enable_interrupts(mp);
>> +
>> + return 0;
>> +}
>> +
>> +static const struct dw_pcie_host_ops meson_pcie_host_ops = {
>> + .rd_own_conf = meson_pcie_rd_own_conf,
>> + .wr_own_conf = meson_pcie_wr_own_conf,
>> + .host_init = meson_pcie_host_init,
>> +};
>> +
>> +static int meson_add_pcie_port(struct meson_pcie *mp,
>> + struct platform_device *pdev)
>> +{
>> + struct dw_pcie *pci = &mp->pci;
>> + struct pcie_port *pp = &pci->pp;
>> + struct device *dev = &pdev->dev;
>> + int ret;
>> +
>> + if (IS_ENABLED(CONFIG_PCI_MSI)) {
>> + pp->msi_irq = platform_get_irq(pdev, 0);
>> + if (pp->msi_irq < 0) {
>> + dev_err(dev, "failed to get msi irq\n");
>> + return pp->msi_irq;
>> + }
>> + }
>> +
>> + pp->root_bus_nr = -1;
>
> This is useless, since it is initialized in dw_pcie_host_init(), remove
> it.
>

Yes, we will remove it.

>> + pp->ops = &meson_pcie_host_ops;
>> + pci->dbi_base = mp->mem_res.elbi_base;
>> +
>> + ret = dw_pcie_host_init(pp);
>> + if (ret) {
>> + dev_err(dev, "failed to initialize host\n");
>> + return ret;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static const struct dw_pcie_ops dw_pcie_ops = {
>> + .read_dbi = meson_pcie_read_dbi,
>> + .write_dbi = meson_pcie_write_dbi,
>> + .link_up = meson_pcie_link_up,
>> +};
>> +
>> +static int meson_pcie_probe(struct platform_device *pdev)
>> +{
>> + struct device *dev = &pdev->dev;
>> + struct dw_pcie *pci;
>> + struct meson_pcie *mp;
>> + int ret;
>> +
>> + mp = devm_kzalloc(dev, sizeof(*mp), GFP_KERNEL);
>> + if (!mp)
>> + return -ENOMEM;
>> +
>> + pci = &mp->pci;
>> + pci->dev = dev;
>> + pci->ops = &dw_pcie_ops;
>> +
>> + mp->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
>> + if (IS_ERR(mp->reset_gpio)) {
>> + dev_err(dev, "Get reset gpio failed\n");
>> + return PTR_ERR(mp->reset_gpio);
>> + }
>> +
>> + ret = meson_pcie_get_resets(mp);
>> + if (ret) {
>> + dev_err(dev, "Get reset resource failed, %d\n", ret);
>> + return ret;
>> + }
>> +
>> + ret = meson_pcie_get_mems(pdev, mp);
>> + if (ret) {
>> + dev_err(dev, "Get memory resource failed, %d\n", ret);
>> + return ret;
>> + }
>> +
>> + meson_pcie_power_on(mp);
>> + meson_pcie_reset(mp);
>> +
>> + ret = meson_pcie_probe_clocks(mp);
>> + if (ret) {
>> + dev_err(dev, "Init clock resources failed, %d\n", ret);
>> + return ret;
>> + }
>> +
>> + platform_set_drvdata(pdev, mp);
>> +
>> + ret = meson_add_pcie_port(mp, pdev);
>> + if (ret < 0)
>> + dev_err(dev, "Add PCIE port failed, %d\n", ret);
>
> Isn't this fatal ?
>
> Thanks,
> Lorenzo
>

Yes, it's a fatal error we will fix it.

Thanks,
Hanjie

>> + return ret;
>> +}
>> +
>> +static const struct of_device_id meson_pcie_of_match[] = {
>> + {
>> + .compatible = "amlogic,axg-pcie",
>> + },
>> + {},
>> +};
>> +
>> +static struct platform_driver meson_pcie_driver = {
>> + .probe = meson_pcie_probe,
>> + .driver = {
>> + .name = "meson-pcie",
>> + .of_match_table = meson_pcie_of_match,
>> + },
>> +};
>> +
>> +builtin_platform_driver(meson_pcie_driver);
>> --
>> 2.7.4
>>
>
> .
>

2018-09-26 06:47:13

by Hanjie Lin

[permalink] [raw]
Subject: Re: [RESEND PATCH v3 2/2] PCI: meson: add the Amlogic Meson PCIe controller driver



On 2018/9/24 16:34, Gustavo Pimentel wrote:
> Hi Hanjie,
>
> On 21/09/2018 07:03, Hanjie Lin wrote:
>> From: Yue Wang <[email protected]>
>>
>> The Amlogic Meson PCIe host controller is based on the Synopsys DesignWare
>> PCI core. This patch adds the driver support for Meson PCIe controller.
>>
>> Signed-off-by: Yue Wang <[email protected]>
>> Signed-off-by: Hanjie Lin <[email protected]>
>> ---
>> drivers/pci/controller/dwc/Kconfig | 12 +
>> drivers/pci/controller/dwc/Makefile | 1 +
>> drivers/pci/controller/dwc/pci-meson.c | 617 +++++++++++++++++++++++++++++++++
>> 3 files changed, 630 insertions(+)
>> create mode 100644 drivers/pci/controller/dwc/pci-meson.c
>>
>> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
>> index 91b0194..6cb36f6 100644
>> --- a/drivers/pci/controller/dwc/Kconfig
>> +++ b/drivers/pci/controller/dwc/Kconfig
>> @@ -193,4 +193,16 @@ config PCIE_HISI_STB
>> help
>> Say Y here if you want PCIe controller support on HiSilicon STB SoCs
>>
>> +config PCI_MESON
>> + bool "MESON PCIe controller"
>> + depends on PCI
>> + depends on PCI_MSI_IRQ_DOMAIN
>
> I would suggest to compress the previous 2 line into in just one, like:
> depends on PCI && PCI_MSI_IRQ_DOMAIN
>
> Regards,
> Gustavo
>

Right, depends on PCI is unnecessary actually.
I will fix it.

Thanks,
Hanjie

>> + select PCIEPORTBUS
>> + select PCIE_DW_HOST
>> + help
>> + Say Y here if you want to enable PCI controller support on Amlogic
>> + SoCs. The PCI controller on Amlogic is based on DesignWare hardware
>> + and therefore the driver re-uses the DesignWare core functions to
>> + implement the driver.
>> +
>> endmenu
>> diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
>> index 5d2ce72..cf676bd 100644
>> --- a/drivers/pci/controller/dwc/Makefile
>> +++ b/drivers/pci/controller/dwc/Makefile
>> @@ -14,6 +14,7 @@ obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o
>> obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o
>> obj-$(CONFIG_PCIE_KIRIN) += pcie-kirin.o
>> obj-$(CONFIG_PCIE_HISI_STB) += pcie-histb.o
>> +obj-$(CONFIG_PCI_MESON) += pci-meson.o
>>
>> # The following drivers are for devices that use the generic ACPI
>> # pci_root.c driver but don't support standard ECAM config access.
>> diff --git a/drivers/pci/controller/dwc/pci-meson.c b/drivers/pci/controller/dwc/pci-meson.c
>> new file mode 100644
>> index 0000000..9c92a89
>> --- /dev/null
>> +++ b/drivers/pci/controller/dwc/pci-meson.c
>> @@ -0,0 +1,617 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * PCIe host controller driver for Amlogic MESON SoCs
>> + *
>> + * Copyright (c) 2018 Amlogic, inc.
>> + * Author: Yue Wang <[email protected]>
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/delay.h>
>> +#include <linux/of_device.h>
>> +#include <linux/of_gpio.h>
>> +#include <linux/pci.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/resource.h>
>> +#include <linux/types.h>
>> +#include <linux/reset.h>
>> +
>> +#include "pcie-designware.h"
>> +
>> +#define to_meson_pcie(x) dev_get_drvdata((x)->dev)
>> +
>> +/* External local bus interface registers */
>> +#define PLR_OFFSET 0x700
>> +#define PCIE_PORT_LINK_CTRL_OFF (PLR_OFFSET + 0x10)
>> +#define FAST_LINK_MODE BIT(7)
>> +#define LINK_CAPABLE_MASK GENMASK(21, 16)
>> +#define LINK_CAPABLE_X1 BIT(16)
>> +
>> +#define PCIE_GEN2_CTRL_OFF (PLR_OFFSET + 0x10c)
>> +#define NUM_OF_LANES_MASK GENMASK(12, 8)
>> +#define NUM_OF_LANES_X1 BIT(8)
>> +#define DIRECT_SPEED_CHANGE BIT(17)
>> +
>> +#define TYPE1_HDR_OFFSET 0x0
>> +#define PCIE_STATUS_COMMAND (TYPE1_HDR_OFFSET + 0x04)
>> +#define PCI_IO_EN BIT(0)
>> +#define PCI_MEM_SPACE_EN BIT(1)
>> +#define PCI_BUS_MASTER_EN BIT(2)
>> +
>> +#define PCIE_BASE_ADDR0 (TYPE1_HDR_OFFSET + 0x10)
>> +#define PCIE_BASE_ADDR1 (TYPE1_HDR_OFFSET + 0x14)
>> +
>> +#define PCIE_CAP_OFFSET 0x70
>> +#define PCIE_DEV_CTRL_DEV_STUS (PCIE_CAP_OFFSET + 0x08)
>> +#define PCIE_CAP_MAX_PAYLOAD_MASK GENMASK(7, 5)
>> +#define PCIE_CAP_MAX_PAYLOAD_SIZE(x) ((x) << 5)
>> +#define PCIE_CAP_MAX_READ_REQ_MASK GENMASK(14, 12)
>> +#define PCIE_CAP_MAX_READ_REQ_SIZE(x) ((x) << 12)
>> +
>> +#define PCI_CLASS_REVISION_MASK GENMASK(7, 0)
>> +
>> +/* PCIe specific config registers */
>> +#define PCIE_CFG0 0x0
>> +#define APP_LTSSM_ENABLE BIT(7)
>> +
>> +#define PCIE_CFG_STATUS12 0x30
>> +#define IS_SMLH_LINK_UP(x) ((x) & (1 << 6))
>> +#define IS_RDLH_LINK_UP(x) ((x) & (1 << 16))
>> +#define IS_LTSSM_UP(x) ((((x) >> 10) & 0x1f) == 0x11)
>> +
>> +#define PCIE_CFG_STATUS17 0x44
>> +#define PM_CURRENT_STATE(x) (((x) >> 7) & 0x1)
>> +
>> +#define WAIT_LINKUP_TIMEOUT 2000
>> +#define PORT_CLK_RATE 100000000UL
>> +#define MAX_PAYLOAD_SIZE 256
>> +#define MAX_READ_REQ_SIZE 256
>> +#define MESON_PCIE_PHY_POWERUP 0x1c
>> +#define PCIE_RESET_DELAY 500
>> +#define PCIE_SHARED_RESET 1
>> +#define PCIE_NORMAL_RESET 0
>> +
>> +enum pcie_data_rate {
>> + PCIE_GEN1,
>> + PCIE_GEN2,
>> + PCIE_GEN3,
>> + PCIE_GEN4
>> +};
>> +
>> +struct meson_pcie_mem_res {
>> + void __iomem *elbi_base; /* DT 0th resource */
>> + void __iomem *cfg_base; /* DT 1nd resource */
>> + void __iomem *phy_base; /* DT 2nd resource */
>> +};
>> +
>> +struct meson_pcie_clk_res {
>> + struct clk *clk;
>> + struct clk *mipi_gate;
>> + struct clk *port_clk;
>> + struct clk *general_clk;
>> +};
>> +
>> +struct meson_pcie_rc_reset {
>> + struct reset_control *phy;
>> + struct reset_control *port;
>> + struct reset_control *apb;
>> +};
>> +
>> +struct meson_pcie {
>> + struct dw_pcie pci;
>> + struct meson_pcie_mem_res mem_res;
>> + struct meson_pcie_clk_res clk_res;
>> + struct meson_pcie_rc_reset mrst;
>> + struct gpio_desc *reset_gpio;
>> +
>> + enum of_gpio_flags gpio_flag;
>> + int pcie_num;
>> + u32 port_num;
>> + u32 device_attch;
>> +};
>> +
>> +static struct reset_control *meson_pcie_get_reset(struct meson_pcie *mp,
>> + const char *id,
>> + u32 reset_type)
>> +{
>> + struct device *dev = mp->pci.dev;
>> + struct reset_control *reset;
>> +
>> + if (reset_type == PCIE_SHARED_RESET)
>> + reset = devm_reset_control_get_shared(dev, id);
>> + else
>> + reset = devm_reset_control_get(dev, id);
>> +
>> + return reset;
>> +}
>> +
>> +static int meson_pcie_get_resets(struct meson_pcie *mp)
>> +{
>> + struct meson_pcie_rc_reset *mrst = &mp->mrst;
>> +
>> + mrst->phy = meson_pcie_get_reset(mp, "phy", PCIE_SHARED_RESET);
>> + if (IS_ERR(mrst->phy))
>> + return PTR_ERR(mrst->phy);
>> + reset_control_deassert(mrst->phy);
>> +
>> + mrst->port = meson_pcie_get_reset(mp, "port", PCIE_NORMAL_RESET);
>> + if (IS_ERR(mrst->port))
>> + return PTR_ERR(mrst->port);
>> + reset_control_deassert(mrst->port);
>> +
>> + mrst->apb = meson_pcie_get_reset(mp, "apb", PCIE_SHARED_RESET);
>> + if (IS_ERR(mrst->apb))
>> + return PTR_ERR(mrst->apb);
>> + reset_control_deassert(mrst->apb);
>> +
>> + return 0;
>> +}
>> +
>> +static void __iomem *meson_pcie_get_mem_shared(struct platform_device *pdev,
>> + struct meson_pcie *mp,
>> + const char *id)
>> +{
>> + struct resource *res;
>> +
>> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, id);
>> +
>> + return ioremap(res->start, resource_size(res));
>> +}
>> +
>> +static void __iomem *meson_pcie_get_mem(struct platform_device *pdev,
>> + struct meson_pcie *mp,
>> + const char *id)
>> +{
>> + struct device *dev = mp->pci.dev;
>> + struct resource *res;
>> +
>> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, id);
>> +
>> + return devm_ioremap_resource(dev, res);
>> +}
>> +
>> +static int meson_pcie_get_mems(struct platform_device *pdev,
>> + struct meson_pcie *mp)
>> +{
>> + mp->mem_res.elbi_base = meson_pcie_get_mem(pdev, mp, "elbi");
>> + if (IS_ERR(mp->mem_res.elbi_base))
>> + return PTR_ERR(mp->mem_res.elbi_base);
>> +
>> + mp->mem_res.cfg_base = meson_pcie_get_mem(pdev, mp, "cfg");
>> + if (IS_ERR(mp->mem_res.cfg_base))
>> + return PTR_ERR(mp->mem_res.cfg_base);
>> +
>> + mp->mem_res.phy_base = meson_pcie_get_mem_shared(pdev, mp, "phy");
>> + if (IS_ERR(mp->mem_res.phy_base))
>> + return PTR_ERR(mp->mem_res.phy_base);
>> +
>> + return 0;
>> +}
>> +
>> +static void meson_pcie_power_on(struct meson_pcie *mp)
>> +{
>> + writel(MESON_PCIE_PHY_POWERUP, mp->mem_res.phy_base);
>> +}
>> +
>> +static void meson_pcie_reset(struct meson_pcie *mp)
>> +{
>> + struct meson_pcie_rc_reset *mrst = &mp->mrst;
>> +
>> + reset_control_assert(mrst->phy);
>> + udelay(PCIE_RESET_DELAY);
>> + reset_control_deassert(mrst->phy);
>> + udelay(PCIE_RESET_DELAY);
>> +
>> + reset_control_assert(mrst->port);
>> + reset_control_assert(mrst->apb);
>> + udelay(PCIE_RESET_DELAY);
>> + reset_control_deassert(mrst->port);
>> + reset_control_deassert(mrst->apb);
>> + udelay(PCIE_RESET_DELAY);
>> +}
>> +
>> +static inline struct clk *meson_pcie_probe_clock(struct device *dev,
>> + const char *id, u64 rate)
>> +{
>> + struct clk *clk = NULL;
>> + int ret;
>> +
>> + clk = devm_clk_get(dev, id);
>> + if (IS_ERR(clk))
>> + return clk;
>> +
>> + if (rate) {
>> + ret = clk_set_rate(clk, rate);
>> + if (ret) {
>> + dev_err(dev, "set clk rate failed, ret = %d\n", ret);
>> + return ERR_PTR(ret);
>> + }
>> + }
>> +
>> + ret = clk_prepare_enable(clk);
>> + if (ret) {
>> + dev_err(dev, "couldn't enable clk\n");
>> + return ERR_PTR(ret);
>> + }
>> +
>> + devm_add_action_or_reset(dev,
>> + (void (*) (void *))clk_disable_unprepare,
>> + clk);
>> +
>> + return clk;
>> +}
>> +
>> +static int meson_pcie_probe_clocks(struct meson_pcie *mp)
>> +{
>> + struct device *dev = mp->pci.dev;
>> + struct meson_pcie_clk_res *res = &mp->clk_res;
>> +
>> + res->port_clk = meson_pcie_probe_clock(dev, "port", PORT_CLK_RATE);
>> + if (IS_ERR(res->port_clk))
>> + return PTR_ERR(res->port_clk);
>> +
>> + res->mipi_gate = meson_pcie_probe_clock(dev, "pcie_mipi_en", 0);
>> + if (IS_ERR(res->mipi_gate))
>> + return PTR_ERR(res->mipi_gate);
>> +
>> + res->general_clk = meson_pcie_probe_clock(dev, "pcie_general", 0);
>> + if (IS_ERR(res->general_clk))
>> + return PTR_ERR(res->general_clk);
>> +
>> + res->clk = meson_pcie_probe_clock(dev, "pcie", 0);
>> + if (IS_ERR(res->clk))
>> + return PTR_ERR(res->clk);
>> +
>> + return 0;
>> +}
>> +
>> +static inline void meson_elb_writel(struct meson_pcie *mp, u32 val, u32 reg)
>> +{
>> + writel(val, mp->mem_res.elbi_base + reg);
>> +}
>> +
>> +static inline u32 meson_elb_readl(struct meson_pcie *mp, u32 reg)
>> +{
>> + return readl(mp->mem_res.elbi_base + reg);
>> +}
>> +
>> +static inline u32 meson_cfg_readl(struct meson_pcie *mp, u32 reg)
>> +{
>> + return readl(mp->mem_res.cfg_base + reg);
>> +}
>> +
>> +static inline void meson_cfg_writel(struct meson_pcie *mp, u32 val, u32 reg)
>> +{
>> + writel(val, mp->mem_res.cfg_base + reg);
>> +}
>> +
>> +static void meson_pcie_assert_reset(struct meson_pcie *mp)
>> +{
>> + gpiod_set_value_cansleep(mp->reset_gpio, 0);
>> + udelay(500);
>> + gpiod_set_value_cansleep(mp->reset_gpio, 1);
>> +}
>> +
>> +static void meson_pcie_init_dw(struct meson_pcie *mp)
>> +{
>> + u32 val = 0;
>> +
>> + val = meson_cfg_readl(mp, PCIE_CFG0);
>> + val |= APP_LTSSM_ENABLE;
>> + meson_cfg_writel(mp, val, PCIE_CFG0);
>> +
>> + val = meson_elb_readl(mp, PCIE_PORT_LINK_CTRL_OFF);
>> + val &= ~LINK_CAPABLE_MASK;
>> + meson_elb_writel(mp, val, PCIE_PORT_LINK_CTRL_OFF);
>> +
>> + val = meson_elb_readl(mp, PCIE_PORT_LINK_CTRL_OFF);
>> + val |= LINK_CAPABLE_X1 | FAST_LINK_MODE;
>> + meson_elb_writel(mp, val, PCIE_PORT_LINK_CTRL_OFF);
>> +
>> + val = meson_elb_readl(mp, PCIE_GEN2_CTRL_OFF);
>> + val &= ~NUM_OF_LANES_MASK;
>> + meson_elb_writel(mp, val, PCIE_GEN2_CTRL_OFF);
>> +
>> + val = meson_elb_readl(mp, PCIE_GEN2_CTRL_OFF);
>> + val |= NUM_OF_LANES_X1 | DIRECT_SPEED_CHANGE;
>> + meson_elb_writel(mp, val, PCIE_GEN2_CTRL_OFF);
>> +
>> + meson_elb_writel(mp, 0x0, PCIE_BASE_ADDR0);
>> + meson_elb_writel(mp, 0x0, PCIE_BASE_ADDR1);
>> +}
>> +
>> +static int meson_size_to_payload(int size)
>> +{
>> + if (size & (size - 1) || size < 128 || size > 4096)
>> + return 1;
>> +
>> + return fls(size) - 8;
>> +}
>> +
>> +static void meson_set_max_payload(struct meson_pcie *mp, int size)
>> +{
>> + u32 val = 0;
>> + int max_payload_size = meson_size_to_payload(size);
>> +
>> + val = meson_elb_readl(mp, PCIE_DEV_CTRL_DEV_STUS);
>> + val &= ~PCIE_CAP_MAX_PAYLOAD_MASK;
>> + meson_elb_writel(mp, val, PCIE_DEV_CTRL_DEV_STUS);
>> +
>> + val = meson_elb_readl(mp, PCIE_DEV_CTRL_DEV_STUS);
>> + val |= PCIE_CAP_MAX_PAYLOAD_SIZE(max_payload_size);
>> + meson_elb_writel(mp, val, PCIE_DEV_CTRL_DEV_STUS);
>> +}
>> +
>> +static void meson_set_max_rd_req_size(struct meson_pcie *mp, int size)
>> +{
>> + u32 val = 0;
>> + int max_rd_req_size = meson_size_to_payload(size);
>> +
>> + val = meson_elb_readl(mp, PCIE_DEV_CTRL_DEV_STUS);
>> + val &= ~PCIE_CAP_MAX_READ_REQ_MASK;
>> + meson_elb_writel(mp, val, PCIE_DEV_CTRL_DEV_STUS);
>> +
>> + val = meson_elb_readl(mp, PCIE_DEV_CTRL_DEV_STUS);
>> + val |= PCIE_CAP_MAX_READ_REQ_SIZE(max_rd_req_size);
>> + meson_elb_writel(mp, val, PCIE_DEV_CTRL_DEV_STUS);
>> +}
>> +
>> +static inline void meson_enable_memory_space(struct meson_pcie *mp)
>> +{
>> + /* Set the RC Bus Master, Memory Space and I/O Space enables */
>> + meson_elb_writel(mp, PCI_IO_EN | PCI_MEM_SPACE_EN | PCI_BUS_MASTER_EN,
>> + PCIE_STATUS_COMMAND);
>> +}
>> +
>> +static int meson_pcie_establish_link(struct meson_pcie *mp)
>> +{
>> + struct dw_pcie *pci = &mp->pci;
>> + struct pcie_port *pp = &pci->pp;
>> +
>> + meson_pcie_init_dw(mp);
>> + meson_set_max_payload(mp, MAX_PAYLOAD_SIZE);
>> + meson_set_max_rd_req_size(mp, MAX_READ_REQ_SIZE);
>> +
>> + dw_pcie_setup_rc(pp);
>> + meson_enable_memory_space(mp);
>> +
>> + meson_pcie_assert_reset(mp);
>> +
>> + /* check if the link is up or not */
>> + if (!dw_pcie_wait_for_link(pci))
>> + return 0;
>> +
>> + return -ETIMEDOUT;
>> +}
>> +
>> +static void meson_pcie_msi_init(struct meson_pcie *mp)
>> +{
>> + struct pcie_port *pp = &mp->pci.pp;
>> +
>> + dw_pcie_msi_init(pp);
>> +}
>> +
>> +static void meson_pcie_enable_interrupts(struct meson_pcie *mp)
>> +{
>> + if (IS_ENABLED(CONFIG_PCI_MSI))
>> + meson_pcie_msi_init(mp);
>> +}
>> +
>> +static u32 meson_pcie_read_dbi(struct dw_pcie *pci, void __iomem *base,
>> + u32 reg, size_t size)
>> +{
>> + u32 val;
>> +
>> + dw_pcie_read(base + reg, size, &val);
>> +
>> + return val;
>> +}
>> +
>> +static void meson_pcie_write_dbi(struct dw_pcie *pci, void __iomem *base,
>> + u32 reg, size_t size, u32 val)
>> +{
>> + dw_pcie_write(base + reg, size, val);
>> +}
>> +
>> +static int meson_pcie_rd_own_conf(struct pcie_port *pp, int where, int size,
>> + u32 *val)
>> +{
>> + struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>> + struct meson_pcie *mp = to_meson_pcie(pci);
>> +
>> + if (!mp->device_attch)
>> + return 0;
>> +
>> + /* the device class is not reported correctly from the register */
>> + if (where == PCI_CLASS_REVISION) {
>> + *val = readl(pci->dbi_base + PCI_CLASS_REVISION);
>> + /* keep revision id */
>> + *val &= PCI_CLASS_REVISION_MASK;
>> + *val |= PCI_CLASS_BRIDGE_PCI << 16;
>> + return PCIBIOS_SUCCESSFUL;
>> + }
>> +
>> + return dw_pcie_read(pci->dbi_base + where, size, val);
>> +}
>> +
>> +static int meson_pcie_wr_own_conf(struct pcie_port *pp, int where,
>> + int size, u32 val)
>> +{
>> + struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>> + struct meson_pcie *mp = to_meson_pcie(pci);
>> +
>> + if (!mp->device_attch)
>> + return 0;
>> +
>> + return dw_pcie_write(pci->dbi_base + where, size, val);
>> +}
>> +
>> +static int meson_pcie_link_up(struct dw_pcie *pci)
>> +{
>> + struct meson_pcie *mp = to_meson_pcie(pci);
>> + struct device *dev = pci->dev;
>> + u32 smlh_up = 0;
>> + u32 ltssm_up = 0;
>> + u32 rdlh_up = 0;
>> + u32 speed_okay = 0;
>> + u32 cnt = 0;
>> + u32 state12, state17;
>> +
>> + while (smlh_up == 0 || rdlh_up == 0 || ltssm_up == 0 ||
>> + speed_okay == 0) {
>> + udelay(20);
>> +
>> + state12 = meson_cfg_readl(mp, PCIE_CFG_STATUS12);
>> + state17 = meson_cfg_readl(mp, PCIE_CFG_STATUS17);
>> + smlh_up = IS_SMLH_LINK_UP(state12);
>> + rdlh_up = IS_RDLH_LINK_UP(state12);
>> + ltssm_up = IS_LTSSM_UP(state12);
>> +
>> + if (PM_CURRENT_STATE(state17) < PCIE_GEN3)
>> + speed_okay = 1;
>> +
>> + if (smlh_up)
>> + dev_dbg(dev, "smlh_link_up is on\n");
>> + if (rdlh_up)
>> + dev_dbg(dev, "rdlh_link_up is on\n");
>> + if (ltssm_up)
>> + dev_dbg(dev, "ltssm_up is on\n");
>> + if (speed_okay)
>> + dev_dbg(dev, "speed_okay\n");
>> +
>> + cnt++;
>> +
>> + if (cnt >= WAIT_LINKUP_TIMEOUT) {
>> + dev_err(dev, "Error: Wait linkup timeout.\n");
>> + return 0;
>> + }
>> + }
>> +
>> + return 1;
>> +}
>> +
>> +static int meson_pcie_host_init(struct pcie_port *pp)
>> +{
>> + struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>> + struct meson_pcie *mp = to_meson_pcie(pci);
>> + int ret;
>> +
>> + ret = meson_pcie_establish_link(mp);
>> +
>> + if (ret)
>> + return ret;
>> +
>> + mp->device_attch = 1;
>> + meson_pcie_enable_interrupts(mp);
>> +
>> + return 0;
>> +}
>> +
>> +static const struct dw_pcie_host_ops meson_pcie_host_ops = {
>> + .rd_own_conf = meson_pcie_rd_own_conf,
>> + .wr_own_conf = meson_pcie_wr_own_conf,
>> + .host_init = meson_pcie_host_init,
>> +};
>> +
>> +static int meson_add_pcie_port(struct meson_pcie *mp,
>> + struct platform_device *pdev)
>> +{
>> + struct dw_pcie *pci = &mp->pci;
>> + struct pcie_port *pp = &pci->pp;
>> + struct device *dev = &pdev->dev;
>> + int ret;
>> +
>> + if (IS_ENABLED(CONFIG_PCI_MSI)) {
>> + pp->msi_irq = platform_get_irq(pdev, 0);
>> + if (pp->msi_irq < 0) {
>> + dev_err(dev, "failed to get msi irq\n");
>> + return pp->msi_irq;
>> + }
>> + }
>> +
>> + pp->root_bus_nr = -1;
>> + pp->ops = &meson_pcie_host_ops;
>> + pci->dbi_base = mp->mem_res.elbi_base;
>> +
>> + ret = dw_pcie_host_init(pp);
>> + if (ret) {
>> + dev_err(dev, "failed to initialize host\n");
>> + return ret;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static const struct dw_pcie_ops dw_pcie_ops = {
>> + .read_dbi = meson_pcie_read_dbi,
>> + .write_dbi = meson_pcie_write_dbi,
>> + .link_up = meson_pcie_link_up,
>> +};
>> +
>> +static int meson_pcie_probe(struct platform_device *pdev)
>> +{
>> + struct device *dev = &pdev->dev;
>> + struct dw_pcie *pci;
>> + struct meson_pcie *mp;
>> + int ret;
>> +
>> + mp = devm_kzalloc(dev, sizeof(*mp), GFP_KERNEL);
>> + if (!mp)
>> + return -ENOMEM;
>> +
>> + pci = &mp->pci;
>> + pci->dev = dev;
>> + pci->ops = &dw_pcie_ops;
>> +
>> + mp->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
>> + if (IS_ERR(mp->reset_gpio)) {
>> + dev_err(dev, "Get reset gpio failed\n");
>> + return PTR_ERR(mp->reset_gpio);
>> + }
>> +
>> + ret = meson_pcie_get_resets(mp);
>> + if (ret) {
>> + dev_err(dev, "Get reset resource failed, %d\n", ret);
>> + return ret;
>> + }
>> +
>> + ret = meson_pcie_get_mems(pdev, mp);
>> + if (ret) {
>> + dev_err(dev, "Get memory resource failed, %d\n", ret);
>> + return ret;
>> + }
>> +
>> + meson_pcie_power_on(mp);
>> + meson_pcie_reset(mp);
>> +
>> + ret = meson_pcie_probe_clocks(mp);
>> + if (ret) {
>> + dev_err(dev, "Init clock resources failed, %d\n", ret);
>> + return ret;
>> + }
>> +
>> + platform_set_drvdata(pdev, mp);
>> +
>> + ret = meson_add_pcie_port(mp, pdev);
>> + if (ret < 0)
>> + dev_err(dev, "Add PCIE port failed, %d\n", ret);
>> +
>> + return ret;
>> +}
>> +
>> +static const struct of_device_id meson_pcie_of_match[] = {
>> + {
>> + .compatible = "amlogic,axg-pcie",
>> + },
>> + {},
>> +};
>> +
>> +static struct platform_driver meson_pcie_driver = {
>> + .probe = meson_pcie_probe,
>> + .driver = {
>> + .name = "meson-pcie",
>> + .of_match_table = meson_pcie_of_match,
>> + },
>> +};
>> +
>> +builtin_platform_driver(meson_pcie_driver);
>>
>
> .
>