[PATCH RESEND v2 0/5] Introduction of PCIe support on STi platform

[PATCH RESEND v2 0/5] Introduction of PCIe support on STi platform

[Alain Volmat]

The STi platform SoC embed a designware based PCIe controller.
This serie include the driver for the controller and DT for the
stih407-family and for the stih418-b2264 into which it is used.

Within the DT and the binding, only st,stih407-pcie compatible is
used.
Rob, could you clarify if I need to mention both compatible (st,stih407-pcie
and snps,dw-pcie) or if st,stih407-pcie is enought ?

RESEND since not all patches made their way to the mailing list on
previous attempt.

v2: update of the pcie-st driver to add possibility to build as module
    and remove the __init of the probe

Alain Volmat (5):
  dt-bindings: pci: st-pcie: PCIe controller found on STi platforms
  pci: dwc: pcie-st: Add PCIe driver for STi platforms
  MAINTAINERS: add entry for ST STI PCIE driver
  ARM: dts: sti: add the PCIe controller node within stih407-family
  ARM: dts: sti: enable PCIe on the stih418-b2264 board

 .../devicetree/bindings/pci/snps,dw-pcie.yaml |   2 +-
 .../devicetree/bindings/pci/st,st-pcie.yaml   | 112 +++++
 MAINTAINERS                                   |   6 +
 arch/arm/boot/dts/stih407-family.dtsi         |  40 ++
 arch/arm/boot/dts/stih418-b2264.dts           |   5 +
 drivers/pci/controller/dwc/Kconfig            |  11 +
 drivers/pci/controller/dwc/Makefile           |   1 +
 drivers/pci/controller/dwc/pcie-sti.c         | 386 ++++++++++++++++++
 8 files changed, 562 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/pci/st,st-pcie.yaml
 create mode 100644 drivers/pci/controller/dwc/pcie-sti.c

-- 
2.25.1

[PATCH RESEND v2 1/5] dt-bindings: pci: st-pcie: PCIe controller found on STi platforms

[Alain Volmat]

Addition of the bindings for the Designware based PCIe controller
that can be found on STi platforms such as STiH407, STiH410 or STiH418.

Signed-off-by: Alain Volmat <avolmat@me.com>
---
 .../devicetree/bindings/pci/snps,dw-pcie.yaml |   2 +-
 .../devicetree/bindings/pci/st,st-pcie.yaml   | 112 ++++++++++++++++++
 2 files changed, 113 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/pci/st,st-pcie.yaml

diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
index 9ed0dfba7f89..3a92078128f7 100644
--- a/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
@@ -35,7 +35,7 @@ properties:
     maxItems: 5
     items:
       enum: [ dbi, dbi2, config, atu, app, elbi, mgmt, ctrl, parf, cfg, link,
-              ulreg, smu, mpu, apb, phy ]
+              ulreg, smu, mpu, apb, phy, mem-window ]
 
   num-lanes:
     description: |
diff --git a/Documentation/devicetree/bindings/pci/st,st-pcie.yaml b/Documentation/devicetree/bindings/pci/st,st-pcie.yaml
new file mode 100644
index 000000000000..2fa686d573c3
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/st,st-pcie.yaml
@@ -0,0 +1,112 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/st,st-pcie.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: PCIe RC controller on ST STi platform
+
+maintainers:
+  - Alain Volmat <avolmat@me.com>
+
+allOf:
+  - $ref: /schemas/pci/snps,dw-pcie.yaml#
+
+properties:
+  compatible:
+    const: st,stih407-pcie
+
+  reg:
+    items:
+      - description: Controller control and status registers.
+      - description: PCIe configuration registers.
+      - description: Memory made available to the controller
+
+  reg-names:
+    items:
+      - const: dbi
+      - const: config
+      - const: mem-window
+
+  interrupts:
+    maxItems: 1
+
+  resets:
+    items:
+    - description: Controller reset
+    - description: Powerdown reset (optional)
+    minItems: 1
+
+  reset-names:
+    items:
+      - const: softreset
+      - const: powerdown
+    minItems: 1
+
+  phys:
+    maxItems: 1
+
+  phy-names:
+    const: pcie
+
+  reset-gpios: true
+
+  st,syscfg:
+    $ref: "/schemas/types.yaml#/definitions/phandle-array"
+    description: syscfg node phandle and offsets of the 2 registers
+                 controlling root complex and ltssm.
+
+required:
+  - interrupts
+  - '#interrupt-cells'
+  - interrupt-map
+  - interrupt-map-mask
+  - resets
+  - reset-names
+  - phys
+  - phy-names
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/phy/phy.h>
+    #include <dt-bindings/reset/stih407-resets.h>
+    pcie1: pcie@9b10000 {
+      compatible = "st,stih407-pcie";
+      device_type = "pci";
+      #address-cells = <3>;
+      #size-cells = <2>;
+      reg = <0x09b10000 0x1000>,
+            <0x3fff0000 0x10000>,
+            <0x40000000 0xc0000000>;
+      reg-names = "dbi", "config", "mem-window";
+
+      #interrupt-cells = <1>;
+      interrupts = <GIC_SPI 175 IRQ_TYPE_LEVEL_HIGH>;
+      interrupt-names = "msi";
+      interrupt-map-mask = <0 0 0 7>;
+      interrupt-map = <0 0 0 1 &intc GIC_SPI 171 IRQ_TYPE_LEVEL_HIGH>,
+                      <0 0 0 2 &intc GIC_SPI 172 IRQ_TYPE_LEVEL_HIGH>,
+                      <0 0 0 3 &intc GIC_SPI 173 IRQ_TYPE_LEVEL_HIGH>,
+                      <0 0 0 4 &intc GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
+
+      st,syscfg = <&syscfg_core 0xdc 0xe4>;
+
+      ranges = <0x82000000 0 0x30000000 0x30000000 0 0x05550000>, /* non-prefetchable memory */
+               <0xc2000000 0 0x35550000 0x35550000 0 0x0AAA0000>; /* prefetchable memory */
+      bus-range = <0x00 0xff>;
+
+      resets = <&softreset STIH407_PCIE1_SOFTRESET>,
+               <&powerdown STIH407_PCIE1_POWERDOWN>;
+
+      reset-names = "softreset", "powerdown";
+
+      phys = <&phy_port1 PHY_TYPE_PCIE>;
+      phy-names = "pcie";
+
+      reset-gpios = <&pio34 5 GPIO_ACTIVE_LOW>;
+    };
-- 
2.25.1

[PATCH RESEND v2 2/5] pci: dwc: pcie-st: Add PCIe driver for STi platforms

[Alain Volmat]

Addition of the PCIe driver (supporting RC) for controllers
found on some STi platforms such as STiH407, STiH410 or
STiH418.
The controller is based on the designware PCIe controller.

Signed-off-by: Alain Volmat <avolmat@me.com>
---
v2: update of the pcie-st driver to add possibility to build as module
    and remove the __init of the probe

 drivers/pci/controller/dwc/Kconfig    |  11 +
 drivers/pci/controller/dwc/Makefile   |   1 +
 drivers/pci/controller/dwc/pcie-sti.c | 386 ++++++++++++++++++++++++++
 3 files changed, 398 insertions(+)
 create mode 100644 drivers/pci/controller/dwc/pcie-sti.c

diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
index 62ce3abf0f19..232b8ffd54af 100644
--- a/drivers/pci/controller/dwc/Kconfig
+++ b/drivers/pci/controller/dwc/Kconfig
@@ -384,4 +384,15 @@ config PCIE_FU740
 	  Say Y here if you want PCIe controller support for the SiFive
 	  FU740.
 
+config PCIE_STI
+	tristate "STMicroelectronics PCIe Controller for STi SoCs"
+	depends on ARCH_STI || COMPILE_TEST
+	depends on OF && PCI_MSI_IRQ_DOMAIN
+	select PCIE_DW_HOST
+	help
+	  Enable PCIe controller support on STMicroelectronics STi SoCs.
+	  This controller 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 8ba7b67f5e50..c98fa18714ce 100644
--- a/drivers/pci/controller/dwc/Makefile
+++ b/drivers/pci/controller/dwc/Makefile
@@ -21,6 +21,7 @@ obj-$(CONFIG_PCIE_KEEMBAY) += pcie-keembay.o
 obj-$(CONFIG_PCIE_KIRIN) += pcie-kirin.o
 obj-$(CONFIG_PCIE_HISI_STB) += pcie-histb.o
 obj-$(CONFIG_PCI_MESON) += pci-meson.o
+obj-$(CONFIG_PCIE_STI) += pcie-sti.o
 obj-$(CONFIG_PCIE_TEGRA194) += pcie-tegra194.o
 obj-$(CONFIG_PCIE_UNIPHIER) += pcie-uniphier.o
 obj-$(CONFIG_PCIE_UNIPHIER_EP) += pcie-uniphier-ep.o
