Re: [PATCH V4 1/6] PCI: tegra: Fix ASPM-L1SS advertisement disable code

[Vidya Sagar]

> -----Original Message-----
> From: Thierry Reding <thierry.reding@xxxxxxxxx>
> Sent: Thursday, November 26, 2020 5:03 PM
> To: Vidya Sagar <vidyas@xxxxxxxxxx>
> Cc: lorenzo.pieralisi@xxxxxxx; robh+dt@xxxxxxxxxx; bhelgaas@xxxxxxxxxx;
> Jonathan Hunter <jonathanh@xxxxxxxxxx>; amanharitsh123@xxxxxxxxx;
> dinghao.liu@xxxxxxxxxx; kw@xxxxxxxxx; linux-pci@xxxxxxxxxxxxxxx; linux-
> tegra@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Krishna Thota
> <kthota@xxxxxxxxxx>; Manikanta Maddireddy <mmaddireddy@xxxxxxxxxx>;
> sagar.tv@xxxxxxxxx
> Subject: Re: [PATCH V4 1/6] PCI: tegra: Fix ASPM-L1SS advertisement disable
> code
>
> On Mon, Nov 09, 2020 at 10:49:32PM +0530, Vidya Sagar wrote:
> > If the absence of CLKREQ# signal is indicated by the absence of
> > "supports-clkreq" in the device-tree node, current driver is disabling
> > the advertisement of ASPM-L1 Sub-States *before* the ASPM-L1
> > Sub-States offset is correctly initialized. Since default value of the
> > ASPM-L1SS offset is zero, this is causing the Vendor-ID wrongly
> > programmed to 0x10d2 instead of Nvidia's 0x10de thereby the quirks
> > applicable for Tegra194 are not being applied. This patch fixes this
> > issue by refactoring the code that disables the ASPM-L1SS advertisement.
> >
> > Fixes: 56e15a238d92 ("PCI: tegra: Add Tegra194 PCIe support")
> > Signed-off-by: Vidya Sagar <vidyas@xxxxxxxxxx>
> > ---
> > V4:
> > * None
> >
> > V3:
> > * None
> >
> > V2:
> > * None
> >
> >  drivers/pci/controller/dwc/pcie-tegra194.c | 12 ++++++------
> >  1 file changed, 6 insertions(+), 6 deletions(-)
>
> Looks like this no longer applies cleanly after that other fix that you sent earlier.
> But looking more closely, that's because that other fix already incorporates an
> equivalent change, so I think this can be dropped from this series.
Yes. This is no longer applies cleanly and I'll fix it in the next 
series, but, the current patch is still required.
The other change I pushed is taking care of getting a valid 'dbi' 
address before accessing the dbi region, but, this current change would 
make sure that 'pcie->cfg_link_cap_l1sub' would have a valid value 
before calling disable_aspm_l1/2() APIs.

Thanks,
Vidya Sagar
> Thierry