Re: [PATCH 02/10] omap mailbox: add initial omap3 support

From: Felipe Balbi
Date: Fri Jan 16 2009 - 10:44:22 EST


Hi Hiroshi,

On Fri, Jan 16, 2009 at 10:27:11AM +0200, Hiroshi DOYU wrote:
> static inline void omap_init_mbox(void)
> {
> + if (cpu_is_omap2420()) {
> + mbox_device.num_resources = ARRAY_SIZE(omap2_mbox_resources);
> + mbox_device.resource = omap2_mbox_resources;
> + } else if (cpu_is_omap3430()) {
> + mbox_device.num_resources = ARRAY_SIZE(omap3_mbox_resources);
> + mbox_device.resource = omap3_mbox_resources;
> + } else {

how about a pr_info() or pr_err() here ?? Something like:
pr_err("%s: platform not supported yet\n", __func__);

> + return;
> + }
> platform_device_register(&mbox_device);
> }
> #else
> static inline void omap_init_mbox(void) { }
> -#endif
> +#endif /* CONFIG_OMAP_MBOX_FWK */
>
> #if defined(CONFIG_OMAP_STI)
>
> diff --git a/arch/arm/mach-omap2/mailbox.c b/arch/arm/mach-omap2/mailbox.c
> index 0609e2d..3176bb7 100644
> --- a/arch/arm/mach-omap2/mailbox.c
> +++ b/arch/arm/mach-omap2/mailbox.c
> @@ -30,7 +30,7 @@
> #define MAILBOX_IRQ_NEWMSG(u) (1 << (2 * (u)))
> #define MAILBOX_IRQ_NOTFULL(u) (1 << (2 * (u) + 1))
>
> -static unsigned long mbox_base;
> +static void __iomem *mbox_base;
>
> struct omap_mbox2_fifo {
> unsigned long msg;
> @@ -52,14 +52,14 @@ static struct clk *mbox_ick_handle;
> static void omap2_mbox_enable_irq(struct omap_mbox *mbox,
> omap_mbox_type_t irq);
>
> -static inline unsigned int mbox_read_reg(unsigned int reg)
> +static inline unsigned int mbox_read_reg(size_t ofs)
> {
> - return __raw_readl(mbox_base + reg);
> + return __raw_readl(mbox_base + ofs);
> }
>
> -static inline void mbox_write_reg(unsigned int val, unsigned int reg)
> +static inline void mbox_write_reg(u32 val, size_t ofs)
> {
> - __raw_writel(val, mbox_base + reg);
> + __raw_writel(val, mbox_base + ofs);
> }
>
> /* Mailbox H/W preparations */
> @@ -208,7 +208,7 @@ struct omap_mbox mbox_dsp_info = {
> };
> EXPORT_SYMBOL(mbox_dsp_info);
>
> -/* IVA */
> +#if defined(CONFIG_ARCH_OMAP2420) /* IVA */

would be nice to have something like if (cpu_has_iva()) { ... } and move
this check to a later location e.g. on probe(), but this wouldn't prevent
this patch from going in now. Just a suggestion for later improvements.

> static struct omap_mbox2_priv omap2_mbox_iva_priv = {
> .tx_fifo = {
> .msg = MAILBOX_MESSAGE(2),
> @@ -229,17 +229,12 @@ static struct omap_mbox mbox_iva_info = {
> .ops = &omap2_mbox_ops,
> .priv = &omap2_mbox_iva_priv,
> };
> +#endif

[snip]

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

platform_get_irq(pdev, 0) would look more standard, I think.

--
balbi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/