Messages in this thread Patch in this message | | | Subject | Re: [PATCH] net: davinci_emac: Fix interrupt pacing disable | From | Grygorii Strashko <> | Date | Mon, 1 Nov 2021 14:33:51 +0200 |
| |
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@ti.com>: >> >> >> >> 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@gmail.com> >>> --- >>> 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
| |