mbox series

[0/6] Add PCIe support for Tesla FSD SoC

Message ID 20221121105210.68596-1-shradha.t@samsung.com
Headers show
Series Add PCIe support for Tesla FSD SoC | expand

Message

Shradha Todi Nov. 21, 2022, 10:52 a.m. UTC
FSD platform has three instances of DesignWare based PCIe IP,
one is in FSYS0 block and other two in FSYS1 block.
This patch series add required DT binding, DT file modifications,
Controller driver support and PHY driver support for the same.

This series needs following three patches to be merged before
this patchset
[1]: https://www.spinics.net/lists/netdev/msg854161.html
[2]: https://www.spinics.net/lists/netdev/msg854158.html
[3]: https://lore.kernel.org/all/20221013104024.50179-2-p.rajanbabu@samsung.com/


Shradha Todi (6):
  dt-bindings: phy: Add PCIe PHY bindings for FSD
  dt-bindings: PCI: Add PCIe controller bindings for FSD
  PCI: dwc: fsd: Add FSD PCIe Controller driver support
  phy: tesla-pcie: Add PCIe PHY driver support for FSD
  arm64: dts: fsd: Add PCIe support for Tesla FSD SoC
  misc: pci_endpoint_test: Add driver data for FSD PCIe controllers

 .../bindings/pci/tesla,pcie-fsd-ep.yaml       |  107 ++
 .../bindings/pci/tesla,pcie-fsd.yaml          |  117 ++
 .../bindings/phy/phy-tesla-pcie.yaml          |   75 ++
 arch/arm64/boot/dts/tesla/fsd-evb.dts         |   48 +
 arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi    |   65 ++
 arch/arm64/boot/dts/tesla/fsd.dtsi            |  171 +++
 drivers/misc/pci_endpoint_test.c              |    3 +
 drivers/pci/controller/dwc/Kconfig            |   35 +
 drivers/pci/controller/dwc/Makefile           |    1 +
 drivers/pci/controller/dwc/pcie-fsd.c         | 1021 +++++++++++++++++
 drivers/phy/samsung/Kconfig                   |   10 +
 drivers/phy/samsung/Makefile                  |    1 +
 drivers/phy/samsung/phy-tesla-pcie.c          |  397 +++++++
 include/linux/pci_ids.h                       |    2 +
 14 files changed, 2053 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/tesla,pcie-fsd-ep.yaml
 create mode 100644 Documentation/devicetree/bindings/pci/tesla,pcie-fsd.yaml
 create mode 100644 Documentation/devicetree/bindings/phy/phy-tesla-pcie.yaml
 create mode 100644 drivers/pci/controller/dwc/pcie-fsd.c
 create mode 100644 drivers/phy/samsung/phy-tesla-pcie.c

Comments

Krzysztof Kozlowski Nov. 21, 2022, 12:07 p.m. UTC | #1
On 21/11/2022 11:52, Shradha Todi wrote:
> Add PCIe controller driver file for PCIe controller
> found in fsd SoC family. This driver adds support for both RC
> and EP mode.
> 
> Signed-off-by: Niyas Ahmed S T <niyas.ahmed@samsung.com>
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> Signed-off-by: Padmanabhan Rajanbabu <p.rajanbabu@samsung.com>
> Signed-off-by: Shradha Todi <shradha.t@samsung.com>
> ---
>  drivers/pci/controller/dwc/Kconfig    |   35 +
>  drivers/pci/controller/dwc/Makefile   |    1 +
>  drivers/pci/controller/dwc/pcie-fsd.c | 1021 +++++++++++++++++++++++++
>  3 files changed, 1057 insertions(+)
>  create mode 100644 drivers/pci/controller/dwc/pcie-fsd.c
> 

Yeah, when Samsung started upstreaming Artpec-8 PCI and said "it is
entirely different, cannot be merged with anything else", I had a
feeling it will bite us.

So now we see one more.

Then in some days there will be separate PCI for Exynos in Google
products. Then in company X, then Y.

No, work on unified approach not 3 different drivers.

Best regards,
Krzysztof
Krzysztof Kozlowski Nov. 21, 2022, 12:08 p.m. UTC | #2
On 21/11/2022 11:52, Shradha Todi wrote:
> This patch adds PHY driver support for PCIe controller
> found in Tesla FSD SoC.
> 
> Signed-off-by: Niyas Ahmed S T <niyas.ahmed@samsung.com>
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> Signed-off-by: Shradha Todi <shradha.t@samsung.com>
> Signed-off-by: Padmanabhan Rajanbabu <p.rajanbabu@samsung.com>
> ---
>  drivers/phy/samsung/Kconfig          |  10 +
>  drivers/phy/samsung/Makefile         |   1 +
>  drivers/phy/samsung/phy-tesla-pcie.c | 397 +++++++++++++++++++++++++++
>  3 files changed, 408 insertions(+)
>  create mode 100644 drivers/phy/samsung/phy-tesla-pcie.c
> 

Same comment as for PCI. It's the third driver for Samsung Exynos PCIe
PHY and every time you will say "but it is different".

No, work on unified approach not 3 different drivers.

