[PATCH v7 0/3] PCI: Add Intel PCIe Driver and respective dt-binding yaml file

[PATCH v7 0/3] PCI: Add Intel PCIe Driver and respective dt-binding yaml file

[Dilip Kota]

Intel PCIe is Synopsys based controller. Intel PCIe driver uses
DesignWare PCIe framework for host initialization and register
configurations.

Dilip Kota (3):
  dt-bindings: PCI: intel: Add YAML schemas for the PCIe RC controller
  dwc: PCI: intel: PCIe RC controller driver
  PCI: artpec6: Configure FTS with dwc helper function

 .../devicetree/bindings/pci/intel-gw-pcie.yaml     | 138 ++++++
 drivers/pci/controller/dwc/Kconfig                 |  10 +
 drivers/pci/controller/dwc/Makefile                |   1 +
 drivers/pci/controller/dwc/pcie-artpec6.c          |   8 +-
 drivers/pci/controller/dwc/pcie-designware.c       |  57 +++
 drivers/pci/controller/dwc/pcie-designware.h       |  12 +
 drivers/pci/controller/dwc/pcie-intel-gw.c         | 542 +++++++++++++++++++++
 include/uapi/linux/pci_regs.h                      |   1 +
 8 files changed, 762 insertions(+), 7 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pci/intel-gw-pcie.yaml
 create mode 100644 drivers/pci/controller/dwc/pcie-intel-gw.c

-- 
2.11.0

[PATCH v7 1/3] dt-bindings: PCI: intel: Add YAML schemas for the PCIe RC controller

[Dilip Kota]

Add YAML schemas for PCIe RC controller on Intel Gateway SoCs
which is Synopsys DesignWare based PCIe core.

Signed-off-by: Dilip Kota <eswara.kota@linux.intel.com>
Reviewed-by: Andrew Murray <andrew.murray@arm.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
Changes on v7:
	No change

Changes on v6:
	Add Reviewed-by: Rob Herring <robh@kernel.org>

Changes on v5:
	Add Reviewed-by Andrew Murray.
	Add possible values and default value for max-link-speed.
	Remove $ref and add maximum and default for reset-assert-ms.
	Set true flag for linux,pci-domain.
	Define maxItems for ranges and clock.
	Define maximum for num-lanes.
	Update required list:
	  Add #address-cells, #size-cells, #interrupt-cells.
	  Remove num-lanes and linux,pci-domain.
	Add required header files in example.
	Remove status entry in example.

changes on v4:
	Add "snps,dw-pcie" compatible.
	Rename phy-names property value to pcie.
	And maximum and minimum values to num-lanes.
	Add ref for reset-assert-ms entry and update the
	 description for easy understanding.
	Remove PCIe core interrupt entry.

changes on v3:
        Add the appropriate License-Identifier
        Rename intel,rst-interval to 'reset-assert-us'
        Add additionalProperties: false
        Rename phy-names to 'pciephy'
        Remove the dtsi node split of SoC and board in the example
        Add #interrupt-cells = <1>; or else interrupt parsing will fail
        Name yaml file with compatible name

 .../devicetree/bindings/pci/intel-gw-pcie.yaml     | 138 +++++++++++++++++++++
 1 file changed, 138 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/intel-gw-pcie.yaml

diff --git a/Documentation/devicetree/bindings/pci/intel-gw-pcie.yaml b/Documentation/devicetree/bindings/pci/intel-gw-pcie.yaml
new file mode 100644
index 000000000000..db605d8a387d
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/intel-gw-pcie.yaml
@@ -0,0 +1,138 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pci/intel-gw-pcie.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: PCIe RC controller on Intel Gateway SoCs
+
+maintainers:
+  - Dilip Kota <eswara.kota@linux.intel.com>
+
+properties:
+  compatible:
+    items:
+      - const: intel,lgm-pcie
+      - const: snps,dw-pcie
+
+  device_type:
+    const: pci
+
+  "#address-cells":
+    const: 3
+
+  "#size-cells":
+    const: 2
+
+  reg:
+    items:
+      - description: Controller control and status registers.
+      - description: PCIe configuration registers.
+      - description: Controller application registers.
+
+  reg-names:
+    items:
+      - const: dbi
+      - const: config
+      - const: app
+
+  ranges:
+    maxItems: 1
+
+  resets:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  phys:
+    maxItems: 1
+
+  phy-names:
+    const: pcie
+
+  reset-gpios:
+    maxItems: 1
+
+  linux,pci-domain: true
+
+  num-lanes:
+    maximum: 2
+    description: Number of lanes to use for this port.
+
+  '#interrupt-cells':
+    const: 1
+
+  interrupt-map-mask:
+    description: Standard PCI IRQ mapping properties.
+
+  interrupt-map:
+    description: Standard PCI IRQ mapping properties.
+
+  max-link-speed:
+    description: Specify PCI Gen for link capability.
+    allOf:
+      - $ref: /schemas/types.yaml#/definitions/uint32
+      - enum: [ 1, 2, 3, 4 ]
+      - default: 1
+
+  bus-range:
+    description: Range of bus numbers associated with this controller.
+
+  reset-assert-ms:
+    description: |
+      Delay after asserting reset to the PCIe device.
+    maximum: 500
+    default: 100
+
+required:
+  - compatible
+  - device_type
+  - "#address-cells"
+  - "#size-cells"
+  - reg
+  - reg-names
+  - ranges
+  - resets
+  - clocks
+  - phys
+  - phy-names
+  - reset-gpios
+  - '#interrupt-cells'
+  - interrupt-map
+  - interrupt-map-mask
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/clock/intel,lgm-clk.h>
+    pcie10: pcie@d0e00000 {
+      compatible = "intel,lgm-pcie", "snps,dw-pcie";
+      device_type = "pci";
+      #address-cells = <3>;
+      #size-cells = <2>;
+      reg = <0xd0e00000 0x1000>,
+            <0xd2000000 0x800000>,
+            <0xd0a41000 0x1000>;
+      reg-names = "dbi", "config", "app";
+      linux,pci-domain = <0>;
+      max-link-speed = <4>;
+      bus-range = <0x00 0x08>;
+      interrupt-parent = <&ioapic1>;
+      #interrupt-cells = <1>;
+      interrupt-map-mask = <0 0 0 0x7>;
+      interrupt-map = <0 0 0 1 &ioapic1 27 1>,
+                      <0 0 0 2 &ioapic1 28 1>,
+                      <0 0 0 3 &ioapic1 29 1>,
+                      <0 0 0 4 &ioapic1 30 1>;
+      ranges = <0x02000000 0 0xd4000000 0xd4000000 0 0x04000000>;
+      resets = <&rcu0 0x50 0>;
+      clocks = <&cgu0 LGM_GCLK_PCIE10>;
+      phys = <&cb0phy0>;
+      phy-names = "pcie";
+      reset-assert-ms = <500>;
+      reset-gpios = <&gpio0 3 GPIO_ACTIVE_LOW>;
+      num-lanes = <2>;
+    };
-- 
2.11.0

[PATCH v7 2/3] dwc: PCI: intel: PCIe RC controller driver

[Dilip Kota]

Add support to PCIe RC controller on Intel Gateway SoCs.
PCIe controller is based of Synopsys DesignWare PCIe core.

Intel PCIe driver requires Upconfigure support, Fast Training
Sequence and link speed configurations. So adding the respective
helper functions in the PCIe DesignWare framework.
It also programs hardware autonomous speed during speed
configuration so defining it in pci_regs.h.

Signed-off-by: Dilip Kota <eswara.kota@linux.intel.com>
Reviewed-by: Andrew Murray <andrew.murray@arm.com>
Acked-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
---
Changes on v7:
	Add Acked-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
	Remove unused header files.
	  #include <linux/interrupt.h>
	  #include <linux/of_irq.h>
	  #include <linux/of_pci.h>
	  #include <linux/of_platform.h>

Changes on v6:
	Add Reviewed-by: Andrew Murray <andrew.murray@arm.com>
	Address review comments:
	  Remove extra space in Kconfig
	  Fix style indents in switch case
	  Apply mask for the n_fts argument in dw_pcie_link_set_n_fts()
	   to prevent the case of n_fts value having outside the mask bits
	  Update PORT_LOGIC_N_FTS to PORT_LOGIC_N_FTS_MASK
	  Update BUS_IATU_OFFS to BUS_IATU_OFFSET
	  Update RST_INTRVL_DFT_MS to RESET_INTERVAL_MS
	  Reduce local variables in pcie_update_bits()
	  Remove blank lines
	  Remove extra paranthesis during readl_poll_timeout() call
	  Add error handling for dw_pcie_find_capability() call
	  Add blank line before dev_info() in probe()

Changes on v5:
	Use new, old variables in pcie_update_bits()
	Correct naming:
	  s/Designware/DesignWare
	  s/pcie/PCIe
	  s/pci/PCI
	  s/Hw/HW
	  Upconfig to Upconfigure
	Remove extra lines after the intel_pcie_max_speed_setup() def.
	Use pcie_link_speed[] and remove enum for pcie gen.
	Remove dw_pcie_link_speed_change() def.
	Remove "linux,pci-domain" parsing.
	Remove 'id' variable in intel_pcie_port structure.
	Remove extra spaces for lpp->link_gen =
	Correct the offset of PCI_EXP_LNKCTL2_HASD to 0x0020.
	Remove programming of RCB and CCC bits in PCI_EXP_LCTL reg.
	Remove programming Slot clock cfg in PCI_EXP_LSTS reg.
	Update the comments at num_viewport setting.
	Update the description in Kconfig.
	Get PCIe cap offset from the registers using dw_pcie_find_capability()
	Define pcie_cap_ofst var in intel_pcie_port struct to store the same.
	Remove the PCIE_CAP_OFST macro.
	Move intel_pcie_max_speed_setup() function to DesignWare framework,
	 defined as dw_pcie_link_set_max_speed()
	Set EXPORT_SYMBOL_GPL for the newer functions defined in
	  pcie-designware.c

Changes on v4:
	Rename the driver naming and description to
	 "PCIe RC controller on Intel Gateway SoCs".
	Use PCIe core register macros defined in pci_regs.h
	 and remove respective local definitions.
	Remove PCIe core interrupt handling.
	Move PCIe link control speed change, upconfig and FTS.
	configuration functions to DesignWare framework.
	Use of_pci_get_max_link_speed().
	Mark dependency on X86 and COMPILE_TEST in Kconfig.
	Remove lanes and add n_fts variables in intel_pcie_port structure.
	Rename rst_interval variable to rst_intrvl in intel_pcie_port structure.
	Remove intel_pcie_mem_iatu() as it is already perfomed in dw_setup_rc()
	Move sysfs attributes specific code to separate patch.
	Remove redundant error handling.
	Reduce LoCs by doing variable initializations while declaration itself.
	Add extra line after closing parenthesis.
	Move intel_pcie_ep_rst_init() out of get_resources()

changes on v3:
	Rename PCIe app logic registers with PCIE_APP prefix.
	PCIE_IOP_CTRL configuration is not required. Remove respective code.
	Remove wrapper functions for clk enable/disable APIs.
	Use platform_get_resource_byname() instead of
	  devm_platform_ioremap_resource() to be similar with DWC framework.
	Rename phy name to "pciephy".
	Modify debug message in msi_init() callback to be more specific.
	Remove map_irq() callback.
	Enable the INTx interrupts at the time of PCIe initialization.
	Reduce memory fragmentation by using variable "struct dw_pcie pci"
	  instead of allocating memory.
	Reduce the delay to 100us during enpoint initialization
	  intel_pcie_ep_rst_init().
	Call  dw_pcie_host_deinit() during remove() instead of directly
	  calling PCIe core APIs.
	Rename "intel,rst-interval" to "reset-assert-ms".
	Remove unused APP logic Interrupt bit macro definitions.
 	Use dwc framework's dw_pcie_setup_rc() for PCIe host specific
	 configuration instead of redefining the same functionality in
	 the driver.
	Move the whole DT parsing specific code to intel_pcie_get_resources()

 drivers/pci/controller/dwc/Kconfig           |  10 +
 drivers/pci/controller/dwc/Makefile          |   1 +
 drivers/pci/controller/dwc/pcie-designware.c |  57 +++
 drivers/pci/controller/dwc/pcie-designware.h |  12 +
 drivers/pci/controller/dwc/pcie-intel-gw.c   | 542 +++++++++++++++++++++++++++
 include/uapi/linux/pci_regs.h                |   1 +
 6 files changed, 623 insertions(+)
 create mode 100644 drivers/pci/controller/dwc/pcie-intel-gw.c

diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
index 0ba988b5b5bc..fb6d474477df 100644
--- a/drivers/pci/controller/dwc/Kconfig
+++ b/drivers/pci/controller/dwc/Kconfig
@@ -82,6 +82,16 @@ config PCIE_DW_PLAT_EP
 	  order to enable device-specific features PCI_DW_PLAT_EP must be
 	  selected.
 
+config PCIE_INTEL_GW
+	bool "Intel Gateway PCIe host controller support"
+	depends on OF && (X86 || COMPILE_TEST)
+	select PCIE_DW_HOST
+	help
+	  Say 'Y' here to enable PCIe Host controller support on Intel
+	  Gateway SoCs.
+	  The PCIe controller uses the DesignWare core plus Intel-specific
+	  hardware wrappers.
+
 config PCI_EXYNOS
 	bool "Samsung Exynos PCIe controller"
 	depends on SOC_EXYNOS5440 || COMPILE_TEST
diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
index b30336181d46..99db83cd2f35 100644
--- a/drivers/pci/controller/dwc/Makefile
+++ b/drivers/pci/controller/dwc/Makefile
@@ -3,6 +3,7 @@ obj-$(CONFIG_PCIE_DW) += pcie-designware.o
 obj-$(CONFIG_PCIE_DW_HOST) += pcie-designware-host.o
 obj-$(CONFIG_PCIE_DW_EP) += pcie-designware-ep.o
 obj-$(CONFIG_PCIE_DW_PLAT) += pcie-designware-plat.o
+obj-$(CONFIG_PCIE_INTEL_GW) += pcie-intel-gw.o
 obj-$(CONFIG_PCI_DRA7XX) += pci-dra7xx.o
 obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
 obj-$(CONFIG_PCI_IMX6) += pci-imx6.o
diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index 820488dfeaed..479e250695a0 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -14,6 +14,8 @@
 
 #include "pcie-designware.h"
 
