On Wed, Nov 29, 2017 at 10:11:38PM +0300, Dan Carpenter wrote: > On Wed, Nov 29, 2017 at 09:37:15PM +0530, Souptick Joarder wrote: > > >> +static int bgx_port_sgmii_set_link_speed(struct bgx_port_priv *priv, struct port_status status) > > >> +{ > > >> + u64 data; > > >> + u64 prtx; > > >> + u64 miscx; > > >> + int timeout; > > >> + > > > > >> + > > >> + switch (status.speed) { > > >> + case 10: > > > > > > In my opinion, instead of hard coding the value, is it fine to use ENUM ? > > Similar comments applicable in other places where hard coded values are used. > > > > 10 means 10M right? That's not really a magic number. It's fine. There are also : uapi/linux/ethtool.h:#define SPEED_10 10 uapi/linux/ethtool.h:#define SPEED_100 100 uapi/linux/ethtool.h:#define SPEED_1000 1000 uapi/linux/ethtool.h:#define SPEED_10000 10000 uapi/linux/ethtool.h:#define SPEED_100000 100000 Andrew -- 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