Re: [PATCH net-next 4/6] bnxt_en: Skip MAC loopback selftest if it is unsupported by FW

[Michal Swiatkowski <michal.swiatkowski@linux.intel.com>]

On Sun, Dec 15, 2024 at 12:59:41PM -0800, Michael Chan wrote:
> Call the new HWRM_PORT_MAC_QCAPS to check if mac loopback is
> supported.  Skip the MAC loopback ethtool self test if it is
> not supported.
> 
> Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
> Reviewed-by: Pavan Chebbi <pavan.chebbi@broadcom.com>
> Reviewed-by: Andy Gospodarek <andrew.gospodarek@broadcom.com>
> Signed-off-by: Michael Chan <michael.chan@broadcom.com>
> ---
>  drivers/net/ethernet/broadcom/bnxt/bnxt.c     | 29 +++++++++++++++++++
>  drivers/net/ethernet/broadcom/bnxt/bnxt.h     |  5 ++++
>  .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 11 ++++---
>  3 files changed, 41 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> index 631dbda725ab..5a19146d6902 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> @@ -11551,6 +11551,31 @@ static int bnxt_hwrm_phy_qcaps(struct bnxt *bp)
>  	return rc;
>  }
>  
> +static int bnxt_hwrm_mac_qcaps(struct bnxt *bp)
> +{
> +	struct hwrm_port_mac_qcaps_output *resp;
> +	struct hwrm_port_mac_qcaps_input *req;
> +	int rc;
> +
> +	if (bp->hwrm_spec_code < 0x10a03)
> +		return 0;
> +
> +	rc = hwrm_req_init(bp, req, HWRM_PORT_MAC_QCAPS);
> +	if (rc)
> +		return rc;
> +
> +	resp = hwrm_req_hold(bp, req);
> +	rc = hwrm_req_send_silent(bp, req);
> +	if (rc)
> +		goto hwrm_mac_qcaps_exit;
> +
> +	bp->mac_flags = resp->flags;
> +
> +hwrm_mac_qcaps_exit:
> +	hwrm_req_drop(bp, req);
> +	return rc;
> +}
> +
>  static bool bnxt_support_dropped(u16 advertising, u16 supported)
>  {
>  	u16 diff = advertising ^ supported;
> @@ -15679,6 +15704,10 @@ static int bnxt_probe_phy(struct bnxt *bp, bool fw_dflt)
>  		bp->dev->priv_flags |= IFF_SUPP_NOFCS;
>  	else
>  		bp->dev->priv_flags &= ~IFF_SUPP_NOFCS;
> +
> +	bp->mac_flags = 0;
> +	bnxt_hwrm_mac_qcaps(bp);
The value returned from the function is ignored. Change it to return
void, or do sth here with returned value.

Thanks
> +
>  	if (!fw_dflt)
>  		return 0;
>  
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
> index d5e81e008ab5..094c9e95b463 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
> @@ -2668,6 +2668,11 @@ struct bnxt {
>  #define BNXT_PHY_FL_BANK_SEL		(PORT_PHY_QCAPS_RESP_FLAGS2_BANK_ADDR_SUPPORTED << 8)
>  #define BNXT_PHY_FL_SPEEDS2		(PORT_PHY_QCAPS_RESP_FLAGS2_SPEEDS2_SUPPORTED << 8)
>  
> +	/* copied from flags in hwrm_port_mac_qcaps_output */
> +	u8			mac_flags;
> +#define BNXT_MAC_FL_NO_MAC_LPBK		\
> +	PORT_MAC_QCAPS_RESP_FLAGS_LOCAL_LPBK_NOT_SUPPORTED
> +
>  	u8			num_tests;
>  	struct bnxt_test_info	*test_info;
>  
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> index e5904f2d56df..3bc2bd732021 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> @@ -4896,21 +4896,24 @@ static void bnxt_self_test(struct net_device *dev, struct ethtool_test *etest,
>  		bnxt_close_nic(bp, true, false);
>  		bnxt_run_fw_tests(bp, test_mask, &test_results);
>  
> -		buf[BNXT_MACLPBK_TEST_IDX] = 1;
> -		bnxt_hwrm_mac_loopback(bp, true);
> -		msleep(250);
>  		rc = bnxt_half_open_nic(bp);
>  		if (rc) {
> -			bnxt_hwrm_mac_loopback(bp, false);
>  			etest->flags |= ETH_TEST_FL_FAILED;
>  			return;
>  		}
> +		buf[BNXT_MACLPBK_TEST_IDX] = 1;
> +		if (bp->mac_flags & BNXT_MAC_FL_NO_MAC_LPBK)
> +			goto skip_mac_loopback;
> +
> +		bnxt_hwrm_mac_loopback(bp, true);
> +		msleep(250);
>  		if (bnxt_run_loopback(bp))
>  			etest->flags |= ETH_TEST_FL_FAILED;
>  		else
>  			buf[BNXT_MACLPBK_TEST_IDX] = 0;
>  
>  		bnxt_hwrm_mac_loopback(bp, false);
> +skip_mac_loopback:
>  		buf[BNXT_PHYLPBK_TEST_IDX] = 1;
>  		if (bp->phy_flags & BNXT_PHY_FL_NO_PHY_LPBK)
>  			goto skip_phy_loopback;
> -- 
> 2.30.1