| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <9ea68c8b-59ed-48a5-9289-861ae6077fcf@quicinc.com>
Date: Mon, 16 Jun 2025 18:47:10 +0800
From: Ziyue Zhang <quic_ziyuzhan@...cinc.com>
To: Konrad Dybcio <konrad.dybcio@....qualcomm.com>, <andersson@...nel.org>,
<konradybcio@...nel.org>, <robh@...nel.org>, <krzk+dt@...nel.org>,
<conor+dt@...nel.org>, <jingoohan1@...il.com>, <mani@...nel.org>,
<lpieralisi@...nel.org>, <kwilczynski@...nel.org>,
<bhelgaas@...gle.com>, <johan+linaro@...nel.org>, <vkoul@...nel.org>,
<kishon@...nel.org>, <dmitry.baryshkov@...aro.org>,
<manivannan.sadhasivam@...aro.org>, <neil.armstrong@...aro.org>,
<abel.vesa@...aro.org>, <kw@...ux.com>
CC: <linux-arm-msm@...r.kernel.org>, <devicetree@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, <linux-pci@...r.kernel.org>,
<linux-phy@...ts.infradead.org>, <qiang.yu@....qualcomm.com>,
<quic_krichai@...cinc.com>, <quic_vbadigan@...cinc.com>
Subject: Re: [PATCH v2 1/2] PCI: qcom: Add equalization settings for 8.0 GT/s
On 6/11/2025 11:31 PM, Konrad Dybcio wrote:
> On 6/11/25 12:03 PM, 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).
>>
>> 8.0 GT/s and 16.0GT/s 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 8.0 GT/s and
>> 16.0 GT/s.
>>
>> Co-developed-by: Qiang Yu <qiang.yu@....qualcomm.com>
>> Signed-off-by: Qiang Yu <qiang.yu@....qualcomm.com>
>> Signed-off-by: Ziyue Zhang <quic_ziyuzhan@...cinc.com>
>> ---
> [...]
>
>> -void qcom_pcie_common_set_16gt_equalization(struct dw_pcie *pci)
>> +void qcom_pcie_common_set_equalization(struct dw_pcie *pci)
>> {
>> u32 reg;
>> + u16 speed, max_speed = PCIE_SPEED_16_0GT;
>> + struct device *dev = pci->dev;
>>
>> /*
>> * GEN3_RELATED_OFF register is repurposed to apply equalization
>> @@ -18,33 +20,43 @@ void qcom_pcie_common_set_16gt_equalization(struct dw_pcie *pci)
>> * GEN3_EQ_*. The RATE_SHADOW_SEL bit field of GEN3_RELATED_OFF
>> * determines the data rate for which these equalization settings are
>> * applied.
>> + *
>> + * TODO:
>> + * EQ settings need to be added for 32.0 T/s in future
>> */
>> - 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);
>> + if (pcie_link_speed[pci->max_link_speed] < PCIE_SPEED_32_0GT)
>> + max_speed = pcie_link_speed[pci->max_link_speed];
>> + else
>> + dev_warn(dev, "The target supports 32.0 GT/s, but the EQ setting for 32.0 GT/s is not configured.\n");
>>
>> - 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);
>> + for (speed = PCIE_SPEED_8_0GT; speed <= max_speed; ++speed) {
>> + 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,
>> + speed - PCIE_SPEED_8_0GT);
>> + dw_pcie_writel_dbi(pci, GEN3_RELATED_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_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);
>> + }
> this function could receive `speed` as a parameter instead, so that
> it's easier to parse
>
> Konrad
Hi Konrad,
On the current platform, the register write configurations for both
8.0 GT/s and 16.0 GT/s are identical, so we believe it's unnecessary to
pass ‘speed’ as a parameter at this stage.
However, I agree that if future platforms or speed modes introduce
configuration differences, it would make sense to revisit this and
consider adding speed as a parameter for better flexibility.
BRs
Ziyue
Powered by blists - more mailing lists