* [PATCH] PCI: tegra: Enable Relaxed Ordering only for Tegra20 & Tegra30 @ 2019-06-18 7:38 Vidya Sagar 2019-06-20 11:18 ` Thierry Reding 2019-06-27 10:06 ` Lorenzo Pieralisi 0 siblings, 2 replies; 7+ messages in thread From: Vidya Sagar @ 2019-06-18 7:38 UTC (permalink / raw) To: bhelgaas, lorenzo.pieralisi, treding, jonathanh Cc: linux-tegra, linux-pci, kthota, mmaddireddy, vidyas, sagar.tv Currently Relaxed Ordering bit in the configuration space is enabled for all devices whereas it should be enabled only for root ports for Tegra20 and Tegra30 chips to avoid deadlock in hardware. Signed-off-by: Vidya Sagar <vidyas@nvidia.com> --- drivers/pci/controller/pci-tegra.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c index 464ba2538d52..bc7be369c1b3 100644 --- a/drivers/pci/controller/pci-tegra.c +++ b/drivers/pci/controller/pci-tegra.c @@ -545,12 +545,15 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NVIDIA, 0x0bf1, tegra_pcie_fixup_class); DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NVIDIA, 0x0e1c, tegra_pcie_fixup_class); DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NVIDIA, 0x0e1d, tegra_pcie_fixup_class); -/* Tegra PCIE requires relaxed ordering */ +/* Tegra20 and Tegra30 PCIE requires relaxed ordering */ static void tegra_pcie_relax_enable(struct pci_dev *dev) { pcie_capability_set_word(dev, PCI_EXP_DEVCTL, PCI_EXP_DEVCTL_RELAX_EN); } -DECLARE_PCI_FIXUP_FINAL(PCI_ANY_ID, PCI_ANY_ID, tegra_pcie_relax_enable); +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_NVIDIA, 0x0bf0, tegra_pcie_relax_enable); +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_NVIDIA, 0x0bf1, tegra_pcie_relax_enable); +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_NVIDIA, 0x0e1c, tegra_pcie_relax_enable); +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_NVIDIA, 0x0e1d, tegra_pcie_relax_enable); static int tegra_pcie_request_resources(struct tegra_pcie *pcie) { -- 2.17.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] PCI: tegra: Enable Relaxed Ordering only for Tegra20 & Tegra30 2019-06-18 7:38 [PATCH] PCI: tegra: Enable Relaxed Ordering only for Tegra20 & Tegra30 Vidya Sagar @ 2019-06-20 11:18 ` Thierry Reding 2019-06-26 9:48 ` Vidya Sagar 2019-06-27 10:06 ` Lorenzo Pieralisi 1 sibling, 1 reply; 7+ messages in thread From: Thierry Reding @ 2019-06-20 11:18 UTC (permalink / raw) To: Vidya Sagar Cc: bhelgaas, lorenzo.pieralisi, treding, jonathanh, linux-tegra, linux-pci, kthota, mmaddireddy, sagar.tv [-- Attachment #1: Type: text/plain, Size: 479 bytes --] On Tue, Jun 18, 2019 at 01:08:10PM +0530, Vidya Sagar wrote: > Currently Relaxed Ordering bit in the configuration space is enabled for > all devices whereas it should be enabled only for root ports for Tegra20 > and Tegra30 chips to avoid deadlock in hardware. > > Signed-off-by: Vidya Sagar <vidyas@nvidia.com> > --- > drivers/pci/controller/pci-tegra.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) Acked-by: Thierry Reding <treding@nvidia.com> [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] PCI: tegra: Enable Relaxed Ordering only for Tegra20 & Tegra30 2019-06-20 11:18 ` Thierry Reding @ 2019-06-26 9:48 ` Vidya Sagar 0 siblings, 0 replies; 7+ messages in thread From: Vidya Sagar @ 2019-06-26 9:48 UTC (permalink / raw) To: bhelgaas, lorenzo.pieralisi Cc: Thierry Reding, treding, jonathanh, linux-tegra, linux-pci, kthota, mmaddireddy, sagar.tv On 6/20/2019 4:48 PM, Thierry Reding wrote: > On Tue, Jun 18, 2019 at 01:08:10PM +0530, Vidya Sagar wrote: >> Currently Relaxed Ordering bit in the configuration space is enabled for >> all devices whereas it should be enabled only for root ports for Tegra20 >> and Tegra30 chips to avoid deadlock in hardware. >> >> Signed-off-by: Vidya Sagar <vidyas@nvidia.com> >> --- >> drivers/pci/controller/pci-tegra.c | 7 +++++-- >> 1 file changed, 5 insertions(+), 2 deletions(-) > > Acked-by: Thierry Reding <treding@nvidia.com> > Bjorn / Lorenzo, Can you please consider this patch? Thanks, Vidya Sagar ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] PCI: tegra: Enable Relaxed Ordering only for Tegra20 & Tegra30 2019-06-18 7:38 [PATCH] PCI: tegra: Enable Relaxed Ordering only for Tegra20 & Tegra30 Vidya Sagar 2019-06-20 11:18 ` Thierry Reding @ 2019-06-27 10:06 ` Lorenzo Pieralisi 2019-06-27 15:11 ` Vidya Sagar 1 sibling, 1 reply; 7+ messages in thread From: Lorenzo Pieralisi @ 2019-06-27 10:06 UTC (permalink / raw) To: Vidya Sagar Cc: bhelgaas, treding, jonathanh, linux-tegra, linux-pci, kthota, mmaddireddy, sagar.tv On Tue, Jun 18, 2019 at 01:08:10PM +0530, Vidya Sagar wrote: > Currently Relaxed Ordering bit in the configuration space is enabled for > all devices whereas it should be enabled only for root ports for Tegra20 > and Tegra30 chips to avoid deadlock in hardware. This is a fix so I think you had better add a Fixes: tag and if the erratum is a public document it would be good to add it to the log to explain what the problem is. Thanks, Lorenzo > Signed-off-by: Vidya Sagar <vidyas@nvidia.com> > --- > drivers/pci/controller/pci-tegra.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c > index 464ba2538d52..bc7be369c1b3 100644 > --- a/drivers/pci/controller/pci-tegra.c > +++ b/drivers/pci/controller/pci-tegra.c > @@ -545,12 +545,15 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NVIDIA, 0x0bf1, tegra_pcie_fixup_class); > DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NVIDIA, 0x0e1c, tegra_pcie_fixup_class); > DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NVIDIA, 0x0e1d, tegra_pcie_fixup_class); > > -/* Tegra PCIE requires relaxed ordering */ > +/* Tegra20 and Tegra30 PCIE requires relaxed ordering */ > static void tegra_pcie_relax_enable(struct pci_dev *dev) > { > pcie_capability_set_word(dev, PCI_EXP_DEVCTL, PCI_EXP_DEVCTL_RELAX_EN); > } > -DECLARE_PCI_FIXUP_FINAL(PCI_ANY_ID, PCI_ANY_ID, tegra_pcie_relax_enable); > +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_NVIDIA, 0x0bf0, tegra_pcie_relax_enable); > +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_NVIDIA, 0x0bf1, tegra_pcie_relax_enable); > +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_NVIDIA, 0x0e1c, tegra_pcie_relax_enable); > +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_NVIDIA, 0x0e1d, tegra_pcie_relax_enable); > > static int tegra_pcie_request_resources(struct tegra_pcie *pcie) > { > -- > 2.17.1 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] PCI: tegra: Enable Relaxed Ordering only for Tegra20 & Tegra30 2019-06-27 10:06 ` Lorenzo Pieralisi @ 2019-06-27 15:11 ` Vidya Sagar 2019-06-27 15:23 ` Lorenzo Pieralisi 0 siblings, 1 reply; 7+ messages in thread From: Vidya Sagar @ 2019-06-27 15:11 UTC (permalink / raw) To: Lorenzo Pieralisi Cc: bhelgaas, treding, jonathanh, linux-tegra, linux-pci, kthota, mmaddireddy, sagar.tv On 6/27/2019 3:36 PM, Lorenzo Pieralisi wrote: > On Tue, Jun 18, 2019 at 01:08:10PM +0530, Vidya Sagar wrote: >> Currently Relaxed Ordering bit in the configuration space is enabled for >> all devices whereas it should be enabled only for root ports for Tegra20 >> and Tegra30 chips to avoid deadlock in hardware. > > This is a fix so I think you had better add a Fixes: tag and if the > erratum is a public document it would be good to add it to the log > to explain what the problem is. > > Thanks, > Lorenzo This has been there from the beginning. I mean, if I have to give a tag as part of Fixes: , then I should give the tag that adds this driver itself. Is that fine? I can quote the following document section 31.1 for Tegra20, but the same information is not mentioned in Tegra30 specific public document. https://www.chiark.greenend.org.uk/~theom/riscos/docs/Tegra2_TRM_DP04508001v01p.pdf Is it fine to just quote only this document? Thanks, Vidya Sagar > >> Signed-off-by: Vidya Sagar <vidyas@nvidia.com> >> --- >> drivers/pci/controller/pci-tegra.c | 7 +++++-- >> 1 file changed, 5 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c >> index 464ba2538d52..bc7be369c1b3 100644 >> --- a/drivers/pci/controller/pci-tegra.c >> +++ b/drivers/pci/controller/pci-tegra.c >> @@ -545,12 +545,15 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NVIDIA, 0x0bf1, tegra_pcie_fixup_class); >> DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NVIDIA, 0x0e1c, tegra_pcie_fixup_class); >> DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NVIDIA, 0x0e1d, tegra_pcie_fixup_class); >> >> -/* Tegra PCIE requires relaxed ordering */ >> +/* Tegra20 and Tegra30 PCIE requires relaxed ordering */ >> static void tegra_pcie_relax_enable(struct pci_dev *dev) >> { >> pcie_capability_set_word(dev, PCI_EXP_DEVCTL, PCI_EXP_DEVCTL_RELAX_EN); >> } >> -DECLARE_PCI_FIXUP_FINAL(PCI_ANY_ID, PCI_ANY_ID, tegra_pcie_relax_enable); >> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_NVIDIA, 0x0bf0, tegra_pcie_relax_enable); >> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_NVIDIA, 0x0bf1, tegra_pcie_relax_enable); >> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_NVIDIA, 0x0e1c, tegra_pcie_relax_enable); >> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_NVIDIA, 0x0e1d, tegra_pcie_relax_enable); >> >> static int tegra_pcie_request_resources(struct tegra_pcie *pcie) >> { >> -- >> 2.17.1 >> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] PCI: tegra: Enable Relaxed Ordering only for Tegra20 & Tegra30 2019-06-27 15:11 ` Vidya Sagar @ 2019-06-27 15:23 ` Lorenzo Pieralisi 2019-07-01 10:50 ` Vidya Sagar 0 siblings, 1 reply; 7+ messages in thread From: Lorenzo Pieralisi @ 2019-06-27 15:23 UTC (permalink / raw) To: Vidya Sagar Cc: bhelgaas, treding, jonathanh, linux-tegra, linux-pci, kthota, mmaddireddy, sagar.tv On Thu, Jun 27, 2019 at 08:41:37PM +0530, Vidya Sagar wrote: > On 6/27/2019 3:36 PM, Lorenzo Pieralisi wrote: > > On Tue, Jun 18, 2019 at 01:08:10PM +0530, Vidya Sagar wrote: > > > Currently Relaxed Ordering bit in the configuration space is enabled for > > > all devices whereas it should be enabled only for root ports for Tegra20 > > > and Tegra30 chips to avoid deadlock in hardware. > > > > This is a fix so I think you had better add a Fixes: tag and if the > > erratum is a public document it would be good to add it to the log > > to explain what the problem is. > > > > Thanks, > > Lorenzo > This has been there from the beginning. I mean, if I have to give a tag as part > of Fixes: , then I should give the tag that adds this driver itself. Is that fine? > I can quote the following document section 31.1 for Tegra20, but the same information > is not mentioned in Tegra30 specific public document. > https://www.chiark.greenend.org.uk/~theom/riscos/docs/Tegra2_TRM_DP04508001v01p.pdf > Is it fine to just quote only this document? I am pretty sure an errata document should exist about this and Nvidia owns it (if it is not public, well, there is nothing to quote then); I would not quote this website, it does not look like Nvidia official documentation. For the Fixes: tag, it is up to you, to me this seems like a critical bug (deadlock in hardware) and I do not think you want to ship stable kernels with it. It depends on the entity of the bug we are fixing, I leave it to you to decide the best course of action, I am just trying to help. Lorenzo > Thanks, > Vidya Sagar > > > > > Signed-off-by: Vidya Sagar <vidyas@nvidia.com> > > > --- > > > drivers/pci/controller/pci-tegra.c | 7 +++++-- > > > 1 file changed, 5 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c > > > index 464ba2538d52..bc7be369c1b3 100644 > > > --- a/drivers/pci/controller/pci-tegra.c > > > +++ b/drivers/pci/controller/pci-tegra.c > > > @@ -545,12 +545,15 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NVIDIA, 0x0bf1, tegra_pcie_fixup_class); > > > DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NVIDIA, 0x0e1c, tegra_pcie_fixup_class); > > > DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NVIDIA, 0x0e1d, tegra_pcie_fixup_class); > > > -/* Tegra PCIE requires relaxed ordering */ > > > +/* Tegra20 and Tegra30 PCIE requires relaxed ordering */ > > > static void tegra_pcie_relax_enable(struct pci_dev *dev) > > > { > > > pcie_capability_set_word(dev, PCI_EXP_DEVCTL, PCI_EXP_DEVCTL_RELAX_EN); > > > } > > > -DECLARE_PCI_FIXUP_FINAL(PCI_ANY_ID, PCI_ANY_ID, tegra_pcie_relax_enable); > > > +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_NVIDIA, 0x0bf0, tegra_pcie_relax_enable); > > > +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_NVIDIA, 0x0bf1, tegra_pcie_relax_enable); > > > +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_NVIDIA, 0x0e1c, tegra_pcie_relax_enable); > > > +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_NVIDIA, 0x0e1d, tegra_pcie_relax_enable); > > > static int tegra_pcie_request_resources(struct tegra_pcie *pcie) > > > { > > > -- > > > 2.17.1 > > > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] PCI: tegra: Enable Relaxed Ordering only for Tegra20 & Tegra30 2019-06-27 15:23 ` Lorenzo Pieralisi @ 2019-07-01 10:50 ` Vidya Sagar 0 siblings, 0 replies; 7+ messages in thread From: Vidya Sagar @ 2019-07-01 10:50 UTC (permalink / raw) To: Lorenzo Pieralisi Cc: bhelgaas, treding, jonathanh, linux-tegra, linux-pci, kthota, mmaddireddy, sagar.tv On 6/27/2019 8:53 PM, Lorenzo Pieralisi wrote: > On Thu, Jun 27, 2019 at 08:41:37PM +0530, Vidya Sagar wrote: >> On 6/27/2019 3:36 PM, Lorenzo Pieralisi wrote: >>> On Tue, Jun 18, 2019 at 01:08:10PM +0530, Vidya Sagar wrote: >>>> Currently Relaxed Ordering bit in the configuration space is enabled for >>>> all devices whereas it should be enabled only for root ports for Tegra20 >>>> and Tegra30 chips to avoid deadlock in hardware. >>> >>> This is a fix so I think you had better add a Fixes: tag and if the >>> erratum is a public document it would be good to add it to the log >>> to explain what the problem is. >>> >>> Thanks, >>> Lorenzo >> This has been there from the beginning. I mean, if I have to give a tag as part >> of Fixes: , then I should give the tag that adds this driver itself. Is that fine? >> I can quote the following document section 31.1 for Tegra20, but the same information >> is not mentioned in Tegra30 specific public document. >> https://www.chiark.greenend.org.uk/~theom/riscos/docs/Tegra2_TRM_DP04508001v01p.pdf >> Is it fine to just quote only this document? > > I am pretty sure an errata document should exist about this and Nvidia > owns it (if it is not public, well, there is nothing to quote then); I > would not quote this website, it does not look like Nvidia official > documentation. We have the TRM (Technical Reference Manual) document available for Tegra20 at https://developer.nvidia.com/embedded/downloads#?search=tegra%202 but basic registration is required to download the document. I'll mention this link in the commit message. > > For the Fixes: tag, it is up to you, to me this seems like a > critical bug (deadlock in hardware) and I do not think you > want to ship stable kernels with it. It depends on the entity > of the bug we are fixing, I leave it to you to decide the > best course of action, I am just trying to help. Since there is no specific commit that introduced this issue and the issue has been there from the beginning, I'm of the opinion that giving initial commit tag (the one that added the driver itself) for 'Fixes: tag doesn't really help here. > > Lorenzo > >> Thanks, >> Vidya Sagar >>> >>>> Signed-off-by: Vidya Sagar <vidyas@nvidia.com> >>>> --- >>>> drivers/pci/controller/pci-tegra.c | 7 +++++-- >>>> 1 file changed, 5 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c >>>> index 464ba2538d52..bc7be369c1b3 100644 >>>> --- a/drivers/pci/controller/pci-tegra.c >>>> +++ b/drivers/pci/controller/pci-tegra.c >>>> @@ -545,12 +545,15 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NVIDIA, 0x0bf1, tegra_pcie_fixup_class); >>>> DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NVIDIA, 0x0e1c, tegra_pcie_fixup_class); >>>> DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NVIDIA, 0x0e1d, tegra_pcie_fixup_class); >>>> -/* Tegra PCIE requires relaxed ordering */ >>>> +/* Tegra20 and Tegra30 PCIE requires relaxed ordering */ >>>> static void tegra_pcie_relax_enable(struct pci_dev *dev) >>>> { >>>> pcie_capability_set_word(dev, PCI_EXP_DEVCTL, PCI_EXP_DEVCTL_RELAX_EN); >>>> } >>>> -DECLARE_PCI_FIXUP_FINAL(PCI_ANY_ID, PCI_ANY_ID, tegra_pcie_relax_enable); >>>> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_NVIDIA, 0x0bf0, tegra_pcie_relax_enable); >>>> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_NVIDIA, 0x0bf1, tegra_pcie_relax_enable); >>>> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_NVIDIA, 0x0e1c, tegra_pcie_relax_enable); >>>> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_NVIDIA, 0x0e1d, tegra_pcie_relax_enable); >>>> static int tegra_pcie_request_resources(struct tegra_pcie *pcie) >>>> { >>>> -- >>>> 2.17.1 >>>> >> ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-07-01 10:50 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-06-18 7:38 [PATCH] PCI: tegra: Enable Relaxed Ordering only for Tegra20 & Tegra30 Vidya Sagar 2019-06-20 11:18 ` Thierry Reding 2019-06-26 9:48 ` Vidya Sagar 2019-06-27 10:06 ` Lorenzo Pieralisi 2019-06-27 15:11 ` Vidya Sagar 2019-06-27 15:23 ` Lorenzo Pieralisi 2019-07-01 10:50 ` Vidya Sagar
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).