Re: [PATCH net-next] amd-xgbe: Configure and retrieve 'tx-usecs' for Tx coalescing

[Badole, Vishal]

On 6/16/2025 4:59 PM, Vadim Fedorenko wrote:
> On 16/06/2025 11:42, Vishal Badole wrote:
> > Ethtool has advanced with additional configurable options, but the
> > current driver does not support tx-usecs configuration.
> >
> > Add support to configure and retrieve 'tx-usecs' using ethtool, which
> > specifies the wait time before servicing an interrupt for Tx coalescing.
> >
> > Signed-off-by: Vishal Badole <Vishal.Badole@xxxxxxx>
> > Acked-by: Shyam Sundar S K <Shyam-sundar.S-k@xxxxxxx>
> > ---
> >   drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c | 19 +++++++++++++++++--
> >   drivers/net/ethernet/amd/xgbe/xgbe.h         |  1 +
> >   2 files changed, 18 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c b/drivers/ 
> > net/ethernet/amd/xgbe/xgbe-ethtool.c
> > index 12395428ffe1..362f8623433a 100644
> > --- a/drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c
> > +++ b/drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c
> > @@ -450,6 +450,7 @@ static int xgbe_get_coalesce(struct net_device 
> > *netdev,
> >       ec->rx_coalesce_usecs = pdata->rx_usecs;
> >       ec->rx_max_coalesced_frames = pdata->rx_frames;
> > +    ec->tx_coalesce_usecs = pdata->tx_usecs;
> >       ec->tx_max_coalesced_frames = pdata->tx_frames;
> >       return 0;
> > @@ -463,7 +464,7 @@ static int xgbe_set_coalesce(struct net_device 
> > *netdev,
> >       struct xgbe_prv_data *pdata = netdev_priv(netdev);
> >       struct xgbe_hw_if *hw_if = &pdata->hw_if;
> >       unsigned int rx_frames, rx_riwt, rx_usecs;
> > -    unsigned int tx_frames;
> > +    unsigned int tx_frames, tx_usecs;
> >       rx_riwt = hw_if->usec_to_riwt(pdata, ec->rx_coalesce_usecs);
> >       rx_usecs = ec->rx_coalesce_usecs;
> > @@ -485,9 +486,22 @@ static int xgbe_set_coalesce(struct net_device 
> > *netdev,
> >           return -EINVAL;
> >       }
> > +    tx_usecs = ec->tx_coalesce_usecs;
> >       tx_frames = ec->tx_max_coalesced_frames;
> > +    /* Check if both tx_usecs and tx_frames are set to 0 
> > simultaneously */
> > +    if (!tx_usecs && !tx_frames) {
> > +        netdev_err(netdev,
> > +               "tx_usecs and tx_frames must not be 0 together\n");
> > +        return -EINVAL;
> > +    }
> > +
> >       /* Check the bounds of values for Tx */
> > +    if (tx_usecs > XGMAC_MAX_COAL_TX_TICK) {
> > +        netdev_err(netdev, "tx-usecs is limited to %d usec\n",
> > +               XGMAC_MAX_COAL_TX_TICK);
> > +        return -EINVAL;
> > +    }
>
> ethtool uses netlink interface now and coalesce callbacks have extack
> parameters to return error information back to user-space. It would be
> great to switch to use it instead of adding more netdev_err messages.
Hi Vadim.
Thank you for your observations. Since this driver is quite old, we have 
used netdev_err() to report errors to maintain consistency. In the 
future, we plan to upgrade the driver to use netlink interfaces with 
extack parameters for returning error information to user-space.
> [...]