From: neil.armstrong@linaro.org
To: Ziyue Zhang <quic_ziyuzhan@quicinc.com>,
lpieralisi@kernel.org, kwilczynski@kernel.org,
manivannan.sadhasivam@linaro.org, robh@kernel.org,
bhelgaas@google.com, krzk+dt@kernel.org, abel.vesa@linaro.org,
kw@linux.com, conor+dt@kernel.org, vkoul@kernel.org,
kishon@kernel.org, andersson@kernel.org, konradybcio@kernel.org
Cc: linux-arm-msm@vger.kernel.org, linux-pci@vger.kernel.org,
linux-phy@lists.infradead.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, quic_qianyu@quicinc.com,
Qiang Yu <qiang.yu@oss.qualcomm.com>
Subject: Re: [PATCH v1 1/2] PCI: qcom: Add equalization settings for 8.0 GT/s
Date: Wed, 4 Jun 2025 20:16:36 +0200 [thread overview]
Message-ID: <c09fda8d-a023-47f7-a89b-80a368d06193@linaro.org> (raw)
In-Reply-To: <20250604091946.1890602-2-quic_ziyuzhan@quicinc.com>
Hi,
On 04/06/2025 11:19, Ziyue Zhang wrote:
> Adding lane equalization setting for 8.0 GT/s to enhance link stability
> and fix AER correctable errors reported on some platforms (eg. SA8775P).
>
> GEN3 and GEN4 require the same equalization setting. This setting is
> programmed into a group of shadow registers, which can be switched to
> configure equalization for different GEN speeds by writing 00b, 01b
> to `RATE_SHADOW_SEL`.
>
> Hence program equalization registers in a loop using link speed as index,
> so that equalization setting can be programmed for both GEN3 and GEN4.
>
> Co-developed-by: Qiang Yu <qiang.yu@oss.qualcomm.com>
> Signed-off-by: Qiang Yu <qiang.yu@oss.qualcomm.com>
> Signed-off-by: Ziyue Zhang <quic_ziyuzhan@quicinc.com>
> ---
> drivers/pci/controller/dwc/pcie-designware.h | 1 -
> drivers/pci/controller/dwc/pcie-qcom-common.c | 55 ++++++++++---------
> drivers/pci/controller/dwc/pcie-qcom-common.h | 2 +-
> drivers/pci/controller/dwc/pcie-qcom.c | 3 +-
> 4 files changed, 32 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index ce9e18554e42..388306991467 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -127,7 +127,6 @@
> #define GEN3_RELATED_OFF_GEN3_EQ_DISABLE BIT(16)
> #define GEN3_RELATED_OFF_RATE_SHADOW_SEL_SHIFT 24
> #define GEN3_RELATED_OFF_RATE_SHADOW_SEL_MASK GENMASK(25, 24)
> -#define GEN3_RELATED_OFF_RATE_SHADOW_SEL_16_0GT 0x1
>
> #define GEN3_EQ_CONTROL_OFF 0x8A8
> #define GEN3_EQ_CONTROL_OFF_FB_MODE GENMASK(3, 0)
> diff --git a/drivers/pci/controller/dwc/pcie-qcom-common.c b/drivers/pci/controller/dwc/pcie-qcom-common.c
> index 3aad19b56da8..48040f20b29c 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom-common.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom-common.c
> @@ -8,43 +8,46 @@
> #include "pcie-designware.h"
> #include "pcie-qcom-common.h"
>
> -void qcom_pcie_common_set_16gt_equalization(struct dw_pcie *pci)
> +void qcom_pcie_common_set_equalization(struct dw_pcie *pci)
> {
> u32 reg;
> + u16 i;
>
> /*
> * GEN3_RELATED_OFF register is repurposed to apply equalization
> - * settings at various data transmission rates through registers namely
> - * GEN3_EQ_*. The RATE_SHADOW_SEL bit field of GEN3_RELATED_OFF
> + @@ -19,32 +21,34 @@ void qcom_pcie_common_set_16gt_equalization(struct dw_pcie *pci)
> * determines the data rate for which these equalization settings are
> * applied.
> */
> - 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 |= FIELD_PREP(GEN3_RELATED_OFF_RATE_SHADOW_SEL_MASK,
> - GEN3_RELATED_OFF_RATE_SHADOW_SEL_16_0GT);
> - dw_pcie_writel_dbi(pci, GEN3_RELATED_OFF, reg);
> + for (i = PCIE_SPEED_8_0GT; i <= (pcie_link_speed[pci->max_link_speed] < PCIE_SPEED_32_0GT ?
> + pcie_link_speed[pci->max_link_speed] : PCIE_SPEED_16_0GT); i++) {
This is pretty hard to read, please simplify like:
u16 speed, max_speed = PCIE_SPEED_16_0GT;
if (pcie_link_speed[pci->max_link_speed] < PCIE_SPEED_32_0GT)
max_speed = pcie_link_speed[pci->max_link_speed];
for (speed = PCIE_SPEED_8_0GT; speed < max_speedl; ++speed) {
blah;
}
> + 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 |= FIELD_PREP(GEN3_RELATED_OFF_RATE_SHADOW_SEL_MASK,
> + i - PCIE_SPEED_8_0GT);
> + 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 |
> - GEN3_EQ_FMDC_N_EVALS |
> - GEN3_EQ_FMDC_MAX_PRE_CUSROR_DELTA |
> - GEN3_EQ_FMDC_MAX_POST_CUSROR_DELTA);
> - reg |= FIELD_PREP(GEN3_EQ_FMDC_T_MIN_PHASE23, 0x1) |
> - FIELD_PREP(GEN3_EQ_FMDC_N_EVALS, 0xd) |
> - FIELD_PREP(GEN3_EQ_FMDC_MAX_PRE_CUSROR_DELTA, 0x5) |
> - FIELD_PREP(GEN3_EQ_FMDC_MAX_POST_CUSROR_DELTA, 0x5);
> - dw_pcie_writel_dbi(pci, GEN3_EQ_FB_MODE_DIR_CHANGE_OFF, reg);
> + reg = dw_pcie_readl_dbi(pci, GEN3_EQ_FB_MODE_DIR_CHANGE_OFF);
> + reg &= ~(GEN3_EQ_FMDC_T_MIN_PHASE23 |
> + GEN3_EQ_FMDC_N_EVALS |
> + GEN3_EQ_FMDC_MAX_PRE_CUSROR_DELTA |
> + GEN3_EQ_FMDC_MAX_POST_CUSROR_DELTA);
> + reg |= FIELD_PREP(GEN3_EQ_FMDC_T_MIN_PHASE23, 0x1) |
> + FIELD_PREP(GEN3_EQ_FMDC_N_EVALS, 0xd) |
> + FIELD_PREP(GEN3_EQ_FMDC_MAX_PRE_CUSROR_DELTA, 0x5) |
> + FIELD_PREP(GEN3_EQ_FMDC_MAX_POST_CUSROR_DELTA, 0x5);
> + 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 |
> - GEN3_EQ_CONTROL_OFF_PHASE23_EXIT_MODE |
> - GEN3_EQ_CONTROL_OFF_FOM_INC_INITIAL_EVAL |
> - GEN3_EQ_CONTROL_OFF_PSET_REQ_VEC);
> - dw_pcie_writel_dbi(pci, GEN3_EQ_CONTROL_OFF, reg);
> + reg = dw_pcie_readl_dbi(pci, GEN3_EQ_CONTROL_OFF);
> + reg &= ~(GEN3_EQ_CONTROL_OFF_FB_MODE |
> + GEN3_EQ_CONTROL_OFF_PHASE23_EXIT_MODE |
> + GEN3_EQ_CONTROL_OFF_FOM_INC_INITIAL_EVAL |
> + GEN3_EQ_CONTROL_OFF_PSET_REQ_VEC);
> + dw_pcie_writel_dbi(pci, GEN3_EQ_CONTROL_OFF, reg);
> + }
> }
> -EXPORT_SYMBOL_GPL(qcom_pcie_common_set_16gt_equalization);
> +EXPORT_SYMBOL_GPL(qcom_pcie_common_set_equalization);
>
> void qcom_pcie_common_set_16gt_lane_margining(struct dw_pcie *pci)
> {
> diff --git a/drivers/pci/controller/dwc/pcie-qcom-common.h b/drivers/pci/controller/dwc/pcie-qcom-common.h
> index 7d88d29e4766..7f5ca2fd9a72 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom-common.h
> +++ b/drivers/pci/controller/dwc/pcie-qcom-common.h
> @@ -8,7 +8,7 @@
>
> struct dw_pcie;
>
> -void qcom_pcie_common_set_16gt_equalization(struct dw_pcie *pci);
> +void qcom_pcie_common_set_equalization(struct dw_pcie *pci);
> void qcom_pcie_common_set_16gt_lane_margining(struct dw_pcie *pci);
>
> #endif
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index c789e3f85655..51eac2dc6222 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -298,8 +298,9 @@ static int qcom_pcie_start_link(struct dw_pcie *pci)
> {
> struct qcom_pcie *pcie = to_qcom_pcie(pci);
>
> + qcom_pcie_common_set_equalization(pci);
> +
> if (pcie_link_speed[pci->max_link_speed] == PCIE_SPEED_16_0GT) {
> - qcom_pcie_common_set_16gt_equalization(pci);
> qcom_pcie_common_set_16gt_lane_margining(pci);
> }
>
Thanks,
Neil
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
next prev parent reply other threads:[~2025-06-04 18:16 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-06-04 9:19 [PATCH v1 0/2] Add Equalization Settings for 8.0 GT/s and Add PCIe Lane Equalization Preset Properties for 8.0 GT/s and 16.0 GT/s Ziyue Zhang
2025-06-04 9:19 ` [PATCH v1 1/2] PCI: qcom: Add equalization settings for 8.0 GT/s Ziyue Zhang
2025-06-04 16:16 ` kernel test robot
2025-06-04 18:16 ` neil.armstrong [this message]
2025-06-04 9:19 ` [PATCH v1 2/2] arm64: dts: qcom: sa8775p: Add PCIe lane equalization preset properties Ziyue Zhang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=c09fda8d-a023-47f7-a89b-80a368d06193@linaro.org \
--to=neil.armstrong@linaro.org \
--cc=abel.vesa@linaro.org \
--cc=andersson@kernel.org \
--cc=bhelgaas@google.com \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=kishon@kernel.org \
--cc=konradybcio@kernel.org \
--cc=krzk+dt@kernel.org \
--cc=kw@linux.com \
--cc=kwilczynski@kernel.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=linux-phy@lists.infradead.org \
--cc=lpieralisi@kernel.org \
--cc=manivannan.sadhasivam@linaro.org \
--cc=qiang.yu@oss.qualcomm.com \
--cc=quic_qianyu@quicinc.com \
--cc=quic_ziyuzhan@quicinc.com \
--cc=robh@kernel.org \
--cc=vkoul@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).