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

[Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>]

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>
>>