Message-ID: <75684350-48e9-4438-ae42-431d7eb2a5f2@oracle.com>
Date: Wed, 19 Nov 2025 15:15:49 +0530
From: ALOK TIWARI <alok.a.tiwari@...cle.com>
To: Bhargava Marreddy <bhargava.marreddy@...adcom.com>, davem@...emloft.net,
        edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com,
        andrew+netdev@...n.ch, horms@...nel.org
Cc: netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        michael.chan@...adcom.com, pavan.chebbi@...adcom.com,
        vsrama-krishna.nemani@...adcom.com, vikas.gupta@...adcom.com,
        Rajashekar Hudumula <rajashekar.hudumula@...adcom.com>
Subject: Re: [External] : [v2, net-next 01/12] bng_en: Query PHY and report
 link status


> @@ -9,8 +9,10 @@
>   
>   #include <linux/etherdevice.h>
>   #include <linux/bnxt/hsi.h>
> +#include <linux/ethtool.h>
>   #include "bnge_rmem.h"
>   #include "bnge_resc.h"
> +#include "bnge_link.h"
>   
>   #define DRV_VER_MAJ	1
>   #define DRV_VER_MIN	15
> @@ -141,6 +143,17 @@ struct bnge_dev {
>   	struct bnge_ctx_mem_info	*ctx;
>   
>   	u64			flags;
> +#define BNGE_PF(bd)		(1)
> +#define BNGE_VF(bd)		(0)
> +#define BNGE_NPAR(bd)		(0)
> +#define BNGE_MH(bd)		(0)
> +#define BNGE_SINGLE_PF(bd)	(BNGE_PF(bd) && !BNGE_NPAR(bd) && !BNGE_MH(bn))

bn is wrong.

> +#define BNGE_SH_PORT_CFG_OK(bd)			\
> +	(BNGE_PF(bd) && ((bd)->phy_flags & BNGE_PHY_FL_SHARED_PORT_CFG))
> +#define BNGE_PHY_CFG_ABLE(bd)			\
> +	((BNGE_SINGLE_PF(bd) ||			\
> +	  BNGE_SH_PORT_CFG_OK(bd)) &&		\
> +	 (bd)->link_info.phy_state == BNGE_PHY_STATE_ENABLED)
>   
>   	struct bnge_hw_resc	hw_resc;
>   
> @@ -197,6 +210,22 @@ struct bnge_dev {
>   
>   	struct bnge_irq		*irq_tbl;
>   	u16			irqs_acquired;
> +
> +	/* To protect link related settings during link changes and
> +	 * ethtool settings changes.
> +	 */
> +	struct mutex		link_lock;
> +	struct bnge_link_info	link_info;
> +
> +	/* copied from flags and flags2 in hwrm_port_phy_qcaps_output */
> +	u32			phy_flags;
> +#define BNGE_PHY_FL_SHARED_PORT_CFG	\
> +	PORT_PHY_QCAPS_RESP_FLAGS_SHARED_PHY_CFG_SUPPORTED
> +#define BNGE_PHY_FL_NO_FCS		PORT_PHY_QCAPS_RESP_FLAGS_NO_FCS
> +#define BNGE_PHY_FL_SPEEDS2		\
> +	(PORT_PHY_QCAPS_RESP_FLAGS2_SPEEDS2_SUPPORTED << 8)
> +
> +	u32                     msg_enable;
>   };
>   
>   static inline bool bnge_is_roce_en(struct bnge_dev *bd)
> diff --git a/drivers/net/ethernet/broadcom/bnge/bnge_hwrm_lib.c b/drivers/net/ethernet/broadcom/bnge/bnge_hwrm_lib.c
> index 198f49b40db..b0e941ad18b 100644
> --- a/drivers/net/ethernet/broadcom/bnge/bnge_hwrm_lib.c
> +++ b/drivers/net/ethernet/broadcom/bnge/bnge_hwrm_lib.c
> @@ -14,6 +14,7 @@
>   #include "bnge_hwrm_lib.h"
>   #include "bnge_rmem.h"
>   #include "bnge_resc.h"
> +#include "bnge_link.h"
>   
>   int bnge_hwrm_ver_get(struct bnge_dev *bd)
>   {
> @@ -981,6 +982,192 @@ void bnge_hwrm_vnic_ctx_free_one(struct bnge_dev *bd,
>   	vnic->fw_rss_cos_lb_ctx[ctx_idx] = INVALID_HW_RING_ID;
>   }
>   
> +int bnge_hwrm_phy_qcaps(struct bnge_dev *bd)
> +{
> +	struct bnge_link_info *link_info = &bd->link_info;
> +	struct hwrm_port_phy_qcaps_output *resp;
> +	struct hwrm_port_phy_qcaps_input *req;
> +	int rc = 0;
> +
> +	rc = bnge_hwrm_req_init(bd, req, HWRM_PORT_PHY_QCAPS);
> +	if (rc)
> +		return rc;
> +
> +	resp = bnge_hwrm_req_hold(bd, req);
> +	rc = bnge_hwrm_req_send(bd, req);
> +	if (rc)
> +		goto hwrm_phy_qcaps_exit;
> +
> +	bd->phy_flags = resp->flags | (le16_to_cpu(resp->flags2) << 8);
> +
> +	if (bnge_phy_qcaps_no_speed(resp)) {
> +		link_info->phy_state = BNGE_PHY_STATE_DISABLED;
> +		netdev_warn(bd->netdev, "Ethernet link disabled\n");
> +	} else if (link_info->phy_state == BNGE_PHY_STATE_DISABLED) {
> +		link_info->phy_state = BNGE_PHY_STATE_ENABLED;
> +		netdev_info(bd->netdev, "Ethernet link enabled\n");
> +		/* Phy re-enabled, reprobe the speeds */
> +		link_info->support_auto_speeds = 0;
> +		link_info->support_pam4_auto_speeds = 0;
> +		link_info->support_auto_speeds2 = 0;
> +	}
> +	if (resp->supported_speeds_auto_mode)
> +		link_info->support_auto_speeds =
> +			le16_to_cpu(resp->supported_speeds_auto_mode);
> +	if (resp->supported_pam4_speeds_auto_mode)
> +		link_info->support_pam4_auto_speeds =
> +			le16_to_cpu(resp->supported_pam4_speeds_auto_mode);
> +	if (resp->supported_speeds2_auto_mode)
> +		link_info->support_auto_speeds2 =
> +			le16_to_cpu(resp->supported_speeds2_auto_mode);
> +
> +	bd->port_count = resp->port_cnt;
> +
> +hwrm_phy_qcaps_exit:
> +	bnge_hwrm_req_drop(bd, req);
> +	return rc;
> +}
> +
> +int bnge_hwrm_set_link_setting(struct bnge_net *bn, bool set_pause)
> +{
> +	struct hwrm_port_phy_cfg_input *req;
> +	struct bnge_dev *bd = bn->bd;
> +	int rc;
> +
> +	rc = bnge_hwrm_req_init(bd, req, HWRM_PORT_PHY_CFG);
> +	if (rc)
> +		return rc;
> +
> +	if (set_pause)
> +		bnge_hwrm_set_pause_common(bn, req);
> +
> +	bnge_hwrm_set_link_common(bn, req);
> +
> +	return bnge_hwrm_req_send(bd, req);
> +}
> +
[snip]
> 
> +
> +int bnge_update_phy_setting(struct bnge_net *bn)
> +{
> +	struct bnge_ethtool_link_info *elink_info;
> +	struct bnge_link_info *link_info;
> +	struct bnge_dev *bd = bn->bd;
> +	bool update_pause = false;
> +	bool update_link = false;
> +	int rc;
> +
> +	link_info = &bd->link_info;
> +	elink_info = &bn->eth_link_info;
> +	rc = bnge_update_link(bn, true);
> +	if (rc) {
> +		netdev_err(bd->netdev, "failed to update link (rc: %d)\n",
> +			   rc);
> +		return rc;
> +	}
> +	if (!BNGE_SINGLE_PF(bd))
> +		return 0;
> +
> +	if ((elink_info->autoneg & BNGE_AUTONEG_FLOW_CTRL) &&
> +	    (link_info->auto_pause_setting & BNGE_LINK_PAUSE_BOTH) !=
> +	    elink_info->req_flow_ctrl)
> +		update_pause = true;
> +	if (!(elink_info->autoneg & BNGE_AUTONEG_FLOW_CTRL) &&
> +	    link_info->force_pause_setting != elink_info->req_flow_ctrl)
> +		update_pause = true;
> +	if (!(elink_info->autoneg & BNGE_AUTONEG_SPEED)) {
> +		if (BNGE_AUTO_MODE(link_info->auto_mode))
> +			update_link = true;
> +		if (bnge_force_speed_updated(bn))
> +			update_link = true;
> +		if (elink_info->req_duplex != link_info->duplex_setting)
> +			update_link = true;
> +	} else {
> +		if (link_info->auto_mode == BNGE_LINK_AUTO_NONE)
> +			update_link = true;
> +		if (bnge_auto_speed_updated(bn))
> +			update_link = true;
> +	}
> +
> +	/* The last close may have shutdown the link, so need to call
> +	 * PHY_CFG to bring it back up.
> +	 */
> +	if (!BNGE_LINK_IS_UP(bd))
> +		update_link = true;
> +
> +	if (update_link)
> +		rc = bnge_hwrm_set_link_setting(bn, update_pause);
> +	else if (update_pause)
> +		rc = bnge_hwrm_set_pause(bn);

new line help here

if
update_link == false
update_pause == false

it will return rc = 0 assign by bnge_update_link() correct?

> +	if (rc) {
> +		netdev_err(bd->netdev,
> +			   "failed to update phy setting (rc: %d)\n", rc);
> +		return rc;
> +	}
> +
> +	return rc;
> +}
> +
> +void bnge_get_port_module_status(struct bnge_net *bn)
> +{
> +	struct bnge_dev *bd = bn->bd;
> +	struct bnge_link_info *link_info = &bd->link_info;
> +	struct hwrm_port_phy_qcfg_output *resp = &link_info->phy_qcfg_resp;
> +	u8 module_status;
> +
> +	if (bnge_update_link(bn, true))
> +		return;
> +
> +	module_status = link_info->module_status;
> +	switch (module_status) {
> +	case PORT_PHY_QCFG_RESP_MODULE_STATUS_DISABLETX:
> +	case PORT_PHY_QCFG_RESP_MODULE_STATUS_PWRDOWN:
> +	case PORT_PHY_QCFG_RESP_MODULE_STATUS_WARNINGMSG:
> +		netdev_warn(bd->netdev,
> +			    "Unqualified SFP+ module detected on port %d\n",
> +			    bd->pf.port_id);
> +		netdev_warn(bd->netdev, "Module part number %s\n",
> +			    resp->phy_vendor_partnumber);
> +		if (module_status == PORT_PHY_QCFG_RESP_MODULE_STATUS_DISABLETX)
> +			netdev_warn(bd->netdev, "TX is disabled\n");
> +		if (module_status == PORT_PHY_QCFG_RESP_MODULE_STATUS_PWRDOWN)
> +			netdev_warn(bd->netdev, "SFP+ module is shutdown\n");
> +	}
> +}
> +
> +static bool bnge_support_dropped(u16 advertising, u16 supported)
> +{
> +	u16 diff = advertising ^ supported;
> +
> +	return ((supported | diff) != supported);
> +}
> +
> +bool bnge_support_speed_dropped(struct bnge_net *bn)
> +{
> +	struct bnge_ethtool_link_info *elink_info = &bn->eth_link_info;
> +	struct bnge_dev *bd = bn->bd;
> +	struct bnge_link_info *link_info = &bd->link_info;
> +
> +	/* Check if any advertised speeds are no longer supported. The caller
> +	 * holds the link_lock mutex, so we can modify link_info settings.
> +	 */
> +	if (bd->phy_flags & BNGE_PHY_FL_SPEEDS2) {
> +		if (bnge_support_dropped(elink_info->advertising,
> +					 link_info->support_auto_speeds2)) {
> +			elink_info->advertising =
> +				link_info->support_auto_speeds2;
> +			return true;
> +		}
> +		return false;
> +	}
> +	if (bnge_support_dropped(elink_info->advertising,
> +				 link_info->support_auto_speeds)) {
> +		elink_info->advertising = link_info->support_auto_speeds;
> +		return true;
> +	}
> +	if (bnge_support_dropped(elink_info->advertising_pam4,
> +				 link_info->support_pam4_auto_speeds)) {
> +		elink_info->advertising_pam4 =
> +			link_info->support_pam4_auto_speeds;
> +		return true;
> +	}
> +	return false;
> +}
> +
> +static char *bnge_report_fec(struct bnge_link_info *link_info)
> +{
> +	u8 active_fec = link_info->active_fec_sig_mode &
> +			PORT_PHY_QCFG_RESP_ACTIVE_FEC_MASK;
> +
> +	switch (active_fec) {
> +	default:
> +	case PORT_PHY_QCFG_RESP_ACTIVE_FEC_FEC_NONE_ACTIVE:
> +		return "None";
> +	case PORT_PHY_QCFG_RESP_ACTIVE_FEC_FEC_CLAUSE74_ACTIVE:
> +		return "Clause 74 BaseR";
> +	case PORT_PHY_QCFG_RESP_ACTIVE_FEC_FEC_CLAUSE91_ACTIVE:
> +		return "Clause 91 RS(528,514)";
> +	case PORT_PHY_QCFG_RESP_ACTIVE_FEC_FEC_RS544_1XN_ACTIVE:
> +		return "Clause 91 RS544_1XN";
> +	case PORT_PHY_QCFG_RESP_ACTIVE_FEC_FEC_RS544_IEEE_ACTIVE:
> +		return "Clause 91 RS(544,514)";
> +	case PORT_PHY_QCFG_RESP_ACTIVE_FEC_FEC_RS272_1XN_ACTIVE:
> +		return "Clause 91 RS272_1XN";
> +	case PORT_PHY_QCFG_RESP_ACTIVE_FEC_FEC_RS272_IEEE_ACTIVE:
> +		return "Clause 91 RS(272,257)";
> +	}
> +}
> +
> +void bnge_report_link(struct bnge_dev *bd)
> +{
> +	if (BNGE_LINK_IS_UP(bd)) {
> +		const char *signal = "";
> +		const char *flow_ctrl;
> +		const char *duplex;
> +		u32 speed;
> +		u16 fec;
> +
> +		netif_carrier_on(bd->netdev);
> +		speed = bnge_fw_to_ethtool_speed(bd->link_info.link_speed);
> +		if (speed == SPEED_UNKNOWN) {
> +			netdev_info(bd->netdev,
> +				    "NIC Link is Up, speed unknown\n");
> +			return;
> +		}
> +		if (bd->link_info.duplex == BNGE_LINK_DUPLEX_FULL)
> +			duplex = "full";
> +		else
> +			duplex = "half";
> +		if (bd->link_info.pause == BNGE_LINK_PAUSE_BOTH)
> +			flow_ctrl = "ON - receive & transmit";
> +		else if (bd->link_info.pause == BNGE_LINK_PAUSE_TX)
> +			flow_ctrl = "ON - transmit";
> +		else if (bd->link_info.pause == BNGE_LINK_PAUSE_RX)
> +			flow_ctrl = "ON - receive";
> +		else
> +			flow_ctrl = "none";
> +		if (bd->link_info.phy_qcfg_resp.option_flags &
> +		    PORT_PHY_QCFG_RESP_OPTION_FLAGS_SIGNAL_MODE_KNOWN) {
> +			u8 sig_mode = bd->link_info.active_fec_sig_mode &
> +				      PORT_PHY_QCFG_RESP_SIGNAL_MODE_MASK;
> +			switch (sig_mode) {
> +			case PORT_PHY_QCFG_RESP_SIGNAL_MODE_NRZ:
> +				signal = "(NRZ) ";
> +				break;
> +			case PORT_PHY_QCFG_RESP_SIGNAL_MODE_PAM4:
> +				signal = "(PAM4 56Gbps) ";
> +				break;
> +			case PORT_PHY_QCFG_RESP_SIGNAL_MODE_PAM4_112:
> +				signal = "(PAM4 112Gbps) ";
> +				break;
> +			default:
> +				break;
> +			}
> +		}
> +		netdev_info(bd->netdev, "NIC Link is Up, %u Mbps %s%s duplex, Flow control: %s\n",
> +			    speed, signal, duplex, flow_ctrl);
> +		fec = bd->link_info.fec_cfg;
> +		if (!(fec & PORT_PHY_QCFG_RESP_FEC_CFG_FEC_NONE_SUPPORTED))
> +			netdev_info(bd->netdev, "FEC autoneg %s encoding: %s\n",
> +				    (fec & BNGE_FEC_AUTONEG) ? "on" : "off",
> +				    bnge_report_fec(&bd->link_info));
> +	} else {
> +		netif_carrier_off(bd->netdev);
> +		netdev_err(bd->netdev, "NIC Link is Down\n");
> +	}
> +}
> diff --git a/drivers/net/ethernet/broadcom/bnge/bnge_link.h b/drivers/net/ethernet/broadcom/bnge/bnge_link.h
> new file mode 100644
> index 00000000000..65da27c510b
> --- /dev/null
> +++ b/drivers/net/ethernet/broadcom/bnge/bnge_link.h
> @@ -0,0 +1,185 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (c) 2025 Broadcom */
> +
> +#ifndef _BNGE_LINK_H_
> +#define _BNGE_LINK_H_
> +
> +#define BNGE_PHY_AUTO_SPEEDS2_MASK	\
> +	PORT_PHY_CFG_REQ_ENABLES_AUTO_LINK_SPEEDS2_MASK
> +#define BNGE_PHY_AUTO_SPEED_MASK	\
> +	PORT_PHY_CFG_REQ_ENABLES_AUTO_LINK_SPEED_MASK
> +#define BNGE_PHY_AUTO_PAM4_SPEED_MASK	\
> +	PORT_PHY_CFG_REQ_ENABLES_AUTO_PAM4_LINK_SPEED_MASK
> +#define BNGE_PHY_FLAGS_RESTART_AUTO	\
> +	PORT_PHY_CFG_REQ_FLAGS_RESTART_AUTONEG
> +#define BNGE_PHY_FLAGS_ENA_FORCE_SPEEDS2	\
> +	PORT_PHY_CFG_REQ_ENABLES_FORCE_LINK_SPEEDS2
> +#define BNGE_PHY_FLAGS_ENA_FORCE_PM4_SPEED	\

typo should be PAM4 ?

> +	PORT_PHY_CFG_REQ_ENABLES_FORCE_PAM4_LINK_SPEED
> +
> +struct bnge_link_info {
> +	u8			phy_type;
> +	u8			media_type;
> +	u8			transceiver;
> +	u8			phy_addr;
> +	u8			phy_link_status;
> +#define BNGE_LINK_LINK		PORT_PHY_QCFG_RESP_LINK_LINK
> +	u8			phy_state;
> +#define BNGE_PHY_STATE_ENABLED		0
> +#define BNGE_PHY_STATE_DISABLED		1
> +
> +	u8			link_state;
> +#define BNGE_LINK_STATE_UNKNOWN	0
> +#define BNGE_LINK_STATE_DOWN	1
> +#define BNGE_LINK_STATE_UP	2
> +#define BNGE_LINK_IS_UP(bd)		\
> +	((bd)->link_info.link_state == BNGE_LINK_STATE_UP)
> +	u8			active_lanes;


Thanks,
Alok

Powered by blists - more mailing lists