[8/8] net/bnxt: fix the check for autoneg enablement in the PHY FW

Message ID 20220615145703.6613-9-kalesh-anakkur.purayil@broadcom.com (mailing list archive)
State Accepted, archived
Delegated to: Ajit Khaparde
Headers
Series bnxt PMD fixes |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel-compilation success Compilation OK
ci/iol-mellanox-Performance success Performance Testing PASS
ci/intel-Testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/github-robot: build success github build: passed
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-x86_64-unit-testing fail Testing issues
ci/iol-abi-testing success Testing PASS

Commit Message

Kalesh A P June 15, 2022, 2:57 p.m. UTC
  From: Somnath Kotur <somnath.kotur@broadcom.com>

The current combination of checks to determine whether autoneg is
enabled in the card is a bit convoluted and may become incorrect
as well in the future as one of the fields being used - auto_link_speed
might become deprecated.
Switch to using the 'auto_mode' field obtained from the response of
HWRM_PHY_QCFG cmd as that is always deterministically set by the PHY
FW.
Fixed a bug in the 40G check to only look for the bit setting and
not the actual value.
Also, check the forced speeds first before trying to enforce the
auto speeds

Allow the user to set autoneg speed in all cases except for PAM4 200G
as PAM4 200G will come up only in forced mode.

Fixes: c23f9ded0391 ("net/bnxt: support 200G PAM4 link")

Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com>
Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
 drivers/net/bnxt/bnxt_hwrm.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)
  

Comments

Ferruh Yigit June 16, 2022, 5:04 p.m. UTC | #1
On 6/15/2022 3:57 PM, Kalesh A P wrote:
> 
> From: Somnath Kotur<somnath.kotur@broadcom.com>
> 
> The current combination of checks to determine whether autoneg is
> enabled in the card is a bit convoluted and may become incorrect
> as well in the future as one of the fields being used - auto_link_speed
> might become deprecated.
> Switch to using the 'auto_mode' field obtained from the response of
> HWRM_PHY_QCFG cmd as that is always deterministically set by the PHY
> FW.
> Fixed a bug in the 40G check to only look for the bit setting and
> not the actual value.
> Also, check the forced speeds first before trying to enforce the
> auto speeds
> 
> Allow the user to set autoneg speed in all cases except for PAM4 200G
> as PAM4 200G will come up only in forced mode.
> 
> Fixes: c23f9ded0391 ("net/bnxt: support 200G PAM4 link")

Is stable tag (stable@dpdk.org) omitted explicitly?
  
Ajit Khaparde June 19, 2022, 11:11 p.m. UTC | #2
On Thu, Jun 16, 2022 at 10:04 AM Ferruh Yigit <ferruh.yigit@xilinx.com> wrote:
>
> On 6/15/2022 3:57 PM, Kalesh A P wrote:
> >
> > From: Somnath Kotur<somnath.kotur@broadcom.com>
> >
> > The current combination of checks to determine whether autoneg is
> > enabled in the card is a bit convoluted and may become incorrect
> > as well in the future as one of the fields being used - auto_link_speed
> > might become deprecated.
> > Switch to using the 'auto_mode' field obtained from the response of
> > HWRM_PHY_QCFG cmd as that is always deterministically set by the PHY
> > FW.
> > Fixed a bug in the 40G check to only look for the bit setting and
> > not the actual value.
> > Also, check the forced speeds first before trying to enforce the
> > auto speeds
> >
> > Allow the user to set autoneg speed in all cases except for PAM4 200G
> > as PAM4 200G will come up only in forced mode.
> >
> > Fixes: c23f9ded0391 ("net/bnxt: support 200G PAM4 link")
>
> Is stable tag (stable@dpdk.org) omitted explicitly?
I think it was an oversight.
  

Patch

diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index 206ac20..9c52573 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -3235,9 +3235,11 @@  int bnxt_set_hwrm_link_config(struct bnxt *bp, bool link_up)
 	if (!link_up)
 		goto port_phy_cfg;
 
+	/* Get user requested autoneg setting */
 	autoneg = bnxt_check_eth_link_autoneg(dev_conf->link_speeds);
+
 	if (BNXT_CHIP_P5(bp) &&
-	    dev_conf->link_speeds == RTE_ETH_LINK_SPEED_40G) {
+	    dev_conf->link_speeds & RTE_ETH_LINK_SPEED_40G) {
 		/* 40G is not supported as part of media auto detect.
 		 * The speed should be forced and autoneg disabled
 		 * to configure 40G speed.
@@ -3246,11 +3248,13 @@  int bnxt_set_hwrm_link_config(struct bnxt *bp, bool link_up)
 		autoneg = 0;
 	}
 
-	/* No auto speeds and no auto_pam4_link. Disable autoneg */
-	if (bp->link_info->auto_link_speed == 0 &&
-	    bp->link_info->link_signal_mode &&
-	    bp->link_info->auto_pam4_link_speed_mask == 0)
+	/* Override based on current Autoneg setting in PHY for 200G */
+	if (autoneg == 1 && BNXT_CHIP_P5(bp) && bp->link_info->auto_mode == 0 &&
+	    bp->link_info->force_pam4_link_speed ==
+	    HWRM_PORT_PHY_CFG_INPUT_FORCE_PAM4_LINK_SPEED_200GB) {
 		autoneg = 0;
+		PMD_DRV_LOG(DEBUG, "Disabling autoneg for 200G\n");
+	}
 
 	speed = bnxt_parse_eth_link_speed(dev_conf->link_speeds,
 					  bp->link_info);
@@ -3283,14 +3287,14 @@  int bnxt_set_hwrm_link_config(struct bnxt *bp, bool link_up)
 		else if (bp->link_info->force_pam4_link_speed)
 			link_req.link_speed =
 				bp->link_info->force_pam4_link_speed;
+		else if (bp->link_info->force_link_speed)
+			link_req.link_speed = bp->link_info->force_link_speed;
 		else if (bp->link_info->auto_pam4_link_speed_mask)
 			link_req.link_speed =
 				bp->link_info->auto_pam4_link_speed_mask;
 		else if (bp->link_info->support_pam4_speeds)
 			link_req.link_speed =
 				bp->link_info->support_pam4_speeds;
-		else if (bp->link_info->force_link_speed)
-			link_req.link_speed = bp->link_info->force_link_speed;
 		else
 			link_req.link_speed = bp->link_info->auto_link_speed;
 		/* Auto PAM4 link speed is zero, but auto_link_speed is not