RE: [EXTERNAL] Re: [net-next PATCH v5 1/6] octeontx2: Set appropriate PF, VF masks and shifts based on silicon

Hi Jakub,

From: Jakub Kicinski <kuba@kernel.org>
Sent: Sunday, December 8, 2024 8:08 AM
To: Sai Krishna Gajula <redacted>
Cc: davem@davemloft.net; edumazet@google.com; pabeni@redhat.com;
netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Sunil Kovvuri Goutham
[off-list ref]; Geethasowjanya Akula [off-list ref];
Linu Cherian [off-list ref]; Jerin Jacob [off-list ref];
Hariprasad Kelam [off-list ref]; Subbaraya Sundeep Bhatta
[off-list ref]; andrew+netdev@lunn.ch; kalesh-
anakkur.purayil@broadcom.com
Subject: Re: [net-next PATCH v5 1/6] octeontx2: Set appropriate PF, VF masks
and shifts based on silicon

‌
On Wed, 4 Dec 2024 19:38:16 +0530 Sai Krishna wrote:

quoted

-#define RVU_PFVF_PF_SHIFT	10
-#define RVU_PFVF_PF_MASK	0x3F
-#define RVU_PFVF_FUNC_SHIFT	0
-#define RVU_PFVF_FUNC_MASK	0x3FF
+#define RVU_PFVF_PF_SHIFT	rvu_pcifunc_pf_shift
+#define RVU_PFVF_PF_MASK	rvu_pcifunc_pf_mask
+#define RVU_PFVF_FUNC_SHIFT	rvu_pcifunc_func_shift
+#define RVU_PFVF_FUNC_MASK	rvu_pcifunc_func_mask

Why do you maintain these defines? Looks like an unnecessary
indirection.

Given these are simple mask and shift values they probably have trivial
users. Start by adding helpers which perform the conversions using
those, then you can more easily update constants.

There are too many places these masks are used hence added this
indirection.
# grep RVU_PFVF_ drivers/* -inr | wc -l
135

Thanks,
Sundeep

On Mon, 9 Dec 2024 09:09:35 +0000 Subbaraya Sundeep Bhatta wrote:

quoted

On Wed, 4 Dec 2024 19:38:16 +0530 Sai Krishna wrote:  

quoted

-#define RVU_PFVF_PF_SHIFT	10
-#define RVU_PFVF_PF_MASK	0x3F
-#define RVU_PFVF_FUNC_SHIFT	0
-#define RVU_PFVF_FUNC_MASK	0x3FF
+#define RVU_PFVF_PF_SHIFT	rvu_pcifunc_pf_shift
+#define RVU_PFVF_PF_MASK	rvu_pcifunc_pf_mask
+#define RVU_PFVF_FUNC_SHIFT	rvu_pcifunc_func_shift
+#define RVU_PFVF_FUNC_MASK	rvu_pcifunc_func_mask  

Why do you maintain these defines? Looks like an unnecessary
indirection.

Given these are simple mask and shift values they probably have trivial
users. Start by adding helpers which perform the conversions using
those, then you can more easily update constants.

There are too many places these masks are used hence added this
indirection.
# grep RVU_PFVF_ drivers/* -inr | wc -l
135

Yes, I have checked before making the suggestion.
Add a helper first, you can use cocci to do the conversions.