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

[Dilip Kota <eswara.kota@xxxxxxxxxxxxxxx>]

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@xxxxxxxxx>
>
> Although few nitpicks below.
Thanks Andy for reviewing the patch.
> > Signed-off-by: Dilip Kota <eswara.kota@xxxxxxxxxxxxxxx>
> > Reviewed-by: Andrew Murray <andrew.murray@xxxxxxx>
> > Acked-by: Gustavo Pimentel <gustavo.pimentel@xxxxxxxxxxxx>
> > ---
> > Changes on v7:
> > 	Add Acked-by: Gustavo Pimentel <gustavo.pimentel@xxxxxxxxxxxx>
> > 	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@xxxxxxx>
> > 	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

>