[PATCH v2 1/2] net: brcm: netXtreme driver

[Roman Bacik]

On Mon, Oct 25, 2021 at 6:58 AM Marek Behún <kabel at kernel.org> wrote:
>
> NAK for this driver.
>
> - display_banner() spams the output unnecessarily, the information
>   should be printed with debug()

We will make the change as requested.

>
> - 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.

These are chip internal settings stored internally in NVM. They are
not modified via mii/mdio.

>
> - print_mac() - the driver shouldn't even have this function, it should
>   just set appropriate ethNaddr variable

The function was added for user convenience. We will remove it.

>
> - 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

We will try to modify it.

>
> 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

Thank you very much for your review,

Roman

-- 
This electronic communication and the information and any files transmitted 
with it, or attached to it, are confidential and are intended solely for 
the use of the individual or entity to whom it is addressed and may contain 
information that is confidential, legally privileged, protected by privacy 
laws, or otherwise restricted from disclosure to anyone else. If you are 
not the intended recipient or the person responsible for delivering the 
e-mail to the intended recipient, you are hereby notified that any use, 
copying, distributing, dissemination, forwarding, printing, or copying of 
this e-mail is strictly prohibited. If you received this e-mail in error, 
please return the e-mail to the sender, delete it from your computer, and 
destroy any printed copy of it.