diff --git a/drivers/pci/controller/dwc/pcie-sti.c b/drivers/pci/controller/dwc/pcie-sti.c
new file mode 100644
index 000000000000..61da00a079ac
--- /dev/null
+++ b/drivers/pci/controller/dwc/pcie-sti.c
@@ -0,0 +1,386 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2021 STMicroelectronics
+ *
+ * STMicroelectronics PCI express Driver for STi SoCs.
+ * ST PCIe IPs are built around a Synopsys IP Core.
+ *
+ * Authors: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
+ *          Alain Volmat <avolmat@me.com>
+ */
+
+#include <linux/delay.h>
+#include <linux/interrupt.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_gpio.h>
+#include <linux/of_pci.h>
+#include <linux/of_platform.h>
+#include <linux/phy/phy.h>
+#include <linux/regmap.h>
+#include <linux/reset.h>
+
+#include "pcie-designware.h"
+
+/* RC_ADDRESS_TRANSLATION Registers */
+#define TRANSLATION_CONTROL		0x900
+/* Controls if area is inclusive or exclusive */
+#define RC_PASS_ADDR_RANGE		BIT(1)
+
+/* Base of area reserved for config accesses. Fixed size of 64K. */
+#define CFG_BASE_ADDRESS		0x92c
+#define CFG_REGION_SIZE			65536
+#define CFG_SPACE1_OFFSET		0x1000
+
+/* First 4K of config space has this BDF (bus,device,function) */
+#define FUNC0_BDF_NUM			0x930
+
+/* Mem regions */
+#define IN0_MEM_ADDR_START		0x964
+#define IN0_MEM_ADDR_LIMIT		0x968
+#define IN1_MEM_ADDR_START		0x974
+#define IN1_MEM_ADDR_LIMIT		0x978
+
+/* syscfg1 bits */
+#define PCIE_APP_LTSSM_ENABLE		BIT(2)
+/* syscfg0 bits */
+#define PCIE_TYPE_ROOT_COMPLEX		BIT(0)
+
+/* st,syscfg offsets */
+#define SYSCFG0_REG	1
+#define SYSCFG1_REG	2
+
+#define to_st_pcie(x)	dev_get_drvdata((x)->dev)
+
+/**
+ * struct st_pcie - private data of the controller
+ * @dw: designware pcie
+ * @syscfg0: PCIe conf register setting root complex, regmap offset
+ * @syscfg1: PCIe conf register for PCIE_APP_LTSSM_ENABLE, regmap offset
+ * @phy: associated pcie phy
+ * @lmi: memory made available to the controller
+ * @regmap: Syscfg registers bank in which PCIe port is configured
+ * @pwr: power control
+ * @rst: reset control
+ * @reset_gpio: optional reset gpio
+ * @config_window_start: start address of 64K config space area
+ */
+struct st_pcie {
+	struct dw_pcie *dw;
+	int syscfg0;
+	int syscfg1;
+	struct phy *phy;
+	struct resource	*lmi;
+	struct regmap *regmap;
+	struct reset_control *pwr;
+	struct reset_control *rst;
+	struct gpio_desc *reset_gpio;
+	phys_addr_t config_window_start;
+};
+
+/*
+ * The PCI express core IP expects the following arrangement on it's address
+ * bus (slv_haddr) when driving config cycles.
+ * bus_number		[31:24]
+ * dev_number		[23:19]
+ * func_number		[18:16]
+ * unused		[15:12]
+ * ext_reg_number	[11:8]
+ * reg_number		[7:2]
+ *
+ * Bits [15:12] are unused.
+ *
+ * In the glue logic there is a 64K region of address space that can be
+ * written/read to generate config cycles. The base address of this is
+ * controlled by CFG_BASE_ADDRESS. There are 8 16 bit registers called
+ * FUNC0_BDF_NUM to FUNC8_BDF_NUM. These split the bottom half of the 64K
+ * window into 8 regions at 4K boundaries. These control the bus, device and
+ * function number you are trying to talk to.
+ *
+ * The decision on whether to generate a type 0 or type 1 access is controlled
+ * by bits 15:12 of the address you write to.  If they are zero, then a type 0
+ * is generated, if anything else it will be a type 1. Thus the bottom 4K
+ * region controlled by FUNC0_BDF_NUM can only generate type 0, all the others
+ * can only generate type 1.
+ *
+ * We only use FUNC0_BDF_NUM and FUNC1_BDF_NUM. Which one you use is selected
+ * by bit 12 of the address you write to. The selected register is then used
+ * for the top 16 bits of the slv_haddr to form the bus/dev/func, bit 15:12 are
+ * wired to zero, and bits 11:2 form the address of the register you want to
+ * read in config space.
+ *
+ * We always write FUNC0_BDF_NUM as a 32 bit write. So if we want type 1
+ * accesses we have to shift by 16 so in effect we are writing to FUNC1_BDF_NUM
+ */
+static inline u32 bdf_num(int bus, int devfn, int is_root_bus)
+{
+	return ((bus << 8) | devfn) << (is_root_bus ? 0 : 16);
+}
+
+static void __iomem *st_pcie_other_map_bus(struct pci_bus *bus, unsigned int devfn, int where)
+{
+	struct pcie_port *pp = bus->sysdata;
+	struct dw_pcie *dw = to_dw_pcie_from_pp(pp);
+	u32 bdf;
+
+	bdf = bdf_num(bus->number, devfn, pci_is_root_bus(bus));
+
+	/* Set the config packet devfn */
+	dw_pcie_writel_dbi(dw, FUNC0_BDF_NUM, bdf);
+	dw_pcie_readl_dbi(dw, FUNC0_BDF_NUM);
+
+	return pp->va_cfg0_base + where + (pci_is_root_bus(bus->parent) ? 0 : CFG_SPACE1_OFFSET);
+}
+
+static struct pci_ops st_child_pcie_ops = {
+	.map_bus = st_pcie_other_map_bus,
+	.read = pci_generic_config_read,
+	.write = pci_generic_config_write,
+};
+
+static void st_pcie_hw_setup(struct dw_pcie *dw)
+{
+	struct st_pcie *pcie = to_st_pcie(dw);
+
+	/* Set up the config window to the top of the PCI address space */
+	dw_pcie_writel_dbi(dw, CFG_BASE_ADDRESS, pcie->config_window_start);
+
+	/*
+	 * Open up memory to the PCI controller. We could do slightly
+	 * better than this and exclude the kernel text segment and bss etc.
+	 * They are base/limit registers so can be of arbitrary alignment
+	 * presumably
+	 */
+	dw_pcie_writel_dbi(dw, IN0_MEM_ADDR_START, pcie->lmi->start);
+	dw_pcie_writel_dbi(dw, IN0_MEM_ADDR_LIMIT, pcie->lmi->end);
+
+	/* Disable the 2nd region */
+	dw_pcie_writel_dbi(dw, IN1_MEM_ADDR_START, ~0);
+	dw_pcie_writel_dbi(dw, IN1_MEM_ADDR_LIMIT, 0);
+
+	dw_pcie_writel_dbi(dw, TRANSLATION_CONTROL, RC_PASS_ADDR_RANGE);
+}
+
+static int st_pcie_init(struct pcie_port *pp)
+{
+	struct dw_pcie *dw = to_dw_pcie_from_pp(pp);
+	struct st_pcie *pcie = to_st_pcie(dw);
+	int ret;
+
+	/* Set device type : Root Complex */
+	ret = regmap_write(pcie->regmap, pcie->syscfg0, PCIE_TYPE_ROOT_COMPLEX);
+	if (ret < 0) {
+		dev_err(dw->dev, "unable to set device type\n");
+		return ret;
+	}
+
+	ret = reset_control_deassert(pcie->pwr);
+	if (ret) {
+		dev_err(dw->dev, "unable to bring out of powerdown\n");
+		return ret;
+	}
+
+	ret = reset_control_deassert(pcie->rst);
+	if (ret) {
+		dev_err(dw->dev, "unable to bring out of softreset\n");
+		return ret;
+	}
+
+	/* let the controller initialize in the proper mode (RC) after reset */
+	usleep_range(1000, 2000);
+
+	return ret;
+}
+
+static int st_pcie_control_ltssm(struct dw_pcie *dw, bool enable)
+{
+	struct st_pcie *pcie = to_st_pcie(dw);
+
+	return regmap_update_bits(pcie->regmap, pcie->syscfg1, PCIE_APP_LTSSM_ENABLE,
+				  FIELD_PREP(PCIE_APP_LTSSM_ENABLE, enable));
+}
+
+static int st_pcie_host_init(struct pcie_port *pp)
+{
+	struct dw_pcie *dw = to_dw_pcie_from_pp(pp);
+	struct st_pcie *pcie = to_st_pcie(dw);
+	int err;
+
+	pcie->config_window_start = pp->cfg0_base;
+
+	/*
+	 * "Override" default ops provided by designware driver as STI
+	 * PCIe uses its own translation unit rather than iATU
+	 */
+	pp->bridge->child_ops = &st_child_pcie_ops;
+
+	/*
+	 * We have to initialise the PCIe cell on some hardware before we can
+	 * talk to the phy
+	 */
+	err = st_pcie_init(pp);
+	if (err)
+		return err;
+
+	err = st_pcie_control_ltssm(dw, false);
+	if (err) {
+		dev_err(dw->dev, "disable ltssm failed, %d\n", err);
+		return err;
+	}
+
+	/* Init the associated miphy */
+	err = phy_init(pcie->phy);
+	if (err < 0) {
+		dev_err(dw->dev, "Cannot init PHY: %d\n", err);
+		return err;
+	}
+
+	return 0;
+}
+
+static int st_pcie_start_link(struct dw_pcie *dw)
+{
+	struct st_pcie *pcie = to_st_pcie(dw);
+	int err;
+
+	/* Do all the register poking */
+	st_pcie_hw_setup(dw);
+
+	if (pcie->reset_gpio) {
+		/* Assert the PERST# signal */
+		gpiod_set_value(pcie->reset_gpio, 1);
+
+		/* PERST# signal must stay asserted for at least 100us (Tperst-clk) */
+		usleep_range(100, 200);
+
+		/* Release PERST# signal */
+		gpiod_set_value(pcie->reset_gpio, 0);
+	}
+
+	/* Re-enable the link, link training must begin shortly after reset */
+	err = st_pcie_control_ltssm(dw, true);
+	if (err) {
+		dev_err(dw->dev, "enable ltssm failed, %d\n", err);
+		return err;
+	}
+
+	err = dw_pcie_wait_for_link(dw);
+	if (err) {
+		dev_err(dw->dev, "wait for link failed, %d\n", err);
+		return err;
+	}
+
+	/*
+	 * PCIe specification states that you should not issue any config
+	 * requests until 100ms after asserting reset, so we enforce that here
+	 */
+	if  (pcie->reset_gpio)
+		msleep(100);
+
+	return 0;
+}
+
+static struct dw_pcie_host_ops st_pcie_host_ops = {
+	.host_init = st_pcie_host_init,
+};
+
+static const struct dw_pcie_ops dw_pcie_ops = {
+	.start_link = st_pcie_start_link,
+};
+
+static int st_pcie_probe(struct platform_device *pdev)
+{
+	struct st_pcie *pcie;
+	struct dw_pcie *dw;
+	struct device_node *np = pdev->dev.of_node;
+	struct pcie_port *pp;
+	int ret;
+
+	pcie = devm_kzalloc(&pdev->dev, sizeof(*pcie), GFP_KERNEL);
+	if (!pcie)
+		return -ENOMEM;
+
+	dw = devm_kzalloc(&pdev->dev, sizeof(*dw), GFP_KERNEL);
+	if (!dw)
+		return -ENOMEM;
+	pcie->dw = dw;
+	dw->dev = &pdev->dev;
+	dw->ops = &dw_pcie_ops;
+
+	pp = &dw->pp;
+	pp->ops = &st_pcie_host_ops;
+
+	/* mem regions */
+	pcie->lmi = platform_get_resource_byname(pdev, IORESOURCE_MEM, "mem-window");
+	if (!pcie->lmi)
+		return -ENXIO;
+
+	/* regmap registers for PCIe IP configuration */
+	pcie->regmap = syscon_regmap_lookup_by_phandle(np, "st,syscfg");
+	if (IS_ERR(pcie->regmap))
+		return dev_err_probe(&pdev->dev, PTR_ERR(pcie->regmap),
+				     "No syscfg phandle specified\n");
+
+	ret = of_property_read_u32_index(np, "st,syscfg", SYSCFG0_REG, &pcie->syscfg0);
+	if (ret) {
+		dev_err(&pdev->dev, "can't get syscfg0 offset (%d)\n", ret);
+		return ret;
+	}
+
+	ret = of_property_read_u32_index(np, "st,syscfg", SYSCFG1_REG, &pcie->syscfg1);
+	if (ret) {
+		dev_err(&pdev->dev, "can't get syscfg1 offset (%d)\n", ret);
+		return ret;
+	}
+
+	/* powerdown / resets */
+	pcie->pwr = devm_reset_control_get_optional(&pdev->dev, "powerdown");
+	if (IS_ERR(pcie->pwr))
+		return dev_err_probe(&pdev->dev, PTR_ERR(pcie->pwr),
+				     "Error getting powerdown reset control\n");
+
+	pcie->rst = devm_reset_control_get(&pdev->dev, "softreset");
+	if (IS_ERR(pcie->rst))
+		return dev_err_probe(&pdev->dev, PTR_ERR(pcie->rst),
+				     "Error getting softreset control\n");
+
+	/* phy */
+	pcie->phy = devm_phy_get(&pdev->dev, "pcie");
+	if (IS_ERR(pcie->phy))
+		return dev_err_probe(&pdev->dev, PTR_ERR(pcie->phy), "no PHY configured\n");
+
+	/* Claim the GPIO for PRST# if available, keep it de-asserted */
+	pcie->reset_gpio = devm_gpiod_get_optional(&pdev->dev, "reset", GPIOD_OUT_LOW);
+	if (IS_ERR(pcie->reset_gpio))
+		return dev_err_probe(&pdev->dev, PTR_ERR(pcie->reset_gpio),
+				     "Cannot request reset-gpios\n");
+
+	platform_set_drvdata(pdev, pcie);
+
+	ret = dw_pcie_host_init(pp);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret, "failed to initialize host\n");
+
+	return 0;
+}
+
+static const struct of_device_id st_pcie_of_match[] = {
+	{ .compatible = "st,stih407-pcie", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, st_pcie_of_match);
+
+static struct platform_driver st_pcie_driver __refdata = {
+	.probe  = st_pcie_probe,
+	.driver = {
+		.name = "st-pcie",
+		.of_match_table = st_pcie_of_match,
+	},
+};
+module_platform_driver(st_pcie_driver);
+
+MODULE_AUTHOR("Fabrice Gasnier <fabrice.gasnier@foss.st.com>");
+MODULE_AUTHOR("Alain Volmat <avolmat@me.com>");
+MODULE_DESCRIPTION("STi PCIe Controller driver");
+MODULE_LICENSE("GPL v2");
-- 
2.25.1

[PATCH RESEND v2 3/5] MAINTAINERS: add entry for ST STI PCIE driver

[Alain Volmat]

Add PCIE Driver entry for STI family from ST Microelectronics.

Signed-off-by: Alain Volmat <avolmat@me.com>
---
 MAINTAINERS | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 360e9aa0205d..081ccdfbd89c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14666,6 +14666,12 @@ L:	linux-samsung-soc@vger.kernel.org
 S:	Maintained
 F:	drivers/pci/controller/dwc/pci-exynos.c
 
+PCI DRIVER FOR ST STI PLATFORM
+M:	Alain Volmat <avolmat@me.com>
+L:	linux-pci@vger.kernel.org
+S:	Maintained
+F:	drivers/pci/controller/dwc/pcie-sti.c
+
 PCI DRIVER FOR SYNOPSYS DESIGNWARE
 M:	Jingoo Han <jingoohan1@gmail.com>
 M:	Gustavo Pimentel <gustavo.pimentel@synopsys.com>
-- 
2.25.1

[PATCH RESEND v2 4/5] ARM: dts: sti: add the PCIe controller node within stih407-family

[Alain Volmat]

Add the pcie1 entry within stih407-family dtsi.

Signed-off-by: Alain Volmat <avolmat@me.com>
---
 arch/arm/boot/dts/stih407-family.dtsi | 40 +++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/arch/arm/boot/dts/stih407-family.dtsi b/arch/arm/boot/dts/stih407-family.dtsi
index 21f3347a91d6..fe4ea2d5b583 100644
--- a/arch/arm/boot/dts/stih407-family.dtsi
+++ b/arch/arm/boot/dts/stih407-family.dtsi
@@ -631,6 +631,46 @@ spifsm: spifsm@9022000{
 			status = "disabled";
 		};
 
+		pcie1: pcie@9b10000 {
+			compatible = "st,stih407-pcie";
+			device_type = "pci";
+			reg = <0x09b10000 0x00001000>,	/* cntrl registers */
+			      <0x3fff0000 0x00010000>,	/* config space */
+			      <0x40000000 0xc0000000>;	/* lmi mem window */
+
+			reg-names = "dbi",
+				    "config",
+				    "mem-window";
+
+			st,syscfg = <&syscfg_core 0xdc 0xe4>;
+
+			#interrupt-cells = <1>;
+			interrupts = <GIC_SPI 175 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "msi";
+			interrupt-map-mask = <0 0 0 7>;
+			interrupt-map = <0 0 0 1 &intc GIC_SPI 171 IRQ_TYPE_LEVEL_HIGH>,
+					<0 0 0 2 &intc GIC_SPI 172 IRQ_TYPE_LEVEL_HIGH>,
+					<0 0 0 3 &intc GIC_SPI 173 IRQ_TYPE_LEVEL_HIGH>,
+					<0 0 0 4 &intc GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
+
+			#address-cells = <3>;
+			#size-cells = <2>;
+			/* non-prefetchable and prefetchable */
+			ranges = <0x82000000 0 0x30000000 0x30000000 0 0x05550000>,
+				 <0xc2000000 0 0x35550000 0x35550000 0 0x0AAA0000>;
+			bus-range = <0x00 0xff>;
+
+			resets = <&softreset STIH407_PCIE1_SOFTRESET>,
+				 <&powerdown STIH407_PCIE1_POWERDOWN>;
+
+			reset-names = "softreset", "powerdown";
+
+			phys = <&phy_port1 PHY_TYPE_PCIE>;
+			phy-names = "pcie";
+
+			status = "disabled";
+		};
+
 		sata0: sata@9b20000 {
 			compatible = "st,ahci";
 			reg = <0x9b20000 0x1000>;
-- 
2.25.1

[PATCH RESEND v2 5/5] ARM: dts: sti: enable PCIe on the stih418-b2264 board

[Alain Volmat]

Enable the PCIe controller with proper reset gpio pin for this board.

Signed-off-by: Alain Volmat <avolmat@me.com>
---
 arch/arm/boot/dts/stih418-b2264.dts | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/stih418-b2264.dts b/arch/arm/boot/dts/stih418-b2264.dts
index a99604bebf8c..ed183292a669 100644
--- a/arch/arm/boot/dts/stih418-b2264.dts
+++ b/arch/arm/boot/dts/stih418-b2264.dts
@@ -130,6 +130,11 @@ &ohci1 {
 	status = "okay";
 };
 
+&pcie1 {
+	reset-gpios = <&pio34 5 GPIO_ACTIVE_LOW>;
+	status = "okay";
+};
+
 &pwm1 {
 	status = "okay";
 };
-- 
2.25.1

Re: [PATCH RESEND v2 2/5] pci: dwc: pcie-st: Add PCIe driver for STi platforms

[Patrice CHOTARD]

Hi Alain

Some minor remarks below

You can add my reviewed-by

Thanks
Patrice

On 1/3/22 08:47, Alain Volmat wrote:
> Addition of the PCIe driver (supporting RC) for controllers
> found on some STi platforms such as STiH407, STiH410 or
> STiH418.
> The controller is based on the designware PCIe controller.
> 
> Signed-off-by: Alain Volmat <avolmat@me.com>
> ---
> v2: update of the pcie-st driver to add possibility to build as module
>     and remove the __init of the probe
> 
>  drivers/pci/controller/dwc/Kconfig    |  11 +
>  drivers/pci/controller/dwc/Makefile   |   1 +
>  drivers/pci/controller/dwc/pcie-sti.c | 386 ++++++++++++++++++++++++++
>  3 files changed, 398 insertions(+)
>  create mode 100644 drivers/pci/controller/dwc/pcie-sti.c
> 
> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> index 62ce3abf0f19..232b8ffd54af 100644
> --- a/drivers/pci/controller/dwc/Kconfig
> +++ b/drivers/pci/controller/dwc/Kconfig
> @@ -384,4 +384,15 @@ config PCIE_FU740
>  	  Say Y here if you want PCIe controller support for the SiFive
>  	  FU740.
>  
> +config PCIE_STI
> +	tristate "STMicroelectronics PCIe Controller for STi SoCs"
> +	depends on ARCH_STI || COMPILE_TEST
> +	depends on OF && PCI_MSI_IRQ_DOMAIN
> +	select PCIE_DW_HOST
> +	help
> +	  Enable PCIe controller support on STMicroelectronics STi SoCs.
> +	  This controller 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 8ba7b67f5e50..c98fa18714ce 100644
> --- a/drivers/pci/controller/dwc/Makefile
> +++ b/drivers/pci/controller/dwc/Makefile
> @@ -21,6 +21,7 @@ obj-$(CONFIG_PCIE_KEEMBAY) += pcie-keembay.o
>  obj-$(CONFIG_PCIE_KIRIN) += pcie-kirin.o
>  obj-$(CONFIG_PCIE_HISI_STB) += pcie-histb.o
>  obj-$(CONFIG_PCI_MESON) += pci-meson.o
> +obj-$(CONFIG_PCIE_STI) += pcie-sti.o
>  obj-$(CONFIG_PCIE_TEGRA194) += pcie-tegra194.o
>  obj-$(CONFIG_PCIE_UNIPHIER) += pcie-uniphier.o
>  obj-$(CONFIG_PCIE_UNIPHIER_EP) += pcie-uniphier-ep.o
> diff --git a/drivers/pci/controller/dwc/pcie-sti.c b/drivers/pci/controller/dwc/pcie-sti.c
> new file mode 100644
> index 000000000000..61da00a079ac
> --- /dev/null
> +++ b/drivers/pci/controller/dwc/pcie-sti.c
> @@ -0,0 +1,386 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2021 STMicroelectronics

2022

> + *
> + * STMicroelectronics PCI express Driver for STi SoCs.
> + * ST PCIe IPs are built around a Synopsys IP Core.
> + *
> + * Authors: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
> + *          Alain Volmat <avolmat@me.com>
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/interrupt.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/of_gpio.h>
> +#include <linux/of_pci.h>
> +#include <linux/of_platform.h>
> +#include <linux/phy/phy.h>
> +#include <linux/regmap.h>
> +#include <linux/reset.h>
> +
> +#include "pcie-designware.h"
> +
> +/* RC_ADDRESS_TRANSLATION Registers */
> +#define TRANSLATION_CONTROL		0x900
> +/* Controls if area is inclusive or exclusive */
> +#define RC_PASS_ADDR_RANGE		BIT(1)
> +
> +/* Base of area reserved for config accesses. Fixed size of 64K. */
> +#define CFG_BASE_ADDRESS		0x92c
> +#define CFG_REGION_SIZE			65536

s/65536/SZ_64K

> +#define CFG_SPACE1_OFFSET		0x1000
> +
> +/* First 4K of config space has this BDF (bus,device,function) */
> +#define FUNC0_BDF_NUM			0x930
> +
> +/* Mem regions */
> +#define IN0_MEM_ADDR_START		0x964
> +#define IN0_MEM_ADDR_LIMIT		0x968
> +#define IN1_MEM_ADDR_START		0x974
> +#define IN1_MEM_ADDR_LIMIT		0x978
> +
> +/* syscfg1 bits */
> +#define PCIE_APP_LTSSM_ENABLE		BIT(2)
> +/* syscfg0 bits */
> +#define PCIE_TYPE_ROOT_COMPLEX		BIT(0)
> +
> +/* st,syscfg offsets */
> +#define SYSCFG0_REG	1
> +#define SYSCFG1_REG	2
> +
> +#define to_st_pcie(x)	dev_get_drvdata((x)->dev)
> +
> +/**
> + * struct st_pcie - private data of the controller
> + * @dw: designware pcie
> + * @syscfg0: PCIe conf register setting root complex, regmap offset
> + * @syscfg1: PCIe conf register for PCIE_APP_LTSSM_ENABLE, regmap offset
> + * @phy: associated pcie phy
> + * @lmi: memory made available to the controller
> + * @regmap: Syscfg registers bank in which PCIe port is configured
> + * @pwr: power control
> + * @rst: reset control
> + * @reset_gpio: optional reset gpio
> + * @config_window_start: start address of 64K config space area
> + */
> +struct st_pcie {
> +	struct dw_pcie *dw;
> +	int syscfg0;
> +	int syscfg1;
> +	struct phy *phy;
> +	struct resource	*lmi;
> +	struct regmap *regmap;
> +	struct reset_control *pwr;
> +	struct reset_control *rst;
> +	struct gpio_desc *reset_gpio;
> +	phys_addr_t config_window_start;
> +};
> +
> +/*
> + * The PCI express core IP expects the following arrangement on it's address
> + * bus (slv_haddr) when driving config cycles.
> + * bus_number		[31:24]
> + * dev_number		[23:19]
> + * func_number		[18:16]
> + * unused		[15:12]
> + * ext_reg_number	[11:8]
> + * reg_number		[7:2]
> + *
> + * Bits [15:12] are unused.
> + *
> + * In the glue logic there is a 64K region of address space that can be
> + * written/read to generate config cycles. The base address of this is
> + * controlled by CFG_BASE_ADDRESS. There are 8 16 bit registers called
> + * FUNC0_BDF_NUM to FUNC8_BDF_NUM. These split the bottom half of the 64K
> + * window into 8 regions at 4K boundaries. These control the bus, device and
> + * function number you are trying to talk to.
> + *
> + * The decision on whether to generate a type 0 or type 1 access is controlled
> + * by bits 15:12 of the address you write to.  If they are zero, then a type 0
> + * is generated, if anything else it will be a type 1. Thus the bottom 4K
> + * region controlled by FUNC0_BDF_NUM can only generate type 0, all the others
> + * can only generate type 1.
> + *
> + * We only use FUNC0_BDF_NUM and FUNC1_BDF_NUM. Which one you use is selected
> + * by bit 12 of the address you write to. The selected register is then used
> + * for the top 16 bits of the slv_haddr to form the bus/dev/func, bit 15:12 are
> + * wired to zero, and bits 11:2 form the address of the register you want to
> + * read in config space.
> + *
> + * We always write FUNC0_BDF_NUM as a 32 bit write. So if we want type 1
> + * accesses we have to shift by 16 so in effect we are writing to FUNC1_BDF_NUM
> + */
> +static inline u32 bdf_num(int bus, int devfn, int is_root_bus)
> +{
> +	return ((bus << 8) | devfn) << (is_root_bus ? 0 : 16);
> +}
> +
> +static void __iomem *st_pcie_other_map_bus(struct pci_bus *bus, unsigned int devfn, int where)
> +{
> +	struct pcie_port *pp = bus->sysdata;
> +	struct dw_pcie *dw = to_dw_pcie_from_pp(pp);
> +	u32 bdf;
> +
> +	bdf = bdf_num(bus->number, devfn, pci_is_root_bus(bus));
> +
> +	/* Set the config packet devfn */
> +	dw_pcie_writel_dbi(dw, FUNC0_BDF_NUM, bdf);
> +	dw_pcie_readl_dbi(dw, FUNC0_BDF_NUM);

Can you add an explaination about this sequence write/read the same register ?

> +
> +	return pp->va_cfg0_base + where + (pci_is_root_bus(bus->parent) ? 0 : CFG_SPACE1_OFFSET);
> +}
> +
> +static struct pci_ops st_child_pcie_ops = {
> +	.map_bus = st_pcie_other_map_bus,
> +	.read = pci_generic_config_read,
> +	.write = pci_generic_config_write,
> +};
> +
> +static void st_pcie_hw_setup(struct dw_pcie *dw)
> +{
> +	struct st_pcie *pcie = to_st_pcie(dw);
> +
> +	/* Set up the config window to the top of the PCI address space */
> +	dw_pcie_writel_dbi(dw, CFG_BASE_ADDRESS, pcie->config_window_start);
> +
> +	/*
> +	 * Open up memory to the PCI controller. We could do slightly
> +	 * better than this and exclude the kernel text segment and bss etc.
> +	 * They are base/limit registers so can be of arbitrary alignment
> +	 * presumably
> +	 */
> +	dw_pcie_writel_dbi(dw, IN0_MEM_ADDR_START, pcie->lmi->start);
> +	dw_pcie_writel_dbi(dw, IN0_MEM_ADDR_LIMIT, pcie->lmi->end);
> +
> +	/* Disable the 2nd region */
> +	dw_pcie_writel_dbi(dw, IN1_MEM_ADDR_START, ~0);
> +	dw_pcie_writel_dbi(dw, IN1_MEM_ADDR_LIMIT, 0);
> +
> +	dw_pcie_writel_dbi(dw, TRANSLATION_CONTROL, RC_PASS_ADDR_RANGE);
> +}
> +
> +static int st_pcie_init(struct pcie_port *pp)
> +{
> +	struct dw_pcie *dw = to_dw_pcie_from_pp(pp);
> +	struct st_pcie *pcie = to_st_pcie(dw);
> +	int ret;
> +
> +	/* Set device type : Root Complex */
> +	ret = regmap_write(pcie->regmap, pcie->syscfg0, PCIE_TYPE_ROOT_COMPLEX);
> +	if (ret < 0) {
> +		dev_err(dw->dev, "unable to set device type\n");
> +		return ret;
> +	}
> +
> +	ret = reset_control_deassert(pcie->pwr);
> +	if (ret) {
> +		dev_err(dw->dev, "unable to bring out of powerdown\n");
> +		return ret;
> +	}
> +
> +	ret = reset_control_deassert(pcie->rst);
> +	if (ret) {
> +		dev_err(dw->dev, "unable to bring out of softreset\n");
> +		return ret;
> +	}
> +
> +	/* let the controller initialize in the proper mode (RC) after reset */
> +	usleep_range(1000, 2000);
> +
> +	return ret;
> +}
> +
> +static int st_pcie_control_ltssm(struct dw_pcie *dw, bool enable)
> +{
> +	struct st_pcie *pcie = to_st_pcie(dw);
> +
> +	return regmap_update_bits(pcie->regmap, pcie->syscfg1, PCIE_APP_LTSSM_ENABLE,
> +				  FIELD_PREP(PCIE_APP_LTSSM_ENABLE, enable));
> +}
> +
> +static int st_pcie_host_init(struct pcie_port *pp)
> +{
> +	struct dw_pcie *dw = to_dw_pcie_from_pp(pp);
> +	struct st_pcie *pcie = to_st_pcie(dw);
> +	int err;
> +
> +	pcie->config_window_start = pp->cfg0_base;
> +
> +	/*
> +	 * "Override" default ops provided by designware driver as STI

s/STI/STi

> +	 * PCIe uses its own translation unit rather than iATU
> +	 */
> +	pp->bridge->child_ops = &st_child_pcie_ops;
> +
> +	/*
> +	 * We have to initialise the PCIe cell on some hardware before we can
> +	 * talk to the phy
> +	 */
> +	err = st_pcie_init(pp);
> +	if (err)
> +		return err;
> +
> +	err = st_pcie_control_ltssm(dw, false);
> +	if (err) {
> +		dev_err(dw->dev, "disable ltssm failed, %d\n", err);
> +		return err;
> +	}
> +
> +	/* Init the associated miphy */
> +	err = phy_init(pcie->phy);
> +	if (err < 0) {
> +		dev_err(dw->dev, "Cannot init PHY: %d\n", err);
> +		return err;
> +	}
> +
> +	return 0;
> +}
> +
> +static int st_pcie_start_link(struct dw_pcie *dw)
> +{
> +	struct st_pcie *pcie = to_st_pcie(dw);
> +	int err;
> +
> +	/* Do all the register poking */
> +	st_pcie_hw_setup(dw);
> +
> +	if (pcie->reset_gpio) {
> +		/* Assert the PERST# signal */
> +		gpiod_set_value(pcie->reset_gpio, 1);
> +
> +		/* PERST# signal must stay asserted for at least 100us (Tperst-clk) */
> +		usleep_range(100, 200);
> +
> +		/* Release PERST# signal */
> +		gpiod_set_value(pcie->reset_gpio, 0);
> +	}
> +
> +	/* Re-enable the link, link training must begin shortly after reset */
> +	err = st_pcie_control_ltssm(dw, true);
> +	if (err) {
> +		dev_err(dw->dev, "enable ltssm failed, %d\n", err);
> +		return err;
> +	}
> +
> +	err = dw_pcie_wait_for_link(dw);
> +	if (err) {
> +		dev_err(dw->dev, "wait for link failed, %d\n", err);
> +		return err;
> +	}
> +
> +	/*
> +	 * PCIe specification states that you should not issue any config
> +	 * requests until 100ms after asserting reset, so we enforce that here
> +	 */
> +	if  (pcie->reset_gpio)
> +		msleep(100);
> +
> +	return 0;
> +}
> +
> +static struct dw_pcie_host_ops st_pcie_host_ops = {
> +	.host_init = st_pcie_host_init,
> +};
> +
> +static const struct dw_pcie_ops dw_pcie_ops = {
> +	.start_link = st_pcie_start_link,
> +};
> +
> +static int st_pcie_probe(struct platform_device *pdev)
> +{
> +	struct st_pcie *pcie;
> +	struct dw_pcie *dw;
> +	struct device_node *np = pdev->dev.of_node;
> +	struct pcie_port *pp;
> +	int ret;
> +
> +	pcie = devm_kzalloc(&pdev->dev, sizeof(*pcie), GFP_KERNEL);
> +	if (!pcie)
> +		return -ENOMEM;
> +
> +	dw = devm_kzalloc(&pdev->dev, sizeof(*dw), GFP_KERNEL);
> +	if (!dw)
> +		return -ENOMEM;
> +	pcie->dw = dw;
> +	dw->dev = &pdev->dev;
> +	dw->ops = &dw_pcie_ops;
> +
> +	pp = &dw->pp;
> +	pp->ops = &st_pcie_host_ops;
> +
> +	/* mem regions */
> +	pcie->lmi = platform_get_resource_byname(pdev, IORESOURCE_MEM, "mem-window");
> +	if (!pcie->lmi)
> +		return -ENXIO;
> +
> +	/* regmap registers for PCIe IP configuration */
> +	pcie->regmap = syscon_regmap_lookup_by_phandle(np, "st,syscfg");
> +	if (IS_ERR(pcie->regmap))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(pcie->regmap),
> +				     "No syscfg phandle specified\n");
> +
> +	ret = of_property_read_u32_index(np, "st,syscfg", SYSCFG0_REG, &pcie->syscfg0);
> +	if (ret) {
> +		dev_err(&pdev->dev, "can't get syscfg0 offset (%d)\n", ret);
> +		return ret;
> +	}
> +
> +	ret = of_property_read_u32_index(np, "st,syscfg", SYSCFG1_REG, &pcie->syscfg1);
> +	if (ret) {
> +		dev_err(&pdev->dev, "can't get syscfg1 offset (%d)\n", ret);
> +		return ret;
> +	}
> +
> +	/* powerdown / resets */
> +	pcie->pwr = devm_reset_control_get_optional(&pdev->dev, "powerdown");
> +	if (IS_ERR(pcie->pwr))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(pcie->pwr),
> +				     "Error getting powerdown reset control\n");
> +
> +	pcie->rst = devm_reset_control_get(&pdev->dev, "softreset");
> +	if (IS_ERR(pcie->rst))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(pcie->rst),
> +				     "Error getting softreset control\n");
> +
> +	/* phy */
> +	pcie->phy = devm_phy_get(&pdev->dev, "pcie");
> +	if (IS_ERR(pcie->phy))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(pcie->phy), "no PHY configured\n");
> +
> +	/* Claim the GPIO for PRST# if available, keep it de-asserted */
> +	pcie->reset_gpio = devm_gpiod_get_optional(&pdev->dev, "reset", GPIOD_OUT_LOW);
> +	if (IS_ERR(pcie->reset_gpio))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(pcie->reset_gpio),
> +				     "Cannot request reset-gpios\n");
> +
> +	platform_set_drvdata(pdev, pcie);
> +
> +	ret = dw_pcie_host_init(pp);
> +	if (ret)
> +		return dev_err_probe(&pdev->dev, ret, "failed to initialize host\n");
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id st_pcie_of_match[] = {
> +	{ .compatible = "st,stih407-pcie", },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, st_pcie_of_match);
> +
> +static struct platform_driver st_pcie_driver __refdata = {
> +	.probe  = st_pcie_probe,
> +	.driver = {
> +		.name = "st-pcie",
> +		.of_match_table = st_pcie_of_match,
> +	},
> +};
> +module_platform_driver(st_pcie_driver);
> +
> +MODULE_AUTHOR("Fabrice Gasnier <fabrice.gasnier@foss.st.com>");
> +MODULE_AUTHOR("Alain Volmat <avolmat@me.com>");
> +MODULE_DESCRIPTION("STi PCIe Controller driver");
> +MODULE_LICENSE("GPL v2");

