Messages in this thread |  | | Date | Sat, 16 Apr 2022 14:53:44 +0300 | Subject | Re: [PATCH] net: ax88179: add proper error handling of usb read errors | From | Pavel Skripkin <> |
| |
Hi David,
On 4/16/22 14:49, David Kahurani wrote: >> > @@ -1303,8 +1448,12 @@ static void ax88179_get_mac_addr(struct usbnet *dev) >> > netif_dbg(dev, ifup, dev->net, >> > "MAC address read from device tree"); >> > } else { >> > - ax88179_read_cmd(dev, AX_ACCESS_MAC, AX_NODE_ID, ETH_ALEN, >> > - ETH_ALEN, mac); >> > + ret = ax88179_read_cmd(dev, AX_ACCESS_MAC, AX_NODE_ID, ETH_ALEN, >> > + ETH_ALEN, mac); >> > + >> > + if (ret) >> > + netdev_dbg(dev->net, "Failed to read NODE_ID: %d", ret); >> > + >> > netif_dbg(dev, ifup, dev->net, >> > "MAC address read from ASIX chip"); >> > } >> >> >> This message sequence is confusing. >> >> In case of ax88179_read_cmd() failure mac read from device actually >> failed, but message says, that it was successfully finished. > > I suppose the code should return in case of an error that way the next > message does not get executed. >
No, this will break the driver. This function should set mac address in netdev structure and if read from device fails code calls
eth_hw_addr_set(dev->net, mac);
which will generate random mac addr. I.e. read failure is not critical in that function
So, no need to return with an error, just don't print confusing messages :)
With regards, Pavel Skripkin [unhandled content-type:application/pgp-signature] |  |