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

[Manivannan Sadhasivam]

On Sun, Mar 15, 2026 at 07:19:47PM +0530, Manikanta Maddireddy wrote:
> 
> 
> 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@xxxxxxxxxx>
> > > 
> > > 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@xxxxxxxxxx>
> > > Tested-by: Jon Hunter <jonathanh@xxxxxxxxxx>
> > > Signed-off-by: Vidya Sagar <vidyas@xxxxxxxxxx>
> > > Signed-off-by: Manikanta Maddireddy <mmaddireddy@xxxxxxxxxx>
> > > ---
> > > 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.
> 

Okay, this should also be clarified in commit message.

- Mani

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