Re: [PATCH RESEND v2 3/5] MAINTAINERS: add entry for ST STI PCIE driver

[Patrice CHOTARD]

Hi Alain

On 1/3/22 08:47, Alain Volmat wrote:
> Add PCIE Driver entry for STI family from ST Microelectronics.
> 
> Signed-off-by: Alain Volmat <avolmat@me.com>
> ---
>  MAINTAINERS | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 360e9aa0205d..081ccdfbd89c 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -14666,6 +14666,12 @@ L:	linux-samsung-soc@vger.kernel.org
>  S:	Maintained
>  F:	drivers/pci/controller/dwc/pci-exynos.c
>  
> +PCI DRIVER FOR ST STI PLATFORM
> +M:	Alain Volmat <avolmat@me.com>
> +L:	linux-pci@vger.kernel.org
> +S:	Maintained
> +F:	drivers/pci/controller/dwc/pcie-sti.c
> +
>  PCI DRIVER FOR SYNOPSYS DESIGNWARE
>  M:	Jingoo Han <jingoohan1@gmail.com>
>  M:	Gustavo Pimentel <gustavo.pimentel@synopsys.com>

Reviewed-by: Patrice Chotard <patrice.chotard@foss.st.com>
Thanks
Patrice

Re: [PATCH RESEND v2 4/5] ARM: dts: sti: add the PCIe controller node within stih407-family

