On 4/19/2010 10:40 AM, Michael Ellerman wrote:

On Fri, 2010-04-16 at 15:34 +0800, Li Yang wrote:
   

quoted

From: Zhao Chenhui<redacted>

In fsl_of_msi_probe(), the virt_msir's chip_data have been stored
the pointer to struct mpic. We add a struct fsl_msi_cascade_data
to store the pointer to struct fsl_msi and msir_index.  Otherwise,
the pointer to struct mpic will be over-written, and will cause
problem when calling eoi() of the irq.
     

I don't quite understand. Do you mean someone was overwriting
handler_data somewhere?
   

The patch at http://patchwork.ozlabs.org/patch/48794/ was overwriting 
the chip_data.  We move the newly added pointer to fsl_msi structure to 
the handler data.

quoted

@@ -309,9 +319,19 @@ static int __devinit fsl_of_msi_probe(struct of_device *dev,
  			break;
  		virt_msir = irq_of_parse_and_map(dev->node, i);
  		if (virt_msir != NO_IRQ) {
-			set_irq_data(virt_msir, (void *)i);
+			cascade_data = kzalloc(
+					sizeof(struct fsl_msi_cascade_data),
+					GFP_KERNEL);
+			if (!cascade_data) {
+				dev_err(&dev->dev,
+					"No memory for MSI cascade data\n");
+				err = -ENOMEM;
+				goto error_out;
     

The error handling in this routine is not great, most of the setup is
not torn down properly in the error paths AFAICS, this adds another.
   

You are right.  Need to add a separate patch to fix all these.

Thanks.
- Leo