[PATCH v8 0/2 RESEND] add the Amlogic Meson PCIe controller driver

[PATCH v8 0/2 RESEND] add the Amlogic Meson PCIe controller driver

[Hanjie Lin]

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.

Changes since v7: [6]
 - include files in alphabetical order
 - get rid of unused MACROs and variables
 - optimize meson_pcie_link_up() while loop
 - correct cover-letter version

Changes since v6: [5]
 - fix bad usage of ERR_PTR(ENXIO)
 - fix meson_pcie_rd_own_conf() when read PCI_CLASS_DEVICE reg 

Changes since v5: [4]
 - update MAINTAINER file in alphabetical order
 - remove meaningless comment
 - use ERR_PTR function instead of (void *) cast
 - use is_power_of_2(size) instead of size & (size - 1)
 - add comment for PCI_CLASS_REVISION register operation
 
Changes since v4: [3]
 - fix kbuild test robot and compile warnings

Changes since v3: [2]
 - modify subject format
 - update Kconfig
 - update MAINTAINER file
 - add comment and error handle for meson_pcie_get_mem_shared()
 - drop useless initialization code
 - add comment for meson_size_to_payload()
 - optimize meson_pcie_establish_link() return code
 - optimize meson_pcie_enable_interrupts() redundant function
 - drop device_attch related code
 - drop dw_pcie_ops read_dbi and write_dbi function
 - add error handle for meson_add_pcie_port() when probe

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/1534227522-186798-1-git-send-email-hanjie.lin@amlogic.com
[1] : https://lkml.kernel.org/r/1535096165-45827-1-git-send-email-hanjie.lin@amlogic.com
[2] : https://lkml.kernel.org/r/1537509820-52040-1-git-send-email-hanjie.lin@amlogic.com 
[3] : https://lkml.kernel.org/r/1538999834-156423-3-git-send-email-hanjie.lin@amlogic.com
[4] : https://lkml.kernel.org/r/1539049990-30810-1-git-send-email-hanjie.lin@amlogic.com
[5] : https://lkml.kernel.org/r/1542876836-191355-1-git-send-email-hanjie.lin@amlogic.com
[6] : https://lkml.kernel.org/r/1544097760-85834-1-git-send-email-hanjie.lin@amlogic.com

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

 .../devicetree/bindings/pci/amlogic,meson-pcie.txt |  70 +++
 MAINTAINERS                                        |   7 +
 drivers/pci/controller/dwc/Kconfig                 |  10 +
 drivers/pci/controller/dwc/Makefile                |   1 +
 drivers/pci/controller/dwc/pci-meson.c             | 595 +++++++++++++++++++++
 5 files changed, 683 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


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[PATCH v8 1/2] dt-bindings: PCI: meson: add DT bindings for Amlogic Meson PCIe controller

[Hanjie Lin]

From: Yue Wang <yue.wang@amlogic.com>

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 <yue.wang@amlogic.com>
Signed-off-by: Hanjie Lin <hanjie.lin@amlogic.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../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


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[PATCH v8 2/2] PCI: amlogic: Add the Amlogic Meson PCIe controller driver

[Hanjie Lin]

From: Yue Wang <yue.wang@amlogic.com>

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 <yue.wang@amlogic.com>
Signed-off-by: Hanjie Lin <hanjie.lin@amlogic.com>
---
 MAINTAINERS                            |   7 +
 drivers/pci/controller/dwc/Kconfig     |  10 +
 drivers/pci/controller/dwc/Makefile    |   1 +
 drivers/pci/controller/dwc/pci-meson.c | 595 +++++++++++++++++++++++++++++++++
 4 files changed, 613 insertions(+)
 create mode 100644 drivers/pci/controller/dwc/pci-meson.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 7fe120f..21ed916 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11600,6 +11600,13 @@ T:	git git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/pci.git/
 S:	Supported
 F:	drivers/pci/controller/
 
+PCIE DRIVER FOR AMLOGIC MESON
+M:	Yue Wang <yue.wang@Amlogic.com>
+L:	linux-pci@vger.kernel.org
+L:	linux-amlogic@lists.infradead.org
+S:	Maintained
+F:	drivers/pci/controller/dwc/pci-meson.c
+
 PCIE DRIVER FOR AXIS ARTPEC
 M:	Jesper Nilsson <jesper.nilsson@axis.com>
 L:	linux-arm-kernel@axis.com
diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
index 91b0194..7800322 100644
--- a/drivers/pci/controller/dwc/Kconfig
+++ b/drivers/pci/controller/dwc/Kconfig
@@ -193,4 +193,14 @@ 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_MSI_IRQ_DOMAIN
+	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 fcf91ea..e05a015 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..7993f9d
--- /dev/null
+++ b/drivers/pci/controller/dwc/pci-meson.c
@@ -0,0 +1,595 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * PCIe host controller driver for Amlogic MESON SoCs
+ *
+ * Copyright (c) 2018 Amlogic, inc.
+ * Author: Yue Wang <yue.wang@amlogic.com>
+ */
+
+#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/reset.h>
+#include <linux/resource.h>
+#include <linux/types.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)
+
+/* 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		4000
+#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;
+	void __iomem *cfg_base;
+	void __iomem *phy_base;
+};
+
+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;
+};
+
+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(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 void __iomem *meson_pcie_get_mem_shared(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);
+	if (!res) {
+		dev_err(dev, "No REG resource %s\n", id);
+		return ERR_PTR(-ENXIO);
+	}
+
+	return devm_ioremap(dev, res->start, resource_size(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);
+
+	/* Meson SoC has two PCI controllers use same phy register*/
+	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;
+	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;
+
+	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(struct meson_pcie *mp, int size)
+{
+	struct device *dev = mp->pci.dev;
+
+	/*
+	 * dwc supports 2^(val+7) payload size, which val is 0~5 default to 1.
+	 * So if input size is not 2^order alignment or less than 2^7 or bigger
+	 * than 2^12, just set to default size 2^(1+7).
+	 */
+	if (!is_power_of_2(size) || size < 128 || size > 4096) {
+		dev_warn(dev, "playload size %d, set to default 256\n", size);
+		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(mp, 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;
+	int max_rd_req_size = meson_size_to_payload(mp, 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);
+
+	return dw_pcie_wait_for_link(pci);
+}
+
+static void meson_pcie_enable_interrupts(struct meson_pcie *mp)
+{
+	if (IS_ENABLED(CONFIG_PCI_MSI))
+		dw_pcie_msi_init(&mp->pci.pp);
+}
+
+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);
+	int ret;
+
+	ret = dw_pcie_read(pci->dbi_base + where, size, val);
+	if (ret != PCIBIOS_SUCCESSFUL)
+		return ret;
+
+	/*
+	 * There is a bug in the MESON AXG pcie controller whereby software
+	 * cannot programme the PCI_CLASS_DEVICE register, so we must fabricate
+	 * the return value in the config accessors.
+	 */
+	if (where == PCI_CLASS_REVISION && size == 4)
+		*val = (PCI_CLASS_BRIDGE_PCI << 16) | (*val & 0xffff);
+	else if (where == PCI_CLASS_DEVICE && size == 2)
+		*val = PCI_CLASS_BRIDGE_PCI;
+	else if (where == PCI_CLASS_DEVICE && size == 1)
+		*val = PCI_CLASS_BRIDGE_PCI & 0xff;
+	else if (where == PCI_CLASS_DEVICE + 1 && size == 1)
+		*val = (PCI_CLASS_BRIDGE_PCI >> 8) & 0xff;
+
+	return PCIBIOS_SUCCESSFUL;
+}
+
+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);
+
+	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;
+
+	do {
+		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");
+
+		if (smlh_up && rdlh_up && ltssm_up && speed_okay)
+			return 1;
+
+		cnt++;
+
+		udelay(10);
+	} while (cnt < WAIT_LINKUP_TIMEOUT);
+
+	dev_err(dev, "Error: Wait linkup timeout.\n");
+	return 0;
+}
+
+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;
+
+	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->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 = {
+	.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;
+	}
+
+	return 0;
+}
+
+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


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH v8 2/2] PCI: amlogic: Add the Amlogic Meson PCIe controller driver

[Bjorn Helgaas]

On Tue, Dec 18, 2018 at 04:04:46PM +0800, Hanjie Lin wrote:
> From: Yue Wang <yue.wang@amlogic.com>
> 
> The Amlogic Meson PCIe host controller is based on the Synopsys DesignWare
> PCI core. This patch adds the driver support for Meson PCIe controller.

I don't have any comments on the code itself; just the trivial things
below.  No need to repost for these unless you're changing something
else.

I thought it looked very pretty overall, thanks for paying attention
to that!