[Patrice CHOTARD]

Hi Alain

On 1/3/22 08:47, Alain Volmat wrote:
> Add the pcie1 entry within stih407-family dtsi.
> 
> Signed-off-by: Alain Volmat <avolmat@me.com>
> ---
>  arch/arm/boot/dts/stih407-family.dtsi | 40 +++++++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/stih407-family.dtsi b/arch/arm/boot/dts/stih407-family.dtsi
> index 21f3347a91d6..fe4ea2d5b583 100644
> --- a/arch/arm/boot/dts/stih407-family.dtsi
> +++ b/arch/arm/boot/dts/stih407-family.dtsi
> @@ -631,6 +631,46 @@ spifsm: spifsm@9022000{
>  			status = "disabled";
>  		};
>  
> +		pcie1: pcie@9b10000 {
> +			compatible = "st,stih407-pcie";
> +			device_type = "pci";
> +			reg = <0x09b10000 0x00001000>,	/* cntrl registers */
> +			      <0x3fff0000 0x00010000>,	/* config space */
> +			      <0x40000000 0xc0000000>;	/* lmi mem window */
> +
> +			reg-names = "dbi",
> +				    "config",
> +				    "mem-window";
> +
> +			st,syscfg = <&syscfg_core 0xdc 0xe4>;
> +
> +			#interrupt-cells = <1>;
> +			interrupts = <GIC_SPI 175 IRQ_TYPE_LEVEL_HIGH>;
> +			interrupt-names = "msi";
> +			interrupt-map-mask = <0 0 0 7>;
> +			interrupt-map = <0 0 0 1 &intc GIC_SPI 171 IRQ_TYPE_LEVEL_HIGH>,
> +					<0 0 0 2 &intc GIC_SPI 172 IRQ_TYPE_LEVEL_HIGH>,
> +					<0 0 0 3 &intc GIC_SPI 173 IRQ_TYPE_LEVEL_HIGH>,
> +					<0 0 0 4 &intc GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
> +
> +			#address-cells = <3>;
> +			#size-cells = <2>;
> +			/* non-prefetchable and prefetchable */
> +			ranges = <0x82000000 0 0x30000000 0x30000000 0 0x05550000>,
> +				 <0xc2000000 0 0x35550000 0x35550000 0 0x0AAA0000>;
> +			bus-range = <0x00 0xff>;
> +
> +			resets = <&softreset STIH407_PCIE1_SOFTRESET>,
> +				 <&powerdown STIH407_PCIE1_POWERDOWN>;
> +
> +			reset-names = "softreset", "powerdown";
> +
> +			phys = <&phy_port1 PHY_TYPE_PCIE>;
> +			phy-names = "pcie";
> +
> +			status = "disabled";
> +		};
> +
>  		sata0: sata@9b20000 {
>  			compatible = "st,ahci";
>  			reg = <0x9b20000 0x1000>;
Reviewed-by: Patrice Chotard <patrice.chotard@foss.st.com>

Thanks
Patrice

Re: [PATCH RESEND v2 5/5] ARM: dts: sti: enable PCIe on the stih418-b2264 board

[Patrice CHOTARD]

Hi Alain

On 1/3/22 08:47, Alain Volmat wrote:
> Enable the PCIe controller with proper reset gpio pin for this board.
> 
> Signed-off-by: Alain Volmat <avolmat@me.com>
> ---
>  arch/arm/boot/dts/stih418-b2264.dts | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/stih418-b2264.dts b/arch/arm/boot/dts/stih418-b2264.dts
> index a99604bebf8c..ed183292a669 100644
> --- a/arch/arm/boot/dts/stih418-b2264.dts
> +++ b/arch/arm/boot/dts/stih418-b2264.dts
> @@ -130,6 +130,11 @@ &ohci1 {
>  	status = "okay";
>  };
>  
> +&pcie1 {
> +	reset-gpios = <&pio34 5 GPIO_ACTIVE_LOW>;
> +	status = "okay";
> +};
> +
>  &pwm1 {
>  	status = "okay";
>  };
Reviewed-by: Patrice Chotard <patrice.chotard@foss.st.com>

Thanks
Patrice

Re: [PATCH RESEND v2 1/5] dt-bindings: pci: st-pcie: PCIe controller found on STi platforms

[Bjorn Helgaas]

On Mon, Jan 03, 2022 at 08:47:27AM +0100, Alain Volmat wrote:
> Addition of the bindings for the Designware based PCIe controller
> that can be found on STi platforms such as STiH407, STiH410 or STiH418.
> 
> Signed-off-by: Alain Volmat <avolmat@me.com>
> ---
>  .../devicetree/bindings/pci/snps,dw-pcie.yaml |   2 +-
>  .../devicetree/bindings/pci/st,st-pcie.yaml   | 112 ++++++++++++++++++
>  2 files changed, 113 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/devicetree/bindings/pci/st,st-pcie.yaml
> 
> diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
> index 9ed0dfba7f89..3a92078128f7 100644
> --- a/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
> @@ -35,7 +35,7 @@ properties:
>      maxItems: 5
>      items:
>        enum: [ dbi, dbi2, config, atu, app, elbi, mgmt, ctrl, parf, cfg, link,
> -              ulreg, smu, mpu, apb, phy ]
> +              ulreg, smu, mpu, apb, phy, mem-window ]
>  
>    num-lanes:
>      description: |
> diff --git a/Documentation/devicetree/bindings/pci/st,st-pcie.yaml b/Documentation/devicetree/bindings/pci/st,st-pcie.yaml
> new file mode 100644
> index 000000000000..2fa686d573c3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/st,st-pcie.yaml
> @@ -0,0 +1,112 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pci/st,st-pcie.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: PCIe RC controller on ST STi platform
> +
> +maintainers:
> +  - Alain Volmat <avolmat@me.com>
> +
> +allOf:
> +  - $ref: /schemas/pci/snps,dw-pcie.yaml#
> +
> +properties:
> +  compatible:
> +    const: st,stih407-pcie
> +
> +  reg:
> +    items:
> +      - description: Controller control and status registers.
> +      - description: PCIe configuration registers.
> +      - description: Memory made available to the controller
> +
> +  reg-names:
> +    items:
> +      - const: dbi
> +      - const: config
> +      - const: mem-window