+extern const unsigned char pcie_link_speed[];
+
 /*
  * These interfaces resemble the pci_find_*capability() interfaces, but these
  * are for configuring host controllers, which are bridges *to* PCI devices but
@@ -474,6 +476,61 @@ int dw_pcie_link_up(struct dw_pcie *pci)
 		(!(val & PCIE_PORT_DEBUG1_LINK_IN_TRAINING)));
 }
 
+void dw_pcie_upconfig_setup(struct dw_pcie *pci)
+{
+	u32 val;
+
+	val = dw_pcie_readl_dbi(pci, PCIE_PORT_MULTI_LANE_CTRL);
+	val |= PORT_MLTI_UPCFG_SUPPORT;
+	dw_pcie_writel_dbi(pci, PCIE_PORT_MULTI_LANE_CTRL, val);
+}
+EXPORT_SYMBOL_GPL(dw_pcie_upconfig_setup);
+
+void dw_pcie_link_set_max_speed(struct dw_pcie *pci, u32 link_gen)
+{
+	u32 reg, val;
+	u8 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
+
+	reg = dw_pcie_readl_dbi(pci, offset + PCI_EXP_LNKCTL2);
+	reg &= ~PCI_EXP_LNKCTL2_TLS;
+
+	switch (pcie_link_speed[link_gen]) {
+	case PCIE_SPEED_2_5GT:
+		reg |= PCI_EXP_LNKCTL2_TLS_2_5GT;
+		break;
+	case PCIE_SPEED_5_0GT:
+		reg |= PCI_EXP_LNKCTL2_TLS_5_0GT;
+		break;
+	case PCIE_SPEED_8_0GT:
+		reg |= PCI_EXP_LNKCTL2_TLS_8_0GT;
+		break;
+	case PCIE_SPEED_16_0GT:
+		reg |= PCI_EXP_LNKCTL2_TLS_16_0GT;
+		break;
+	default:
+		/* Use hardware capability */
+		val = dw_pcie_readl_dbi(pci, offset + PCI_EXP_LNKCAP);
+		val = FIELD_GET(PCI_EXP_LNKCAP_SLS, val);
+		reg &= ~PCI_EXP_LNKCTL2_HASD;
+		reg |= FIELD_PREP(PCI_EXP_LNKCTL2_TLS, val);
+		break;
+	}
+
+	dw_pcie_writel_dbi(pci, offset + PCI_EXP_LNKCTL2, reg);
+}
+EXPORT_SYMBOL_GPL(dw_pcie_link_set_max_speed);
+
+void dw_pcie_link_set_n_fts(struct dw_pcie *pci, u32 n_fts)
+{
+	u32 val;
+
+	val = dw_pcie_readl_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL);
+	val &= ~PORT_LOGIC_N_FTS_MASK;
+	val |= n_fts & PORT_LOGIC_N_FTS_MASK;
+	dw_pcie_writel_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL, val);
+}
+EXPORT_SYMBOL_GPL(dw_pcie_link_set_n_fts);
+
 static u8 dw_pcie_iatu_unroll_enabled(struct dw_pcie *pci)
 {
 	u32 val;
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 5a18e94e52c8..3b0be5cd9235 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -30,7 +30,12 @@
 #define LINK_WAIT_IATU			9
 
 /* Synopsys-specific PCIe configuration registers */
+#define PCIE_PORT_AFR			0x70C
+#define PORT_AFR_N_FTS_MASK		GENMASK(15, 8)
+#define PORT_AFR_CC_N_FTS_MASK		GENMASK(23, 16)
+
 #define PCIE_PORT_LINK_CONTROL		0x710
+#define PORT_LINK_DLL_LINK_EN		BIT(5)
 #define PORT_LINK_MODE_MASK		GENMASK(21, 16)
 #define PORT_LINK_MODE(n)		FIELD_PREP(PORT_LINK_MODE_MASK, n)
 #define PORT_LINK_MODE_1_LANES		PORT_LINK_MODE(0x1)
@@ -46,6 +51,7 @@
 #define PCIE_PORT_DEBUG1_LINK_IN_TRAINING	BIT(29)
 
 #define PCIE_LINK_WIDTH_SPEED_CONTROL	0x80C
+#define PORT_LOGIC_N_FTS_MASK		GENMASK(7, 0)
 #define PORT_LOGIC_SPEED_CHANGE		BIT(17)
 #define PORT_LOGIC_LINK_WIDTH_MASK	GENMASK(12, 8)
 #define PORT_LOGIC_LINK_WIDTH(n)	FIELD_PREP(PORT_LOGIC_LINK_WIDTH_MASK, n)
@@ -60,6 +66,9 @@
 #define PCIE_MSI_INTR0_MASK		0x82C
 #define PCIE_MSI_INTR0_STATUS		0x830
 
+#define PCIE_PORT_MULTI_LANE_CTRL	0x8C0
+#define PORT_MLTI_UPCFG_SUPPORT		BIT(7)
+
 #define PCIE_ATU_VIEWPORT		0x900
 #define PCIE_ATU_REGION_INBOUND		BIT(31)
 #define PCIE_ATU_REGION_OUTBOUND	0
@@ -273,6 +282,9 @@ void dw_pcie_write_dbi2(struct dw_pcie *pci, u32 reg, size_t size, u32 val);
 u32 dw_pcie_read_atu(struct dw_pcie *pci, u32 reg, size_t size);
 void dw_pcie_write_atu(struct dw_pcie *pci, u32 reg, size_t size, u32 val);
 int dw_pcie_link_up(struct dw_pcie *pci);
+void dw_pcie_upconfig_setup(struct dw_pcie *pci);
+void dw_pcie_link_set_max_speed(struct dw_pcie *pci, u32 link_gen);
+void dw_pcie_link_set_n_fts(struct dw_pcie *pci, u32 n_fts);
 int dw_pcie_wait_for_link(struct dw_pcie *pci);
 void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index,
 			       int type, u64 cpu_addr, u64 pci_addr,
diff --git a/drivers/pci/controller/dwc/pcie-intel-gw.c b/drivers/pci/controller/dwc/pcie-intel-gw.c
new file mode 100644
index 000000000000..cd000cd5bed7
--- /dev/null
+++ b/drivers/pci/controller/dwc/pcie-intel-gw.c
@@ -0,0 +1,542 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * PCIe host controller driver for Intel Gateway SoCs
+ *
+ * Copyright (c) 2019 Intel Corporation.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/gpio/consumer.h>
+#include <linux/iopoll.h>
+#include <linux/pci_regs.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/reset.h>
+
+#include "../../pci.h"
+#include "pcie-designware.h"
+
+#define PORT_AFR_N_FTS_GEN12_DFT	(SZ_128 - 1)
+#define PORT_AFR_N_FTS_GEN3		180
+#define PORT_AFR_N_FTS_GEN4		196
+
+/* PCIe Application logic Registers */
+#define PCIE_APP_CCR			0x10
+#define PCIE_APP_CCR_LTSSM_ENABLE	BIT(0)
+
+#define PCIE_APP_MSG_CR			0x30
+#define PCIE_APP_MSG_XMT_PM_TURNOFF	BIT(0)
+
+#define PCIE_APP_PMC			0x44
+#define PCIE_APP_PMC_IN_L2		BIT(20)
+
+#define PCIE_APP_IRNEN			0xF4
+#define PCIE_APP_IRNCR			0xF8
+#define PCIE_APP_IRN_AER_REPORT		BIT(0)
+#define PCIE_APP_IRN_PME		BIT(2)
+#define PCIE_APP_IRN_RX_VDM_MSG		BIT(4)
+#define PCIE_APP_IRN_PM_TO_ACK		BIT(9)
+#define PCIE_APP_IRN_LINK_AUTO_BW_STAT	BIT(11)
+#define PCIE_APP_IRN_BW_MGT		BIT(12)
+#define PCIE_APP_IRN_MSG_LTR		BIT(18)
+#define PCIE_APP_IRN_SYS_ERR_RC		BIT(29)
+#define PCIE_APP_INTX_OFST		12
+
+#define PCIE_APP_IRN_INT \
+			(PCIE_APP_IRN_AER_REPORT | PCIE_APP_IRN_PME | \
+			PCIE_APP_IRN_RX_VDM_MSG | PCIE_APP_IRN_SYS_ERR_RC | \
+			PCIE_APP_IRN_PM_TO_ACK | PCIE_APP_IRN_MSG_LTR | \
+			PCIE_APP_IRN_BW_MGT | PCIE_APP_IRN_LINK_AUTO_BW_STAT | \
+			(PCIE_APP_INTX_OFST + PCI_INTERRUPT_INTA) | \
+			(PCIE_APP_INTX_OFST + PCI_INTERRUPT_INTB) | \
+			(PCIE_APP_INTX_OFST + PCI_INTERRUPT_INTC) | \
+			(PCIE_APP_INTX_OFST + PCI_INTERRUPT_INTD))
+
+#define BUS_IATU_OFFSET			SZ_256M
+#define RESET_INTERVAL_MS		100
+
+struct intel_pcie_soc {
+	unsigned int pcie_ver;
+	unsigned int pcie_atu_offset;
+	u32 num_viewport;
+};
+
+struct intel_pcie_port {
+	struct dw_pcie		pci;
+	void __iomem		*app_base;
+	struct gpio_desc	*reset_gpio;
+	u32			rst_intrvl;
+	u32			max_speed;
+	u32			link_gen;
+	u32			max_width;
+	u32			n_fts;
+	struct clk		*core_clk;
+	struct reset_control	*core_rst;
+	struct phy		*phy;
+	u8			pcie_cap_ofst;
+};
+
+static void pcie_update_bits(void __iomem *base, u32 mask, u32 val, u32 ofs)
+{
+	u32 old;
+
+	old = readl(base + ofs);
+	val = (old & ~mask) | (val & mask);
+
+	if (val != old)
+		writel(val, base + ofs);
+}
+
+static inline u32 pcie_app_rd(struct intel_pcie_port *lpp, u32 ofs)
+{
+	return readl(lpp->app_base + ofs);
+}
+
+static inline void pcie_app_wr(struct intel_pcie_port *lpp, u32 val, u32 ofs)
+{
+	writel(val, lpp->app_base + ofs);
+}
+
+static void pcie_app_wr_mask(struct intel_pcie_port *lpp,
+			     u32 mask, u32 val, u32 ofs)
+{
+	pcie_update_bits(lpp->app_base, mask, val, ofs);
+}
+
+static inline u32 pcie_rc_cfg_rd(struct intel_pcie_port *lpp, u32 ofs)
+{
+	return dw_pcie_readl_dbi(&lpp->pci, ofs);
+}
+
+static inline void pcie_rc_cfg_wr(struct intel_pcie_port *lpp, u32 val, u32 ofs)
+{
+	dw_pcie_writel_dbi(&lpp->pci, ofs, val);
+}
+
+static void pcie_rc_cfg_wr_mask(struct intel_pcie_port *lpp,
+				u32 mask, u32 val, u32 ofs)
+{
+	pcie_update_bits(lpp->pci.dbi_base, mask, val, ofs);
+}
+
+static void intel_pcie_ltssm_enable(struct intel_pcie_port *lpp)
+{
+	pcie_app_wr_mask(lpp, PCIE_APP_CCR_LTSSM_ENABLE,
+			 PCIE_APP_CCR_LTSSM_ENABLE, PCIE_APP_CCR);
+}
+
+static void intel_pcie_ltssm_disable(struct intel_pcie_port *lpp)
+{
+	pcie_app_wr_mask(lpp, PCIE_APP_CCR_LTSSM_ENABLE, 0, PCIE_APP_CCR);
+}
+
+static void intel_pcie_link_setup(struct intel_pcie_port *lpp)
+{
+	u32 val;
+	u8 offset = lpp->pcie_cap_ofst;
+
+	val = pcie_rc_cfg_rd(lpp, offset + PCI_EXP_LNKCAP);
+	lpp->max_speed = FIELD_GET(PCI_EXP_LNKCAP_SLS, val);
+	lpp->max_width = FIELD_GET(PCI_EXP_LNKCAP_MLW, val);
+
+	val = pcie_rc_cfg_rd(lpp, offset + PCI_EXP_LNKCTL);
+
+	val &= ~(PCI_EXP_LNKCTL_LD | PCI_EXP_LNKCTL_ASPMC);
+	pcie_rc_cfg_wr(lpp, val, offset + PCI_EXP_LNKCTL);
+}
+
+static void intel_pcie_port_logic_setup(struct intel_pcie_port *lpp)
+{
+	u32 val, mask;
+
+	switch (pcie_link_speed[lpp->max_speed]) {
+	case PCIE_SPEED_8_0GT:
+		lpp->n_fts = PORT_AFR_N_FTS_GEN3;
+		break;
+	case PCIE_SPEED_16_0GT:
+		lpp->n_fts = PORT_AFR_N_FTS_GEN4;
+		break;
+	default:
+		lpp->n_fts = PORT_AFR_N_FTS_GEN12_DFT;
+		break;
+	}
+
+	mask = PORT_AFR_N_FTS_MASK | PORT_AFR_CC_N_FTS_MASK;
+	val = FIELD_PREP(PORT_AFR_N_FTS_MASK, lpp->n_fts) |
+	       FIELD_PREP(PORT_AFR_CC_N_FTS_MASK, lpp->n_fts);
+	pcie_rc_cfg_wr_mask(lpp, mask, val, PCIE_PORT_AFR);
+
+	/* Port Link Control Register */
+	pcie_rc_cfg_wr_mask(lpp, PORT_LINK_DLL_LINK_EN,
+			    PORT_LINK_DLL_LINK_EN, PCIE_PORT_LINK_CONTROL);
+}
+
+static void intel_pcie_rc_setup(struct intel_pcie_port *lpp)
+{
+	intel_pcie_ltssm_disable(lpp);
+	intel_pcie_link_setup(lpp);
+	dw_pcie_setup_rc(&lpp->pci.pp);
+	dw_pcie_upconfig_setup(&lpp->pci);
+	intel_pcie_port_logic_setup(lpp);
+	dw_pcie_link_set_max_speed(&lpp->pci, lpp->link_gen);
+	dw_pcie_link_set_n_fts(&lpp->pci, lpp->n_fts);
+}
+
+static int intel_pcie_ep_rst_init(struct intel_pcie_port *lpp)
+{
+	struct device *dev = lpp->pci.dev;
+	int ret;
+
+	lpp->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
+	if (IS_ERR(lpp->reset_gpio)) {
+		ret = PTR_ERR(lpp->reset_gpio);
+		if (ret != -EPROBE_DEFER)
+			dev_err(dev, "failed to request PCIe GPIO: %d\n", ret);
+		return ret;
+	}
+
+	/* Make initial reset last for 100us */
+	usleep_range(100, 200);
+
+	return 0;
+}
+
+static void intel_pcie_core_rst_assert(struct intel_pcie_port *lpp)
+{
+	reset_control_assert(lpp->core_rst);
+}
+
+static void intel_pcie_core_rst_deassert(struct intel_pcie_port *lpp)
+{
+	/*
+	 * One micro-second delay to make sure the reset pulse
+	 * wide enough so that core reset is clean.
+	 */
+	udelay(1);
+	reset_control_deassert(lpp->core_rst);
+
+	/*
+	 * Some SoC core reset also reset PHY, more delay needed
+	 * to make sure the reset process is done.
+	 */
+	usleep_range(1000, 2000);
+}
+
+static void intel_pcie_device_rst_assert(struct intel_pcie_port *lpp)
+{
+	gpiod_set_value_cansleep(lpp->reset_gpio, 1);
+}
+
+static void intel_pcie_device_rst_deassert(struct intel_pcie_port *lpp)
+{
+	msleep(lpp->rst_intrvl);
+	gpiod_set_value_cansleep(lpp->reset_gpio, 0);
+}
+
+static int intel_pcie_app_logic_setup(struct intel_pcie_port *lpp)
+{
+	intel_pcie_device_rst_deassert(lpp);
+	intel_pcie_ltssm_enable(lpp);
+
+	return dw_pcie_wait_for_link(&lpp->pci);
+}
+
+static void intel_pcie_core_irq_disable(struct intel_pcie_port *lpp)
+{
+	pcie_app_wr(lpp, 0, PCIE_APP_IRNEN);
+	pcie_app_wr(lpp, PCIE_APP_IRN_INT,  PCIE_APP_IRNCR);
+}
+
+static int intel_pcie_get_resources(struct platform_device *pdev)
+{
+	struct intel_pcie_port *lpp = platform_get_drvdata(pdev);
+	struct dw_pcie *pci = &lpp->pci;
+	struct device *dev = pci->dev;
+	struct resource *res;
+	int ret;
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi");
+	pci->dbi_base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(pci->dbi_base))
+		return PTR_ERR(pci->dbi_base);
+
+	lpp->core_clk = devm_clk_get(dev, NULL);
+	if (IS_ERR(lpp->core_clk)) {
+		ret = PTR_ERR(lpp->core_clk);
+		if (ret != -EPROBE_DEFER)
+			dev_err(dev, "failed to get clks: %d\n", ret);
+		return ret;
+	}
+
+	lpp->core_rst = devm_reset_control_get(dev, NULL);
+	if (IS_ERR(lpp->core_rst)) {
+		ret = PTR_ERR(lpp->core_rst);
+		if (ret != -EPROBE_DEFER)
+			dev_err(dev, "failed to get resets: %d\n", ret);
+		return ret;
+	}
+
+	ret = device_property_match_string(dev, "device_type", "pci");
+	if (ret) {
+		dev_err(dev, "failed to find pci device type: %d\n", ret);
+		return ret;
+	}
+
+	if (device_property_read_u32(dev, "reset-assert-ms", &lpp->rst_intrvl))
+		lpp->rst_intrvl = RESET_INTERVAL_MS;
+
+	ret = of_pci_get_max_link_speed(dev->of_node);
+	lpp->link_gen = ret < 0 ? 0 : ret;
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "app");
+	lpp->app_base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(lpp->app_base))
+		return PTR_ERR(lpp->app_base);
+
+	lpp->phy = devm_phy_get(dev, "pcie");
+	if (IS_ERR(lpp->phy)) {
+		ret = PTR_ERR(lpp->phy);
+		if (ret != -EPROBE_DEFER)
+			dev_err(dev, "couldn't get pcie-phy: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static void intel_pcie_deinit_phy(struct intel_pcie_port *lpp)
+{
+	phy_exit(lpp->phy);
+}
+
+static int intel_pcie_wait_l2(struct intel_pcie_port *lpp)
+{
+	u32 value;
+	int ret;
+
+	if (pcie_link_speed[lpp->max_speed] < PCIE_SPEED_8_0GT)
+		return 0;
+
+	/* Send PME_TURN_OFF message */
+	pcie_app_wr_mask(lpp, PCIE_APP_MSG_XMT_PM_TURNOFF,
+			 PCIE_APP_MSG_XMT_PM_TURNOFF, PCIE_APP_MSG_CR);
+
+	/* Read PMC status and wait for falling into L2 link state */
+	ret = readl_poll_timeout(lpp->app_base + PCIE_APP_PMC, value,
+				 value & PCIE_APP_PMC_IN_L2, 20,
+				 jiffies_to_usecs(5 * HZ));
+	if (ret)
+		dev_err(lpp->pci.dev, "PCIe link enter L2 timeout!\n");
+
+	return ret;
+}
+
+static void intel_pcie_turn_off(struct intel_pcie_port *lpp)
+{
+	if (dw_pcie_link_up(&lpp->pci))
+		intel_pcie_wait_l2(lpp);
+
+	/* Put endpoint device in reset state */
+	intel_pcie_device_rst_assert(lpp);
+	pcie_rc_cfg_wr_mask(lpp, PCI_COMMAND_MEMORY, 0, PCI_COMMAND);
+}
+
+static int intel_pcie_host_setup(struct intel_pcie_port *lpp)
+{
+	struct device *dev = lpp->pci.dev;
+	int ret;
+
+	intel_pcie_core_rst_assert(lpp);
+	intel_pcie_device_rst_assert(lpp);
+
+	ret = phy_init(lpp->phy);
+	if (ret)
+		return ret;
+
+	intel_pcie_core_rst_deassert(lpp);
+
+	ret = clk_prepare_enable(lpp->core_clk);
+	if (ret) {
+		dev_err(lpp->pci.dev, "Core clock enable failed: %d\n", ret);
+		goto clk_err;
+	}
+
+	if (!lpp->pcie_cap_ofst) {
+		ret = dw_pcie_find_capability(&lpp->pci, PCI_CAP_ID_EXP);
+		if (!ret) {
+			ret = -ENXIO;
+			dev_err(dev, "Invalid PCIe capability offset\n");
+			goto app_init_err;
+		}
+
+		lpp->pcie_cap_ofst = ret;
+	}
+
+	intel_pcie_rc_setup(lpp);
+	ret = intel_pcie_app_logic_setup(lpp);
+	if (ret)
+		goto app_init_err;
+
+	/* Enable integrated interrupts */
+	pcie_app_wr_mask(lpp, PCIE_APP_IRN_INT, PCIE_APP_IRN_INT,
+			 PCIE_APP_IRNEN);
+	return 0;
+
+app_init_err:
+	clk_disable_unprepare(lpp->core_clk);
+clk_err:
+	intel_pcie_core_rst_assert(lpp);
+	intel_pcie_deinit_phy(lpp);
+
+	return ret;
+}
+
+static void __intel_pcie_remove(struct intel_pcie_port *lpp)
+{
+	intel_pcie_core_irq_disable(lpp);
+	intel_pcie_turn_off(lpp);
+	clk_disable_unprepare(lpp->core_clk);
+	intel_pcie_core_rst_assert(lpp);
+	intel_pcie_deinit_phy(lpp);
+}
+
+static int intel_pcie_remove(struct platform_device *pdev)
+{
+	struct intel_pcie_port *lpp = platform_get_drvdata(pdev);
+	struct pcie_port *pp = &lpp->pci.pp;
+
+	dw_pcie_host_deinit(pp);
+	__intel_pcie_remove(lpp);
+
+	return 0;
+}
+
+static int __maybe_unused intel_pcie_suspend_noirq(struct device *dev)
+{
+	struct intel_pcie_port *lpp = dev_get_drvdata(dev);
+	int ret;
+
+	intel_pcie_core_irq_disable(lpp);
+	ret = intel_pcie_wait_l2(lpp);
+	if (ret)
+		return ret;
+
+	intel_pcie_deinit_phy(lpp);
+	clk_disable_unprepare(lpp->core_clk);
+	return ret;
+}
+
+static int __maybe_unused intel_pcie_resume_noirq(struct device *dev)
+{
+	struct intel_pcie_port *lpp = dev_get_drvdata(dev);
+
+	return intel_pcie_host_setup(lpp);
+}
+
+static int intel_pcie_rc_init(struct pcie_port *pp)
+{
+	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+	struct intel_pcie_port *lpp = dev_get_drvdata(pci->dev);
+
+	return intel_pcie_host_setup(lpp);
+}
+
+int intel_pcie_msi_init(struct pcie_port *pp)
+{
+	/* PCIe MSI/MSIx is handled by MSI in x86 processor */
+	return 0;
+}
+
+u64 intel_pcie_cpu_addr(struct dw_pcie *pcie, u64 cpu_addr)
+{
+	return cpu_addr + BUS_IATU_OFFSET;
+}
+
+static const struct dw_pcie_ops intel_pcie_ops = {
+	.cpu_addr_fixup = intel_pcie_cpu_addr,
+};
+
+static const struct dw_pcie_host_ops intel_pcie_dw_ops = {
+	.host_init =		intel_pcie_rc_init,
+	.msi_host_init =	intel_pcie_msi_init,
+};
+
+static const struct intel_pcie_soc pcie_data = {
+	.pcie_ver =		0x520A,
+	.pcie_atu_offset =	0xC0000,
+	.num_viewport =		3,
+};
+
+static int intel_pcie_probe(struct platform_device *pdev)
+{
+	const struct intel_pcie_soc *data;
+	struct device *dev = &pdev->dev;
+	struct intel_pcie_port *lpp;
+	struct pcie_port *pp;
+	struct dw_pcie *pci;
+	int ret;
+
+	lpp = devm_kzalloc(dev, sizeof(*lpp), GFP_KERNEL);
+	if (!lpp)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, lpp);
+	pci = &lpp->pci;
+	pci->dev = dev;
+	pp = &pci->pp;
+
+	ret = intel_pcie_get_resources(pdev);
+	if (ret)
+		return ret;
+
+	ret = intel_pcie_ep_rst_init(lpp);
+	if (ret)
+		return ret;
+
+	data = device_get_match_data(dev);
+	if (!data)
+		return -ENODEV;
+
+	pci->ops = &intel_pcie_ops;
+	pci->version = data->pcie_ver;
+	pci->atu_base = pci->dbi_base + data->pcie_atu_offset;
+	pp->ops = &intel_pcie_dw_ops;
+
+	ret = dw_pcie_host_init(pp);
+	if (ret) {
+		dev_err(dev, "cannot initialize host\n");
+		return ret;
+	}
+
+	/*
+	 * Intel PCIe doesn't configure IO region, so set viewport
+	 * to not to perform IO region access.
+	 */
+	pci->num_viewport = data->num_viewport;
+
+	dev_info(dev, "Intel PCIe Root Complex Port init done\n");
+
+	return ret;
+}
+
+static const struct dev_pm_ops intel_pcie_pm_ops = {
+	SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(intel_pcie_suspend_noirq,
+				      intel_pcie_resume_noirq)
+};
+
+static const struct of_device_id of_intel_pcie_match[] = {
+	{ .compatible = "intel,lgm-pcie", .data = &pcie_data },
+	{}
+};
+
+static struct platform_driver intel_pcie_driver = {
+	.probe = intel_pcie_probe,
+	.remove = intel_pcie_remove,
+	.driver = {
+		.name = "intel-gw-pcie",
+		.of_match_table = of_intel_pcie_match,
+		.pm = &intel_pcie_pm_ops,
+	},
+};
+builtin_platform_driver(intel_pcie_driver);
diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
index 29d6e93fd15e..548e22e07a52 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -673,6 +673,7 @@
 #define  PCI_EXP_LNKCTL2_TLS_8_0GT	0x0003 /* Supported Speed 8GT/s */
 #define  PCI_EXP_LNKCTL2_TLS_16_0GT	0x0004 /* Supported Speed 16GT/s */
 #define  PCI_EXP_LNKCTL2_TLS_32_0GT	0x0005 /* Supported Speed 32GT/s */
+#define  PCI_EXP_LNKCTL2_HASD		0x0020 /* HW Autonomous Speed Disable */
 #define PCI_EXP_LNKSTA2		50	/* Link Status 2 */
 #define PCI_CAP_EXP_ENDPOINT_SIZEOF_V2	52	/* v2 endpoints with link end here */
 #define PCI_EXP_SLTCAP2		52	/* Slot Capabilities 2 */
-- 
2.11.0

Re: [PATCH v7 2/3] dwc: PCI: intel: PCIe RC controller driver

[Andy Shevchenko]

On Fri, Nov 15, 2019 at 10:30:43AM +0800, Dilip Kota wrote:
> Add support to PCIe RC controller on Intel Gateway SoCs.
> PCIe controller is based of Synopsys DesignWare PCIe core.
> 
> Intel PCIe driver requires Upconfigure support, Fast Training
> Sequence and link speed configurations. So adding the respective
> helper functions in the PCIe DesignWare framework.
> It also programs hardware autonomous speed during speed
> configuration so defining it in pci_regs.h.

FWIW,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>

Although few nitpicks below.

> Signed-off-by: Dilip Kota <eswara.kota@linux.intel.com>
> Reviewed-by: Andrew Murray <andrew.murray@arm.com>
> Acked-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
> ---
> Changes on v7:
> 	Add Acked-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
> 	Remove unused header files.
> 	  #include <linux/interrupt.h>
> 	  #include <linux/of_irq.h>
> 	  #include <linux/of_pci.h>
> 	  #include <linux/of_platform.h>
> 
> Changes on v6:
> 	Add Reviewed-by: Andrew Murray <andrew.murray@arm.com>
> 	Address review comments:
> 	  Remove extra space in Kconfig
> 	  Fix style indents in switch case
> 	  Apply mask for the n_fts argument in dw_pcie_link_set_n_fts()
> 	   to prevent the case of n_fts value having outside the mask bits
> 	  Update PORT_LOGIC_N_FTS to PORT_LOGIC_N_FTS_MASK
> 	  Update BUS_IATU_OFFS to BUS_IATU_OFFSET
> 	  Update RST_INTRVL_DFT_MS to RESET_INTERVAL_MS
> 	  Reduce local variables in pcie_update_bits()
> 	  Remove blank lines
> 	  Remove extra paranthesis during readl_poll_timeout() call
> 	  Add error handling for dw_pcie_find_capability() call
> 	  Add blank line before dev_info() in probe()
> 
> Changes on v5:
> 	Use new, old variables in pcie_update_bits()
> 	Correct naming:
> 	  s/Designware/DesignWare
> 	  s/pcie/PCIe
> 	  s/pci/PCI
> 	  s/Hw/HW
> 	  Upconfig to Upconfigure
> 	Remove extra lines after the intel_pcie_max_speed_setup() def.
> 	Use pcie_link_speed[] and remove enum for pcie gen.
> 	Remove dw_pcie_link_speed_change() def.
> 	Remove "linux,pci-domain" parsing.
> 	Remove 'id' variable in intel_pcie_port structure.
> 	Remove extra spaces for lpp->link_gen =
> 	Correct the offset of PCI_EXP_LNKCTL2_HASD to 0x0020.
> 	Remove programming of RCB and CCC bits in PCI_EXP_LCTL reg.
> 	Remove programming Slot clock cfg in PCI_EXP_LSTS reg.
> 	Update the comments at num_viewport setting.
> 	Update the description in Kconfig.
> 	Get PCIe cap offset from the registers using dw_pcie_find_capability()
> 	Define pcie_cap_ofst var in intel_pcie_port struct to store the same.
> 	Remove the PCIE_CAP_OFST macro.
> 	Move intel_pcie_max_speed_setup() function to DesignWare framework,
> 	 defined as dw_pcie_link_set_max_speed()
> 	Set EXPORT_SYMBOL_GPL for the newer functions defined in
> 	  pcie-designware.c
> 
> Changes on v4:
> 	Rename the driver naming and description to
> 	 "PCIe RC controller on Intel Gateway SoCs".
> 	Use PCIe core register macros defined in pci_regs.h
> 	 and remove respective local definitions.
> 	Remove PCIe core interrupt handling.
> 	Move PCIe link control speed change, upconfig and FTS.
> 	configuration functions to DesignWare framework.
> 	Use of_pci_get_max_link_speed().
> 	Mark dependency on X86 and COMPILE_TEST in Kconfig.
> 	Remove lanes and add n_fts variables in intel_pcie_port structure.
> 	Rename rst_interval variable to rst_intrvl in intel_pcie_port structure.
> 	Remove intel_pcie_mem_iatu() as it is already perfomed in dw_setup_rc()
> 	Move sysfs attributes specific code to separate patch.
> 	Remove redundant error handling.
> 	Reduce LoCs by doing variable initializations while declaration itself.
> 	Add extra line after closing parenthesis.
> 	Move intel_pcie_ep_rst_init() out of get_resources()
> 
> changes on v3:
> 	Rename PCIe app logic registers with PCIE_APP prefix.
> 	PCIE_IOP_CTRL configuration is not required. Remove respective code.
> 	Remove wrapper functions for clk enable/disable APIs.
> 	Use platform_get_resource_byname() instead of
> 	  devm_platform_ioremap_resource() to be similar with DWC framework.
> 	Rename phy name to "pciephy".
> 	Modify debug message in msi_init() callback to be more specific.
> 	Remove map_irq() callback.
> 	Enable the INTx interrupts at the time of PCIe initialization.
> 	Reduce memory fragmentation by using variable "struct dw_pcie pci"
> 	  instead of allocating memory.
> 	Reduce the delay to 100us during enpoint initialization
> 	  intel_pcie_ep_rst_init().
> 	Call  dw_pcie_host_deinit() during remove() instead of directly
> 	  calling PCIe core APIs.
> 	Rename "intel,rst-interval" to "reset-assert-ms".
> 	Remove unused APP logic Interrupt bit macro definitions.
>  	Use dwc framework's dw_pcie_setup_rc() for PCIe host specific
> 	 configuration instead of redefining the same functionality in
> 	 the driver.
> 	Move the whole DT parsing specific code to intel_pcie_get_resources()
> 
>  drivers/pci/controller/dwc/Kconfig           |  10 +
>  drivers/pci/controller/dwc/Makefile          |   1 +
>  drivers/pci/controller/dwc/pcie-designware.c |  57 +++
>  drivers/pci/controller/dwc/pcie-designware.h |  12 +
>  drivers/pci/controller/dwc/pcie-intel-gw.c   | 542 +++++++++++++++++++++++++++
>  include/uapi/linux/pci_regs.h                |   1 +
>  6 files changed, 623 insertions(+)
>  create mode 100644 drivers/pci/controller/dwc/pcie-intel-gw.c
> 
> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> index 0ba988b5b5bc..fb6d474477df 100644
> --- a/drivers/pci/controller/dwc/Kconfig
> +++ b/drivers/pci/controller/dwc/Kconfig
> @@ -82,6 +82,16 @@ config PCIE_DW_PLAT_EP
>  	  order to enable device-specific features PCI_DW_PLAT_EP must be
>  	  selected.
>  
> +config PCIE_INTEL_GW
> +	bool "Intel Gateway PCIe host controller support"
> +	depends on OF && (X86 || COMPILE_TEST)
> +	select PCIE_DW_HOST
> +	help
> +	  Say 'Y' here to enable PCIe Host controller support on Intel
> +	  Gateway SoCs.
> +	  The PCIe controller uses the DesignWare core plus Intel-specific
> +	  hardware wrappers.
> +
>  config PCI_EXYNOS
>  	bool "Samsung Exynos PCIe controller"
>  	depends on SOC_EXYNOS5440 || COMPILE_TEST
> diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
> index b30336181d46..99db83cd2f35 100644
> --- a/drivers/pci/controller/dwc/Makefile
> +++ b/drivers/pci/controller/dwc/Makefile
> @@ -3,6 +3,7 @@ obj-$(CONFIG_PCIE_DW) += pcie-designware.o
>  obj-$(CONFIG_PCIE_DW_HOST) += pcie-designware-host.o
>  obj-$(CONFIG_PCIE_DW_EP) += pcie-designware-ep.o
>  obj-$(CONFIG_PCIE_DW_PLAT) += pcie-designware-plat.o
> +obj-$(CONFIG_PCIE_INTEL_GW) += pcie-intel-gw.o
>  obj-$(CONFIG_PCI_DRA7XX) += pci-dra7xx.o
>  obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
>  obj-$(CONFIG_PCI_IMX6) += pci-imx6.o
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index 820488dfeaed..479e250695a0 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -14,6 +14,8 @@
>  
>  #include "pcie-designware.h"
>  
> +extern const unsigned char pcie_link_speed[];
> +
>  /*
>   * These interfaces resemble the pci_find_*capability() interfaces, but these
>   * are for configuring host controllers, which are bridges *to* PCI devices but
> @@ -474,6 +476,61 @@ int dw_pcie_link_up(struct dw_pcie *pci)
>  		(!(val & PCIE_PORT_DEBUG1_LINK_IN_TRAINING)));
>  }
>  
> +void dw_pcie_upconfig_setup(struct dw_pcie *pci)
> +{
> +	u32 val;
> +
> +	val = dw_pcie_readl_dbi(pci, PCIE_PORT_MULTI_LANE_CTRL);
> +	val |= PORT_MLTI_UPCFG_SUPPORT;
> +	dw_pcie_writel_dbi(pci, PCIE_PORT_MULTI_LANE_CTRL, val);
> +}
> +EXPORT_SYMBOL_GPL(dw_pcie_upconfig_setup);
> +
> +void dw_pcie_link_set_max_speed(struct dw_pcie *pci, u32 link_gen)
> +{
> +	u32 reg, val;
> +	u8 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> +
> +	reg = dw_pcie_readl_dbi(pci, offset + PCI_EXP_LNKCTL2);
> +	reg &= ~PCI_EXP_LNKCTL2_TLS;
> +
> +	switch (pcie_link_speed[link_gen]) {
> +	case PCIE_SPEED_2_5GT:
> +		reg |= PCI_EXP_LNKCTL2_TLS_2_5GT;
> +		break;
> +	case PCIE_SPEED_5_0GT:
> +		reg |= PCI_EXP_LNKCTL2_TLS_5_0GT;
> +		break;
> +	case PCIE_SPEED_8_0GT:
> +		reg |= PCI_EXP_LNKCTL2_TLS_8_0GT;
> +		break;
> +	case PCIE_SPEED_16_0GT:
> +		reg |= PCI_EXP_LNKCTL2_TLS_16_0GT;
> +		break;
> +	default:
> +		/* Use hardware capability */
> +		val = dw_pcie_readl_dbi(pci, offset + PCI_EXP_LNKCAP);
> +		val = FIELD_GET(PCI_EXP_LNKCAP_SLS, val);
> +		reg &= ~PCI_EXP_LNKCTL2_HASD;
> +		reg |= FIELD_PREP(PCI_EXP_LNKCTL2_TLS, val);
> +		break;
> +	}
> +
> +	dw_pcie_writel_dbi(pci, offset + PCI_EXP_LNKCTL2, reg);
> +}
> +EXPORT_SYMBOL_GPL(dw_pcie_link_set_max_speed);
> +
> +void dw_pcie_link_set_n_fts(struct dw_pcie *pci, u32 n_fts)
> +{
> +	u32 val;
> +
> +	val = dw_pcie_readl_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL);
> +	val &= ~PORT_LOGIC_N_FTS_MASK;
> +	val |= n_fts & PORT_LOGIC_N_FTS_MASK;
> +	dw_pcie_writel_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL, val);
> +}
> +EXPORT_SYMBOL_GPL(dw_pcie_link_set_n_fts);
> +
>  static u8 dw_pcie_iatu_unroll_enabled(struct dw_pcie *pci)
>  {
>  	u32 val;
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 5a18e94e52c8..3b0be5cd9235 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -30,7 +30,12 @@
>  #define LINK_WAIT_IATU			9
>  
>  /* Synopsys-specific PCIe configuration registers */
> +#define PCIE_PORT_AFR			0x70C
> +#define PORT_AFR_N_FTS_MASK		GENMASK(15, 8)
> +#define PORT_AFR_CC_N_FTS_MASK		GENMASK(23, 16)
> +
>  #define PCIE_PORT_LINK_CONTROL		0x710
> +#define PORT_LINK_DLL_LINK_EN		BIT(5)
>  #define PORT_LINK_MODE_MASK		GENMASK(21, 16)
>  #define PORT_LINK_MODE(n)		FIELD_PREP(PORT_LINK_MODE_MASK, n)
>  #define PORT_LINK_MODE_1_LANES		PORT_LINK_MODE(0x1)
> @@ -46,6 +51,7 @@
>  #define PCIE_PORT_DEBUG1_LINK_IN_TRAINING	BIT(29)
>  
>  #define PCIE_LINK_WIDTH_SPEED_CONTROL	0x80C
> +#define PORT_LOGIC_N_FTS_MASK		GENMASK(7, 0)
>  #define PORT_LOGIC_SPEED_CHANGE		BIT(17)
>  #define PORT_LOGIC_LINK_WIDTH_MASK	GENMASK(12, 8)
>  #define PORT_LOGIC_LINK_WIDTH(n)	FIELD_PREP(PORT_LOGIC_LINK_WIDTH_MASK, n)
> @@ -60,6 +66,9 @@
>  #define PCIE_MSI_INTR0_MASK		0x82C
>  #define PCIE_MSI_INTR0_STATUS		0x830
>  
> +#define PCIE_PORT_MULTI_LANE_CTRL	0x8C0
> +#define PORT_MLTI_UPCFG_SUPPORT		BIT(7)
> +
>  #define PCIE_ATU_VIEWPORT		0x900
>  #define PCIE_ATU_REGION_INBOUND		BIT(31)
>  #define PCIE_ATU_REGION_OUTBOUND	0
> @@ -273,6 +282,9 @@ void dw_pcie_write_dbi2(struct dw_pcie *pci, u32 reg, size_t size, u32 val);
>  u32 dw_pcie_read_atu(struct dw_pcie *pci, u32 reg, size_t size);
>  void dw_pcie_write_atu(struct dw_pcie *pci, u32 reg, size_t size, u32 val);
>  int dw_pcie_link_up(struct dw_pcie *pci);
> +void dw_pcie_upconfig_setup(struct dw_pcie *pci);
> +void dw_pcie_link_set_max_speed(struct dw_pcie *pci, u32 link_gen);
> +void dw_pcie_link_set_n_fts(struct dw_pcie *pci, u32 n_fts);
>  int dw_pcie_wait_for_link(struct dw_pcie *pci);
>  void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index,
>  			       int type, u64 cpu_addr, u64 pci_addr,
> diff --git a/drivers/pci/controller/dwc/pcie-intel-gw.c b/drivers/pci/controller/dwc/pcie-intel-gw.c
> new file mode 100644
> index 000000000000..cd000cd5bed7
> --- /dev/null
> +++ b/drivers/pci/controller/dwc/pcie-intel-gw.c
> @@ -0,0 +1,542 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * PCIe host controller driver for Intel Gateway SoCs
> + *
> + * Copyright (c) 2019 Intel Corporation.
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/clk.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/iopoll.h>
> +#include <linux/pci_regs.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset.h>
> +
> +#include "../../pci.h"
> +#include "pcie-designware.h"
> +
> +#define PORT_AFR_N_FTS_GEN12_DFT	(SZ_128 - 1)
> +#define PORT_AFR_N_FTS_GEN3		180
> +#define PORT_AFR_N_FTS_GEN4		196
> +
> +/* PCIe Application logic Registers */
> +#define PCIE_APP_CCR			0x10
> +#define PCIE_APP_CCR_LTSSM_ENABLE	BIT(0)
> +
> +#define PCIE_APP_MSG_CR			0x30
> +#define PCIE_APP_MSG_XMT_PM_TURNOFF	BIT(0)
> +
> +#define PCIE_APP_PMC			0x44
> +#define PCIE_APP_PMC_IN_L2		BIT(20)
> +
> +#define PCIE_APP_IRNEN			0xF4
> +#define PCIE_APP_IRNCR			0xF8
> +#define PCIE_APP_IRN_AER_REPORT		BIT(0)
> +#define PCIE_APP_IRN_PME		BIT(2)
> +#define PCIE_APP_IRN_RX_VDM_MSG		BIT(4)
> +#define PCIE_APP_IRN_PM_TO_ACK		BIT(9)
> +#define PCIE_APP_IRN_LINK_AUTO_BW_STAT	BIT(11)
> +#define PCIE_APP_IRN_BW_MGT		BIT(12)
> +#define PCIE_APP_IRN_MSG_LTR		BIT(18)
> +#define PCIE_APP_IRN_SYS_ERR_RC		BIT(29)
> +#define PCIE_APP_INTX_OFST		12
> +
> +#define PCIE_APP_IRN_INT \
> +			(PCIE_APP_IRN_AER_REPORT | PCIE_APP_IRN_PME | \
> +			PCIE_APP_IRN_RX_VDM_MSG | PCIE_APP_IRN_SYS_ERR_RC | \
> +			PCIE_APP_IRN_PM_TO_ACK | PCIE_APP_IRN_MSG_LTR | \
> +			PCIE_APP_IRN_BW_MGT | PCIE_APP_IRN_LINK_AUTO_BW_STAT | \
> +			(PCIE_APP_INTX_OFST + PCI_INTERRUPT_INTA) | \
> +			(PCIE_APP_INTX_OFST + PCI_INTERRUPT_INTB) | \
> +			(PCIE_APP_INTX_OFST + PCI_INTERRUPT_INTC) | \
> +			(PCIE_APP_INTX_OFST + PCI_INTERRUPT_INTD))

These can be shifted left by two TABs.

> +
> +#define BUS_IATU_OFFSET			SZ_256M
> +#define RESET_INTERVAL_MS		100
> +
> +struct intel_pcie_soc {
> +	unsigned int pcie_ver;
> +	unsigned int pcie_atu_offset;
> +	u32 num_viewport;
> +};
> +
> +struct intel_pcie_port {
> +	struct dw_pcie		pci;
> +	void __iomem		*app_base;
> +	struct gpio_desc	*reset_gpio;
> +	u32			rst_intrvl;
> +	u32			max_speed;
> +	u32			link_gen;
> +	u32			max_width;
> +	u32			n_fts;
> +	struct clk		*core_clk;
> +	struct reset_control	*core_rst;
> +	struct phy		*phy;
> +	u8			pcie_cap_ofst;
> +};
> +

> +static void pcie_update_bits(void __iomem *base, u32 mask, u32 val, u32 ofs)

More logical ordering of the parameters will be base, offset, mask, value.

> +{
> +	u32 old;
> +
> +	old = readl(base + ofs);
> +	val = (old & ~mask) | (val & mask);
> +
> +	if (val != old)
> +		writel(val, base + ofs);
> +}
> +
> +static inline u32 pcie_app_rd(struct intel_pcie_port *lpp, u32 ofs)
> +{
> +	return readl(lpp->app_base + ofs);
> +}
> +
> +static inline void pcie_app_wr(struct intel_pcie_port *lpp, u32 val, u32 ofs)

Ditto.

> +{
> +	writel(val, lpp->app_base + ofs);
> +}
> +
> +static void pcie_app_wr_mask(struct intel_pcie_port *lpp,
> +			     u32 mask, u32 val, u32 ofs)

Ditto.

> +{
> +	pcie_update_bits(lpp->app_base, mask, val, ofs);
> +}
> +
> +static inline u32 pcie_rc_cfg_rd(struct intel_pcie_port *lpp, u32 ofs)
> +{
> +	return dw_pcie_readl_dbi(&lpp->pci, ofs);
> +}
> +
> +static inline void pcie_rc_cfg_wr(struct intel_pcie_port *lpp, u32 val, u32 ofs)

Ditto.

> +{
> +	dw_pcie_writel_dbi(&lpp->pci, ofs, val);
> +}
> +
> +static void pcie_rc_cfg_wr_mask(struct intel_pcie_port *lpp,
> +				u32 mask, u32 val, u32 ofs)

Ditto.

> +{
> +	pcie_update_bits(lpp->pci.dbi_base, mask, val, ofs);
> +}
> +
> +static void intel_pcie_ltssm_enable(struct intel_pcie_port *lpp)
> +{
> +	pcie_app_wr_mask(lpp, PCIE_APP_CCR_LTSSM_ENABLE,
> +			 PCIE_APP_CCR_LTSSM_ENABLE, PCIE_APP_CCR);
> +}
> +
> +static void intel_pcie_ltssm_disable(struct intel_pcie_port *lpp)
> +{
> +	pcie_app_wr_mask(lpp, PCIE_APP_CCR_LTSSM_ENABLE, 0, PCIE_APP_CCR);
> +}
> +
> +static void intel_pcie_link_setup(struct intel_pcie_port *lpp)
> +{
> +	u32 val;
> +	u8 offset = lpp->pcie_cap_ofst;
> +
> +	val = pcie_rc_cfg_rd(lpp, offset + PCI_EXP_LNKCAP);
> +	lpp->max_speed = FIELD_GET(PCI_EXP_LNKCAP_SLS, val);
> +	lpp->max_width = FIELD_GET(PCI_EXP_LNKCAP_MLW, val);
> +
> +	val = pcie_rc_cfg_rd(lpp, offset + PCI_EXP_LNKCTL);
> +
> +	val &= ~(PCI_EXP_LNKCTL_LD | PCI_EXP_LNKCTL_ASPMC);
> +	pcie_rc_cfg_wr(lpp, val, offset + PCI_EXP_LNKCTL);
> +}
> +
> +static void intel_pcie_port_logic_setup(struct intel_pcie_port *lpp)
> +{
> +	u32 val, mask;
> +
> +	switch (pcie_link_speed[lpp->max_speed]) {
> +	case PCIE_SPEED_8_0GT:
> +		lpp->n_fts = PORT_AFR_N_FTS_GEN3;
> +		break;
> +	case PCIE_SPEED_16_0GT:
> +		lpp->n_fts = PORT_AFR_N_FTS_GEN4;
> +		break;
> +	default:
> +		lpp->n_fts = PORT_AFR_N_FTS_GEN12_DFT;
> +		break;
> +	}
> +
> +	mask = PORT_AFR_N_FTS_MASK | PORT_AFR_CC_N_FTS_MASK;
> +	val = FIELD_PREP(PORT_AFR_N_FTS_MASK, lpp->n_fts) |
> +	       FIELD_PREP(PORT_AFR_CC_N_FTS_MASK, lpp->n_fts);
> +	pcie_rc_cfg_wr_mask(lpp, mask, val, PCIE_PORT_AFR);
> +
> +	/* Port Link Control Register */
> +	pcie_rc_cfg_wr_mask(lpp, PORT_LINK_DLL_LINK_EN,
> +			    PORT_LINK_DLL_LINK_EN, PCIE_PORT_LINK_CONTROL);
> +}
> +
> +static void intel_pcie_rc_setup(struct intel_pcie_port *lpp)
> +{
> +	intel_pcie_ltssm_disable(lpp);
> +	intel_pcie_link_setup(lpp);
> +	dw_pcie_setup_rc(&lpp->pci.pp);
> +	dw_pcie_upconfig_setup(&lpp->pci);
> +	intel_pcie_port_logic_setup(lpp);
> +	dw_pcie_link_set_max_speed(&lpp->pci, lpp->link_gen);
> +	dw_pcie_link_set_n_fts(&lpp->pci, lpp->n_fts);
> +}
> +
> +static int intel_pcie_ep_rst_init(struct intel_pcie_port *lpp)
> +{
> +	struct device *dev = lpp->pci.dev;
> +	int ret;
> +
> +	lpp->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
> +	if (IS_ERR(lpp->reset_gpio)) {
> +		ret = PTR_ERR(lpp->reset_gpio);
> +		if (ret != -EPROBE_DEFER)
> +			dev_err(dev, "failed to request PCIe GPIO: %d\n", ret);
> +		return ret;
> +	}
> +
> +	/* Make initial reset last for 100us */
> +	usleep_range(100, 200);
> +
> +	return 0;
> +}
> +
> +static void intel_pcie_core_rst_assert(struct intel_pcie_port *lpp)
> +{
> +	reset_control_assert(lpp->core_rst);
> +}
> +
> +static void intel_pcie_core_rst_deassert(struct intel_pcie_port *lpp)
> +{
> +	/*
> +	 * One micro-second delay to make sure the reset pulse
> +	 * wide enough so that core reset is clean.
> +	 */
> +	udelay(1);
> +	reset_control_deassert(lpp->core_rst);
> +
> +	/*
> +	 * Some SoC core reset also reset PHY, more delay needed
> +	 * to make sure the reset process is done.
> +	 */
> +	usleep_range(1000, 2000);
> +}
> +
> +static void intel_pcie_device_rst_assert(struct intel_pcie_port *lpp)
> +{
> +	gpiod_set_value_cansleep(lpp->reset_gpio, 1);
> +}
> +
> +static void intel_pcie_device_rst_deassert(struct intel_pcie_port *lpp)
> +{
> +	msleep(lpp->rst_intrvl);
> +	gpiod_set_value_cansleep(lpp->reset_gpio, 0);
> +}
> +
> +static int intel_pcie_app_logic_setup(struct intel_pcie_port *lpp)
> +{
> +	intel_pcie_device_rst_deassert(lpp);
> +	intel_pcie_ltssm_enable(lpp);
> +
> +	return dw_pcie_wait_for_link(&lpp->pci);
> +}
> +
> +static void intel_pcie_core_irq_disable(struct intel_pcie_port *lpp)
> +{
> +	pcie_app_wr(lpp, 0, PCIE_APP_IRNEN);
> +	pcie_app_wr(lpp, PCIE_APP_IRN_INT,  PCIE_APP_IRNCR);
> +}
> +
> +static int intel_pcie_get_resources(struct platform_device *pdev)
> +{
> +	struct intel_pcie_port *lpp = platform_get_drvdata(pdev);
> +	struct dw_pcie *pci = &lpp->pci;
> +	struct device *dev = pci->dev;
> +	struct resource *res;
> +	int ret;
> +
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi");
> +	pci->dbi_base = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(pci->dbi_base))
> +		return PTR_ERR(pci->dbi_base);
> +
> +	lpp->core_clk = devm_clk_get(dev, NULL);
> +	if (IS_ERR(lpp->core_clk)) {
> +		ret = PTR_ERR(lpp->core_clk);
> +		if (ret != -EPROBE_DEFER)
> +			dev_err(dev, "failed to get clks: %d\n", ret);
> +		return ret;
> +	}
> +
> +	lpp->core_rst = devm_reset_control_get(dev, NULL);
> +	if (IS_ERR(lpp->core_rst)) {
> +		ret = PTR_ERR(lpp->core_rst);
> +		if (ret != -EPROBE_DEFER)
> +			dev_err(dev, "failed to get resets: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = device_property_match_string(dev, "device_type", "pci");
> +	if (ret) {
> +		dev_err(dev, "failed to find pci device type: %d\n", ret);
> +		return ret;
> +	}
> +

> +	if (device_property_read_u32(dev, "reset-assert-ms", &lpp->rst_intrvl))

I would rather use above pattern, i.e.

	ret = device_property_read_u32(...);
	if (ret)
		...

> +		lpp->rst_intrvl = RESET_INTERVAL_MS;
> +
> +	ret = of_pci_get_max_link_speed(dev->of_node);
> +	lpp->link_gen = ret < 0 ? 0 : ret;
> +

> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "app");
> +	lpp->app_base = devm_ioremap_resource(dev, res);

This can use newly introduced (c9c8641d3ebd)
devm_platform_ioremap_resource_byname() API, however it will require PCI
to merge an immutable branch (if any) from driver core.

> +	if (IS_ERR(lpp->app_base))
> +		return PTR_ERR(lpp->app_base);
> +
> +	lpp->phy = devm_phy_get(dev, "pcie");
> +	if (IS_ERR(lpp->phy)) {
> +		ret = PTR_ERR(lpp->phy);
> +		if (ret != -EPROBE_DEFER)
> +			dev_err(dev, "couldn't get pcie-phy: %d\n", ret);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static void intel_pcie_deinit_phy(struct intel_pcie_port *lpp)
> +{
> +	phy_exit(lpp->phy);
> +}
> +
> +static int intel_pcie_wait_l2(struct intel_pcie_port *lpp)
> +{
> +	u32 value;
> +	int ret;
> +
> +	if (pcie_link_speed[lpp->max_speed] < PCIE_SPEED_8_0GT)
> +		return 0;
> +
> +	/* Send PME_TURN_OFF message */
> +	pcie_app_wr_mask(lpp, PCIE_APP_MSG_XMT_PM_TURNOFF,
> +			 PCIE_APP_MSG_XMT_PM_TURNOFF, PCIE_APP_MSG_CR);
> +
> +	/* Read PMC status and wait for falling into L2 link state */
> +	ret = readl_poll_timeout(lpp->app_base + PCIE_APP_PMC, value,
> +				 value & PCIE_APP_PMC_IN_L2, 20,
> +				 jiffies_to_usecs(5 * HZ));
> +	if (ret)
> +		dev_err(lpp->pci.dev, "PCIe link enter L2 timeout!\n");
> +
> +	return ret;
> +}
> +
> +static void intel_pcie_turn_off(struct intel_pcie_port *lpp)
> +{
> +	if (dw_pcie_link_up(&lpp->pci))
> +		intel_pcie_wait_l2(lpp);
> +
> +	/* Put endpoint device in reset state */
> +	intel_pcie_device_rst_assert(lpp);
> +	pcie_rc_cfg_wr_mask(lpp, PCI_COMMAND_MEMORY, 0, PCI_COMMAND);
> +}
> +
> +static int intel_pcie_host_setup(struct intel_pcie_port *lpp)
> +{
> +	struct device *dev = lpp->pci.dev;
> +	int ret;
> +
> +	intel_pcie_core_rst_assert(lpp);
> +	intel_pcie_device_rst_assert(lpp);
> +
> +	ret = phy_init(lpp->phy);
> +	if (ret)
> +		return ret;
> +
> +	intel_pcie_core_rst_deassert(lpp);
> +
> +	ret = clk_prepare_enable(lpp->core_clk);
> +	if (ret) {
> +		dev_err(lpp->pci.dev, "Core clock enable failed: %d\n", ret);
> +		goto clk_err;
> +	}
> +
> +	if (!lpp->pcie_cap_ofst) {
> +		ret = dw_pcie_find_capability(&lpp->pci, PCI_CAP_ID_EXP);
> +		if (!ret) {
> +			ret = -ENXIO;
> +			dev_err(dev, "Invalid PCIe capability offset\n");
> +			goto app_init_err;
> +		}
> +
> +		lpp->pcie_cap_ofst = ret;
> +	}
> +
> +	intel_pcie_rc_setup(lpp);
> +	ret = intel_pcie_app_logic_setup(lpp);
> +	if (ret)
> +		goto app_init_err;
> +
> +	/* Enable integrated interrupts */
> +	pcie_app_wr_mask(lpp, PCIE_APP_IRN_INT, PCIE_APP_IRN_INT,
> +			 PCIE_APP_IRNEN);
> +	return 0;
> +
> +app_init_err:
> +	clk_disable_unprepare(lpp->core_clk);
> +clk_err:
> +	intel_pcie_core_rst_assert(lpp);
> +	intel_pcie_deinit_phy(lpp);
> +
> +	return ret;
> +}
> +
> +static void __intel_pcie_remove(struct intel_pcie_port *lpp)
> +{
> +	intel_pcie_core_irq_disable(lpp);
> +	intel_pcie_turn_off(lpp);
> +	clk_disable_unprepare(lpp->core_clk);
> +	intel_pcie_core_rst_assert(lpp);
> +	intel_pcie_deinit_phy(lpp);
> +}
> +
> +static int intel_pcie_remove(struct platform_device *pdev)
> +{
> +	struct intel_pcie_port *lpp = platform_get_drvdata(pdev);
> +	struct pcie_port *pp = &lpp->pci.pp;
> +
> +	dw_pcie_host_deinit(pp);
> +	__intel_pcie_remove(lpp);
> +
> +	return 0;
> +}
> +
> +static int __maybe_unused intel_pcie_suspend_noirq(struct device *dev)
> +{
> +	struct intel_pcie_port *lpp = dev_get_drvdata(dev);
> +	int ret;
> +
> +	intel_pcie_core_irq_disable(lpp);
> +	ret = intel_pcie_wait_l2(lpp);
> +	if (ret)
> +		return ret;
> +
> +	intel_pcie_deinit_phy(lpp);
> +	clk_disable_unprepare(lpp->core_clk);
> +	return ret;
> +}
> +
> +static int __maybe_unused intel_pcie_resume_noirq(struct device *dev)
> +{
> +	struct intel_pcie_port *lpp = dev_get_drvdata(dev);
> +
> +	return intel_pcie_host_setup(lpp);
> +}
> +
> +static int intel_pcie_rc_init(struct pcie_port *pp)
> +{
> +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> +	struct intel_pcie_port *lpp = dev_get_drvdata(pci->dev);
> +
> +	return intel_pcie_host_setup(lpp);
> +}
> +
> +int intel_pcie_msi_init(struct pcie_port *pp)
> +{
> +	/* PCIe MSI/MSIx is handled by MSI in x86 processor */
> +	return 0;
> +}
> +
> +u64 intel_pcie_cpu_addr(struct dw_pcie *pcie, u64 cpu_addr)
> +{
> +	return cpu_addr + BUS_IATU_OFFSET;
> +}
> +
> +static const struct dw_pcie_ops intel_pcie_ops = {
> +	.cpu_addr_fixup = intel_pcie_cpu_addr,
> +};
> +
> +static const struct dw_pcie_host_ops intel_pcie_dw_ops = {
> +	.host_init =		intel_pcie_rc_init,
> +	.msi_host_init =	intel_pcie_msi_init,
> +};
> +
> +static const struct intel_pcie_soc pcie_data = {
> +	.pcie_ver =		0x520A,
> +	.pcie_atu_offset =	0xC0000,
> +	.num_viewport =		3,
> +};
> +
> +static int intel_pcie_probe(struct platform_device *pdev)
> +{
> +	const struct intel_pcie_soc *data;
> +	struct device *dev = &pdev->dev;
> +	struct intel_pcie_port *lpp;
> +	struct pcie_port *pp;
> +	struct dw_pcie *pci;
> +	int ret;
> +
> +	lpp = devm_kzalloc(dev, sizeof(*lpp), GFP_KERNEL);
> +	if (!lpp)
> +		return -ENOMEM;
> +
> +	platform_set_drvdata(pdev, lpp);
> +	pci = &lpp->pci;
> +	pci->dev = dev;
> +	pp = &pci->pp;
> +
> +	ret = intel_pcie_get_resources(pdev);
> +	if (ret)
> +		return ret;
> +
> +	ret = intel_pcie_ep_rst_init(lpp);
> +	if (ret)
> +		return ret;
> +
> +	data = device_get_match_data(dev);
> +	if (!data)
> +		return -ENODEV;
> +
> +	pci->ops = &intel_pcie_ops;
> +	pci->version = data->pcie_ver;
> +	pci->atu_base = pci->dbi_base + data->pcie_atu_offset;
> +	pp->ops = &intel_pcie_dw_ops;
> +
> +	ret = dw_pcie_host_init(pp);
> +	if (ret) {
> +		dev_err(dev, "cannot initialize host\n");
> +		return ret;
> +	}
> +
> +	/*
> +	 * Intel PCIe doesn't configure IO region, so set viewport
> +	 * to not to perform IO region access.
> +	 */
> +	pci->num_viewport = data->num_viewport;
> +
> +	dev_info(dev, "Intel PCIe Root Complex Port init done\n");
> +
> +	return ret;
> +}
> +
> +static const struct dev_pm_ops intel_pcie_pm_ops = {
> +	SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(intel_pcie_suspend_noirq,
> +				      intel_pcie_resume_noirq)
> +};
> +
> +static const struct of_device_id of_intel_pcie_match[] = {
> +	{ .compatible = "intel,lgm-pcie", .data = &pcie_data },
> +	{}
> +};
> +
> +static struct platform_driver intel_pcie_driver = {
> +	.probe = intel_pcie_probe,
> +	.remove = intel_pcie_remove,
> +	.driver = {
> +		.name = "intel-gw-pcie",
> +		.of_match_table = of_intel_pcie_match,
> +		.pm = &intel_pcie_pm_ops,
> +	},
> +};
> +builtin_platform_driver(intel_pcie_driver);
> diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
> index 29d6e93fd15e..548e22e07a52 100644
> --- a/include/uapi/linux/pci_regs.h
> +++ b/include/uapi/linux/pci_regs.h
> @@ -673,6 +673,7 @@
>  #define  PCI_EXP_LNKCTL2_TLS_8_0GT	0x0003 /* Supported Speed 8GT/s */
>  #define  PCI_EXP_LNKCTL2_TLS_16_0GT	0x0004 /* Supported Speed 16GT/s */
>  #define  PCI_EXP_LNKCTL2_TLS_32_0GT	0x0005 /* Supported Speed 32GT/s */
> +#define  PCI_EXP_LNKCTL2_HASD		0x0020 /* HW Autonomous Speed Disable */
>  #define PCI_EXP_LNKSTA2		50	/* Link Status 2 */
>  #define PCI_CAP_EXP_ENDPOINT_SIZEOF_V2	52	/* v2 endpoints with link end here */
>  #define PCI_EXP_SLTCAP2		52	/* Slot Capabilities 2 */
> -- 
> 2.11.0
> 

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v7 2/3] dwc: PCI: intel: PCIe RC controller driver

[Dilip Kota]

On 11/15/2019 6:19 PM, Andy Shevchenko wrote:
> On Fri, Nov 15, 2019 at 10:30:43AM +0800, Dilip Kota wrote:
>> Add support to PCIe RC controller on Intel Gateway SoCs.
>> PCIe controller is based of Synopsys DesignWare PCIe core.
>>
>> Intel PCIe driver requires Upconfigure support, Fast Training
>> Sequence and link speed configurations. So adding the respective
>> helper functions in the PCIe DesignWare framework.
>> It also programs hardware autonomous speed during speed
>> configuration so defining it in pci_regs.h.
> FWIW,
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
>
> Although few nitpicks below.
Thanks Andy for reviewing the patch.
>
>> Signed-off-by: Dilip Kota <eswara.kota@linux.intel.com>
>> Reviewed-by: Andrew Murray <andrew.murray@arm.com>
>> Acked-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
>> ---
>> Changes on v7:
>> 	Add Acked-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
>> 	Remove unused header files.
>> 	  #include <linux/interrupt.h>
>> 	  #include <linux/of_irq.h>
>> 	  #include <linux/of_pci.h>
>> 	  #include <linux/of_platform.h>
>>
>> Changes on v6:
>> 	Add Reviewed-by: Andrew Murray <andrew.murray@arm.com>
>> 	Address review comments:
>> 	  Remove extra space in Kconfig
>> 	  Fix style indents in switch case
>> 	  Apply mask for the n_fts argument in dw_pcie_link_set_n_fts()
>> 	   to prevent the case of n_fts value having outside the mask bits
>> 	  Update PORT_LOGIC_N_FTS to PORT_LOGIC_N_FTS_MASK
>> 	  Update BUS_IATU_OFFS to BUS_IATU_OFFSET
>> 	  Update RST_INTRVL_DFT_MS to RESET_INTERVAL_MS
>> 	  Reduce local variables in pcie_update_bits()
>> 	  Remove blank lines
>> 	  Remove extra paranthesis during readl_poll_timeout() call
>> 	  Add error handling for dw_pcie_find_capability() call
>> 	  Add blank line before dev_info() in probe()
>>
>> Changes on v5:
>> 	Use new, old variables in pcie_update_bits()
>> 	Correct naming:
>> 	  s/Designware/DesignWare
>> 	  s/pcie/PCIe
>> 	  s/pci/PCI
>> 	  s/Hw/HW
>> 	  Upconfig to Upconfigure
>> 	Remove extra lines after the intel_pcie_max_speed_setup() def.
>> 	Use pcie_link_speed[] and remove enum for pcie gen.
>> 	Remove dw_pcie_link_speed_change() def.
>> 	Remove "linux,pci-domain" parsing.
>> 	Remove 'id' variable in intel_pcie_port structure.
>> 	Remove extra spaces for lpp->link_gen =
>> 	Correct the offset of PCI_EXP_LNKCTL2_HASD to 0x0020.
>> 	Remove programming of RCB and CCC bits in PCI_EXP_LCTL reg.
>> 	Remove programming Slot clock cfg in PCI_EXP_LSTS reg.
>> 	Update the comments at num_viewport setting.
>> 	Update the description in Kconfig.
>> 	Get PCIe cap offset from the registers using dw_pcie_find_capability()
>> 	Define pcie_cap_ofst var in intel_pcie_port struct to store the same.
>> 	Remove the PCIE_CAP_OFST macro.
>> 	Move intel_pcie_max_speed_setup() function to DesignWare framework,
>> 	 defined as dw_pcie_link_set_max_speed()
>> 	Set EXPORT_SYMBOL_GPL for the newer functions defined in
>> 	  pcie-designware.c
>>
>> Changes on v4:
>> 	Rename the driver naming and description to
>> 	 "PCIe RC controller on Intel Gateway SoCs".
>> 	Use PCIe core register macros defined in pci_regs.h
>> 	 and remove respective local definitions.
>> 	Remove PCIe core interrupt handling.
>> 	Move PCIe link control speed change, upconfig and FTS.
>> 	configuration functions to DesignWare framework.
>> 	Use of_pci_get_max_link_speed().
>> 	Mark dependency on X86 and COMPILE_TEST in Kconfig.
>> 	Remove lanes and add n_fts variables in intel_pcie_port structure.
>> 	Rename rst_interval variable to rst_intrvl in intel_pcie_port structure.
>> 	Remove intel_pcie_mem_iatu() as it is already perfomed in dw_setup_rc()
>> 	Move sysfs attributes specific code to separate patch.
>> 	Remove redundant error handling.
>> 	Reduce LoCs by doing variable initializations while declaration itself.
>> 	Add extra line after closing parenthesis.
>> 	Move intel_pcie_ep_rst_init() out of get_resources()
>>
>> changes on v3:
>> 	Rename PCIe app logic registers with PCIE_APP prefix.
>> 	PCIE_IOP_CTRL configuration is not required. Remove respective code.
>> 	Remove wrapper functions for clk enable/disable APIs.
>> 	Use platform_get_resource_byname() instead of
>> 	  devm_platform_ioremap_resource() to be similar with DWC framework.
>> 	Rename phy name to "pciephy".
>> 	Modify debug message in msi_init() callback to be more specific.
>> 	Remove map_irq() callback.
>> 	Enable the INTx interrupts at the time of PCIe initialization.
>> 	Reduce memory fragmentation by using variable "struct dw_pcie pci"
>> 	  instead of allocating memory.
>> 	Reduce the delay to 100us during enpoint initialization
>> 	  intel_pcie_ep_rst_init().
>> 	Call  dw_pcie_host_deinit() during remove() instead of directly
>> 	  calling PCIe core APIs.
>> 	Rename "intel,rst-interval" to "reset-assert-ms".
>> 	Remove unused APP logic Interrupt bit macro definitions.
>>   	Use dwc framework's dw_pcie_setup_rc() for PCIe host specific
>> 	 configuration instead of redefining the same functionality in
>> 	 the driver.
>> 	Move the whole DT parsing specific code to intel_pcie_get_resources()
>>
>>   drivers/pci/controller/dwc/Kconfig           |  10 +
>>   drivers/pci/controller/dwc/Makefile          |   1 +
>>   drivers/pci/controller/dwc/pcie-designware.c |  57 +++
>>   drivers/pci/controller/dwc/pcie-designware.h |  12 +
>>   drivers/pci/controller/dwc/pcie-intel-gw.c   | 542 +++++++++++++++++++++++++++
>>   include/uapi/linux/pci_regs.h                |   1 +
>>   6 files changed, 623 insertions(+)
>>   create mode 100644 drivers/pci/controller/dwc/pcie-intel-gw.c
>>
>> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
>> index 0ba988b5b5bc..fb6d474477df 100644
>> --- a/drivers/pci/controller/dwc/Kconfig
>> +++ b/drivers/pci/controller/dwc/Kconfig
>> @@ -82,6 +82,16 @@ config PCIE_DW_PLAT_EP
>>   	  order to enable device-specific features PCI_DW_PLAT_EP must be
>>   	  selected.
>>   
>> +config PCIE_INTEL_GW
>> +	bool "Intel Gateway PCIe host controller support"
>> +	depends on OF && (X86 || COMPILE_TEST)
>> +	select PCIE_DW_HOST
>> +	help
>> +	  Say 'Y' here to enable PCIe Host controller support on Intel
>> +	  Gateway SoCs.
>> +	  The PCIe controller uses the DesignWare core plus Intel-specific
>> +	  hardware wrappers.
>> +
>>   config PCI_EXYNOS
>>   	bool "Samsung Exynos PCIe controller"
>>   	depends on SOC_EXYNOS5440 || COMPILE_TEST
>> diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
>> index b30336181d46..99db83cd2f35 100644
>> --- a/drivers/pci/controller/dwc/Makefile
>> +++ b/drivers/pci/controller/dwc/Makefile
>> @@ -3,6 +3,7 @@ obj-$(CONFIG_PCIE_DW) += pcie-designware.o
>>   obj-$(CONFIG_PCIE_DW_HOST) += pcie-designware-host.o
>>   obj-$(CONFIG_PCIE_DW_EP) += pcie-designware-ep.o
>>   obj-$(CONFIG_PCIE_DW_PLAT) += pcie-designware-plat.o
>> +obj-$(CONFIG_PCIE_INTEL_GW) += pcie-intel-gw.o
>>   obj-$(CONFIG_PCI_DRA7XX) += pci-dra7xx.o
>>   obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
>>   obj-$(CONFIG_PCI_IMX6) += pci-imx6.o
>> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
>> index 820488dfeaed..479e250695a0 100644
>> --- a/drivers/pci/controller/dwc/pcie-designware.c
>> +++ b/drivers/pci/controller/dwc/pcie-designware.c
>> @@ -14,6 +14,8 @@
>>   
>>   #include "pcie-designware.h"
>>   
>> +extern const unsigned char pcie_link_speed[];
>> +
>>   /*
>>    * These interfaces resemble the pci_find_*capability() interfaces, but these
>>    * are for configuring host controllers, which are bridges *to* PCI devices but
>> @@ -474,6 +476,61 @@ int dw_pcie_link_up(struct dw_pcie *pci)
>>   		(!(val & PCIE_PORT_DEBUG1_LINK_IN_TRAINING)));
>>   }
>>   
>> +void dw_pcie_upconfig_setup(struct dw_pcie *pci)
>> +{
>> +	u32 val;
>> +
>> +	val = dw_pcie_readl_dbi(pci, PCIE_PORT_MULTI_LANE_CTRL);
>> +	val |= PORT_MLTI_UPCFG_SUPPORT;
>> +	dw_pcie_writel_dbi(pci, PCIE_PORT_MULTI_LANE_CTRL, val);
>> +}
>> +EXPORT_SYMBOL_GPL(dw_pcie_upconfig_setup);
>> +
>> +void dw_pcie_link_set_max_speed(struct dw_pcie *pci, u32 link_gen)
>> +{
>> +	u32 reg, val;
>> +	u8 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
>> +
>> +	reg = dw_pcie_readl_dbi(pci, offset + PCI_EXP_LNKCTL2);
>> +	reg &= ~PCI_EXP_LNKCTL2_TLS;
>> +
>> +	switch (pcie_link_speed[link_gen]) {
>> +	case PCIE_SPEED_2_5GT:
>> +		reg |= PCI_EXP_LNKCTL2_TLS_2_5GT;
>> +		break;
>> +	case PCIE_SPEED_5_0GT:
>> +		reg |= PCI_EXP_LNKCTL2_TLS_5_0GT;
>> +		break;
>> +	case PCIE_SPEED_8_0GT:
>> +		reg |= PCI_EXP_LNKCTL2_TLS_8_0GT;
>> +		break;
>> +	case PCIE_SPEED_16_0GT:
>> +		reg |= PCI_EXP_LNKCTL2_TLS_16_0GT;
>> +		break;
>> +	default:
>> +		/* Use hardware capability */
>> +		val = dw_pcie_readl_dbi(pci, offset + PCI_EXP_LNKCAP);
>> +		val = FIELD_GET(PCI_EXP_LNKCAP_SLS, val);
>> +		reg &= ~PCI_EXP_LNKCTL2_HASD;
>> +		reg |= FIELD_PREP(PCI_EXP_LNKCTL2_TLS, val);
>> +		break;
>> +	}
>> +
>> +	dw_pcie_writel_dbi(pci, offset + PCI_EXP_LNKCTL2, reg);
>> +}
>> +EXPORT_SYMBOL_GPL(dw_pcie_link_set_max_speed);
>> +
>> +void dw_pcie_link_set_n_fts(struct dw_pcie *pci, u32 n_fts)
>> +{
>> +	u32 val;
>> +
>> +	val = dw_pcie_readl_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL);
>> +	val &= ~PORT_LOGIC_N_FTS_MASK;
>> +	val |= n_fts & PORT_LOGIC_N_FTS_MASK;
>> +	dw_pcie_writel_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL, val);
>> +}
>> +EXPORT_SYMBOL_GPL(dw_pcie_link_set_n_fts);
>> +
>>   static u8 dw_pcie_iatu_unroll_enabled(struct dw_pcie *pci)
>>   {
>>   	u32 val;
>> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
>> index 5a18e94e52c8..3b0be5cd9235 100644
>> --- a/drivers/pci/controller/dwc/pcie-designware.h
>> +++ b/drivers/pci/controller/dwc/pcie-designware.h
>> @@ -30,7 +30,12 @@
>>   #define LINK_WAIT_IATU			9
>>   
>>   /* Synopsys-specific PCIe configuration registers */
>> +#define PCIE_PORT_AFR			0x70C
>> +#define PORT_AFR_N_FTS_MASK		GENMASK(15, 8)
>> +#define PORT_AFR_CC_N_FTS_MASK		GENMASK(23, 16)
>> +
>>   #define PCIE_PORT_LINK_CONTROL		0x710
>> +#define PORT_LINK_DLL_LINK_EN		BIT(5)
>>   #define PORT_LINK_MODE_MASK		GENMASK(21, 16)
>>   #define PORT_LINK_MODE(n)		FIELD_PREP(PORT_LINK_MODE_MASK, n)
>>   #define PORT_LINK_MODE_1_LANES		PORT_LINK_MODE(0x1)
>> @@ -46,6 +51,7 @@
>>   #define PCIE_PORT_DEBUG1_LINK_IN_TRAINING	BIT(29)
>>   
>>   #define PCIE_LINK_WIDTH_SPEED_CONTROL	0x80C
>> +#define PORT_LOGIC_N_FTS_MASK		GENMASK(7, 0)
>>   #define PORT_LOGIC_SPEED_CHANGE		BIT(17)
>>   #define PORT_LOGIC_LINK_WIDTH_MASK	GENMASK(12, 8)
>>   #define PORT_LOGIC_LINK_WIDTH(n)	FIELD_PREP(PORT_LOGIC_LINK_WIDTH_MASK, n)
>> @@ -60,6 +66,9 @@
>>   #define PCIE_MSI_INTR0_MASK		0x82C
>>   #define PCIE_MSI_INTR0_STATUS		0x830
>>   
>> +#define PCIE_PORT_MULTI_LANE_CTRL	0x8C0
>> +#define PORT_MLTI_UPCFG_SUPPORT		BIT(7)
>> +
>>   #define PCIE_ATU_VIEWPORT		0x900
>>   #define PCIE_ATU_REGION_INBOUND		BIT(31)
>>   #define PCIE_ATU_REGION_OUTBOUND	0
>> @@ -273,6 +282,9 @@ void dw_pcie_write_dbi2(struct dw_pcie *pci, u32 reg, size_t size, u32 val);
>>   u32 dw_pcie_read_atu(struct dw_pcie *pci, u32 reg, size_t size);
>>   void dw_pcie_write_atu(struct dw_pcie *pci, u32 reg, size_t size, u32 val);
>>   int dw_pcie_link_up(struct dw_pcie *pci);
>> +void dw_pcie_upconfig_setup(struct dw_pcie *pci);
>> +void dw_pcie_link_set_max_speed(struct dw_pcie *pci, u32 link_gen);
>> +void dw_pcie_link_set_n_fts(struct dw_pcie *pci, u32 n_fts);
>>   int dw_pcie_wait_for_link(struct dw_pcie *pci);
>>   void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index,
>>   			       int type, u64 cpu_addr, u64 pci_addr,
>> diff --git a/drivers/pci/controller/dwc/pcie-intel-gw.c b/drivers/pci/controller/dwc/pcie-intel-gw.c
>> new file mode 100644
>> index 000000000000..cd000cd5bed7
>> --- /dev/null
>> +++ b/drivers/pci/controller/dwc/pcie-intel-gw.c
>> @@ -0,0 +1,542 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * PCIe host controller driver for Intel Gateway SoCs
>> + *
>> + * Copyright (c) 2019 Intel Corporation.
>> + */
>> +
>> +#include <linux/bitfield.h>
>> +#include <linux/clk.h>
>> +#include <linux/gpio/consumer.h>
>> +#include <linux/iopoll.h>
>> +#include <linux/pci_regs.h>
>> +#include <linux/phy/phy.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/reset.h>
>> +
>> +#include "../../pci.h"
>> +#include "pcie-designware.h"
>> +
>> +#define PORT_AFR_N_FTS_GEN12_DFT	(SZ_128 - 1)
>> +#define PORT_AFR_N_FTS_GEN3		180
>> +#define PORT_AFR_N_FTS_GEN4		196
>> +
>> +/* PCIe Application logic Registers */
>> +#define PCIE_APP_CCR			0x10
>> +#define PCIE_APP_CCR_LTSSM_ENABLE	BIT(0)
>> +
>> +#define PCIE_APP_MSG_CR			0x30
>> +#define PCIE_APP_MSG_XMT_PM_TURNOFF	BIT(0)
>> +
>> +#define PCIE_APP_PMC			0x44
>> +#define PCIE_APP_PMC_IN_L2		BIT(20)
>> +
>> +#define PCIE_APP_IRNEN			0xF4
>> +#define PCIE_APP_IRNCR			0xF8
>> +#define PCIE_APP_IRN_AER_REPORT		BIT(0)
>> +#define PCIE_APP_IRN_PME		BIT(2)
>> +#define PCIE_APP_IRN_RX_VDM_MSG		BIT(4)
>> +#define PCIE_APP_IRN_PM_TO_ACK		BIT(9)
>> +#define PCIE_APP_IRN_LINK_AUTO_BW_STAT	BIT(11)
>> +#define PCIE_APP_IRN_BW_MGT		BIT(12)
>> +#define PCIE_APP_IRN_MSG_LTR		BIT(18)
>> +#define PCIE_APP_IRN_SYS_ERR_RC		BIT(29)
>> +#define PCIE_APP_INTX_OFST		12
>> +
>> +#define PCIE_APP_IRN_INT \
>> +			(PCIE_APP_IRN_AER_REPORT | PCIE_APP_IRN_PME | \
>> +			PCIE_APP_IRN_RX_VDM_MSG | PCIE_APP_IRN_SYS_ERR_RC | \
>> +			PCIE_APP_IRN_PM_TO_ACK | PCIE_APP_IRN_MSG_LTR | \
>> +			PCIE_APP_IRN_BW_MGT | PCIE_APP_IRN_LINK_AUTO_BW_STAT | \
>> +			(PCIE_APP_INTX_OFST + PCI_INTERRUPT_INTA) | \
>> +			(PCIE_APP_INTX_OFST + PCI_INTERRUPT_INTB) | \
>> +			(PCIE_APP_INTX_OFST + PCI_INTERRUPT_INTC) | \
>> +			(PCIE_APP_INTX_OFST + PCI_INTERRUPT_INTD))
> These can be shifted left by two TABs.
Ok, will fix them in next patch revision.
>
>> +
>> +#define BUS_IATU_OFFSET			SZ_256M
>> +#define RESET_INTERVAL_MS		100
>> +
>> +struct intel_pcie_soc {
>> +	unsigned int pcie_ver;
>> +	unsigned int pcie_atu_offset;
>> +	u32 num_viewport;
>> +};
>> +
>> +struct intel_pcie_port {
>> +	struct dw_pcie		pci;
>> +	void __iomem		*app_base;
>> +	struct gpio_desc	*reset_gpio;
>> +	u32			rst_intrvl;
>> +	u32			max_speed;
>> +	u32			link_gen;
>> +	u32			max_width;
>> +	u32			n_fts;
>> +	struct clk		*core_clk;
>> +	struct reset_control	*core_rst;
>> +	struct phy		*phy;
>> +	u8			pcie_cap_ofst;
>> +};
>> +
>> +static void pcie_update_bits(void __iomem *base, u32 mask, u32 val, u32 ofs)
> More logical ordering of the parameters will be base, offset, mask, value.
Agree, will fix it in next patch revision.
>
>> +{
>> +	u32 old;
>> +
>> +	old = readl(base + ofs);
>> +	val = (old & ~mask) | (val & mask);
>> +
>> +	if (val != old)
>> +		writel(val, base + ofs);
>> +}
>> +
>> +static inline u32 pcie_app_rd(struct intel_pcie_port *lpp, u32 ofs)
>> +{
>> +	return readl(lpp->app_base + ofs);
>> +}
>> +
>> +static inline void pcie_app_wr(struct intel_pcie_port *lpp, u32 val, u32 ofs)
> Ditto.
Agree, will fix it in next patch revision.
>
>> +{
>> +	writel(val, lpp->app_base + ofs);
>> +}
>> +
>> +static void pcie_app_wr_mask(struct intel_pcie_port *lpp,
>> +			     u32 mask, u32 val, u32 ofs)
> Ditto.
Agree, will fix it in next patch revision.
>
>> +{
>> +	pcie_update_bits(lpp->app_base, mask, val, ofs);
>> +}
>> +
>> +static inline u32 pcie_rc_cfg_rd(struct intel_pcie_port *lpp, u32 ofs)
>> +{
>> +	return dw_pcie_readl_dbi(&lpp->pci, ofs);
>> +}
>> +
>> +static inline void pcie_rc_cfg_wr(struct intel_pcie_port *lpp, u32 val, u32 ofs)
> Ditto.
Agree, will fix it in next patch revision.
>
>> +{
>> +	dw_pcie_writel_dbi(&lpp->pci, ofs, val);
>> +}
>> +
>> +static void pcie_rc_cfg_wr_mask(struct intel_pcie_port *lpp,
>> +				u32 mask, u32 val, u32 ofs)
> Ditto.
Agree, will fix it in next patch revision.
>
>> +{
>> +	pcie_update_bits(lpp->pci.dbi_base, mask, val, ofs);
>> +}
>> +
>> +static void intel_pcie_ltssm_enable(struct intel_pcie_port *lpp)
>> +{
>> +	pcie_app_wr_mask(lpp, PCIE_APP_CCR_LTSSM_ENABLE,
>> +			 PCIE_APP_CCR_LTSSM_ENABLE, PCIE_APP_CCR);
>> +}
>> +
>> +static void intel_pcie_ltssm_disable(struct intel_pcie_port *lpp)
>> +{
>> +	pcie_app_wr_mask(lpp, PCIE_APP_CCR_LTSSM_ENABLE, 0, PCIE_APP_CCR);
>> +}
>> +
>> +static void intel_pcie_link_setup(struct intel_pcie_port *lpp)
>> +{
>> +	u32 val;
>> +	u8 offset = lpp->pcie_cap_ofst;
>> +
>> +	val = pcie_rc_cfg_rd(lpp, offset + PCI_EXP_LNKCAP);
>> +	lpp->max_speed = FIELD_GET(PCI_EXP_LNKCAP_SLS, val);
>> +	lpp->max_width = FIELD_GET(PCI_EXP_LNKCAP_MLW, val);
>> +
>> +	val = pcie_rc_cfg_rd(lpp, offset + PCI_EXP_LNKCTL);
>> +
>> +	val &= ~(PCI_EXP_LNKCTL_LD | PCI_EXP_LNKCTL_ASPMC);
>> +	pcie_rc_cfg_wr(lpp, val, offset + PCI_EXP_LNKCTL);
>> +}
>> +
>> +static void intel_pcie_port_logic_setup(struct intel_pcie_port *lpp)
>> +{
>> +	u32 val, mask;
>> +
>> +	switch (pcie_link_speed[lpp->max_speed]) {
>> +	case PCIE_SPEED_8_0GT:
>> +		lpp->n_fts = PORT_AFR_N_FTS_GEN3;
>> +		break;
>> +	case PCIE_SPEED_16_0GT:
>> +		lpp->n_fts = PORT_AFR_N_FTS_GEN4;
>> +		break;
>> +	default:
>> +		lpp->n_fts = PORT_AFR_N_FTS_GEN12_DFT;
>> +		break;
>> +	}
>> +
>> +	mask = PORT_AFR_N_FTS_MASK | PORT_AFR_CC_N_FTS_MASK;
>> +	val = FIELD_PREP(PORT_AFR_N_FTS_MASK, lpp->n_fts) |
>> +	       FIELD_PREP(PORT_AFR_CC_N_FTS_MASK, lpp->n_fts);
>> +	pcie_rc_cfg_wr_mask(lpp, mask, val, PCIE_PORT_AFR);
>> +
>> +	/* Port Link Control Register */
>> +	pcie_rc_cfg_wr_mask(lpp, PORT_LINK_DLL_LINK_EN,
>> +			    PORT_LINK_DLL_LINK_EN, PCIE_PORT_LINK_CONTROL);
>> +}
>> +
>> +static void intel_pcie_rc_setup(struct intel_pcie_port *lpp)
>> +{
>> +	intel_pcie_ltssm_disable(lpp);
>> +	intel_pcie_link_setup(lpp);
>> +	dw_pcie_setup_rc(&lpp->pci.pp);
>> +	dw_pcie_upconfig_setup(&lpp->pci);
>> +	intel_pcie_port_logic_setup(lpp);
>> +	dw_pcie_link_set_max_speed(&lpp->pci, lpp->link_gen);
>> +	dw_pcie_link_set_n_fts(&lpp->pci, lpp->n_fts);
>> +}
>> +
>> +static int intel_pcie_ep_rst_init(struct intel_pcie_port *lpp)
>> +{
>> +	struct device *dev = lpp->pci.dev;
>> +	int ret;
>> +
>> +	lpp->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
>> +	if (IS_ERR(lpp->reset_gpio)) {
>> +		ret = PTR_ERR(lpp->reset_gpio);
>> +		if (ret != -EPROBE_DEFER)
>> +			dev_err(dev, "failed to request PCIe GPIO: %d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	/* Make initial reset last for 100us */
>> +	usleep_range(100, 200);
>> +
>> +	return 0;
>> +}
>> +
>> +static void intel_pcie_core_rst_assert(struct intel_pcie_port *lpp)
>> +{
>> +	reset_control_assert(lpp->core_rst);
>> +}
>> +
>> +static void intel_pcie_core_rst_deassert(struct intel_pcie_port *lpp)
>> +{
>> +	/*
>> +	 * One micro-second delay to make sure the reset pulse
>> +	 * wide enough so that core reset is clean.
>> +	 */
>> +	udelay(1);
>> +	reset_control_deassert(lpp->core_rst);
>> +
>> +	/*
>> +	 * Some SoC core reset also reset PHY, more delay needed
>> +	 * to make sure the reset process is done.
>> +	 */
>> +	usleep_range(1000, 2000);
>> +}
>> +
>> +static void intel_pcie_device_rst_assert(struct intel_pcie_port *lpp)
>> +{
>> +	gpiod_set_value_cansleep(lpp->reset_gpio, 1);
>> +}
>> +
>> +static void intel_pcie_device_rst_deassert(struct intel_pcie_port *lpp)
>> +{
>> +	msleep(lpp->rst_intrvl);
>> +	gpiod_set_value_cansleep(lpp->reset_gpio, 0);
>> +}
>> +
>> +static int intel_pcie_app_logic_setup(struct intel_pcie_port *lpp)
>> +{
>> +	intel_pcie_device_rst_deassert(lpp);
>> +	intel_pcie_ltssm_enable(lpp);
>> +
>> +	return dw_pcie_wait_for_link(&lpp->pci);
>> +}
>> +
>> +static void intel_pcie_core_irq_disable(struct intel_pcie_port *lpp)
>> +{
>> +	pcie_app_wr(lpp, 0, PCIE_APP_IRNEN);
>> +	pcie_app_wr(lpp, PCIE_APP_IRN_INT,  PCIE_APP_IRNCR);
>> +}
>> +
>> +static int intel_pcie_get_resources(struct platform_device *pdev)
>> +{
>> +	struct intel_pcie_port *lpp = platform_get_drvdata(pdev);
>> +	struct dw_pcie *pci = &lpp->pci;
>> +	struct device *dev = pci->dev;
>> +	struct resource *res;
>> +	int ret;
>> +
>> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi");
>> +	pci->dbi_base = devm_ioremap_resource(dev, res);
>> +	if (IS_ERR(pci->dbi_base))
>> +		return PTR_ERR(pci->dbi_base);
>> +
>> +	lpp->core_clk = devm_clk_get(dev, NULL);
>> +	if (IS_ERR(lpp->core_clk)) {
>> +		ret = PTR_ERR(lpp->core_clk);
>> +		if (ret != -EPROBE_DEFER)
>> +			dev_err(dev, "failed to get clks: %d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	lpp->core_rst = devm_reset_control_get(dev, NULL);
>> +	if (IS_ERR(lpp->core_rst)) {
>> +		ret = PTR_ERR(lpp->core_rst);
>> +		if (ret != -EPROBE_DEFER)
>> +			dev_err(dev, "failed to get resets: %d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	ret = device_property_match_string(dev, "device_type", "pci");
>> +	if (ret) {
>> +		dev_err(dev, "failed to find pci device type: %d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	if (device_property_read_u32(dev, "reset-assert-ms", &lpp->rst_intrvl))
> I would rather use above pattern, i.e.
>
> 	ret = device_property_read_u32(...);
> 	if (ret)
> 		...
Ok, will update it in next patch revision.
>
>> +		lpp->rst_intrvl = RESET_INTERVAL_MS;
>> +
>> +	ret = of_pci_get_max_link_speed(dev->of_node);
>> +	lpp->link_gen = ret < 0 ? 0 : ret;
>> +
>> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "app");
>> +	lpp->app_base = devm_ioremap_resource(dev, res);
> This can use newly introduced (c9c8641d3ebd)
> devm_platform_ioremap_resource_byname() API, however it will require PCI
> to merge an immutable branch (if any) from driver core.
Who can help on merging the devm_platform_ioremap_byname patch!

Regards,
Dilip

>

Re: [PATCH v7 2/3] dwc: PCI: intel: PCIe RC controller driver

[Jingoo Han]

On 11/14/19, 9:31 PM, Dilip Kota wrote:

> Add support to PCIe RC controller on Intel Gateway SoCs.
> PCIe controller is based of Synopsys DesignWare PCIe core.
>
> Intel PCIe driver requires Upconfigure support, Fast Training
> Sequence and link speed configurations. So adding the respective
> helper functions in the PCIe DesignWare framework.
> It also programs hardware autonomous speed during speed
> configuration so defining it in pci_regs.h.
>
> Signed-off-by: Dilip Kota <eswara.kota@linux.intel.com>
> Reviewed-by: Andrew Murray <andrew.murray@arm.com>
> Acked-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
> ---

[.....]

>  drivers/pci/controller/dwc/Kconfig           |  10 +
>  drivers/pci/controller/dwc/Makefile          |   1 +
>  drivers/pci/controller/dwc/pcie-designware.c |  57 +++
>  drivers/pci/controller/dwc/pcie-designware.h |  12 +
>  drivers/pci/controller/dwc/pcie-intel-gw.c   | 542 +++++++++++++++++++++++++++
>  include/uapi/linux/pci_regs.h                |   1 +
>  6 files changed, 623 insertions(+)
>  create mode 100644 drivers/pci/controller/dwc/pcie-intel-gw.c
>
> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> index 0ba988b5b5bc..fb6d474477df 100644
> --- a/drivers/pci/controller/dwc/Kconfig
> +++ b/drivers/pci/controller/dwc/Kconfig
> @@ -82,6 +82,16 @@ config PCIE_DW_PLAT_EP
>  	  order to enable device-specific features PCI_DW_PLAT_EP must be
>  	  selected.
>  
> +config PCIE_INTEL_GW
> +	bool "Intel Gateway PCIe host controller support"
> +	depends on OF && (X86 || COMPILE_TEST)
> +	select PCIE_DW_HOST
> +	help
> +	  Say 'Y' here to enable PCIe Host controller support on Intel
> +	  Gateway SoCs.
> +	  The PCIe controller uses the DesignWare core plus Intel-specific
> +	  hardware wrappers.
> +

Please add this config alphabetical order!
So, this config should be after 'config PCI_IMX6'.
There is no reason to put this config at the first place.

>  config PCI_EXYNOS
>  	bool "Samsung Exynos PCIe controller"
>  	depends on SOC_EXYNOS5440 || COMPILE_TEST
> diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
> index b30336181d46..99db83cd2f35 100644
> --- a/drivers/pci/controller/dwc/Makefile
> +++ b/drivers/pci/controller/dwc/Makefile
> @@ -3,6 +3,7 @@ obj-$(CONFIG_PCIE_DW) += pcie-designware.o
>  obj-$(CONFIG_PCIE_DW_HOST) += pcie-designware-host.o
>  obj-$(CONFIG_PCIE_DW_EP) += pcie-designware-ep.o
>  obj-$(CONFIG_PCIE_DW_PLAT) += pcie-designware-plat.o
> +obj-$(CONFIG_PCIE_INTEL_GW) += pcie-intel-gw.o

Ditto.

Best regards,
Jingoo Han

>  obj-$(CONFIG_PCI_DRA7XX) += pci-dra7xx.o
>  obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
>  obj-$(CONFIG_PCI_IMX6) += pci-imx6.o
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index 820488dfeaed..479e250695a0 100644

[.....]

Re: [PATCH v7 2/3] dwc: PCI: intel: PCIe RC controller driver

[Dilip Kota]

On 11/16/2019 4:40 AM, Jingoo Han wrote:
> On 11/14/19, 9:31 PM, Dilip Kota wrote:
>
>> Add support to PCIe RC controller on Intel Gateway SoCs.
>> PCIe controller is based of Synopsys DesignWare PCIe core.
>>
>> Intel PCIe driver requires Upconfigure support, Fast Training
>> Sequence and link speed configurations. So adding the respective
>> helper functions in the PCIe DesignWare framework.
>> It also programs hardware autonomous speed during speed
>> configuration so defining it in pci_regs.h.
>>
>> Signed-off-by: Dilip Kota <eswara.kota@linux.intel.com>
>> Reviewed-by: Andrew Murray <andrew.murray@arm.com>
>> Acked-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
>> ---
> [.....]
>
>>   drivers/pci/controller/dwc/Kconfig           |  10 +
>>   drivers/pci/controller/dwc/Makefile          |   1 +
>>   drivers/pci/controller/dwc/pcie-designware.c |  57 +++
>>   drivers/pci/controller/dwc/pcie-designware.h |  12 +
>>   drivers/pci/controller/dwc/pcie-intel-gw.c   | 542 +++++++++++++++++++++++++++
>>   include/uapi/linux/pci_regs.h                |   1 +
>>   6 files changed, 623 insertions(+)
>>   create mode 100644 drivers/pci/controller/dwc/pcie-intel-gw.c
>>
>> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
>> index 0ba988b5b5bc..fb6d474477df 100644
>> --- a/drivers/pci/controller/dwc/Kconfig
>> +++ b/drivers/pci/controller/dwc/Kconfig
>> @@ -82,6 +82,16 @@ config PCIE_DW_PLAT_EP
>>   	  order to enable device-specific features PCI_DW_PLAT_EP must be
>>   	  selected.
>>   
>> +config PCIE_INTEL_GW
>> +	bool "Intel Gateway PCIe host controller support"
>> +	depends on OF && (X86 || COMPILE_TEST)
>> +	select PCIE_DW_HOST
>> +	help
>> +	  Say 'Y' here to enable PCIe Host controller support on Intel
>> +	  Gateway SoCs.
>> +	  The PCIe controller uses the DesignWare core plus Intel-specific
>> +	  hardware wrappers.
>> +
> Please add this config alphabetical order!
> So, this config should be after 'config PCI_IMX6'.
> There is no reason to put this config at the first place.
>
>>   config PCI_EXYNOS
>>   	bool "Samsung Exynos PCIe controller"
>>   	depends on SOC_EXYNOS5440 || COMPILE_TEST
>> diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
>> index b30336181d46..99db83cd2f35 100644
>> --- a/drivers/pci/controller/dwc/Makefile
>> +++ b/drivers/pci/controller/dwc/Makefile
>> @@ -3,6 +3,7 @@ obj-$(CONFIG_PCIE_DW) += pcie-designware.o
>>   obj-$(CONFIG_PCIE_DW_HOST) += pcie-designware-host.o
>>   obj-$(CONFIG_PCIE_DW_EP) += pcie-designware-ep.o
>>   obj-$(CONFIG_PCIE_DW_PLAT) += pcie-designware-plat.o
>> +obj-$(CONFIG_PCIE_INTEL_GW) += pcie-intel-gw.o
> Ditto.
PCIE_INTEL_GW wouldnt come after PCI_IMX6, the complete Makefile and 
Kconfig are not in order,( PCI_* and PCIE_* are not in any order). So i 
just followed PCIE_DW and placed PCIE_INTEL_GW after PCIE_DW as I is 
after D (and i see PCI_* immediately after the PCIE_DW*, so i placed 
PCIE_INTEL_GW after PCIE_DW* and before PCI_*).

Regards,
Dilip

>
> Best regards,
> Jingoo Han
>
>>   obj-$(CONFIG_PCI_DRA7XX) += pci-dra7xx.o
>>   obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
>>   obj-$(CONFIG_PCI_IMX6) += pci-imx6.o
>> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
>> index 820488dfeaed..479e250695a0 100644
> [.....]

Re: [PATCH v7 2/3] dwc: PCI: intel: PCIe RC controller driver

[Jingoo Han]

On 11/18/19, 2:58 AM, Dilip Kota wrote:

> On 11/16/2019 4:40 AM, Jingoo Han wrote:
> > On 11/14/19, 9:31 PM, Dilip Kota wrote:
> >
> >> Add support to PCIe RC controller on Intel Gateway SoCs.
> >> PCIe controller is based of Synopsys DesignWare PCIe core.
> >>
> >> Intel PCIe driver requires Upconfigure support, Fast Training
> >> Sequence and link speed configurations. So adding the respective
> >> helper functions in the PCIe DesignWare framework.
> >> It also programs hardware autonomous speed during speed
> >> configuration so defining it in pci_regs.h.
> >>
> >> Signed-off-by: Dilip Kota <eswara.kota@linux.intel.com>
> >> Reviewed-by: Andrew Murray <andrew.murray@arm.com>
> >> Acked-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
> >> ---
> > [.....]
> >
> >>   drivers/pci/controller/dwc/Kconfig           |  10 +
> >>   drivers/pci/controller/dwc/Makefile          |   1 +
> >>   drivers/pci/controller/dwc/pcie-designware.c |  57 +++
> >>   drivers/pci/controller/dwc/pcie-designware.h |  12 +
> >>   drivers/pci/controller/dwc/pcie-intel-gw.c   | 542 +++++++++++++++++++++++++++
> >>   include/uapi/linux/pci_regs.h                |   1 +
> >>   6 files changed, 623 insertions(+)
> >>   create mode 100644 drivers/pci/controller/dwc/pcie-intel-gw.c
> >>
> >> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> >> index 0ba988b5b5bc..fb6d474477df 100644
> >> --- a/drivers/pci/controller/dwc/Kconfig
> >> +++ b/drivers/pci/controller/dwc/Kconfig
> >> @@ -82,6 +82,16 @@ config PCIE_DW_PLAT_EP
> >>   	  order to enable device-specific features PCI_DW_PLAT_EP must be
> >>   	  selected.
> >>   
> >> +config PCIE_INTEL_GW
> >> +	bool "Intel Gateway PCIe host controller support"
> >> +	depends on OF && (X86 || COMPILE_TEST)
> >> +	select PCIE_DW_HOST
> >> +	help
> >> +	  Say 'Y' here to enable PCIe Host controller support on Intel
> >> +	  Gateway SoCs.
> >> +	  The PCIe controller uses the DesignWare core plus Intel-specific
> >> +	  hardware wrappers.
> >> +
> > Please add this config alphabetical order!
> > So, this config should be after 'config PCI_IMX6'.
> > There is no reason to put this config at the first place.
> >
> >>   config PCI_EXYNOS
> >>   	bool "Samsung Exynos PCIe controller"
> >>   	depends on SOC_EXYNOS5440 || COMPILE_TEST
> >> diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
> >> index b30336181d46..99db83cd2f35 100644
> >> --- a/drivers/pci/controller/dwc/Makefile
> >> +++ b/drivers/pci/controller/dwc/Makefile
> >> @@ -3,6 +3,7 @@ obj-$(CONFIG_PCIE_DW) += pcie-designware.o
> >>   obj-$(CONFIG_PCIE_DW_HOST) += pcie-designware-host.o
> >>   obj-$(CONFIG_PCIE_DW_EP) += pcie-designware-ep.o
> >>   obj-$(CONFIG_PCIE_DW_PLAT) += pcie-designware-plat.o
> >> +obj-$(CONFIG_PCIE_INTEL_GW) += pcie-intel-gw.o
> > Ditto.
> PCIE_INTEL_GW wouldnt come after PCI_IMX6, the complete Makefile and 
> Kconfig are not in order,( PCI_* and PCIE_* are not in any order). So i 
> just followed PCIE_DW and placed PCIE_INTEL_GW after PCIE_DW as I is 
> after D (and i see PCI_* immediately after the PCIE_DW*, so i placed 
> PCIE_INTEL_GW after PCIE_DW* and before PCI_*).

Hey, although some of them are not in order, you don't have a right to do so.
If some people do not follow the law, it does not mean that you can break the law.
Anyway, if you don't follow an alphabetical order, my answer is NACK.
Also, other people or I will send a patch to fix the order of other drivers.


> Regards,
> Dilip

>
> Best regards,
> Jingoo Han
>
>>   obj-$(CONFIG_PCI_DRA7XX) += pci-dra7xx.o
>>   obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
>>   obj-$(CONFIG_PCI_IMX6) += pci-imx6.o
>> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
>> index 820488dfeaed..479e250695a0 100644
> [.....]

Re: [PATCH v7 2/3] dwc: PCI: intel: PCIe RC controller driver

[Dilip Kota]

On 11/19/2019 12:40 AM, Jingoo Han wrote:
>
> On 11/18/19, 2:58 AM, Dilip Kota wrote:
>
>> On 11/16/2019 4:40 AM, Jingoo Han wrote:
>>> On 11/14/19, 9:31 PM, Dilip Kota wrote:
>>>
>>>> Add support to PCIe RC controller on Intel Gateway SoCs.
>>>> PCIe controller is based of Synopsys DesignWare PCIe core.
>>>>
>>>> Intel PCIe driver requires Upconfigure support, Fast Training
>>>> Sequence and link speed configurations. So adding the respective
>>>> helper functions in the PCIe DesignWare framework.
>>>> It also programs hardware autonomous speed during speed
>>>> configuration so defining it in pci_regs.h.
>>>>
>>>> Signed-off-by: Dilip Kota <eswara.kota@linux.intel.com>
>>>> Reviewed-by: Andrew Murray <andrew.murray@arm.com>
>>>> Acked-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
>>>> ---
>>> [.....]
>>>
>>>>    drivers/pci/controller/dwc/Kconfig           |  10 +
>>>>    drivers/pci/controller/dwc/Makefile          |   1 +
>>>>    drivers/pci/controller/dwc/pcie-designware.c |  57 +++
>>>>    drivers/pci/controller/dwc/pcie-designware.h |  12 +
>>>>    drivers/pci/controller/dwc/pcie-intel-gw.c   | 542 +++++++++++++++++++++++++++
>>>>    include/uapi/linux/pci_regs.h                |   1 +
>>>>    6 files changed, 623 insertions(+)
>>>>    create mode 100644 drivers/pci/controller/dwc/pcie-intel-gw.c
>>>>
>>>> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
>>>> index 0ba988b5b5bc..fb6d474477df 100644
>>>> --- a/drivers/pci/controller/dwc/Kconfig
>>>> +++ b/drivers/pci/controller/dwc/Kconfig
>>>> @@ -82,6 +82,16 @@ config PCIE_DW_PLAT_EP
>>>>    	  order to enable device-specific features PCI_DW_PLAT_EP must be
>>>>    	  selected.
>>>>    
>>>> +config PCIE_INTEL_GW
>>>> +	bool "Intel Gateway PCIe host controller support"
>>>> +	depends on OF && (X86 || COMPILE_TEST)
>>>> +	select PCIE_DW_HOST
>>>> +	help
>>>> +	  Say 'Y' here to enable PCIe Host controller support on Intel
>>>> +	  Gateway SoCs.
>>>> +	  The PCIe controller uses the DesignWare core plus Intel-specific
>>>> +	  hardware wrappers.
>>>> +
>>> Please add this config alphabetical order!
>>> So, this config should be after 'config PCI_IMX6'.
>>> There is no reason to put this config at the first place.
>>>
>>>>    config PCI_EXYNOS
>>>>    	bool "Samsung Exynos PCIe controller"
>>>>    	depends on SOC_EXYNOS5440 || COMPILE_TEST
>>>> diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
>>>> index b30336181d46..99db83cd2f35 100644
>>>> --- a/drivers/pci/controller/dwc/Makefile
>>>> +++ b/drivers/pci/controller/dwc/Makefile
>>>> @@ -3,6 +3,7 @@ obj-$(CONFIG_PCIE_DW) += pcie-designware.o
>>>>    obj-$(CONFIG_PCIE_DW_HOST) += pcie-designware-host.o
>>>>    obj-$(CONFIG_PCIE_DW_EP) += pcie-designware-ep.o
>>>>    obj-$(CONFIG_PCIE_DW_PLAT) += pcie-designware-plat.o
>>>> +obj-$(CONFIG_PCIE_INTEL_GW) += pcie-intel-gw.o
>>> Ditto.
>> PCIE_INTEL_GW wouldnt come after PCI_IMX6, the complete Makefile and
>> Kconfig are not in order,( PCI_* and PCIE_* are not in any order). So i
>> just followed PCIE_DW and placed PCIE_INTEL_GW after PCIE_DW as I is
>> after D (and i see PCI_* immediately after the PCIE_DW*, so i placed
>> PCIE_INTEL_GW after PCIE_DW* and before PCI_*).
> Hey, although some of them are not in order, you don't have a right to do so.
> If some people do not follow the law, it does not mean that you can break the law.
> Anyway, if you don't follow an alphabetical order, my answer is NACK.
> Also, other people or I will send a patch to fix the order of other drivers.

I am not against following the order. I kept PCIE_INTEL_GW after 
PCIE_DW* by checking the best possible order.
As per the alphabetical order, i see all CONFIG_PCIE_* comes first and 
CONFIG_PCI_* follows. So, by following this, i placed PCIE_INTEL_GW 
after PCIE_DW* (for the same reason PCIE_INTEL_GW cannot be placed after 
PCI_IMX6).
Even after re-ordering the Kconfig and Makefile, still PCIE_INTEL_GW 
comes after PCIE_DW_PLAT( and PCIE_HISI_STB).

Regards,
Dilip

>
>
>> Regards,
>> Dilip
>> Best regards,
>> Jingoo Han
>>
>>>    obj-$(CONFIG_PCI_DRA7XX) += pci-dra7xx.o
>>>    obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
>>>    obj-$(CONFIG_PCI_IMX6) += pci-imx6.o
>>> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
>>> index 820488dfeaed..479e250695a0 100644
>> [.....]

Re: [PATCH v7 2/3] dwc: PCI: intel: PCIe RC controller driver

[Jingoo Han]

On 11/18/19, 8:26 PM, Dilip Kota wrote:
> 
> On 11/19/2019 12:40 AM, Jingoo Han wrote:
> >
> > On 11/18/19, 2:58 AM, Dilip Kota wrote:
> >
> >> On 11/16/2019 4:40 AM, Jingoo Han wrote:
> >>> On 11/14/19, 9:31 PM, Dilip Kota wrote:
> >>>
> >>>> Add support to PCIe RC controller on Intel Gateway SoCs.
> >>>> PCIe controller is based of Synopsys DesignWare PCIe core.
> >>>>
> >>>> Intel PCIe driver requires Upconfigure support, Fast Training
> >>>> Sequence and link speed configurations. So adding the respective
> >>>> helper functions in the PCIe DesignWare framework.
> >>>> It also programs hardware autonomous speed during speed
> >>>> configuration so defining it in pci_regs.h.
> >>>>
> >>>> Signed-off-by: Dilip Kota <eswara.kota@linux.intel.com>
> >>>> Reviewed-by: Andrew Murray <andrew.murray@arm.com>
> >>>> Acked-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
> >>>> ---
> >>> [.....]
> >>>
> >>>>    drivers/pci/controller/dwc/Kconfig           |  10 +
> >>>>    drivers/pci/controller/dwc/Makefile          |   1 +
> >>>>    drivers/pci/controller/dwc/pcie-designware.c |  57 +++
> >>>>    drivers/pci/controller/dwc/pcie-designware.h |  12 +
> >>>>    drivers/pci/controller/dwc/pcie-intel-gw.c   | 542 +++++++++++++++++++++++++++
> >>>>    include/uapi/linux/pci_regs.h                |   1 +
> >>>>    6 files changed, 623 insertions(+)
> >>>>    create mode 100644 drivers/pci/controller/dwc/pcie-intel-gw.c
> >>>>
> >>>> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> >>>> index 0ba988b5b5bc..fb6d474477df 100644
> >>>> --- a/drivers/pci/controller/dwc/Kconfig
> >>>> +++ b/drivers/pci/controller/dwc/Kconfig
> >>>> @@ -82,6 +82,16 @@ config PCIE_DW_PLAT_EP
> >>>>    	  order to enable device-specific features PCI_DW_PLAT_EP must be
> >>>>    	  selected.
> >>>>    
> >>>> +config PCIE_INTEL_GW
> >>>> +	bool "Intel Gateway PCIe host controller support"
> >>>> +	depends on OF && (X86 || COMPILE_TEST)
> >>>> +	select PCIE_DW_HOST
> >>>> +	help
> >>>> +	  Say 'Y' here to enable PCIe Host controller support on Intel
> >>>> +	  Gateway SoCs.
> >>>> +	  The PCIe controller uses the DesignWare core plus Intel-specific
> >>>> +	  hardware wrappers.
> >>>> +
> >>> Please add this config alphabetical order!
> >>> So, this config should be after 'config PCI_IMX6'.
> >>> There is no reason to put this config at the first place.
> >>>
> >>>>    config PCI_EXYNOS
> >>>>    	bool "Samsung Exynos PCIe controller"
> >>>>    	depends on SOC_EXYNOS5440 || COMPILE_TEST
> >>>> diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
> >>>> index b30336181d46..99db83cd2f35 100644
> >>>> --- a/drivers/pci/controller/dwc/Makefile
> >>>> +++ b/drivers/pci/controller/dwc/Makefile
> >>>> @@ -3,6 +3,7 @@ obj-$(CONFIG_PCIE_DW) += pcie-designware.o
> >>>>    obj-$(CONFIG_PCIE_DW_HOST) += pcie-designware-host.o
> >>>>    obj-$(CONFIG_PCIE_DW_EP) += pcie-designware-ep.o
> >>>>    obj-$(CONFIG_PCIE_DW_PLAT) += pcie-designware-plat.o
> >>>> +obj-$(CONFIG_PCIE_INTEL_GW) += pcie-intel-gw.o
> >>> Ditto.
> >> PCIE_INTEL_GW wouldnt come after PCI_IMX6, the complete Makefile and
> >> Kconfig are not in order,( PCI_* and PCIE_* are not in any order). So i
> >> just followed PCIE_DW and placed PCIE_INTEL_GW after PCIE_DW as I is
> >> after D (and i see PCI_* immediately after the PCIE_DW*, so i placed
> >> PCIE_INTEL_GW after PCIE_DW* and before PCI_*).
> > Hey, although some of them are not in order, you don't have a right to do so.
> > If some people do not follow the law, it does not mean that you can break the law.
> > Anyway, if you don't follow an alphabetical order, my answer is NACK.
> > Also, other people or I will send a patch to fix the order of other drivers.
>
> I am not against following the order. I kept PCIE_INTEL_GW after 
> PCIE_DW* by checking the best possible order.
> As per the alphabetical order, i see all CONFIG_PCIE_* comes first and 
> CONFIG_PCI_* follows. So, by following this, i placed PCIE_INTEL_GW 
> after PCIE_DW* (for the same reason PCIE_INTEL_GW cannot be placed after 
> PCI_IMX6).
> Even after re-ordering the Kconfig and Makefile, still PCIE_INTEL_GW 
> comes after PCIE_DW_PLAT( and PCIE_HISI_STB).

Are you kidding me?

Most PCIE_* drivers are located after PCI_*. Look at PCIE_QCOM, PCIE_ARMADA_8K,
PCIE_ARTPEC6, PCIE_KIRIN, PCIE_HISI_STB, PCIE_TEGRA194, PCIE_UNIPHIER, PCIE_AL.
Put PCIE_INTEL_GW between PCIE_ARTPEC6_EP and PCIE_KIRIN.


> Regards,
Dilip

>
>
>> Regards,
>> Dilip
>> Best regards,
>> Jingoo Han
>>
>>>    obj-$(CONFIG_PCI_DRA7XX) += pci-dra7xx.o
>>>    obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
>>>    obj-$(CONFIG_PCI_IMX6) += pci-imx6.o
>>> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
>>> index 820488dfeaed..479e250695a0 100644
>> [.....]

Re: [PATCH v7 2/3] dwc: PCI: intel: PCIe RC controller driver

[Dilip Kota]

On 11/19/2019 10:33 AM, Jingoo Han wrote:
> On 11/18/19, 8:26 PM, Dilip Kota wrote:
>> On 11/19/2019 12:40 AM, Jingoo Han wrote:
>>> On 11/18/19, 2:58 AM, Dilip Kota wrote:
>>>
>>>> On 11/16/2019 4:40 AM, Jingoo Han wrote:
>>>>> On 11/14/19, 9:31 PM, Dilip Kota wrote:
>>>>>
>>>>>> Add support to PCIe RC controller on Intel Gateway SoCs.
>>>>>> PCIe controller is based of Synopsys DesignWare PCIe core.
>>>>>>
>>>>>> Intel PCIe driver requires Upconfigure support, Fast Training
>>>>>> Sequence and link speed configurations. So adding the respective
>>>>>> helper functions in the PCIe DesignWare framework.
>>>>>> It also programs hardware autonomous speed during speed
>>>>>> configuration so defining it in pci_regs.h.
>>>>>>
>>>>>> Signed-off-by: Dilip Kota <eswara.kota@linux.intel.com>
>>>>>> Reviewed-by: Andrew Murray <andrew.murray@arm.com>
>>>>>> Acked-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
>>>>>> ---
>>>>> [.....]
>>>>>
>>>>>>     drivers/pci/controller/dwc/Kconfig           |  10 +
>>>>>>     drivers/pci/controller/dwc/Makefile          |   1 +
>>>>>>     drivers/pci/controller/dwc/pcie-designware.c |  57 +++
>>>>>>     drivers/pci/controller/dwc/pcie-designware.h |  12 +
>>>>>>     drivers/pci/controller/dwc/pcie-intel-gw.c   | 542 +++++++++++++++++++++++++++
>>>>>>     include/uapi/linux/pci_regs.h                |   1 +
>>>>>>     6 files changed, 623 insertions(+)
>>>>>>     create mode 100644 drivers/pci/controller/dwc/pcie-intel-gw.c
>>>>>>
>>>>>> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
>>>>>> index 0ba988b5b5bc..fb6d474477df 100644
>>>>>> --- a/drivers/pci/controller/dwc/Kconfig
>>>>>> +++ b/drivers/pci/controller/dwc/Kconfig
>>>>>> @@ -82,6 +82,16 @@ config PCIE_DW_PLAT_EP
>>>>>>     	  order to enable device-specific features PCI_DW_PLAT_EP must be
>>>>>>     	  selected.
>>>>>>     
>>>>>> +config PCIE_INTEL_GW
>>>>>> +	bool "Intel Gateway PCIe host controller support"
>>>>>> +	depends on OF && (X86 || COMPILE_TEST)
>>>>>> +	select PCIE_DW_HOST
>>>>>> +	help
>>>>>> +	  Say 'Y' here to enable PCIe Host controller support on Intel
>>>>>> +	  Gateway SoCs.
>>>>>> +	  The PCIe controller uses the DesignWare core plus Intel-specific
>>>>>> +	  hardware wrappers.
>>>>>> +
>>>>> Please add this config alphabetical order!
>>>>> So, this config should be after 'config PCI_IMX6'.
>>>>> There is no reason to put this config at the first place.
>>>>>
>>>>>>     config PCI_EXYNOS
>>>>>>     	bool "Samsung Exynos PCIe controller"
>>>>>>     	depends on SOC_EXYNOS5440 || COMPILE_TEST
>>>>>> diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
>>>>>> index b30336181d46..99db83cd2f35 100644
>>>>>> --- a/drivers/pci/controller/dwc/Makefile
>>>>>> +++ b/drivers/pci/controller/dwc/Makefile
>>>>>> @@ -3,6 +3,7 @@ obj-$(CONFIG_PCIE_DW) += pcie-designware.o
>>>>>>     obj-$(CONFIG_PCIE_DW_HOST) += pcie-designware-host.o
>>>>>>     obj-$(CONFIG_PCIE_DW_EP) += pcie-designware-ep.o
>>>>>>     obj-$(CONFIG_PCIE_DW_PLAT) += pcie-designware-plat.o
>>>>>> +obj-$(CONFIG_PCIE_INTEL_GW) += pcie-intel-gw.o
>>>>> Ditto.
>>>> PCIE_INTEL_GW wouldnt come after PCI_IMX6, the complete Makefile and
>>>> Kconfig are not in order,( PCI_* and PCIE_* are not in any order). So i
>>>> just followed PCIE_DW and placed PCIE_INTEL_GW after PCIE_DW as I is
>>>> after D (and i see PCI_* immediately after the PCIE_DW*, so i placed
>>>> PCIE_INTEL_GW after PCIE_DW* and before PCI_*).
>>> Hey, although some of them are not in order, you don't have a right to do so.
>>> If some people do not follow the law, it does not mean that you can break the law.
>>> Anyway, if you don't follow an alphabetical order, my answer is NACK.
>>> Also, other people or I will send a patch to fix the order of other drivers.
>> I am not against following the order. I kept PCIE_INTEL_GW after
>> PCIE_DW* by checking the best possible order.
>> As per the alphabetical order, i see all CONFIG_PCIE_* comes first and
>> CONFIG_PCI_* follows. So, by following this, i placed PCIE_INTEL_GW
>> after PCIE_DW* (for the same reason PCIE_INTEL_GW cannot be placed after
>> PCI_IMX6).
>> Even after re-ordering the Kconfig and Makefile, still PCIE_INTEL_GW
>> comes after PCIE_DW_PLAT( and PCIE_HISI_STB).
> Are you kidding me?
>
> Most PCIE_* drivers are located after PCI_*. Look at PCIE_QCOM, PCIE_ARMADA_8K,
> PCIE_ARTPEC6, PCIE_KIRIN, PCIE_HISI_STB, PCIE_TEGRA194, PCIE_UNIPHIER, PCIE_AL.
Ok, So the understanding is PCIE_DW* will be at top as they are 
framework and then comes CONFIG_PCI_*, CONFIG_PCIE_*.
> Put PCIE_INTEL_GW between PCIE_ARTPEC6_EP and PCIE_KIRIN.

Ok. I will update in the next patch version.

Regards,
Dilip

>
>
>> Regards,
> Dilip
>
>>
>>> Regards,
>>> Dilip
>>> Best regards,
>>> Jingoo Han
>>>
>>>>     obj-$(CONFIG_PCI_DRA7XX) += pci-dra7xx.o
>>>>     obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
>>>>     obj-$(CONFIG_PCI_IMX6) += pci-imx6.o
>>>> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
>>>> index 820488dfeaed..479e250695a0 100644
>>> [.....]

Re: [PATCH v7 2/3] dwc: PCI: intel: PCIe RC controller driver

[Jingoo Han]

On 11/18/19, 10:15 PM, Dilip Kota wrote:
>On 11/19/2019 10:33 AM, Jingoo Han wrote:
>> On 11/18/19, 8:26 PM, Dilip Kota wrote:
>>> On 11/19/2019 12:40 AM, Jingoo Han wrote:
>>>> On 11/18/19, 2:58 AM, Dilip Kota wrote:
>>>>
>>>>> On 11/16/2019 4:40 AM, Jingoo Han wrote:
>>>>>> On 11/14/19, 9:31 PM, Dilip Kota wrote:
>>>>>>
>>>>>>> Add support to PCIe RC controller on Intel Gateway SoCs.
>>>>>>> PCIe controller is based of Synopsys DesignWare PCIe core.
>>>>>>>
>>>>>>> Intel PCIe driver requires Upconfigure support, Fast Training
>>>>>>> Sequence and link speed configurations. So adding the respective
>>>>>>> helper functions in the PCIe DesignWare framework.
>>>>>>> It also programs hardware autonomous speed during speed
>>>>>>> configuration so defining it in pci_regs.h.
>>>>>>>
>>>>>>> Signed-off-by: Dilip Kota <eswara.kota@linux.intel.com>
>>>>>>> Reviewed-by: Andrew Murray <andrew.murray@arm.com>
>>>>>>> Acked-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
>>>>>>> ---
>>>>>> [.....]
>>>>>>
>>>>>>>     drivers/pci/controller/dwc/Kconfig           |  10 +
>>>>>>>     drivers/pci/controller/dwc/Makefile          |   1 +
>>>>>>>     drivers/pci/controller/dwc/pcie-designware.c |  57 +++
>>>>>>>     drivers/pci/controller/dwc/pcie-designware.h |  12 +
>>>>>>>     drivers/pci/controller/dwc/pcie-intel-gw.c   | 542 +++++++++++++++++++++++++++
>>>>>>>     include/uapi/linux/pci_regs.h                |   1 +
>>>>>>>     6 files changed, 623 insertions(+)
>>>>>>>     create mode 100644 drivers/pci/controller/dwc/pcie-intel-gw.c
>>>>>>>
>>>>>>> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
>>>>>>> index 0ba988b5b5bc..fb6d474477df 100644
>>>>>>> --- a/drivers/pci/controller/dwc/Kconfig
>>>>>>> +++ b/drivers/pci/controller/dwc/Kconfig
>>>>>>> @@ -82,6 +82,16 @@ config PCIE_DW_PLAT_EP
>>>>>>>     	  order to enable device-specific features PCI_DW_PLAT_EP must be
>>>>>>>     	  selected.
>>>>>>>     
>>>>>>> +config PCIE_INTEL_GW
>>>>>>> +	bool "Intel Gateway PCIe host controller support"
>>>>>>> +	depends on OF && (X86 || COMPILE_TEST)
>>>>>>> +	select PCIE_DW_HOST
>>>>>>> +	help
>>>>>>> +	  Say 'Y' here to enable PCIe Host controller support on Intel
>>>>>>> +	  Gateway SoCs.
>>>>>>> +	  The PCIe controller uses the DesignWare core plus Intel-specific
>>>>>>> +	  hardware wrappers.
>>>>>>> +
>>>>>> Please add this config alphabetical order!
>>>>>> So, this config should be after 'config PCI_IMX6'.
>>>>>> There is no reason to put this config at the first place.
>>>>>>
>>>>>>>     config PCI_EXYNOS
>>>>>>>     	bool "Samsung Exynos PCIe controller"
>>>>>>>     	depends on SOC_EXYNOS5440 || COMPILE_TEST
>>>>>>> diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
>>>>>>> index b30336181d46..99db83cd2f35 100644
>>>>>>> --- a/drivers/pci/controller/dwc/Makefile
>>>>>>> +++ b/drivers/pci/controller/dwc/Makefile
>>>>>>> @@ -3,6 +3,7 @@ obj-$(CONFIG_PCIE_DW) += pcie-designware.o
>>>>>>>     obj-$(CONFIG_PCIE_DW_HOST) += pcie-designware-host.o
>>>>>>>     obj-$(CONFIG_PCIE_DW_EP) += pcie-designware-ep.o
>>>>>>>     obj-$(CONFIG_PCIE_DW_PLAT) += pcie-designware-plat.o
>>>>>>> +obj-$(CONFIG_PCIE_INTEL_GW) += pcie-intel-gw.o
>>>>>> Ditto.
>>>>> PCIE_INTEL_GW wouldnt come after PCI_IMX6, the complete Makefile and
>>>>> Kconfig are not in order,( PCI_* and PCIE_* are not in any order). So i
>>>>> just followed PCIE_DW and placed PCIE_INTEL_GW after PCIE_DW as I is
>>>>> after D (and i see PCI_* immediately after the PCIE_DW*, so i placed
>>>>> PCIE_INTEL_GW after PCIE_DW* and before PCI_*).
>>>> Hey, although some of them are not in order, you don't have a right to do so.
>>>> If some people do not follow the law, it does not mean that you can break the law.
>>>> Anyway, if you don't follow an alphabetical order, my answer is NACK.
>>>> Also, other people or I will send a patch to fix the order of other drivers.
>>> I am not against following the order. I kept PCIE_INTEL_GW after
>>> PCIE_DW* by checking the best possible order.
>>> As per the alphabetical order, i see all CONFIG_PCIE_* comes first and
>>> CONFIG_PCI_* follows. So, by following this, i placed PCIE_INTEL_GW
>>> after PCIE_DW* (for the same reason PCIE_INTEL_GW cannot be placed after
>>> PCI_IMX6).
>>> Even after re-ordering the Kconfig and Makefile, still PCIE_INTEL_GW
>>> comes after PCIE_DW_PLAT( and PCIE_HISI_STB).
>> Are you kidding me?
>>
>> Most PCIE_* drivers are located after PCI_*. Look at PCIE_QCOM, PCIE_ARMADA_8K,
>> PCIE_ARTPEC6, PCIE_KIRIN, PCIE_HISI_STB, PCIE_TEGRA194, PCIE_UNIPHIER, PCIE_AL.
>Ok, So the understanding is PCIE_DW* will be at top as they are 
>framework and then comes CONFIG_PCI_*, CONFIG_PCIE_*.

Yes, right.
Ah, I will explain the order of Kconfig options in the dwc directory.
The order is PCIE_DW, PCI_*, and PCIE_* as below.

1. Common Frameworks:
    These options are used by other controller drivers.
    e.g., PCIE_DW, PCIE_DW_HOST, PCIE_DW_EP.

2. PCI_* controller drivers:
    PCI_* was used earlier than PCIE_*. If a chip vendor's controllers provide
    both conventional PCI and PCI Express, or only conventional PCI, PCI_* can
    be used. For example, the reason PCI_EXYNOS was chosen instead of
    PCIE_EXYNOS, is that EXYNOS had controllers to support either conventional
    PCI or PCI Express.

3. PCIE_* controller drivers
    If a controller can support only PCI Express, not conventional PCI,
    PCIE_* is the proper naming.

Then, within PCI_* or PCIE_* categories, each controller option should be
in an alphabetical order for the readability. Also, other person or I will
send a patch to fix orders. (e.g., PCIE_QCOM, PCIE_AL, etc.)

>> Put PCIE_INTEL_GW between PCIE_ARTPEC6_EP and PCIE_KIRIN.
>
>Ok. I will update in the next patch version.

Ok, thank you.

Best regards,
Jingoo Han

Re: [PATCH v7 2/3] dwc: PCI: intel: PCIe RC controller driver

[Dilip Kota]

On 11/19/2019 11:54 AM, Jingoo Han wrote:
>
> On 11/18/19, 10:15 PM, Dilip Kota wrote:
>> On 11/19/2019 10:33 AM, Jingoo Han wrote:
>>> On 11/18/19, 8:26 PM, Dilip Kota wrote:
>>>> On 11/19/2019 12:40 AM, Jingoo Han wrote:
>>>>> On 11/18/19, 2:58 AM, Dilip Kota wrote:
>>>>>
>>>>>> On 11/16/2019 4:40 AM, Jingoo Han wrote:
>>>>>>> On 11/14/19, 9:31 PM, Dilip Kota wrote:
>>>>>>>
>>>>>>>> Add support to PCIe RC controller on Intel Gateway SoCs.
>>>>>>>> PCIe controller is based of Synopsys DesignWare PCIe core.
>>>>>>>>
>>>>>>>> Intel PCIe driver requires Upconfigure support, Fast Training
>>>>>>>> Sequence and link speed configurations. So adding the respective
>>>>>>>> helper functions in the PCIe DesignWare framework.
>>>>>>>> It also programs hardware autonomous speed during speed
>>>>>>>> configuration so defining it in pci_regs.h.
>>>>>>>>
>>>>>>>> Signed-off-by: Dilip Kota <eswara.kota@linux.intel.com>
>>>>>>>> Reviewed-by: Andrew Murray <andrew.murray@arm.com>
>>>>>>>> Acked-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
>>>>>>>> ---
>>>>>>> [.....]
>>>>>>>
>>>>>>>>      drivers/pci/controller/dwc/Kconfig           |  10 +
>>>>>>>>      drivers/pci/controller/dwc/Makefile          |   1 +
>>>>>>>>      drivers/pci/controller/dwc/pcie-designware.c |  57 +++
>>>>>>>>      drivers/pci/controller/dwc/pcie-designware.h |  12 +
>>>>>>>>      drivers/pci/controller/dwc/pcie-intel-gw.c   | 542 +++++++++++++++++++++++++++
>>>>>>>>      include/uapi/linux/pci_regs.h                |   1 +
>>>>>>>>      6 files changed, 623 insertions(+)
>>>>>>>>      create mode 100644 drivers/pci/controller/dwc/pcie-intel-gw.c
>>>>>>>>
>>>>>>>> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
>>>>>>>> index 0ba988b5b5bc..fb6d474477df 100644
>>>>>>>> --- a/drivers/pci/controller/dwc/Kconfig
>>>>>>>> +++ b/drivers/pci/controller/dwc/Kconfig
>>>>>>>> @@ -82,6 +82,16 @@ config PCIE_DW_PLAT_EP
>>>>>>>>      	  order to enable device-specific features PCI_DW_PLAT_EP must be
>>>>>>>>      	  selected.
>>>>>>>>      
>>>>>>>> +config PCIE_INTEL_GW
>>>>>>>> +	bool "Intel Gateway PCIe host controller support"
>>>>>>>> +	depends on OF && (X86 || COMPILE_TEST)
>>>>>>>> +	select PCIE_DW_HOST
>>>>>>>> +	help
>>>>>>>> +	  Say 'Y' here to enable PCIe Host controller support on Intel
>>>>>>>> +	  Gateway SoCs.
>>>>>>>> +	  The PCIe controller uses the DesignWare core plus Intel-specific
>>>>>>>> +	  hardware wrappers.
>>>>>>>> +
>>>>>>> Please add this config alphabetical order!
>>>>>>> So, this config should be after 'config PCI_IMX6'.
>>>>>>> There is no reason to put this config at the first place.
>>>>>>>
>>>>>>>>      config PCI_EXYNOS
>>>>>>>>      	bool "Samsung Exynos PCIe controller"
>>>>>>>>      	depends on SOC_EXYNOS5440 || COMPILE_TEST
>>>>>>>> diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
>>>>>>>> index b30336181d46..99db83cd2f35 100644
>>>>>>>> --- a/drivers/pci/controller/dwc/Makefile
>>>>>>>> +++ b/drivers/pci/controller/dwc/Makefile
>>>>>>>> @@ -3,6 +3,7 @@ obj-$(CONFIG_PCIE_DW) += pcie-designware.o
>>>>>>>>      obj-$(CONFIG_PCIE_DW_HOST) += pcie-designware-host.o
>>>>>>>>      obj-$(CONFIG_PCIE_DW_EP) += pcie-designware-ep.o
>>>>>>>>      obj-$(CONFIG_PCIE_DW_PLAT) += pcie-designware-plat.o
>>>>>>>> +obj-$(CONFIG_PCIE_INTEL_GW) += pcie-intel-gw.o
>>>>>>> Ditto.
>>>>>> PCIE_INTEL_GW wouldnt come after PCI_IMX6, the complete Makefile and
>>>>>> Kconfig are not in order,( PCI_* and PCIE_* are not in any order). So i
>>>>>> just followed PCIE_DW and placed PCIE_INTEL_GW after PCIE_DW as I is
>>>>>> after D (and i see PCI_* immediately after the PCIE_DW*, so i placed
>>>>>> PCIE_INTEL_GW after PCIE_DW* and before PCI_*).
>>>>> Hey, although some of them are not in order, you don't have a right to do so.
>>>>> If some people do not follow the law, it does not mean that you can break the law.
>>>>> Anyway, if you don't follow an alphabetical order, my answer is NACK.
>>>>> Also, other people or I will send a patch to fix the order of other drivers.
>>>> I am not against following the order. I kept PCIE_INTEL_GW after
>>>> PCIE_DW* by checking the best possible order.
>>>> As per the alphabetical order, i see all CONFIG_PCIE_* comes first and
>>>> CONFIG_PCI_* follows. So, by following this, i placed PCIE_INTEL_GW
>>>> after PCIE_DW* (for the same reason PCIE_INTEL_GW cannot be placed after
>>>> PCI_IMX6).
>>>> Even after re-ordering the Kconfig and Makefile, still PCIE_INTEL_GW
>>>> comes after PCIE_DW_PLAT( and PCIE_HISI_STB).
>>> Are you kidding me?
>>>
>>> Most PCIE_* drivers are located after PCI_*. Look at PCIE_QCOM, PCIE_ARMADA_8K,
>>> PCIE_ARTPEC6, PCIE_KIRIN, PCIE_HISI_STB, PCIE_TEGRA194, PCIE_UNIPHIER, PCIE_AL.
>> Ok, So the understanding is PCIE_DW* will be at top as they are
>> framework and then comes CONFIG_PCI_*, CONFIG_PCIE_*.
> Yes, right.
> Ah, I will explain the order of Kconfig options in the dwc directory.
> The order is PCIE_DW, PCI_*, and PCIE_* as below.
>
> 1. Common Frameworks:
>      These options are used by other controller drivers.
>      e.g., PCIE_DW, PCIE_DW_HOST, PCIE_DW_EP.
>
> 2. PCI_* controller drivers:
>      PCI_* was used earlier than PCIE_*. If a chip vendor's controllers provide
>      both conventional PCI and PCI Express, or only conventional PCI, PCI_* can
>      be used. For example, the reason PCI_EXYNOS was chosen instead of
>      PCIE_EXYNOS, is that EXYNOS had controllers to support either conventional
>      PCI or PCI Express.
>
> 3. PCIE_* controller drivers
>      If a controller can support only PCI Express, not conventional PCI,
>      PCIE_* is the proper naming.
>
> Then, within PCI_* or PCIE_* categories, each controller option should be
> in an alphabetical order for the readability. Also, other person or I will
> send a patch to fix orders. (e.g., PCIE_QCOM, PCIE_AL, etc.)

Thanks Jingoo Han for explaining in detail.

Regards,
Dilip

>>> Put PCIE_INTEL_GW between PCIE_ARTPEC6_EP and PCIE_KIRIN.
>> Ok. I will update in the next patch version.
> Ok, thank you.
>
> Best regards,
> Jingoo Han
>
>

[PATCH v7 3/3] PCI: artpec6: Configure FTS with dwc helper function

[Dilip Kota]

Use DesignWare helper functions to configure Fast Training
Sequence. Drop the respective code in the driver.

Signed-off-by: Dilip Kota <eswara.kota@linux.intel.com>
Reviewed-by: Andrew Murray <andrew.murray@arm.com>
---
Changes on v7:
	No change

Changes on v6:
	Typo fix:s/DesugnWare/DesignWare
	Update 'Utilize DesignWare' --> 'Use DesignWare'
	Add Reviewed-by: Andrew Murray <andrew.murray@arm.com>

 drivers/pci/controller/dwc/pcie-artpec6.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-artpec6.c b/drivers/pci/controller/dwc/pcie-artpec6.c
index d00252bd8fae..02d93b8c7942 100644
--- a/drivers/pci/controller/dwc/pcie-artpec6.c
+++ b/drivers/pci/controller/dwc/pcie-artpec6.c
@@ -51,9 +51,6 @@ static const struct of_device_id artpec6_pcie_of_match[];
 #define ACK_N_FTS_MASK			GENMASK(15, 8)
 #define ACK_N_FTS(x)			(((x) << 8) & ACK_N_FTS_MASK)
 
-#define FAST_TRAINING_SEQ_MASK		GENMASK(7, 0)
-#define FAST_TRAINING_SEQ(x)		(((x) << 0) & FAST_TRAINING_SEQ_MASK)
-
 /* ARTPEC-6 specific registers */
 #define PCIECFG				0x18
 #define  PCIECFG_DBG_OEN		BIT(24)
@@ -313,10 +310,7 @@ static void artpec6_pcie_set_nfts(struct artpec6_pcie *artpec6_pcie)
 	 * Set the Number of Fast Training Sequences that the core
 	 * advertises as its N_FTS during Gen2 or Gen3 link training.
 	 */
-	val = dw_pcie_readl_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL);
-	val &= ~FAST_TRAINING_SEQ_MASK;
-	val |= FAST_TRAINING_SEQ(180);
-	dw_pcie_writel_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL, val);
+	dw_pcie_link_set_n_fts(pci, 180);
 }
 
 static void artpec6_pcie_assert_core_reset(struct artpec6_pcie *artpec6_pcie)
-- 
2.11.0