linux-kernel - Re: [PATCH v2 1/2] PCI: qcom: Add equalization settings for 8.0 GT/s

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <t4dwk5buo5yuuz3au5lnt4ol4ow343qjj2jxa3ktxmxoxrkm3x@c4pena57zsn3>
Date: Tue, 24 Jun 2025 14:50:24 -0600
From: Manivannan Sadhasivam <mani@...nel.org>
To: Ziyue Zhang <quic_ziyuzhan@...cinc.com>
Cc: andersson@...nel.org, konradybcio@...nel.org, robh@...nel.org, 
	krzk+dt@...nel.org, conor+dt@...nel.org, jingoohan1@...il.com, 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, 
	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 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@....qualcomm.com>
> Signed-off-by: Qiang Yu <qiang.yu@....qualcomm.com>
> Signed-off-by: Ziyue Zhang <quic_ziyuzhan@...cinc.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

-- 
மணிவண்ணன் சதாசிவம்