Re: [PATCH v4]PPC4xx: Adding PCI(E) MSI support

On Wed, Jun 29, 2011 at 09:15:28AM +1000, Benjamin Herrenschmidt wrote:

On Tue, 2011-06-28 at 17:31 -0500, Ayman El-Khashab wrote:

quoted

quoted

quoted

+static int ppc4xx_setup_pcieh_hw(struct platform_device *dev,
+                            struct resource res, struct

ppc4xx_msi *msi)

quoted

quoted

+{
+

<snip>

quoted

quoted

+
+   msi->msi_dev = of_find_node_by_name(NULL, "ppc4xx-msi");
+   if (msi->msi_dev)
+           return -ENODEV;

This does not look correct. I guess it should probably read 

if (!msi->msi_dev) .....

Indeed, that looks bogus. Rupjyoti, please test and send fixes if
necessary, obviously this code has not been tested.

This is not part of the bits I fixed up so I looks to me like the
original patch was wrong (and thus obviously untested !!!)

Looking back through the mailing list, there have been
various incarnations of this patch to add MSI support to the
44x.  Every one that I looked at had this same line of code
in it so I am not sure they worked.  In any case I am trying
to make it work on my system (which is how I found the bug).
When I enable the "sdr-base" line in the MSI section of my
dts, it just reboots continuosly right after "Loading Device
Tree ....".  I tried renaming it to "msi-sdr-base" just in
case there was a conflict (since it is reading through the
entire tree) but that did not help.  If I understand
correctly, the ppc4xx_msi_probe function must be executing
very early since I suspect something in setup_pcieh_hw is 
what causes it to fail.  

ayman