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

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

[+Greg]

On Fri, Jul 05, 2019 at 09:57:25AM +0100, Jon Hunter wrote:
> Hi Lorenzo,
> 
> On 04/07/2019 17:09, Lorenzo Pieralisi wrote:
> > On Thu, Jul 04, 2019 at 08:34:28PM +0530, Vidya Sagar wrote:
> >> Currently Relaxed Ordering bit in the configuration space is enabled for
> >> all PCIe devices as the quirk uses PCI_ANY_ID for both Vendor-ID and
> >> Device-ID, but, as per the Technical Reference Manual of Tegra20 which is
> >> available at https://developer.nvidia.com/embedded/downloads#?search=tegra%202
> >> in Sec 34.1, it is mentioned that Relexed Ordering bit needs to be enabled in
> >> its root ports to avoid deadlock in hardware. The same is applicable for
> >> Tegra30 as well though it is not explicitly mentioned in Tegra30 TRM document,
> >> but the same must not be extended to root ports of other Tegra SoCs or
> >> other hosts as the same issue doesn't exist there.
> >>
> >> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
> > 
> > You forgot Thierry's ACK, I added it back but next time pay more
> > attention please.
> > 
> > You should link the versions through eg git send-email
> > --in-reply-to=Message-Id so that it is easier to follow.
> > 
> >> ---
> >> V3:
> >> * Modified commit message to make it more precise and explicit
> >>
> >> V2:
> >> * Modified commit message to include reference to Tegra20 TRM document.
> >>
> >>  drivers/pci/controller/pci-tegra.c | 7 +++++--
> >>  1 file changed, 5 insertions(+), 2 deletions(-)
> > 
> > I applied it to pci/tegra after rewriting the whole commit log and
> > adding a Fixes: tag that you or someone at Nvidia will follow up;
> > I will check.
> 
> I had a chat with Vidya last night to understand the issue, so now I
> have a good understanding of the problem this has caused, which is very
> unfortunate indeed!
> 
> Vidya mentioned that you would like us to get this backported to stable
> branches. Please correct me if I am wrong here. We can certainly do
> that, but I do have concerns about doing so, for non-Tegra devices
> inparticularly, given that this has been around for sometime now. Hence,
> I was wondering if we should leave this soak in the mainline for at
> least a kernel release cycle before doing so. I really don't want to
> break stable for anyone. What are your thoughts on this?

I looped in Greg to pick his brain, since it is unclear how we should
apply the stable kernel rules on this specific patch. Basically, this
technically is not a bug, it is just bad code that forces a feature on
ALL kernels that compile the PCI Tegra Controller driver in the kernel.
I would really really want to have this patch applied to all stable
kernels but first as you said it is better to apply it to mainline and
check it does not cause any issues on any other arch/platform then
we can think about backporting it to stable kernels.

I am not happy to force Relaxed Ordering on any PCIe device on
any platform/arch compiling PCI Tegra controller in, so somehow
we must rectify this situation, this is gross as I said before.

I added a Fixes: tag but I am not sure it is appropriate for the
above to happen and I think it is better for this patch to brew
at least a release cycle in the mainline before sending it back
to stable kernels, if appropriate, how to translate this into
stable kernel rules that's the question I am asking.

Thanks,
Lorenzo