[PATCH 0/2] sparc32,leon: PCI patches

[PATCH 0/2] sparc32,leon: PCI patches

[Daniel Hellstrom]

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(-)

Re: [PATCH 0/2] sparc32,leon: PCI patches

[David Miller]

From: Daniel Hellstrom <daniel@gaisler.com>
Date: Tue, 26 Apr 2011 10:54:16 +0200

> 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.

Re: [PATCH 0/2] sparc32,leon: PCI patches

[Daniel Hellstrom]

David Miller wrote:

>From: Daniel Hellstrom <daniel@gaisler.com>
>Date: Tue, 26 Apr 2011 10:54:16 +0200
>
>  
>
>>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)

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

>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

Re: [PATCH 0/2] sparc32,leon: PCI patches

[David Miller]

From: Daniel Hellstrom <daniel@gaisler.com>
Date: Thu, 19 May 2011 15:13:14 +0200

> 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.

Re: [PATCH 0/2] sparc32,leon: PCI patches

[Daniel Hellstrom]

David Miller wrote:

>From: Daniel Hellstrom <daniel@gaisler.com>
>Date: Thu, 19 May 2011 15:13:14 +0200
>
>  
>
>>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.
>  
>
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.

>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

Re: [PATCH 0/2] sparc32,leon: PCI patches

[David Miller]

From: Daniel Hellstrom <daniel@gaisler.com>
Date: Fri, 20 May 2011 10:01:11 +0200

> David Miller wrote:
> 
>>From: Daniel Hellstrom <daniel@gaisler.com>
>>Date: Thu, 19 May 2011 15:13:14 +0200
>>
>>  
>>>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.
>>  
> 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.

Re: [PATCH 0/2] sparc32,leon: PCI patches

[Daniel Hellstrom]

David Miller wrote:

>From: Daniel Hellstrom <daniel@gaisler.com>
>Date: Fri, 20 May 2011 10:01:11 +0200
>
>  
>
>>David Miller wrote:
>>
>>    
>>
>>>From: Daniel Hellstrom <daniel@gaisler.com>
>>>Date: Thu, 19 May 2011 15:13:14 +0200
>>>
>>> 
>>>      
>>>
>>>>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.
>>> 
>>>      
>>>
>>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.

>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

Re: [PATCH 0/2] sparc32,leon: PCI patches

[David Miller]

From: Daniel Hellstrom <daniel@gaisler.com>
Date: Fri, 20 May 2011 16:22:33 +0200

> 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.

Re: [PATCH 0/2] sparc32,leon: PCI patches

[Daniel Hellstrom]

David Miller wrote:

>From: Daniel Hellstrom <daniel@gaisler.com>
>Date: Fri, 20 May 2011 16:22:33 +0200
>
>  
>
>>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.
>  
>
Thanks, it means a lot to us.

>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