On Sat, Nov 03, 2012 at 21:33:47, Shilimkar, Santosh wrote:
[...]
> > +static int omap2_mbox_fifo_needs_flush(struct omap_mbox *mbox)
> > +{
> > + struct omap_mbox2_fifo *fifo =
> > + &((struct omap_mbox2_priv *)mbox->priv)->tx_fifo;
> type casting is generally avoided in linux code.
I was just trying to be consistent with the rest of the mailbox FIFO
related code :)
Will see how to get rid of these globally in the next version.
> > + return mbox_read_reg(fifo->msg_stat);
> > +}
> > +
> > +static mbox_msg_t omap2_mbox_fifo_readback(struct omap_mbox *mbox)
> > +{
> > + struct omap_mbox2_fifo *fifo =
> > + &((struct omap_mbox2_priv *)mbox->priv)->tx_fifo;
> > + return (mbox_msg_t) mbox_read_reg(fifo->msg);
> same here.
Ok.
> > +}
> > +
> > /* Mailbox IRQ handle functions */
> > static void omap2_mbox_enable_irq(struct omap_mbox *mbox,
> > omap_mbox_type_t irq)
> > @@ -205,19 +219,21 @@ static void omap2_mbox_restore_ctx(struct omap_mbox *mbox)
> > }
> >
> > static struct omap_mbox_ops omap2_mbox_ops = {
> > - .type = OMAP_MBOX_TYPE2,
> > - .startup = omap2_mbox_startup,
> > - .shutdown = omap2_mbox_shutdown,
> > - .fifo_read = omap2_mbox_fifo_read,
> > - .fifo_write = omap2_mbox_fifo_write,
> > - .fifo_empty = omap2_mbox_fifo_empty,
> > - .fifo_full = omap2_mbox_fifo_full,
> > - .enable_irq = omap2_mbox_enable_irq,
> > - .disable_irq = omap2_mbox_disable_irq,
> > - .ack_irq = omap2_mbox_ack_irq,
> > - .is_irq = omap2_mbox_is_irq,
> > - .save_ctx = omap2_mbox_save_ctx,
> > - .restore_ctx = omap2_mbox_restore_ctx,
> > + .type = OMAP_MBOX_TYPE2,
> > + .startup = omap2_mbox_startup,
> > + .shutdown = omap2_mbox_shutdown,
> > + .fifo_read = omap2_mbox_fifo_read,
> > + .fifo_write = omap2_mbox_fifo_write,
> > + .fifo_empty = omap2_mbox_fifo_empty,
> > + .fifo_full = omap2_mbox_fifo_full,
> > + .fifo_needs_flush = omap2_mbox_fifo_needs_flush,
> > + .fifo_readback = omap2_mbox_fifo_readback,
> > + .enable_irq = omap2_mbox_enable_irq,
> > + .disable_irq = omap2_mbox_disable_irq,
> > + .ack_irq = omap2_mbox_ack_irq,
> > + .is_irq = omap2_mbox_is_irq,
> > + .save_ctx = omap2_mbox_save_ctx,
> > + .restore_ctx = omap2_mbox_restore_ctx,
> You should do the indentation fix in another patch.
>
Ok will split it up.
> > };
> >
> > /*
> > diff --git a/arch/arm/plat-omap/include/plat/mailbox.h b/arch/arm/plat-omap/include/plat/mailbox.h
> > index cc3921e..e136529 100644
> > --- a/arch/arm/plat-omap/include/plat/mailbox.h
> > +++ b/arch/arm/plat-omap/include/plat/mailbox.h
> > @@ -29,6 +29,8 @@ struct omap_mbox_ops {
> > void (*fifo_write)(struct omap_mbox *mbox, mbox_msg_t msg);
> > int (*fifo_empty)(struct omap_mbox *mbox);
> > int (*fifo_full)(struct omap_mbox *mbox);
> > + int (*fifo_needs_flush)(struct omap_mbox *mbox);
> > + mbox_msg_t (*fifo_readback)(struct omap_mbox *mbox);
> Do you think passing the msg structure as an argument and letting the
> function populate it will be better instead of returning the msg
> structure ? No strong opinion since from read_foo() point of view
> what you have done might be right thing. In either case, please
> get rid of typecasting.
>
Passing the msg structure looks fine. Will do that in the next version.
Regards,
Vaibhav
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
