On Mon, Nov 9, 2015 at 11:35 PM, Duc Dang <dhdang@xxxxxxx> wrote: > X-Gene mailbox controller provides 8 mailbox channels, with > each channel has a dedicated interrupt line. > > [dhdang: rebase over 4.3-rc5, some minor changes to > address comment in v2 patch set] > Do you want this to go into git logs? > Signed-off-by: Feng Kan <fkan@xxxxxxx> > Signed-off-by: Duc Dang <dhdang@xxxxxxx> > --- here is the right place for any history. And it is good practice to specify whatever changes were made from last revision. > + > +struct slimpro_mbox_chan { > + struct device *dev; > + struct mbox_chan *chan; > + void __iomem *reg; > + int id; > You don't seem to really need 'id'. > + > +static void mb_chan_enable_int(struct slimpro_mbox_chan *mb_chan, u32 mask) > +{ > + u32 val = readl(mb_chan->reg + REG_DB_STATMASK); > + > + val &= ~mask; > + writel(val, mb_chan->reg + REG_DB_STATMASK); > +} > + > +static void mb_chan_disable_int(struct slimpro_mbox_chan *mb_chan, u32 mask) > +{ > + u32 val = readl(mb_chan->reg + REG_DB_STATMASK); > + > + val |= mask; > + writel(val, mb_chan->reg + REG_DB_STATMASK); > +} > + These 2 functions are called just once. And do what other drivers usually inline. Wouldn't it be better to directly set & clear the bit when needed? > +static int slimpro_mbox_send_data(struct mbox_chan *chan, void *msg) > +{ > + struct slimpro_mbox_chan *mb_chan = > + (struct slimpro_mbox_chan *)chan->con_priv; > + You don't need to typecast a void pointer. Here and elsewhere. > + > + /* Setup mailbox links */ > + for (i = 0; i < MBOX_CNT; i++) { > + ctx->mc[i].irq = platform_get_irq(pdev, i); > You expect every platform to provide exactly 'MBOX_CNT' irqs and they must be different (because you don't 'SHARE' when you request). Usually developers relax either of the conditions. > +static struct platform_driver slimpro_mbox_driver = { > + .probe = slimpro_mbox_probe, > + .remove = slimpro_mbox_remove, > + .driver = { > + .name = "xgene-slimpro-mbox", > + .owner = THIS_MODULE, > No need to set .owner. Cheers. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html