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

[Michael Ellerman <michael@ellerman.id.au>]

[-- Attachment #1: Type: text/plain, Size: 1390 bytes --]

On Fri, 2010-04-16 at 15:34 +0800, Li Yang wrote:
> From: Zhao Chenhui <b26998@freescale.com>
> 
> 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?
 
> @@ -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.

> +			}
> +			cascade_data->index = i;
> +			cascade_data->data = msi;
> +			set_irq_data(virt_msir, (void *)cascade_data);
>  			set_irq_chained_handler(virt_msir, fsl_msi_cascade);
> -			set_irq_chip_data(virt_msir, msi);
>  		}
>  	}
>  

cheers

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]