On Tuesday, July 30, 2024 5:59 PM, Kurt Kanzenbach <kurt@xxxxxxxxxxxxx> wrote: >> --- a/drivers/net/ethernet/intel/igc/igc_regs.h >> +++ b/drivers/net/ethernet/intel/igc/igc_regs.h >> @@ -63,6 +63,12 @@ >> /* RSS registers */ >> #define IGC_MRQC 0x05818 /* Multiple Receive Control - RW */ >> >> +/* MRQC register bit definitions */ > >Nit: Now, the MRQC register definitions are scattered over two files: >igc_regs.h and igc.h. igc.h has > >#define IGC_MRQC_ENABLE_RSS_MQ 0x00000002 >#define IGC_MRQC_RSS_FIELD_IPV4_UDP 0x00400000 >#define IGC_MRQC_RSS_FIELD_IPV6_UDP 0x00800000 > >Maybe combine them into a single location? > Hi Kurt Kanzenbach, Thanks for your review comment. Sure, I will try to combine them into single location. >> +#define IGC_MRQC_ENABLE_MQ 0x00000000 >> +#define IGC_MRQC_ENABLE_MASK GENMASK(2, 0) >> +#define IGC_MRQC_DEFAULT_QUEUE_MASK GENMASK(5, 3) >> +#define IGC_MRQC_DEFAULT_QUEUE_SHIFT 3 > >Nit: FIELD_GET() and FIELD_PREP() can help to get rid of the manual >shifting. See below. > Noted. [...] >> + return (mrqc & IGC_MRQC_DEFAULT_QUEUE_MASK) >> >> + IGC_MRQC_DEFAULT_QUEUE_SHIFT; > >Nit: return FIELD_GET(IGC_MRQC_DEFAULT_QUEUE_MASK, mrqc); > Noted. [...] >> + mrqc &= ~IGC_MRQC_DEFAULT_QUEUE_MASK; >> + mrqc |= queue << IGC_MRQC_DEFAULT_QUEUE_SHIFT; > >Nit: mrqc |= FIELD_PREP(IGC_MRQC_DEFAULT_QUEUE_MASK, queue); > Noted. [...] >Thanks, >Kurt Thanks & Regards Siang
