Messages in this thread | | | From | "Gan, Yi Fang" <> | Subject | RE: [PATCH net-next 1/1] net: stmmac: Remove redundant checking for rx_coalesce_usecs | Date | Thu, 12 Oct 2023 08:11:03 +0000 |
| |
Hi Andrew,
Thank you for your feedback. I will submit V2 to update the commit message.
The value of rx-usecs will not be affected when the tx-usecs is set. When the command "ethtool -C eth24 tx-usecs 42" is applied, the value of rx-usecs is remaining the same as previously.
Best Regards, Gan Yi Fang
> -----Original Message----- > From: Andrew Lunn <andrew@lunn.ch> > Sent: Tuesday, May 23, 2023 8:48 PM > To: Gan, Yi Fang <yi.fang.gan@intel.com> > Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>; Alexandre Torgue > <alexandre.torgue@st.com>; Jose Abreu <joabreu@synopsys.com>; David S . > Miller <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Jakub > Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; Maxime > Coquelin <mcoquelin.stm32@gmail.com>; netdev@vger.kernel.org; linux- > stm32@st-md-mailman.stormreply.com; linux-arm-kernel@lists.infradead.org; > linux-kernel@vger.kernel.org; Looi, Hong Aun <hong.aun.looi@intel.com>; Sit, > Michael Wei Hong <michael.wei.hong.sit@intel.com> > Subject: Re: [PATCH net-next 1/1] net: stmmac: Remove redundant checking for > rx_coalesce_usecs > > On Tue, May 23, 2023 at 02:19:52AM -0400, Gan Yi Fang wrote: > > The datatype of rx_coalesce_usecs is u32, always larger or equal to zero. > > Previous checking does not include value 0, this patch removes the > > checking to handle the value 0. > > > > Signed-off-by: Gan Yi Fang <yi.fang.gan@intel.com> > > --- > > drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c > > b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c > > index 35c8dd92d369..6ed0e683b5e0 100644 > > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c > > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c > > @@ -917,7 +917,7 @@ static int __stmmac_set_coalesce(struct net_device > *dev, > > else if (queue >= max_cnt) > > return -EINVAL; > > > > - if (priv->use_riwt && (ec->rx_coalesce_usecs > 0)) { > > + if (priv->use_riwt) { > > rx_riwt = stmmac_usec2riwt(ec->rx_coalesce_usecs, priv); > > > > if ((rx_riwt > MAX_DMA_RIWT) || (rx_riwt < > MIN_DMA_RIWT)) > > This appears to be a user visible ABI change. For the current code, a value of > zero here is ignored, and 0 is returned. With this change, 0 will result in rx_riwt > being calculated as 0, which is less than MIN_DMA_RIWT, so you get -EINVAL > returned. > > I don't know this uAPI too well. What values are passed to this function for: > > ethtool -C eth24 tx-usecs 42 > > where you only want to change transmit coalesce? Is rx_usecs 0? > > At minimum you need to explain in the commit message: "This change in > behaviour making the value of 0 cause an error is not a problem because...." > > Andrew > > --- > pw-bot: cr
| |