I guess "mem-window" is the range for inbound DMA?  Other controllers
should have similar ranges.  Can we use the same DT reg names used by
other controllers instead of adding something new and STi-specific?

> +  interrupts:
> +    maxItems: 1
> +
> +  resets:
> +    items:
> +    - description: Controller reset
> +    - description: Powerdown reset (optional)
> +    minItems: 1
> +
> +  reset-names:
> +    items:
> +      - const: softreset
> +      - const: powerdown

"softreset" and "powerdown" appear only in this binding.  Are they
really unique to STi, or can we use the same names used for other
controllers?

Re: [PATCH RESEND v2 2/5] pci: dwc: pcie-st: Add PCIe driver for STi platforms

[Bjorn Helgaas]

On Mon, Jan 03, 2022 at 08:47:28AM +0100, Alain Volmat wrote:
> Addition of the PCIe driver (supporting RC) for controllers
> found on some STi platforms such as STiH407, STiH410 or
> STiH418.
> The controller is based on the designware PCIe controller.

Please follow the subject and commit log conventions in
drivers/pci/controller.  Update capitalization, use imperative mood,
blank lines between paragraphs, etc.  Wrap to fill 75 columns in the
commit log.

Follow OEM usage ("DesignWare"), also below in Kconfig help text and
comments.

