[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <PH0PR11MB58307F45C9803E808D7F984AD8B22@PH0PR11MB5830.namprd11.prod.outlook.com>
Date: Thu, 1 Aug 2024 07:20:26 +0000
From: "Song, Yoong Siang" <yoong.siang.song@...el.com>
To: Kurt Kanzenbach <kurt@...utronix.de>, "Nguyen, Anthony L"
<anthony.l.nguyen@...el.com>, "David S . Miller" <davem@...emloft.net>, "Eric
Dumazet" <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni
<pabeni@...hat.com>, Richard Cochran <richardcochran@...il.com>, "Alexei
Starovoitov" <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>, Jesper
Dangaard Brouer <hawk@...nel.org>, John Fastabend <john.fastabend@...il.com>,
"Gomes, Vinicius" <vinicius.gomes@...el.com>, Jonathan Corbet
<corbet@....net>, "Kitszel, Przemyslaw" <przemyslaw.kitszel@...el.com>,
Shinas Rasheed <srasheed@...vell.com>, "Tian, Kevin" <kevin.tian@...el.com>,
Brett Creeley <brett.creeley@....com>, "Blanco Alcaine, Hector"
<hector.blanco.alcaine@...el.com>, "Hay, Joshua A" <joshua.a.hay@...el.com>,
"Neftin, Sasha" <sasha.neftin@...el.com>
CC: "intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"bpf@...r.kernel.org" <bpf@...r.kernel.org>, "linux-doc@...r.kernel.org"
<linux-doc@...r.kernel.org>
Subject: RE: [PATCH iwl-next,v1 2/3] igc: Add default Rx queue configuration
via sysfs
On Tuesday, July 30, 2024 5:59 PM, Kurt Kanzenbach <kurt@...utronix.de> 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
Powered by blists - more mailing lists