Cmnt: [SRU][J:linux-bluefield][PATCH v1 1/1] UBUNTU: SAUCE: mlxbf_gige: move BF2 autoneg logic out of open()
David Thompson
davthompson at nvidia.com
Tue Mar 4 14:29:20 UTC 2025
> -----Original Message-----
> From: Stewart Hore <stewart.hore at canonical.com>
> Sent: Monday, March 3, 2025 7:56 PM
> To: David Thompson <davthompson at nvidia.com>
> Cc: kernel-team at lists.ubuntu.com
> Subject: Cmnt: [SRU][J:linux-bluefield][PATCH v1 1/1] UBUNTU: SAUCE:
> mlxbf_gige: move BF2 autoneg logic out of open()
>
> On Mon, Mar 03, 2025 at 12:02:25PM -0500, David Thompson wrote:
> > BugLink: https://bugs.launchpad.net/bugs/2100785
> >
> > This patch moves the BF2 autoneg logic from open() into a workqueue
> > function scheduled by a timer. So, the logic to check for autoneg
> > done, and restart autoneg if necessary is now done outside the open().
> >
> > Signed-off-by: David Thompson <davthompson at nvidia.com>
> > Reviewed-by: Asmaa Mnebhi <asmaa at nvidia.com>
> > ---
> > .../ethernet/mellanox/mlxbf_gige/mlxbf_gige.h | 6 ++
> > .../mellanox/mlxbf_gige/mlxbf_gige_main.c | 59 +++++++++++++++----
> > 2 files changed, 55 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige.h
> > b/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige.h
> > index e7777700ee18..1fc33039c231 100644
> > --- a/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige.h
> > +++ b/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige.h
> > @@ -130,8 +130,14 @@ struct mlxbf_gige {
> > u8 hw_version;
> > struct mlxbf_gige_mdio_gw *mdio_gw;
> > int prev_speed;
> > + struct timer_list media_timer;
> > + struct work_struct phy_task;
> > + u8 aneg_timeout;
> > };
> >
> > +/* BF2 autoneg takes 3-10 secs, set timeout to 50% over max */
> > +#define MLXBF_GIGE_ANEG_TIMEOUT 15
> > +
> > /* Rx Work Queue Element definitions */
> > #define MLXBF_GIGE_RX_WQE_SZ 8
> >
> > diff --git
> > a/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_main.c
> > b/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_main.c
> > index fd164f31cb0f..d99bb9dd82c8 100644
> > --- a/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_main.c
> > +++ b/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_main.c
> > @@ -5,6 +5,8 @@
> > * Copyright (C) 2020-2021 NVIDIA CORPORATION & AFFILIATES
> > */
> >
> > +#define DEBUG
> > +
>
> Did you intentionally leave DEBUG defined here?
>
>
> > #include <linux/acpi.h>
> > #include <linux/device.h>
> > #include <linux/dma-mapping.h>
> > @@ -17,6 +19,7 @@
> > #include <linux/platform_device.h>
> > #include <linux/rtnetlink.h>
> > #include <linux/skbuff.h>
> > +#include <linux/timer.h>
> >
> > #include "mlxbf_gige.h"
> > #include "mlxbf_gige_regs.h"
> > @@ -128,11 +131,43 @@ static int mlxbf_gige_clean_port(struct mlxbf_gige
> *priv)
> > return err;
> > }
> >
> > +static void mlxbf_gige_phy_task(struct work_struct *work) {
> > + struct mlxbf_gige *priv = container_of(work, struct mlxbf_gige, phy_task);
> > + struct phy_device *phydev = priv->netdev->phydev;
> > + int aneg_done;
> > +
> > + aneg_done = phy_aneg_done(phydev);
> > +
> > + if (aneg_done == 0) {
> > + dev_dbg(priv->dev, "phy_task: autoneg pending, timeout=%d\n",
> priv->aneg_timeout);
> > + if (priv->aneg_timeout--) {
> > + /* Start timer to check again in one second */
> > + priv->media_timer.expires = jiffies + HZ;
> > + add_timer(&priv->media_timer);
> > + } else {
> > + /* Upon timeout, restart autoneg once */
> > + dev_dbg(priv->dev, "phy_task: restarting autoneg,
> status=0x%x\n",
> > + phy_restart_aneg(phydev));
> > + }
> > + } else if (aneg_done < 0) {
> > + dev_dbg(priv->dev, "phy_task: autoneg failed\n");
> > + } else {
> > + dev_dbg(priv->dev, "phy_task: autoneg succeeded\n");
> > + }
> > +}
> > +
> > +static void mlxbf_gige_phy_timer(struct timer_list *t) {
> > + struct mlxbf_gige *priv = from_timer(priv, t, media_timer);
> > +
> > + schedule_work(&priv->phy_task);
> > +}
> > +
> > static int mlxbf_gige_open(struct net_device *netdev) {
> > struct mlxbf_gige *priv = netdev_priv(netdev);
> > struct phy_device *phydev = netdev->phydev;
> > - u8 timeout = 10;
> > u64 control;
> > u64 int_en;
> > int err;
> > @@ -162,15 +197,10 @@ static int mlxbf_gige_open(struct net_device
> *netdev)
> > * to complete autonegotiation and so the link remains down.
> > * The software workaround is to restart autonegotiation.
> > */
> > - while (timeout) {
> > - if (phy_aneg_done(phydev))
> > - break;
> > - msleep(1000);
> > - timeout--;
> > - };
> > -
> > - if (timeout == 0)
> > - phy_restart_aneg(phydev);
> > + priv->aneg_timeout = MLXBF_GIGE_ANEG_TIMEOUT;
> > + timer_setup(&priv->media_timer, mlxbf_gige_phy_timer, 0);
> > + priv->media_timer.expires = jiffies + HZ;
> > + add_timer(&priv->media_timer);
> > }
> >
> > err = mlxbf_gige_tx_init(priv);
> > @@ -233,6 +263,11 @@ static int mlxbf_gige_stop(struct net_device *netdev)
> > struct mlxbf_gige *priv = netdev_priv(netdev);
> > u64 control;
> >
> > + if (priv->hw_version == MLXBF_GIGE_VERSION_BF2) {
> > + del_timer_sync(&priv->media_timer);
> > + cancel_work_sync(&priv->phy_task);
> > + }
> > +
> > control = readq(priv->base + MLXBF_GIGE_CONTROL);
> > control &= ~MLXBF_GIGE_CONTROL_PORT_EN;
> > writeq(control, priv->base + MLXBF_GIGE_CONTROL); @@ -460,6
> +495,10
> > @@ static int mlxbf_gige_probe(struct platform_device *pdev)
> > priv->llu_base = llu_base;
> > priv->plu_base = plu_base;
> >
> > + if (priv->hw_version == MLXBF_GIGE_VERSION_BF2) {
> > + INIT_WORK(&priv->phy_task, mlxbf_gige_phy_task);
> > + }
> > +
> > priv->rx_q_entries = MLXBF_GIGE_DEFAULT_RXQ_SZ;
> > priv->tx_q_entries = MLXBF_GIGE_DEFAULT_TXQ_SZ;
> >
> > --
> > 2.43.2
>
> Hi David, you have added `#define DEBUG` in mlxbf_gige_main.c. Just want to
> check this was deliberate.
>
> Thanks
> Stew
>
Hello Stew,
Yes, the DEBUG has been added deliberately.
- Dave
> > --
> > kernel-team mailing list
> > kernel-team at lists.ubuntu.com
> > https://lists.ubuntu.com/mailman/listinfo/kernel-team
More information about the kernel-team
mailing list