This series of patches do some enhancements and some bug fixes to the
Tegra194 PCIe platform driver like
- Fix Vendor-ID corruption
- Map DBI space correctly
- Update DWC IP version
- Continue with uninitialization sequence even if parts fail
- Check return value of tegra_pcie_init_controller()
V4:
* Added a new patch to address link-up issues with some of the cards
V3:
* Addressed Bjorn's review comments
* Split earlier patch-4 into two
- Continue with the uninitialization sequence even if some parts fail
- Check return value of tegra_pcie_init_controller() and exit accordingly
V2:
* Addressed Rob's comments. Changed 'Strongly Ordered' to 'nGnRnE'
Vidya Sagar (6):
PCI: tegra: Fix ASPM-L1SS advertisement disable code
PCI: tegra: Map configuration space as nGnRnE
PCI: tegra: Set DesignWare IP version
PCI: tegra: Continue unconfig sequence even if parts fail
PCI: tegra: Check return value of tegra_pcie_init_controller()
PCI: tegra: Disable LTSSM during L2 entry
drivers/pci/controller/dwc/pcie-tegra194.c | 78 +++++++++++-----------
1 file changed, 39 insertions(+), 39 deletions(-)
--
2.17.1
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 <[email protected]>
---
V4:
* None
V3:
* None
V2:
* None
drivers/pci/controller/dwc/pcie-tegra194.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
index aa511ec0d800..b172b1d49713 100644
--- a/drivers/pci/controller/dwc/pcie-tegra194.c
+++ b/drivers/pci/controller/dwc/pcie-tegra194.c
@@ -896,6 +896,12 @@ static void tegra_pcie_prepare_host(struct pcie_port *pp)
init_host_aspm(pcie);
+ /* Disable ASPM-L1SS advertisement if there is no CLKREQ routing */
+ if (!pcie->supports_clkreq) {
+ disable_aspm_l11(pcie);
+ disable_aspm_l12(pcie);
+ }
+
val = dw_pcie_readl_dbi(pci, GEN3_RELATED_OFF);
val &= ~GEN3_RELATED_OFF_GEN3_ZRXDC_NONCOMPL;
dw_pcie_writel_dbi(pci, GEN3_RELATED_OFF, val);
@@ -1400,12 +1406,6 @@ static int tegra_pcie_config_controller(struct tegra_pcie_dw *pcie,
pcie->pcie_cap_base = dw_pcie_find_capability(&pcie->pci,
PCI_CAP_ID_EXP);
- /* Disable ASPM-L1SS advertisement as there is no CLKREQ routing */
- if (!pcie->supports_clkreq) {
- disable_aspm_l11(pcie);
- disable_aspm_l12(pcie);
- }
-
return ret;
fail_phy:
--
2.17.1
As specified in the comment for pci_remap_cfgspace() define in
arch/arm64/include/asm/io.h file, PCIe configuration space should be
mapped as nGnRnE. Hence changing to dev_pci_remap_cfgspace() from
devm_ioremap_resource() for mapping DBI space as that is nothing but
the root port's own configuration space.
Signed-off-by: Vidya Sagar <[email protected]>
---
V4:
* None
V3:
* None
V2:
* Changed 'Strongly Ordered' to 'nGnRnE'
drivers/pci/controller/dwc/pcie-tegra194.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
index b172b1d49713..7a0c64436861 100644
--- a/drivers/pci/controller/dwc/pcie-tegra194.c
+++ b/drivers/pci/controller/dwc/pcie-tegra194.c
@@ -2108,7 +2108,9 @@ static int tegra_pcie_dw_probe(struct platform_device *pdev)
}
pcie->dbi_res = dbi_res;
- pci->dbi_base = devm_ioremap_resource(dev, dbi_res);
+ pci->dbi_base = devm_pci_remap_cfgspace(dev,
+ dbi_res->start,
+ resource_size(dbi_res));
if (IS_ERR(pci->dbi_base))
return PTR_ERR(pci->dbi_base);
--
2.17.1
On Mon, Nov 09, 2020 at 10:49:31PM +0530, Vidya Sagar wrote:
> This series of patches do some enhancements and some bug fixes to the
> Tegra194 PCIe platform driver like
> - Fix Vendor-ID corruption
> - Map DBI space correctly
> - Update DWC IP version
> - Continue with uninitialization sequence even if parts fail
> - Check return value of tegra_pcie_init_controller()
>
> V4:
> * Added a new patch to address link-up issues with some of the cards
>
> V3:
> * Addressed Bjorn's review comments
> * Split earlier patch-4 into two
> - Continue with the uninitialization sequence even if some parts fail
> - Check return value of tegra_pcie_init_controller() and exit accordingly
>
> V2:
> * Addressed Rob's comments. Changed 'Strongly Ordered' to 'nGnRnE'
>
> Vidya Sagar (6):
> PCI: tegra: Fix ASPM-L1SS advertisement disable code
> PCI: tegra: Map configuration space as nGnRnE
> PCI: tegra: Set DesignWare IP version
> PCI: tegra: Continue unconfig sequence even if parts fail
> PCI: tegra: Check return value of tegra_pcie_init_controller()
> PCI: tegra: Disable LTSSM during L2 entry
>
> drivers/pci/controller/dwc/pcie-tegra194.c | 78 +++++++++++-----------
> 1 file changed, 39 insertions(+), 39 deletions(-)
I was going to test this series, but then I noticed that PCI is causing
a crash on linux-next (as of fairly recently). So I tried applying this
on top of v5.10-rc1, but that gives me the following:
[ 3.595161] ahci 0001:01:00.0: version 3.0
[ 3.595726] ahci 0001:01:00.0: SSS flag set, parallel bus scan disabled
[ 4.609923] ahci 0001:01:00.0: controller reset failed (0xffffffff)
[ 4.610343] ahci: probe of 0001:01:00.0 failed with error -5
So the device enumerates fine, but it's not able to reset the SATA
controller. That said, this seems to happen regardless of this patch
series, so plain v5.10-rc1 also shows the above.
Given the above, I think we should hold off on applying this series
until we've fixed PCI on linux-next and made sure that SATA also works
properly, otherwise we don't have a known good baseline to test this
against.
Thierry
> -----Original Message-----
> From: Thierry Reding <[email protected]>
> Sent: Wednesday, November 25, 2020 11:27 PM
> To: Vidya Sagar <[email protected]>
> Cc: [email protected]; [email protected]; [email protected];
> Jonathan Hunter <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected]; linux-
> [email protected]; [email protected]; Krishna Thota
> <[email protected]>; Manikanta Maddireddy <[email protected]>;
> [email protected]
> Subject: Re: [PATCH V4 0/6] Enhancements to Tegra194 PCIe driver
>
> On Mon, Nov 09, 2020 at 10:49:31PM +0530, Vidya Sagar wrote:
> > This series of patches do some enhancements and some bug fixes to the
> > Tegra194 PCIe platform driver like
> > - Fix Vendor-ID corruption
> > - Map DBI space correctly
> > - Update DWC IP version
> > - Continue with uninitialization sequence even if parts fail
> > - Check return value of tegra_pcie_init_controller()
> >
> > V4:
> > * Added a new patch to address link-up issues with some of the cards
> >
> > V3:
> > * Addressed Bjorn's review comments
> > * Split earlier patch-4 into two
> > - Continue with the uninitialization sequence even if some parts fail
> > - Check return value of tegra_pcie_init_controller() and exit
> > accordingly
> >
> > V2:
> > * Addressed Rob's comments. Changed 'Strongly Ordered' to 'nGnRnE'
> >
> > Vidya Sagar (6):
> > PCI: tegra: Fix ASPM-L1SS advertisement disable code
> > PCI: tegra: Map configuration space as nGnRnE
> > PCI: tegra: Set DesignWare IP version
> > PCI: tegra: Continue unconfig sequence even if parts fail
> > PCI: tegra: Check return value of tegra_pcie_init_controller()
> > PCI: tegra: Disable LTSSM during L2 entry
> >
> > drivers/pci/controller/dwc/pcie-tegra194.c | 78
> > +++++++++++-----------
> > 1 file changed, 39 insertions(+), 39 deletions(-)
>
> I was going to test this series, but then I noticed that PCI is causing a crash on
> linux-next (as of fairly recently).
I root caused the crash issue to the following commit
a0fd361db8e5 ("PCI: dwc: Move "dbi", "dbi2", and "addr_space" resource
setup into common code")
I also pushed the following two patches to fix this issue for review
http://patchwork.ozlabs.org/project/linux-pci/patch/[email protected]/
http://patchwork.ozlabs.org/project/linux-pci/patch/[email protected]/
> So I tried applying this on top of v5.10-rc1, but
> that gives me the following:
>
> [ 3.595161] ahci 0001:01:00.0: version 3.0
> [ 3.595726] ahci 0001:01:00.0: SSS flag set, parallel bus scan disabled
> [ 4.609923] ahci 0001:01:00.0: controller reset failed (0xffffffff)
> [ 4.610343] ahci: probe of 0001:01:00.0 failed with error -5
>
> So the device enumerates fine, but it's not able to reset the SATA controller.
> That said, this seems to happen regardless of this patch series, so plain v5.10-rc1
> also shows the above.
This was also a known issue and we need the following commit to make
things work (FWIW, it is already accepted)
9fff3256f93d PCI: dwc: Restore ATU memory resource setup to use last entry
Otherwise, v5.10-rc3 can be used which has working state of PCIe on
Tegra194.
>
> Given the above, I think we should hold off on applying this series until we've
> fixed PCI on linux-next and made sure that SATA also works properly, otherwise
> we don't have a known good baseline to test this against.
>
> Thierry
On Thu, Nov 26, 2020 at 01:21:44AM +0530, Vidya Sagar wrote:
>
>
> > -----Original Message-----
> > From: Thierry Reding <[email protected]>
> > Sent: Wednesday, November 25, 2020 11:27 PM
> > To: Vidya Sagar <[email protected]>
> > Cc: [email protected]; [email protected]; [email protected];
> > Jonathan Hunter <[email protected]>; [email protected];
> > [email protected]; [email protected]; [email protected]; linux-
> > [email protected]; [email protected]; Krishna Thota
> > <[email protected]>; Manikanta Maddireddy <[email protected]>;
> > [email protected]
> > Subject: Re: [PATCH V4 0/6] Enhancements to Tegra194 PCIe driver
> >
> > On Mon, Nov 09, 2020 at 10:49:31PM +0530, Vidya Sagar wrote:
> > > This series of patches do some enhancements and some bug fixes to the
> > > Tegra194 PCIe platform driver like
> > > - Fix Vendor-ID corruption
> > > - Map DBI space correctly
> > > - Update DWC IP version
> > > - Continue with uninitialization sequence even if parts fail
> > > - Check return value of tegra_pcie_init_controller()
> > >
> > > V4:
> > > * Added a new patch to address link-up issues with some of the cards
> > >
> > > V3:
> > > * Addressed Bjorn's review comments
> > > * Split earlier patch-4 into two
> > > - Continue with the uninitialization sequence even if some parts fail
> > > - Check return value of tegra_pcie_init_controller() and exit
> > > accordingly
> > >
> > > V2:
> > > * Addressed Rob's comments. Changed 'Strongly Ordered' to 'nGnRnE'
> > >
> > > Vidya Sagar (6):
> > > PCI: tegra: Fix ASPM-L1SS advertisement disable code
> > > PCI: tegra: Map configuration space as nGnRnE
> > > PCI: tegra: Set DesignWare IP version
> > > PCI: tegra: Continue unconfig sequence even if parts fail
> > > PCI: tegra: Check return value of tegra_pcie_init_controller()
> > > PCI: tegra: Disable LTSSM during L2 entry
> > >
> > > drivers/pci/controller/dwc/pcie-tegra194.c | 78
> > > +++++++++++-----------
> > > 1 file changed, 39 insertions(+), 39 deletions(-)
> >
> > I was going to test this series, but then I noticed that PCI is causing a crash on
> > linux-next (as of fairly recently).
> I root caused the crash issue to the following commit
> a0fd361db8e5 ("PCI: dwc: Move "dbi", "dbi2", and "addr_space" resource setup
> into common code")
>
> I also pushed the following two patches to fix this issue for review
>
> http://patchwork.ozlabs.org/project/linux-pci/patch/[email protected]/
> http://patchwork.ozlabs.org/project/linux-pci/patch/[email protected]/
Great, those fix Tegra194 PCIe on next-20201126 for me!
> > So I tried applying this on top of v5.10-rc1, but
> > that gives me the following:
> >
> > [ 3.595161] ahci 0001:01:00.0: version 3.0
> > [ 3.595726] ahci 0001:01:00.0: SSS flag set, parallel bus scan disabled
> > [ 4.609923] ahci 0001:01:00.0: controller reset failed (0xffffffff)
> > [ 4.610343] ahci: probe of 0001:01:00.0 failed with error -5
> >
> > So the device enumerates fine, but it's not able to reset the SATA controller.
> > That said, this seems to happen regardless of this patch series, so plain v5.10-rc1
> > also shows the above.
> This was also a known issue and we need the following commit to make things
> work (FWIW, it is already accepted)
> 9fff3256f93d PCI: dwc: Restore ATU memory resource setup to use last entry
>
> Otherwise, v5.10-rc3 can be used which has working state of PCIe on
> Tegra194.
Okay, I see that now. Thanks for following up on those.
Thierry
On Mon, Nov 09, 2020 at 10:49:33PM +0530, Vidya Sagar wrote:
> As specified in the comment for pci_remap_cfgspace() define in
> arch/arm64/include/asm/io.h file, PCIe configuration space should be
> mapped as nGnRnE. Hence changing to dev_pci_remap_cfgspace() from
> devm_ioremap_resource() for mapping DBI space as that is nothing but
> the root port's own configuration space.
>
> Signed-off-by: Vidya Sagar <[email protected]>
> ---
> V4:
> * None
>
> V3:
> * None
>
> V2:
> * Changed 'Strongly Ordered' to 'nGnRnE'
>
> drivers/pci/controller/dwc/pcie-tegra194.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
> index b172b1d49713..7a0c64436861 100644
> --- a/drivers/pci/controller/dwc/pcie-tegra194.c
> +++ b/drivers/pci/controller/dwc/pcie-tegra194.c
> @@ -2108,7 +2108,9 @@ static int tegra_pcie_dw_probe(struct platform_device *pdev)
> }
> pcie->dbi_res = dbi_res;
>
> - pci->dbi_base = devm_ioremap_resource(dev, dbi_res);
> + pci->dbi_base = devm_pci_remap_cfgspace(dev,
> + dbi_res->start,
> + resource_size(dbi_res));
> if (IS_ERR(pci->dbi_base))
> return PTR_ERR(pci->dbi_base);
>
Similarly to patch 1/6, this is no longer required because it's already
part of one of Rob's earlier patches, so this, too, can be dropped.
Thierry
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 <[email protected]>
> ---
> 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.
Thierry
> -----Original Message-----
> From: Thierry Reding <[email protected]>
> Sent: Thursday, November 26, 2020 5:03 PM
> To: Vidya Sagar <[email protected]>
> Cc: [email protected]; [email protected]; [email protected];
> Jonathan Hunter <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected]; linux-
> [email protected]; [email protected]; Krishna Thota
> <[email protected]>; Manikanta Maddireddy <[email protected]>;
> [email protected]
> 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 <[email protected]>
> > ---
> > 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
> -----Original Message-----
> From: Thierry Reding <[email protected]>
> Sent: Thursday, November 26, 2020 5:04 PM
> To: Vidya Sagar <[email protected]>
> Cc: [email protected]; [email protected]; [email protected];
> Jonathan Hunter <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected]; linux-
> [email protected]; [email protected]; Krishna Thota
> <[email protected]>; Manikanta Maddireddy <[email protected]>;
> [email protected]
> Subject: Re: [PATCH V4 2/6] PCI: tegra: Map configuration space as nGnRnE
>
> On Mon, Nov 09, 2020 at 10:49:33PM +0530, Vidya Sagar wrote:
> > As specified in the comment for pci_remap_cfgspace() define in
> > arch/arm64/include/asm/io.h file, PCIe configuration space should be
> > mapped as nGnRnE. Hence changing to dev_pci_remap_cfgspace() from
> > devm_ioremap_resource() for mapping DBI space as that is nothing but
> > the root port's own configuration space.
> >
> > Signed-off-by: Vidya Sagar <[email protected]>
> > ---
> > V4:
> > * None
> >
> > V3:
> > * None
> >
> > V2:
> > * Changed 'Strongly Ordered' to 'nGnRnE'
> >
> > drivers/pci/controller/dwc/pcie-tegra194.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c
> > b/drivers/pci/controller/dwc/pcie-tegra194.c
> > index b172b1d49713..7a0c64436861 100644
> > --- a/drivers/pci/controller/dwc/pcie-tegra194.c
> > +++ b/drivers/pci/controller/dwc/pcie-tegra194.c
> > @@ -2108,7 +2108,9 @@ static int tegra_pcie_dw_probe(struct
> platform_device *pdev)
> > }
> > pcie->dbi_res = dbi_res;
> >
> > - pci->dbi_base = devm_ioremap_resource(dev, dbi_res);
> > + pci->dbi_base = devm_pci_remap_cfgspace(dev,
> > + dbi_res->start,
> > + resource_size(dbi_res));
> > if (IS_ERR(pci->dbi_base))
> > return PTR_ERR(pci->dbi_base);
> >
>
> Similarly to patch 1/6, this is no longer required because it's already part of one
> of Rob's earlier patches, so this, too, can be dropped.
Yes. This patch is not required now. I'll drop it from the next patch
series.
Thanks,
Vidya Sagar
>
> Thierry