[PATCH v2 1/2] net: brcm: netXtreme driver
Marek Behún
kabel at kernel.org
Mon Oct 25 15:58:38 CEST 2021
NAK for this driver.
- display_banner() spams the output unnecessarily, the information
should be printed with debug()
- you are introducing custom mechanism for setting / getting PHY
parameters, via custom specific env variables, for example in the
set_phy_speed() and set_phy_link() functions, i.e.:
sprintf(name1, "bnxt_eth%u_phy_speed", bp->cardnum);
env_set(name1, name);
The whole point of several people in the past few years was to create
generic mechanisms for such things. We have ethernet PHY DM class,
you should use this. That way you won't need to introduce custom
mechanisms to get the infromation, since there are mii/mdio commands.
- print_mac() - the driver shouldn't even have this function, it should
just set appropriate ethNaddr variable
- in bnxt_eth_probe() you are looking at the variable "ethaddr":
if (env_get("ethaddr"))
secondary = 1;
a driver should never look itself at this variable.
Since your driver should be of UCLASS_ETH, the generic mechanism
should use appropriate env variable by calling you .write_hwaddr
method
Basically you are going against all the points of the whole idea to
have a generic API to set network driver parameters, and instead you
are adding driver-specific custom mechanisms.
Please change that in next version.
Marek
More information about the U-Boot
mailing list