Re: [PATCH net-next v3 02/12] cxgb4: rework the interrupt handling framework

[Potnuri Bharat Teja <bharat@chelsio.com>]

On Friday, October 10/09/26, 2026 at 19:26:13 +0530, Simon Horman wrote:
> On Mon, Oct 05, 2026 at 06:48:20PM -0400, Potnuri Bharat Teja wrote:
> > The slow interrupt path used a per module handler that walked a table
> > of (mask, message) pairs with t4_handle_intr_status() and printed a
> > message for every bit it found set.  Each module handler open coded
> > the read, report and clear sequence, and there was no way to express
> > that a cause bit should be handled by inspecting a sub-register, or
> > that a bit is only fatal when it is also enabled.
> > 
> > Replace it with a data driven framework:
> > 
> >   struct intr_info describes one cause register: its name, the cause
> >   and enable register offsets, the mask of fatal bits, a table of
> >   struct intr_details for decoding individual bits, and an optional
> >   table of struct intr_action entries naming a function to call for a
> >   given cause bit.
> > 
> >   t4_handle_intr() implements the common read, report, dispatch and
> >   clear sequence once, so a module handler reduces to describing its
> >   registers and any nested cause registers it owns.
> > 
> > The module interrupt handlers now return bool rather than void so a
> > fatal condition propagates back to t4_slow_intr_handler() instead of
> > being signalled out of band, and t4_slow_intr_handler() takes a
> > verbose flag controlling whether individual cause bits are decoded
> > and logged.
> > 
> > This changes the interrupt path for all supported adapters.  No
> > functional change is intended for T4, T5 or T6 beyond the wording of
> > the messages emitted.
> > 
> > Signed-off-by: Potnuri Bharat Teja <bharat@chelsio.com>
> 
> Hi Potnuri,
> 
> Thanks for your patch.
> 
> x86_64 allmodconfig W=1 builds with clang 22.1.8 warn that:
> 
>   .../t4_hw.c:6959:3: warning: variable 'fatal' is uninitialized when used here [-Wuninitialized]
>    6959 |                 fatal |= t4_handle_intr(adap, &ii, 0, verbose);
>         |                 ^~~~~
>   .../t4_hw.c:6945:12: note: initialize the variable 'fatal' to silence this warning
>    6945 |         bool fatal;
>         |                   ^
>         |                    = 0
>   1 warning generated.
>   Warning: .../t4_hw.c:7956 function parameter 'adap' not described in 't4_slow_intr_handler'
>   Warning: .../t4_hw.c:7956 Excess function parameter 'adapter' description in 't4_slow_intr_handler' (did you mean one of: 'adap')
>   Warning: .../t4_hw.c:8209 function parameter 'adap' not described in 't4_intr_enable'
>   Warning: .../t4_hw.c:8209 Excess function parameter 'adapter' description in 't4_intr_enable' (did you mean one of: 'adap')
>   Warning: .../t4_hw.c:7956 function parameter 'adap' not described in 't4_slow_intr_handler'
>   Warning: .../t4_hw.c:7956 Excess function parameter 'adapter' description in 't4_slow_intr_handler' (did you mean one of: 'adap')
>   Warning: .../t4_hw.c:8209 function parameter 'adap' not described in 't4_intr_enable'
>   Warning: .../t4_hw.c:8209 Excess function parameter 'adapter' description in 't4_intr_enable' (did you mean one of: 'adap')
> 
> Similarly building patch 3/12, 5/12, 10/12, and 11/12,
> seem to add new warnings reported by gcc 16.2.1 or clang.
> 
> Please make sure your patch compiles for x86_64 allmodconfig W=1
> with out new warnings emitted by either gcc or clang.
> 
> 
> The following functions added in .c files use the inline keyword.
> 
>   static inline char intr_alert_char(u32 cause, u32 enable, u32 fatal)
>   static inline u32 t7_tlstx_reg(u8 instance, u8 channel, u32 reg)
>   static inline uint32_t
> 
> Please don't use inline like that, instead please let the compiler
> choose to inline functions (or not).
> 
> inline functions in .h files are of course fine.
>

Shall send revised patches with above fixes. 
> 
> And checkpatch reports the following errors for patch 4/12.
> 
>   ERROR: missing sentinel in ID array
>   #1734: FILE: drivers/net/ethernet/chelsio/cxgb4/cxgb4_pci.c:289:
>   +static const struct pci_device_id cxgb4_pci_tbl[] = {
>    #define CH_PCI_DEVICE_ID_FUNCTION CXGB4_UNIFIED_PF
>    #define CH_PCI_DEVICE_ID_FUNCTION2 0x0
> 
>    #define CH_PCI_ID_TABLE_ENTRY(devid) \
> 	{ PCI_VDEVICE(CHELSIO, (devid)), CXGB4_UNIFIED_PF }
> 
>    #define CH_PCI_DEVICE_ID_TABLE_DEFINE_END \
> 	{ 0, } \
>    }
> 
>   ERROR: Macros with complex values should be enclosed in parentheses
>   #1741: FILE: drivers/net/ethernet/chelsio/cxgb4/cxgb4_pci.c:296:
>   +#define CH_PCI_DEVICE_ID_TABLE_DEFINE_END \
>   +	{ 0, } \
>   +}
> 
> Please make sure the patchset does not add checkpatch errors.

The above warning is a false positive. I have left these check patch warnings as is the patch only 
moves the older code to a new file but introduce any new warnings. Ideally this can be fixed by 
removing the unwanted # define wrappers. However that needs that same changes needs to be done in csiostor 
driver as well. Therefore i left them as is to confine the current serices to cxgb4 driver changes 
and do it as seperate patch later.
I have included the same in cover letter about these checkpatch warnings.

If you suggest fixing this, i will include the changes to my next series.

Thanks!

> 
> ...
> 
> -- 
> pw-bot: changes-requested