Re: [Bug 215540] New: mvebu: no pcie devices detected on turris omnia (5.16.3 regression)

From: "Pali Rohár" <pali@kernel.org>
To: Bjorn Helgaas <helgaas@kernel.org>,
	Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: "Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	linux-pci@vger.kernel.org, "Jan Palus" <jpalus@fastmail.com>,
	"Thomas Petazzoni" <thomas.petazzoni@bootlin.com>,
	"Marek Behún" <kabel@kernel.org>
Subject: Re: [Bug 215540] New: mvebu: no pcie devices detected on turris omnia (5.16.3 regression)
Date: Fri, 11 Feb 2022 20:43:30 +0100	[thread overview]
Message-ID: <20220211194330.siiwntko6b3lldw4@pali> (raw)
In-Reply-To: <20220203154728.GA96160@bhelgaas>

On Thursday 03 February 2022 09:47:28 Bjorn Helgaas wrote:
> [+cc Lorenzo, beginning of thread:
> https://lore.kernel.org/r/20220127234917.GA150851@bhelgaas]
> 
> On Thu, Feb 03, 2022 at 01:55:28PM +0100, Pali Rohár wrote:
> > On Thursday 03 February 2022 13:26:42 Pali Rohár wrote:
> > > On Thursday 27 January 2022 17:49:17 Bjorn Helgaas wrote:
> > > > On Thu, Jan 27, 2022 at 10:52:43PM +0000, bugzilla-daemon@bugzilla.kernel.org wrote:
> > > > > https://bugzilla.kernel.org/show_bug.cgi?id=215540
> > > > > 
> > > > >             Bug ID: 215540
> > > > >            Summary: mvebu: no pcie devices detected on turris omnia
> > > > >                     (5.16.3 regression)
> > > > >            Product: Drivers
> > > > >            Version: 2.5
> > > > >     Kernel Version: 5.16.3
> > > > >           Hardware: ARM
> > > > >                 OS: Linux
> > > > >               Tree: Mainline
> > > > >             Status: NEW
> > > > >           Severity: normal
> > > > >           Priority: P1
> > > > >          Component: PCI
> > > > >           Assignee: drivers_pci@kernel-bugs.osdl.org
> > > > >           Reporter: jpalus@fastmail.com
> > > > >         Regression: No
> > > > > 
> > > > > After kernel upgrade from 5.16.1 to 5.16.3 Turris Omnia (Armada 385)
> > > > > no longer detects pcie devices (wifi/msata). Haven't tried 5.16.2
> > > > > but it doesn't seem to have any relevant changes, while 5.16.3
> > > > > carries a few.
> > 
> > I found another issue: Into stable tree was backported "modified" patch.
> > I'm not sure it is is source of this issue but looks like it is related.
> > 
> > If you open mentioned problematic commit in web ui:
> > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=linux-5.16.y&id=7cde9bf0731688896831f90da9fe755f44a6d5e0
> > 
> > And compare it with patch which is on "Link:" line from commit message:
> > https://lore.kernel.org/r/20211125124605.25915-12-pali@kernel.org
> > 
> > You will see that diffs are different. In my original patch (which I
> > sent to ML) is:
> > 
> >  		mvebu_pcie_setup_hw(port);
> >  		mvebu_pcie_set_local_dev_nr(port, 1);
> > +		mvebu_pcie_set_local_bus_nr(port, 0);
> > 
> > But in git web ui is:
> > 
> >  		mvebu_pcie_setup_hw(port);
> > -		mvebu_pcie_set_local_dev_nr(port, 1);
> > +		mvebu_pcie_set_local_dev_nr(port, 0);
> > 
> > I do not know how it could happen. But local **device** number must be
> > always set to 1 (see comment above code for explanation) and default
> > value of local **bus** number should be 0 (as is in my original patch).
> > 
> > So above patch in stable tree is broken.
> 
> I think current mainline is broken, too, isn't it?  See below.

I see, it is.

> > Bjorn & Greg: How do you want to handle this situation? Should I prepare
> > special patch for stable which fix it? Or something else?
> > 
> > Anyway, do you know how it could happen that patch was incorrectly
> > auto-backported into stable? Differences between original and
> > wrongly-modified patch looks very similar (both "bus" and "dev" keywords
> > have same number of characters) and it was hard for me to see that there
> > are differences. So probably overlooking could happen or maybe git or
> > patch tools could do such small changes when doing backports?
> 
> Your patch on the mailing list [1] contains:
> 
>         mvebu_pcie_setup_hw(port);
>         mvebu_pcie_set_local_dev_nr(port, 1);
>   +     mvebu_pcie_set_local_bus_nr(port, 0);
> 
> 91a8d79fc797 ("PCI: mvebu: Fix configuring secondary bus of PCIe Root
> Port via emulated bridge") [2] appeared in v5.17-rc1 and contains:
> 
>         mvebu_pcie_setup_hw(port);
>   -     mvebu_pcie_set_local_dev_nr(port, 1);
>   +     mvebu_pcie_set_local_dev_nr(port, 0);
> 
> And this is the current state of mainline [3].
> 
> 91a8d79fc797 was backported to v5.16.3 as 7cde9bf07316 [4], which also
> contains:
> 
>         mvebu_pcie_setup_hw(port);
>   -     mvebu_pcie_set_local_dev_nr(port, 1);
>   +     mvebu_pcie_set_local_dev_nr(port, 0);
> 
> So I think the problem was a merge error when we first applied this
> for mainline, and we just need to make a patch for mainline, apply it
> for v5.17, and mark it for stable.

Should I prepare and send fixup patch?

> Bjorn
> 
> [1] https://lore.kernel.org/r/20211125124605.25915-12-pali@kernel.org
> [2] https://git.kernel.org/linus/91a8d79fc797
> [3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pci/controller/pci-mvebu.c?id=v5.17-rc2#n1323
> [4] https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=linux-5.16.y&id=7cde9bf07316