Skip to contentSkip to conversation

sparc32,leon: PCI patches

PATCH Superseded Daniel Hellstrom 2 participants 9 messages Apr 26, 2011 – May 23, 2011

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

cover sparc32,leon: PCI patches

Hello Dave,

The patches in this series must be applied ontop of the genriq patches sent previously.

The patches implement PCI for the LEON architecture. The LEON supports different PCI Host controllers, I have tried to collect common parts in leon_pci.c and host drivers will be named leon_pci_*.c.

There is no BIOS or bootloader initializing PCI for use in comparison to the PCIC-SPARC32 code. LEON rely on the Linux generic resource & IRQ code to probe and setup the bus for us, drivers/pci/Makefile is dependent on CONFIG_SPARC_LEON in order not to brake PCIC. PCIC can not be defined at the same time as the LEON PCI layer, so the CONFIG_PCI ifdeffs has been replaced with CONFIG_PCI && !CONFIG_SPARC_LEON.

Thanks,
Daniel

 arch/sparc/Kconfig                  |    7 +
 arch/sparc/include/asm/leon.h       |    3 +
 arch/sparc/include/asm/pci_32.h     |    5 +
 arch/sparc/include/asm/pci_leon.h   |   36 ++
 arch/sparc/include/asm/pcic.h       |    2 +-
 arch/sparc/kernel/Makefile          |    5 +
 arch/sparc/kernel/entry.S           |    2 +-
 arch/sparc/kernel/leon_kernel.c     |   15 +
 arch/sparc/kernel/leon_pci.c        |  237 +++++++++
 arch/sparc/kernel/leon_pci_grpci2.c |  905 
+++++++++++++++++++++++++++++++++++
 drivers/pci/Makefile                |    1 +
 11 files changed, 1216 insertions(+), 2 deletions(-)
#1 lore raw

From: Daniel Hellstrom <[email protected]>
Date: Tue, 26 Apr 2011 10:54:16 +0200

Quoted 2 lines

There is no BIOS or bootloader initializing PCI for use in comparison to the PCIC-SPARC32 code. LEON rely on the Linux generic resource &

Well since the kernel doesn't support Leon/PCI yet you still have an opportunity to "fix" this.

This is going to create an enormous huge inconsistency between Leon and other SPARC PCI implementations, so please avoid this special case.

Create the device tree nodes, via whatever means possible, and that way you won't have to implement anything special in the kernel.

I'm not applying these patches.

#2 lore raw
In reply to Daniel Hellstrom
Quoted 9 lines · David Miller

From: Daniel Hellstrom <[email protected]>
Date: Tue, 26 Apr 2011 10:54:16 +0200

Quoted 4 lines

There is no BIOS or bootloader initializing PCI for use in comparison to the PCIC-SPARC32 code. LEON rely on the Linux generic resource &

I mean that by looking at the pcic.c code I came to the conclusion that:
* the "interrupt line" register is not set by Linux
* BAR / Bridge-Window resources are not set by Linux
(pci_assign_unassigned_resources not called)

Quoted 4 lines

Well since the kernel doesn't support Leon/PCI yet you still have an opportunity to "fix" this.

Setting up this in the bootloader, means a lot of work. I need to come up with a PCI Library, that not only scans the PCI bus but also configures and sizes all resources, which is a huge job. I hope that Linux could do this since many other platforms rely on the generic Linux code in drivers/pci to set this up (drivers/pci/Makefile):

#
# Some architectures use the generic PCI setup functions #
obj-$(CONFIG_X86) += setup-bus.o obj-$(CONFIG_ALPHA) += setup-bus.o setup-irq.o obj-$(CONFIG_ARM) += setup-bus.o setup-irq.o obj-$(CONFIG_UNICORE32) += setup-bus.o setup-irq.o obj-$(CONFIG_PARISC) += setup-bus.o obj-$(CONFIG_SUPERH) += setup-bus.o setup-irq.o obj-$(CONFIG_PPC) += setup-bus.o obj-$(CONFIG_MIPS) += setup-bus.o setup-irq.o obj-$(CONFIG_X86_VISWS) += setup-irq.o obj-$(CONFIG_MN10300) += setup-bus.o obj-$(CONFIG_MICROBLAZE) += setup-bus.o obj-$(CONFIG_TILE) += setup-bus.o setup-irq.o +obj-$(CONFIG_SPARC_LEON) += setup-bus.o setup-irq.o

