On Wed, 2014-03-05 at 20:28 +0900, Byungho An wrote: > From: Siva Reddy <siva.kallam@xxxxxxxxxxx> Just a few trivial comments on a brief scan. > +/* Context descriptor structure */ > +struct xgmac_tx_ctxt_desc { > + u32 tstamp_lo:32; > + u32 tstamp_hi:32; I think u32 foo:32; is odd at best and may cause compiler oddities. > diff --git a/drivers/net/ethernet/samsung/xgmac_main.c b/drivers/net/ethernet/samsung/xgmac_main.c [] > +static void print_pkt(unsigned char *buf, int len) > +{ > + int j; > + pr_debug("len = %d byte, buf addr: 0x%p", len, buf); > + for (j = 0; j < len; j++) { > + if ((j % 16) == 0) > + pr_debug("\n %03x:", j); > + pr_debug(" %02x", buf[j]); > + } > + pr_debug("\n"); > +} This will make a mess. Use print_hex_dump_debug. > +static int xgmac_open(struct net_device *dev) > +{ [] > if (ret) { > + pr_err("%s: Cannot attach to PHY (error: %d)\n", > + __func__, ret); A lot of the message logging would be better using more verbose forms like; netdev_<level>(dev, etc...) [] > +/* xgmac_config - entry point for changing configuration mode passed on by > + * ifconfig > + * @dev : pointer to the device structure > + * @map : pointer to the device mapping structure > + * Description: > + * This function is a driver entry point which gets called by the kernel > + * whenever some device configuration is changed. > + * Return value: > + * This function returns 0 if success and appropriate error otherwise. > + */ It might be nicer if you used kernel-doc forms startig with /** for these comments everywhere. -- 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