Yes, I can write 0 to INTCTRL for ` case EMAC_VERSION_2` but we also
need to handle `default case`
пн, 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.
>
> >
> > 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
