diff mbox

PCI changes 2.6.26 => 2.6.28

Message ID 49F07A15.9010006@mlbassoc.com (mailing list archive)
State Not Applicable, archived
Delegated to: Kumar Gala
Headers show

Commit Message

Gary Thomas April 23, 2009, 2:24 p.m. UTC 
Kumar Gala wrote:
> 
> On Apr 21, 2009, at 6:45 PM, Gary Thomas wrote:
> 
>> Kumar Gala wrote:
>>>
>>> On Apr 21, 2009, at 6:00 PM, Gary Thomas wrote:
>>>
>>>>
>>>> I found the difference - in 2.6.28 the inbound/outbound windows
>>>> don't seem to be set up at all.  In 2.6.26, the function
>>>> 'fsl_add_bridge'
>>>> was common among architectures and ended up calling 'setup_pci_atmu'
>>>> which created those mappings.  In 2.6.28, the 83xx PCI setup code
>>>> has been refactored.  It uses 'mpc83xx_add_bridge' instead of
>>>> 'fsl_add_bridge' and 'setup_pci_atmu' is not called at all :-(
>>>>
>>>> I'm sure this is the problem.
>>>
>>> Looking at a diff between 2.6.26 and .28 I don't see the 83xx pci code
>>> calling setup_pci_atmu().
>>
>> It did not directly; it called 'fsl_add_bridge' which in turn called
>> 'setup_pci_atmu'
> 
> Don't ever see 83xx boards calling fsl_add_bridge -- that is 85xx/86xx
> only in 83xx.

Sorry, you're correct.  I got lost looking through the code :-(
No matter, none of that code seems to be relevant to the change
in behaviour.

>> I modified the 2.6.28 driver to also call this function.  Now the
>> inbound/outbound windows are set up, but the bridge still has
>> no allocations, so the problem remains.
>>
>> I need to move on; I may just live with sliding the PCI space
>> up for now (doesn't really hurt anything, just seems like a hack)
> 
> It is and you are glossing over a real bug.

I have found the culprit - in arch/powerpc/kernel/pci_32.c

static void
fixup_hide_host_resource_fsl(struct pci_dev *dev)
{
	int i, class = dev->class >> 8;

#if 0
	if ((class == PCI_CLASS_PROCESSOR_POWERPC ||
	     class == PCI_CLASS_BRIDGE_OTHER) &&
#else
        if ((class == PCI_CLASS_PROCESSOR_POWERPC) &&
#endif
		(dev->hdr_type == PCI_HEADER_TYPE_NORMAL) &&
		(dev->bus->parent == NULL)) {
		for (i = 0; i < DEVICE_COUNT_RESOURCE; i++) {
			dev->resource[i].start = 0;
			dev->resource[i].end = 0;
			dev->resource[i].flags = 0;
		}
	}
}
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MOTOROLA, PCI_ANY_ID, fixup_hide_host_resource_fsl);
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_FREESCALE, PCI_ANY_ID, fixup_hide_host_resource_fsl);

This function is now (the #if 0 case is in 2.6.28) tossing out
the memory resources used by the PCI bridge itself.  This makes
everything fall over, at least on my 834x platform.

This change was applied 2008-10-08, but it seems incorrect on the 834x.

> Are you using u-boot to boot?  If so is the board port public?

My systems use RedBoot (I'm the original author of RedBoot, so one would
expect that).  At this moment, the code isn't public, sorry.

Comments

Kumar Gala April 23, 2009, 6:47 p.m. UTC | #1 
On Apr 23, 2009, at 9:24 AM, Gary Thomas wrote:

> I have found the culprit - in arch/powerpc/kernel/pci_32.c
>
> static void
> fixup_hide_host_resource_fsl(struct pci_dev *dev)
> {
> 	int i, class = dev->class >> 8;
>
> #if 0
> 	if ((class == PCI_CLASS_PROCESSOR_POWERPC ||
> 	     class == PCI_CLASS_BRIDGE_OTHER) &&
> #else
>        if ((class == PCI_CLASS_PROCESSOR_POWERPC) &&
> #endif
> 		(dev->hdr_type == PCI_HEADER_TYPE_NORMAL) &&
> 		(dev->bus->parent == NULL)) {
> 		for (i = 0; i < DEVICE_COUNT_RESOURCE; i++) {
> 			dev->resource[i].start = 0;
> 			dev->resource[i].end = 0;
> 			dev->resource[i].flags = 0;
> 		}
> 	}
> }
> DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MOTOROLA, PCI_ANY_ID,  
> fixup_hide_host_resource_fsl);
> DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_FREESCALE, PCI_ANY_ID,  
> fixup_hide_host_resource_fsl);
>
> This function is now (the #if 0 case is in 2.6.28) tossing out
> the memory resources used by the PCI bridge itself.  This makes
> everything fall over, at least on my 834x platform.
>
> This change was applied 2008-10-08, but it seems incorrect on the  
> 834x.

Its not.  The PCI subsystem shouldn't be allocating or seeing the PHBs  
resources.

>> Are you using u-boot to boot?  If so is the board port public?
>
> My systems use RedBoot (I'm the original author of RedBoot, so one  
> would
> expect that).  At this moment, the code isn't public, sorry.

Ok.  Not sure if RedBoot has a simple memory dump command, but if you  
can dump the IMMR registers for PCI (0x8400 - IOS and 0x8500 - PCI1).   
(I'm assuming PCI1 is the one you are using).  From IOS I wanted the  
outbound registers, for PCI1 the inbound ATU registers.

- k
Gary Thomas April 23, 2009, 10:27 p.m. UTC | #2 
Kumar Gala wrote:
> 
> On Apr 23, 2009, at 9:24 AM, Gary Thomas wrote:
> 
>> I have found the culprit - in arch/powerpc/kernel/pci_32.c
>>
>> static void
>> fixup_hide_host_resource_fsl(struct pci_dev *dev)
>> {
>>     int i, class = dev->class >> 8;
>>
>> #if 0
>>     if ((class == PCI_CLASS_PROCESSOR_POWERPC ||
>>          class == PCI_CLASS_BRIDGE_OTHER) &&
>> #else
>>        if ((class == PCI_CLASS_PROCESSOR_POWERPC) &&
>> #endif
>>         (dev->hdr_type == PCI_HEADER_TYPE_NORMAL) &&
>>         (dev->bus->parent == NULL)) {
>>         for (i = 0; i < DEVICE_COUNT_RESOURCE; i++) {
>>             dev->resource[i].start = 0;
>>             dev->resource[i].end = 0;
>>             dev->resource[i].flags = 0;
>>         }
>>     }
>> }
>> DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MOTOROLA, PCI_ANY_ID,
>> fixup_hide_host_resource_fsl);
>> DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_FREESCALE, PCI_ANY_ID,
>> fixup_hide_host_resource_fsl);
>>
>> This function is now (the #if 0 case is in 2.6.28) tossing out
>> the memory resources used by the PCI bridge itself.  This makes
>> everything fall over, at least on my 834x platform.
>>
>> This change was applied 2008-10-08, but it seems incorrect on the 834x.
> 
> Its not.  The PCI subsystem shouldn't be allocating or seeing the PHBs
> resources.
> 
>>> Are you using u-boot to boot?  If so is the board port public?
>>
>> My systems use RedBoot (I'm the original author of RedBoot, so one would
>> expect that).  At this moment, the code isn't public, sorry.
> 
> Ok.  Not sure if RedBoot has a simple memory dump command, but if you
> can dump the IMMR registers for PCI (0x8400 - IOS and 0x8500 - PCI1). 
> (I'm assuming PCI1 is the one you are using).  From IOS I wanted the

I don't think any of this matters.  It turns out that even
the 2.6.26 kernel fails on an identical board with a newer
revision of the 8347 chip.  I'm sure that the problem is
that the Coral-P fails when mapped to 0 (PCI relative).

I couldn't find another reliable way to get the Coral-P assigned
an address other than 0, so I'm happy accepting the work around
of allowing the kernel to map those windows, even if it's not
necessary.

For completeness, here are the values you asked for:
pcilawbar0     : 0xc0000000  -1073741824
pcilawar       : 0x8000001b  -2147483621
pcilawbar1     : 0xb8000000  -1207959552
pcilawar1      : 0x80000013  -2147483629

8347>md 0xff008400
ff008400 : 00000000 00000000 000c0000 00000000  ................
ff008410 : 800f0000 00000000 00000000 00000000  ................
ff008420 : 000b8000 00000000 c00fff00 00000000  ................
ff008430 : 00000000 00000000 00000000 00000000  ................
ff008440 : 00000000 00000000 00000000 00000000  ................
ff008450 : 00000000 00000000 00000000 00000000  ................
ff008460 : 00000000 00000000 00000000 00000000  ................
ff008470 : 00000000 00000000 00000000 00000000  ................
ff008480 : 00000000 00000000 00000000 00000000  ................
ff008490 : 00000000 00000000 00000000 00000000  ................
ff0084a0 : 00000000 00000000 00000000 00000000  ................
ff0084b0 : 00000000 00000000 00000000 00000000  ................
ff0084c0 : 00000000 00000000 00000000 00000000  ................
ff0084d0 : 00000000 00000000 00000000 00000000  ................
ff0084e0 : 00000000 00000000 00000000 00000000  ................
ff0084f0 : 00000001 00010006 00000000 00000000  ................

8347>md 0xff008500
ff008500 : 00000000 00000000 00000000 00000000  ................
ff008510 : 00000000 00000000 00000000 00000000  ................
ff008520 : 00000001 00000000 00000001 00000000  ................
ff008530 : 00000000 00000000 00000000 00000000  ................
ff008540 : 00000000 00000000 00000000 00000000  ................
ff008550 : 00000000 00000000 00000000 00000000  ................
ff008560 : a005501a 00000000 00000000 00000000  ..P.............
ff008570 : 00000000 00000000 00000000 00000000  ................
ff008580 : 00000000 00000000 00000000 00000000  ................
ff008590 : 00000000 00000000 00000000 00000000  ................
ff0085a0 : 00000000 00000000 00000000 00000000  ................
ff0085b0 : 00000000 00000000 00000000 00000000  ................
ff0085c0 : 00000000 00000000 00000000 00000000  ................
ff0085d0 : 00000000 00000000 00000000 00000000  ................
ff0085e0 : 00000000 00000000 00000000 00000000  ................
ff0085f0 : 00000000 00000000 00000000 00000000  ................
Benjamin Herrenschmidt April 27, 2009, 1:17 p.m. UTC | #3 
On Thu, 2009-04-23 at 16:27 -0600, Gary Thomas wrote:

> I don't think any of this matters.  It turns out that even
> the 2.6.26 kernel fails on an identical board with a newer
> revision of the 8347 chip.  I'm sure that the problem is
> that the Coral-P fails when mapped to 0 (PCI relative).

There are macro that the PCI code uses to define the min address to
assign to devices but it's broken for us as it doesn't account
for the kind of remapping we do... I've been wanting to fix those
for some time but didn't get a chance yet, that would sort this out.

Ben.
diff mbox

Patch

diff --git a/arch/powerpc/kernel/pci_32.c b/arch/powerpc/kernel/pci_32.c
index 174b77e..a848c63 100644
--- a/arch/powerpc/kernel/pci_32.c
+++ b/arch/powerpc/kernel/pci_32.c
@@ -54,11 +54,12 @@  LIST_HEAD(hose_list);
 static int pci_bus_count;
 
 static void
-fixup_hide_host_resource_fsl(struct pci_dev* dev)
+fixup_hide_host_resource_fsl(struct pci_dev *dev)
 {
 	int i, class = dev->class >> 8;
 
-	if ((class == PCI_CLASS_PROCESSOR_POWERPC) &&
+	if ((class == PCI_CLASS_PROCESSOR_POWERPC ||
+	     class == PCI_CLASS_BRIDGE_OTHER) &&
 		(dev->hdr_type == PCI_HEADER_TYPE_NORMAL) &&
 		(dev->bus->parent == NULL)) {
 		for (i = 0; i < DEVICE_COUNT_RESOURCE; i++) {