diff mbox series

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 Not Applicable
Headers show
Series SB600 for the Nemo board has non-zero devices on non-root bus | expand

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
Olof Johansson Dec. 2, 2017, 11 p.m. UTC | #3 
On Sat, Dec 02, 2017 at 01:54:41PM +0100, Christian Zigotzky wrote:
> 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?


The below patch, together with Bjorn's, should do it. Christian, can you test
and report back?

I'm guessing it won't do any harm to set this on non-X1000 platforms. My
test system is currently powered down so I can't check.


From a3b390277627b0342c8ccfc16e58679e0d8abdde Mon Sep 17 00:00:00 2001
From: Olof Johansson <olof@lixom.net>
Date: Sat, 2 Dec 2017 14:56:36 -0800
Subject: [PATCH] powerpc/pasemi: set PCI_SCAN_ALL_PCI_DEVS

Needed on Amiga X1000 with SB600.

Reported-by: Christian Zigotzky <chzigotzky@xenosoft.de>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Olof Johansson <olof@lixom.net>
---
 arch/powerpc/platforms/pasemi/pci.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/powerpc/platforms/pasemi/pci.c b/arch/powerpc/platforms/pasemi/pci.c
index 5ff6108..ea54ed2 100644
--- a/arch/powerpc/platforms/pasemi/pci.c
+++ b/arch/powerpc/platforms/pasemi/pci.c
@@ -224,6 +224,8 @@ void __init pas_pci_init(void)
 		return;
 	}
 
+	pci_set_flag(PCI_SCAN_ALL_PCIE_DEVS):
+
 	for (np = NULL; (np = of_get_next_child(root, np)) != NULL;)
 		if (np->name && !strcmp(np->name, "pxp") && !pas_add_bridge(np))
 			of_node_get(np);
Olof Johansson Dec. 2, 2017, 11:02 p.m. UTC | #4 
On Sat, Dec 2, 2017 at 3:00 PM, Olof Johansson <olof@lixom.net> wrote:
> On Sat, Dec 02, 2017 at 01:54:41PM +0100, Christian Zigotzky wrote:
>> 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?
>
>
> The below patch, together with Bjorn's, should do it. Christian, can you test
> and report back?
>
> I'm guessing it won't do any harm to set this on non-X1000 platforms. My
> test system is currently powered down so I can't check.
>
>
> From a3b390277627b0342c8ccfc16e58679e0d8abdde Mon Sep 17 00:00:00 2001
> From: Olof Johansson <olof@lixom.net>
> Date: Sat, 2 Dec 2017 14:56:36 -0800
> Subject: [PATCH] powerpc/pasemi: set PCI_SCAN_ALL_PCI_DEVS
>
> Needed on Amiga X1000 with SB600.
>
> Reported-by: Christian Zigotzky <chzigotzky@xenosoft.de>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Signed-off-by: Olof Johansson <olof@lixom.net>
> ---
>  arch/powerpc/platforms/pasemi/pci.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/arch/powerpc/platforms/pasemi/pci.c b/arch/powerpc/platforms/pasemi/pci.c
> index 5ff6108..ea54ed2 100644
> --- a/arch/powerpc/platforms/pasemi/pci.c
> +++ b/arch/powerpc/platforms/pasemi/pci.c
> @@ -224,6 +224,8 @@ void __init pas_pci_init(void)
>                 return;
>         }
>
> +       pci_set_flag(PCI_SCAN_ALL_PCIE_DEVS):

Typo, should be ';', not ':'. I obviously didn't even try compiling this. :)


