diff mbox

SB600 for the Nemo board has non-zero devices on non-root bus

Message ID 406ba7c4-7305-4069-227f-81afed202e47@xenosoft.de (mailing list archive)
State New, archived
Delegated to: Bjorn Helgaas
Headers show

Commit Message

Christian Zigotzky Dec. 1, 2017, 10:08 p.m. UTC
On 30.11.2017 23:42, Bjorn Helgaas wrote:
 >
 > 00:11.0 claims to be a PCIe Root Port leading to [bus 05-06]. That
 > means there's a Link (presumably this A-Link II Express thing), and the
 > downstream end of the Link *should* be a PCIe Upstream Port on bus 05,
 > but no such device is visible.  I suppose the SB600 does implement
 > some sort of PCIe Port there, but keeps it invisible to software, and
 > at the same time, contains an invisible bridge that connects the Link
 > to all the conventional PCI devices on bus 05.
 >
 > When we scan bus 05, we do this:
 >
 >   pci_scan_child_bus_extend(bus=05)
 >     for (devfn = 0; devfn < 0x100; devfn += 8)
 >       pci_scan_slot(05, 00.0)
 >         pci_scan_single_device
 >           pci_scan_device(05, 00.0)           # fails; no 05:00.0
 >       pci_scan_slot(05, 01.0)
 >         only_one_child(bus=05)
 >           parent = 00:11.0
 >           pci_pcie_type(00:11.0) == ROOT_PORT # returns true
 >
 > Since only_one_child() sees that 00:11.0 is a Root Port, we give up
 > before we even get to the PCI_SCAN_ALL_PCIE_DEVS test.
 >
 > I *think* something like the patch below should make this work if you
 > use the "pci=pcie_scan_all" parameter.  We have some x86 DMI quirks
 > that set PCI_SCAN_ALL_PCIE_DEVS automatically.  I don't know how to do
 > something similar on powerpc, but maybe you do?
 >

Hi Bjorn,

I tested your new patch today. It boots with the boot argument 
"pci=pcie_scan_all". Well done! :-)

It doesn't boot without the boot argument "pci=pcie_scan_all".

Many thanks for your help.

Cheers,
Christian

Comments

Bjorn Helgaas Dec. 1, 2017, 11:27 p.m. UTC | #1
On Fri, Dec 01, 2017 at 11:08:46PM +0100, Christian Zigotzky wrote:
> On 30.11.2017 23:42, Bjorn Helgaas wrote:
> >
> > 00:11.0 claims to be a PCIe Root Port leading to [bus 05-06]. That
> > means there's a Link (presumably this A-Link II Express thing), and the
> > downstream end of the Link *should* be a PCIe Upstream Port on bus 05,
> > but no such device is visible.  I suppose the SB600 does implement
> > some sort of PCIe Port there, but keeps it invisible to software, and
> > at the same time, contains an invisible bridge that connects the Link
> > to all the conventional PCI devices on bus 05.
> >
> > When we scan bus 05, we do this:
> >
> >   pci_scan_child_bus_extend(bus=05)
> >     for (devfn = 0; devfn < 0x100; devfn += 8)
> >       pci_scan_slot(05, 00.0)
> >         pci_scan_single_device
> >           pci_scan_device(05, 00.0)           # fails; no 05:00.0
> >       pci_scan_slot(05, 01.0)
> >         only_one_child(bus=05)
> >           parent = 00:11.0
> >           pci_pcie_type(00:11.0) == ROOT_PORT # returns true
> >
> > Since only_one_child() sees that 00:11.0 is a Root Port, we give up
> > before we even get to the PCI_SCAN_ALL_PCIE_DEVS test.
> >
> > I *think* something like the patch below should make this work if you
> > use the "pci=pcie_scan_all" parameter.  We have some x86 DMI quirks
> > that set PCI_SCAN_ALL_PCIE_DEVS automatically.  I don't know how to do
> > something similar on powerpc, but maybe you do?
> >
> 
> Hi Bjorn,
> 
> I tested your new patch today. It boots with the boot argument
> "pci=pcie_scan_all". Well done! :-)
> 
> It doesn't boot without the boot argument "pci=pcie_scan_all".

Thanks for testing that.  I'll merge a similar patch for v4.16.

I don't think using "pci=pcie_scan_all" is really an acceptable
long-term answer for you, though.  Is there some way we can identify
at run-time whether we're on a Nemo system?  If so, we can make this
happen automatically.

Bjorn
Christian Zigotzky Dec. 2, 2017, 12:54 p.m. UTC | #2
On 02 December 2017 at 00:27AM, Bjorn Helgaas wrote:
>
> Thanks for testing that.  I'll merge a similar patch for v4.16.
>
> I don't think using "pci=pcie_scan_all" is really an acceptable
> long-term answer for you, though.  Is there some way we can identify
> at run-time whether we're on a Nemo system?  If so, we can make this
> happen automatically.
>
> Bjorn
>
Hi Bjorn,

Many thanks for your effort! I appreciate it very much. :-)

We can identify the Nemo board at the boot time. See dmesg output: [    
0.061592] NEMO SB600 IOB base e0000000

@linuxppc-dev
Any other ideas? Maybe the same as we can identify the other P.A. Semi 
boards (Electra, Chitra, and Athena).

@Olof
Maybe you know how we can identify the P.A. Semi Nemo board at the run-time.

@Darren
Do you have an idea?

Thanks to all for your help.