> +static int meson_size_to_payload(struct meson_pcie *mp, int size)
> +{
> +	struct device *dev = mp->pci.dev;
> +
> +	/*
> +	 * dwc supports 2^(val+7) payload size, which val is 0~5 default to 1.
> +	 * So if input size is not 2^order alignment or less than 2^7 or bigger
> +	 * than 2^12, just set to default size 2^(1+7).
> +	 */
> +	if (!is_power_of_2(size) || size < 128 || size > 4096) {
> +		dev_warn(dev, "playload size %d, set to default 256\n", size);

s/playload/payload/

> +static void meson_set_max_payload(struct meson_pcie *mp, int size)
> +{
> +	u32 val = 0;

Unnecessary initialization.

> +	int max_payload_size = meson_size_to_payload(mp, size);
> +
> +	val = meson_elb_readl(mp, PCIE_DEV_CTRL_DEV_STUS);

> +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);
> +	int ret;
> +
> +	ret = dw_pcie_read(pci->dbi_base + where, size, val);
> +	if (ret != PCIBIOS_SUCCESSFUL)
> +		return ret;
> +
> +	/*
> +	 * There is a bug in the MESON AXG pcie controller whereby software
> +	 * cannot programme the PCI_CLASS_DEVICE register, so we must fabricate
> +	 * the return value in the config accessors.

s/pcie/PCIe/
s/programme/program/ (IIUC, "programme" is British and only used as a
noun, where here you need a verb)

> +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;

Unnecessary initialization of smlh_up, ltssm_up, and rdlh_up.

> +	u32 speed_okay = 0;
> +	u32 cnt = 0;
> +	u32 state12, state17;
> +
> +	do {
> +		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);

> +	dev_err(dev, "Error: Wait linkup timeout.\n");

Message doesn't match others from driver (capitalization and trailing
period).

> +			dev_err(dev, "failed to get msi irq\n");

s/msi irq/MSI IRQ/

> +	ret = meson_add_pcie_port(mp, pdev);
> +	if (ret < 0) {
> +		dev_err(dev, "Add PCIE port failed, %d\n", ret);

s/PCIE/PCIe/

All the messages in this function are capitalized differently than
other messages in the driver.

Bjorn

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH v8 1/2] dt-bindings: PCI: meson: add DT bindings for Amlogic Meson PCIe controller

[Martin Blumenstingl]

Hi Rob, Hi Hanjie,

(sorry for being late with my question)

On Tue, Dec 18, 2018 at 9:05 AM Hanjie Lin <hanjie.lin@amlogic.com> wrote:
[...]
> +- reg-names: Must be
> +       - "elbi"        External local bus interface registers
> +       - "cfg"         Meson specific registers
> +       - "phy"         Meson PCIE PHY registers
I have learned that there are two PHY register designs:
- AXG only has a PCIe PHY
- G12A has a PHY which supports PCIe and USB 3.0. The PCIe part of
this PHY design is compatible with AXG, but this design also supports
a USB 3.0 port (it's an exclusive choice: either PCIe *or* USB 3.0)

The PCIe controller itself is identical on both, AXG and G12A.
This patch adds support for the AXG PCIe controller and PHY within one
device-tree node.

For G12A I propose to add a separate "phys" property with a phandle to
the "combo" PCIe and USB3.0 PHY - this can be part of a separate patch
though.
I would like to know whether it's OK that for AXG the PCIe PHY is
described in the same device-tree node as the PCIe controller (in
other words: we're not using a "phys" property here)?


Kind Regards
Martin

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH v8 1/2] dt-bindings: PCI: meson: add DT bindings for Amlogic Meson PCIe controller

[Hanjie Lin]

On 2018/12/19 7:14, Martin Blumenstingl wrote:
> Hi Rob, Hi Hanjie,
> 
> (sorry for being late with my question)
> 
> On Tue, Dec 18, 2018 at 9:05 AM Hanjie Lin <hanjie.lin@amlogic.com> wrote:
> [...]
>> +- reg-names: Must be
>> +       - "elbi"        External local bus interface registers
>> +       - "cfg"         Meson specific registers
>> +       - "phy"         Meson PCIE PHY registers
> I have learned that there are two PHY register designs:
> - AXG only has a PCIe PHY
> - G12A has a PHY which supports PCIe and USB 3.0. The PCIe part of
> this PHY design is compatible with AXG, but this design also supports
> a USB 3.0 port (it's an exclusive choice: either PCIe *or* USB 3.0)
> 
> The PCIe controller itself is identical on both, AXG and G12A.
> This patch adds support for the AXG PCIe controller and PHY within one
> device-tree node.
> 
> For G12A I propose to add a separate "phys" property with a phandle to
> the "combo" PCIe and USB3.0 PHY - this can be part of a separate patch
> though.
> I would like to know whether it's OK that for AXG the PCIe PHY is
> described in the same device-tree node as the PCIe controller (in
> other words: we're not using a "phys" property here)?
> 
> 
> Kind Regards
> Martin
> 
> .
> 

hi matrin,

We do had a dedicated PHY driver for a time at the begining
of this patch series, but we decided to remove it and integrate
into the controller driver after series reviews and disscussions,
and the main reason is it's too overkill to have a dedicated 
PHY driver which only do the RESET job.

Of course we can consider the dedicated PHY driver for G12A upstream
in future.

thanks
hanjie


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH v8 2/2] PCI: amlogic: Add the Amlogic Meson PCIe controller driver

[Hanjie Lin]

On 2018/12/19 6:47, Bjorn Helgaas wrote:
> On Tue, Dec 18, 2018 at 04:04:46PM +0800, Hanjie Lin wrote:
>> From: Yue Wang <yue.wang@amlogic.com>
>>
>> The Amlogic Meson PCIe host controller is based on the Synopsys DesignWare
>> PCI core. This patch adds the driver support for Meson PCIe controller.
> 
> I don't have any comments on the code itself; just the trivial things
> below.  No need to repost for these unless you're changing something
> else.
> 
> I thought it looked very pretty overall, thanks for paying attention
> to that!
> 
>> +static int meson_size_to_payload(struct meson_pcie *mp, int size)
>> +{
>> +	struct device *dev = mp->pci.dev;
>> +
>> +	/*
>> +	 * dwc supports 2^(val+7) payload size, which val is 0~5 default to 1.
>> +	 * So if input size is not 2^order alignment or less than 2^7 or bigger
>> +	 * than 2^12, just set to default size 2^(1+7).
>> +	 */
>> +	if (!is_power_of_2(size) || size < 128 || size > 4096) {
>> +		dev_warn(dev, "playload size %d, set to default 256\n", size);
> 
> s/playload/payload/
> 
>> +static void meson_set_max_payload(struct meson_pcie *mp, int size)
>> +{
>> +	u32 val = 0;
> 
> Unnecessary initialization.
> 
>> +	int max_payload_size = meson_size_to_payload(mp, size);
>> +
>> +	val = meson_elb_readl(mp, PCIE_DEV_CTRL_DEV_STUS);
> 
>> +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);
>> +	int ret;
>> +
>> +	ret = dw_pcie_read(pci->dbi_base + where, size, val);
>> +	if (ret != PCIBIOS_SUCCESSFUL)
>> +		return ret;
>> +
>> +	/*
>> +	 * There is a bug in the MESON AXG pcie controller whereby software
>> +	 * cannot programme the PCI_CLASS_DEVICE register, so we must fabricate
>> +	 * the return value in the config accessors.
> 
> s/pcie/PCIe/
> s/programme/program/ (IIUC, "programme" is British and only used as a
> noun, where here you need a verb)
> 
>> +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;
> 
> Unnecessary initialization of smlh_up, ltssm_up, and rdlh_up.
> 
>> +	u32 speed_okay = 0;
>> +	u32 cnt = 0;
>> +	u32 state12, state17;
>> +
>> +	do {
>> +		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);
> 
>> +	dev_err(dev, "Error: Wait linkup timeout.\n");
> 
> Message doesn't match others from driver (capitalization and trailing
> period).
> 
>> +			dev_err(dev, "failed to get msi irq\n");
> 
> s/msi irq/MSI IRQ/
> 
>> +	ret = meson_add_pcie_port(mp, pdev);
>> +	if (ret < 0) {
>> +		dev_err(dev, "Add PCIE port failed, %d\n", ret);
> 
> s/PCIE/PCIe/
> 
> All the messages in this function are capitalized differently than
> other messages in the driver.
> 
> Bjorn
> 
> .
> 

hi Bjorn:

Thanks for the all suggestions and corrections.

There were too many code details unnecessary initialization, typing errors
and coding style etc. I will pay more attention to these rules and code-details
in the future patches also my daily work.

thanks again

hanjie.

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Re: [PATCH v8 2/2] PCI: amlogic: Add the Amlogic Meson PCIe controller driver

[Lorenzo Pieralisi]

On Wed, Dec 19, 2018 at 07:13:47PM +0800, Hanjie Lin wrote:
> 
> 
> On 2018/12/19 6:47, Bjorn Helgaas wrote:
> > On Tue, Dec 18, 2018 at 04:04:46PM +0800, Hanjie Lin wrote:
> >> From: Yue Wang <yue.wang@amlogic.com>
> >>
> >> The Amlogic Meson PCIe host controller is based on the Synopsys DesignWare
> >> PCI core. This patch adds the driver support for Meson PCIe controller.
> > 
> > I don't have any comments on the code itself; just the trivial things
> > below.  No need to repost for these unless you're changing something
> > else.
> > 
> > I thought it looked very pretty overall, thanks for paying attention
> > to that!
> > 
> >> +static int meson_size_to_payload(struct meson_pcie *mp, int size)
> >> +{
> >> +	struct device *dev = mp->pci.dev;
> >> +
> >> +	/*
> >> +	 * dwc supports 2^(val+7) payload size, which val is 0~5 default to 1.
> >> +	 * So if input size is not 2^order alignment or less than 2^7 or bigger
> >> +	 * than 2^12, just set to default size 2^(1+7).
> >> +	 */
> >> +	if (!is_power_of_2(size) || size < 128 || size > 4096) {
> >> +		dev_warn(dev, "playload size %d, set to default 256\n", size);
> > 
> > s/playload/payload/
> > 
> >> +static void meson_set_max_payload(struct meson_pcie *mp, int size)
> >> +{
> >> +	u32 val = 0;
> > 
> > Unnecessary initialization.
> > 
> >> +	int max_payload_size = meson_size_to_payload(mp, size);
> >> +
> >> +	val = meson_elb_readl(mp, PCIE_DEV_CTRL_DEV_STUS);
> > 
> >> +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);
> >> +	int ret;
> >> +
> >> +	ret = dw_pcie_read(pci->dbi_base + where, size, val);
> >> +	if (ret != PCIBIOS_SUCCESSFUL)
> >> +		return ret;
> >> +
> >> +	/*
> >> +	 * There is a bug in the MESON AXG pcie controller whereby software
> >> +	 * cannot programme the PCI_CLASS_DEVICE register, so we must fabricate
> >> +	 * the return value in the config accessors.
> > 
> > s/pcie/PCIe/
> > s/programme/program/ (IIUC, "programme" is British and only used as a
> > noun, where here you need a verb)
> > 
> >> +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;
> > 
> > Unnecessary initialization of smlh_up, ltssm_up, and rdlh_up.
> > 
> >> +	u32 speed_okay = 0;
> >> +	u32 cnt = 0;
> >> +	u32 state12, state17;
> >> +
> >> +	do {
> >> +		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);
> > 
> >> +	dev_err(dev, "Error: Wait linkup timeout.\n");
> > 
> > Message doesn't match others from driver (capitalization and trailing
> > period).
> > 
> >> +			dev_err(dev, "failed to get msi irq\n");
> > 
> > s/msi irq/MSI IRQ/
> > 
> >> +	ret = meson_add_pcie_port(mp, pdev);
> >> +	if (ret < 0) {
> >> +		dev_err(dev, "Add PCIE port failed, %d\n", ret);
> > 
> > s/PCIE/PCIe/
> > 
> > All the messages in this function are capitalized differently than
> > other messages in the driver.
> > 
> > Bjorn
> > 
> > .
> > 
> 
> hi Bjorn:
> 
> Thanks for the all suggestions and corrections.
> 
> There were too many code details unnecessary initialization, typing errors
> and coding style etc. I will pay more attention to these rules and code-details
> in the future patches also my daily work.

I have merged your patches with Bjorn's suggestions in my pci/amlogic
branch, aiming at v4.21, please have a look.

Lorenzo

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[PATCH v8 1/2] dt-bindings: PCI: meson: add DT bindings for Amlogic Meson PCIe controller

[Hanjie Lin]

From: Yue Wang <yue.wang@amlogic.com>

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 <yue.wang@amlogic.com>
Signed-off-by: Hanjie Lin <hanjie.lin@amlogic.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../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


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic