Add Gen4 specific equalization and rx margining settings. These
settings are inline with respective PHY settings for Gen4
operation.
In addition, current QCOM EP and RC drivers do not share common
codebase which would result in code duplication. Hence, adding
common files for code reusability among RC and EP drivers.
v1 -> v2:
- Capitilized commit message to be inline with history
- Dropped stubs from header file.
- Moved Designware specific register offsets and masks to
pcie-designware.h header file.
- Applied settings based on bus data rate rather than link generation.
- Addressed review comments from Bjorn and Frank.
Shashank Babu Chinta Venkata (3):
PCI: qcom: Refactor common code
PCI: qcom: Add equalization settings for gen4
PCI: qcom: Add rx margining settings for gen4
drivers/pci/controller/dwc/Kconfig | 5 +
drivers/pci/controller/dwc/Makefile | 1 +
drivers/pci/controller/dwc/pcie-designware.h | 38 +++++
drivers/pci/controller/dwc/pcie-qcom-cmn.c | 152 +++++++++++++++++++
drivers/pci/controller/dwc/pcie-qcom-cmn.h | 28 ++++
drivers/pci/controller/dwc/pcie-qcom-ep.c | 44 ++----
drivers/pci/controller/dwc/pcie-qcom.c | 72 ++-------
7 files changed, 246 insertions(+), 94 deletions(-)
create mode 100644 drivers/pci/controller/dwc/pcie-qcom-cmn.c
create mode 100644 drivers/pci/controller/dwc/pcie-qcom-cmn.h
--
2.43.2
Add rx margining settings for gen4 operation.
Signed-off-by: Shashank Babu Chinta Venkata <[email protected]>
---
drivers/pci/controller/dwc/pcie-designware.h | 23 +++++++++++++
drivers/pci/controller/dwc/pcie-qcom-cmn.c | 35 ++++++++++++++++++++
drivers/pci/controller/dwc/pcie-qcom-cmn.h | 11 +++++-
drivers/pci/controller/dwc/pcie-qcom-ep.c | 4 ++-
drivers/pci/controller/dwc/pcie-qcom.c | 4 ++-
5 files changed, 74 insertions(+), 3 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 064744bfe35a..ce1c5f9c406a 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -206,6 +206,29 @@
#define PCIE_PL_CHK_REG_ERR_ADDR 0xB28
+/*
+ * GEN4 lane margining register definitions
+ */
+#define GEN4_LANE_MARGINING_1_OFF 0xb80
+#define MARGINING_MAX_VOLTAGE_OFFSET_MASK GENMASK(29, 24)
+#define MARGINING_NUM_VOLTAGE_STEPS_MASK GENMASK(22, 16)
+#define MARGINING_MAX_TIMING_OFFSET_MASK GENMASK(13, 8)
+#define MARGINING_NUM_TIMING_STEPS_MASK GENMASK(5, 0)
+#define MARGINING_MAX_VOLTAGE_OFFSET_SHIFT 24
+#define MARGINING_NUM_VOLTAGE_STEPS_SHIFT 16
+#define MARGINING_MAX_TIMING_OFFSET_SHIFT 8
+
+#define GEN4_LANE_MARGINING_2_OFF 0xb84
+#define MARGINING_IND_ERROR_SAMPLER BIT(28)
+#define MARGINING_SAMPLE_REPORTING_METHOD BIT(27)
+#define MARGINING_IND_LEFT_RIGHT_TIMING BIT(26)
+#define MARGINING_IND_UP_DOWN_VOLTAGE BIT(25)
+#define MARGINING_VOLTAGE_SUPPORTED BIT(24)
+#define MARGINING_MAXLANES_MASK GENMASK(20, 16)
+#define MARGINING_SAMPLE_RATE_TIMING_MASK GENMASK(13, 8)
+#define MARGINING_SAMPLE_RATE_VOLTAGE_MASK GENMASK(5, 0)
+#define MARGINING_MAXLANES_SHIFT 16
+#define MARGINING_SAMPLE_RATE_TIMING_SHIFT 8
/*
* iATU Unroll-specific register definitions
* From 4.80 core version the address translation will be made by unroll
diff --git a/drivers/pci/controller/dwc/pcie-qcom-cmn.c b/drivers/pci/controller/dwc/pcie-qcom-cmn.c
index 208a55e8e9a1..bf6b27ee8327 100644
--- a/drivers/pci/controller/dwc/pcie-qcom-cmn.c
+++ b/drivers/pci/controller/dwc/pcie-qcom-cmn.c
@@ -53,6 +53,41 @@ void qcom_pcie_cmn_set_16gt_eq_settings(struct dw_pcie *pci)
}
EXPORT_SYMBOL_GPL(qcom_pcie_cmn_set_16gt_eq_settings);
+void qcom_pcie_cmn_set_16gt_rx_margining_settings(struct dw_pcie *pci)
+{
+ u32 reg;
+
+ reg = dw_pcie_readl_dbi(pci, GEN4_LANE_MARGINING_1_OFF);
+ reg &= ~MARGINING_MAX_VOLTAGE_OFFSET_MASK;
+ reg |= (MARGINING_MAX_VOLTAGE_OFFSET_VAL <<
+ MARGINING_MAX_VOLTAGE_OFFSET_SHIFT);
+ reg &= ~MARGINING_NUM_VOLTAGE_STEPS_MASK;
+ reg |= (MARGINING_NUM_VOLTAGE_STEPS_VAL <<
+ MARGINING_NUM_VOLTAGE_STEPS_SHIFT);
+ reg &= ~MARGINING_MAX_TIMING_OFFSET_MASK;
+ reg |= (MARGINING_MAX_TIMING_OFFSET_VAL <<
+ MARGINING_MAX_TIMING_OFFSET_SHIFT);
+ reg &= ~MARGINING_NUM_TIMING_STEPS_MASK;
+ reg |= MARGINING_NUM_TIMING_STEPS_VAL;
+ dw_pcie_writel_dbi(pci, GEN4_LANE_MARGINING_1_OFF, reg);
+
+ reg = dw_pcie_readl_dbi(pci, GEN4_LANE_MARGINING_2_OFF);
+ reg |= MARGINING_IND_ERROR_SAMPLER;
+ reg |= MARGINING_SAMPLE_REPORTING_METHOD;
+ reg |= MARGINING_IND_LEFT_RIGHT_TIMING;
+ reg |= MARGINING_VOLTAGE_SUPPORTED;
+ reg &= ~MARGINING_IND_UP_DOWN_VOLTAGE;
+ reg &= ~MARGINING_MAXLANES_MASK;
+ reg |= (pci->num_lanes <<
+ MARGINING_MAXLANES_SHIFT);
+ reg &= ~MARGINING_SAMPLE_RATE_TIMING_MASK;
+ reg |= (MARGINING_SAMPLE_RATE_TIMING_VAL <<
+ MARGINING_SAMPLE_RATE_TIMING_SHIFT);
+ reg |= MARGINING_SAMPLE_RATE_VOLTAGE_VAL;
+ dw_pcie_writel_dbi(pci, GEN4_LANE_MARGINING_2_OFF, reg);
+}
+EXPORT_SYMBOL_GPL(qcom_pcie_cmn_set_16gt_rx_margining_settings);
+
int qcom_pcie_cmn_icc_get_resource(struct dw_pcie *pci, struct icc_path *icc_mem)
{
if (IS_ERR(pci))
diff --git a/drivers/pci/controller/dwc/pcie-qcom-cmn.h b/drivers/pci/controller/dwc/pcie-qcom-cmn.h
index 97302e8fafa8..11d6b00372ec 100644
--- a/drivers/pci/controller/dwc/pcie-qcom-cmn.h
+++ b/drivers/pci/controller/dwc/pcie-qcom-cmn.h
@@ -11,9 +11,18 @@
#define GEN3_EQ_FMDC_MAX_PRE_CUSROR_DELTA_16GT_VAL 0x5
#define GEN3_EQ_FMDC_MAX_POST_CUSROR_DELTA_16GT_VAL 0x5
-#define GEN3_EQ_FMDC_N_EVALS_16GT_VAL 0xD
+#define GEN3_EQ_FMDC_N_EVALS_16GT_VAL 0xD
+
+/* GEN4(16GT/s) RX margining settings */
+#define MARGINING_MAX_VOLTAGE_OFFSET_VAL 0x24
+#define MARGINING_NUM_VOLTAGE_STEPS_VAL 0x78
+#define MARGINING_MAX_TIMING_OFFSET_VAL 0x32
+#define MARGINING_NUM_TIMING_STEPS_VAL 0x10
+#define MARGINING_SAMPLE_RATE_TIMING_VAL 0x3f
+#define MARGINING_SAMPLE_RATE_VOLTAGE_VAL 0x3f
int qcom_pcie_cmn_icc_get_resource(struct dw_pcie *pci, struct icc_path *icc_mem);
int qcom_pcie_cmn_icc_init(struct dw_pcie *pci, struct icc_path *icc_mem);
void qcom_pcie_cmn_icc_update(struct dw_pcie *pci, struct icc_path *icc_mem);
void qcom_pcie_cmn_set_16gt_eq_settings(struct dw_pcie *pci);
+void qcom_pcie_cmn_set_16gt_rx_margining_settings(struct dw_pcie *pci);
diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
index b6bcab21bb9f..b4d53f96eb9e 100644
--- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
+++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
@@ -438,8 +438,10 @@ static int qcom_pcie_perst_deassert(struct dw_pcie *pci)
goto err_disable_resources;
}
- if (pcie_link_speed[pci->link_gen] == PCIE_SPEED_16_0GT)
+ if (pcie_link_speed[pci->link_gen] == PCIE_SPEED_16_0GT) {
qcom_pcie_cmn_set_16gt_eq_settings(pci);
+ qcom_pcie_cmn_set_16gt_rx_margining_settings(pci);
+ }
/*
* The physical address of the MMIO region which is exposed as the BAR
diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index b0a22a000fa3..3dfb4d165ba5 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -263,8 +263,10 @@ static int qcom_pcie_start_link(struct dw_pcie *pci)
{
struct qcom_pcie *pcie = to_qcom_pcie(pci);
- if (pcie_link_speed[pci->link_gen] == PCIE_SPEED_16_0GT)
+ if (pcie_link_speed[pci->link_gen] == PCIE_SPEED_16_0GT) {
qcom_pcie_cmn_set_16gt_eq_settings(pci);
+ qcom_pcie_cmn_set_16gt_rx_margining_settings(pci);
+ }
/* Enable Link Training state machine */
if (pcie->cfg->ops->ltssm_enable)
--
2.43.2
GEN3_RELATED_OFFSET is being used as shadow register for generation4 and
generation5 data rates based on rate select mask settings on this register.
Select relevant mask and equalization settings for generation4 operation.
Signed-off-by: Shashank Babu Chinta Venkata <[email protected]>
---
drivers/pci/controller/dwc/pcie-designware.h | 15 ++++++++
drivers/pci/controller/dwc/pcie-qcom-cmn.c | 36 ++++++++++++++++++++
drivers/pci/controller/dwc/pcie-qcom-cmn.h | 5 +++
drivers/pci/controller/dwc/pcie-qcom-ep.c | 3 ++
drivers/pci/controller/dwc/pcie-qcom.c | 3 ++
5 files changed, 62 insertions(+)
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 26dae4837462..064744bfe35a 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -122,6 +122,21 @@
#define GEN3_RELATED_OFF_RATE_SHADOW_SEL_SHIFT 24
#define GEN3_RELATED_OFF_RATE_SHADOW_SEL_MASK GENMASK(25, 24)
+#define GEN3_EQ_CONTROL_OFF 0x8a8
+#define GEN3_EQ_CONTROL_OFF_FB_MODE_MASK GENMASK(3, 0)
+#define GEN3_EQ_CONTROL_OFF_PHASE23_EXIT_MODE BIT(4)
+#define GEN3_EQ_CONTROL_OFF_PSET_REQ_VEC_MASK GENMASK(23, 8)
+#define GEN3_EQ_CONTROL_OFF_FOM_INC_INITIAL_EVAL BIT(24)
+
+#define GEN3_EQ_FB_MODE_DIR_CHANGE_OFF 0x8ac
+#define GEN3_EQ_FMDC_T_MIN_PHASE23_MASK GENMASK(4, 0)
+#define GEN3_EQ_FMDC_N_EVALS_MASK GENMASK(9, 5)
+#define GEN3_EQ_FMDC_MAX_PRE_CUSROR_DELTA_MASK GENMASK(13, 10)
+#define GEN3_EQ_FMDC_MAX_POST_CUSROR_DELTA_MASK GENMASK(17, 14)
+#define GEN3_EQ_FMDC_N_EVALS_SHIFT 5
+#define GEN3_EQ_FMDC_MAX_PRE_CUSROR_DELTA_SHIFT 10
+#define GEN3_EQ_FMDC_MAX_POST_CUSROR_DELTA_SHIFT 14
+
#define PCIE_PORT_MULTI_LANE_CTRL 0x8C0
#define PORT_MLTI_UPCFG_SUPPORT BIT(7)
diff --git a/drivers/pci/controller/dwc/pcie-qcom-cmn.c b/drivers/pci/controller/dwc/pcie-qcom-cmn.c
index 64fa412ec293..208a55e8e9a1 100644
--- a/drivers/pci/controller/dwc/pcie-qcom-cmn.c
+++ b/drivers/pci/controller/dwc/pcie-qcom-cmn.c
@@ -17,6 +17,42 @@
#define QCOM_PCIE_LINK_SPEED_TO_BW(speed) \
Mbps_to_icc(PCIE_SPEED2MBS_ENC(pcie_link_speed[speed]))
+void qcom_pcie_cmn_set_16gt_eq_settings(struct dw_pcie *pci)
+{
+ u32 reg;
+
+ /*
+ * GEN3_RELATED_OFF is repurposed to be used with GEN4(16GT/s) rate
+ * as well based on RATE_SHADOW_SEL_MASK settings on this register.
+ */
+ reg = dw_pcie_readl_dbi(pci, GEN3_RELATED_OFF);
+ reg &= ~GEN3_RELATED_OFF_GEN3_ZRXDC_NONCOMPL;
+ reg &= ~GEN3_RELATED_OFF_RATE_SHADOW_SEL_MASK;
+ reg |= (0x1 << GEN3_RELATED_OFF_RATE_SHADOW_SEL_SHIFT);
+ dw_pcie_writel_dbi(pci, GEN3_RELATED_OFF, reg);
+
+ reg = dw_pcie_readl_dbi(pci, GEN3_EQ_FB_MODE_DIR_CHANGE_OFF);
+ reg &= ~GEN3_EQ_FMDC_T_MIN_PHASE23_MASK;
+ reg &= ~GEN3_EQ_FMDC_N_EVALS_MASK;
+ reg |= (GEN3_EQ_FMDC_N_EVALS_16GT_VAL <<
+ GEN3_EQ_FMDC_N_EVALS_SHIFT);
+ reg &= ~GEN3_EQ_FMDC_MAX_PRE_CUSROR_DELTA_MASK;
+ reg |= (GEN3_EQ_FMDC_MAX_PRE_CUSROR_DELTA_16GT_VAL <<
+ GEN3_EQ_FMDC_MAX_PRE_CUSROR_DELTA_SHIFT);
+ reg &= ~GEN3_EQ_FMDC_MAX_POST_CUSROR_DELTA_MASK;
+ reg |= (GEN3_EQ_FMDC_MAX_POST_CUSROR_DELTA_16GT_VAL <<
+ GEN3_EQ_FMDC_MAX_POST_CUSROR_DELTA_SHIFT);
+ dw_pcie_writel_dbi(pci, GEN3_EQ_FB_MODE_DIR_CHANGE_OFF, reg);
+
+ reg = dw_pcie_readl_dbi(pci, GEN3_EQ_CONTROL_OFF);
+ reg &= ~GEN3_EQ_CONTROL_OFF_FB_MODE_MASK;
+ reg &= ~GEN3_EQ_CONTROL_OFF_PHASE23_EXIT_MODE;
+ reg &= ~GEN3_EQ_CONTROL_OFF_FOM_INC_INITIAL_EVAL;
+ reg &= ~GEN3_EQ_CONTROL_OFF_PSET_REQ_VEC_MASK;
+ dw_pcie_writel_dbi(pci, GEN3_EQ_CONTROL_OFF, reg);
+}
+EXPORT_SYMBOL_GPL(qcom_pcie_cmn_set_16gt_eq_settings);
+
int qcom_pcie_cmn_icc_get_resource(struct dw_pcie *pci, struct icc_path *icc_mem)
{
if (IS_ERR(pci))
diff --git a/drivers/pci/controller/dwc/pcie-qcom-cmn.h b/drivers/pci/controller/dwc/pcie-qcom-cmn.h
index 845eda23ae59..97302e8fafa8 100644
--- a/drivers/pci/controller/dwc/pcie-qcom-cmn.h
+++ b/drivers/pci/controller/dwc/pcie-qcom-cmn.h
@@ -9,6 +9,11 @@
#include "../../pci.h"
#include "pcie-designware.h"
+#define GEN3_EQ_FMDC_MAX_PRE_CUSROR_DELTA_16GT_VAL 0x5
+#define GEN3_EQ_FMDC_MAX_POST_CUSROR_DELTA_16GT_VAL 0x5
+#define GEN3_EQ_FMDC_N_EVALS_16GT_VAL 0xD
+
int qcom_pcie_cmn_icc_get_resource(struct dw_pcie *pci, struct icc_path *icc_mem);
int qcom_pcie_cmn_icc_init(struct dw_pcie *pci, struct icc_path *icc_mem);
void qcom_pcie_cmn_icc_update(struct dw_pcie *pci, struct icc_path *icc_mem);
+void qcom_pcie_cmn_set_16gt_eq_settings(struct dw_pcie *pci);
diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
index ce6343426de8..b6bcab21bb9f 100644
--- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
+++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
@@ -438,6 +438,9 @@ static int qcom_pcie_perst_deassert(struct dw_pcie *pci)
goto err_disable_resources;
}
+ if (pcie_link_speed[pci->link_gen] == PCIE_SPEED_16_0GT)
+ qcom_pcie_cmn_set_16gt_eq_settings(pci);
+
/*
* The physical address of the MMIO region which is exposed as the BAR
* should be written to MHI BASE registers.
diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 57a08294c561..b0a22a000fa3 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -263,6 +263,9 @@ static int qcom_pcie_start_link(struct dw_pcie *pci)
{
struct qcom_pcie *pcie = to_qcom_pcie(pci);
+ if (pcie_link_speed[pci->link_gen] == PCIE_SPEED_16_0GT)
+ qcom_pcie_cmn_set_16gt_eq_settings(pci);
+
/* Enable Link Training state machine */
if (pcie->cfg->ops->ltssm_enable)
pcie->cfg->ops->ltssm_enable(pcie);
--
2.43.2
Refactor common code from RC(Root Complex) and EP(End Point)
drivers and move them to a common repository. This acts as placeholder
for common source code for both drivers avoiding duplication.
Signed-off-by: Shashank Babu Chinta Venkata <[email protected]>
---
drivers/pci/controller/dwc/Kconfig | 5 ++
drivers/pci/controller/dwc/Makefile | 1 +
drivers/pci/controller/dwc/pcie-qcom-cmn.c | 81 ++++++++++++++++++++++
drivers/pci/controller/dwc/pcie-qcom-cmn.h | 14 ++++
drivers/pci/controller/dwc/pcie-qcom-ep.c | 39 ++---------
drivers/pci/controller/dwc/pcie-qcom.c | 67 +++---------------
6 files changed, 113 insertions(+), 94 deletions(-)
create mode 100644 drivers/pci/controller/dwc/pcie-qcom-cmn.c
create mode 100644 drivers/pci/controller/dwc/pcie-qcom-cmn.h
diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
index 8afacc90c63b..41d2746edc5f 100644
--- a/drivers/pci/controller/dwc/Kconfig
+++ b/drivers/pci/controller/dwc/Kconfig
@@ -265,12 +265,16 @@ config PCIE_DW_PLAT_EP
order to enable device-specific features PCI_DW_PLAT_EP must be
selected.
+config PCIE_QCOM_CMN
+ bool
+
config PCIE_QCOM
bool "Qualcomm PCIe controller (host mode)"
depends on OF && (ARCH_QCOM || COMPILE_TEST)
depends on PCI_MSI
select PCIE_DW_HOST
select CRC8
+ select PCIE_QCOM_CMN
help
Say Y here to enable PCIe controller support on Qualcomm SoCs. The
PCIe controller uses the DesignWare core plus Qualcomm-specific
@@ -281,6 +285,7 @@ config PCIE_QCOM_EP
depends on OF && (ARCH_QCOM || COMPILE_TEST)
depends on PCI_ENDPOINT
select PCIE_DW_EP
+ select PCIE_QCOM_CMN
help
Say Y here to enable support for the PCIe controllers on Qualcomm SoCs
to work in endpoint mode. The PCIe controller uses the DesignWare core
diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
index bac103faa523..521572093ebf 100644
--- a/drivers/pci/controller/dwc/Makefile
+++ b/drivers/pci/controller/dwc/Makefile
@@ -14,6 +14,7 @@ obj-$(CONFIG_PCI_LAYERSCAPE) += pci-layerscape.o
obj-$(CONFIG_PCI_LAYERSCAPE_EP) += pci-layerscape-ep.o
obj-$(CONFIG_PCIE_QCOM) += pcie-qcom.o
obj-$(CONFIG_PCIE_QCOM_EP) += pcie-qcom-ep.o
+obj-$(CONFIG_PCIE_QCOM_CMN) += pcie-qcom-cmn.o
obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o
obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o
obj-$(CONFIG_PCIE_ROCKCHIP_DW_HOST) += pcie-dw-rockchip.o
diff --git a/drivers/pci/controller/dwc/pcie-qcom-cmn.c b/drivers/pci/controller/dwc/pcie-qcom-cmn.c
new file mode 100644
index 000000000000..64fa412ec293
--- /dev/null
+++ b/drivers/pci/controller/dwc/pcie-qcom-cmn.c
@@ -0,0 +1,81 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2014-2015, 2020 The Linux Foundation. All rights reserved.
+ * Copyright 2015, 2021 Linaro Limited.
+ * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
+ *
+ */
+
+#include <linux/debugfs.h>
+#include <linux/pci.h>
+#include <linux/interconnect.h>
+
+#include "../../pci.h"
+#include "pcie-designware.h"
+#include "pcie-qcom-cmn.h"
+
+#define QCOM_PCIE_LINK_SPEED_TO_BW(speed) \
+ Mbps_to_icc(PCIE_SPEED2MBS_ENC(pcie_link_speed[speed]))
+
+int qcom_pcie_cmn_icc_get_resource(struct dw_pcie *pci, struct icc_path *icc_mem)
+{
+ if (IS_ERR(pci))
+ return PTR_ERR(pci);
+
+ icc_mem = devm_of_icc_get(pci->dev, "pcie-mem");
+ if (IS_ERR(icc_mem))
+ return PTR_ERR(icc_mem);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(qcom_pcie_cmn_icc_get_resource);
+
+int qcom_pcie_cmn_icc_init(struct dw_pcie *pci, struct icc_path *icc_mem)
+{
+ int ret;
+
+ if (IS_ERR(pci))
+ return PTR_ERR(pci);
+
+ if (IS_ERR(icc_mem))
+ return PTR_ERR(icc_mem);
+
+ /*
+ * Some Qualcomm platforms require interconnect bandwidth constraints
+ * to be set before enabling interconnect clocks.
+ *
+ * Set an initial peak bandwidth corresponding to single-lane Gen 1
+ * for the pcie-mem path.
+ */
+ ret = icc_set_bw(icc_mem, 0, QCOM_PCIE_LINK_SPEED_TO_BW(1));
+ if (ret) {
+ dev_err(pci->dev, "failed to set interconnect bandwidth: %d\n",
+ ret);
+ return ret;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(qcom_pcie_cmn_icc_init);
+
+void qcom_pcie_cmn_icc_update(struct dw_pcie *pci, struct icc_path *icc_mem)
+{
+ u32 offset, status;
+ int speed, width;
+ int ret;
+
+ if (!icc_mem)
+ return;
+
+ offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
+ status = readw(pci->dbi_base + offset + PCI_EXP_LNKSTA);
+
+ speed = FIELD_GET(PCI_EXP_LNKSTA_CLS, status);
+ width = FIELD_GET(PCI_EXP_LNKSTA_NLW, status);
+
+ ret = icc_set_bw(icc_mem, 0, width * QCOM_PCIE_LINK_SPEED_TO_BW(speed));
+ if (ret)
+ dev_err(pci->dev, "failed to set interconnect bandwidth: %d\n",
+ ret);
+}
+EXPORT_SYMBOL_GPL(qcom_pcie_cmn_icc_update);
diff --git a/drivers/pci/controller/dwc/pcie-qcom-cmn.h b/drivers/pci/controller/dwc/pcie-qcom-cmn.h
new file mode 100644
index 000000000000..845eda23ae59
--- /dev/null
+++ b/drivers/pci/controller/dwc/pcie-qcom-cmn.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2014-2015, 2020 The Linux Foundation. All rights reserved.
+ * Copyright 2015, 2021 Linaro Limited.
+ * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+
+#include <linux/pci.h>
+#include "../../pci.h"
+#include "pcie-designware.h"
+
+int qcom_pcie_cmn_icc_get_resource(struct dw_pcie *pci, struct icc_path *icc_mem);
+int qcom_pcie_cmn_icc_init(struct dw_pcie *pci, struct icc_path *icc_mem);
+void qcom_pcie_cmn_icc_update(struct dw_pcie *pci, struct icc_path *icc_mem);
diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
index 36e5e80cd22f..ce6343426de8 100644
--- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
+++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
@@ -25,6 +25,7 @@
#include "../../pci.h"
#include "pcie-designware.h"
+#include "pcie-qcom-cmn.h"
/* PARF registers */
#define PARF_SYS_CTRL 0x00
@@ -137,9 +138,6 @@
#define CORE_RESET_TIME_US_MAX 1005
#define WAKE_DELAY_US 2000 /* 2 ms */
-#define QCOM_PCIE_LINK_SPEED_TO_BW(speed) \
- Mbps_to_icc(PCIE_SPEED2MBS_ENC(pcie_link_speed[speed]))
-
#define to_pcie_ep(x) dev_get_drvdata((x)->dev)
enum qcom_pcie_ep_link_status {
@@ -278,28 +276,6 @@ static void qcom_pcie_dw_write_dbi2(struct dw_pcie *pci, void __iomem *base,
writel(0, pcie_ep->elbi + ELBI_CS2_ENABLE);
}
-static void qcom_pcie_ep_icc_update(struct qcom_pcie_ep *pcie_ep)
-{
- struct dw_pcie *pci = &pcie_ep->pci;
- u32 offset, status;
- int speed, width;
- int ret;
-
- if (!pcie_ep->icc_mem)
- return;
-
- offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
- status = readw(pci->dbi_base + offset + PCI_EXP_LNKSTA);
-
- speed = FIELD_GET(PCI_EXP_LNKSTA_CLS, status);
- width = FIELD_GET(PCI_EXP_LNKSTA_NLW, status);
-
- ret = icc_set_bw(pcie_ep->icc_mem, 0, width * QCOM_PCIE_LINK_SPEED_TO_BW(speed));
- if (ret)
- dev_err(pci->dev, "failed to set interconnect bandwidth: %d\n",
- ret);
-}
-
static int qcom_pcie_enable_resources(struct qcom_pcie_ep *pcie_ep)
{
struct dw_pcie *pci = &pcie_ep->pci;
@@ -325,14 +301,7 @@ static int qcom_pcie_enable_resources(struct qcom_pcie_ep *pcie_ep)
if (ret)
goto err_phy_exit;
- /*
- * Some Qualcomm platforms require interconnect bandwidth constraints
- * to be set before enabling interconnect clocks.
- *
- * Set an initial peak bandwidth corresponding to single-lane Gen 1
- * for the pcie-mem path.
- */
- ret = icc_set_bw(pcie_ep->icc_mem, 0, QCOM_PCIE_LINK_SPEED_TO_BW(1));
+ ret = qcom_pcie_cmn_icc_init(pci, pcie_ep->icc_mem);
if (ret) {
dev_err(pci->dev, "failed to set interconnect bandwidth: %d\n",
ret);
@@ -616,7 +585,7 @@ static int qcom_pcie_ep_get_resources(struct platform_device *pdev,
if (IS_ERR(pcie_ep->phy))
ret = PTR_ERR(pcie_ep->phy);
- pcie_ep->icc_mem = devm_of_icc_get(dev, "pcie-mem");
+ ret = qcom_pcie_cmn_icc_get_resource(&pcie_ep->pci, pcie_ep->icc_mem);
if (IS_ERR(pcie_ep->icc_mem))
ret = PTR_ERR(pcie_ep->icc_mem);
@@ -643,7 +612,7 @@ static irqreturn_t qcom_pcie_ep_global_irq_thread(int irq, void *data)
} else if (FIELD_GET(PARF_INT_ALL_BME, status)) {
dev_dbg(dev, "Received BME event. Link is enabled!\n");
pcie_ep->link_status = QCOM_PCIE_EP_LINK_ENABLED;
- qcom_pcie_ep_icc_update(pcie_ep);
+ qcom_pcie_cmn_icc_update(pci, pcie_ep->icc_mem);
pci_epc_bme_notify(pci->ep.epc);
} else if (FIELD_GET(PARF_INT_ALL_PM_TURNOFF, status)) {
dev_dbg(dev, "Received PM Turn-off event! Entering L23\n");
diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 2ce2a3bd932b..57a08294c561 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -32,6 +32,7 @@
#include <linux/types.h>
#include "../../pci.h"
+#include "pcie-qcom-cmn.h"
#include "pcie-designware.h"
/* PARF registers */
@@ -147,9 +148,6 @@
#define QCOM_PCIE_CRC8_POLYNOMIAL (BIT(2) | BIT(1) | BIT(0))
-#define QCOM_PCIE_LINK_SPEED_TO_BW(speed) \
- Mbps_to_icc(PCIE_SPEED2MBS_ENC(pcie_link_speed[speed]))
-
#define QCOM_PCIE_1_0_0_MAX_CLOCKS 4
struct qcom_pcie_resources_1_0_0 {
struct clk_bulk_data clks[QCOM_PCIE_1_0_0_MAX_CLOCKS];
@@ -1363,59 +1361,6 @@ static const struct dw_pcie_ops dw_pcie_ops = {
.start_link = qcom_pcie_start_link,
};
-static int qcom_pcie_icc_init(struct qcom_pcie *pcie)
-{
- struct dw_pcie *pci = pcie->pci;
- int ret;
-
- pcie->icc_mem = devm_of_icc_get(pci->dev, "pcie-mem");
- if (IS_ERR(pcie->icc_mem))
- return PTR_ERR(pcie->icc_mem);
-
- /*
- * Some Qualcomm platforms require interconnect bandwidth constraints
- * to be set before enabling interconnect clocks.
- *
- * Set an initial peak bandwidth corresponding to single-lane Gen 1
- * for the pcie-mem path.
- */
- ret = icc_set_bw(pcie->icc_mem, 0, QCOM_PCIE_LINK_SPEED_TO_BW(1));
- if (ret) {
- dev_err(pci->dev, "failed to set interconnect bandwidth: %d\n",
- ret);
- return ret;
- }
-
- return 0;
-}
-
-static void qcom_pcie_icc_update(struct qcom_pcie *pcie)
-{
- struct dw_pcie *pci = pcie->pci;
- u32 offset, status;
- int speed, width;
- int ret;
-
- if (!pcie->icc_mem)
- return;
-
- offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
- status = readw(pci->dbi_base + offset + PCI_EXP_LNKSTA);
-
- /* Only update constraints if link is up. */
- if (!(status & PCI_EXP_LNKSTA_DLLLA))
- return;
-
- speed = FIELD_GET(PCI_EXP_LNKSTA_CLS, status);
- width = FIELD_GET(PCI_EXP_LNKSTA_NLW, status);
-
- ret = icc_set_bw(pcie->icc_mem, 0, width * QCOM_PCIE_LINK_SPEED_TO_BW(speed));
- if (ret) {
- dev_err(pci->dev, "failed to set interconnect bandwidth: %d\n",
- ret);
- }
-}
-
static int qcom_pcie_link_transition_count(struct seq_file *s, void *data)
{
struct qcom_pcie *pcie = (struct qcom_pcie *)dev_get_drvdata(s->private);
@@ -1524,7 +1469,11 @@ static int qcom_pcie_probe(struct platform_device *pdev)
goto err_pm_runtime_put;
}
- ret = qcom_pcie_icc_init(pcie);
+ ret = qcom_pcie_cmn_icc_get_resource(pcie->pci, pcie->icc_mem);
+ if (ret)
+ goto err_pm_runtime_put;
+
+ ret = qcom_pcie_cmn_icc_init(pcie->pci, pcie->icc_mem);
if (ret)
goto err_pm_runtime_put;
@@ -1546,7 +1495,7 @@ static int qcom_pcie_probe(struct platform_device *pdev)
goto err_phy_exit;
}
- qcom_pcie_icc_update(pcie);
+ qcom_pcie_cmn_icc_update(pcie->pci, pcie->icc_mem);
if (pcie->mhi)
qcom_pcie_init_debugfs(pcie);
@@ -1613,7 +1562,7 @@ static int qcom_pcie_resume_noirq(struct device *dev)
pcie->suspended = false;
}
- qcom_pcie_icc_update(pcie);
+ qcom_pcie_cmn_icc_update(pcie->pci, pcie->icc_mem);
return 0;
}
--
2.43.2
On 20.03.2024 08:14, Shashank Babu Chinta Venkata wrote:
> GEN3_RELATED_OFFSET is being used as shadow register for generation4 and
> generation5 data rates based on rate select mask settings on this register.
> Select relevant mask and equalization settings for generation4 operation.
>
> Signed-off-by: Shashank Babu Chinta Venkata <[email protected]>
> ---
[...]
> +
> +#define GEN3_EQ_FB_MODE_DIR_CHANGE_OFF 0x8ac
> +#define GEN3_EQ_FMDC_T_MIN_PHASE23_MASK GENMASK(4, 0)
> +#define GEN3_EQ_FMDC_N_EVALS_MASK GENMASK(9, 5)
> +#define GEN3_EQ_FMDC_MAX_PRE_CUSROR_DELTA_MASK GENMASK(13, 10)
> +#define GEN3_EQ_FMDC_MAX_POST_CUSROR_DELTA_MASK GENMASK(17, 14)
> +#define GEN3_EQ_FMDC_N_EVALS_SHIFT 5
> +#define GEN3_EQ_FMDC_MAX_PRE_CUSROR_DELTA_SHIFT 10
> +#define GEN3_EQ_FMDC_MAX_POST_CUSROR_DELTA_SHIFT 14
The beauty of bitops.h is you no longer need to define these shifts..
Just use FIELD_GET/FIELD_PREP with the field! Please also drop _MASK
from the leftover definitions.
> +void qcom_pcie_cmn_set_16gt_eq_settings(struct dw_pcie *pci)
> +{
> + u32 reg;
> +
> + /*
> + * GEN3_RELATED_OFF is repurposed to be used with GEN4(16GT/s) rate
> + * as well based on RATE_SHADOW_SEL_MASK settings on this register.
> + */
Given this comment and the commit message, should setting of this field
be factored out to a function that would accept a generation argument?
> + reg = dw_pcie_readl_dbi(pci, GEN3_RELATED_OFF);
> + reg &= ~GEN3_RELATED_OFF_GEN3_ZRXDC_NONCOMPL;
> + reg &= ~GEN3_RELATED_OFF_RATE_SHADOW_SEL_MASK;
> + reg |= (0x1 << GEN3_RELATED_OFF_RATE_SHADOW_SEL_SHIFT);
> + dw_pcie_writel_dbi(pci, GEN3_RELATED_OFF, reg);
> +
> + reg = dw_pcie_readl_dbi(pci, GEN3_EQ_FB_MODE_DIR_CHANGE_OFF);
> + reg &= ~GEN3_EQ_FMDC_T_MIN_PHASE23_MASK;
> + reg &= ~GEN3_EQ_FMDC_N_EVALS_MASK;
> + reg |= (GEN3_EQ_FMDC_N_EVALS_16GT_VAL <<
> + GEN3_EQ_FMDC_N_EVALS_SHIFT);
> + reg &= ~GEN3_EQ_FMDC_MAX_PRE_CUSROR_DELTA_MASK;
> + reg |= (GEN3_EQ_FMDC_MAX_PRE_CUSROR_DELTA_16GT_VAL <<
> + GEN3_EQ_FMDC_MAX_PRE_CUSROR_DELTA_SHIFT);
> + reg &= ~GEN3_EQ_FMDC_MAX_POST_CUSROR_DELTA_MASK;
> + reg |= (GEN3_EQ_FMDC_MAX_POST_CUSROR_DELTA_16GT_VAL <<
> + GEN3_EQ_FMDC_MAX_POST_CUSROR_DELTA_SHIFT);
> + dw_pcie_writel_dbi(pci, GEN3_EQ_FB_MODE_DIR_CHANGE_OFF, reg);
> +
> + reg = dw_pcie_readl_dbi(pci, GEN3_EQ_CONTROL_OFF);
> + reg &= ~GEN3_EQ_CONTROL_OFF_FB_MODE_MASK;
> + reg &= ~GEN3_EQ_CONTROL_OFF_PHASE23_EXIT_MODE;
> + reg &= ~GEN3_EQ_CONTROL_OFF_FOM_INC_INITIAL_EVAL;
> + reg &= ~GEN3_EQ_CONTROL_OFF_PSET_REQ_VEC_MASK;
> + dw_pcie_writel_dbi(pci, GEN3_EQ_CONTROL_OFF, reg);
> +}
On 20.03.2024 08:14, Shashank Babu Chinta Venkata wrote:
> Add rx margining settings for gen4 operation.
Why are these necessary? What do they change?
>
> Signed-off-by: Shashank Babu Chinta Venkata <[email protected]>
> ---
> drivers/pci/controller/dwc/pcie-designware.h | 23 +++++++++++++
> drivers/pci/controller/dwc/pcie-qcom-cmn.c | 35 ++++++++++++++++++++
> drivers/pci/controller/dwc/pcie-qcom-cmn.h | 11 +++++-
> drivers/pci/controller/dwc/pcie-qcom-ep.c | 4 ++-
> drivers/pci/controller/dwc/pcie-qcom.c | 4 ++-
> 5 files changed, 74 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 064744bfe35a..ce1c5f9c406a 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -206,6 +206,29 @@
>
> #define PCIE_PL_CHK_REG_ERR_ADDR 0xB28
>
> +/*
> + * GEN4 lane margining register definitions
> + */
> +#define GEN4_LANE_MARGINING_1_OFF 0xb80
> +#define MARGINING_MAX_VOLTAGE_OFFSET_MASK GENMASK(29, 24)
> +#define MARGINING_NUM_VOLTAGE_STEPS_MASK GENMASK(22, 16)
> +#define MARGINING_MAX_TIMING_OFFSET_MASK GENMASK(13, 8)
> +#define MARGINING_NUM_TIMING_STEPS_MASK GENMASK(5, 0)
> +#define MARGINING_MAX_VOLTAGE_OFFSET_SHIFT 24
> +#define MARGINING_NUM_VOLTAGE_STEPS_SHIFT 16
> +#define MARGINING_MAX_TIMING_OFFSET_SHIFT 8
> +
> +#define GEN4_LANE_MARGINING_2_OFF 0xb84
The file
drivers/accel/habanalabs/include/gaudi2/asic_reg/pcie_dbi_regs.h
defines registers with exactly the same names at exacly the same offsets.
If this is a DWC-common thing, it should go to DWC-common code.
Konrad
On Wed, Mar 20, 2024 at 12:14:45AM -0700, Shashank Babu Chinta Venkata wrote:
> Refactor common code from RC(Root Complex) and EP(End Point)
> drivers and move them to a common repository. This acts as placeholder
s/repository/driver
> for common source code for both drivers avoiding duplication.
'thus avoiding'
>
> Signed-off-by: Shashank Babu Chinta Venkata <[email protected]>
> ---
> drivers/pci/controller/dwc/Kconfig | 5 ++
> drivers/pci/controller/dwc/Makefile | 1 +
> drivers/pci/controller/dwc/pcie-qcom-cmn.c | 81 ++++++++++++++++++++++
I'd prefer, pcie-qcom-common.c
> drivers/pci/controller/dwc/pcie-qcom-cmn.h | 14 ++++
> drivers/pci/controller/dwc/pcie-qcom-ep.c | 39 ++---------
> drivers/pci/controller/dwc/pcie-qcom.c | 67 +++---------------
> 6 files changed, 113 insertions(+), 94 deletions(-)
> create mode 100644 drivers/pci/controller/dwc/pcie-qcom-cmn.c
> create mode 100644 drivers/pci/controller/dwc/pcie-qcom-cmn.h
>
> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> index 8afacc90c63b..41d2746edc5f 100644
> --- a/drivers/pci/controller/dwc/Kconfig
> +++ b/drivers/pci/controller/dwc/Kconfig
> @@ -265,12 +265,16 @@ config PCIE_DW_PLAT_EP
> order to enable device-specific features PCI_DW_PLAT_EP must be
> selected.
>
> +config PCIE_QCOM_CMN
I'd prefer, 'PCIE_QCOM_COMMON'.
> + bool
> +
> config PCIE_QCOM
> bool "Qualcomm PCIe controller (host mode)"
> depends on OF && (ARCH_QCOM || COMPILE_TEST)
> depends on PCI_MSI
> select PCIE_DW_HOST
> select CRC8
> + select PCIE_QCOM_CMN
> help
> Say Y here to enable PCIe controller support on Qualcomm SoCs. The
> PCIe controller uses the DesignWare core plus Qualcomm-specific
> @@ -281,6 +285,7 @@ config PCIE_QCOM_EP
> depends on OF && (ARCH_QCOM || COMPILE_TEST)
> depends on PCI_ENDPOINT
> select PCIE_DW_EP
> + select PCIE_QCOM_CMN
> help
> Say Y here to enable support for the PCIe controllers on Qualcomm SoCs
> to work in endpoint mode. The PCIe controller uses the DesignWare core
> diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
> index bac103faa523..521572093ebf 100644
> --- a/drivers/pci/controller/dwc/Makefile
> +++ b/drivers/pci/controller/dwc/Makefile
> @@ -14,6 +14,7 @@ obj-$(CONFIG_PCI_LAYERSCAPE) += pci-layerscape.o
> obj-$(CONFIG_PCI_LAYERSCAPE_EP) += pci-layerscape-ep.o
> obj-$(CONFIG_PCIE_QCOM) += pcie-qcom.o
> obj-$(CONFIG_PCIE_QCOM_EP) += pcie-qcom-ep.o
> +obj-$(CONFIG_PCIE_QCOM_CMN) += pcie-qcom-cmn.o
> obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o
> obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o
> obj-$(CONFIG_PCIE_ROCKCHIP_DW_HOST) += pcie-dw-rockchip.o
> diff --git a/drivers/pci/controller/dwc/pcie-qcom-cmn.c b/drivers/pci/controller/dwc/pcie-qcom-cmn.c
> new file mode 100644
> index 000000000000..64fa412ec293
> --- /dev/null
> +++ b/drivers/pci/controller/dwc/pcie-qcom-cmn.c
> @@ -0,0 +1,81 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2014-2015, 2020 The Linux Foundation. All rights reserved.
> + * Copyright 2015, 2021 Linaro Limited.
Copyright (c)
> + * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
> + *
> + */
> +
> +#include <linux/debugfs.h>
Why do you need this header in this patch?
> +#include <linux/pci.h>
> +#include <linux/interconnect.h>
> +
> +#include "../../pci.h"
> +#include "pcie-designware.h"
> +#include "pcie-qcom-cmn.h"
> +
> +#define QCOM_PCIE_LINK_SPEED_TO_BW(speed) \
> + Mbps_to_icc(PCIE_SPEED2MBS_ENC(pcie_link_speed[speed]))
> +
> +int qcom_pcie_cmn_icc_get_resource(struct dw_pcie *pci, struct icc_path *icc_mem)
qcom_pcie_common_icc_get?
> +{
> + if (IS_ERR(pci))
> + return PTR_ERR(pci);
Why this check is needed?
> +
> + icc_mem = devm_of_icc_get(pci->dev, "pcie-mem");
> + if (IS_ERR(icc_mem))
> + return PTR_ERR(icc_mem);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(qcom_pcie_cmn_icc_get_resource);
> +
> +int qcom_pcie_cmn_icc_init(struct dw_pcie *pci, struct icc_path *icc_mem)
qcom_pcie_common_icc_init?
> +{
> + int ret;
> +
> + if (IS_ERR(pci))
> + return PTR_ERR(pci);
> +
Again, why this is needed?
> + if (IS_ERR(icc_mem))
> + return PTR_ERR(icc_mem);
> +
If 'devm_of_icc_get' has failed previously we wouldn't reach here. And also,
there is no need to check for NULL since the ICC core already does that.
> + /*
> + * Some Qualcomm platforms require interconnect bandwidth constraints
> + * to be set before enabling interconnect clocks.
> + *
> + * Set an initial peak bandwidth corresponding to single-lane Gen 1
> + * for the pcie-mem path.
> + */
> + ret = icc_set_bw(icc_mem, 0, QCOM_PCIE_LINK_SPEED_TO_BW(1));
> + if (ret) {
> + dev_err(pci->dev, "failed to set interconnect bandwidth: %d\n",
> + ret);
> + return ret;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(qcom_pcie_cmn_icc_init);
> +
> +void qcom_pcie_cmn_icc_update(struct dw_pcie *pci, struct icc_path *icc_mem)
qcom_pcie_common_icc_update?
> +{
> + u32 offset, status;
> + int speed, width;
> + int ret;
> +
> + if (!icc_mem)
> + return;
> +
> + offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> + status = readw(pci->dbi_base + offset + PCI_EXP_LNKSTA);
> +
You can keep the link check here since that should work for both RC and EP.
> + speed = FIELD_GET(PCI_EXP_LNKSTA_CLS, status);
> + width = FIELD_GET(PCI_EXP_LNKSTA_NLW, status);
> +
> + ret = icc_set_bw(icc_mem, 0, width * QCOM_PCIE_LINK_SPEED_TO_BW(speed));
> + if (ret)
> + dev_err(pci->dev, "failed to set interconnect bandwidth: %d\n",
> + ret);
> +}
> +EXPORT_SYMBOL_GPL(qcom_pcie_cmn_icc_update);
> diff --git a/drivers/pci/controller/dwc/pcie-qcom-cmn.h b/drivers/pci/controller/dwc/pcie-qcom-cmn.h
> new file mode 100644
> index 000000000000..845eda23ae59
> --- /dev/null
> +++ b/drivers/pci/controller/dwc/pcie-qcom-cmn.h
> @@ -0,0 +1,14 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2014-2015, 2020 The Linux Foundation. All rights reserved.
> + * Copyright 2015, 2021 Linaro Limited.
> + * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
> + */
> +
> +#include <linux/pci.h>
> +#include "../../pci.h"
> +#include "pcie-designware.h"
Again, headers should be included only if it is used in the same file itself.
- Mani
--
மணிவண்ணன் சதாசிவம்
On Wed, Mar 20, 2024 at 12:14:46AM -0700, Shashank Babu Chinta Venkata wrote:
Here, you are referring to 16 GT/s as the Gen4 datarate. So please mention that
explicitly.
> GEN3_RELATED_OFFSET is being used as shadow register for generation4 and
What is 'GEN3_RELATED_OFFSET' register? Where it is defined? More info on this
register would be helpful.
> generation5 data rates based on rate select mask settings on this register.
Please reword this sentence to make it more readable.
> Select relevant mask and equalization settings for generation4 operation.
>
> Signed-off-by: Shashank Babu Chinta Venkata <[email protected]>
> ---
> drivers/pci/controller/dwc/pcie-designware.h | 15 ++++++++
> drivers/pci/controller/dwc/pcie-qcom-cmn.c | 36 ++++++++++++++++++++
> drivers/pci/controller/dwc/pcie-qcom-cmn.h | 5 +++
> drivers/pci/controller/dwc/pcie-qcom-ep.c | 3 ++
> drivers/pci/controller/dwc/pcie-qcom.c | 3 ++
> 5 files changed, 62 insertions(+)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 26dae4837462..064744bfe35a 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -122,6 +122,21 @@
> #define GEN3_RELATED_OFF_RATE_SHADOW_SEL_SHIFT 24
> #define GEN3_RELATED_OFF_RATE_SHADOW_SEL_MASK GENMASK(25, 24)
>
> +#define GEN3_EQ_CONTROL_OFF 0x8a8
> +#define GEN3_EQ_CONTROL_OFF_FB_MODE_MASK GENMASK(3, 0)
> +#define GEN3_EQ_CONTROL_OFF_PHASE23_EXIT_MODE BIT(4)
> +#define GEN3_EQ_CONTROL_OFF_PSET_REQ_VEC_MASK GENMASK(23, 8)
> +#define GEN3_EQ_CONTROL_OFF_FOM_INC_INITIAL_EVAL BIT(24)
> +
> +#define GEN3_EQ_FB_MODE_DIR_CHANGE_OFF 0x8ac
> +#define GEN3_EQ_FMDC_T_MIN_PHASE23_MASK GENMASK(4, 0)
> +#define GEN3_EQ_FMDC_N_EVALS_MASK GENMASK(9, 5)
> +#define GEN3_EQ_FMDC_MAX_PRE_CUSROR_DELTA_MASK GENMASK(13, 10)
> +#define GEN3_EQ_FMDC_MAX_POST_CUSROR_DELTA_MASK GENMASK(17, 14)
> +#define GEN3_EQ_FMDC_N_EVALS_SHIFT 5
> +#define GEN3_EQ_FMDC_MAX_PRE_CUSROR_DELTA_SHIFT 10
> +#define GEN3_EQ_FMDC_MAX_POST_CUSROR_DELTA_SHIFT 14
> +
You are adding the definitions in designware header, but funtion definitions in
Qcom driver. Does this mean, this configuration is specific to Qcom and not
applicable to other DWC drivers?
> #define PCIE_PORT_MULTI_LANE_CTRL 0x8C0
> #define PORT_MLTI_UPCFG_SUPPORT BIT(7)
>
> diff --git a/drivers/pci/controller/dwc/pcie-qcom-cmn.c b/drivers/pci/controller/dwc/pcie-qcom-cmn.c
> index 64fa412ec293..208a55e8e9a1 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom-cmn.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom-cmn.c
> @@ -17,6 +17,42 @@
> #define QCOM_PCIE_LINK_SPEED_TO_BW(speed) \
> Mbps_to_icc(PCIE_SPEED2MBS_ENC(pcie_link_speed[speed]))
>
> +void qcom_pcie_cmn_set_16gt_eq_settings(struct dw_pcie *pci)
> +{
> + u32 reg;
> +
> + /*
> + * GEN3_RELATED_OFF is repurposed to be used with GEN4(16GT/s) rate
> + * as well based on RATE_SHADOW_SEL_MASK settings on this register.
> + */
> + reg = dw_pcie_readl_dbi(pci, GEN3_RELATED_OFF);
> + reg &= ~GEN3_RELATED_OFF_GEN3_ZRXDC_NONCOMPL;
> + reg &= ~GEN3_RELATED_OFF_RATE_SHADOW_SEL_MASK;
> + reg |= (0x1 << GEN3_RELATED_OFF_RATE_SHADOW_SEL_SHIFT);
Please use FIELD_* macros where applicable.
> + dw_pcie_writel_dbi(pci, GEN3_RELATED_OFF, reg);
> +
> + reg = dw_pcie_readl_dbi(pci, GEN3_EQ_FB_MODE_DIR_CHANGE_OFF);
> + reg &= ~GEN3_EQ_FMDC_T_MIN_PHASE23_MASK;
> + reg &= ~GEN3_EQ_FMDC_N_EVALS_MASK;
> + reg |= (GEN3_EQ_FMDC_N_EVALS_16GT_VAL <<
> + GEN3_EQ_FMDC_N_EVALS_SHIFT);
> + reg &= ~GEN3_EQ_FMDC_MAX_PRE_CUSROR_DELTA_MASK;
> + reg |= (GEN3_EQ_FMDC_MAX_PRE_CUSROR_DELTA_16GT_VAL <<
> + GEN3_EQ_FMDC_MAX_PRE_CUSROR_DELTA_SHIFT);
> + reg &= ~GEN3_EQ_FMDC_MAX_POST_CUSROR_DELTA_MASK;
> + reg |= (GEN3_EQ_FMDC_MAX_POST_CUSROR_DELTA_16GT_VAL <<
> + GEN3_EQ_FMDC_MAX_POST_CUSROR_DELTA_SHIFT);
> + dw_pcie_writel_dbi(pci, GEN3_EQ_FB_MODE_DIR_CHANGE_OFF, reg);
> +
> + reg = dw_pcie_readl_dbi(pci, GEN3_EQ_CONTROL_OFF);
> + reg &= ~GEN3_EQ_CONTROL_OFF_FB_MODE_MASK;
> + reg &= ~GEN3_EQ_CONTROL_OFF_PHASE23_EXIT_MODE;
> + reg &= ~GEN3_EQ_CONTROL_OFF_FOM_INC_INITIAL_EVAL;
> + reg &= ~GEN3_EQ_CONTROL_OFF_PSET_REQ_VEC_MASK;
> + dw_pcie_writel_dbi(pci, GEN3_EQ_CONTROL_OFF, reg);
> +}
> +EXPORT_SYMBOL_GPL(qcom_pcie_cmn_set_16gt_eq_settings);
> +
> int qcom_pcie_cmn_icc_get_resource(struct dw_pcie *pci, struct icc_path *icc_mem)
> {
> if (IS_ERR(pci))
> diff --git a/drivers/pci/controller/dwc/pcie-qcom-cmn.h b/drivers/pci/controller/dwc/pcie-qcom-cmn.h
> index 845eda23ae59..97302e8fafa8 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom-cmn.h
> +++ b/drivers/pci/controller/dwc/pcie-qcom-cmn.h
> @@ -9,6 +9,11 @@
> #include "../../pci.h"
> #include "pcie-designware.h"
>
> +#define GEN3_EQ_FMDC_MAX_PRE_CUSROR_DELTA_16GT_VAL 0x5
> +#define GEN3_EQ_FMDC_MAX_POST_CUSROR_DELTA_16GT_VAL 0x5
> +#define GEN3_EQ_FMDC_N_EVALS_16GT_VAL 0xD
> +
So these settings are Qcom specific? I'd expect this to be documented in commit
message.
> int qcom_pcie_cmn_icc_get_resource(struct dw_pcie *pci, struct icc_path *icc_mem);
> int qcom_pcie_cmn_icc_init(struct dw_pcie *pci, struct icc_path *icc_mem);
> void qcom_pcie_cmn_icc_update(struct dw_pcie *pci, struct icc_path *icc_mem);
> +void qcom_pcie_cmn_set_16gt_eq_settings(struct dw_pcie *pci);
> diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> index ce6343426de8..b6bcab21bb9f 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> @@ -438,6 +438,9 @@ static int qcom_pcie_perst_deassert(struct dw_pcie *pci)
> goto err_disable_resources;
> }
>
> + if (pcie_link_speed[pci->link_gen] == PCIE_SPEED_16_0GT)
> + qcom_pcie_cmn_set_16gt_eq_settings(pci);
So this relies on the optional 'max-link-speed' DT property, but not mentioned
in the commit message :/
- Mani
> +
> /*
> * The physical address of the MMIO region which is exposed as the BAR
> * should be written to MHI BASE registers.
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 57a08294c561..b0a22a000fa3 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -263,6 +263,9 @@ static int qcom_pcie_start_link(struct dw_pcie *pci)
> {
> struct qcom_pcie *pcie = to_qcom_pcie(pci);
>
> + if (pcie_link_speed[pci->link_gen] == PCIE_SPEED_16_0GT)
> + qcom_pcie_cmn_set_16gt_eq_settings(pci);
> +
> /* Enable Link Training state machine */
> if (pcie->cfg->ops->ltssm_enable)
> pcie->cfg->ops->ltssm_enable(pcie);
> --
> 2.43.2
>
--
மணிவண்ணன் சதாசிவம்
On Wed, Mar 20, 2024 at 12:14:47AM -0700, Shashank Babu Chinta Venkata wrote:
> Add rx margining settings for gen4 operation.
>
What is 'rx margining'? As mentioned in the previous patch, use 16 GT/s.
> Signed-off-by: Shashank Babu Chinta Venkata <[email protected]>
> ---
> drivers/pci/controller/dwc/pcie-designware.h | 23 +++++++++++++
> drivers/pci/controller/dwc/pcie-qcom-cmn.c | 35 ++++++++++++++++++++
> drivers/pci/controller/dwc/pcie-qcom-cmn.h | 11 +++++-
> drivers/pci/controller/dwc/pcie-qcom-ep.c | 4 ++-
> drivers/pci/controller/dwc/pcie-qcom.c | 4 ++-
> 5 files changed, 74 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 064744bfe35a..ce1c5f9c406a 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -206,6 +206,29 @@
>
> #define PCIE_PL_CHK_REG_ERR_ADDR 0xB28
>
> +/*
> + * GEN4 lane margining register definitions
> + */
> +#define GEN4_LANE_MARGINING_1_OFF 0xb80
> +#define MARGINING_MAX_VOLTAGE_OFFSET_MASK GENMASK(29, 24)
> +#define MARGINING_NUM_VOLTAGE_STEPS_MASK GENMASK(22, 16)
> +#define MARGINING_MAX_TIMING_OFFSET_MASK GENMASK(13, 8)
> +#define MARGINING_NUM_TIMING_STEPS_MASK GENMASK(5, 0)
> +#define MARGINING_MAX_VOLTAGE_OFFSET_SHIFT 24
> +#define MARGINING_NUM_VOLTAGE_STEPS_SHIFT 16
> +#define MARGINING_MAX_TIMING_OFFSET_SHIFT 8
> +
> +#define GEN4_LANE_MARGINING_2_OFF 0xb84
> +#define MARGINING_IND_ERROR_SAMPLER BIT(28)
> +#define MARGINING_SAMPLE_REPORTING_METHOD BIT(27)
> +#define MARGINING_IND_LEFT_RIGHT_TIMING BIT(26)
> +#define MARGINING_IND_UP_DOWN_VOLTAGE BIT(25)
> +#define MARGINING_VOLTAGE_SUPPORTED BIT(24)
> +#define MARGINING_MAXLANES_MASK GENMASK(20, 16)
> +#define MARGINING_SAMPLE_RATE_TIMING_MASK GENMASK(13, 8)
> +#define MARGINING_SAMPLE_RATE_VOLTAGE_MASK GENMASK(5, 0)
> +#define MARGINING_MAXLANES_SHIFT 16
> +#define MARGINING_SAMPLE_RATE_TIMING_SHIFT 8
Add a newline
> /*
> * iATU Unroll-specific register definitions
> * From 4.80 core version the address translation will be made by unroll
> diff --git a/drivers/pci/controller/dwc/pcie-qcom-cmn.c b/drivers/pci/controller/dwc/pcie-qcom-cmn.c
> index 208a55e8e9a1..bf6b27ee8327 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom-cmn.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom-cmn.c
> @@ -53,6 +53,41 @@ void qcom_pcie_cmn_set_16gt_eq_settings(struct dw_pcie *pci)
> }
> EXPORT_SYMBOL_GPL(qcom_pcie_cmn_set_16gt_eq_settings);
>
> +void qcom_pcie_cmn_set_16gt_rx_margining_settings(struct dw_pcie *pci)
> +{
> + u32 reg;
> +
> + reg = dw_pcie_readl_dbi(pci, GEN4_LANE_MARGINING_1_OFF);
> + reg &= ~MARGINING_MAX_VOLTAGE_OFFSET_MASK;
> + reg |= (MARGINING_MAX_VOLTAGE_OFFSET_VAL <<
> + MARGINING_MAX_VOLTAGE_OFFSET_SHIFT);
Same comment as previous patch to use FIELD_* macros.
- Mani
--
மணிவண்ணன் சதாசிவம்