* Re: [PATCH v2 1/2] PCI: qcom: Add equalization settings for 8.0 GT/s
2025-06-11 10:03 ` [PATCH v2 1/2] PCI: qcom: Add equalization settings for 8.0 GT/s Ziyue Zhang
@ 2025-06-11 15:31 ` Konrad Dybcio
2025-06-16 10:47 ` Ziyue Zhang
2025-06-13 19:21 ` Bjorn Helgaas
2025-06-24 20:50 ` Manivannan Sadhasivam
2 siblings, 1 reply; 9+ messages in thread
From: Konrad Dybcio @ 2025-06-11 15:31 UTC (permalink / raw)
To: Ziyue Zhang, andersson, konradybcio, robh, krzk+dt, conor+dt,
jingoohan1, mani, lpieralisi, kwilczynski, bhelgaas, johan+linaro,
vkoul, kishon, dmitry.baryshkov, manivannan.sadhasivam,
neil.armstrong, abel.vesa, kw
Cc: linux-arm-msm, devicetree, linux-kernel, linux-pci, linux-phy,
qiang.yu, quic_krichai, quic_vbadigan
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@oss.qualcomm.com>
> Signed-off-by: Qiang Yu <qiang.yu@oss.qualcomm.com>
> Signed-off-by: Ziyue Zhang <quic_ziyuzhan@quicinc.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
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v2 1/2] PCI: qcom: Add equalization settings for 8.0 GT/s
2025-06-11 15:31 ` Konrad Dybcio
@ 2025-06-16 10:47 ` Ziyue Zhang
0 siblings, 0 replies; 9+ messages in thread
From: Ziyue Zhang @ 2025-06-16 10:47 UTC (permalink / raw)
To: Konrad Dybcio, andersson, konradybcio, robh, krzk+dt, conor+dt,
jingoohan1, mani, lpieralisi, kwilczynski, bhelgaas, johan+linaro,
vkoul, kishon, dmitry.baryshkov, manivannan.sadhasivam,
neil.armstrong, abel.vesa, kw
Cc: linux-arm-msm, devicetree, linux-kernel, linux-pci, linux-phy,
qiang.yu, quic_krichai, quic_vbadigan
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@oss.qualcomm.com>
>> Signed-off-by: Qiang Yu <qiang.yu@oss.qualcomm.com>
>> Signed-off-by: Ziyue Zhang <quic_ziyuzhan@quicinc.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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/2] PCI: qcom: Add equalization settings for 8.0 GT/s
2025-06-11 10:03 ` [PATCH v2 1/2] PCI: qcom: Add equalization settings for 8.0 GT/s Ziyue Zhang
2025-06-11 15:31 ` Konrad Dybcio
@ 2025-06-13 19:21 ` Bjorn Helgaas
2025-06-24 20:50 ` Manivannan Sadhasivam
2 siblings, 0 replies; 9+ messages in thread
From: Bjorn Helgaas @ 2025-06-13 19:21 UTC (permalink / raw)
To: Ziyue Zhang
Cc: andersson, konradybcio, robh, krzk+dt, conor+dt, jingoohan1, mani,
lpieralisi, kwilczynski, bhelgaas, johan+linaro, vkoul, kishon,
dmitry.baryshkov, manivannan.sadhasivam, neil.armstrong,
abel.vesa, kw, linux-arm-msm, devicetree, linux-kernel, linux-pci,
linux-phy, qiang.yu, quic_krichai, quic_vbadigan
On Wed, Jun 11, 2025 at 06:03:18PM +0800, 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).
s/Adding/Add/
s/fix AER correctable errors/avoid AER Correctable Errors/
so we know that (a) this avoids the errors (it's not some kind of
recovery), and (b) readers know that "Correctable Error" is something
that can be looked up in a spec.
> 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`.
s|16.0GT/s|16.0 GT/s| to match "8.0 GT/s"
s/different GEN speeds/different speeds/ (PCIe spec rev ("GEN") is not
a one-to-one mapping to speeds)
> @@ -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
Drop this comment since it's not an indication of a problem in this
patch. The list of possible future work is infinite, and you can keep
track of a TODO list internally.
> */
> - 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");
Drop period at end of message.
> - 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);
Is "CUSROR" a typo for "CURSOR", or is "CUSROR" a real thing? (Either
way, it's not something you added, so not a problem with this patch.)
Bjorn
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v2 1/2] PCI: qcom: Add equalization settings for 8.0 GT/s
2025-06-11 10:03 ` [PATCH v2 1/2] PCI: qcom: Add equalization settings for 8.0 GT/s Ziyue Zhang
2025-06-11 15:31 ` Konrad Dybcio
2025-06-13 19:21 ` Bjorn Helgaas
@ 2025-06-24 20:50 ` Manivannan Sadhasivam
2 siblings, 0 replies; 9+ messages in thread
From: Manivannan Sadhasivam @ 2025-06-24 20:50 UTC (permalink / raw)
To: Ziyue Zhang
Cc: andersson, konradybcio, robh, krzk+dt, conor+dt, jingoohan1,
lpieralisi, kwilczynski, bhelgaas, johan+linaro, vkoul, kishon,
dmitry.baryshkov, manivannan.sadhasivam, neil.armstrong,
abel.vesa, kw, linux-arm-msm, devicetree, linux-kernel, linux-pci,
linux-phy, qiang.yu, quic_krichai, quic_vbadigan
On Wed, Jun 11, 2025 at 06:03:18PM +0800, 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@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 | 60 +++++++++++--------
> drivers/pci/controller/dwc/pcie-qcom-common.h | 2 +-
> drivers/pci/controller/dwc/pcie-qcom-ep.c | 6 +-
> drivers/pci/controller/dwc/pcie-qcom.c | 6 +-
> 5 files changed, 43 insertions(+), 32 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..4ff97ec13818 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom-common.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom-common.c
> @@ -8,9 +8,11 @@
> #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 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");
I believe the warning is enough to inform the users/developers that the driver
update is needed. So the TODO above looks redundant.
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 9+ messages in thread