In commit logs, comments, etc, use "PCIe" consistently, not "PCI
express" or "pcie".  Similarly, use "PHY" instead of "phy"; "GPIO"
instead of "gpio".

> Signed-off-by: Alain Volmat <avolmat@me.com>
> ---
> v2: update of the pcie-st driver to add possibility to build as module
>     and remove the __init of the probe
> 
>  drivers/pci/controller/dwc/Kconfig    |  11 +
>  drivers/pci/controller/dwc/Makefile   |   1 +
>  drivers/pci/controller/dwc/pcie-sti.c | 386 ++++++++++++++++++++++++++
>  3 files changed, 398 insertions(+)
>  create mode 100644 drivers/pci/controller/dwc/pcie-sti.c
> 
> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> index 62ce3abf0f19..232b8ffd54af 100644
> --- a/drivers/pci/controller/dwc/Kconfig
> +++ b/drivers/pci/controller/dwc/Kconfig
> @@ -384,4 +384,15 @@ config PCIE_FU740
>  	  Say Y here if you want PCIe controller support for the SiFive
>  	  FU740.
>  
> +config PCIE_STI
> +	tristate "STMicroelectronics PCIe Controller for STi SoCs"
> +	depends on ARCH_STI || COMPILE_TEST
> +	depends on OF && PCI_MSI_IRQ_DOMAIN
> +	select PCIE_DW_HOST
> +	help
> +	  Enable PCIe controller support on STMicroelectronics STi SoCs.
> +	  This controller 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 8ba7b67f5e50..c98fa18714ce 100644
> --- a/drivers/pci/controller/dwc/Makefile
> +++ b/drivers/pci/controller/dwc/Makefile
> @@ -21,6 +21,7 @@ obj-$(CONFIG_PCIE_KEEMBAY) += pcie-keembay.o
>  obj-$(CONFIG_PCIE_KIRIN) += pcie-kirin.o
>  obj-$(CONFIG_PCIE_HISI_STB) += pcie-histb.o
>  obj-$(CONFIG_PCI_MESON) += pci-meson.o
> +obj-$(CONFIG_PCIE_STI) += pcie-sti.o
>  obj-$(CONFIG_PCIE_TEGRA194) += pcie-tegra194.o
>  obj-$(CONFIG_PCIE_UNIPHIER) += pcie-uniphier.o
>  obj-$(CONFIG_PCIE_UNIPHIER_EP) += pcie-uniphier-ep.o
> diff --git a/drivers/pci/controller/dwc/pcie-sti.c b/drivers/pci/controller/dwc/pcie-sti.c
> new file mode 100644
> index 000000000000..61da00a079ac
> --- /dev/null
> +++ b/drivers/pci/controller/dwc/pcie-sti.c
> @@ -0,0 +1,386 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2021 STMicroelectronics
> + *
> + * STMicroelectronics PCI express Driver for STi SoCs.
> + * ST PCIe IPs are built around a Synopsys IP Core.
> + *
> + * Authors: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
> + *          Alain Volmat <avolmat@me.com>
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/interrupt.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/of_gpio.h>
> +#include <linux/of_pci.h>
> +#include <linux/of_platform.h>
> +#include <linux/phy/phy.h>
> +#include <linux/regmap.h>
> +#include <linux/reset.h>
> +
> +#include "pcie-designware.h"
> +
> +/* RC_ADDRESS_TRANSLATION Registers */
> +#define TRANSLATION_CONTROL		0x900
> +/* Controls if area is inclusive or exclusive */
> +#define RC_PASS_ADDR_RANGE		BIT(1)
> +
> +/* Base of area reserved for config accesses. Fixed size of 64K. */
> +#define CFG_BASE_ADDRESS		0x92c
> +#define CFG_REGION_SIZE			65536
> +#define CFG_SPACE1_OFFSET		0x1000
> +
> +/* First 4K of config space has this BDF (bus,device,function) */
> +#define FUNC0_BDF_NUM			0x930
> +
> +/* Mem regions */
> +#define IN0_MEM_ADDR_START		0x964
> +#define IN0_MEM_ADDR_LIMIT		0x968
> +#define IN1_MEM_ADDR_START		0x974
> +#define IN1_MEM_ADDR_LIMIT		0x978
> +
> +/* syscfg1 bits */
> +#define PCIE_APP_LTSSM_ENABLE		BIT(2)
> +/* syscfg0 bits */
> +#define PCIE_TYPE_ROOT_COMPLEX		BIT(0)
> +
> +/* st,syscfg offsets */
> +#define SYSCFG0_REG	1
> +#define SYSCFG1_REG	2
> +
> +#define to_st_pcie(x)	dev_get_drvdata((x)->dev)
> +
> +/**
> + * struct st_pcie - private data of the controller
> + * @dw: designware pcie
> + * @syscfg0: PCIe conf register setting root complex, regmap offset
> + * @syscfg1: PCIe conf register for PCIE_APP_LTSSM_ENABLE, regmap offset
> + * @phy: associated pcie phy
> + * @lmi: memory made available to the controller
> + * @regmap: Syscfg registers bank in which PCIe port is configured
> + * @pwr: power control
> + * @rst: reset control
> + * @reset_gpio: optional reset gpio
> + * @config_window_start: start address of 64K config space area
> + */
> +struct st_pcie {
> +	struct dw_pcie *dw;
> +	int syscfg0;
> +	int syscfg1;
> +	struct phy *phy;
> +	struct resource	*lmi;
> +	struct regmap *regmap;
> +	struct reset_control *pwr;
> +	struct reset_control *rst;
> +	struct gpio_desc *reset_gpio;
> +	phys_addr_t config_window_start;
> +};
> +
> +/*
> + * The PCI express core IP expects the following arrangement on it's address

s/PCI express/PCIe/
s/it's/its/

> + * bus (slv_haddr) when driving config cycles.
> + * bus_number		[31:24]
> + * dev_number		[23:19]
> + * func_number		[18:16]
> + * unused		[15:12]
> + * ext_reg_number	[11:8]
> + * reg_number		[7:2]
> + *
> + * Bits [15:12] are unused.
> + *
> + * In the glue logic there is a 64K region of address space that can be
> + * written/read to generate config cycles. The base address of this is
> + * controlled by CFG_BASE_ADDRESS. There are 8 16 bit registers called
> + * FUNC0_BDF_NUM to FUNC8_BDF_NUM. These split the bottom half of the 64K
> + * window into 8 regions at 4K boundaries. These control the bus, device and
> + * function number you are trying to talk to.
> + *
> + * The decision on whether to generate a type 0 or type 1 access is controlled
> + * by bits 15:12 of the address you write to.  If they are zero, then a type 0
> + * is generated, if anything else it will be a type 1. Thus the bottom 4K
> + * region controlled by FUNC0_BDF_NUM can only generate type 0, all the others
> + * can only generate type 1.
> + *
> + * We only use FUNC0_BDF_NUM and FUNC1_BDF_NUM. Which one you use is selected
> + * by bit 12 of the address you write to. The selected register is then used
> + * for the top 16 bits of the slv_haddr to form the bus/dev/func, bit 15:12 are
> + * wired to zero, and bits 11:2 form the address of the register you want to
> + * read in config space.
> + *
> + * We always write FUNC0_BDF_NUM as a 32 bit write. So if we want type 1
> + * accesses we have to shift by 16 so in effect we are writing to FUNC1_BDF_NUM
> + */
> +static inline u32 bdf_num(int bus, int devfn, int is_root_bus)
> +{
> +	return ((bus << 8) | devfn) << (is_root_bus ? 0 : 16);
> +}
> +
> +static void __iomem *st_pcie_other_map_bus(struct pci_bus *bus, unsigned int devfn, int where)

