Subject: Re: [PATCH] powerpc: Fix irq routing on some PowerMac 32 bits

From: Benjamin Herrenschmidt <[email-masked]>
Date: Wed, 10 Jan 2007 23:13:17 +1100

> Thanks for all your suggestions. I had thought you could point out where
> was wrong :-). Now, I have to trace it myself. I do not have HW
> debugging tools, I only have printk().
> 
> I catch the bug. In your patch:
> 
> +       /* not a root bus, we need to get our parent */
> +       parent = scan_OF_for_pci_bus(bus->parent);
> +       if (parent == NULL)
> +               return NULL;
> +
> +       /* now iterate for children for a match */
> +       np = scan_OF_for_pci_dev(parent, bus->self->devfn);
> +       of_node_put(parent);
> +
> +       /* sanity check */
> +       if (strcmp(np->type, "pci") != 0)
> +               printk(KERN_WARNING "pci: wrong type \"%s\" for bridge %s\n",
> +                      np->type, np->full_name);
> 
> I think you should judge np != NULL before using it.  If np == NULL,
> np->type will cause memory access error. On CDS and some other Freescle
> board, such as 8641HPCN, there is pci bridge, np = NULL , this will
> cause error.
> If you agree, you or I will provide a patch to fix it:-).

While I agree that we should definitely test for NULL and properly fail
instead of crashing, there is still the question of why do you fail at
this point. Do you have PCI devices that don't have corresponding device
nodes ? That is not a problem per-se, I'm just wondering if you have a
device that is supposed to have a node but it's not matching due to a
problem with the "reg" property...

In any case, a patch is definitely needed to properly check for NULL and
should get into 2.6.20, so if you want to do it, it will be much
welcome !

Cheers,
Ben.