Hi, Thanks!
Maybe I should make a clean and split them into small patches. Cheers! Wei. > Hi, > > This is a very large patch. It may be easier to review if it could be > split on some logical way, that is at all possible (I don't > know either > way). This is just a quick note about some more trivial things. > > On Fri, 21 Dec 2007 17:58:43 +0800 Zhang Wei > <[EMAIL PROTECTED]> wrote: > > > > +struct rio_priv { > > + volatile void __iomem *regs_win; > > + volatile struct rio_atmu_regs __iomem *atmu_regs; > > + volatile struct rio_atmu_regs __iomem *maint_atmu_regs; > > + volatile struct rio_atmu_regs __iomem *dbell_atmu_regs; > > + volatile void __iomem *dbell_win; > > + volatile void __iomem *maint_win; > > + volatile struct rio_msg_regs __iomem *msg_regs; > > Paulus has said that any pointer marked __iomem does not need to be > volatile ... > > > +static int of_cells_get(struct device_node *np, const char *str) > > +{ > > + struct device_node *tmp = NULL; > > + const int *var = NULL; > > These initializations are unnecessary. > > > + var = of_get_property(np, str, NULL); > > + tmp = of_get_parent(np); > > + > > + while (!var && tmp) { > > + var = (int *)of_get_property(tmp, str, NULL); > > While I applaud the number of casts remove by this patch, > this one is an > unnecessary addition. > > > + of_node_put(tmp); > > + tmp = of_get_parent(np); > > You should do the above two line in the opposite order. Also do you > really want to keep getting the parent of the same node over and over > (i.e. you never change np)? > > > + } > > You probably want a final of_node_put(tmp). > > > > + INFO("Phy type: "); > > + switch (phy_type) { > > + case RIO_PHY_SERIAL: > > + printk("serial\n"); > > + break; > > + case RIO_PHY_PARALLEL: > > + printk("parallel"); > > Missing \n > > > + port = kzalloc(sizeof(struct rio_mport), GFP_KERNEL); > > + if (!port) { > > + ERR("Can't alloc memory for 'port'\n"); > > + rc = -ENOMEM; > > + goto err; > > + } > > port->id = 0; > > port->index = 0; > > These two could go as you just allocated zeroed memory. > > -- > Cheers, > Stephen Rothwell [EMAIL PROTECTED] > http://www.canb.auug.org.au/~sfr/ > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/