linux-kernel - Re: [PATCH] PCI: qcom: Program correct T_POWER

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] [day] [month] [year] [list]

Message-ID: <20251104175657.GA1861670@bhelgaas>
Date: Tue, 4 Nov 2025 11:56:57 -0600
From: Bjorn Helgaas <helgaas@...nel.org>
To: Krishna Chaitanya Chundru <krishna.chundru@....qualcomm.com>
Cc: Manivannan Sadhasivam <mani@...nel.org>,
	Lorenzo Pieralisi <lpieralisi@...nel.org>,
	Krzysztof Wilczyński <kwilczynski@...nel.org>,
	Rob Herring <robh@...nel.org>, Bjorn Helgaas <bhelgaas@...gle.com>,
	linux-pci@...r.kernel.org, linux-arm-msm@...r.kernel.org,
	linux-kernel@...r.kernel.org, mayank.rana@....qualcomm.com,
	quic_vbadigan@...cinc.com
Subject: Re: [PATCH] PCI: qcom: Program correct T_POWER_ON value for L1.2
 exit timing

On Tue, Nov 04, 2025 at 05:42:45PM +0530, Krishna Chaitanya Chundru wrote:
> The T_POWER_ON indicates the time (in μs) that a Port requires the port
> on the opposite side of Link to wait in L1.2.Exit after sampling CLKREQ#
> asserted before actively driving the interface. This value is used by
> the ASPM driver to compute the LTR_L1.2_THRESHOLD.
> 
> Currently, the root port exposes a T_POWER_ON value of zero in the L1SS
> capability registers, leading to incorrect LTR_L1.2_THRESHOLD calculations.
> This can result in improper L1.2 exit behavior and can trigger AER's.
> 
> To address this, program the T_POWER_ON value to 80us (scale = 1,
> value = 8) in the PCI_L1SS_CAP register during host initialization. This
> ensures that ASPM can take the root port's T_POWER_ON value into account
> while calculating the LTR_L1.2_THRESHOLD value.

I think the question is whether the value depends on the circuit
design of a particular platform (and should therefore come from DT),
or whether it depends solely on the qcom device.

PCIe r7.0, sec 5.5.4, says:

  The T_POWER_ON and Common_Mode_Restore_Time fields must be
  programmed to the appropriate values based on the components and AC
  coupling capacitors used in the connection linking the two
  components. The determination of these values is design
  implementation specific.

That suggests to me that maybe there should be devicetree properties
related to these.  Obviously these would not be qcom-specific since
this is standard PCIe stuff.

Use "μs" or "us" consistently; there's a mix above.

> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@....qualcomm.com>
> ---
>  drivers/pci/controller/dwc/pcie-qcom.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index c48a20602d7fa4c50056ccf6502d3b5bf0a8287f..52a3412bd2584c8bf5d281fa6a0ed22141ad1989 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -1252,6 +1252,27 @@ static bool qcom_pcie_link_up(struct dw_pcie *pci)
>  	return val & PCI_EXP_LNKSTA_DLLLA;
>  }
>  
> +static void qcom_pcie_program_t_pwr_on(struct dw_pcie *pci)
> +{
> +	u16 offset;
> +	u32 val;
> +
> +	offset = dw_pcie_find_ext_capability(pci, PCI_EXT_CAP_ID_L1SS);
> +	if (offset) {
> +		dw_pcie_dbi_ro_wr_en(pci);
> +
> +		val = readl(pci->dbi_base + offset + PCI_L1SS_CAP);
> +		/* Program T power ON value to 80us */
> +		val &= ~(PCI_L1SS_CAP_P_PWR_ON_SCALE | PCI_L1SS_CAP_P_PWR_ON_VALUE);
> +		val |= FIELD_PREP(PCI_L1SS_CAP_P_PWR_ON_SCALE, 1);
> +		val |= FIELD_PREP(PCI_L1SS_CAP_P_PWR_ON_VALUE, 8);
> +
> +		writel(val, pci->dbi_base + offset + PCI_L1SS_CAP);
> +
> +		dw_pcie_dbi_ro_wr_dis(pci);
> +	}
> +}
> +
>  static void qcom_pcie_phy_power_off(struct qcom_pcie *pcie)
>  {
>  	struct qcom_pcie_port *port;
> @@ -1302,6 +1323,8 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
>  			goto err_disable_phy;
>  	}
>  
> +	qcom_pcie_program_t_pwr_on(pci);
> +
>  	qcom_ep_reset_deassert(pcie);
>  
>  	if (pcie->cfg->ops->config_sid) {
> 
> ---
> base-commit: c9cfc122f03711a5124b4aafab3211cf4d35a2ac
> change-id: 20251104-t_power_on_fux-70dc68377941
> 
> Best regards,
> -- 
> Krishna Chaitanya Chundru <krishna.chundru@....qualcomm.com>
>