if_bnxt: rework interrupt coalescing onto the AGGINT_QCAPS scheme

[sumit.saxena_broadcom.com]

Summary

Query firmware-advertised coalescing capabilities and program
per-direction rx/tx coalescing settings against them, replacing the
old hardcoded scheme. Add sysctls for the coalescing mode, budget,
and stats-timer knobs the new scheme exposes.

[gallatin]

sys/dev/bnxt/bnxt_en/bnxt_hwrm.c

3581

I think you need to explicitly take the hwrm lock and call the _hwrm_send_message() variant. As far as I can see, the resp is shared among commands and zeroed at every use, so once you drop the lock (which hwrm_send_message() does before return), the response could be zeroed or for somebody else's command.

[sumit.saxena_broadcom.com]

sys/dev/bnxt/bnxt_en/bnxt_hwrm.c

3581

Yes, need to take hwrm lock explicitly, will fix it up in v2.

[sumit.saxena_broadcom.com]

Handled review bugs/comments

[sumit.saxena_broadcom.com]

AI scan:

• bnxt_hwrm_set_coal()'s Rx loop discarded bnxt_hwrm_set_coal_nq()'s return value, silently swallowing an NQ coalescing failure with no log, unlike the Tx loop right below it which both logs and propagates. Check and log it the same way.
• bnxt_set_stats_coal_ticks() updated softc->stats_coal_ticks and then called bnxt_hwrm_set_coal(), which only ever reads rx_coal/tx_coal - the stats DMA period is programmed once at HWRM_STAT_CTX_ALLOC time (bnxt_hwrm_stat_ctx_alloc(), update_period_ms hardcoded to 1000) and this driver has no HWRM_STAT_CTX_CFG call to reprogram it, so the sysctl update never reached firmware. Drop the ineffective call instead of leaving a misleading no-op that could also surface an unrelated Rx/Tx coalescing error as if it were a stats-timer failure.
• bnxt_set_coal_rx_frames()/_rx_frames_irq()/_tx_frames()/ _tx_frames_irq(), the _usecs/_usecs_irq variants, and bnxt_set_rx_coal_budget() all read user input into a plain int via sysctl_handle_int() and stored it straight into a uint16_t bnxt_coal field (or uint16_t field = val * mult), with no bound on the input: a negative value or one > 65535 silently truncates or wraps, and for the *_frames* handlers val * mult can overflow uint16_t even for an in-range val. Add bnxt_sysctl_check_u16_range() and call it from all nine handlers before the field is written.
• bnxt_hwrm_set_coal_params() gated populating req->cmpl_aggr_dma_tmr_during_int on CMPL_PARAMS_NUM_CMPL_DMA_AGGR_DURING_INT, the QCAPS bit documented (hsi_struct_def.h) for the unrelated count field (num_cmpl_dma_aggr_during_int, populated unconditionally just above), instead of CMPL_PARAMS_CMPL_AGGR_DMA_TMR_DURING_INT, the bit specifically documented for this timer field. Firmware advertising only one of the two capabilities was gated on the wrong one. Separately, hwrm_ring_cmpl_ring_cfg_aggint_params_input's enables bitfield has no bit of its own for cmpl_aggr_dma_tmr_during_int in this header, unlike the QCAPS output side which does distinguish the two counts/timers; BNXT_COAL_CMPL_AGGR_TMR_DURING_INT_ENABLE reuses the count field's enable bit despite its name, which is unverified against the firmware spec and left as-is - documented in place at both the macro and its use site rather than guessing at an enable value the header doesn't support.
• bnxt_clamp_u16() duplicated the already-available clamp_t(uint16_t, ...) macro. Replaced its seven call sites with clamp_t and removed the helper.
• bnxt_set_rx_coalesce_mode()'s TMR_RESET_ON_ALLOC branch updates timer_reset_during_ring_alloc and returns without calling bnxt_hwrm_set_coal(), unlike the TIMER_RESET branch above it. This is correct - bnxt_hwrm_ring_alloc() reads the flag directly when a completion ring is (re)allocated, there is no live HWRM field to push - but add a comment so a future reader doesn't "fix" the asymmetry by adding a spurious bnxt_hwrm_set_coal() call.