Best regards,
Krzysztof
Krzysztof Kozlowski Nov. 21, 2022, 12:11 p.m. UTC | #3
On 21/11/2022 11:52, Shradha Todi wrote:
> Add the support for PCIe controller driver and phy driver
> for Tesla FSD. It includes support for both RC and EP.
> 
> Signed-off-by: Niyas Ahmed S T <niyas.ahmed@samsung.com>
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> Signed-off-by: Shradha Todi <shradha.t@samsung.com>
> ---
>  arch/arm64/boot/dts/tesla/fsd-evb.dts      |  48 ++++++
>  arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi |  65 ++++++++
>  arch/arm64/boot/dts/tesla/fsd.dtsi         | 171 +++++++++++++++++++++
>  3 files changed, 284 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/tesla/fsd-evb.dts b/arch/arm64/boot/dts/tesla/fsd-evb.dts
> index 1db6ddf03f01..cda72b0f76f8 100644
> --- a/arch/arm64/boot/dts/tesla/fsd-evb.dts
> +++ b/arch/arm64/boot/dts/tesla/fsd-evb.dts
> @@ -41,3 +41,51 @@
>  &ufs {
>  	status = "okay";
>  };
> +
> +&pcie_phy0 {
> +	status = "disabled";

It's a double disable, isn't it?

> +};
> +
> +&pcie_phy1 {
> +	status = "disabled";
> +};
> +
> +&pcie4_rc {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pcie1_clkreq>, <&pcie1_wake>, <&pcie1_preset>,
> +			<&pcie0_slot1>;
> +	status = "disabled";

???

> +};
> +
> +&pcie4_ep {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pcie1_clkreq>, <&pcie1_wake>, <&pcie1_preset>,
> +			<&pcie0_slot1>;
> +	status = "disabled";
> +};
> +
> +&pcie0_rc {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pcie0_clkreq>, <&pcie0_wake0>, <&pcie0_preset0>,
> +			 <&pcie0_slot0>;
> +	status = "disabled";
> +};
> +
> +&pcie0_ep {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pcie0_clkreq>, <&pcie0_wake0>, <&pcie0_preset0>,
> +			 <&pcie0_slot0>;
> +	status = "disabled";
> +};
> +
> +&pcie1_rc {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pcie0_clkreq>, <&pcie0_wake1>, <&pcie0_preset0>;
> +	status = "disabled";
> +};
> +
> +&pcie1_ep {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pcie0_clkreq>, <&pcie0_wake1>, <&pcie0_preset0>;
> +	status = "disabled";

Ordering is broken. All overrides/extends are ordered by label name.

> +};
> diff --git a/arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi b/arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi
> index d0abb9aa0e9e..edae62dfa987 100644
> --- a/arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi
> +++ b/arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi
> @@ -64,6 +64,27 @@
>  		samsung,pin-pud = <FSD_PIN_PULL_NONE>;
>  		samsung,pin-drv = <FSD_PIN_DRV_LV2>;
>  	};
> +
> +	pcie1_clkreq: pcie1-clkreq {

Does not look like you tested the DTS against bindings. Please run `make
dtbs_check` (see Documentation/devicetree/bindings/writing-schema.rst
for instructions).

(...)

>  
>  &pinctrl_pmu {
> diff --git a/arch/arm64/boot/dts/tesla/fsd.dtsi b/arch/arm64/boot/dts/tesla/fsd.dtsi
> index f35bc5a288c2..2177f6964553 100644
> --- a/arch/arm64/boot/dts/tesla/fsd.dtsi
> +++ b/arch/arm64/boot/dts/tesla/fsd.dtsi
> @@ -32,6 +32,14 @@
>  		spi0 = &spi_0;
>  		spi1 = &spi_1;
>  		spi2 = &spi_2;
> +		pciephy0 = &pcie_phy0;
> +		pciephy1 = &pcie_phy1;
> +		pcierc0 = &pcie0_rc;
> +		pcieep0 = &pcie0_ep;
> +		pcierc1 = &pcie1_rc;
> +		pcieep1 = &pcie1_ep;
> +		pcierc2 = &pcie4_rc;
> +		pcieep2 = &pcie4_ep;

Since these are disabled, aliases do not belong to DTSI, but to board.

Also, explain why do you need them.

>  	};
>  
>  	cpus {
> @@ -860,6 +868,169 @@
>  			clocks = <&clock_fsys0 UFS0_MPHY_REFCLK_IXTAL26>;
>  			clock-names = "ref_clk";
>  		};
> +
> +		pcie_phy0: pcie-phy@15080000 {
> +			compatible = "tesla,fsd-pcie-phy";
> +			#phy-cells = <0>;
> +			reg = <0x0 0x15080000 0x0 0x2000>,
> +			      <0x0 0x150A0000 0x0 0x1000>;
> +			reg-names = "phy", "pcs";
> +			samsung,pmureg-phandle = <&pmu_system_controller>;
> +			tesla,pcie-sysreg = <&sysreg_fsys0>;
> +			phy-mode = <0>;
> +			status = "disabled";
> +		};
> +
> +		pcie_phy1: pcie-phy@16880000 {
> +			compatible = "tesla,fsd-pcie-phy";
> +			#phy-cells = <0>;
> +			reg = <0x0 0x16880000 0x0 0x2000>,
> +			      <0x0 0x16860000 0x0 0x1000>;
> +			reg-names = "phy", "pcs";
> +			samsung,pmureg-phandle = <&pmu_system_controller>;
> +			tesla,pcie-sysreg = <&sysreg_fsys1>;
> +			phy-mode = <0>;
> +			status = "disabled";
> +		};
> +
> +		pcie4_rc: pcie@15400000 {

Not ordered. Keep nodes sorted by unit address, at least within the
group of devices you add.

> +			compatible = "tesla,fsd-pcie";

reg is second property. reg-names and ranges follow.

> +			clocks = <&clock_fsys0 PCIE_SUBCTRL_INST0_AUX_CLK_SOC>,
> +				 <&clock_fsys0 PCIE_SUBCTRL_INST0_DBI_ACLK_SOC>,
> +				 <&clock_fsys0 PCIE_SUBCTRL_INST0_MSTR_ACLK_SOC>,
> +				 <&clock_fsys0 PCIE_SUBCTRL_INST0_SLV_ACLK_SOC>;
> +			clock-names = "aux_clk", "dbi_clk", "mstr_clk", "slv_clk";
> +			#address-cells = <3>;
> +			#size-cells = <2>;

Best regards,
Krzysztof
Rob Herring (Arm) Nov. 30, 2022, 6:44 p.m. UTC | #4
On Mon, Nov 21, 2022 at 04:22:07PM +0530, Shradha Todi wrote:
> Add PCIe controller driver file for PCIe controller
> found in fsd SoC family. This driver adds support for both RC
> and EP mode.
> 
> Signed-off-by: Niyas Ahmed S T <niyas.ahmed@samsung.com>
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> Signed-off-by: Padmanabhan Rajanbabu <p.rajanbabu@samsung.com>
> Signed-off-by: Shradha Todi <shradha.t@samsung.com>
> ---
>  drivers/pci/controller/dwc/Kconfig    |   35 +
>  drivers/pci/controller/dwc/Makefile   |    1 +
>  drivers/pci/controller/dwc/pcie-fsd.c | 1021 +++++++++++++++++++++++++
>  3 files changed, 1057 insertions(+)
>  create mode 100644 drivers/pci/controller/dwc/pcie-fsd.c
> 
> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> index 62ce3abf0f19..9a3d194c979f 100644
> --- a/drivers/pci/controller/dwc/Kconfig
> +++ b/drivers/pci/controller/dwc/Kconfig
> @@ -14,6 +14,41 @@ config PCIE_DW_EP
>  	bool
>  	select PCIE_DW
>  
> +config PCIE_FSD
> +	bool "Samsung FSD PCIe Controller"
> +	default n
> +	help
> +	  Enables support for the PCIe controller in the FSD SoC. There are
> +	  total three instances of PCIe controller in FSD. This controller
> +	  can work either in RC or EP mode. In order to enable host-specific
> +	  features, PCI_FSD_HOST must be selected and in order to enable
> +	  device-specific feature PCI_FSD_EP must be selected.
> +
> +config PCIE_FSD_HOST
> +	bool "PCIe FSD Host Mode"
> +	depends on PCI
> +	depends on PCI_MSI_IRQ_DOMAIN || PCI_DOMAIN
> +	select PCIE_DW_HOST
> +	select PCIE_FSD
> +	default n
> +	help
> +	  Enables support for the PCIe controller in the FSD SoC to work in
> +	  host (RC) mode. In order to enable host-specific features,
> +	  PCIE_DW_HOST must be selected. PCIE_FSD should be selected for
> +	  fsd controller specific settings.
> +
> +config PCIE_FSD_EP
> +	bool "PCIe FSD Endpoint Mode"
> +	depends on PCI_ENDPOINT
> +	select PCIE_DW_EP
> +	select PCIE_FSD
> +	default n
> +	help
> +	  Enables support for the PCIe controller in the FSD SoC to work in
> +	  endpoint mode. In order to enable device-specific feature
> +	  PCI_FSD_EP must be selected. PCIE_FSD should be selected for
> +	  fsd controller specific settings.
> +
>  config PCI_DRA7XX
>  	tristate
>  
> diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
> index 8ba7b67f5e50..b76fa6b4e79f 100644
> --- a/drivers/pci/controller/dwc/Makefile
> +++ b/drivers/pci/controller/dwc/Makefile
> @@ -25,6 +25,7 @@ obj-$(CONFIG_PCIE_TEGRA194) += pcie-tegra194.o
>  obj-$(CONFIG_PCIE_UNIPHIER) += pcie-uniphier.o
>  obj-$(CONFIG_PCIE_UNIPHIER_EP) += pcie-uniphier-ep.o
>  obj-$(CONFIG_PCIE_VISCONTI_HOST) += pcie-visconti.o
> +obj-$(CONFIG_PCIE_FSD) += pcie-fsd.o
>  
>  # The following drivers are for devices that use the generic ACPI
>  # pci_root.c driver but don't support standard ECAM config access.
> diff --git a/drivers/pci/controller/dwc/pcie-fsd.c b/drivers/pci/controller/dwc/pcie-fsd.c
> new file mode 100644
> index 000000000000..4531efbfc313
> --- /dev/null
> +++ b/drivers/pci/controller/dwc/pcie-fsd.c
> @@ -0,0 +1,1021 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * PCIe host controller driver for Tesla fsd SoC
> + *
> + * Copyright (C) 2017-2022 Samsung Electronics Co., Ltd. http://www.samsung.com
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>

You shouldn't need this header.

> +#include <linux/of_device.h>
> +#include <linux/of_gpio.h>

You shouldn't need this header.

> +#include <linux/pci.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/resource.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/types.h>
> +
> +#include "pcie-designware.h"
> +
> +#define to_fsd_pcie(x)	dev_get_drvdata((x)->dev)
> +
> +/* PCIe ELBI registers */
> +#define PCIE_APP_LTSSM_ENABLE		0x054
> +#define PCIE_ELBI_LTSSM_ENABLE		0x1
> +#define PCIE_ELBI_LTSSM_DISABLE		0x0
> +#define PCIE_ELBI_CXPL_DEBUG_00_31	0x2C8
> +#define LTSSM_STATE_MASK		0x3f
> +#define LTSSM_STATE_L0			0x11
> +#define PCIE_FSD_DEVICE_TYPE		0x080
> +#define DEVICE_TYPE_RC			0x4
> +#define DEVICE_TYPE_EP			0x0
> +#define IRQ_MSI_ENABLE			BIT(17)
> +#define IRQ0_EN				0x10
> +#define IRQ1_EN				0x14
> +#define IRQ2_EN				0x18
> +#define IRQ5_EN				0x1c
> +#define IRQ0_STS			0x0
> +#define IRQ1_STS			0x4
> +#define IRQ2_STS			0x8
> +#define IRQ5_STS			0xc
> +
> +/* Gen3 Control Register */
> +#define PCIE_GEN3_RELATED_OFF		0x890
> +/* Parameters for equalization feature */
> +#define PCIE_GEN3_EQUALIZATION_DISABLE	BIT(16)
> +#define PCIE_GEN3_EQ_PHASE_2_3		BIT(9)
> +#define PCIE_GEN3_RXEQ_PH01_EN		BIT(12)
> +#define PCIE_GEN3_RXEQ_RGRDLESS_RXTS	BIT(13)
> +
> +/**
> + * struct fsd_pcie - representation of the pci controller
> + * @pci: representation of dwc pcie device structure
> + * @aux_clk: auxiliary clock for the pci block
> + * @dbi_clk: DBI clock
> + * @mstr_clk: master clock
> + * @slv_clk: slave clock
> + * @pdata: private data to determine the oprations supported by device
> + * @appl_base: represent the appl base
> + * @sysreg: represent the system register base
> + * @sysreg_base: represents the offset of the system register required
> + * @phy: represents the phy device associated for the controller
> + */
> +struct fsd_pcie {
> +	struct dw_pcie *pci;
> +	struct clk *aux_clk;
> +	struct clk *dbi_clk;
> +	struct clk *mstr_clk;
> +	struct clk *slv_clk;
> +	const struct fsd_pcie_pdata *pdata;
> +	void __iomem *appl_base;
> +	struct regmap *sysreg;
> +	unsigned int sysreg_base;
> +	struct phy *phy;
> +};
> +
> +enum fsd_pcie_addr_type {
> +	ADDR_TYPE_DBI = 0x0,
> +	ADDR_TYPE_DBI2 = 0x32,
> +	ADDR_TYPE_ATU = 0x36,
> +	ADDR_TYPE_DMA = 0x37,
> +};
> +
> +enum IRQ0_ERR_BITS {
> +	APP_PARITY_ERRS_0,
> +	APP_PARITY_ERRS_1,
> +	APP_PARITY_ERRS_2,
> +	CFG_BW_MGT_INT = 4,
> +	CFG_LINK_AUTO_BW_INT,
> +	CFG_SYS_ERR_RC = 7,
> +	DPA_SUBSTATE_UPDATE,
> +	FLUSH_DONE,
> +	RADM_CORRECTABLE_ERR = 12,
> +	RADM_FATAL_ERR,
> +	RADM_MSG_CPU_ACTIVE = 22,
> +	RADM_MSG_IDLE,
> +	RADM_MSG_LTR,
> +	RADM_MSG_OBFF,
> +	RADM_MSG_UNLOCK,
> +	RADM_NONFATAL_ERR,
> +	RADM_PM_PME,
> +	RADM_PM_TO_ACK,
> +	RADM_PM_TURNOFF,
> +	RADM_VENDOR_MSG,
> +};
> +
> +enum IRQ1_ERR_BITS {
> +	TRGT_CPL_TIMEOUT = 0,
> +	VEN_MSG_GRANT,
> +	VEN_MSI_GRANT,
> +};
> +
> +enum IRQ2_ERR_BITS {
> +	APP_LTR_MSG_GRANT = 0,
> +	APP_OBFF_MSG_GRANT,
> +	CFG_AER_RC_ERR_INT,
> +	CFG_BUS_MASTER_EN,
> +	CFG_LINK_EQ_REQ_INT,
> +	CFG_PME_INT,
> +	EDMA_INT_0,
> +	EDMA_INT_1,
> +	EDMA_INT_2,
> +	EDMA_INT_3,
> +	EDMA_INT_4,
> +	EDMA_INT_5,
> +	EDMA_INT_6,
> +	EDMA_INT_7,
> +	PM_LINKST_IN_L0S = 18,
> +	PM_LINKST_IN_L1,
> +	PM_LINKST_IN_L1SUB_0,
> +	PM_LINKST_IN_L2,
> +	PM_LINKST_L2_EXIT,
> +	PM_XTLH_BLOCK_TLP,
> +	RADM_CPL_TIMEOUT,
> +	RADM_Q_NOT_EMPTY,
> +	RDLH_LINK_UP_0,
> +	SMLH_LINK_UP = 29,
> +	WAKE,
> +	COMPARE_END_CHECKER,
> +};
> +
> +enum IRQ5_ERR_BITS {
> +	LINK_REQ_RST_NOT,
> +	PM_LINKST_IN_L1SUB_1,
> +	RDLH_LINK_UP_1,
> +	SMLH_REQ_RST_NOT,
> +};
> +
> +struct fsd_pcie_res_ops {
> +	int (*get_mem_resources)(struct platform_device *pdev,
> +				 struct fsd_pcie *fsd_ctrl);
> +	int (*get_clk_resources)(struct platform_device *pdev,
> +				 struct fsd_pcie *fsd_ctrl);
> +	int (*init_clk_resources)(struct fsd_pcie *fsd_ctrl);
> +	void (*deinit_clk_resources)(struct fsd_pcie *fsd_ctrl);
> +};
> +
> +struct fsd_pcie_irq {
> +	irqreturn_t (*pcie_msi_irq_handler)(int irq, void *arg);
> +	void (*pcie_msi_init)(struct fsd_pcie *fsd_ctrl);
> +	irqreturn_t (*pcie_sub_ctrl_handler)(int irq, void *arg);
> +};

Why the indirection? You only have 1 version of all these functions.

> +
> +struct fsd_pcie_pdata {
> +	const struct dw_pcie_ops *dwc_ops;
> +	struct dw_pcie_host_ops	*host_ops;
> +	const struct fsd_pcie_res_ops *res_ops;
> +	const struct fsd_pcie_irq *irq_data;
> +	unsigned int appl_cxpl_debug_00_31;
> +	int op_mode;
> +};
> +
> +static int fsd_pcie_get_mem_resources(struct platform_device *pdev,
> +					  struct fsd_pcie *fsd_ctrl)
> +{
> +	struct resource *res;
> +	struct device *dev = &pdev->dev;
> +	int ret;
> +
> +	/* External Local Bus interface(ELBI) Register */
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "appl");
> +	if (!res)
> +		return -EINVAL;
> +	fsd_ctrl->appl_base = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(fsd_ctrl->appl_base)) {
> +		dev_err(dev, "Failed to map appl_base\n");
> +		return PTR_ERR(fsd_ctrl->appl_base);
> +	}
> +
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi");

The DW core code takes care of this.

> +	if (!res)
> +		return -EINVAL;
> +	fsd_ctrl->pci->dbi_base = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(fsd_ctrl->pci->dbi_base)) {
> +		dev_err(dev, "failed to map dbi_base\n");
> +		return PTR_ERR(fsd_ctrl->pci->dbi_base);
> +	}
> +
> +	/* sysreg regmap handle */
> +	fsd_ctrl->sysreg = syscon_regmap_lookup_by_phandle(dev->of_node,
> +			"tesla,pcie-sysreg");
> +	if (IS_ERR(fsd_ctrl->sysreg)) {
> +		dev_err(dev, "Sysreg regmap lookup failed.\n");
> +		return PTR_ERR(fsd_ctrl->sysreg);
> +	}
> +
> +	ret = of_property_read_u32_index(dev->of_node, "tesla,pcie-sysreg", 1,
> +					 &fsd_ctrl->sysreg_base);
> +	if (ret) {
> +		dev_err(dev, "Couldn't get the register offset for syscon!\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int fsd_pcie_get_clk_resources(struct platform_device *pdev,
> +				       struct fsd_pcie *fsd_ctrl)
> +{
> +	struct device *dev = &pdev->dev;
> +
> +	fsd_ctrl->aux_clk = devm_clk_get(dev, "aux_clk");
> +	if (IS_ERR(fsd_ctrl->aux_clk)) {
> +		dev_err(dev, "couldn't get aux clock\n");
> +		return -EINVAL;
> +	}
> +
> +	fsd_ctrl->dbi_clk = devm_clk_get(dev, "dbi_clk");
> +	if (IS_ERR(fsd_ctrl->dbi_clk)) {
> +		dev_err(dev, "couldn't get dbi clk\n");
> +		return -EINVAL;
> +	}
> +
> +	fsd_ctrl->slv_clk = devm_clk_get(dev, "slv_clk");
> +	if (IS_ERR(fsd_ctrl->slv_clk)) {
> +		dev_err(dev, "couldn't get slave clock\n");
> +		return -EINVAL;
> +	}
> +
> +	fsd_ctrl->mstr_clk = devm_clk_get(dev, "mstr_clk");
> +	if (IS_ERR(fsd_ctrl->mstr_clk)) {
> +		dev_err(dev, "couldn't get master clk\n");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int fsd_pcie_init_clk_resources(struct fsd_pcie *fsd_ctrl)
> +{
> +	clk_prepare_enable(fsd_ctrl->aux_clk);
> +	clk_prepare_enable(fsd_ctrl->dbi_clk);
> +	clk_prepare_enable(fsd_ctrl->mstr_clk);
> +	clk_prepare_enable(fsd_ctrl->slv_clk);
> +
> +	return 0;
> +}
> +
> +static void fsd_pcie_deinit_clk_resources(struct fsd_pcie *fsd_ctrl)
> +{
> +	clk_disable_unprepare(fsd_ctrl->slv_clk);
> +	clk_disable_unprepare(fsd_ctrl->mstr_clk);
> +	clk_disable_unprepare(fsd_ctrl->dbi_clk);
> +	clk_disable_unprepare(fsd_ctrl->aux_clk);
> +}
> +
> +static const struct fsd_pcie_res_ops fsd_pcie_res_ops_data = {
> +	.get_mem_resources	= fsd_pcie_get_mem_resources,
> +	.get_clk_resources	= fsd_pcie_get_clk_resources,
> +	.init_clk_resources	= fsd_pcie_init_clk_resources,
> +	.deinit_clk_resources	= fsd_pcie_deinit_clk_resources,
> +};
> +
> +static void fsd_pcie_stop_link(struct dw_pcie *pci)
> +{
> +	u32 reg;
> +	struct fsd_pcie *fsd_ctrl = to_fsd_pcie(pci);
> +
> +	reg = readl(fsd_ctrl->appl_base + PCIE_APP_LTSSM_ENABLE);
> +	reg &= ~PCIE_ELBI_LTSSM_ENABLE;
> +	writel(reg, fsd_ctrl->appl_base + PCIE_APP_LTSSM_ENABLE);
> +}
> +
> +static int fsd_pcie_establish_link(struct dw_pcie *pci)
> +{
> +	struct device *dev = pci->dev;
> +	struct fsd_pcie *fsd_ctrl = to_fsd_pcie(pci);
> +	struct dw_pcie_ep *ep;
> +
> +	if (dw_pcie_link_up(pci)) {
> +		dev_info(dev, "Link already up\n");

Print messages on failure, not normal operation.

> +		return 0;
> +	}
> +
> +	/* assert LTSSM enable */
> +	writel(PCIE_ELBI_LTSSM_ENABLE, fsd_ctrl->appl_base +
> +			PCIE_APP_LTSSM_ENABLE);
> +
> +	/* check if the link is up or not */
> +	if (!dw_pcie_wait_for_link(pci)) {

IIRC, the DW core will do the wait for you.

> +		dev_info(dev, "Link up done successfully\n");
> +		if (fsd_ctrl->pdata->op_mode == DEVICE_TYPE_EP) {
> +			ep = &pci->ep;
> +			dw_pcie_ep_linkup(ep);
> +		}
> +		return 0;
> +	}
> +
> +	if (fsd_ctrl->pdata->op_mode == DEVICE_TYPE_RC) {
> +		/* Return success as link might come up later */
> +		return 0;
> +	}
> +
> +	return -ETIMEDOUT;
> +}
> +
> +static void handle_irq0_interrupts(u32 val, u32 is_en)
> +{
> +	u32 bit_off = 0;
> +
> +	if (val) {
> +		while (bit_off < 32) {
> +			if ((val & (0x1 << bit_off)) == 0 || (is_en &
> +						(0x1 << bit_off)) == 0) {
> +				bit_off++;
> +				continue;
> +			}
> +			switch (bit_off) {
> +			case RADM_VENDOR_MSG:
> +				pr_info("Interrupt received for\n");

Printing messages is not handling an interrupt. Remove all these.

> +				break;
> +			case RADM_PM_TURNOFF:
> +				pr_info("Interrupt received for RADM_PM_TURNOFF\n");
> +				break;
> +			case RADM_PM_TO_ACK:
> +				pr_info("Interrupt received for RADM_PM_TO_ACK\n");
> +				break;
> +			case RADM_PM_PME:
> +				pr_info("Interrupt received for RADM_PM_PME\n");
> +				break;
> +			case RADM_NONFATAL_ERR:
> +				pr_info("Interrupt received for RADM_NONFATAL_ERR\n");
> +				break;
> +			case RADM_MSG_UNLOCK:
> +				pr_info("Interrupt received for RADM_MSG_UNLOCK\n");
> +				break;
> +			case RADM_MSG_OBFF:
> +				pr_info("Interrupt received for RADM_MSG_OBFF\n");
> +				break;
> +			case RADM_MSG_LTR:
> +				pr_info("Interrupt received for RADM_MSG_LTR\n");
> +				break;
> +			case RADM_MSG_IDLE:
> +				pr_info("Interrupt received for RADM_MSG_IDLE\n");
> +				break;
> +			case RADM_MSG_CPU_ACTIVE:
> +				pr_info("Interrupt received for RADM_MSG_CPU_ACTIVE\n");
> +				break;
> +			case RADM_FATAL_ERR:
> +				pr_info("Interrupt received for RADM_FATAL_ERR\n");
> +				break;
> +			case RADM_CORRECTABLE_ERR:
> +				pr_info("Interrupt received for RADM_CORRECTABLE_ERR\n");
> +				break;
> +			case FLUSH_DONE:
> +				pr_info("Interrupt received for FLUSH_DONE\n");
> +				break;
> +			case DPA_SUBSTATE_UPDATE:
> +				pr_info("Interrupt received for DPA_SUBSTATE_UPDATE\n");
> +				break;
> +			case CFG_SYS_ERR_RC:
> +				pr_info("Interrupt received for CFG_SYS_ERR_RC\n");
> +				break;
> +			case CFG_LINK_AUTO_BW_INT:
> +				pr_info("Interrupt received for CFG_LINK_AUTO_BW_INT\n");
> +				break;
> +			case CFG_BW_MGT_INT:
> +				pr_info("Interrupt received for CFG_BW_MGT_INT\n");
> +				break;
> +			case APP_PARITY_ERRS_2:
> +				pr_info("Interrupt received for APP_PARITY_ERRS_2\n");
> +				break;
> +			case APP_PARITY_ERRS_1:
> +				pr_info("Interrupt received for APP_PARITY_ERRS_1\n");
> +				break;
> +			case APP_PARITY_ERRS_0:
> +				pr_info("Interrupt received for APP_PARITY_ERRS_0\n");
> +				break;
> +			default:
> +				pr_info("Unknown Interrupt in IRQ0[%d]\n", bit_off);
> +				break;
> +			}
> +			bit_off++;
> +		}
> +	}
> +}
> +
> +static void handle_irq1_interrupts(u32 val, u32 is_en)
> +{
> +	u32 bit_off = 0;
> +
> +	if (val) {
> +		while (bit_off < 32) {
> +			if ((val & (0x1 << bit_off)) == 0 || (is_en &
> +						(0x1 << bit_off)) == 0) {
> +				bit_off++;
> +				continue;
> +			}
> +			switch (bit_off) {
> +			case TRGT_CPL_TIMEOUT:
> +				pr_info("Interrupt for TRGT_CPL_TIMEOUT\n");
> +				break;
> +			case VEN_MSG_GRANT:
> +				pr_info("Interrupt for VEN_MSG_GRANT\n");
> +				break;
> +			case VEN_MSI_GRANT:
> +				pr_info("Interrupt for VEN_MSI_GRANT\n");
> +				break;
> +			default:
> +				pr_info("Unknown Interrupt in IRQ1[%d]\n", bit_off);
> +				break;
> +			}
> +			bit_off++;
> +		}
> +	}
> +}
> +
> +static void handle_irq2_interrupts(u32 val, u32 is_en)
> +{
> +	u32 bit_off = 0;
> +
> +	if (val) {
> +		while (bit_off < 32) {
> +			if ((val & (0x1 << bit_off)) == 0 || (is_en &
> +						(0x1 << bit_off)) == 0) {
> +				bit_off++;
> +				continue;
> +			}
> +			switch (bit_off) {
> +			/* To indicate that controller has accepted to send
> +			 * Latency Tolerance reporting message
> +			 */
> +			case APP_LTR_MSG_GRANT:
> +				pr_info("Interrupt for APP_LTR_MSG_GRANT\n");
> +				break;
> +			case APP_OBFF_MSG_GRANT:
> +				pr_info("Interrupt for APP_OBFF_MSG_GRANT\n");
> +				break;
> +			case CFG_AER_RC_ERR_INT:
> +				pr_info("Interrupt for CFG_AER_RC_ERR_INT\n");
> +				break;
> +			/* IRQ when bus master is enabled */
> +			case CFG_BUS_MASTER_EN:
> +				pr_info("Interrupt for CFG_BUS_MASTER_EN\n");
> +				break;
> +			/* IRQ to indicate that link Equalization request has been set */
> +			case CFG_LINK_EQ_REQ_INT:
> +				pr_info("Interrupt for CFG_LINK_EQ_REQ_INT\n");
> +				break;
> +			case CFG_PME_INT:
> +				pr_info("Interrupt for CFG_PME_INIT\n");
> +				break;
> +			case EDMA_INT_0:
> +			case EDMA_INT_1:
> +			case EDMA_INT_2:
> +			case EDMA_INT_3:
> +			case EDMA_INT_4:
> +			case EDMA_INT_5:
> +			case EDMA_INT_6:
> +			case EDMA_INT_7:
> +				pr_info("Interrupt for DMA\n");
> +				break;
> +			/* IRQ when link entres L0s */
> +			case PM_LINKST_IN_L0S:
> +				pr_info("Interrupt for PM_LINKST_IN_L0S\n");
> +				break;
> +			/* IRQ when link enters L1 */
> +			case PM_LINKST_IN_L1:
> +				pr_info("Interrupt for PM_LINKST_IN_L1\n");
> +				break;
> +			/* IRQ when link enters L1 substate */
> +			case PM_LINKST_IN_L1SUB_0:
> +				pr_info("Interrupt for PM_LINKST_IN_L1SUB_0\n");
> +				break;
> +			/* IRQ when link enters L2 */
> +			case PM_LINKST_IN_L2:
> +				pr_info("Interrupt for PM_LINKST_IN_L2\n");
> +				break;
> +			/* IRQ when link exits L2 */
> +			case PM_LINKST_L2_EXIT:
> +				pr_info("Interrupt for PM_LINKST_L2_EXIT\n");
> +				break;
> +			/* Indicates that application must stop sending new
> +			 * outbound TLP requests due to current power state
> +			 */
> +			case PM_XTLH_BLOCK_TLP:
> +				pr_info("Interrupt for PM_XTLH_BLOCK_TLP\n");
> +				break;
> +			/* Request failed to complete in time */
> +			case RADM_CPL_TIMEOUT:
> +				pr_info("Interrupt for RADM_CPL_TIMEOUT\n");
> +				break;
> +			/* Level indicating that receive queues contain TLP header/data */
> +			case RADM_Q_NOT_EMPTY:
> +				pr_info("Interrupt for RADM_Q_NOT_EMPTY\n");
> +				break;
> +			/* Data link layer up/down indicator */
> +			case RDLH_LINK_UP_0:
> +				pr_info("Interrupt for RDLH_LINK_UP_0\n");
> +				break;
> +			/* Phy link up/down indicator */
> +			case SMLH_LINK_UP:
> +				pr_info("Interrupt for SMLH_LINK_UP\n");
> +				break;
> +			case WAKE:
> +				pr_info("Interrupt for WAKE\n");
> +				break;
> +			case COMPARE_END_CHECKER:
> +				pr_info("Interrupt for COMPARE_END_CHECKER\n");
> +				break;
> +			default:
> +				pr_info("Unknown Interrupt in IRQ2[%d]\n", bit_off);
> +				break;
> +			}
> +			bit_off++;
> +		}
> +	}
> +}
> +
> +static void handle_irq5_interrupts(u32 val, u32 is_en)
> +{
> +	u32 bit_off = 0;
> +
> +	if (val) {
> +		while (bit_off < 32) {
> +			if ((val & (0x1 << bit_off)) == 0 || (is_en &
> +						(0x1 << bit_off)) == 0) {
> +				bit_off++;
> +				continue;
> +			}
> +			switch (bit_off) {
> +			case LINK_REQ_RST_NOT:
> +				pr_info("Interrupt for LINK_REQ_RST_NOT\n");
> +				break;
> +			case PM_LINKST_IN_L1SUB_1:
> +				pr_info("Interrupt for L1 SUB state Exit\n");
> +				break;
> +			case RDLH_LINK_UP_1:
> +				pr_info("Interrupt for RDLH_LINK_UP_1\n");
> +				break;
> +			/* Reset request because PHY link went down/ or got hot reset */
> +			case SMLH_REQ_RST_NOT:
> +				pr_info("Interrupt for SMLH_REQ_RST_NOT\n");
> +				break;
> +			default:
> +				pr_info("Unknown Interrupt in IRQ5[%d]\n", bit_off);
> +				break;
> +			}
> +			bit_off++;
> +		}
> +	}
> +}
> +
> +/*
> + * fsd_pcie_sub_ctrl_handler : Interrupt handler for all PCIe interrupts.
> + *
> + * These interrupts trigger on different events happening in the PCIe
> + * controller like link status, link entering and exiting low power
> + * states like L0s, L1, DMA completion/abort interrupts, wake being
> + * triggered and other information.
> + *
> + * IRQ_0: (offset 0x0): IRQ for pulse output 1
> + *	Enable these interrupts at offset 0x10
> + * IRQ_1: (offset 0x4): IRQ for pulse output 2
> + *	Enable these interrupts at offset 0x14
> + * IRQ_2: (offset 0x8): IRQ for level output, rising edge
> + *	Enable these interrupts at offset 0x18
> + * IRQ_5: (offset 0xC): IRQ for level output, falling edge
> + *	Enable these interrupts at offset 0x1C
> + */
> +
> +static irqreturn_t fsd_pcie_sub_ctrl_handler(int irq, void *arg)
> +{
> +	u32 irq0, irq1, irq2, irq5;
> +	struct fsd_pcie *fsd_ctrl = arg;
> +	u32 irq0_en, irq1_en, irq2_en, irq5_en;
> +
> +	/* Read IRQ0 status */
> +	irq0 = readl(fsd_ctrl->appl_base + IRQ0_STS);
> +	/* Clear IRQ0 status after storing status value */
> +	writel(irq0, fsd_ctrl->appl_base + IRQ0_STS);
> +
> +	/* Read IRQ1 status */
> +	irq1 = readl(fsd_ctrl->appl_base + IRQ1_STS);
> +	/* Clear IRQ1 status after storing status value */
> +	writel(irq1, fsd_ctrl->appl_base + IRQ1_STS);
> +
> +	/* Read IRQ2 status */
> +	irq2 = readl(fsd_ctrl->appl_base + IRQ2_STS);
> +	/* Clear IRQ2 status after storing status value */
> +	writel(irq2, fsd_ctrl->appl_base + IRQ2_STS);
> +
> +	/* Read IRQ5 status */
> +	irq5 = readl(fsd_ctrl->appl_base + IRQ5_STS);
> +	/* Clear IRQ5 status after storing status value */
> +	writel(irq5, fsd_ctrl->appl_base + IRQ5_STS);
> +
> +	irq0_en = readl(fsd_ctrl->appl_base + IRQ0_EN);
> +	irq1_en = readl(fsd_ctrl->appl_base + IRQ1_EN);
> +	irq2_en = readl(fsd_ctrl->appl_base + IRQ2_EN);
> +	irq5_en = readl(fsd_ctrl->appl_base + IRQ5_EN);
> +	/* Handle all interrupts */
> +	handle_irq0_interrupts(irq0, irq0_en);
> +	handle_irq1_interrupts(irq1, irq1_en);
> +	handle_irq2_interrupts(irq2, irq2_en);
> +	handle_irq5_interrupts(irq5, irq5_en);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t fsd_pcie_msi_irq_handler(int irq, void *arg)
> +{
> +	u32 val;
> +	struct fsd_pcie *fsd_ctrl = arg;
> +	struct dw_pcie *pci = fsd_ctrl->pci;
> +	struct dw_pcie_rp *pp = &pci->pp;
> +
> +	val = readl(fsd_ctrl->appl_base + IRQ2_STS);
> +
> +	if ((val & IRQ_MSI_ENABLE) == IRQ_MSI_ENABLE) {
> +		val &= IRQ_MSI_ENABLE;
> +		writel(val, fsd_ctrl->appl_base + IRQ2_STS);
> +		dw_handle_msi_irq(pp);
> +	} else {
> +		fsd_pcie_sub_ctrl_handler(irq, arg);
> +	}
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static void fsd_pcie_msi_init(struct fsd_pcie *fsd_ctrl)
> +{
> +	int val;
> +
> +	/* enable MSI interrupt */
> +	val = readl(fsd_ctrl->appl_base + IRQ2_EN);
> +	val |= IRQ_MSI_ENABLE;
> +	writel(val, fsd_ctrl->appl_base + IRQ2_EN);
> +}
> +
> +static void fsd_pcie_enable_interrupts(struct fsd_pcie *fsd_ctrl)
> +{
> +	if (IS_ENABLED(CONFIG_PCI_MSI))
> +		fsd_ctrl->pdata->irq_data->pcie_msi_init(fsd_ctrl);
> +}
> +
> +static u32 fsd_pcie_read_dbi(struct dw_pcie *pci, void __iomem *base,
> +				u32 reg, size_t size)
> +{
> +	bool is_atu = false;
> +	struct fsd_pcie *fsd_ctrl = to_fsd_pcie(pci);
> +	u32 val;
> +
> +	if (pci->atu_base) {
> +		if (base >= pci->atu_base) {
> +
> +			is_atu = true;
> +			regmap_write(fsd_ctrl->sysreg, fsd_ctrl->sysreg_base,
> +					ADDR_TYPE_ATU);
> +			base = base - DEFAULT_DBI_ATU_OFFSET;
> +		}
> +	}
> +
> +	dw_pcie_read(base + reg, size, &val);
> +
> +	if (is_atu)
> +		regmap_write(fsd_ctrl->sysreg, fsd_ctrl->sysreg_base, ADDR_TYPE_DBI);

You've got the same code twice. Rework this to be a common function 
that does just:

if (ATU)
    regmap_write(fsd_ctrl->sysreg, fsd_ctrl->sysreg_base, ADDR_TYPE_ATU);
else
    regmap_write(fsd_ctrl->sysreg, fsd_ctrl->sysreg_base, ADDR_TYPE_DBI);

Then you just call it followed by dw_pcie_read or dw_pcie_write.

> +
> +	return val;
> +}
> +
> +static void fsd_pcie_write_dbi(struct dw_pcie *pci, void __iomem *base,
> +				u32 reg, size_t size, u32 val)
> +{
> +	struct fsd_pcie *fsd_ctrl = to_fsd_pcie(pci);
> +	bool is_atu = false;
> +
> +	if (pci->atu_base) {
> +		if (base >= pci->atu_base) {
> +			is_atu = true;
> +			regmap_write(fsd_ctrl->sysreg, fsd_ctrl->sysreg_base,
> +					ADDR_TYPE_ATU);
> +			base = base - DEFAULT_DBI_ATU_OFFSET;
> +		}
> +	}
> +
> +	dw_pcie_write(base + reg, size, val);
> +
> +	if (is_atu)
> +		regmap_write(fsd_ctrl->sysreg, fsd_ctrl->sysreg_base, ADDR_TYPE_DBI);
> +}
> +
> +static void fsd_pcie_write_dbi2(struct dw_pcie *pci, void __iomem *base,
> +				u32 reg, size_t size, u32 val)
> +{
> +	struct fsd_pcie *fsd_ctrl = to_fsd_pcie(pci);
> +
> +	regmap_write(fsd_ctrl->sysreg, fsd_ctrl->sysreg_base, ADDR_TYPE_DBI2);
> +	dw_pcie_write(pci->dbi_base + reg, size, val);
> +	regmap_write(fsd_ctrl->sysreg, fsd_ctrl->sysreg_base, ADDR_TYPE_DBI);
> +}
> +
> +static int fsd_pcie_link_up(struct dw_pcie *pci)
> +{
> +	u32 val;
> +	struct fsd_pcie *fsd_ctrl = to_fsd_pcie(pci);
> +
> +	val = readl(fsd_ctrl->appl_base +
> +			fsd_ctrl->pdata->appl_cxpl_debug_00_31);
> +
> +	return (val & LTSSM_STATE_MASK) == LTSSM_STATE_L0;
> +}
> +
> +static int fsd_pcie_host_init(struct dw_pcie_rp *pp)
> +{
> +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> +	struct fsd_pcie *fsd_ctrl = to_fsd_pcie(pci);
> +
> +	dw_pcie_writel_dbi(pci, PCIE_GEN3_RELATED_OFF,
> +				(PCIE_GEN3_EQ_PHASE_2_3 |
> +				 PCIE_GEN3_RXEQ_PH01_EN |
> +				 PCIE_GEN3_RXEQ_RGRDLESS_RXTS));
> +
> +	fsd_pcie_enable_interrupts(fsd_ctrl);
> +
> +	return 0;
> +}
> +
> +static struct dw_pcie_host_ops fsd_pcie_host_ops = {
> +	.host_init = fsd_pcie_host_init,
> +};
> +
> +static int fsd_pcie_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
> +				 enum pci_epc_irq_type type, u16 interrupt_num)
> +{
> +	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> +
> +	switch (type) {
> +	case PCI_EPC_IRQ_LEGACY:
> +		dev_err(pci->dev, "EP does not support legacy IRQs\n");
> +		return -EINVAL;
> +	case PCI_EPC_IRQ_MSI:
> +		return dw_pcie_ep_raise_msi_irq(ep, func_no, interrupt_num);
> +	default:
> +		dev_err(pci->dev, "UNKNOWN IRQ type\n");
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct pci_epc_features fsd_pcie_epc_features = {
> +	.linkup_notifier = false,
> +	.msi_capable = true,
> +	.msix_capable = false,
> +};
> +
> +static const struct pci_epc_features*
> +	fsd_pcie_get_features(struct dw_pcie_ep *ep)
> +{
> +	return &fsd_pcie_epc_features;
> +}
> +
> +static struct dw_pcie_ep_ops fsd_dw_pcie_ep_ops = {
> +	.raise_irq	= fsd_pcie_raise_irq,
> +	.get_features	= fsd_pcie_get_features,
> +};
> +
> +static const struct fsd_pcie_irq fsd_pcie_irq_data = {
> +	.pcie_msi_irq_handler	= fsd_pcie_msi_irq_handler,
> +	.pcie_msi_init		= fsd_pcie_msi_init,
> +	.pcie_sub_ctrl_handler	= fsd_pcie_sub_ctrl_handler,
> +};
> +
> +static int __init fsd_add_pcie_ep(struct fsd_pcie *fsd_ctrl,
> +		struct platform_device *pdev)
> +{
> +	struct dw_pcie_ep *ep;
> +	struct dw_pcie *pci = fsd_ctrl->pci;
> +	int ret;
> +	struct device *dev = &pdev->dev;
> +
> +	ep = &pci->ep;
> +	ep->ops = &fsd_dw_pcie_ep_ops;
> +
> +	dw_pcie_writel_dbi(pci, PCIE_GEN3_RELATED_OFF,
> +				(PCIE_GEN3_EQUALIZATION_DISABLE |
> +				 PCIE_GEN3_RXEQ_PH01_EN |
> +				 PCIE_GEN3_RXEQ_RGRDLESS_RXTS));
> +
> +	ret = dw_pcie_ep_init(ep);
> +	if (ret)
> +		dev_err(dev, "failed to initialize endpoint\n");
> +
> +	return ret;
> +}
> +
> +static int __init fsd_add_pcie_port(struct fsd_pcie *fsd_ctrl,
> +					struct platform_device *pdev)
> +{
> +	int irq;
> +	struct device *dev = &pdev->dev;
> +	int irq_flags;
> +	int ret;
> +	struct dw_pcie *pci = fsd_ctrl->pci;
> +	struct dw_pcie_rp *pp = &pci->pp;
> +
> +	if (IS_ENABLED(CONFIG_PCI_MSI)) {
> +		irq = platform_get_irq_byname(pdev, "msi");

Wasn't in the binding. And the DWC core does this for you.

> +		if (!irq) {
> +			dev_err(dev, "failed to get msi irq\n");
> +			return -ENODEV;
> +		}
> +
> +		irq_flags = IRQF_TRIGGER_RISING | IRQF_SHARED | IRQF_NO_THREAD;
> +
> +		ret = devm_request_irq(dev, irq,
> +					fsd_ctrl->pdata->irq_data->pcie_msi_irq_handler,
> +					irq_flags, "fsd-pcie", fsd_ctrl);
> +		if (ret) {
> +			dev_err(dev, "failed to request msi irq\n");
> +			return ret;
> +		}
> +		pp->msi_irq[0] = -ENODEV;
> +	}
> +
> +	ret = dw_pcie_host_init(pp);
> +	if (ret)
> +		dev_err(dev, "failed to initialize host\n");
> +
> +	return ret;
> +}
> +
> +static const struct dw_pcie_ops fsd_dw_pcie_ops = {
> +	.read_dbi	= fsd_pcie_read_dbi,
> +	.write_dbi	= fsd_pcie_write_dbi,
> +	.write_dbi2	= fsd_pcie_write_dbi2,
> +	.start_link	= fsd_pcie_establish_link,
> +	.stop_link	= fsd_pcie_stop_link,
> +	.link_up	= fsd_pcie_link_up,
> +};
> +
> +static int fsd_pcie_probe(struct platform_device *pdev)
> +{
> +	int ret;
> +	int irq, irq_flags;
> +	struct dw_pcie *pci;
> +	struct dw_pcie_rp *pp;
> +	struct fsd_pcie *fsd_ctrl;
> +	struct device *dev = &pdev->dev;
> +	const struct fsd_pcie_pdata *pdata;
> +	struct device_node *np = dev->of_node;
> +
> +	fsd_ctrl = devm_kzalloc(dev, sizeof(*fsd_ctrl), GFP_KERNEL);
> +	if (!fsd_ctrl)
> +		return -ENOMEM;
> +
> +	pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL);
> +	if (!pci)
> +		return -ENOMEM;
> +
> +	pdata = (const struct fsd_pcie_pdata *) of_device_get_match_data(dev);
> +
> +	fsd_ctrl->pci = pci;
> +	fsd_ctrl->pdata = pdata;
> +
> +	pci->dev = dev;
> +	pci->ops = pdata->dwc_ops;
> +	pci->dbi_base2 = NULL;
> +	pci->dbi_base = NULL;
> +	pci->atu_base = NULL;
> +	pp = &pci->pp;
> +	pp->ops = fsd_ctrl->pdata->host_ops;
> +
> +	fsd_ctrl->phy = devm_of_phy_get(dev, np, NULL);

Use the non-DT version.

> +	if (IS_ERR(fsd_ctrl->phy)) {
> +		if (PTR_ERR(fsd_ctrl->phy) == -EPROBE_DEFER)
> +			return PTR_ERR(fsd_ctrl->phy);
> +	}
> +
> +	phy_init(fsd_ctrl->phy);
> +
> +	if (pdata->res_ops && pdata->res_ops->get_mem_resources) {
> +		ret = pdata->res_ops->get_mem_resources(pdev, fsd_ctrl);

Again, get rid of the indirection. If it does vary, don't invent your 
own ops. We only need 1 level of abstraction.

> +		if (ret)
> +			return ret;
> +	}
> +
> +	if (pdata->res_ops && pdata->res_ops->get_clk_resources) {
> +		ret = pdata->res_ops->get_clk_resources(pdev, fsd_ctrl);
> +		if (ret)
> +			return ret;
> +		ret = pdata->res_ops->init_clk_resources(fsd_ctrl);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	platform_set_drvdata(pdev, fsd_ctrl);
> +
> +	ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(36));
> +	if (ret)
> +		goto fail_dma_set;
> +
> +	switch (fsd_ctrl->pdata->op_mode) {
> +	case DEVICE_TYPE_RC:
> +		writel(DEVICE_TYPE_RC, fsd_ctrl->appl_base +
> +					PCIE_FSD_DEVICE_TYPE);
> +		ret = fsd_add_pcie_port(fsd_ctrl, pdev);
> +		if (ret)
> +			goto fail_add_pcie_port;
> +		break;
> +	case DEVICE_TYPE_EP:
> +		writel(DEVICE_TYPE_EP, fsd_ctrl->appl_base +
> +				PCIE_FSD_DEVICE_TYPE);
> +
> +		ret = fsd_add_pcie_ep(fsd_ctrl, pdev);
> +		if (ret)
> +			goto fail_add_pcie_ep;
> +		break;
> +	}
> +
> +	irq = platform_get_irq_byname(pdev, "sub_ctrl_intr");
> +	if (irq > 0) {
> +
> +		irq_flags = IRQF_TRIGGER_RISING | IRQF_SHARED | IRQF_NO_THREAD;
> +
> +		ret = devm_request_irq(dev, irq,
> +				fsd_ctrl->pdata->irq_data->pcie_sub_ctrl_handler,
> +				irq_flags, "fsd-sub-ctrl-pcie", fsd_ctrl);
> +		if (ret)
> +			dev_err(dev, "failed to request sub ctrl irq\n");
> +	}
> +
> +	dev_info(dev, "FSD PCIe probe completed successfully\n");
> +
> +	return 0;
> +
> +fail_dma_set:
> +	dev_err(dev, "PCIe Failed to set 36 bit dma mask\n");
> +fail_add_pcie_port:
> +	phy_exit(fsd_ctrl->phy);
> +fail_add_pcie_ep:
> +	if (pdata->res_ops && pdata->res_ops->deinit_clk_resources)
> +		pdata->res_ops->deinit_clk_resources(fsd_ctrl);
> +	return ret;
> +}
> +
> +static int __exit fsd_pcie_remove(struct platform_device *pdev)
> +{
> +	struct fsd_pcie *fsd_ctrl = platform_get_drvdata(pdev);
> +	const struct fsd_pcie_pdata *pdata = fsd_ctrl->pdata;
> +
> +	if (pdata->res_ops && pdata->res_ops->deinit_clk_resources)
> +		pdata->res_ops->deinit_clk_resources(fsd_ctrl);
> +
> +	return 0;
> +}
> +
> +static const struct fsd_pcie_pdata fsd_pcie_rc_pdata = {
> +	.dwc_ops		= &fsd_dw_pcie_ops,
> +	.host_ops		= &fsd_pcie_host_ops,
> +	.res_ops		= &fsd_pcie_res_ops_data,
> +	.irq_data		= &fsd_pcie_irq_data,
> +	.appl_cxpl_debug_00_31	= PCIE_ELBI_CXPL_DEBUG_00_31,
> +	.op_mode		= DEVICE_TYPE_RC,
> +};
> +
> +static const struct fsd_pcie_pdata fsd_pcie_ep_pdata = {
> +	.dwc_ops		= &fsd_dw_pcie_ops,
> +	.host_ops		= &fsd_pcie_host_ops,
> +	.res_ops		= &fsd_pcie_res_ops_data,
> +	.irq_data		= &fsd_pcie_irq_data,
> +	.appl_cxpl_debug_00_31	= PCIE_ELBI_CXPL_DEBUG_00_31,
> +	.op_mode		= DEVICE_TYPE_EP,
> +};
> +
> +static const struct of_device_id fsd_pcie_of_match[] = {
> +	{
> +		.compatible = "tesla,fsd-pcie",
> +		.data = (void *) &fsd_pcie_rc_pdata,
> +	},
> +	{
> +		.compatible = "tesla,fsd-pcie-ep",
> +		.data = (void *) &fsd_pcie_ep_pdata,
> +	},
> +
> +	{},
> +};
> +
> +static struct platform_driver fsd_pcie_driver = {
> +	.probe		= fsd_pcie_probe,
> +	.remove		= __exit_p(fsd_pcie_remove),
> +	.driver		= {
> +		.name	= "fsd-pcie",
> +		.of_match_table = fsd_pcie_of_match,
> +	},
> +};
> +
> +static int __init fsd_pcie_init(void)
> +{
> +	return platform_driver_register(&fsd_pcie_driver);
> +}
> +module_init(fsd_pcie_init);
> -- 
> 2.17.1
> 
>