Cheers,
Christian
Christian Zigotzky March 16, 2018, 12:10 p.m. UTC | #3
On 02 December 2017 at 00:27PM, Bjorn Helgaas wrote:
 > On Fri, Dec 01, 2017 at 11:08:46PM +0100, Christian Zigotzky wrote:
 >> On 30.11.2017 23:42, Bjorn Helgaas wrote:
 >>>
 >>> 00:11.0 claims to be a PCIe Root Port leading to [bus 05-06]. That
 >>> means there's a Link (presumably this A-Link II Express thing), and the
 >>> downstream end of the Link *should* be a PCIe Upstream Port on bus 05,
 >>> but no such device is visible.  I suppose the SB600 does implement
 >>> some sort of PCIe Port there, but keeps it invisible to software, and
 >>> at the same time, contains an invisible bridge that connects the Link
 >>> to all the conventional PCI devices on bus 05.
 >>>
 >>> When we scan bus 05, we do this:
 >>>
 >>>    pci_scan_child_bus_extend(bus=05)
 >>>      for (devfn = 0; devfn < 0x100; devfn += 8)
 >>>        pci_scan_slot(05, 00.0)
 >>>          pci_scan_single_device
 >>>            pci_scan_device(05, 00.0)           # fails; no 05:00.0
 >>>        pci_scan_slot(05, 01.0)
 >>>          only_one_child(bus=05)
 >>>            parent = 00:11.0
 >>>            pci_pcie_type(00:11.0) == ROOT_PORT # returns true
 >>>
 >>> Since only_one_child() sees that 00:11.0 is a Root Port, we give up
 >>> before we even get to the PCI_SCAN_ALL_PCIE_DEVS test.
 >>>
 >>> I *think* something like the patch below should make this work if you
 >>> use the "pci=pcie_scan_all" parameter.  We have some x86 DMI quirks
 >>> that set PCI_SCAN_ALL_PCIE_DEVS automatically.  I don't know how to do
 >>> something similar on powerpc, but maybe you do?
 >>>
 >>
 >> Hi Bjorn,
 >>
 >> I tested your new patch today. It boots with the boot argument
 >> "pci=pcie_scan_all". Well done! :-)
 >>
 >> It doesn't boot without the boot argument "pci=pcie_scan_all".
 >
 > Thanks for testing that.  I'll merge a similar patch for v4.16.
 >
 > I don't think using "pci=pcie_scan_all" is really an acceptable
 > long-term answer for you, though.  Is there some way we can identify
 > at run-time whether we're on a Nemo system?  If so, we can make this
 > happen automatically.
 >
 > Bjorn
 >

Hi Bjorn,
Hi All,

Olof Johansson has created a patch for us. With this patch, 
"pci=pcie_scan_all" executes automatically on P.A. Semi boards. We don't 
need to add 'pci=pcie_scan_all' to the
kernel boot arguments anymore. Could you please add Olof's patch to the 
official kernel source code?

arch/powerpc/platforms/pasemi/pci.c | 2 ++
  1 file changed, 2 insertions(+)

Thanks,
Christian
diff mbox

Patch

commit 75eaf674066590e79b3e03d32488871fc881ab40
Author: Bjorn Helgaas <bhelgaas@google.com>
Date:   Thu Nov 30 15:22:39 2017 -0600

    PCI: Make PCI_SCAN_ALL_PCIE_DEVS work for Root Ports as well as Downstream
    
    Previously PCI_SCAN_ALL_PCIE_DEVS (set by quirks or the "pci=pcie_scan_all"
    kernel parameter) only affected Switch Downstream Ports, not Root Ports.
    
    Simplify and restructure only_one_child() so PCI_SCAN_ALL_PCIE_DEVS means
    we scan for all possible devices below Root Ports as well as Switch
    Downstream Ports.
    
    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 14e0ea1ff38b..9e57d4ef0c1f 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2217,20 +2217,28 @@  static int only_one_child(struct pci_bus *bus)
 {
 	struct pci_dev *parent = bus->self;
 
-	if (!parent || !pci_is_pcie(parent))
+	if (!parent)
+		return 0;
+
+	/*
+	 * Systems with unusual topologies set PCI_SCAN_ALL_PCIE_DEVS so
+	 * we scan for all possible devices, not just Device 0.
+	 */
+	if (pci_has_flag(PCI_SCAN_ALL_PCIE_DEVS))
 		return 0;
-	if (pci_pcie_type(parent) == PCI_EXP_TYPE_ROOT_PORT)
-		return 1;
 
 	/*
-	 * PCIe downstream ports are bridges that normally lead to only a
-	 * device 0, but if PCI_SCAN_ALL_PCIE_DEVS is set, scan all
-	 * possible devices, not just device 0.  See PCIe spec r3.0,
-	 * sec 7.3.1.
+	 * A PCIe Downstream Port normally leads to a Link with only Device
+	 * 0 on it (PCIe spec r3.1, sec 7.3.1).  As an optimization, scan
+	 * only for Device 0 in that situation.
+	 *
+	 * Checking has_secondary_link is a hack to identify Downstream
+	 * Ports because sometimes Switches are configured such that the
+	 * PCIe Port Type labels are backwards.
 	 */
-	if (parent->has_secondary_link &&
-	    !pci_has_flag(PCI_SCAN_ALL_PCIE_DEVS))
+	if (pci_is_pcie(parent) && parent->has_secondary_link)
 		return 1;
+
 	return 0;
 }