Wrap to fit in 80 columns (see other drivers for examples).  Wrap rest
of file similarly.

> +{
> +	struct pcie_port *pp = bus->sysdata;
> +	struct dw_pcie *dw = to_dw_pcie_from_pp(pp);
> +	u32 bdf;
> +
> +	bdf = bdf_num(bus->number, devfn, pci_is_root_bus(bus));
> +
> +	/* Set the config packet devfn */
> +	dw_pcie_writel_dbi(dw, FUNC0_BDF_NUM, bdf);
> +	dw_pcie_readl_dbi(dw, FUNC0_BDF_NUM);
> +
> +	return pp->va_cfg0_base + where + (pci_is_root_bus(bus->parent) ? 0 : CFG_SPACE1_OFFSET);
> +}
> +
> +static struct pci_ops st_child_pcie_ops = {
> +	.map_bus = st_pcie_other_map_bus,
> +	.read = pci_generic_config_read,
> +	.write = pci_generic_config_write,
> +};
> +
> +static void st_pcie_hw_setup(struct dw_pcie *dw)
> +{
> +	struct st_pcie *pcie = to_st_pcie(dw);
> +
> +	/* Set up the config window to the top of the PCI address space */
> +	dw_pcie_writel_dbi(dw, CFG_BASE_ADDRESS, pcie->config_window_start);
> +
> +	/*
> +	 * Open up memory to the PCI controller. We could do slightly
> +	 * better than this and exclude the kernel text segment and bss etc.
> +	 * They are base/limit registers so can be of arbitrary alignment
> +	 * presumably
> +	 */
> +	dw_pcie_writel_dbi(dw, IN0_MEM_ADDR_START, pcie->lmi->start);
> +	dw_pcie_writel_dbi(dw, IN0_MEM_ADDR_LIMIT, pcie->lmi->end);
> +
> +	/* Disable the 2nd region */
> +	dw_pcie_writel_dbi(dw, IN1_MEM_ADDR_START, ~0);
> +	dw_pcie_writel_dbi(dw, IN1_MEM_ADDR_LIMIT, 0);
> +
> +	dw_pcie_writel_dbi(dw, TRANSLATION_CONTROL, RC_PASS_ADDR_RANGE);
> +}
> +
> +static int st_pcie_init(struct pcie_port *pp)
> +{
> +	struct dw_pcie *dw = to_dw_pcie_from_pp(pp);
> +	struct st_pcie *pcie = to_st_pcie(dw);
> +	int ret;
> +
> +	/* Set device type : Root Complex */
> +	ret = regmap_write(pcie->regmap, pcie->syscfg0, PCIE_TYPE_ROOT_COMPLEX);
> +	if (ret < 0) {
> +		dev_err(dw->dev, "unable to set device type\n");
> +		return ret;
> +	}
> +
> +	ret = reset_control_deassert(pcie->pwr);
> +	if (ret) {
> +		dev_err(dw->dev, "unable to bring out of powerdown\n");
> +		return ret;
> +	}
> +
> +	ret = reset_control_deassert(pcie->rst);
> +	if (ret) {
> +		dev_err(dw->dev, "unable to bring out of softreset\n");
> +		return ret;
> +	}
> +
> +	/* let the controller initialize in the proper mode (RC) after reset */

Capitalize comments consistently.  Not sure what this comment is
telling us anyway.  RC mode is determined above.  I assume this delay
an STi-specific thing?  A reference to how this was derived might be a
useful thing, e.g., a section number in the STi spec.

> +	usleep_range(1000, 2000);
> +
> +	return ret;
> +}
> +
> +static int st_pcie_control_ltssm(struct dw_pcie *dw, bool enable)
> +{
> +	struct st_pcie *pcie = to_st_pcie(dw);
> +
> +	return regmap_update_bits(pcie->regmap, pcie->syscfg1, PCIE_APP_LTSSM_ENABLE,
> +				  FIELD_PREP(PCIE_APP_LTSSM_ENABLE, enable));
> +}
> +
> +static int st_pcie_host_init(struct pcie_port *pp)
> +{
> +	struct dw_pcie *dw = to_dw_pcie_from_pp(pp);
> +	struct st_pcie *pcie = to_st_pcie(dw);
> +	int err;
> +
> +	pcie->config_window_start = pp->cfg0_base;
> +
> +	/*
> +	 * "Override" default ops provided by designware driver as STI
> +	 * PCIe uses its own translation unit rather than iATU
> +	 */

No need for the scare quotes around "override".  You are in fact
overriding the ops.

> +	pp->bridge->child_ops = &st_child_pcie_ops;
> +
> +	/*
> +	 * We have to initialise the PCIe cell on some hardware before we can
> +	 * talk to the phy
> +	 */
> +	err = st_pcie_init(pp);
> +	if (err)
> +		return err;
> +
> +	err = st_pcie_control_ltssm(dw, false);
> +	if (err) {
> +		dev_err(dw->dev, "disable ltssm failed, %d\n", err);

s/ltssm/LTSSM/

> +		return err;
> +	}
> +
> +	/* Init the associated miphy */
> +	err = phy_init(pcie->phy);
> +	if (err < 0) {
> +		dev_err(dw->dev, "Cannot init PHY: %d\n", err);
> +		return err;
> +	}
> +
> +	return 0;
> +}
> +
> +static int st_pcie_start_link(struct dw_pcie *dw)
> +{
> +	struct st_pcie *pcie = to_st_pcie(dw);
> +	int err;
> +
> +	/* Do all the register poking */

Pointless comment.

> +	st_pcie_hw_setup(dw);
> +
> +	if (pcie->reset_gpio) {
> +		/* Assert the PERST# signal */
> +		gpiod_set_value(pcie->reset_gpio, 1);
> +
> +		/* PERST# signal must stay asserted for at least 100us (Tperst-clk) */

Thanks for this reference to a specific delay from the spec!

> +		usleep_range(100, 200);
> +		/* Release PERST# signal */
> +		gpiod_set_value(pcie->reset_gpio, 0);
> +	}
> +
> +	/* Re-enable the link, link training must begin shortly after reset */
> +	err = st_pcie_control_ltssm(dw, true);
> +	if (err) {
> +		dev_err(dw->dev, "enable ltssm failed, %d\n", err);
> +		return err;
> +	}
> +
> +	err = dw_pcie_wait_for_link(dw);
> +	if (err) {
> +		dev_err(dw->dev, "wait for link failed, %d\n", err);
> +		return err;
> +	}
> +
> +	/*
> +	 * PCIe specification states that you should not issue any config
> +	 * requests until 100ms after asserting reset, so we enforce that here
> +	 */
> +	if  (pcie->reset_gpio)
> +		msleep(100);
> +
> +	return 0;
> +}
> +
> +static struct dw_pcie_host_ops st_pcie_host_ops = {
> +	.host_init = st_pcie_host_init,
> +};
> +
> +static const struct dw_pcie_ops dw_pcie_ops = {
> +	.start_link = st_pcie_start_link,
> +};
> +
> +static int st_pcie_probe(struct platform_device *pdev)
> +{
> +	struct st_pcie *pcie;
> +	struct dw_pcie *dw;
> +	struct device_node *np = pdev->dev.of_node;
> +	struct pcie_port *pp;
> +	int ret;
> +
> +	pcie = devm_kzalloc(&pdev->dev, sizeof(*pcie), GFP_KERNEL);
> +	if (!pcie)
> +		return -ENOMEM;
> +
> +	dw = devm_kzalloc(&pdev->dev, sizeof(*dw), GFP_KERNEL);
> +	if (!dw)
> +		return -ENOMEM;
> +	pcie->dw = dw;
> +	dw->dev = &pdev->dev;
> +	dw->ops = &dw_pcie_ops;
> +
> +	pp = &dw->pp;
> +	pp->ops = &st_pcie_host_ops;
> +
> +	/* mem regions */
> +	pcie->lmi = platform_get_resource_byname(pdev, IORESOURCE_MEM, "mem-window");
> +	if (!pcie->lmi)
> +		return -ENXIO;
> +
> +	/* regmap registers for PCIe IP configuration */
> +	pcie->regmap = syscon_regmap_lookup_by_phandle(np, "st,syscfg");
> +	if (IS_ERR(pcie->regmap))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(pcie->regmap),
> +				     "No syscfg phandle specified\n");

