Re: [PATCH] PCI: tegra: Enable Relaxed Ordering only for Tegra20 & Tegra30

[Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>]

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
> > > 
>