From: Andrew Lunn <and...@lunn.ch> Date: Thu, 1 Feb 2018 16:02:09 +0100
>> +static void dwmac5_log_error(struct net_device *ndev, u32 value, bool corr, >> + const char *module_name, const char **errors_str) { >> + unsigned long loc, mask; >> + >> + mask = value; >> + for_each_set_bit(loc, &mask, 32) { >> + netdev_err(ndev, "Found %s error in %s: '%s'\n", corr ? >> + "correctable" : "uncorrectable", module_name, >> + errors_str[loc]); >> + } >> +} > > How about also adding ethtool -S stats. You have a text string, so all > you need to add is a counter. And i expect statistics are looked at > more than dmesg output. I agree. Perhaps for extremely catastrophic errors (those which require a complete chip reset, for example), statistics are really the way to go. Also, all of these functions are not style properly. The openning curly braces of a function belong on a separate line of their own, not at the end of the arguments. The braces for structure assignment blocks have a similar problem, the final closing brace and semicolon should be on a line by itself.