-Olof
Christian Zigotzky Dec. 3, 2017, 9:43 a.m. UTC | #5 
On 3. Dec 2017, at 00:02, Olof Johansson <olof@lixom.net> wrote:
> 
>> On Sat, Dec 2, 2017 at 3:00 PM, Olof Johansson <olof@lixom.net> wrote:
>> 
>> The below patch, together with Bjorn's, should do it. Christian, can you test
>> and report back?
>> 
>> I'm guessing it won't do any harm to set this on non-X1000 platforms. My
>> test system is currently powered down so I can't check.
>> 
>> 
>> From a3b390277627b0342c8ccfc16e58679e0d8abdde Mon Sep 17 00:00:00 2001
>> From: Olof Johansson <olof@lixom.net>
>> Date: Sat, 2 Dec 2017 14:56:36 -0800
>> Subject: [PATCH] powerpc/pasemi: set PCI_SCAN_ALL_PCI_DEVS
>> 
>> Needed on Amiga X1000 with SB600.
>> 
>> Reported-by: Christian Zigotzky <chzigotzky@xenosoft.de>
>> Cc: Bjorn Helgaas <bhelgaas@google.com>
>> Signed-off-by: Olof Johansson <olof@lixom.net>
>> ---
>> arch/powerpc/platforms/pasemi/pci.c | 2 ++
>> 1 file changed, 2 insertions(+)
>> 
>> diff --git a/arch/powerpc/platforms/pasemi/pci.c b/arch/powerpc/platforms/pasemi/pci.c
>> index 5ff6108..ea54ed2 100644
>> --- a/arch/powerpc/platforms/pasemi/pci.c
>> +++ b/arch/powerpc/platforms/pasemi/pci.c
>> @@ -224,6 +224,8 @@ void __init pas_pci_init(void)
>>                return;
>>        }
>> 
>> +       pci_set_flag(PCI_SCAN_ALL_PCIE_DEVS):
> 
> Typo, should be ';', not ':'. I obviously didn't even try compiling this. :)
> 
> 
> -Olof

Hi Olof,

Thanks a lot for your patch! I will test it on Wednesday.

Cheers,
Christian
Michael Ellerman Dec. 6, 2017, 12:44 p.m. UTC | #6 
Olof Johansson <olof@lixom.net> writes:

> On Sat, Dec 02, 2017 at 01:54:41PM +0100, Christian Zigotzky wrote:
>> 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?
>
>
> The below patch, together with Bjorn's, should do it. Christian, can you test
> and report back?
>
> I'm guessing it won't do any harm to set this on non-X1000 platforms. My
> test system is currently powered down so I can't check.

My pasemi board had been powered off for a while and when I turned it
back on something popped, the power supply blew up and tripped a
breaker.

So I also can't test this, at least for now, until I get my "allowed to
use hardware" license back from my colleagues in the office.

cheers
Olof Johansson Dec. 6, 2017, 3:53 p.m. UTC | #7 
On Wed, Dec 6, 2017 at 4:44 AM, Michael Ellerman <mpe@ellerman.id.au> wrote:
> Olof Johansson <olof@lixom.net> writes:
>
>> On Sat, Dec 02, 2017 at 01:54:41PM +0100, Christian Zigotzky wrote:
>>> 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?
>>
>>
>> The below patch, together with Bjorn's, should do it. Christian, can you test
>> and report back?
>>
>> I'm guessing it won't do any harm to set this on non-X1000 platforms. My
>> test system is currently powered down so I can't check.
>
> My pasemi board had been powered off for a while and when I turned it
> back on something popped, the power supply blew up and tripped a
> breaker.
>
> So I also can't test this, at least for now, until I get my "allowed to
> use hardware" license back from my colleagues in the office.

Ouch. Sounds like it's the PSU not the board. Hopefully there was no
board damage, let me know if you need a replacement though and I'll
see what I can find.


-Olof
Michael Ellerman Dec. 8, 2017, 11:57 a.m. UTC | #8 
Olof Johansson <olof@lixom.net> writes:
> On Wed, Dec 6, 2017 at 4:44 AM, Michael Ellerman <mpe@ellerman.id.au> wrote:
>> Olof Johansson <olof@lixom.net> writes:
>>>
>>> The below patch, together with Bjorn's, should do it. Christian, can you test
>>> and report back?
>>>
>>> I'm guessing it won't do any harm to set this on non-X1000 platforms. My
>>> test system is currently powered down so I can't check.
>>
>> My pasemi board had been powered off for a while and when I turned it
>> back on something popped, the power supply blew up and tripped a
>> breaker.
>>
>> So I also can't test this, at least for now, until I get my "allowed to
>> use hardware" license back from my colleagues in the office.
>
> Ouch. Sounds like it's the PSU not the board. Hopefully there was no
> board damage, let me know if you need a replacement though and I'll
> see what I can find.

Thanks. Yeah the power supply is toast. One of the guys is going to see
if he can fix it.

Will try and get the board going, will let you know if it's dead too.

cheers
Christian Zigotzky March 16, 2018, 12:10 p.m. UTC | #9 
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 series

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