Re: [RESEND] [PATCH 1/5] omap2/3/4: mailbox: remove compiler warning

[Omar Ramirez Luna <omar.ramirez@ti.com>]

On 2/22/2010 5:41 PM, Anna, Suman wrote:
>  From 657358cd06a3f20bef2472945991749bacef12e6 Mon Sep 17 00:00:00 2001
> From: Suman Anna<s-anna@ti.com>
> Date: Mon, 25 Jan 2010 18:27:21 -0600
> Subject: [PATCH] omap2/3/4: mailbox: remove compiler warning
>
> Remove a compiler warning in device-specific
> mailbox module.
>
> Signed-off-by: Suman Anna<s-anna@ti.com>
> ---
>   arch/arm/mach-omap2/mailbox.c |    5 ++++-
>   1 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/mailbox.c b/arch/arm/mach-omap2/mailbox.c
> index 2c9fd1c..8b5fc2a 100644
> --- a/arch/arm/mach-omap2/mailbox.c
> +++ b/arch/arm/mach-omap2/mailbox.c
> @@ -419,8 +419,11 @@ static int __devinit omap2_mbox_probe(struct platform_device *pdev)
>   #endif
>   	return 0;
>
> +#if defined(CONFIG_ARCH_OMAP2420) /* IVA */
>   err_iva1:
> -	omap_mbox_unregister(&mbox_dsp_info);
> +	if (cpu_is_omap2420())
> +		omap_mbox_unregister(&mbox_dsp_info);
> +#endif

why not moving omap_mbox_unregister to the block containing the ifdefs? 
at least you won't be adding an ifdef and if(cpu_xx)

from mailbox.c:

> #if defined(CONFIG_ARCH_OMAP2420) /* IVA */
> 	if (cpu_is_omap2420()) {
> 		/* IVA IRQ */
> 		res = platform_get_resource(pdev, IORESOURCE_IRQ, 1);
> 		if (unlikely(!res)) {
> 			dev_err(&pdev->dev, "invalid irq resource\n");
> 			ret = -ENODEV;
> 			goto err_iva1;

this is not even needed because at this point mailbox has not been 
registered (unless i'm missing something)

> 		}
> 		mbox_iva_info.irq = res->start;
> 		ret = omap_mbox_register(&pdev->dev, &mbox_iva_info);
> 		if (ret)
> 			goto err_iva1;

this should be omap_mbox_unregister... I haven't looked to the code to 
comment if its needed to call omap_mbox_unregister when 
omap_mbox_register fails.

> 	}
> #endif

Regards,

- omar