On Sat, 14 May 2022 16:32:34 +0300 David Kahurani wrote: > - ax88179_read_cmd(dev, AX_ACCESS_MAC, AX_SROM_CMD, > - 1, 1, &buf); > + ret = ax88179_read_cmd(dev, AX_ACCESS_MAC, AX_SROM_CMD, > + 1, 1, &buf); > + if (ret) { > + netdev_dbg(dev->net, > + "Failed to read SROM_CMD: %d\n", > + ret); > + return ret; > + } > > if (time_after(jiffies, jtimeout)) > return -EINVAL; > > } while (buf & EEP_BUSY); I agree with Pavel, this seems unnecessarily strict. If the error is not ENODEV we can keep looping. > @@ -1581,7 +1731,13 @@ static int ax88179_link_reset(struct usbnet *dev) > &ax179_data->rxctl); > > /*link up, check the usb device control TX FIFO full or empty*/ > - ax88179_read_cmd(dev, 0x81, 0x8c, 0, 4, &tmp32); > + ret = ax88179_read_cmd(dev, 0x81, 0x8c, 0, 4, &tmp32); > + > + if (ret) { > + netdev_dbg(dev->net, "Failed to read TX FIFO: %d\n", > + ret); > + return ret; > + } Please don't add any empty lines within the error checking. Empty lines are supposed to separate logically separate blocks of code. Error checking is very much logically part of the call. And no empty line betwee netdev_dbg() and return ret; either. In this submission you have all possible configurations of having or not having empty lines before the if or before the return. None of them should be there, and please be consistent.