Message ID | 1470197640-13587-2-git-send-email-siva.kallam@broadcom.com |
---|---|
State | Accepted, archived |
Delegated to: | David Miller |
Headers | show |
On 08/02/2016 09:13 PM, skallam wrote: > From: Satish Baddipadige <satish.baddipadige@broadcom.com> > > When the rx coalescing time is 0, interrupts > are not generated from the controller and rx path hangs. > To avoid this rx hang, updating the driver to not allow > rx coalescing time to be 0. > > Signed-off-by: Satish Baddipadige <satish.baddipadige@broadcom.com> > Signed-off-by: Siva Reddy Kallam <siva.kallam@broadcom.com> > Signed-off-by: Michael Chan <michael.chan@broadcom.com> > --- > drivers/net/ethernet/broadcom/tg3.c | 1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > > diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c > index ff300f7..f3c6c91 100644 > --- a/drivers/net/ethernet/broadcom/tg3.c > +++ b/drivers/net/ethernet/broadcom/tg3.c > @@ -14014,6 +14014,7 @@ static int tg3_set_coalesce(struct net_device *dev, struct ethtool_coalesce *ec) > } > > if ((ec->rx_coalesce_usecs > MAX_RXCOL_TICKS) || > + (!ec->rx_coalesce_usecs) || > (ec->tx_coalesce_usecs > MAX_TXCOL_TICKS) || > (ec->rx_max_coalesced_frames > MAX_RXMAX_FRAMES) || > (ec->tx_max_coalesced_frames > MAX_TXMAX_FRAMES) || > Should anything then happen with: /* No rx interrupts will be generated if both are zero */ if ((ec->rx_coalesce_usecs == 0) && (ec->rx_max_coalesced_frames == 0)) return -EINVAL; which is the next block of code? The logic there seems to suggest that it was intended to be able to have an rx_coalesce_usecs of 0 and rely on packet arrival to trigger an interrupt. Presumably setting rx_max_coalesced_frames to 1 to disable interrupt coalescing. happy benchmarking, rick jones
On Wed, Aug 3, 2016 at 9:04 AM, Rick Jones <rick.jones2@hpe.com> wrote: > > Should anything then happen with: > > /* No rx interrupts will be generated if both are zero */ > if ((ec->rx_coalesce_usecs == 0) && > (ec->rx_max_coalesced_frames == 0)) > return -EINVAL; > > > which is the next block of code? The logic there seems to suggest that it > was intended to be able to have an rx_coalesce_usecs of 0 and rely on packet > arrival to trigger an interrupt. Presumably setting rx_max_coalesced_frames > to 1 to disable interrupt coalescing. > I remember writing this block of code over 10 years ago for early generations of the chip. Newer chips seem to behave differently and rx_coalesce_usecs can never be zero. So this block can be removed now that the condition can never be true. We should probably leave a comment there for future reference.
diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c index ff300f7..f3c6c91 100644 --- a/drivers/net/ethernet/broadcom/tg3.c +++ b/drivers/net/ethernet/broadcom/tg3.c @@ -14014,6 +14014,7 @@ static int tg3_set_coalesce(struct net_device *dev, struct ethtool_coalesce *ec) } if ((ec->rx_coalesce_usecs > MAX_RXCOL_TICKS) || + (!ec->rx_coalesce_usecs) || (ec->tx_coalesce_usecs > MAX_TXCOL_TICKS) || (ec->rx_max_coalesced_frames > MAX_RXMAX_FRAMES) || (ec->tx_max_coalesced_frames > MAX_TXMAX_FRAMES) ||