Re: [PATCH v7 07/13] PCI: tegra194: Set LTR message request before PCIe link up

[Manikanta Maddireddy <mmaddireddy@nvidia.com>]

On 05/03/26 3:48 pm, Manivannan Sadhasivam wrote:
> On Tue, Mar 03, 2026 at 12:24:42PM +0530, Manikanta Maddireddy wrote:
>> From: Vidya Sagar <vidyas@nvidia.com>
>>
>> LTR message should be sent as soon as the Root Port enables LTR in the
>> Endpoint. Set snoop & no snoop LTR timing and LTR message request before
>> PCIe links up. This ensures that LTR message is sent upstream as soon as
>> LTR is enabled.
>>
> 
> 
> 
>> Fixes: c57247f940e8 ("PCI: tegra: Add support for PCIe endpoint mode in Tegra194")
>> Reviewed-by: Jon Hunter <jonathanh@nvidia.com>
>> Tested-by: Jon Hunter <jonathanh@nvidia.com>
>> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
>> Signed-off-by: Manikanta Maddireddy <mmaddireddy@nvidia.com>
>> ---
>> Changes V6 -> V7: Retain FIELD_PREP() usage
>> Changes V1 -> V6: None
>>
>>   drivers/pci/controller/dwc/pcie-tegra194.c | 18 +++++++++---------
>>   1 file changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
>> index 2da3478f0b5f..b50229df890e 100644
>> --- a/drivers/pci/controller/dwc/pcie-tegra194.c
>> +++ b/drivers/pci/controller/dwc/pcie-tegra194.c
>> @@ -485,15 +485,6 @@ static irqreturn_t tegra_pcie_ep_irq_thread(int irq, void *arg)
>>   	if (val & PCI_COMMAND_MASTER) {
>>   		ktime_t timeout;
>>   
>> -		/* 110us for both snoop and no-snoop */
>> -		val = FIELD_PREP(PCI_LTR_VALUE_MASK, 110) |
>> -		      FIELD_PREP(PCI_LTR_SCALE_MASK, 2) |
>> -		      LTR_MSG_REQ |
>> -		      FIELD_PREP(PCI_LTR_NOSNOOP_VALUE, 110) |
>> -		      FIELD_PREP(PCI_LTR_NOSNOOP_SCALE, 2) |
>> -		      LTR_NOSNOOP_MSG_REQ;
>> -		appl_writel(pcie, val, APPL_LTR_MSG_1);
>> -
>>   		/* Send LTR upstream */
>>   		val = appl_readl(pcie, APPL_LTR_MSG_2);
>>   		val |= APPL_LTR_MSG_2_LTR_MSG_REQ_STATE;
>> @@ -1803,6 +1794,15 @@ static void pex_ep_event_pex_rst_deassert(struct tegra_pcie_dw *pcie)
>>   	val |= APPL_INTR_EN_L1_0_0_RDLH_LINK_UP_INT_EN;
>>   	appl_writel(pcie, val, APPL_INTR_EN_L1_0_0);
>>   
>> +	/* 110us for both snoop and no-snoop */
>> +	val = FIELD_PREP(PCI_LTR_VALUE_MASK, 110) |
>> +	      FIELD_PREP(PCI_LTR_SCALE_MASK, 2) |
>> +	      LTR_MSG_REQ |
>> +	      FIELD_PREP(PCI_LTR_NOSNOOP_VALUE, 110) |
>> +	      FIELD_PREP(PCI_LTR_NOSNOOP_SCALE, 2) |
>> +	      LTR_NOSNOOP_MSG_REQ;
> 
> As per the spec, the device is not permitted to request Snoop/No-Snoop latencies
> greater that the Max Snoop/No-Snoop latencies set by the host depending on the
> platform requirement.
> 
> But here the driver is just using a hardcoded value without reading Max values.
> It may be assuming that the host is always going to be another NVidia platform,
> so it sends out fixed LTR latencies, but that's not going to be true always.
> 
> Also, the host can update the Max latencies at any point of time during runtime.
> 
> - Mani
> 
Agree, but this patch is only addressing case where max latencies are 
not yet programmed by the host.  Without this programming Endpoint sends 
0 latencies to the host. Once host sets max latencies in the config 
space, HW compares the above latencies and the max latencies configured 
by host and sends appropriate values to the host.

- Manikanta

-- 
nvpublic