Re: [PATCH net-next v8 03/10] bng_en: add ethtool link settings, get_link, and nway_reset

[Jakub Kicinski <kuba@kernel.org>]

On Thu, 19 Mar 2026 11:21:17 +0530 Vikas Gupta wrote:
> +bnge_force_link_speed(struct net_device *dev, u32 ethtool_speed, u32 lanes)
> +{
> +	u16 support_pam4_spds, support_spds2, support_spds;
> +	struct bnge_ethtool_link_info *elink_info;
> +	struct bnge_net *bn = netdev_priv(dev);
> +	struct bnge_link_info *link_info;
> +	u8 sig_mode = BNGE_SIG_MODE_NRZ;
> +	struct bnge_dev *bd = bn->bd;
> +	u32 lanes_needed = 1;
> +	u16 fw_speed = 0;
> +
> +	elink_info = &bn->eth_link_info;
> +	link_info = &bd->link_info;
> +	support_pam4_spds = link_info->support_pam4_speeds;
> +	support_spds2 = link_info->support_speeds2;
> +	support_spds = link_info->support_speeds;
> +
> +	switch (ethtool_speed) {
> +	case SPEED_50000:
> +		if (((support_spds & BNGE_LINK_SPEED_MSK_50GB) ||
> +		     (support_spds2 & BNGE_LINK_SPEEDS2_MSK_50GB)) &&
> +		    lanes != 1) {
> +			fw_speed = PORT_PHY_CFG_REQ_FORCE_LINK_SPEED_50GB;
> +			lanes_needed = 2;
> +		} else if (support_pam4_spds & BNGE_LINK_PAM4_SPEED_MSK_50GB) {
> +			fw_speed = PORT_PHY_CFG_REQ_FORCE_PAM4_LINK_SPEED_50GB;
> +			sig_mode = BNGE_SIG_MODE_PAM4;
> +		} else if (support_spds2 & BNGE_LINK_SPEEDS2_MSK_50GB_PAM4) {
> +			fw_speed = BNGE_LINK_SPEED_50GB_PAM4;
> +			sig_mode = BNGE_SIG_MODE_PAM4;
> +		}

Looking thru this review:
https://sashiko.dev/#/patchset/20260319055124.1350670-1-vikas.gupta%40broadcom.com

I agree that you are playing a little loose with the requested lane
count. Why are you calculating your own lanes_needed if user provided
explicit lane count? We must match user request if set or return an
error.

Another thing that jumped out at me was that stats may transiently go
backwards in patch 8.

Please look carefully thru that review, there may be more worth
addressing.

Bhargava is correct in the responses to patches 9 and 10 FWIW.
The AI was wrong on those.
-- 
pw-bot: cr