Subject: Re: [PATCH 1/4] fsl_msi: fix the conflict of virt_msir's chip_data

From: Li Yang <[email-masked]>
Date: Mon, 19 Apr 2010 12:50:59 +0800

On 4/19/2010 10:40 AM, Michael Ellerman wrote:
> On Fri, 2010-04-16 at 15:34 +0800, Li Yang wrote:
>    
>> From: Zhao Chenhui<[email-masked]>
>>
>> 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.

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