The PowerPC for example assign resources (arch/powerpc/kernel/pci-common.c), and PowerPC may also have a OpenBoot loader:

    /* Now, if the platform didn't decide to blindly trust the firmware,
     * we proceed to assigning things that were left unassigned
     */
    if (!(ppc_pci_flags & PPC_PCI_PROBE_ONLY)) {
        pr_debug("PCI: Assigning unassigned resources...\n");
        pci_assign_unassigned_resources();
    }
Quoted 8 lines

This is going to create an enormous huge inconsistency between Leon and other SPARC PCI implementations, so please avoid this special case.

Create the device tree nodes, via whatever means possible, and that way you won't have to implement anything special in the kernel.

I'm not sure that I understand exactly what consequences there are, or what the "PCI device tree" can provide for us. I can see some drivers use it as a source of "additional" device properties, which we want to support in the long run of course.

The pcic seems falls back to no device node when the PROM node is not found, should I implement similar code?

        node = pdev_to_pnode(&pcic->pbm, dev);
        if(node = 0)
            node = -1;
        ...
        pcp->prom_node = of_find_node_by_phandle(node);

As many platforms use the Linux generic ways to setup IRQ and other resources in a clean way and LEON do it without affecting PCIC code, we can add PROM device nodes on the user's demand but not for all PCI devices (PCI scanning not required), and the huge amount of work to implement PCI device scanning/setup in the PROM besides we need multi-bus PCI achitecture as well... I kindly ask you to reconsider this.

Either way, I need to update the patches due to a build error for a specific config that Jan Andersson discovered.

Also, in pcic.c I wonder if the PROM loader writes the interrupt routing in the IRQ Line as the PCI standard suggests, the below comment does not say the writer checked that... that could perhaps be the solution for the complicated system specific interrupt routing initialization done in pcic... but that is another story.

