* RE: [PATCH v4 2/3] dwc: PCI: intel: PCIe RC controller driver
2019-10-21 6:39 ` [PATCH v4 2/3] dwc: PCI: intel: PCIe RC controller driver Dilip Kota
@ 2019-10-21 8:29 ` Gustavo Pimentel
2019-10-21 10:44 ` Dilip Kota
2019-10-21 13:03 ` Andrew Murray
` (4 subsequent siblings)
5 siblings, 1 reply; 55+ messages in thread
From: Gustavo Pimentel @ 2019-10-21 8:29 UTC (permalink / raw)
To: Dilip Kota, jingoohan1, gustavo.pimentel@synopsys.com,
lorenzo.pieralisi, andrew.murray, robh, martin.blumenstingl,
linux-pci, hch, devicetree
Cc: linux-kernel, andriy.shevchenko, cheol.yong.kim, chuanhua.lei,
qi-ming.wu
Hi
On Mon, Oct 21, 2019 at 7:39:19, Dilip Kota <eswara.kota@linux.intel.com>
wrote:
> Add support to PCIe RC controller on Intel Gateway SoCs.
> PCIe controller is based of Synopsys DesignWare pci core.
>
> Intel PCIe driver requires Upconfig support, fast training
> sequence configuration and link speed change. 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.
Please do the replacement in all of your patches
s/pcie/PCIe
s/pci/PCI
Also I think the correct term is Upconfigure and not Upconfig
>
> 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()
>
> Signed-off-by: Dilip Kota <eswara.kota@linux.intel.com>
> ---
> drivers/pci/controller/dwc/Kconfig | 10 +
> drivers/pci/controller/dwc/Makefile | 1 +
> drivers/pci/controller/dwc/pcie-designware.c | 34 ++
> drivers/pci/controller/dwc/pcie-designware.h | 12 +
> drivers/pci/controller/dwc/pcie-intel-gw.c | 590 +++++++++++++++++++++++++++
> include/uapi/linux/pci_regs.h | 1 +
> 6 files changed, 648 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..b33ed1cc873d 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 support for PCIe Host controller driver.
> + The PCIe controller on Intel Gateway SoCs is based on the Synopsys
> + DesignWare pcie core and therefore uses the DesignWare core
> + functions for the driver implementation.
> +
> 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..4c391bfd681a 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -474,6 +474,40 @@ 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);
> + dw_pcie_writel_dbi(pci, PCIE_PORT_MULTI_LANE_CTRL,
> + val | PORT_MLTI_UPCFG_SUPPORT);
> +}
> +
> +void dw_pcie_link_speed_change(struct dw_pcie *pci, bool enable)
> +{
> + u32 val;
> +
> + val = dw_pcie_readl_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL);
> +
> + if (enable)
> + val |= PORT_LOGIC_SPEED_CHANGE;
> + else
> + val &= ~PORT_LOGIC_SPEED_CHANGE;
> +
> + dw_pcie_writel_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL, val);
> +}
> +
> +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;
> + val |= n_fts;
> + dw_pcie_writel_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL, val);
> +}
> +
> 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..3beac10e4a4c 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 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_speed_change(struct dw_pcie *pci, bool enable);
> +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..9142c70db808
> --- /dev/null
> +++ b/drivers/pci/controller/dwc/pcie-intel-gw.c
> @@ -0,0 +1,590 @@
> +// 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/interrupt.h>
> +#include <linux/iopoll.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_pci.h>
> +#include <linux/of_platform.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 PCIE_CAP_OFST 0x70
> +
> +#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_OFFS SZ_256M
> +#define RST_INTRVL_DFT_MS 100
> +
> +enum {
> + PCIE_LINK_SPEED_AUTO = 0,
> + PCIE_LINK_SPEED_GEN1,
> + PCIE_LINK_SPEED_GEN2,
> + PCIE_LINK_SPEED_GEN3,
> + PCIE_LINK_SPEED_GEN4,
> +};
> +
> +struct intel_pcie_soc {
> + unsigned int pcie_ver;
> + unsigned int pcie_atu_offset;
> + u32 num_viewport;
> +};
> +
> +struct intel_pcie_port {
> + struct dw_pcie pci;
> + unsigned int id; /* Physical RC Index */
> + 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;
> +};
> +
> +static void pcie_update_bits(void __iomem *base, u32 mask, u32 val, u32 ofs)
> +{
> + u32 orig, tmp;
> +
> + orig = readl(base + ofs);
> +
> + tmp = (orig & ~mask) | (val & mask);
> +
> + if (tmp != orig)
> + writel(tmp, base + ofs);
> +}
I'd suggest to the a take on FIELD_PREP() and FIELD_GET() and use more
intuitive names such as new and old, instead of orig and tmp.
> +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;
> +
> + val = pcie_rc_cfg_rd(lpp, PCIE_CAP_OFST + 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, PCIE_CAP_OFST + PCI_EXP_LNKCTL);
> +
> + val &= ~(PCI_EXP_LNKCTL_LD | PCI_EXP_LNKCTL_ASPMC);
> + val |= (PCI_EXP_LNKSTA_SLC << 16) | PCI_EXP_LNKCTL_CCC |
> + PCI_EXP_LNKCTL_RCB;
> + pcie_rc_cfg_wr(lpp, val, PCIE_CAP_OFST + PCI_EXP_LNKCTL);
> +}
> +
> +static void intel_pcie_max_speed_setup(struct intel_pcie_port *lpp)
> +{
> + u32 reg, val;
> +
> + reg = pcie_rc_cfg_rd(lpp, PCIE_CAP_OFST + PCI_EXP_LNKCTL2);
> + switch (lpp->link_gen) {
> + case PCIE_LINK_SPEED_GEN1:
> + reg &= ~PCI_EXP_LNKCTL2_TLS;
> + reg |= PCI_EXP_LNKCTL2_HASD|
> + PCI_EXP_LNKCTL2_TLS_2_5GT;
> + break;
> + case PCIE_LINK_SPEED_GEN2:
> + reg &= ~PCI_EXP_LNKCTL2_TLS;
> + reg |= PCI_EXP_LNKCTL2_HASD|
> + PCI_EXP_LNKCTL2_TLS_5_0GT;
> + break;
> + case PCIE_LINK_SPEED_GEN3:
> + reg &= ~PCI_EXP_LNKCTL2_TLS;
> + reg |= PCI_EXP_LNKCTL2_HASD|
> + PCI_EXP_LNKCTL2_TLS_8_0GT;
> + break;
> + case PCIE_LINK_SPEED_GEN4:
> + reg &= ~PCI_EXP_LNKCTL2_TLS;
> + reg |= PCI_EXP_LNKCTL2_HASD|
> + PCI_EXP_LNKCTL2_TLS_16_0GT;
> + break;
> + default:
> + /* Use hardware capability */
> + val = pcie_rc_cfg_rd(lpp, PCIE_CAP_OFST + PCI_EXP_LNKCAP);
> + val = FIELD_GET(PCI_EXP_LNKCAP_SLS, val);
> + reg &= ~PCI_EXP_LNKCTL2_HASD;
> + reg |= val;
> + break;
> + }
> +
> + pcie_rc_cfg_wr(lpp, reg, PCIE_CAP_OFST + PCI_EXP_LNKCTL2);
> + dw_pcie_link_set_n_fts(&lpp->pci, lpp->n_fts);
> +}
> +
> +
Reduce the number of empty lines here.
> +
> +static void intel_pcie_port_logic_setup(struct intel_pcie_port *lpp)
> +{
> + u32 val, mask;
> +
> + switch (lpp->max_speed) {
> + case PCIE_LINK_SPEED_GEN3:
> + lpp->n_fts = PORT_AFR_N_FTS_GEN3;
> + break;
> + case PCIE_LINK_SPEED_GEN4:
> + 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);
> + intel_pcie_max_speed_setup(lpp);
> +}
> +
> +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;
> +
> + ret = device_property_read_u32(dev, "linux,pci-domain", &lpp->id);
> + if (ret) {
> + dev_err(dev, "failed to get domain id, errno %d\n", ret);
> + return 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 = RST_INTRVL_DFT_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 (lpp->max_speed < PCIE_LINK_SPEED_GEN3)
> + 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)
> +{
> + 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;
> + }
> +
> + 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_OFFS;
> +}
> +
> +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);
> + 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 configure
> + * viewport to not to access IO region during register
> + * read write operations.
> + */
> + pci->num_viewport = data->num_viewport;
> + dev_info(dev, "Intel PCIe Root Complex Port %d init done\n", lpp->id);
> +
> + 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..f6e7e402f879 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 0x0200 /* Hw Autonomous Speed Disable */
s/Hw/HW
> #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
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH v4 2/3] dwc: PCI: intel: PCIe RC controller driver
2019-10-21 8:29 ` Gustavo Pimentel
@ 2019-10-21 10:44 ` Dilip Kota
2019-10-22 10:18 ` Dilip Kota
0 siblings, 1 reply; 55+ messages in thread
From: Dilip Kota @ 2019-10-21 10:44 UTC (permalink / raw)
To: Gustavo Pimentel, jingoohan1, lorenzo.pieralisi, andrew.murray,
robh, martin.blumenstingl, linux-pci, hch, devicetree
Cc: linux-kernel, andriy.shevchenko, cheol.yong.kim, chuanhua.lei,
qi-ming.wu
Hi Gustavo Pimentel,
On 10/21/2019 4:29 PM, Gustavo Pimentel wrote:
> Hi
>
> On Mon, Oct 21, 2019 at 7:39:19, Dilip Kota <eswara.kota@linux.intel.com>
> wrote:
>
>> Add support to PCIe RC controller on Intel Gateway SoCs.
>> PCIe controller is based of Synopsys DesignWare pci core.
>>
>> Intel PCIe driver requires Upconfig support, fast training
>> sequence configuration and link speed change. 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.
> Please do the replacement in all of your patches
>
> s/pcie/PCIe
> s/pci/PCI
>
> Also I think the correct term is Upconfigure and not Upconfig
Yes, i will update it.
>
>> 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()
>>
>> Signed-off-by: Dilip Kota <eswara.kota@linux.intel.com>
>> ---
>> drivers/pci/controller/dwc/Kconfig | 10 +
>> drivers/pci/controller/dwc/Makefile | 1 +
>> drivers/pci/controller/dwc/pcie-designware.c | 34 ++
>> drivers/pci/controller/dwc/pcie-designware.h | 12 +
>> drivers/pci/controller/dwc/pcie-intel-gw.c | 590 +++++++++++++++++++++++++++
>> include/uapi/linux/pci_regs.h | 1 +
>> 6 files changed, 648 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..b33ed1cc873d 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 support for PCIe Host controller driver.
>> + The PCIe controller on Intel Gateway SoCs is based on the Synopsys
>> + DesignWare pcie core and therefore uses the DesignWare core
>> + functions for the driver implementation.
>> +
>> 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..4c391bfd681a 100644
>> --- a/drivers/pci/controller/dwc/pcie-designware.c
>> +++ b/drivers/pci/controller/dwc/pcie-designware.c
>> @@ -474,6 +474,40 @@ 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);
>> + dw_pcie_writel_dbi(pci, PCIE_PORT_MULTI_LANE_CTRL,
>> + val | PORT_MLTI_UPCFG_SUPPORT);
>> +}
>> +
>> +void dw_pcie_link_speed_change(struct dw_pcie *pci, bool enable)
>> +{
>> + u32 val;
>> +
>> + val = dw_pcie_readl_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL);
>> +
>> + if (enable)
>> + val |= PORT_LOGIC_SPEED_CHANGE;
>> + else
>> + val &= ~PORT_LOGIC_SPEED_CHANGE;
>> +
>> + dw_pcie_writel_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL, val);
>> +}
>> +
>> +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;
>> + val |= n_fts;
>> + dw_pcie_writel_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL, val);
>> +}
>> +
>> 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..3beac10e4a4c 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 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_speed_change(struct dw_pcie *pci, bool enable);
>> +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..9142c70db808
>> --- /dev/null
>> +++ b/drivers/pci/controller/dwc/pcie-intel-gw.c
>> @@ -0,0 +1,590 @@
>> +// 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/interrupt.h>
>> +#include <linux/iopoll.h>
>> +#include <linux/of_irq.h>
>> +#include <linux/of_pci.h>
>> +#include <linux/of_platform.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 PCIE_CAP_OFST 0x70
>> +
>> +#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_OFFS SZ_256M
>> +#define RST_INTRVL_DFT_MS 100
>> +
>> +enum {
>> + PCIE_LINK_SPEED_AUTO = 0,
>> + PCIE_LINK_SPEED_GEN1,
>> + PCIE_LINK_SPEED_GEN2,
>> + PCIE_LINK_SPEED_GEN3,
>> + PCIE_LINK_SPEED_GEN4,
>> +};
>> +
>> +struct intel_pcie_soc {
>> + unsigned int pcie_ver;
>> + unsigned int pcie_atu_offset;
>> + u32 num_viewport;
>> +};
>> +
>> +struct intel_pcie_port {
>> + struct dw_pcie pci;
>> + unsigned int id; /* Physical RC Index */
>> + 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;
>> +};
>> +
>> +static void pcie_update_bits(void __iomem *base, u32 mask, u32 val, u32 ofs)
>> +{
>> + u32 orig, tmp;
>> +
>> + orig = readl(base + ofs);
>> +
>> + tmp = (orig & ~mask) | (val & mask);
>> +
>> + if (tmp != orig)
>> + writel(tmp, base + ofs);
>> +}
> I'd suggest to the a take on FIELD_PREP() and FIELD_GET() and use more
> intuitive names such as new and old, instead of orig and tmp.
Sure, i will update it.
>
>> +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;
>> +
>> + val = pcie_rc_cfg_rd(lpp, PCIE_CAP_OFST + 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, PCIE_CAP_OFST + PCI_EXP_LNKCTL);
>> +
>> + val &= ~(PCI_EXP_LNKCTL_LD | PCI_EXP_LNKCTL_ASPMC);
>> + val |= (PCI_EXP_LNKSTA_SLC << 16) | PCI_EXP_LNKCTL_CCC |
>> + PCI_EXP_LNKCTL_RCB;
>> + pcie_rc_cfg_wr(lpp, val, PCIE_CAP_OFST + PCI_EXP_LNKCTL);
>> +}
>> +
>> +static void intel_pcie_max_speed_setup(struct intel_pcie_port *lpp)
>> +{
>> + u32 reg, val;
>> +
>> + reg = pcie_rc_cfg_rd(lpp, PCIE_CAP_OFST + PCI_EXP_LNKCTL2);
>> + switch (lpp->link_gen) {
>> + case PCIE_LINK_SPEED_GEN1:
>> + reg &= ~PCI_EXP_LNKCTL2_TLS;
>> + reg |= PCI_EXP_LNKCTL2_HASD|
>> + PCI_EXP_LNKCTL2_TLS_2_5GT;
>> + break;
>> + case PCIE_LINK_SPEED_GEN2:
>> + reg &= ~PCI_EXP_LNKCTL2_TLS;
>> + reg |= PCI_EXP_LNKCTL2_HASD|
>> + PCI_EXP_LNKCTL2_TLS_5_0GT;
>> + break;
>> + case PCIE_LINK_SPEED_GEN3:
>> + reg &= ~PCI_EXP_LNKCTL2_TLS;
>> + reg |= PCI_EXP_LNKCTL2_HASD|
>> + PCI_EXP_LNKCTL2_TLS_8_0GT;
>> + break;
>> + case PCIE_LINK_SPEED_GEN4:
>> + reg &= ~PCI_EXP_LNKCTL2_TLS;
>> + reg |= PCI_EXP_LNKCTL2_HASD|
>> + PCI_EXP_LNKCTL2_TLS_16_0GT;
>> + break;
>> + default:
>> + /* Use hardware capability */
>> + val = pcie_rc_cfg_rd(lpp, PCIE_CAP_OFST + PCI_EXP_LNKCAP);
>> + val = FIELD_GET(PCI_EXP_LNKCAP_SLS, val);
>> + reg &= ~PCI_EXP_LNKCTL2_HASD;
>> + reg |= val;
>> + break;
>> + }
>> +
>> + pcie_rc_cfg_wr(lpp, reg, PCIE_CAP_OFST + PCI_EXP_LNKCTL2);
>> + dw_pcie_link_set_n_fts(&lpp->pci, lpp->n_fts);
>> +}
>> +
>> +
> Reduce the number of empty lines here.
Ok, will remove it.
>
>> +
>> +static void intel_pcie_port_logic_setup(struct intel_pcie_port *lpp)
>> +{
>> + u32 val, mask;
>> +
>> + switch (lpp->max_speed) {
>> + case PCIE_LINK_SPEED_GEN3:
>> + lpp->n_fts = PORT_AFR_N_FTS_GEN3;
>> + break;
>> + case PCIE_LINK_SPEED_GEN4:
>> + 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);
>> + intel_pcie_max_speed_setup(lpp);
>> +}
>> +
>> +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;
>> +
>> + ret = device_property_read_u32(dev, "linux,pci-domain", &lpp->id);
>> + if (ret) {
>> + dev_err(dev, "failed to get domain id, errno %d\n", ret);
>> + return 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 = RST_INTRVL_DFT_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 (lpp->max_speed < PCIE_LINK_SPEED_GEN3)
>> + 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)
>> +{
>> + 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;
>> + }
>> +
>> + 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_OFFS;
>> +}
>> +
>> +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);
>> + 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 configure
>> + * viewport to not to access IO region during register
>> + * read write operations.
>> + */
>> + pci->num_viewport = data->num_viewport;
>> + dev_info(dev, "Intel PCIe Root Complex Port %d init done\n", lpp->id);
>> +
>> + 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..f6e7e402f879 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 0x0200 /* Hw Autonomous Speed Disable */
> s/Hw/HW
Sure, i will correct it.
Thanks for the valuable inputs.
Regards,
Dilip
>
>> #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
>
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH v4 2/3] dwc: PCI: intel: PCIe RC controller driver
2019-10-21 10:44 ` Dilip Kota
@ 2019-10-22 10:18 ` Dilip Kota
2019-10-22 11:44 ` andriy.shevchenko
0 siblings, 1 reply; 55+ messages in thread
From: Dilip Kota @ 2019-10-22 10:18 UTC (permalink / raw)
To: Gustavo Pimentel, jingoohan1, lorenzo.pieralisi, andrew.murray,
robh, martin.blumenstingl, linux-pci, hch, devicetree
Cc: linux-kernel, andriy.shevchenko, cheol.yong.kim, chuanhua.lei,
qi-ming.wu
Hi Gustavo Pimentel,
On 10/21/2019 6:44 PM, Dilip Kota wrote:
> Hi Gustavo Pimentel,
>
> On 10/21/2019 4:29 PM, Gustavo Pimentel wrote:
>> Hi
>>
>> On Mon, Oct 21, 2019 at 7:39:19, Dilip Kota
>> <eswara.kota@linux.intel.com>
>> wrote:
>>
>>> Add support to PCIe RC controller on Intel Gateway SoCs.
>>> PCIe controller is based of Synopsys DesignWare pci core.
>>>
>>> Intel PCIe driver requires Upconfig support, fast training
>>> sequence configuration and link speed change. 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.
>> Please do the replacement in all of your patches
>>
>> s/pcie/PCIe
>> s/pci/PCI
>>
>> Also I think the correct term is Upconfigure and not Upconfig
> Yes, i will update it.
>>
>>> 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()
>>>
>>> Signed-off-by: Dilip Kota <eswara.kota@linux.intel.com>
>>> ---
>>> drivers/pci/controller/dwc/Kconfig | 10 +
>>> drivers/pci/controller/dwc/Makefile | 1 +
>>> drivers/pci/controller/dwc/pcie-designware.c | 34 ++
>>> drivers/pci/controller/dwc/pcie-designware.h | 12 +
>>> drivers/pci/controller/dwc/pcie-intel-gw.c | 590
>>> +++++++++++++++++++++++++++
>>> include/uapi/linux/pci_regs.h | 1 +
>>> 6 files changed, 648 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..b33ed1cc873d 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 support for PCIe Host controller
>>> driver.
>>> + The PCIe controller on Intel Gateway SoCs is based on the
>>> Synopsys
>>> + DesignWare pcie core and therefore uses the DesignWare core
>>> + functions for the driver implementation.
>>> +
>>> 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..4c391bfd681a 100644
>>> --- a/drivers/pci/controller/dwc/pcie-designware.c
>>> +++ b/drivers/pci/controller/dwc/pcie-designware.c
>>> @@ -474,6 +474,40 @@ 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);
>>> + dw_pcie_writel_dbi(pci, PCIE_PORT_MULTI_LANE_CTRL,
>>> + val | PORT_MLTI_UPCFG_SUPPORT);
>>> +}
>>> +
>>> +void dw_pcie_link_speed_change(struct dw_pcie *pci, bool enable)
>>> +{
>>> + u32 val;
>>> +
>>> + val = dw_pcie_readl_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL);
>>> +
>>> + if (enable)
>>> + val |= PORT_LOGIC_SPEED_CHANGE;
>>> + else
>>> + val &= ~PORT_LOGIC_SPEED_CHANGE;
>>> +
>>> + dw_pcie_writel_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL, val);
>>> +}
>>> +
>>> +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;
>>> + val |= n_fts;
>>> + dw_pcie_writel_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL, val);
>>> +}
>>> +
>>> 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..3beac10e4a4c 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 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_speed_change(struct dw_pcie *pci, bool enable);
>>> +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..9142c70db808
>>> --- /dev/null
>>> +++ b/drivers/pci/controller/dwc/pcie-intel-gw.c
>>> @@ -0,0 +1,590 @@
>>> +// 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/interrupt.h>
>>> +#include <linux/iopoll.h>
>>> +#include <linux/of_irq.h>
>>> +#include <linux/of_pci.h>
>>> +#include <linux/of_platform.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 PCIE_CAP_OFST 0x70
>>> +
>>> +#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_OFFS SZ_256M
>>> +#define RST_INTRVL_DFT_MS 100
>>> +
>>> +enum {
>>> + PCIE_LINK_SPEED_AUTO = 0,
>>> + PCIE_LINK_SPEED_GEN1,
>>> + PCIE_LINK_SPEED_GEN2,
>>> + PCIE_LINK_SPEED_GEN3,
>>> + PCIE_LINK_SPEED_GEN4,
>>> +};
>>> +
>>> +struct intel_pcie_soc {
>>> + unsigned int pcie_ver;
>>> + unsigned int pcie_atu_offset;
>>> + u32 num_viewport;
>>> +};
>>> +
>>> +struct intel_pcie_port {
>>> + struct dw_pcie pci;
>>> + unsigned int id; /* Physical RC Index */
>>> + 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;
>>> +};
>>> +
>>> +static void pcie_update_bits(void __iomem *base, u32 mask, u32 val,
>>> u32 ofs)
>>> +{
>>> + u32 orig, tmp;
>>> +
>>> + orig = readl(base + ofs);
>>> +
>>> + tmp = (orig & ~mask) | (val & mask);
>>> +
>>> + if (tmp != orig)
>>> + writel(tmp, base + ofs);
>>> +}
>> I'd suggest to the a take on FIELD_PREP() and FIELD_GET() and use more
>> intuitive names such as new and old, instead of orig and tmp.
> Sure, i will update it.
I tried using FIELD_PREP and FIELD_GET but it is failing because
FIELD_PREP and FIELD_GET
are expecting mask should be constant macro. u32 or u32 const are not
accepted.
Regards,
Dilip
>>
>>> +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;
>>> +
>>> + val = pcie_rc_cfg_rd(lpp, PCIE_CAP_OFST + 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, PCIE_CAP_OFST + PCI_EXP_LNKCTL);
>>> +
>>> + val &= ~(PCI_EXP_LNKCTL_LD | PCI_EXP_LNKCTL_ASPMC);
>>> + val |= (PCI_EXP_LNKSTA_SLC << 16) | PCI_EXP_LNKCTL_CCC |
>>> + PCI_EXP_LNKCTL_RCB;
>>> + pcie_rc_cfg_wr(lpp, val, PCIE_CAP_OFST + PCI_EXP_LNKCTL);
>>> +}
>>> +
>>> +static void intel_pcie_max_speed_setup(struct intel_pcie_port *lpp)
>>> +{
>>> + u32 reg, val;
>>> +
>>> + reg = pcie_rc_cfg_rd(lpp, PCIE_CAP_OFST + PCI_EXP_LNKCTL2);
>>> + switch (lpp->link_gen) {
>>> + case PCIE_LINK_SPEED_GEN1:
>>> + reg &= ~PCI_EXP_LNKCTL2_TLS;
>>> + reg |= PCI_EXP_LNKCTL2_HASD|
>>> + PCI_EXP_LNKCTL2_TLS_2_5GT;
>>> + break;
>>> + case PCIE_LINK_SPEED_GEN2:
>>> + reg &= ~PCI_EXP_LNKCTL2_TLS;
>>> + reg |= PCI_EXP_LNKCTL2_HASD|
>>> + PCI_EXP_LNKCTL2_TLS_5_0GT;
>>> + break;
>>> + case PCIE_LINK_SPEED_GEN3:
>>> + reg &= ~PCI_EXP_LNKCTL2_TLS;
>>> + reg |= PCI_EXP_LNKCTL2_HASD|
>>> + PCI_EXP_LNKCTL2_TLS_8_0GT;
>>> + break;
>>> + case PCIE_LINK_SPEED_GEN4:
>>> + reg &= ~PCI_EXP_LNKCTL2_TLS;
>>> + reg |= PCI_EXP_LNKCTL2_HASD|
>>> + PCI_EXP_LNKCTL2_TLS_16_0GT;
>>> + break;
>>> + default:
>>> + /* Use hardware capability */
>>> + val = pcie_rc_cfg_rd(lpp, PCIE_CAP_OFST + PCI_EXP_LNKCAP);
>>> + val = FIELD_GET(PCI_EXP_LNKCAP_SLS, val);
>>> + reg &= ~PCI_EXP_LNKCTL2_HASD;
>>> + reg |= val;
>>> + break;
>>> + }
>>> +
>>> + pcie_rc_cfg_wr(lpp, reg, PCIE_CAP_OFST + PCI_EXP_LNKCTL2);
>>> + dw_pcie_link_set_n_fts(&lpp->pci, lpp->n_fts);
>>> +}
>>> +
>>> +
>> Reduce the number of empty lines here.
> Ok, will remove it.
>>
>>> +
>>> +static void intel_pcie_port_logic_setup(struct intel_pcie_port *lpp)
>>> +{
>>> + u32 val, mask;
>>> +
>>> + switch (lpp->max_speed) {
>>> + case PCIE_LINK_SPEED_GEN3:
>>> + lpp->n_fts = PORT_AFR_N_FTS_GEN3;
>>> + break;
>>> + case PCIE_LINK_SPEED_GEN4:
>>> + 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);
>>> + intel_pcie_max_speed_setup(lpp);
>>> +}
>>> +
>>> +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;
>>> +
>>> + ret = device_property_read_u32(dev, "linux,pci-domain", &lpp->id);
>>> + if (ret) {
>>> + dev_err(dev, "failed to get domain id, errno %d\n", ret);
>>> + return 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 = RST_INTRVL_DFT_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 (lpp->max_speed < PCIE_LINK_SPEED_GEN3)
>>> + 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)
>>> +{
>>> + 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;
>>> + }
>>> +
>>> + 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_OFFS;
>>> +}
>>> +
>>> +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);
>>> + 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 configure
>>> + * viewport to not to access IO region during register
>>> + * read write operations.
>>> + */
>>> + pci->num_viewport = data->num_viewport;
>>> + dev_info(dev, "Intel PCIe Root Complex Port %d init done\n",
>>> lpp->id);
>>> +
>>> + 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..f6e7e402f879 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 0x0200 /* Hw Autonomous Speed
>>> Disable */
>> s/Hw/HW
>
> Sure, i will correct it.
>
> Thanks for the valuable inputs.
>
> Regards,
> Dilip
>
>>
>>> #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
>>
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH v4 2/3] dwc: PCI: intel: PCIe RC controller driver
2019-10-22 10:18 ` Dilip Kota
@ 2019-10-22 11:44 ` andriy.shevchenko
2019-10-25 9:01 ` Andrew Murray
2019-10-29 6:14 ` Dilip Kota
0 siblings, 2 replies; 55+ messages in thread
From: andriy.shevchenko @ 2019-10-22 11:44 UTC (permalink / raw)
To: Dilip Kota
Cc: Gustavo Pimentel, jingoohan1, lorenzo.pieralisi, andrew.murray,
robh, martin.blumenstingl, linux-pci, hch, devicetree,
linux-kernel, cheol.yong.kim, chuanhua.lei, qi-ming.wu
On Tue, Oct 22, 2019 at 06:18:57PM +0800, Dilip Kota wrote:
> On 10/21/2019 6:44 PM, Dilip Kota wrote:
> > On 10/21/2019 4:29 PM, Gustavo Pimentel wrote:
> > > On Mon, Oct 21, 2019 at 7:39:19, Dilip Kota
> > > <eswara.kota@linux.intel.com>
> > > wrote:
First of all, it's a good behaviour to avoid way long quoting.
> > > > +static void pcie_update_bits(void __iomem *base, u32 mask, u32
> > > > val, u32 ofs)
> > > > +{
> > > > + u32 orig, tmp;
> > > > +
> > > > + orig = readl(base + ofs);
> > > > +
> > > > + tmp = (orig & ~mask) | (val & mask);
> > > > +
> > > > + if (tmp != orig)
> > > > + writel(tmp, base + ofs);
> > > > +}
> > > I'd suggest to the a take on FIELD_PREP() and FIELD_GET() and use more
> > > intuitive names such as new and old, instead of orig and tmp.
> > Sure, i will update it.
> I tried using FIELD_PREP and FIELD_GET but it is failing because FIELD_PREP
> and FIELD_GET
> are expecting mask should be constant macro. u32 or u32 const are not
> accepted.
If you look at bitfield.h carefully you may find in particular
u32_replace_bits().
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH v4 2/3] dwc: PCI: intel: PCIe RC controller driver
2019-10-22 11:44 ` andriy.shevchenko
@ 2019-10-25 9:01 ` Andrew Murray
2019-10-29 6:14 ` Dilip Kota
1 sibling, 0 replies; 55+ messages in thread
From: Andrew Murray @ 2019-10-25 9:01 UTC (permalink / raw)
To: andriy.shevchenko
Cc: Dilip Kota, Gustavo Pimentel, jingoohan1, lorenzo.pieralisi,
robh, martin.blumenstingl, linux-pci, hch, devicetree,
linux-kernel, cheol.yong.kim, chuanhua.lei, qi-ming.wu
On Tue, Oct 22, 2019 at 02:44:13PM +0300, andriy.shevchenko@intel.com wrote:
> On Tue, Oct 22, 2019 at 06:18:57PM +0800, Dilip Kota wrote:
> > On 10/21/2019 6:44 PM, Dilip Kota wrote:
> > > On 10/21/2019 4:29 PM, Gustavo Pimentel wrote:
> > > > On Mon, Oct 21, 2019 at 7:39:19, Dilip Kota
> > > > <eswara.kota@linux.intel.com>
> > > > wrote:
>
> First of all, it's a good behaviour to avoid way long quoting.
>
> > > > > +static void pcie_update_bits(void __iomem *base, u32 mask, u32
> > > > > val, u32 ofs)
> > > > > +{
> > > > > + u32 orig, tmp;
> > > > > +
> > > > > + orig = readl(base + ofs);
> > > > > +
> > > > > + tmp = (orig & ~mask) | (val & mask);
> > > > > +
> > > > > + if (tmp != orig)
> > > > > + writel(tmp, base + ofs);
> > > > > +}
> > > > I'd suggest to the a take on FIELD_PREP() and FIELD_GET() and use more
> > > > intuitive names such as new and old, instead of orig and tmp.
> > > Sure, i will update it.
> > I tried using FIELD_PREP and FIELD_GET but it is failing because FIELD_PREP
> > and FIELD_GET
> > are expecting mask should be constant macro. u32 or u32 const are not
> > accepted.
>
> If you look at bitfield.h carefully you may find in particular
> u32_replace_bits().
Thanks Andy - I was looking for something like this when I was reviewing the
patch but wasn't able to find it.
Andrew Murray
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH v4 2/3] dwc: PCI: intel: PCIe RC controller driver
2019-10-22 11:44 ` andriy.shevchenko
2019-10-25 9:01 ` Andrew Murray
@ 2019-10-29 6:14 ` Dilip Kota
1 sibling, 0 replies; 55+ messages in thread
From: Dilip Kota @ 2019-10-29 6:14 UTC (permalink / raw)
To: andriy.shevchenko
Cc: Gustavo Pimentel, jingoohan1, lorenzo.pieralisi, andrew.murray,
robh, martin.blumenstingl, linux-pci, hch, devicetree,
linux-kernel, cheol.yong.kim, chuanhua.lei, qi-ming.wu
On 10/22/2019 7:44 PM, andriy.shevchenko@intel.com wrote:
> On Tue, Oct 22, 2019 at 06:18:57PM +0800, Dilip Kota wrote:
>> On 10/21/2019 6:44 PM, Dilip Kota wrote:
>>> On 10/21/2019 4:29 PM, Gustavo Pimentel wrote:
>>>> On Mon, Oct 21, 2019 at 7:39:19, Dilip Kota
>>>> <eswara.kota@linux.intel.com>
>>>> wrote:
> First of all, it's a good behaviour to avoid way long quoting.
(Sorry for the late reply, I am back today from sick leave.)
Noted. Will take care of it.
>
>>>>> +static void pcie_update_bits(void __iomem *base, u32 mask, u32
>>>>> val, u32 ofs)
>>>>> +{
>>>>> + u32 orig, tmp;
>>>>> +
>>>>> + orig = readl(base + ofs);
>>>>> +
>>>>> + tmp = (orig & ~mask) | (val & mask);
>>>>> +
>>>>> + if (tmp != orig)
>>>>> + writel(tmp, base + ofs);
>>>>> +}
>>>> I'd suggest to the a take on FIELD_PREP() and FIELD_GET() and use more
>>>> intuitive names such as new and old, instead of orig and tmp.
>>> Sure, i will update it.
>> I tried using FIELD_PREP and FIELD_GET but it is failing because FIELD_PREP
>> and FIELD_GET
>> are expecting mask should be constant macro. u32 or u32 const are not
>> accepted.
> If you look at bitfield.h carefully you may find in particular
> u32_replace_bits().
Thanks for pointing it.
But, I found bitfields are not contiguous during few function calls, so
cannot use them here.
Regards,
Dilip
>
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v4 2/3] dwc: PCI: intel: PCIe RC controller driver
2019-10-21 6:39 ` [PATCH v4 2/3] dwc: PCI: intel: PCIe RC controller driver Dilip Kota
2019-10-21 8:29 ` Gustavo Pimentel
@ 2019-10-21 13:03 ` Andrew Murray
2019-10-22 9:04 ` Dilip Kota
2019-10-21 17:17 ` Bjorn Helgaas
` (3 subsequent siblings)
5 siblings, 1 reply; 55+ messages in thread
From: Andrew Murray @ 2019-10-21 13:03 UTC (permalink / raw)
To: Dilip Kota
Cc: jingoohan1, gustavo.pimentel, lorenzo.pieralisi, robh,
martin.blumenstingl, linux-pci, hch, devicetree, linux-kernel,
andriy.shevchenko, cheol.yong.kim, chuanhua.lei, qi-ming.wu
On Mon, Oct 21, 2019 at 02:39:19PM +0800, Dilip Kota wrote:
> Add support to PCIe RC controller on Intel Gateway SoCs.
> PCIe controller is based of Synopsys DesignWare pci core.
>
> Intel PCIe driver requires Upconfig support, fast training
> sequence configuration and link speed change. 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.
>
> 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()
>
> Signed-off-by: Dilip Kota <eswara.kota@linux.intel.com>
> ---
> drivers/pci/controller/dwc/Kconfig | 10 +
> drivers/pci/controller/dwc/Makefile | 1 +
> drivers/pci/controller/dwc/pcie-designware.c | 34 ++
> drivers/pci/controller/dwc/pcie-designware.h | 12 +
> drivers/pci/controller/dwc/pcie-intel-gw.c | 590 +++++++++++++++++++++++++++
> include/uapi/linux/pci_regs.h | 1 +
> 6 files changed, 648 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..b33ed1cc873d 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 support for PCIe Host controller driver.
This sentence is very generic, we don't even say what controller driver.
> + The PCIe controller on Intel Gateway SoCs is based on the Synopsys
> + DesignWare pcie core and therefore uses the DesignWare core
> + functions for the driver implementation.
Thanks for changing the driver name, though the description hasn't really
changed since the last revision. In particular, Christoph's feedback mentioned
that it would be useful to describe where this IP block may be used - is there
any reason why we can't make this more useful for users?
> +
> 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..4c391bfd681a 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -474,6 +474,40 @@ 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);
> + dw_pcie_writel_dbi(pci, PCIE_PORT_MULTI_LANE_CTRL,
> + val | PORT_MLTI_UPCFG_SUPPORT);
> +}
> +
> +void dw_pcie_link_speed_change(struct dw_pcie *pci, bool enable)
> +{
> + u32 val;
> +
> + val = dw_pcie_readl_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL);
> +
> + if (enable)
> + val |= PORT_LOGIC_SPEED_CHANGE;
> + else
> + val &= ~PORT_LOGIC_SPEED_CHANGE;
> +
> + dw_pcie_writel_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL, val);
> +}
It's always great to see code move from specific drivers to parent drivers
as it helps reduce duplication of code.
Though I notice that imx6_pcie_establish_link (pci-imx6.c) and
dw_pcie_setup_rc (pcie-designware-host.c) also does this. Rather than have
duplicated code, I think it makes sense to adopt those users to this new
code.
In any case, the above function isn't used here. Can you add it in any
patch that does use it please?
> +
> +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;
> + val |= n_fts;
> + dw_pcie_writel_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL, val);
> +}
I notice that pcie-artpec6.c (artpec6_pcie_set_nfts) also writes the FTS
and defines a bunch of macros to support this. It doesn't make sense to
duplicate this there. Therefore I think we need to update pcie-artpec6.c
to use this new function.
> +
> 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..3beac10e4a4c 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 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_speed_change(struct dw_pcie *pci, bool enable);
> +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..9142c70db808
> --- /dev/null
> +++ b/drivers/pci/controller/dwc/pcie-intel-gw.c
> @@ -0,0 +1,590 @@
> +// 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/interrupt.h>
> +#include <linux/iopoll.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_pci.h>
> +#include <linux/of_platform.h>
> +#include <linux/pci_regs.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset.h>
> +
> +#include "../../pci.h"
I expected this to be removed, is it needed now?
> +#include "pcie-designware.h"
> +
> +#define PCIE_CAP_OFST 0x70
> +
> +#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_OFFS SZ_256M
> +#define RST_INTRVL_DFT_MS 100
> +
> +enum {
> + PCIE_LINK_SPEED_AUTO = 0,
> + PCIE_LINK_SPEED_GEN1,
> + PCIE_LINK_SPEED_GEN2,
> + PCIE_LINK_SPEED_GEN3,
> + PCIE_LINK_SPEED_GEN4,
> +};
> +
> +struct intel_pcie_soc {
> + unsigned int pcie_ver;
> + unsigned int pcie_atu_offset;
> + u32 num_viewport;
> +};
> +
> +struct intel_pcie_port {
> + struct dw_pcie pci;
> + unsigned int id; /* Physical RC Index */
> + 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;
> +};
> +
> +static void pcie_update_bits(void __iomem *base, u32 mask, u32 val, u32 ofs)
> +{
> + u32 orig, tmp;
> +
> + orig = readl(base + ofs);
> +
> + tmp = (orig & ~mask) | (val & mask);
> +
> + if (tmp != orig)
> + writel(tmp, 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;
> +
> + val = pcie_rc_cfg_rd(lpp, PCIE_CAP_OFST + 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, PCIE_CAP_OFST + PCI_EXP_LNKCTL);
> +
> + val &= ~(PCI_EXP_LNKCTL_LD | PCI_EXP_LNKCTL_ASPMC);
> + val |= (PCI_EXP_LNKSTA_SLC << 16) | PCI_EXP_LNKCTL_CCC |
> + PCI_EXP_LNKCTL_RCB;
> + pcie_rc_cfg_wr(lpp, val, PCIE_CAP_OFST + PCI_EXP_LNKCTL);
> +}
> +
> +static void intel_pcie_max_speed_setup(struct intel_pcie_port *lpp)
> +{
> + u32 reg, val;
> +
> + reg = pcie_rc_cfg_rd(lpp, PCIE_CAP_OFST + PCI_EXP_LNKCTL2);
> + switch (lpp->link_gen) {
> + case PCIE_LINK_SPEED_GEN1:
> + reg &= ~PCI_EXP_LNKCTL2_TLS;
> + reg |= PCI_EXP_LNKCTL2_HASD|
> + PCI_EXP_LNKCTL2_TLS_2_5GT;
> + break;
> + case PCIE_LINK_SPEED_GEN2:
> + reg &= ~PCI_EXP_LNKCTL2_TLS;
> + reg |= PCI_EXP_LNKCTL2_HASD|
> + PCI_EXP_LNKCTL2_TLS_5_0GT;
> + break;
> + case PCIE_LINK_SPEED_GEN3:
> + reg &= ~PCI_EXP_LNKCTL2_TLS;
> + reg |= PCI_EXP_LNKCTL2_HASD|
> + PCI_EXP_LNKCTL2_TLS_8_0GT;
> + break;
> + case PCIE_LINK_SPEED_GEN4:
> + reg &= ~PCI_EXP_LNKCTL2_TLS;
> + reg |= PCI_EXP_LNKCTL2_HASD|
> + PCI_EXP_LNKCTL2_TLS_16_0GT;
> + break;
> + default:
> + /* Use hardware capability */
> + val = pcie_rc_cfg_rd(lpp, PCIE_CAP_OFST + PCI_EXP_LNKCAP);
> + val = FIELD_GET(PCI_EXP_LNKCAP_SLS, val);
> + reg &= ~PCI_EXP_LNKCTL2_HASD;
> + reg |= val;
> + break;
> + }
> +
> + pcie_rc_cfg_wr(lpp, reg, PCIE_CAP_OFST + PCI_EXP_LNKCTL2);
> + dw_pcie_link_set_n_fts(&lpp->pci, lpp->n_fts);
> +}
> +
> +
> +
Lots of space here isn't needed.
> +static void intel_pcie_port_logic_setup(struct intel_pcie_port *lpp)
> +{
> + u32 val, mask;
> +
> + switch (lpp->max_speed) {
> + case PCIE_LINK_SPEED_GEN3:
> + lpp->n_fts = PORT_AFR_N_FTS_GEN3;
> + break;
> + case PCIE_LINK_SPEED_GEN4:
> + 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);
> + intel_pcie_max_speed_setup(lpp);
> +}
> +
> +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;
> +
> + ret = device_property_read_u32(dev, "linux,pci-domain", &lpp->id);
> + if (ret) {
> + dev_err(dev, "failed to get domain id, errno %d\n", ret);
> + return ret;
> + }
Why is this a required property?
> +
> + 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 = RST_INTRVL_DFT_MS;
> +
> + ret = of_pci_get_max_link_speed(dev->of_node);
> + lpp->link_gen = ret < 0 ? 0 : ret;
I think the spacing may be a little off above.
> +
> + 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 (lpp->max_speed < PCIE_LINK_SPEED_GEN3)
> + 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)
> +{
> + 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;
> + }
> +
> + 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_OFFS;
> +}
> +
> +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);
> + 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 configure
> + * viewport to not to access IO region during register
> + * read write operations.
> + */
> + pci->num_viewport = data->num_viewport;
> + dev_info(dev, "Intel PCIe Root Complex Port %d init done\n", lpp->id);
> +
> + 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..f6e7e402f879 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 0x0200 /* Hw Autonomous Speed Disable */
I think this should be 0x0020.
Thanks,
Andrew Murray
> #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
>
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH v4 2/3] dwc: PCI: intel: PCIe RC controller driver
2019-10-21 13:03 ` Andrew Murray
@ 2019-10-22 9:04 ` Dilip Kota
2019-10-25 9:09 ` Andrew Murray
0 siblings, 1 reply; 55+ messages in thread
From: Dilip Kota @ 2019-10-22 9:04 UTC (permalink / raw)
To: Andrew Murray
Cc: jingoohan1, gustavo.pimentel, lorenzo.pieralisi, robh,
martin.blumenstingl, linux-pci, hch, devicetree, linux-kernel,
andriy.shevchenko, cheol.yong.kim, chuanhua.lei, qi-ming.wu
Hi Andrew Murray,
On 10/21/2019 9:03 PM, Andrew Murray wrote:
> On Mon, Oct 21, 2019 at 02:39:19PM +0800, Dilip Kota wrote:
>> Add support to PCIe RC controller on Intel Gateway SoCs.
>> PCIe controller is based of Synopsys DesignWare pci core.
>>
>> Intel PCIe driver requires Upconfig support, fast training
>> sequence configuration and link speed change. 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.
>>
>> 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()
>>
>> Signed-off-by: Dilip Kota <eswara.kota@linux.intel.com>
>> ---
>> drivers/pci/controller/dwc/Kconfig | 10 +
>> drivers/pci/controller/dwc/Makefile | 1 +
>> drivers/pci/controller/dwc/pcie-designware.c | 34 ++
>> drivers/pci/controller/dwc/pcie-designware.h | 12 +
>> drivers/pci/controller/dwc/pcie-intel-gw.c | 590 +++++++++++++++++++++++++++
>> include/uapi/linux/pci_regs.h | 1 +
>> 6 files changed, 648 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..b33ed1cc873d 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 support for PCIe Host controller driver.
> This sentence is very generic, we don't even say what controller driver.
>
>> + The PCIe controller on Intel Gateway SoCs is based on the Synopsys
>> + DesignWare pcie core and therefore uses the DesignWare core
>> + functions for the driver implementation.
> Thanks for changing the driver name, though the description hasn't really
> changed since the last revision. In particular, Christoph's feedback mentioned
> that it would be useful to describe where this IP block may be used - is there
> any reason why we can't make this more useful for users?
I will add more information.
>
>> +
>> 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..4c391bfd681a 100644
>> --- a/drivers/pci/controller/dwc/pcie-designware.c
>> +++ b/drivers/pci/controller/dwc/pcie-designware.c
>> @@ -474,6 +474,40 @@ 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);
>> + dw_pcie_writel_dbi(pci, PCIE_PORT_MULTI_LANE_CTRL,
>> + val | PORT_MLTI_UPCFG_SUPPORT);
>> +}
>> +
>> +void dw_pcie_link_speed_change(struct dw_pcie *pci, bool enable)
>> +{
>> + u32 val;
>> +
>> + val = dw_pcie_readl_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL);
>> +
>> + if (enable)
>> + val |= PORT_LOGIC_SPEED_CHANGE;
>> + else
>> + val &= ~PORT_LOGIC_SPEED_CHANGE;
>> +
>> + dw_pcie_writel_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL, val);
>> +}
> It's always great to see code move from specific drivers to parent drivers
> as it helps reduce duplication of code.
>
> Though I notice that imx6_pcie_establish_link (pci-imx6.c) and
> dw_pcie_setup_rc (pcie-designware-host.c) also does this. Rather than have
> duplicated code, I think it makes sense to adopt those users to this new
> code.
>
> In any case, the above function isn't used here. Can you add it in any
> patch that does use it please?
In the earlier patch version this function is being called in
intel_pcie_rc_setup().
After noticing this operation is being performed in dw_pcie_setup_rc(),
i removed the dw_pcie_link_speed_change() function call, but missed to
move the function definition to sysfs attribute patch as it is not being
called here.
I will move it to the sysfs attribute patch.
Thanks for pointing it.
>
>> +
>> +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;
>> + val |= n_fts;
>> + dw_pcie_writel_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL, val);
>> +}
> I notice that pcie-artpec6.c (artpec6_pcie_set_nfts) also writes the FTS
> and defines a bunch of macros to support this. It doesn't make sense to
> duplicate this there. Therefore I think we need to update pcie-artpec6.c
> to use this new function.
I think we can do in a separate patch after these changes get merged and
keep this patch series for intel PCIe driver and required changes in
PCIe DesignWare framework.
>
>> +
>> 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..3beac10e4a4c 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 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_speed_change(struct dw_pcie *pci, bool enable);
>> +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..9142c70db808
>> --- /dev/null
>> +++ b/drivers/pci/controller/dwc/pcie-intel-gw.c
>> @@ -0,0 +1,590 @@
>> +// 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/interrupt.h>
>> +#include <linux/iopoll.h>
>> +#include <linux/of_irq.h>
>> +#include <linux/of_pci.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/pci_regs.h>
>> +#include <linux/phy/phy.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/reset.h>
>> +
>> +#include "../../pci.h"
> I expected this to be removed, is it needed now?
In the earlier patch you suggested to use "of_pci_get_max_link_speed()"
instead of device_get_*.
And, pci.h is required for it.
>
>> +#include "pcie-designware.h"
>> +
>> +#define PCIE_CAP_OFST 0x70
>> +
>> +#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_OFFS SZ_256M
>> +#define RST_INTRVL_DFT_MS 100
>> +
>> +enum {
>> + PCIE_LINK_SPEED_AUTO = 0,
>> + PCIE_LINK_SPEED_GEN1,
>> + PCIE_LINK_SPEED_GEN2,
>> + PCIE_LINK_SPEED_GEN3,
>> + PCIE_LINK_SPEED_GEN4,
>> +};
>> +
>> +struct intel_pcie_soc {
>> + unsigned int pcie_ver;
>> + unsigned int pcie_atu_offset;
>> + u32 num_viewport;
>> +};
>> +
>> +struct intel_pcie_port {
>> + struct dw_pcie pci;
>> + unsigned int id; /* Physical RC Index */
>> + 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;
>> +};
>> +
>> +static void pcie_update_bits(void __iomem *base, u32 mask, u32 val, u32 ofs)
>> +{
>> + u32 orig, tmp;
>> +
>> + orig = readl(base + ofs);
>> +
>> + tmp = (orig & ~mask) | (val & mask);
>> +
>> + if (tmp != orig)
>> + writel(tmp, 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;
>> +
>> + val = pcie_rc_cfg_rd(lpp, PCIE_CAP_OFST + 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, PCIE_CAP_OFST + PCI_EXP_LNKCTL);
>> +
>> + val &= ~(PCI_EXP_LNKCTL_LD | PCI_EXP_LNKCTL_ASPMC);
>> + val |= (PCI_EXP_LNKSTA_SLC << 16) | PCI_EXP_LNKCTL_CCC |
>> + PCI_EXP_LNKCTL_RCB;
>> + pcie_rc_cfg_wr(lpp, val, PCIE_CAP_OFST + PCI_EXP_LNKCTL);
>> +}
>> +
>> +static void intel_pcie_max_speed_setup(struct intel_pcie_port *lpp)
>> +{
>> + u32 reg, val;
>> +
>> + reg = pcie_rc_cfg_rd(lpp, PCIE_CAP_OFST + PCI_EXP_LNKCTL2);
>> + switch (lpp->link_gen) {
>> + case PCIE_LINK_SPEED_GEN1:
>> + reg &= ~PCI_EXP_LNKCTL2_TLS;
>> + reg |= PCI_EXP_LNKCTL2_HASD|
>> + PCI_EXP_LNKCTL2_TLS_2_5GT;
>> + break;
>> + case PCIE_LINK_SPEED_GEN2:
>> + reg &= ~PCI_EXP_LNKCTL2_TLS;
>> + reg |= PCI_EXP_LNKCTL2_HASD|
>> + PCI_EXP_LNKCTL2_TLS_5_0GT;
>> + break;
>> + case PCIE_LINK_SPEED_GEN3:
>> + reg &= ~PCI_EXP_LNKCTL2_TLS;
>> + reg |= PCI_EXP_LNKCTL2_HASD|
>> + PCI_EXP_LNKCTL2_TLS_8_0GT;
>> + break;
>> + case PCIE_LINK_SPEED_GEN4:
>> + reg &= ~PCI_EXP_LNKCTL2_TLS;
>> + reg |= PCI_EXP_LNKCTL2_HASD|
>> + PCI_EXP_LNKCTL2_TLS_16_0GT;
>> + break;
>> + default:
>> + /* Use hardware capability */
>> + val = pcie_rc_cfg_rd(lpp, PCIE_CAP_OFST + PCI_EXP_LNKCAP);
>> + val = FIELD_GET(PCI_EXP_LNKCAP_SLS, val);
>> + reg &= ~PCI_EXP_LNKCTL2_HASD;
>> + reg |= val;
>> + break;
>> + }
>> +
>> + pcie_rc_cfg_wr(lpp, reg, PCIE_CAP_OFST + PCI_EXP_LNKCTL2);
>> + dw_pcie_link_set_n_fts(&lpp->pci, lpp->n_fts);
>> +}
>> +
>> +
>> +
> Lots of space here isn't needed.
i will remove it.
>
>> +static void intel_pcie_port_logic_setup(struct intel_pcie_port *lpp)
>> +{
>> + u32 val, mask;
>> +
>> + switch (lpp->max_speed) {
>> + case PCIE_LINK_SPEED_GEN3:
>> + lpp->n_fts = PORT_AFR_N_FTS_GEN3;
>> + break;
>> + case PCIE_LINK_SPEED_GEN4:
>> + 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);
>> + intel_pcie_max_speed_setup(lpp);
>> +}
>> +
>> +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;
>> +
>> + ret = device_property_read_u32(dev, "linux,pci-domain", &lpp->id);
>> + if (ret) {
>> + dev_err(dev, "failed to get domain id, errno %d\n", ret);
>> + return ret;
>> + }
> Why is this a required property?
I marked it required property because lpp->id is being used during debug
prints.
-> sprintf(buf, "Port %2u Width x%u Speed %s GT/s\n", lpp->id,
-> dev_info(dev, "Intel PCIe Root Complex Port %d init done\n", lpp->id);
Hmmm.. I found, domain id can be traversed from pci_host_bridge
structure after dw_pcie_host_init().
I will remove the code for getting the pci-domain here.
>
>> +
>> + 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 = RST_INTRVL_DFT_MS;
>> +
>> + ret = of_pci_get_max_link_speed(dev->of_node);
>> + lpp->link_gen = ret < 0 ? 0 : ret;
> I think the spacing may be a little off above.
Yes, typo error. I will correct it.
>
>> +
>> + 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 (lpp->max_speed < PCIE_LINK_SPEED_GEN3)
>> + 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)
>> +{
>> + 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;
>> + }
>> +
>> + 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_OFFS;
>> +}
>> +
>> +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);
>> + 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 configure
>> + * viewport to not to access IO region during register
>> + * read write operations.
>> + */
>> + pci->num_viewport = data->num_viewport;
>> + dev_info(dev, "Intel PCIe Root Complex Port %d init done\n", lpp->id);
>> +
>> + 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..f6e7e402f879 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 0x0200 /* Hw Autonomous Speed Disable */
> I think this should be 0x0020.
Yes, my miss, i will update.
Thanks for pointing it.
Thanks for reviewing and providing the inputs.
Regards,
Dilip
>
> Thanks,
>
> Andrew Murray
>
>> #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
>>
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH v4 2/3] dwc: PCI: intel: PCIe RC controller driver
2019-10-22 9:04 ` Dilip Kota
@ 2019-10-25 9:09 ` Andrew Murray
2019-10-29 8:59 ` Dilip Kota
0 siblings, 1 reply; 55+ messages in thread
From: Andrew Murray @ 2019-10-25 9:09 UTC (permalink / raw)
To: Dilip Kota
Cc: jingoohan1, gustavo.pimentel, lorenzo.pieralisi, robh,
martin.blumenstingl, linux-pci, hch, devicetree, linux-kernel,
andriy.shevchenko, cheol.yong.kim, chuanhua.lei, qi-ming.wu
On Tue, Oct 22, 2019 at 05:04:21PM +0800, Dilip Kota wrote:
> Hi Andrew Murray,
>
> On 10/21/2019 9:03 PM, Andrew Murray wrote:
> > On Mon, Oct 21, 2019 at 02:39:19PM +0800, Dilip Kota wrote:
> > > +
> > > +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;
> > > + val |= n_fts;
> > > + dw_pcie_writel_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL, val);
> > > +}
> > I notice that pcie-artpec6.c (artpec6_pcie_set_nfts) also writes the FTS
> > and defines a bunch of macros to support this. It doesn't make sense to
> > duplicate this there. Therefore I think we need to update pcie-artpec6.c
> > to use this new function.
> I think we can do in a separate patch after these changes get merged and
> keep this patch series for intel PCIe driver and required changes in PCIe
> DesignWare framework.
The pcie-artpec6.c is a DWC driver as well. So I think we can do all this
together. This helps reduce the technical debt that will otherwise build up
in duplicated code.
> > > 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..9142c70db808
> > > --- /dev/null
> > > +++ b/drivers/pci/controller/dwc/pcie-intel-gw.c
> > > @@ -0,0 +1,590 @@
> > > +// 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/interrupt.h>
> > > +#include <linux/iopoll.h>
> > > +#include <linux/of_irq.h>
> > > +#include <linux/of_pci.h>
> > > +#include <linux/of_platform.h>
> > > +#include <linux/pci_regs.h>
> > > +#include <linux/phy/phy.h>
> > > +#include <linux/platform_device.h>
> > > +#include <linux/reset.h>
> > > +
> > > +#include "../../pci.h"
> > I expected this to be removed, is it needed now?
>
> In the earlier patch you suggested to use "of_pci_get_max_link_speed()"
> instead of device_get_*.
> And, pci.h is required for it.
OK that makes sense.
> > > +
> > > +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;
> > > +
> > > + ret = device_property_read_u32(dev, "linux,pci-domain", &lpp->id);
> > > + if (ret) {
> > > + dev_err(dev, "failed to get domain id, errno %d\n", ret);
> > > + return ret;
> > > + }
> > Why is this a required property?
> I marked it required property because lpp->id is being used during debug
> prints.
> -> sprintf(buf, "Port %2u Width x%u Speed %s GT/s\n", lpp->id,
> -> dev_info(dev, "Intel PCIe Root Complex Port %d init done\n", lpp->id);
>
> Hmmm.. I found, domain id can be traversed from pci_host_bridge structure
> after dw_pcie_host_init().
> I will remove the code for getting the pci-domain here.
Excellent.
>
> >
> > > +#define PCI_EXP_LNKCTL2_HASD 0x0200 /* Hw Autonomous Speed Disable */
> > I think this should be 0x0020.
> Yes, my miss, i will update.
> Thanks for pointing it.
>
> Thanks for reviewing and providing the inputs.
> Regards,
> Dilip
> >
> > Thanks,
> >
> > Andrew Murray
Thanks,
Andrew Murray
> >
> > > #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
> > >
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH v4 2/3] dwc: PCI: intel: PCIe RC controller driver
2019-10-25 9:09 ` Andrew Murray
@ 2019-10-29 8:59 ` Dilip Kota
2019-11-01 10:59 ` Andrew Murray
0 siblings, 1 reply; 55+ messages in thread
From: Dilip Kota @ 2019-10-29 8:59 UTC (permalink / raw)
To: Andrew Murray
Cc: jingoohan1, gustavo.pimentel, lorenzo.pieralisi, robh,
martin.blumenstingl, linux-pci, hch, devicetree, linux-kernel,
andriy.shevchenko, cheol.yong.kim, chuanhua.lei, qi-ming.wu
On 10/25/2019 5:09 PM, Andrew Murray wrote:
> On Tue, Oct 22, 2019 at 05:04:21PM +0800, Dilip Kota wrote:
>> Hi Andrew Murray,
>>
>> On 10/21/2019 9:03 PM, Andrew Murray wrote:
>>> On Mon, Oct 21, 2019 at 02:39:19PM +0800, Dilip Kota wrote:
>>>> +
>>>> +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;
>>>> + val |= n_fts;
>>>> + dw_pcie_writel_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL, val);
>>>> +}
>>> I notice that pcie-artpec6.c (artpec6_pcie_set_nfts) also writes the FTS
>>> and defines a bunch of macros to support this. It doesn't make sense to
>>> duplicate this there. Therefore I think we need to update pcie-artpec6.c
>>> to use this new function.
>> I think we can do in a separate patch after these changes get merged and
>> keep this patch series for intel PCIe driver and required changes in PCIe
>> DesignWare framework.
> The pcie-artpec6.c is a DWC driver as well. So I think we can do all this
> together. This helps reduce the technical debt that will otherwise build up
> in duplicated code.
I agree with you to remove duplicated code, but at this point not sure
what all drivers has defined FTS configuration.
Reviewing all other DWC drivers and removing them can be done in one
single separate patch.
Regards,
Dilip
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH v4 2/3] dwc: PCI: intel: PCIe RC controller driver
2019-10-29 8:59 ` Dilip Kota
@ 2019-11-01 10:59 ` Andrew Murray
2019-11-04 9:34 ` Dilip Kota
0 siblings, 1 reply; 55+ messages in thread
From: Andrew Murray @ 2019-11-01 10:59 UTC (permalink / raw)
To: Dilip Kota
Cc: jingoohan1, gustavo.pimentel, lorenzo.pieralisi, robh,
martin.blumenstingl, linux-pci, hch, devicetree, linux-kernel,
andriy.shevchenko, cheol.yong.kim, chuanhua.lei, qi-ming.wu
On Tue, Oct 29, 2019 at 04:59:17PM +0800, Dilip Kota wrote:
>
> On 10/25/2019 5:09 PM, Andrew Murray wrote:
> > On Tue, Oct 22, 2019 at 05:04:21PM +0800, Dilip Kota wrote:
> > > Hi Andrew Murray,
> > >
> > > On 10/21/2019 9:03 PM, Andrew Murray wrote:
> > > > On Mon, Oct 21, 2019 at 02:39:19PM +0800, Dilip Kota wrote:
> > > > > +
> > > > > +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;
> > > > > + val |= n_fts;
> > > > > + dw_pcie_writel_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL, val);
> > > > > +}
> > > > I notice that pcie-artpec6.c (artpec6_pcie_set_nfts) also writes the FTS
> > > > and defines a bunch of macros to support this. It doesn't make sense to
> > > > duplicate this there. Therefore I think we need to update pcie-artpec6.c
> > > > to use this new function.
> > > I think we can do in a separate patch after these changes get merged and
> > > keep this patch series for intel PCIe driver and required changes in PCIe
> > > DesignWare framework.
> > The pcie-artpec6.c is a DWC driver as well. So I think we can do all this
> > together. This helps reduce the technical debt that will otherwise build up
> > in duplicated code.
> I agree with you to remove duplicated code, but at this point not sure what
> all drivers has defined FTS configuration.
> Reviewing all other DWC drivers and removing them can be done in one single
> separate patch.
I'm not asking to set up an FTS configuration for all DWC drivers, but instead
to move this helper function you've created to somewhere like pcie-designware.c
and call it from this driver and pcie-artpec6.c.
Thanks,
Andrew Murray
>
> Regards,
> Dilip
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH v4 2/3] dwc: PCI: intel: PCIe RC controller driver
2019-11-01 10:59 ` Andrew Murray
@ 2019-11-04 9:34 ` Dilip Kota
2019-11-04 10:47 ` Andrew Murray
0 siblings, 1 reply; 55+ messages in thread
From: Dilip Kota @ 2019-11-04 9:34 UTC (permalink / raw)
To: Andrew Murray
Cc: jingoohan1, gustavo.pimentel, lorenzo.pieralisi, robh,
martin.blumenstingl, linux-pci, hch, devicetree, linux-kernel,
andriy.shevchenko, cheol.yong.kim, chuanhua.lei, qi-ming.wu
On 11/1/2019 6:59 PM, Andrew Murray wrote:
> On Tue, Oct 29, 2019 at 04:59:17PM +0800, Dilip Kota wrote:
>> On 10/25/2019 5:09 PM, Andrew Murray wrote:
>>> On Tue, Oct 22, 2019 at 05:04:21PM +0800, Dilip Kota wrote:
>>>> Hi Andrew Murray,
>>>>
>>>> On 10/21/2019 9:03 PM, Andrew Murray wrote:
>>>>> On Mon, Oct 21, 2019 at 02:39:19PM +0800, Dilip Kota wrote:
>>>>>> +
>>>>>> +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;
>>>>>> + val |= n_fts;
>>>>>> + dw_pcie_writel_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL, val);
>>>>>> +}
>>>>> I notice that pcie-artpec6.c (artpec6_pcie_set_nfts) also writes the FTS
>>>>> and defines a bunch of macros to support this. It doesn't make sense to
>>>>> duplicate this there. Therefore I think we need to update pcie-artpec6.c
>>>>> to use this new function.
>>>> I think we can do in a separate patch after these changes get merged and
>>>> keep this patch series for intel PCIe driver and required changes in PCIe
>>>> DesignWare framework.
>>> The pcie-artpec6.c is a DWC driver as well. So I think we can do all this
>>> together. This helps reduce the technical debt that will otherwise build up
>>> in duplicated code.
>> I agree with you to remove duplicated code, but at this point not sure what
>> all drivers has defined FTS configuration.
>> Reviewing all other DWC drivers and removing them can be done in one single
>> separate patch.
> I'm not asking to set up an FTS configuration for all DWC drivers, but instead
> to move this helper function you've created to somewhere like pcie-designware.c
> and call it from this driver and pcie-artpec6.c.
What i mean is, we need to check how many of the current DWC drivers are
configuring the FTS
and call the helper function.
Today i have grep all the DWC based drivers and i see pcie-artpec6.c is
the only driver doing FTS configuration.
I will add the helper function call in pcie-artpec6.c in the next patch
version.
Regards,
Dilip
>
> Thanks,
>
> Andrew Murray
>
>> Regards,
>> Dilip
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH v4 2/3] dwc: PCI: intel: PCIe RC controller driver
2019-11-04 9:34 ` Dilip Kota
@ 2019-11-04 10:47 ` Andrew Murray
0 siblings, 0 replies; 55+ messages in thread
From: Andrew Murray @ 2019-11-04 10:47 UTC (permalink / raw)
To: Dilip Kota
Cc: jingoohan1, gustavo.pimentel, lorenzo.pieralisi, robh,
martin.blumenstingl, linux-pci, hch, devicetree, linux-kernel,
andriy.shevchenko, cheol.yong.kim, chuanhua.lei, qi-ming.wu
On Mon, Nov 04, 2019 at 05:34:54PM +0800, Dilip Kota wrote:
>
> On 11/1/2019 6:59 PM, Andrew Murray wrote:
> > On Tue, Oct 29, 2019 at 04:59:17PM +0800, Dilip Kota wrote:
> > > On 10/25/2019 5:09 PM, Andrew Murray wrote:
> > > > On Tue, Oct 22, 2019 at 05:04:21PM +0800, Dilip Kota wrote:
> > > > > Hi Andrew Murray,
> > > > >
> > > > > On 10/21/2019 9:03 PM, Andrew Murray wrote:
> > > > > > On Mon, Oct 21, 2019 at 02:39:19PM +0800, Dilip Kota wrote:
> > > > > > > +
> > > > > > > +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;
> > > > > > > + val |= n_fts;
> > > > > > > + dw_pcie_writel_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL, val);
> > > > > > > +}
> > > > > > I notice that pcie-artpec6.c (artpec6_pcie_set_nfts) also writes the FTS
> > > > > > and defines a bunch of macros to support this. It doesn't make sense to
> > > > > > duplicate this there. Therefore I think we need to update pcie-artpec6.c
> > > > > > to use this new function.
> > > > > I think we can do in a separate patch after these changes get merged and
> > > > > keep this patch series for intel PCIe driver and required changes in PCIe
> > > > > DesignWare framework.
> > > > The pcie-artpec6.c is a DWC driver as well. So I think we can do all this
> > > > together. This helps reduce the technical debt that will otherwise build up
> > > > in duplicated code.
> > > I agree with you to remove duplicated code, but at this point not sure what
> > > all drivers has defined FTS configuration.
> > > Reviewing all other DWC drivers and removing them can be done in one single
> > > separate patch.
> > I'm not asking to set up an FTS configuration for all DWC drivers, but instead
> > to move this helper function you've created to somewhere like pcie-designware.c
> > and call it from this driver and pcie-artpec6.c.
> What i mean is, we need to check how many of the current DWC drivers are
> configuring the FTS
> and call the helper function.
> Today i have grep all the DWC based drivers and i see pcie-artpec6.c is the
> only driver doing FTS configuration.
>
> I will add the helper function call in pcie-artpec6.c in the next patch
> version.
Thanks that's very much appreciated.
Thanks,
Andrew Murray
>
>
> Regards,
> Dilip
>
>
> >
> > Thanks,
> >
> > Andrew Murray
> >
> > > Regards,
> > > Dilip
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v4 2/3] dwc: PCI: intel: PCIe RC controller driver
2019-10-21 6:39 ` [PATCH v4 2/3] dwc: PCI: intel: PCIe RC controller driver Dilip Kota
2019-10-21 8:29 ` Gustavo Pimentel
2019-10-21 13:03 ` Andrew Murray
@ 2019-10-21 17:17 ` Bjorn Helgaas
2019-10-22 9:07 ` Dilip Kota
2019-10-24 6:57 ` kbuild test robot
` (2 subsequent siblings)
5 siblings, 1 reply; 55+ messages in thread
From: Bjorn Helgaas @ 2019-10-21 17:17 UTC (permalink / raw)
To: Dilip Kota
Cc: jingoohan1, gustavo.pimentel, lorenzo.pieralisi, andrew.murray,
robh, martin.blumenstingl, linux-pci, hch, devicetree,
linux-kernel, andriy.shevchenko, cheol.yong.kim, chuanhua.lei,
qi-ming.wu
On Mon, Oct 21, 2019 at 02:39:19PM +0800, Dilip Kota wrote:
> Add support to PCIe RC controller on Intel Gateway SoCs.
> PCIe controller is based of Synopsys DesignWare pci core.
>
> Intel PCIe driver requires Upconfig support, fast training
> sequence configuration and link speed change. 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.
>
> +static void intel_pcie_link_setup(struct intel_pcie_port *lpp)
> +{
> + u32 val;
> +
> + val = pcie_rc_cfg_rd(lpp, PCIE_CAP_OFST + 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, PCIE_CAP_OFST + PCI_EXP_LNKCTL);
> +
> + val &= ~(PCI_EXP_LNKCTL_LD | PCI_EXP_LNKCTL_ASPMC);
> + val |= (PCI_EXP_LNKSTA_SLC << 16) | PCI_EXP_LNKCTL_CCC |
> + PCI_EXP_LNKCTL_RCB;
Link Control is only 16 bits wide, so "PCI_EXP_LNKSTA_SLC << 16"
wouldn't make sense. But I guess you're writing a device-specific
register that is not actually the Link Control as documented in PCIe
r5.0, sec 7.5.3.7, even though the bits are similar?
Likewise, PCI_EXP_LNKCTL_RCB is RO for Root Ports, but maybe you're
telling the device what it should advertise in its Link Control?
PCI_EXP_LNKCTL_CCC is RW. But doesn't it depend on the components on
both ends of the link? Do you know what device is at the other end?
I would have assumed that you'd have to start with CCC==0, which
should be most conservative, then set CCC=1 only if you know both ends
have a common clock.
> + pcie_rc_cfg_wr(lpp, val, PCIE_CAP_OFST + PCI_EXP_LNKCTL);
> +}
> +
> +static void intel_pcie_max_speed_setup(struct intel_pcie_port *lpp)
> +{
> + u32 reg, val;
> +
> + reg = pcie_rc_cfg_rd(lpp, PCIE_CAP_OFST + PCI_EXP_LNKCTL2);
> + switch (lpp->link_gen) {
> + case PCIE_LINK_SPEED_GEN1:
> + reg &= ~PCI_EXP_LNKCTL2_TLS;
> + reg |= PCI_EXP_LNKCTL2_HASD|
> + PCI_EXP_LNKCTL2_TLS_2_5GT;
> + break;
> + case PCIE_LINK_SPEED_GEN2:
> + reg &= ~PCI_EXP_LNKCTL2_TLS;
> + reg |= PCI_EXP_LNKCTL2_HASD|
> + PCI_EXP_LNKCTL2_TLS_5_0GT;
> + break;
> + case PCIE_LINK_SPEED_GEN3:
> + reg &= ~PCI_EXP_LNKCTL2_TLS;
> + reg |= PCI_EXP_LNKCTL2_HASD|
> + PCI_EXP_LNKCTL2_TLS_8_0GT;
> + break;
> + case PCIE_LINK_SPEED_GEN4:
> + reg &= ~PCI_EXP_LNKCTL2_TLS;
> + reg |= PCI_EXP_LNKCTL2_HASD|
> + PCI_EXP_LNKCTL2_TLS_16_0GT;
> + break;
> + default:
> + /* Use hardware capability */
> + val = pcie_rc_cfg_rd(lpp, PCIE_CAP_OFST + PCI_EXP_LNKCAP);
> + val = FIELD_GET(PCI_EXP_LNKCAP_SLS, val);
> + reg &= ~PCI_EXP_LNKCTL2_HASD;
> + reg |= val;
> + break;
> + }
> +
> + pcie_rc_cfg_wr(lpp, reg, PCIE_CAP_OFST + PCI_EXP_LNKCTL2);
> + dw_pcie_link_set_n_fts(&lpp->pci, lpp->n_fts);
There are other users of of_pci_get_max_link_speed() that look sort of
similar to this (dra7xx_pcie_establish_link(),
ks_pcie_set_link_speed(), tegra_pcie_prepare_host()). Do these *need*
to be different, or is there something that could be factored out?
> +}
> +
> +
> +
Remove extra blank lines here.
> +static void intel_pcie_port_logic_setup(struct intel_pcie_port *lpp)
> ...
> + /* Intel PCIe doesn't configure IO region, so configure
> + * viewport to not to access IO region during register
> + * read write operations.
> + */
This comment doesn't describe the code. Is there supposed to be some
code here that configures the viewport? Where do we tell the viewport
not to access IO?
I guess maybe this means something like "tell
dw_pcie_access_other_conf() not to program an outbound ATU for I/O"?
I don't know that structure well enough to write this in a way that
makes sense, but this code doesn't look like it's configuring any
viewports.
Please use usual multi-line comment style, i.e.,
/*
* Intel PCIe ...
*/
> + pci->num_viewport = data->num_viewport;
> + dev_info(dev, "Intel PCIe Root Complex Port %d init done\n", lpp->id);
> +
> + return ret;
> +}
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH v4 2/3] dwc: PCI: intel: PCIe RC controller driver
2019-10-21 17:17 ` Bjorn Helgaas
@ 2019-10-22 9:07 ` Dilip Kota
2019-10-22 13:09 ` Bjorn Helgaas
0 siblings, 1 reply; 55+ messages in thread
From: Dilip Kota @ 2019-10-22 9:07 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: jingoohan1, gustavo.pimentel, lorenzo.pieralisi, andrew.murray,
robh, martin.blumenstingl, linux-pci, hch, devicetree,
linux-kernel, andriy.shevchenko, cheol.yong.kim, chuanhua.lei,
qi-ming.wu
Hi Bjorn Helgaas,
On 10/22/2019 1:17 AM, Bjorn Helgaas wrote:
> On Mon, Oct 21, 2019 at 02:39:19PM +0800, Dilip Kota wrote:
>> Add support to PCIe RC controller on Intel Gateway SoCs.
>> PCIe controller is based of Synopsys DesignWare pci core.
>>
>> Intel PCIe driver requires Upconfig support, fast training
>> sequence configuration and link speed change. 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.
>>
>> +static void intel_pcie_link_setup(struct intel_pcie_port *lpp)
>> +{
>> + u32 val;
>> +
>> + val = pcie_rc_cfg_rd(lpp, PCIE_CAP_OFST + 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, PCIE_CAP_OFST + PCI_EXP_LNKCTL);
>> +
>> + val &= ~(PCI_EXP_LNKCTL_LD | PCI_EXP_LNKCTL_ASPMC);
>> + val |= (PCI_EXP_LNKSTA_SLC << 16) | PCI_EXP_LNKCTL_CCC |
>> + PCI_EXP_LNKCTL_RCB;
> Link Control is only 16 bits wide, so "PCI_EXP_LNKSTA_SLC << 16"
> wouldn't make sense. But I guess you're writing a device-specific
> register that is not actually the Link Control as documented in PCIe
> r5.0, sec 7.5.3.7, even though the bits are similar?
It is not device specific.
You are correct about register size that pcie spec mentions
PCIE_EXP_LNKCTL at 0x10 and PCIE_EXP_LNKSTS at 0x12 each of 2bytes.
Accessing 4bytes at offset 0x10 ends up accessing LNK control and status
register.
In Synopsys PCIe controller LINK_CONTROL_LINK_STATUS_REG is of 4bytes
size at offset 0x10,
In both the cases everything is same except the size of the register, so
i used PCIE_EXP_LNKCTL macro which is already defined in pci_regs.h
>
> Likewise, PCI_EXP_LNKCTL_RCB is RO for Root Ports, but maybe you're
> telling the device what it should advertise in its Link Control?
You are correct, PCI_EXP_LNKCTL_RCB is RO. I will remove it.
>
> PCI_EXP_LNKCTL_CCC is RW. But doesn't it depend on the components on
> both ends of the link? Do you know what device is at the other end?
> I would have assumed that you'd have to start with CCC==0, which
> should be most conservative, then set CCC=1 only if you know both ends
> have a common clock.
PCIe RC and endpoint device are having the common clock so set the CCC=1.
>
>> + pcie_rc_cfg_wr(lpp, val, PCIE_CAP_OFST + PCI_EXP_LNKCTL);
>> +}
>> +
>> +static void intel_pcie_max_speed_setup(struct intel_pcie_port *lpp)
>> +{
>> + u32 reg, val;
>> +
>> + reg = pcie_rc_cfg_rd(lpp, PCIE_CAP_OFST + PCI_EXP_LNKCTL2);
>> + switch (lpp->link_gen) {
>> + case PCIE_LINK_SPEED_GEN1:
>> + reg &= ~PCI_EXP_LNKCTL2_TLS;
>> + reg |= PCI_EXP_LNKCTL2_HASD|
>> + PCI_EXP_LNKCTL2_TLS_2_5GT;
>> + break;
>> + case PCIE_LINK_SPEED_GEN2:
>> + reg &= ~PCI_EXP_LNKCTL2_TLS;
>> + reg |= PCI_EXP_LNKCTL2_HASD|
>> + PCI_EXP_LNKCTL2_TLS_5_0GT;
>> + break;
>> + case PCIE_LINK_SPEED_GEN3:
>> + reg &= ~PCI_EXP_LNKCTL2_TLS;
>> + reg |= PCI_EXP_LNKCTL2_HASD|
>> + PCI_EXP_LNKCTL2_TLS_8_0GT;
>> + break;
>> + case PCIE_LINK_SPEED_GEN4:
>> + reg &= ~PCI_EXP_LNKCTL2_TLS;
>> + reg |= PCI_EXP_LNKCTL2_HASD|
>> + PCI_EXP_LNKCTL2_TLS_16_0GT;
>> + break;
>> + default:
>> + /* Use hardware capability */
>> + val = pcie_rc_cfg_rd(lpp, PCIE_CAP_OFST + PCI_EXP_LNKCAP);
>> + val = FIELD_GET(PCI_EXP_LNKCAP_SLS, val);
>> + reg &= ~PCI_EXP_LNKCTL2_HASD;
>> + reg |= val;
>> + break;
>> + }
>> +
>> + pcie_rc_cfg_wr(lpp, reg, PCIE_CAP_OFST + PCI_EXP_LNKCTL2);
>> + dw_pcie_link_set_n_fts(&lpp->pci, lpp->n_fts);
> There are other users of of_pci_get_max_link_speed() that look sort of
> similar to this (dra7xx_pcie_establish_link(),
> ks_pcie_set_link_speed(), tegra_pcie_prepare_host()). Do these *need*
> to be different, or is there something that could be factored out?
dra7xx_pcie_establish_link() and ks_pcie_set_link_speed() are doing
speed configuration for GEN1 and GEN1 &2 respectively.
intel_pcie_max_speed_setup() can be moved to dwc framework and dra7xx and ks_pcie drivers can call.
>
>> +}
>> +
>> +
>> +
> Remove extra blank lines here.
i will remove it.
>
>> +static void intel_pcie_port_logic_setup(struct intel_pcie_port *lpp)
>> ...
>> + /* Intel PCIe doesn't configure IO region, so configure
>> + * viewport to not to access IO region during register
>> + * read write operations.
>> + */
> This comment doesn't describe the code. Is there supposed to be some
> code here that configures the viewport? Where do we tell the viewport
> not to access IO?
>
> I guess maybe this means something like "tell
> dw_pcie_access_other_conf() not to program an outbound ATU for I/O"?
> I don't know that structure well enough to write this in a way that
> makes sense, but this code doesn't look like it's configuring any
> viewports.
Yes, you are correct. Telling not to program an outbound ATU for IO.
I will update the description.
> Please use usual multi-line comment style, i.e.,
>
> /*
> * Intel PCIe ...
> */
Sure, i will correct it.
Thanks for reviewing and providing the valuable inputs!
Regards,
Dilip
>> + pci->num_viewport = data->num_viewport;
>> + dev_info(dev, "Intel PCIe Root Complex Port %d init done\n", lpp->id);
>> +
>> + return ret;
>> +}
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH v4 2/3] dwc: PCI: intel: PCIe RC controller driver
2019-10-22 9:07 ` Dilip Kota
@ 2019-10-22 13:09 ` Bjorn Helgaas
2019-10-29 7:45 ` Dilip Kota
0 siblings, 1 reply; 55+ messages in thread
From: Bjorn Helgaas @ 2019-10-22 13:09 UTC (permalink / raw)
To: Dilip Kota
Cc: jingoohan1, gustavo.pimentel, lorenzo.pieralisi, andrew.murray,
robh, martin.blumenstingl, linux-pci, hch, devicetree,
linux-kernel, andriy.shevchenko, cheol.yong.kim, chuanhua.lei,
qi-ming.wu
On Tue, Oct 22, 2019 at 05:07:47PM +0800, Dilip Kota wrote:
> On 10/22/2019 1:17 AM, Bjorn Helgaas wrote:
> > On Mon, Oct 21, 2019 at 02:39:19PM +0800, Dilip Kota wrote:
> > > Add support to PCIe RC controller on Intel Gateway SoCs.
> > > PCIe controller is based of Synopsys DesignWare pci core.
> > >
> > > Intel PCIe driver requires Upconfig support, fast training
> > > sequence configuration and link speed change. 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.
> > >
> > > +static void intel_pcie_link_setup(struct intel_pcie_port *lpp)
> > > +{
> > > + u32 val;
> > > +
> > > + val = pcie_rc_cfg_rd(lpp, PCIE_CAP_OFST + 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, PCIE_CAP_OFST + PCI_EXP_LNKCTL);
> > > +
> > > + val &= ~(PCI_EXP_LNKCTL_LD | PCI_EXP_LNKCTL_ASPMC);
> > > + val |= (PCI_EXP_LNKSTA_SLC << 16) | PCI_EXP_LNKCTL_CCC |
> > > + PCI_EXP_LNKCTL_RCB;
> > PCI_EXP_LNKCTL_CCC is RW. But doesn't it depend on the components on
> > both ends of the link? Do you know what device is at the other end?
> > I would have assumed that you'd have to start with CCC==0, which
> > should be most conservative, then set CCC=1 only if you know both ends
> > have a common clock.
> PCIe RC and endpoint device are having the common clock so set the CCC=1.
How do you know what the endpoint device is? Is this driver only for
a specific embedded configuration where the endpoint is always
soldered down? There's no possibility of this RC being used with a
connector?
Shouldn't this be either discoverable or configurable via DT or
something? pcie_aspm_configure_common_clock() seems to do something
similar, but I can't really vouch for its correctness.
Bjorn
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH v4 2/3] dwc: PCI: intel: PCIe RC controller driver
2019-10-22 13:09 ` Bjorn Helgaas
@ 2019-10-29 7:45 ` Dilip Kota
0 siblings, 0 replies; 55+ messages in thread
From: Dilip Kota @ 2019-10-29 7:45 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: jingoohan1, gustavo.pimentel, lorenzo.pieralisi, andrew.murray,
robh, martin.blumenstingl, linux-pci, hch, devicetree,
linux-kernel, andriy.shevchenko, cheol.yong.kim, chuanhua.lei,
qi-ming.wu
On 10/22/2019 9:09 PM, Bjorn Helgaas wrote:
> On Tue, Oct 22, 2019 at 05:07:47PM +0800, Dilip Kota wrote:
>> On 10/22/2019 1:17 AM, Bjorn Helgaas wrote:
>>> On Mon, Oct 21, 2019 at 02:39:19PM +0800, Dilip Kota wrote:
>>>> Add support to PCIe RC controller on Intel Gateway SoCs.
>>>> PCIe controller is based of Synopsys DesignWare pci core.
>>>>
>>>> Intel PCIe driver requires Upconfig support, fast training
>>>> sequence configuration and link speed change. 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.
>>>>
>>>> +static void intel_pcie_link_setup(struct intel_pcie_port *lpp)
>>>> +{
>>>> + u32 val;
>>>> +
>>>> + val = pcie_rc_cfg_rd(lpp, PCIE_CAP_OFST + 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, PCIE_CAP_OFST + PCI_EXP_LNKCTL);
>>>> +
>>>> + val &= ~(PCI_EXP_LNKCTL_LD | PCI_EXP_LNKCTL_ASPMC);
>>>> + val |= (PCI_EXP_LNKSTA_SLC << 16) | PCI_EXP_LNKCTL_CCC |
>>>> + PCI_EXP_LNKCTL_RCB;
>>> PCI_EXP_LNKCTL_CCC is RW. But doesn't it depend on the components on
>>> both ends of the link? Do you know what device is at the other end?
>>> I would have assumed that you'd have to start with CCC==0, which
>>> should be most conservative, then set CCC=1 only if you know both ends
>>> have a common clock.
>> PCIe RC and endpoint device are having the common clock so set the CCC=1.
> How do you know what the endpoint device is? Is this driver only for
> a specific embedded configuration where the endpoint is always
> soldered down? There's no possibility of this RC being used with a
> connector?
>
> Shouldn't this be either discoverable or configurable via DT or
> something? pcie_aspm_configure_common_clock() seems to do something
> similar, but I can't really vouch for its correctness.
(sorry for the late reply, i am back today from sick leave)
I see pcie_aspm_configure_common_clock() is getting called during pcie
root bus bridge scanning and programming the CCC.
So, CCC configuration can be removed here in intel_pcie_link_setup().
Regards,
Dilip
>
> Bjorn
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v4 2/3] dwc: PCI: intel: PCIe RC controller driver
2019-10-21 6:39 ` [PATCH v4 2/3] dwc: PCI: intel: PCIe RC controller driver Dilip Kota
` (2 preceding siblings ...)
2019-10-21 17:17 ` Bjorn Helgaas
@ 2019-10-24 6:57 ` kbuild test robot
2019-10-25 13:11 ` kbuild test robot
2019-10-25 13:11 ` [RFC PATCH] dwc: PCI: intel: intel_pcie_msi_init() can be static kbuild test robot
5 siblings, 0 replies; 55+ messages in thread
From: kbuild test robot @ 2019-10-24 6:57 UTC (permalink / raw)
To: Dilip Kota
Cc: kbuild-all, jingoohan1, gustavo.pimentel, lorenzo.pieralisi,
andrew.murray, robh, martin.blumenstingl, linux-pci, hch,
devicetree, linux-kernel, andriy.shevchenko, cheol.yong.kim,
chuanhua.lei, qi-ming.wu, Dilip Kota
[-- Attachment #1: Type: text/plain, Size: 32061 bytes --]
Hi Dilip,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on pci/next]
[cannot apply to v5.4-rc4 next-20191023]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Dilip-Kota/PCI-Add-Intel-PCIe-Driver-and-respective-dt-binding-yaml-file/20191024-103204
base: https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
config: sparc64-allmodconfig (attached as .config)
compiler: sparc64-linux-gcc (GCC) 7.4.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.4.0 make.cross ARCH=sparc64
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All error/warnings (new ones prefixed by >>):
>> drivers/pci/controller/dwc/pcie-designware-host.c:72:15: error: variable 'dw_pcie_msi_domain_info' has initializer but incomplete type
static struct msi_domain_info dw_pcie_msi_domain_info = {
^~~~~~~~~~~~~~~
>> drivers/pci/controller/dwc/pcie-designware-host.c:73:3: error: 'struct msi_domain_info' has no member named 'flags'
.flags = (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS |
^~~~~
>> drivers/pci/controller/dwc/pcie-designware-host.c:73:12: error: 'MSI_FLAG_USE_DEF_DOM_OPS' undeclared here (not in a function); did you mean 'SIMPLE_DEV_PM_OPS'?
.flags = (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS |
^~~~~~~~~~~~~~~~~~~~~~~~
SIMPLE_DEV_PM_OPS
>> drivers/pci/controller/dwc/pcie-designware-host.c:73:39: error: 'MSI_FLAG_USE_DEF_CHIP_OPS' undeclared here (not in a function); did you mean 'MSI_FLAG_USE_DEF_DOM_OPS'?
.flags = (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS |
^~~~~~~~~~~~~~~~~~~~~~~~~
MSI_FLAG_USE_DEF_DOM_OPS
>> drivers/pci/controller/dwc/pcie-designware-host.c:74:6: error: 'MSI_FLAG_PCI_MSIX' undeclared here (not in a function); did you mean 'CONFIG_PCI_MSI'?
MSI_FLAG_PCI_MSIX | MSI_FLAG_MULTI_PCI_MSI),
^~~~~~~~~~~~~~~~~
CONFIG_PCI_MSI
>> drivers/pci/controller/dwc/pcie-designware-host.c:74:26: error: 'MSI_FLAG_MULTI_PCI_MSI' undeclared here (not in a function); did you mean 'MSI_FLAG_PCI_MSIX'?
MSI_FLAG_PCI_MSIX | MSI_FLAG_MULTI_PCI_MSI),
^~~~~~~~~~~~~~~~~~~~~~
MSI_FLAG_PCI_MSIX
>> drivers/pci/controller/dwc/pcie-designware-host.c:73:11: warning: excess elements in struct initializer
.flags = (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS |
^
drivers/pci/controller/dwc/pcie-designware-host.c:73:11: note: (near initialization for 'dw_pcie_msi_domain_info')
>> drivers/pci/controller/dwc/pcie-designware-host.c:75:3: error: 'struct msi_domain_info' has no member named 'chip'
.chip = &dw_pcie_msi_irq_chip,
^~~~
drivers/pci/controller/dwc/pcie-designware-host.c:75:10: warning: excess elements in struct initializer
.chip = &dw_pcie_msi_irq_chip,
^
drivers/pci/controller/dwc/pcie-designware-host.c:75:10: note: (near initialization for 'dw_pcie_msi_domain_info')
drivers/pci/controller/dwc/pcie-designware-host.c: In function 'dw_pcie_allocate_domains':
>> drivers/pci/controller/dwc/pcie-designware-host.c:266:19: error: implicit declaration of function 'pci_msi_create_irq_domain'; did you mean 'pci_msi_get_device_domain'? [-Werror=implicit-function-declaration]
pp->msi_domain = pci_msi_create_irq_domain(fwnode,
^~~~~~~~~~~~~~~~~~~~~~~~~
pci_msi_get_device_domain
>> drivers/pci/controller/dwc/pcie-designware-host.c:266:17: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
pp->msi_domain = pci_msi_create_irq_domain(fwnode,
^
drivers/pci/controller/dwc/pcie-designware-host.c: At top level:
>> drivers/pci/controller/dwc/pcie-designware-host.c:72:31: error: storage size of 'dw_pcie_msi_domain_info' isn't known
static struct msi_domain_info dw_pcie_msi_domain_info = {
^~~~~~~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
vim +/dw_pcie_msi_domain_info +72 drivers/pci/controller/dwc/pcie-designware-host.c
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 71
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 @72 static struct msi_domain_info dw_pcie_msi_domain_info = {
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 @73 .flags = (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS |
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 @74 MSI_FLAG_PCI_MSIX | MSI_FLAG_MULTI_PCI_MSI),
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 @75 .chip = &dw_pcie_msi_irq_chip,
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 76 };
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 77
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 78 /* MSI int handler */
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 79 irqreturn_t dw_handle_msi_irq(struct pcie_port *pp)
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 80 {
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 81 int i, pos, irq;
1f319cb0538a10 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 82 u32 val, num_ctrls;
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 83 irqreturn_t ret = IRQ_NONE;
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 84
1f319cb0538a10 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 85 num_ctrls = pp->num_vectors / MAX_MSI_IRQS_PER_CTRL;
1f319cb0538a10 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 86
1f319cb0538a10 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 87 for (i = 0; i < num_ctrls; i++) {
76cbf066b1ab75 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-05-14 88 dw_pcie_rd_own_conf(pp, PCIE_MSI_INTR0_STATUS +
76cbf066b1ab75 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-05-14 89 (i * MSI_REG_CTRL_BLOCK_SIZE),
76cbf066b1ab75 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-05-14 90 4, &val);
dbe4a09e8bbcf8 drivers/pci/dwc/pcie-designware-host.c Bjorn Helgaas 2017-03-16 91 if (!val)
dbe4a09e8bbcf8 drivers/pci/dwc/pcie-designware-host.c Bjorn Helgaas 2017-03-16 92 continue;
dbe4a09e8bbcf8 drivers/pci/dwc/pcie-designware-host.c Bjorn Helgaas 2017-03-16 93
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 94 ret = IRQ_HANDLED;
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 95 pos = 0;
76cbf066b1ab75 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-05-14 96 while ((pos = find_next_bit((unsigned long *) &val,
76cbf066b1ab75 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-05-14 97 MAX_MSI_IRQS_PER_CTRL,
76cbf066b1ab75 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-05-14 98 pos)) != MAX_MSI_IRQS_PER_CTRL) {
76cbf066b1ab75 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-05-14 99 irq = irq_find_mapping(pp->irq_domain,
76cbf066b1ab75 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-05-14 100 (i * MAX_MSI_IRQS_PER_CTRL) +
76cbf066b1ab75 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-05-14 101 pos);
8c934095fa2f33 drivers/pci/dwc/pcie-designware-host.c Faiz Abbas 2017-08-10 102 generic_handle_irq(irq);
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 103 pos++;
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 104 }
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 105 }
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 106
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 107 return ret;
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 108 }
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 109
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 110 /* Chained MSI interrupt service routine */
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 111 static void dw_chained_msi_isr(struct irq_desc *desc)
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 112 {
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 113 struct irq_chip *chip = irq_desc_get_chip(desc);
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 114 struct pcie_port *pp;
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 115
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 116 chained_irq_enter(chip, desc);
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 117
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 118 pp = irq_desc_get_handler_data(desc);
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 119 dw_handle_msi_irq(pp);
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 120
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 121 chained_irq_exit(chip, desc);
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 122 }
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 123
59ea68b3f17294 drivers/pci/controller/dwc/pcie-designware-host.c Gustavo Pimentel 2019-01-31 124 static void dw_pci_setup_msi_msg(struct irq_data *d, struct msi_msg *msg)
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 125 {
59ea68b3f17294 drivers/pci/controller/dwc/pcie-designware-host.c Gustavo Pimentel 2019-01-31 126 struct pcie_port *pp = irq_data_get_irq_chip_data(d);
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 127 struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 128 u64 msi_target;
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 129
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 130 msi_target = (u64)pp->msi_data;
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 131
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 132 msg->address_lo = lower_32_bits(msi_target);
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 133 msg->address_hi = upper_32_bits(msi_target);
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 134
59ea68b3f17294 drivers/pci/controller/dwc/pcie-designware-host.c Gustavo Pimentel 2019-01-31 135 msg->data = d->hwirq;
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 136
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 137 dev_dbg(pci->dev, "msi#%d address_hi %#x address_lo %#x\n",
59ea68b3f17294 drivers/pci/controller/dwc/pcie-designware-host.c Gustavo Pimentel 2019-01-31 138 (int)d->hwirq, msg->address_hi, msg->address_lo);
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 139 }
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 140
fd5288a362ab55 drivers/pci/controller/dwc/pcie-designware-host.c Gustavo Pimentel 2019-01-31 141 static int dw_pci_msi_set_affinity(struct irq_data *d,
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 142 const struct cpumask *mask, bool force)
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 143 {
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 144 return -EINVAL;
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 145 }
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 146
40e9892ef94ce8 drivers/pci/controller/dwc/pcie-designware-host.c Gustavo Pimentel 2019-01-31 147 static void dw_pci_bottom_mask(struct irq_data *d)
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 148 {
40e9892ef94ce8 drivers/pci/controller/dwc/pcie-designware-host.c Gustavo Pimentel 2019-01-31 149 struct pcie_port *pp = irq_data_get_irq_chip_data(d);
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 150 unsigned int res, bit, ctrl;
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 151 unsigned long flags;
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 152
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 153 raw_spin_lock_irqsave(&pp->lock, flags);
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 154
40e9892ef94ce8 drivers/pci/controller/dwc/pcie-designware-host.c Gustavo Pimentel 2019-01-31 155 ctrl = d->hwirq / MAX_MSI_IRQS_PER_CTRL;
76cbf066b1ab75 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-05-14 156 res = ctrl * MSI_REG_CTRL_BLOCK_SIZE;
40e9892ef94ce8 drivers/pci/controller/dwc/pcie-designware-host.c Gustavo Pimentel 2019-01-31 157 bit = d->hwirq % MAX_MSI_IRQS_PER_CTRL;
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 158
657722570a555c drivers/pci/controller/dwc/pcie-designware-host.c Gustavo Pimentel 2019-01-31 159 pp->irq_mask[ctrl] |= BIT(bit);
830920e065e90d drivers/pci/controller/dwc/pcie-designware-host.c Marc Zyngier 2018-11-13 160 dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_MASK + res, 4,
a348d015f0de7a drivers/pci/controller/dwc/pcie-designware-host.c Gustavo Pimentel 2019-01-31 161 pp->irq_mask[ctrl]);
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 162
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 163 raw_spin_unlock_irqrestore(&pp->lock, flags);
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 164 }
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 165
40e9892ef94ce8 drivers/pci/controller/dwc/pcie-designware-host.c Gustavo Pimentel 2019-01-31 166 static void dw_pci_bottom_unmask(struct irq_data *d)
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 167 {
40e9892ef94ce8 drivers/pci/controller/dwc/pcie-designware-host.c Gustavo Pimentel 2019-01-31 168 struct pcie_port *pp = irq_data_get_irq_chip_data(d);
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 169 unsigned int res, bit, ctrl;
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 170 unsigned long flags;
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 171
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 172 raw_spin_lock_irqsave(&pp->lock, flags);
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 173
40e9892ef94ce8 drivers/pci/controller/dwc/pcie-designware-host.c Gustavo Pimentel 2019-01-31 174 ctrl = d->hwirq / MAX_MSI_IRQS_PER_CTRL;
76cbf066b1ab75 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-05-14 175 res = ctrl * MSI_REG_CTRL_BLOCK_SIZE;
40e9892ef94ce8 drivers/pci/controller/dwc/pcie-designware-host.c Gustavo Pimentel 2019-01-31 176 bit = d->hwirq % MAX_MSI_IRQS_PER_CTRL;
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 177
657722570a555c drivers/pci/controller/dwc/pcie-designware-host.c Gustavo Pimentel 2019-01-31 178 pp->irq_mask[ctrl] &= ~BIT(bit);
830920e065e90d drivers/pci/controller/dwc/pcie-designware-host.c Marc Zyngier 2018-11-13 179 dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_MASK + res, 4,
a348d015f0de7a drivers/pci/controller/dwc/pcie-designware-host.c Gustavo Pimentel 2019-01-31 180 pp->irq_mask[ctrl]);
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 181
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 182 raw_spin_unlock_irqrestore(&pp->lock, flags);
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 183 }
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 184
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 185 static void dw_pci_bottom_ack(struct irq_data *d)
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 186 {
3f7bb2ec20ce07 drivers/pci/controller/dwc/pcie-designware-host.c Marc Zyngier 2018-11-13 187 struct pcie_port *pp = irq_data_get_irq_chip_data(d);
3f7bb2ec20ce07 drivers/pci/controller/dwc/pcie-designware-host.c Marc Zyngier 2018-11-13 188 unsigned int res, bit, ctrl;
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 189
3f7bb2ec20ce07 drivers/pci/controller/dwc/pcie-designware-host.c Marc Zyngier 2018-11-13 190 ctrl = d->hwirq / MAX_MSI_IRQS_PER_CTRL;
3f7bb2ec20ce07 drivers/pci/controller/dwc/pcie-designware-host.c Marc Zyngier 2018-11-13 191 res = ctrl * MSI_REG_CTRL_BLOCK_SIZE;
3f7bb2ec20ce07 drivers/pci/controller/dwc/pcie-designware-host.c Marc Zyngier 2018-11-13 192 bit = d->hwirq % MAX_MSI_IRQS_PER_CTRL;
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 193
657722570a555c drivers/pci/controller/dwc/pcie-designware-host.c Gustavo Pimentel 2019-01-31 194 dw_pcie_wr_own_conf(pp, PCIE_MSI_INTR0_STATUS + res, 4, BIT(bit));
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 195 }
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 196
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 197 static struct irq_chip dw_pci_msi_bottom_irq_chip = {
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 198 .name = "DWPCI-MSI",
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 199 .irq_ack = dw_pci_bottom_ack,
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 200 .irq_compose_msi_msg = dw_pci_setup_msi_msg,
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 201 .irq_set_affinity = dw_pci_msi_set_affinity,
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 202 .irq_mask = dw_pci_bottom_mask,
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 203 .irq_unmask = dw_pci_bottom_unmask,
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 204 };
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 205
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 206 static int dw_pcie_irq_domain_alloc(struct irq_domain *domain,
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 207 unsigned int virq, unsigned int nr_irqs,
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 208 void *args)
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 209 {
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 210 struct pcie_port *pp = domain->host_data;
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 211 unsigned long flags;
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 212 u32 i;
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 213 int bit;
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 214
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 215 raw_spin_lock_irqsave(&pp->lock, flags);
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 216
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 217 bit = bitmap_find_free_region(pp->msi_irq_in_use, pp->num_vectors,
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 218 order_base_2(nr_irqs));
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 219
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 220 raw_spin_unlock_irqrestore(&pp->lock, flags);
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 221
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 222 if (bit < 0)
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 223 return -ENOSPC;
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 224
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 225 for (i = 0; i < nr_irqs; i++)
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 226 irq_domain_set_info(domain, virq + i, bit + i,
9f67437b3a0858 drivers/pci/controller/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2019-03-21 227 pp->msi_irq_chip,
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 228 pp, handle_edge_irq,
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 229 NULL, NULL);
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 230
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 231 return 0;
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 232 }
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 233
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 234 static void dw_pcie_irq_domain_free(struct irq_domain *domain,
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 235 unsigned int virq, unsigned int nr_irqs)
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 236 {
4cfae0f1f8ce16 drivers/pci/controller/dwc/pcie-designware-host.c Gustavo Pimentel 2019-01-31 237 struct irq_data *d = irq_domain_get_irq_data(domain, virq);
4cfae0f1f8ce16 drivers/pci/controller/dwc/pcie-designware-host.c Gustavo Pimentel 2019-01-31 238 struct pcie_port *pp = irq_data_get_irq_chip_data(d);
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 239 unsigned long flags;
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 240
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 241 raw_spin_lock_irqsave(&pp->lock, flags);
b4a8a51caf7de4 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-05-14 242
4cfae0f1f8ce16 drivers/pci/controller/dwc/pcie-designware-host.c Gustavo Pimentel 2019-01-31 243 bitmap_release_region(pp->msi_irq_in_use, d->hwirq,
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 244 order_base_2(nr_irqs));
b4a8a51caf7de4 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-05-14 245
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 246 raw_spin_unlock_irqrestore(&pp->lock, flags);
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 247 }
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 248
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 249 static const struct irq_domain_ops dw_pcie_msi_domain_ops = {
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 250 .alloc = dw_pcie_irq_domain_alloc,
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 251 .free = dw_pcie_irq_domain_free,
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 252 };
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 253
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 254 int dw_pcie_allocate_domains(struct pcie_port *pp)
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 255 {
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 256 struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 257 struct fwnode_handle *fwnode = of_node_to_fwnode(pci->dev->of_node);
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 258
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 259 pp->irq_domain = irq_domain_create_linear(fwnode, pp->num_vectors,
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 260 &dw_pcie_msi_domain_ops, pp);
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 261 if (!pp->irq_domain) {
b4a8a51caf7de4 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-05-14 262 dev_err(pci->dev, "Failed to create IRQ domain\n");
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 263 return -ENOMEM;
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 264 }
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 265
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 @266 pp->msi_domain = pci_msi_create_irq_domain(fwnode,
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 267 &dw_pcie_msi_domain_info,
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 268 pp->irq_domain);
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 269 if (!pp->msi_domain) {
b4a8a51caf7de4 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-05-14 270 dev_err(pci->dev, "Failed to create MSI domain\n");
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 271 irq_domain_remove(pp->irq_domain);
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 272 return -ENOMEM;
7c5925afbc58c6 drivers/pci/dwc/pcie-designware-host.c Gustavo Pimentel 2018-03-06 273 }
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 274
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 275 return 0;
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 276 }
feb85d9b1c47ea drivers/pci/dwc/pcie-designware-host.c Kishon Vijay Abraham I 2017-02-15 277
:::::: The code at line 72 was first introduced by commit
:::::: 7c5925afbc58c6d6b384e1dc051bb992969bf787 PCI: dwc: Move MSI IRQs allocation to IRQ domains hierarchical API
:::::: TO: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
:::::: CC: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 59108 bytes --]
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH v4 2/3] dwc: PCI: intel: PCIe RC controller driver
2019-10-21 6:39 ` [PATCH v4 2/3] dwc: PCI: intel: PCIe RC controller driver Dilip Kota
` (3 preceding siblings ...)
2019-10-24 6:57 ` kbuild test robot
@ 2019-10-25 13:11 ` kbuild test robot
2019-10-25 13:11 ` [RFC PATCH] dwc: PCI: intel: intel_pcie_msi_init() can be static kbuild test robot
5 siblings, 0 replies; 55+ messages in thread
From: kbuild test robot @ 2019-10-25 13:11 UTC (permalink / raw)
To: Dilip Kota
Cc: kbuild-all, jingoohan1, gustavo.pimentel, lorenzo.pieralisi,
andrew.murray, robh, martin.blumenstingl, linux-pci, hch,
devicetree, linux-kernel, andriy.shevchenko, cheol.yong.kim,
chuanhua.lei, qi-ming.wu, Dilip Kota
Hi Dilip,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on pci/next]
[cannot apply to v5.4-rc4 next-20191025]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Dilip-Kota/PCI-Add-Intel-PCIe-Driver-and-respective-dt-binding-yaml-file/20191024-103204
base: https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
reproduce:
# apt-get install sparse
# sparse version: v0.6.1-dirty
make ARCH=x86_64 allmodconfig
make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
>> drivers/pci/controller/dwc/pcie-intel-gw.c:497:5: sparse: sparse: symbol 'intel_pcie_msi_init' was not declared. Should it be static?
>> drivers/pci/controller/dwc/pcie-intel-gw.c:503:5: sparse: sparse: symbol 'intel_pcie_cpu_addr' was not declared. Should it be static?
Please review and possibly fold the followup patch.
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply [flat|nested] 55+ messages in thread* [RFC PATCH] dwc: PCI: intel: intel_pcie_msi_init() can be static
2019-10-21 6:39 ` [PATCH v4 2/3] dwc: PCI: intel: PCIe RC controller driver Dilip Kota
` (4 preceding siblings ...)
2019-10-25 13:11 ` kbuild test robot
@ 2019-10-25 13:11 ` kbuild test robot
5 siblings, 0 replies; 55+ messages in thread
From: kbuild test robot @ 2019-10-25 13:11 UTC (permalink / raw)
To: Dilip Kota
Cc: kbuild-all, jingoohan1, gustavo.pimentel, lorenzo.pieralisi,
andrew.murray, robh, martin.blumenstingl, linux-pci, hch,
devicetree, linux-kernel, andriy.shevchenko, cheol.yong.kim,
chuanhua.lei, qi-ming.wu, Dilip Kota
Fixes: 3686a0f46840 ("dwc: PCI: intel: PCIe RC controller driver")
Signed-off-by: kbuild test robot <lkp@intel.com>
---
pcie-intel-gw.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-intel-gw.c b/drivers/pci/controller/dwc/pcie-intel-gw.c
index 9142c70db8085..352d0d03171b2 100644
--- a/drivers/pci/controller/dwc/pcie-intel-gw.c
+++ b/drivers/pci/controller/dwc/pcie-intel-gw.c
@@ -494,13 +494,13 @@ static int intel_pcie_rc_init(struct pcie_port *pp)
return intel_pcie_host_setup(lpp);
}
-int intel_pcie_msi_init(struct pcie_port *pp)
+static 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)
+static u64 intel_pcie_cpu_addr(struct dw_pcie *pcie, u64 cpu_addr)
{
return cpu_addr + BUS_IATU_OFFS;
}
^ permalink raw reply related [flat|nested] 55+ messages in thread