[PATCH] PCI: qcom: Program correct T_POWER_ON value for L1.2 exit timing

[PATCH] PCI: qcom: Program correct T_POWER_ON value for L1.2 exit timing

[Krishna Chaitanya Chundru]

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.

Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.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@oss.qualcomm.com>

Re: [PATCH] PCI: qcom: Program correct T_POWER_ON value for L1.2 exit timing

[Konrad Dybcio]

On 11/4/25 1:12 PM, 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.

Is 80us a meaningful value, or "just happens to work"?

Konrad

Re: [PATCH] PCI: qcom: Program correct T_POWER_ON value for L1.2 exit timing

[Krishna Chaitanya Chundru]

On 11/4/2025 5:59 PM, Konrad Dybcio wrote:
> On 11/4/25 1:12 PM, 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.
> Is 80us a meaningful value, or "just happens to work"?

This value is given by hardware team.

- Krishna Chaitanya.

>
> Konrad
>

Re: [PATCH] PCI: qcom: Program correct T_POWER_ON value for L1.2 exit timing

[Konrad Dybcio]

On 11/4/25 5:38 PM, Krishna Chaitanya Chundru wrote:
> 
> On 11/4/2025 5:59 PM, Konrad Dybcio wrote:
>> On 11/4/25 1:12 PM, 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.
>> Is 80us a meaningful value, or "just happens to work"?
> 
> This value is given by hardware team.

Sorry I asked the wrong question

Is it something that comes from the spec (PCI or DWC), or is it qc
specific?

Konrad

Re: [PATCH] PCI: qcom: Program correct T_POWER_ON value for L1.2 exit timing

[Krishna Chaitanya Chundru]

On 11/4/2025 10:13 PM, Konrad Dybcio wrote:
> On 11/4/25 5:38 PM, Krishna Chaitanya Chundru wrote:
>> On 11/4/2025 5:59 PM, Konrad Dybcio wrote:
>>> On 11/4/25 1:12 PM, 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.
>>> Is 80us a meaningful value, or "just happens to work"?
>> This value is given by hardware team.
> Sorry I asked the wrong question
>
> Is it something that comes from the spec (PCI or DWC), or is it qc
> specific?
T power ON value is part of the PCIe spec, expectation is this value 
needs to be
set in the hardware only, In QC this value is Zero so it requires sw support
to program this correctly before enumeration.

- Krishna Chaitanya.
> Konrad

Re: [PATCH] PCI: qcom: Program correct T_POWER_ON value for L1.2 exit timing

[Bjorn Helgaas]

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@oss.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@oss.qualcomm.com>
> 

Re: [PATCH] PCI: qcom: Program correct T_POWER_ON value for L1.2 exit timing

[Krishna Chaitanya Chundru]

On 11/4/2025 11:26 PM, Bjorn Helgaas wrote:
> 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.
Yes it depends on design.
> 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.

Yes Bjorn these are PCIe stuff only, I can go to Device tree route if we 
have different values for each target, as of now we are using this same 
value in all targets as recommended by our HW team. If there is at least 
one more target or one more vendor who needs to program this we can take 
devicetree property route.

I am ok to go with devicetree way also if you insists. - Krishna Chaitanya.

> Use "μs" or "us" consistently; there's a mix above.
>
>> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.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@oss.qualcomm.com>
>>

Re: [PATCH] PCI: qcom: Program correct T_POWER_ON value for L1.2 exit timing

[Manivannan Sadhasivam]

On Thu, Nov 06, 2025 at 10:30:44AM +0530, Krishna Chaitanya Chundru wrote:
> 
> On 11/4/2025 11:26 PM, Bjorn Helgaas wrote:
> > 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.
> Yes it depends on design.
> > 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.
> 
> Yes Bjorn these are PCIe stuff only, I can go to Device tree route if we
> have different values for each target, as of now we are using this same
> value in all targets as recommended by our HW team. If there is at least one
> more target or one more vendor who needs to program this we can take
> devicetree property route.
> 
> I am ok to go with devicetree way also if you insists. - Krishna Chaitanya.
> 

Since this is a PCI generic value, using devicetree property makes sense to me.

- Mani

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

Re: [PATCH] PCI: qcom: Program correct T_POWER_ON value for L1.2 exit timing

[Krishna Chaitanya Chundru]

On 11/6/2025 11:51 AM, Manivannan Sadhasivam wrote:
> On Thu, Nov 06, 2025 at 10:30:44AM +0530, Krishna Chaitanya Chundru wrote:
>> On 11/4/2025 11:26 PM, Bjorn Helgaas wrote:
>>> 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.
>> Yes it depends on design.
>>> 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.
>> Yes Bjorn these are PCIe stuff only, I can go to Device tree route if we
>> have different values for each target, as of now we are using this same
>> value in all targets as recommended by our HW team. If there is at least one
>> more target or one more vendor who needs to program this we can take
>> devicetree property route.
>>
>> I am ok to go with devicetree way also if you insists. - Krishna Chaitanya.
>>
> Since this is a PCI generic value, using devicetree property makes sense to me.
Raised devicetree change here [PATCH] schemas: pci: Document PCIe T_POWER_ON

<https://lore.kernel.org/all/20251106113951.844312-1-krishna.chundru@oss.qualcomm.com/>- 
Krishna Chaitanya.
> - Mani
>