From pcic.c:
/*
* I studied different documents and many live PROMs both from 2.30
* family and 3.xx versions. I came to the amazing conclusion: there is
* absolutely no way to route interrupts in IIep systems relying on
* information which PROM presents. We must hardcode interrupt routing
* schematics. And this actually sucks. -- zaitcev 1999/05/12
*
* ...

Best Regards,
Daniel

#3 lore raw
In reply to Daniel Hellstrom

From: Daniel Hellstrom <[email protected]>
Date: Thu, 19 May 2011 15:13:14 +0200

Quoted 3 lines

The PowerPC for example assign resources (arch/powerpc/kernel/pci-common.c), and PowerPC may also have a OpenBoot loader:

On some powerpc systems, like sparc64 Niagara, you can't blindly go poking around the PCI config space at all and must rely completely and entirely upon the OpenBoot device tree.

The support you see there is for older and embedded systems.

#4 lore raw
In reply to Daniel Hellstrom
Quoted 11 lines · David Miller

From: Daniel Hellstrom <[email protected]>
Date: Thu, 19 May 2011 15:13:14 +0200

Quoted 5 lines

The PowerPC for example assign resources (arch/powerpc/kernel/pci-common.c), and PowerPC may also have a OpenBoot loader:

Quoted 5 lines

On some powerpc systems, like sparc64 Niagara, you can't blindly go poking around the PCI config space at all and must rely completely and entirely upon the OpenBoot device tree.

I see, that may be because IRQ routing or resource allocation is done in a non-standard way I guess, perhaps because of bugs on some of the motherboards. On all LEON PCI systems I have tested IRQ routing is done the standard way and resource allocation need no special quirks, even though linux provides means for fixing stuff like that in some degree on LEON we currently need no fixups. It just runs beautiful and linux sets everything up for us :)

I'm not sure, but I think on x86 the BIOS setup is sometimes discarded and linux initializes some resources itself.

Quoted 3 lines

The support you see there is for older and embedded systems.

LEON is a embedded system.

I will add support for PROM nodes in LEON PCI layer and fix the build error Jan discovered, I have to do that anyways.

Daniel

#5 lore raw
In reply to Daniel Hellstrom

From: Daniel Hellstrom <[email protected]>
Date: Fri, 20 May 2011 10:01:11 +0200

Quoted 11 lines
Quoted 9 lines · David Miller

From: Daniel Hellstrom <[email protected]>
Date: Thu, 19 May 2011 15:13:14 +0200

Quoted 4 lines

The PowerPC for example assign resources (arch/powerpc/kernel/pci-common.c), and PowerPC may also have a OpenBoot loader:

Quoted 7 lines
Quoted 4 lines

On some powerpc systems, like sparc64 Niagara, you can't blindly go poking around the PCI config space at all and must rely completely and entirely upon the OpenBoot device tree.

I see, that may be because IRQ routing or resource allocation is done in a non-standard way I guess, perhaps because of bugs on some of the motherboards.

Or because PCI accesses have to go through a hypervisor.

The point is that having it all setup in OpenBoot is a great abstraction because you don't have to care.

It also allows things like being able to disable devices with things like "pcia-probe-list" and "pcib-probe-list" properties in the PCI host controller node, which we end up honoring on sparc64 simply because we never directly probe PCI space to probe devices.

#6 lore raw
In reply to Daniel Hellstrom
Quoted 27 lines · David Miller

From: Daniel Hellstrom <[email protected]>
Date: Fri, 20 May 2011 10:01:11 +0200

Quoted 22 lines
Quoted 18 lines · David Miller

From: Daniel Hellstrom <[email protected]>
Date: Thu, 19 May 2011 15:13:14 +0200

Quoted 6 lines

The PowerPC for example assign resources (arch/powerpc/kernel/pci-common.c), and PowerPC may also have a OpenBoot loader:

On some powerpc systems, like sparc64 Niagara, you can't blindly go poking around the PCI config space at all and must rely completely and entirely upon the OpenBoot device tree.

Quoted 9 lines
Quoted 5 lines

I see, that may be because IRQ routing or resource allocation is done in a non-standard way I guess, perhaps because of bugs on some of the motherboards.

Or because PCI accesses have to go through a hypervisor.

Aaah. Its a pity sparc32 doesn't have a hypervisor, we could do really cool stuff with that.

Quoted 9 lines

The point is that having it all setup in OpenBoot is a great abstraction because you don't have to care.

It also allows things like being able to disable devices with things like "pcia-probe-list" and "pcib-probe-list" properties in the PCI host controller node, which we end up honoring on sparc64 simply because we never directly probe PCI space to probe devices.

I agree that it is a better solution in that case and in other cases too. The problem will be for me to write a PCI Library in the limited context of the PROM, it is much more complex that writing a PCI host driver. The non-free PCI specification alone is heavy, compatibility between PCI revisions and then there is the bridge specification... ugg, I will probably have to leave that to the next generation :)

Daniel

#7 lore raw
In reply to Daniel Hellstrom

From: Daniel Hellstrom <[email protected]>
Date: Fri, 20 May 2011 16:22:33 +0200

Quoted 7 lines

I agree that it is a better solution in that case and in other cases too. The problem will be for me to write a PCI Library in the limited context of the PROM, it is much more complex that writing a PCI host driver. The non-free PCI specification alone is heavy, compatibility between PCI revisions and then there is the bridge specification... ugg, I will probably have to leave that to the next generation :)

I guess if it's really that bad we can merge your PCI changes for now, please resubmit them.

#8 lore raw
In reply to Daniel Hellstrom
Quoted 15 lines · David Miller

From: Daniel Hellstrom <[email protected]>
Date: Fri, 20 May 2011 16:22:33 +0200

Quoted 9 lines

I agree that it is a better solution in that case and in other cases too. The problem will be for me to write a PCI Library in the limited context of the PROM, it is much more complex that writing a PCI host driver. The non-free PCI specification alone is heavy, compatibility between PCI revisions and then there is the bridge specification... ugg, I will probably have to leave that to the next generation :)

Quoted 4 lines

I guess if it's really that bad we can merge your PCI changes for now, please resubmit them.

Thanks, it means a lot to us.

Quoted 5 lines

Julian:
When I looked at this earlier, I was thinking that we might want to introduce a new KConfig variable, maybe CONFIG_LEON_PCI, to hide all this stuff behind so we don't have to do any complicated ifdefery or change any existing ifdefs.

I will add a PCIC and a LEON_PCI Kconfig option that depends on !LEON/LEON and PCI, as julian/sam suggests. I'm busy at the moment with other stuff it a pity these problem wasn't brought forward until now, is the merge window open until the end of the week?

Thanks,
Daniel