[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