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@xxxxxxxxxxxxxxxx>
> Signed-off-by: Qiang Yu <qiang.yu@xxxxxxxxxxxxxxxx>
> Signed-off-by: Ziyue Zhang <quic_ziyuzhan@xxxxxxxxxxx>
> ---
[...]
> -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
