Re: [PATCH] net: davinci_emac: Fix interrupt pacing disable
From: Grygorii Strashko
Date: Mon Nov 01 2021 - 08:34:33 EST
On 01/11/2021 14:05, Maxim Kiselev wrote:
Yes, I can write 0 to INTCTRL for ` case EMAC_VERSION_2` but we also
need to handle `default case`
pls, do not top post.
пн, 1 нояб. 2021 г. в 14:54, Grygorii Strashko <grygorii.strashko@xxxxxx>:
On 01/11/2021 11:21, Maxim Kiselev wrote:
This patch allows to use 0 for `coal->rx_coalesce_usecs` param to
disable rx irq coalescing.
Previously we could enable rx irq coalescing via ethtool
(For ex: `ethtool -C eth0 rx-usecs 2000`) but we couldn't disable
it because this part rejects 0 value:
if (!coal->rx_coalesce_usecs)
return -EINVAL;
Fixes: 84da2658a619 ("TI DaVinci EMAC : Implement interrupt pacing
functionality.")
Signed-off-by: Maxim Kiselev <bigunclemax@xxxxxxxxx>
---
drivers/net/ethernet/ti/davinci_emac.c | 77 ++++++++++++++------------
1 file changed, 41 insertions(+), 36 deletions(-)
diff --git a/drivers/net/ethernet/ti/davinci_emac.c b/drivers/net/ethernet/ti/davinci_emac.c
index e8291d8488391..a3a02c4e5eb68 100644
--- a/drivers/net/ethernet/ti/davinci_emac.c
+++ b/drivers/net/ethernet/ti/davinci_emac.c
@@ -417,46 +417,47 @@ static int emac_set_coalesce(struct net_device *ndev,
struct netlink_ext_ack *extack)
{
struct emac_priv *priv = netdev_priv(ndev);
- u32 int_ctrl, num_interrupts = 0;
+ u32 int_ctrl = 0, num_interrupts = 0;
u32 prescale = 0, addnl_dvdr = 1, coal_intvl = 0;
- if (!coal->rx_coalesce_usecs)
- return -EINVAL;
-
coal_intvl = coal->rx_coalesce_usecs;
Wouldn't be more simple if you just handle !coal->rx_coalesce_usecs here and exit?
it seems you can just write 0 t0 INTCTRL.
nothing prevents you from handling all cases here, like
if (!coal->rx_coalesce_usecs)
switch (priv->version) {
case EMAC_VERSION_2:
emac_ctrl_write(EMAC_DM646X_CMINTCTRL, 0);
break;
default:
emac_ctrl_write(EMAC_CTRL_EWINTTCNT, 0);
break;
}
return 0;
}
No?
switch (priv->version) {
case EMAC_VERSION_2:
- int_ctrl = emac_ctrl_read(EMAC_DM646X_CMINTCTRL);
- prescale = priv->bus_freq_mhz * 4;
-
- if (coal_intvl < EMAC_DM646X_CMINTMIN_INTVL)
- coal_intvl = EMAC_DM646X_CMINTMIN_INTVL;
-
- if (coal_intvl > EMAC_DM646X_CMINTMAX_INTVL) {
- /*
- * Interrupt pacer works with 4us Pulse, we can
- * throttle further by dilating the 4us pulse.
- */
- addnl_dvdr = EMAC_DM646X_INTPRESCALE_MASK / prescale;
-
- if (addnl_dvdr > 1) {
- prescale *= addnl_dvdr;
- if (coal_intvl > (EMAC_DM646X_CMINTMAX_INTVL
- * addnl_dvdr))
- coal_intvl = (EMAC_DM646X_CMINTMAX_INTVL
- * addnl_dvdr);
- } else {
- addnl_dvdr = 1;
- coal_intvl = EMAC_DM646X_CMINTMAX_INTVL;
+ if (coal->rx_coalesce_usecs) {
+ int_ctrl = emac_ctrl_read(EMAC_DM646X_CMINTCTRL);
+ prescale = priv->bus_freq_mhz * 4;
+
+ if (coal_intvl < EMAC_DM646X_CMINTMIN_INTVL)
+ coal_intvl = EMAC_DM646X_CMINTMIN_INTVL;
+
+ if (coal_intvl > EMAC_DM646X_CMINTMAX_INTVL) {
+ /*
+ * Interrupt pacer works with 4us Pulse, we can
+ * throttle further by dilating the 4us pulse.
+ */
+ addnl_dvdr =
+ EMAC_DM646X_INTPRESCALE_MASK / prescale;
+
+ if (addnl_dvdr > 1) {
+ prescale *= addnl_dvdr;
+ if (coal_intvl > (EMAC_DM646X_CMINTMAX_INTVL
+ * addnl_dvdr))
+ coal_intvl = (EMAC_DM646X_CMINTMAX_INTVL
+ * addnl_dvdr);
+ } else {
+ addnl_dvdr = 1;
+ coal_intvl = EMAC_DM646X_CMINTMAX_INTVL;
+ }
}
- }
- num_interrupts = (1000 * addnl_dvdr) / coal_intvl;
+ num_interrupts = (1000 * addnl_dvdr) / coal_intvl;
+
+ int_ctrl |= EMAC_DM646X_INTPACEEN;
+ int_ctrl &= (~EMAC_DM646X_INTPRESCALE_MASK);
+ int_ctrl |= (prescale & EMAC_DM646X_INTPRESCALE_MASK);
+ }
- int_ctrl |= EMAC_DM646X_INTPACEEN;
- int_ctrl &= (~EMAC_DM646X_INTPRESCALE_MASK);
- int_ctrl |= (prescale & EMAC_DM646X_INTPRESCALE_MASK);
emac_ctrl_write(EMAC_DM646X_CMINTCTRL, int_ctrl);
emac_ctrl_write(EMAC_DM646X_CMRXINTMAX, num_interrupts);
@@ -466,17 +467,21 @@ static int emac_set_coalesce(struct net_device *ndev,
default:
int_ctrl = emac_ctrl_read(EMAC_CTRL_EWINTTCNT);
int_ctrl &= (~EMAC_DM644X_EWINTCNT_MASK);
- prescale = coal_intvl * priv->bus_freq_mhz;
- if (prescale > EMAC_DM644X_EWINTCNT_MASK) {
- prescale = EMAC_DM644X_EWINTCNT_MASK;
- coal_intvl = prescale / priv->bus_freq_mhz;
+
+ if (coal->rx_coalesce_usecs) {
+ prescale = coal_intvl * priv->bus_freq_mhz;
+ if (prescale > EMAC_DM644X_EWINTCNT_MASK) {
+ prescale = EMAC_DM644X_EWINTCNT_MASK;
+ coal_intvl = prescale / priv->bus_freq_mhz;
+ }
}
+
emac_ctrl_write(EMAC_CTRL_EWINTTCNT, (int_ctrl | prescale));
break;
}
- printk(KERN_INFO"Set coalesce to %d usecs.\n", coal_intvl);
+ netdev_info(ndev, "Set coalesce to %d usecs.\n", coal_intvl);
priv->coal_intvl = coal_intvl;
return 0;
--
Best regards,
grygorii
--
Best regards,
grygorii