Hi Andrew, > > +#define DRIVER_VERSION "1.0.7" > > Hi Raghuram > > Driver version strings a pretty pointless. You might want to remove it. > OK, will remove it. > > + netdev_info(dev->net, "Registered FIXED PHY\n"); > > There are too many detdev_info() messages here. Maybe make them both > netdev_dbg(). >
OK. Will modify to netdev_dbg() > > + dev->interface = PHY_INTERFACE_MODE_RGMII; > > + dev->fixedphy = phydev; > > You can use > > if (!phy_is_pseudo_fixed_link(phydev)) > > to determine is a PHY is a fixed phy. I think you can then do without > dev->fixedphy. > dev->fixedphy stores the fixed phydev, which will be passed to the fixed_phy_unregister routine , so I think phy_is_pseudo_fixed_link check is not necessary. > > + ret = lan78xx_write_reg(dev, HW_CFG, buf); > > + goto phyinit; > > Please don't use a goto like this. Maybe turn this into a switch statement? > OK. Will modify. > > static int lan78xx_phy_init(struct lan78xx_net *dev) > > goto error; > > Please take a look at what happens at error: It does not look correct. > Probably now is a good time to refactor the whole of lan78xx_phy_init() > OK. Will take care. Thanks, -Raghu