Make your error messages all capitalized or all uncapitalized.  I
think uncapitalized is more typical in drivers/pci/

> +	ret = of_property_read_u32_index(np, "st,syscfg", SYSCFG0_REG, &pcie->syscfg0);
> +	if (ret) {
> +		dev_err(&pdev->dev, "can't get syscfg0 offset (%d)\n", ret);
> +		return ret;
> +	}
> +
> +	ret = of_property_read_u32_index(np, "st,syscfg", SYSCFG1_REG, &pcie->syscfg1);
> +	if (ret) {
> +		dev_err(&pdev->dev, "can't get syscfg1 offset (%d)\n", ret);
> +		return ret;
> +	}
> +
> +	/* powerdown / resets */
> +	pcie->pwr = devm_reset_control_get_optional(&pdev->dev, "powerdown");
> +	if (IS_ERR(pcie->pwr))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(pcie->pwr),
> +				     "Error getting powerdown reset control\n");
> +
> +	pcie->rst = devm_reset_control_get(&pdev->dev, "softreset");
> +	if (IS_ERR(pcie->rst))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(pcie->rst),
> +				     "Error getting softreset control\n");
> +
> +	/* phy */
> +	pcie->phy = devm_phy_get(&pdev->dev, "pcie");
> +	if (IS_ERR(pcie->phy))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(pcie->phy), "no PHY configured\n");
> +
> +	/* Claim the GPIO for PRST# if available, keep it de-asserted */
> +	pcie->reset_gpio = devm_gpiod_get_optional(&pdev->dev, "reset", GPIOD_OUT_LOW);
> +	if (IS_ERR(pcie->reset_gpio))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(pcie->reset_gpio),
> +				     "Cannot request reset-gpios\n");
> +
> +	platform_set_drvdata(pdev, pcie);
> +
> +	ret = dw_pcie_host_init(pp);
> +	if (ret)
> +		return dev_err_probe(&pdev->dev, ret, "failed to initialize host\n");
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id st_pcie_of_match[] = {
> +	{ .compatible = "st,stih407-pcie", },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, st_pcie_of_match);
> +
> +static struct platform_driver st_pcie_driver __refdata = {
> +	.probe  = st_pcie_probe,
> +	.driver = {
> +		.name = "st-pcie",
> +		.of_match_table = st_pcie_of_match,
> +	},
> +};
> +module_platform_driver(st_pcie_driver);
> +
> +MODULE_AUTHOR("Fabrice Gasnier <fabrice.gasnier@foss.st.com>");
> +MODULE_AUTHOR("Alain Volmat <avolmat@me.com>");
> +MODULE_DESCRIPTION("STi PCIe Controller driver");
> +MODULE_LICENSE("GPL v2");
> -- 
> 2.25.1
>