Message ID | 20250305094623.2819994-1-saikrishnag@marvell.com (mailing list archive) |
---|---|
State | Changes Requested |
Delegated to: | Netdev Maintainers |
Headers | show |
Series | [net-next,v2] octeontx2-af: fix build warnings flagged by clang, sparse ,kernel test robot | expand |
On Wed, Mar 05, 2025 at 03:16:23PM +0530, Sai Krishna wrote: > This cleanup patch avoids build warnings flagged by clang, > sparse, kernel test robot. > > Warning reported by clang: > drivers/net/ethernet/marvell/octeontx2/af/rvu.c:2993:47: > warning: arithmetic between different enumeration types > ('enum rvu_af_int_vec_e' and 'enum rvu_pf_int_vec_e') > [-Wenum-enum-conversion] > 2993 | return (pfvf->msix.max >= RVU_AF_INT_VEC_CNT + > RVU_PF_INT_VEC_CNT) && Hi Sai, I think it would be good to address each set of errors in separate patches. And in each case include a report of the errors the tools reported. And I think that the subject(s) could be tightened up a bit. E.g.: Subject: octeontx2-af: correct __iomem annotations Sparse flags a number of inconsistent usage of __iomem annotations: .../otx2_pf.c:611:24: sparse: expected void [noderef] __iomem *hwbase .../otx2_pf.c:611:24: sparse: got void * .../otx2_pf.c:620:56: sparse: sparse: cast removes address space '__iomem' of expression .../otx2_pf.c:671:35: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void volatile [noderef] __iomem *addr @@ got void *hwbase @@ .../otx2_pf.c:671:35: sparse: expected void volatile [noderef] __iomem *addr .../otx2_pf.c:671:35: sparse: got void *hwbase .../otx2_pf.c:1344:21: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected unsigned long long [usertype] *ptr @@ got void [noderef] __iomem * @@ .../otx2_pf.c:1344:21: sparse: expected unsigned long long [usertype] *ptr .../otx2_pf.c:1344:21: sparse: got void [noderef] __iomem * .../otx2_pf.c:1383:21: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected unsigned long long [usertype] *ptr @@ got void [noderef] __iomem * @@ .../otx2_pf.c:1383:21: sparse: expected unsigned long long [usertype] *ptr .../otx2_pf.c:1383:21: sparse: got void [noderef] __iomem * .../otx2_pf.c: note: in included file (through .../mbox.h, .../otx2_common.h): Address this by, ... Reported-by: ... ... > > Reported-by: kernel test robot <lkp@intel.com> > Closes: > https://lore.kernel.org/oe-kbuild-all/202410221614.07o9QVjo-lkp@intel.com/ > Signed-off-by: Sai Krishna <saikrishnag@marvell.com> > --- > drivers/net/ethernet/marvell/octeontx2/af/common.h | 2 +- > drivers/net/ethernet/marvell/octeontx2/af/rvu.c | 14 ++++++++------ > .../ethernet/marvell/octeontx2/nic/otx2_common.c | 10 +++++----- > .../net/ethernet/marvell/octeontx2/nic/otx2_pf.c | 9 ++++----- > 4 files changed, 18 insertions(+), 17 deletions(-) > > diff --git a/drivers/net/ethernet/marvell/octeontx2/af/common.h b/drivers/net/ethernet/marvell/octeontx2/af/common.h > index 406c59100a35..8a08bebf08c2 100644 > --- a/drivers/net/ethernet/marvell/octeontx2/af/common.h > +++ b/drivers/net/ethernet/marvell/octeontx2/af/common.h > @@ -39,7 +39,7 @@ struct qmem { > void *base; > dma_addr_t iova; > int alloc_sz; > - u16 entry_sz; > + u32 entry_sz; > u8 align; > u32 qsize; > }; Further to my point above, I am unsure what problem this is addressing. > diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu.c > index cd0d7b7774f1..c850ea5d1960 100644 > --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu.c > +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu.c > @@ -591,7 +591,7 @@ static void rvu_check_min_msix_vec(struct rvu *rvu, int nvecs, int pf, int vf) > > check_pf: > if (pf == 0) > - min_vecs = RVU_AF_INT_VEC_CNT + RVU_PF_INT_VEC_CNT; > + min_vecs = (int)RVU_AF_INT_VEC_CNT + (int)RVU_PF_INT_VEC_CNT; > else > min_vecs = RVU_PF_INT_VEC_CNT; > I think that in the light of Linus's feedback and the subsequent patch that demoted -Wenum-enum-conversion from W=1 to W=1 this is not necessary. [1] https://lore.kernel.org/all/CAHk-=wjMux0w49bTdSbC3DOoc9FRctDrRvaqFUS4KFTmkbtKWg@mail.gmail.com/ [2] 8f6629c004b1 ("kbuild: Move -Wenum-enum-conversion to W=2") > @@ -819,13 +819,14 @@ static int rvu_fwdata_init(struct rvu *rvu) > goto fail; > > BUILD_BUG_ON(offsetof(struct rvu_fwdata, cgx_fw_data) > FWDATA_CGX_LMAC_OFFSET); > - rvu->fwdata = ioremap_wc(fwdbase, sizeof(struct rvu_fwdata)); > + rvu->fwdata = (__force struct rvu_fwdata *) > + ioremap_wc(fwdbase, sizeof(struct rvu_fwdata)); I am concerned that this and similar changes in this patch are masking problems. In my view __iomem annotations are there for a reason, to help use the correct access mechanism for iomem. So my question is why is that not the case for fwdata? Similarly for other cases in this patch where __iomem is cast or cast-away. ...
> -----Original Message----- > From: Simon Horman <horms@kernel.org> > Sent: Thursday, March 6, 2025 10:13 PM > To: Sai Krishna Gajula <saikrishnag@marvell.com> > Cc: davem@davemloft.net; edumazet@google.com; kuba@kernel.org; > pabeni@redhat.com; netdev@vger.kernel.org; linux-kernel@vger.kernel.org; > Sunil Kovvuri Goutham <sgoutham@marvell.com>; Geethasowjanya Akula > <gakula@marvell.com>; Linu Cherian <lcherian@marvell.com>; Jerin Jacob > <jerinj@marvell.com>; Hariprasad Kelam <hkelam@marvell.com>; Subbaraya > Sundeep Bhatta <sbhatta@marvell.com>; andrew+netdev@lunn.ch; Bharat > Bhushan <bbhushan2@marvell.com>; nathan@kernel.org; > ndesaulniers@google.com; morbo@google.com; justinstitt@google.com; > llvm@lists.linux.dev; kernel test robot <lkp@intel.com> > Subject: Re: [net-next PATCH v2] octeontx2-af: fix build warnings > flagged by clang, sparse ,kernel test robot > > On Wed, Mar 05, 2025 at 03: 16: 23PM +0530, Sai Krishna wrote: > This > cleanup patch avoids build warnings flagged by clang, > sparse, kernel test > robot. > > Warning reported by clang: > > drivers/net/ethernet/marvell/octeontx2/af/rvu. c: 2993: 47: > On Wed, Mar 05, 2025 at 03:16:23PM +0530, Sai Krishna wrote: > > This cleanup patch avoids build warnings flagged by clang, sparse, > > kernel test robot. > > > > Warning reported by clang: > > drivers/net/ethernet/marvell/octeontx2/af/rvu.c:2993:47: > > warning: arithmetic between different enumeration types ('enum > > rvu_af_int_vec_e' and 'enum rvu_pf_int_vec_e') > > [-Wenum-enum-conversion] > > 2993 | return (pfvf->msix.max >= RVU_AF_INT_VEC_CNT + > > RVU_PF_INT_VEC_CNT) && > > Hi Sai, > > I think it would be good to address each set of errors in separate patches. > And in each case include a report of the errors the tools reported. > > And I think that the subject(s) could be tightened up a bit. > E.g.: > > Subject: octeontx2-af: correct __iomem annotations > > Sparse flags a number of inconsistent usage of __iomem annotations: > > .../otx2_pf.c:611:24: sparse: expected void [noderef] __iomem > *hwbase > .../otx2_pf.c:611:24: sparse: got void * > .../otx2_pf.c:620:56: sparse: sparse: cast removes address space > '__iomem' of expression > .../otx2_pf.c:671:35: sparse: sparse: incorrect type in argument 1 > (different address spaces) @@ expected void volatile [noderef] __iomem > *addr @@ got void *hwbase @@ > .../otx2_pf.c:671:35: sparse: expected void volatile [noderef] __iomem > *addr > .../otx2_pf.c:671:35: sparse: got void *hwbase > .../otx2_pf.c:1344:21: sparse: sparse: incorrect type in assignment > (different address spaces) @@ expected unsigned long long [usertype] *ptr > @@ got void [noderef] __iomem * @@ > .../otx2_pf.c:1344:21: sparse: expected unsigned long long [usertype] > *ptr > .../otx2_pf.c:1344:21: sparse: got void [noderef] __iomem * > .../otx2_pf.c:1383:21: sparse: sparse: incorrect type in assignment > (different address spaces) @@ expected unsigned long long [usertype] *ptr > @@ got void [noderef] __iomem * @@ > .../otx2_pf.c:1383:21: sparse: expected unsigned long long [usertype] > *ptr > .../otx2_pf.c:1383:21: sparse: got void [noderef] __iomem * > .../otx2_pf.c: note: in included file (through .../mbox.h, > .../otx2_common.h): > > Address this by, ... > > Reported-by: ... > ... > Ack, will submit separate patches(V3) inline with the review comments > > > > Reported-by: kernel test robot <lkp@intel.com> > > Closes: > > https://urldefense.proofpoint.com/v2/url?u=https-3A__lore.kernel.org_o > > e-2Dkbuild-2Dall_202410221614.07o9QVjo-2Dlkp- > 40intel.com_&d=DwIBAg&c=n > > KjWec2b6R0mOyPaz7xtfQ&r=c3MsgrR-U-HFhmFd6R4MWRZG- > 8QeikJn5PkjqMTpBSg&m= > > > ZRaxGaucMplgEgMUkRbXRrEQcd1pdXjKonN2_LRPrXqdhovwf1Byu3KcxVLZr2 > zr&s=xnV > > HrpqVN46FMyWamv4vgtFbV05b4GjT6yDQcACN2gA&e= > > Signed-off-by: Sai Krishna <saikrishnag@marvell.com> > > --- > > drivers/net/ethernet/marvell/octeontx2/af/common.h | 2 +- > > drivers/net/ethernet/marvell/octeontx2/af/rvu.c | 14 ++++++++------ > > .../ethernet/marvell/octeontx2/nic/otx2_common.c | 10 +++++----- > > .../net/ethernet/marvell/octeontx2/nic/otx2_pf.c | 9 ++++----- > > 4 files changed, 18 insertions(+), 17 deletions(-) > > > > diff --git a/drivers/net/ethernet/marvell/octeontx2/af/common.h > > b/drivers/net/ethernet/marvell/octeontx2/af/common.h > > index 406c59100a35..8a08bebf08c2 100644 > > --- a/drivers/net/ethernet/marvell/octeontx2/af/common.h > > +++ b/drivers/net/ethernet/marvell/octeontx2/af/common.h > > @@ -39,7 +39,7 @@ struct qmem { > > void *base; > > dma_addr_t iova; > > int alloc_sz; > > - u16 entry_sz; > > + u32 entry_sz; > > u8 align; > > u32 qsize; > > }; > > Further to my point above, I am unsure what problem this is addressing. Ack, will submit V3 separate patch mentioning the reason for this fix. > > > diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu.c > > b/drivers/net/ethernet/marvell/octeontx2/af/rvu.c > > index cd0d7b7774f1..c850ea5d1960 100644 > > --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu.c > > +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu.c > > @@ -591,7 +591,7 @@ static void rvu_check_min_msix_vec(struct rvu > > *rvu, int nvecs, int pf, int vf) > > > > check_pf: > > if (pf == 0) > > - min_vecs = RVU_AF_INT_VEC_CNT + RVU_PF_INT_VEC_CNT; > > + min_vecs = (int)RVU_AF_INT_VEC_CNT + > (int)RVU_PF_INT_VEC_CNT; > > else > > min_vecs = RVU_PF_INT_VEC_CNT; > > > > I think that in the light of Linus's feedback and the subsequent patch that > demoted -Wenum-enum-conversion from W=1 to W=1 this is not necessary. Ack, will ignore these changes which are flagged by -Wenum-enum-conversion > > [1] https://urldefense.proofpoint.com/v2/url?u=https- > 3A__lore.kernel.org_all_CAHk-2D- > 3DwjMux0w49bTdSbC3DOoc9FRctDrRvaqFUS4KFTmkbtKWg- > 40mail.gmail.com_&d=DwIBAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=c3MsgrR- > U-HFhmFd6R4MWRZG- > 8QeikJn5PkjqMTpBSg&m=ZRaxGaucMplgEgMUkRbXRrEQcd1pdXjKonN2_LRP > rXqdhovwf1Byu3KcxVLZr2zr&s=SbA5_pKyReJi7lYN-OtFzRqw0T- > nxqE1XFOaSZcoNm8&e= > [2] 8f6629c004b1 ("kbuild: Move -Wenum-enum-conversion to W=2") > > > @@ -819,13 +819,14 @@ static int rvu_fwdata_init(struct rvu *rvu) > > goto fail; > > > > BUILD_BUG_ON(offsetof(struct rvu_fwdata, cgx_fw_data) > > FWDATA_CGX_LMAC_OFFSET); > > - rvu->fwdata = ioremap_wc(fwdbase, sizeof(struct rvu_fwdata)); > > + rvu->fwdata = (__force struct rvu_fwdata *) > > + ioremap_wc(fwdbase, sizeof(struct rvu_fwdata)); > > I am concerned that this and similar changes in this patch are masking > problems. In my view __iomem annotations are there for a reason, to help use > the correct access mechanism for iomem. So my question is why is that not > the case for fwdata? > > Similarly for other cases in this patch where __iomem is cast or cast-away. Ack, will fix and submit V3 patch inline with review comments. > > ...
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/common.h b/drivers/net/ethernet/marvell/octeontx2/af/common.h index 406c59100a35..8a08bebf08c2 100644 --- a/drivers/net/ethernet/marvell/octeontx2/af/common.h +++ b/drivers/net/ethernet/marvell/octeontx2/af/common.h @@ -39,7 +39,7 @@ struct qmem { void *base; dma_addr_t iova; int alloc_sz; - u16 entry_sz; + u32 entry_sz; u8 align; u32 qsize; }; diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu.c index cd0d7b7774f1..c850ea5d1960 100644 --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu.c +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu.c @@ -591,7 +591,7 @@ static void rvu_check_min_msix_vec(struct rvu *rvu, int nvecs, int pf, int vf) check_pf: if (pf == 0) - min_vecs = RVU_AF_INT_VEC_CNT + RVU_PF_INT_VEC_CNT; + min_vecs = (int)RVU_AF_INT_VEC_CNT + (int)RVU_PF_INT_VEC_CNT; else min_vecs = RVU_PF_INT_VEC_CNT; @@ -819,13 +819,14 @@ static int rvu_fwdata_init(struct rvu *rvu) goto fail; BUILD_BUG_ON(offsetof(struct rvu_fwdata, cgx_fw_data) > FWDATA_CGX_LMAC_OFFSET); - rvu->fwdata = ioremap_wc(fwdbase, sizeof(struct rvu_fwdata)); + rvu->fwdata = (__force struct rvu_fwdata *) + ioremap_wc(fwdbase, sizeof(struct rvu_fwdata)); if (!rvu->fwdata) goto fail; if (!is_rvu_fwdata_valid(rvu)) { dev_err(rvu->dev, "Mismatch in 'fwdata' struct btw kernel and firmware\n"); - iounmap(rvu->fwdata); + iounmap((void __iomem *)rvu->fwdata); rvu->fwdata = NULL; return -EINVAL; } @@ -838,7 +839,7 @@ static int rvu_fwdata_init(struct rvu *rvu) static void rvu_fwdata_exit(struct rvu *rvu) { if (rvu->fwdata) - iounmap(rvu->fwdata); + iounmap((void __iomem *)rvu->fwdata); } static int rvu_setup_nix_hw_resource(struct rvu *rvu, int blkaddr) @@ -2384,7 +2385,8 @@ static int rvu_get_mbox_regions(struct rvu *rvu, void **mbox_addr, bar4 = rvupf_read64(rvu, RVU_PF_VF_BAR4_ADDR); bar4 += region * MBOX_SIZE; } - mbox_addr[region] = (void *)ioremap_wc(bar4, MBOX_SIZE); + mbox_addr[region] = (__force void *) + ioremap_wc(bar4, MBOX_SIZE); if (!mbox_addr[region]) goto error; } @@ -2407,7 +2409,7 @@ static int rvu_get_mbox_regions(struct rvu *rvu, void **mbox_addr, RVU_AF_PF_BAR4_ADDR); bar4 += region * MBOX_SIZE; } - mbox_addr[region] = (void *)ioremap_wc(bar4, MBOX_SIZE); + mbox_addr[region] = (__force void *)ioremap_wc(bar4, MBOX_SIZE); if (!mbox_addr[region]) goto error; } diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c index 2b49bfec7869..e0e592fd02f7 100644 --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c @@ -29,10 +29,10 @@ static void otx2_nix_rq_op_stats(struct queue_stats *stats, u64 incr = (u64)qidx << 32; u64 *ptr; - ptr = (u64 *)otx2_get_regaddr(pfvf, NIX_LF_RQ_OP_OCTS); + ptr = (__force u64 *)otx2_get_regaddr(pfvf, NIX_LF_RQ_OP_OCTS); stats->bytes = otx2_atomic64_add(incr, ptr); - ptr = (u64 *)otx2_get_regaddr(pfvf, NIX_LF_RQ_OP_PKTS); + ptr = (__force u64 *)otx2_get_regaddr(pfvf, NIX_LF_RQ_OP_PKTS); stats->pkts = otx2_atomic64_add(incr, ptr); } @@ -42,10 +42,10 @@ static void otx2_nix_sq_op_stats(struct queue_stats *stats, u64 incr = (u64)qidx << 32; u64 *ptr; - ptr = (u64 *)otx2_get_regaddr(pfvf, NIX_LF_SQ_OP_OCTS); + ptr = (__force u64 *)otx2_get_regaddr(pfvf, NIX_LF_SQ_OP_OCTS); stats->bytes = otx2_atomic64_add(incr, ptr); - ptr = (u64 *)otx2_get_regaddr(pfvf, NIX_LF_SQ_OP_PKTS); + ptr = (__force u64 *)otx2_get_regaddr(pfvf, NIX_LF_SQ_OP_PKTS); stats->pkts = otx2_atomic64_add(incr, ptr); } @@ -853,7 +853,7 @@ void otx2_sqb_flush(struct otx2_nic *pfvf) struct otx2_snd_queue *sq; u64 incr, *ptr, val; - ptr = (u64 *)otx2_get_regaddr(pfvf, NIX_LF_SQ_OP_STATUS); + ptr = (__force u64 *)otx2_get_regaddr(pfvf, NIX_LF_SQ_OP_STATUS); for (qidx = 0; qidx < otx2_get_total_tx_queues(pfvf); qidx++) { sq = &pfvf->qset.sq[qidx]; if (!sq->sqb_ptrs) diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c index e1dde93e8af8..6c23d64e81f8 100644 --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c @@ -595,8 +595,7 @@ static int otx2_pfvf_mbox_init(struct otx2_nic *pf, int numvfs) base = pci_resource_start(pf->pdev, PCI_MBOX_BAR_NUM) + MBOX_SIZE; else - base = readq((void __iomem *)((u64)pf->reg_base + - RVU_PF_VF_BAR4_ADDR)); + base = readq(pf->reg_base + RVU_PF_VF_BAR4_ADDR); hwbase = ioremap_wc(base, MBOX_SIZE * pf->total_vfs); if (!hwbase) { @@ -645,7 +644,7 @@ static void otx2_pfvf_mbox_destroy(struct otx2_nic *pf) } if (mbox->mbox.hwbase) - iounmap(mbox->mbox.hwbase); + iounmap((void __iomem *)mbox->mbox.hwbase); otx2_mbox_destroy(&mbox->mbox); } @@ -1309,7 +1308,7 @@ static irqreturn_t otx2_q_intr_handler(int irq, void *data) /* CQ */ for (qidx = 0; qidx < pf->qset.cq_cnt; qidx++) { - ptr = otx2_get_regaddr(pf, NIX_LF_CQ_OP_INT); + ptr = (__force u64 *)otx2_get_regaddr(pf, NIX_LF_CQ_OP_INT); val = otx2_atomic64_add((qidx << 44), ptr); otx2_write64(pf, NIX_LF_CQ_OP_INT, (qidx << 44) | @@ -1348,7 +1347,7 @@ static irqreturn_t otx2_q_intr_handler(int irq, void *data) * these are fatal errors. */ - ptr = otx2_get_regaddr(pf, NIX_LF_SQ_OP_INT); + ptr = (__force u64 *)otx2_get_regaddr(pf, NIX_LF_SQ_OP_INT); val = otx2_atomic64_add((qidx << 44), ptr); otx2_write64(pf, NIX_LF_SQ_OP_INT, (qidx << 44) | (val & NIX_SQINT_BITS));
This cleanup patch avoids build warnings flagged by clang, sparse, kernel test robot. Warning reported by clang: drivers/net/ethernet/marvell/octeontx2/af/rvu.c:2993:47: warning: arithmetic between different enumeration types ('enum rvu_af_int_vec_e' and 'enum rvu_pf_int_vec_e') [-Wenum-enum-conversion] 2993 | return (pfvf->msix.max >= RVU_AF_INT_VEC_CNT + RVU_PF_INT_VEC_CNT) && Reported-by: kernel test robot <lkp@intel.com> Closes: https://lore.kernel.org/oe-kbuild-all/202410221614.07o9QVjo-lkp@intel.com/ Signed-off-by: Sai Krishna <saikrishnag@marvell.com> --- drivers/net/ethernet/marvell/octeontx2/af/common.h | 2 +- drivers/net/ethernet/marvell/octeontx2/af/rvu.c | 14 ++++++++------ .../ethernet/marvell/octeontx2/nic/otx2_common.c | 10 +++++----- .../net/ethernet/marvell/octeontx2/nic/otx2_pf.c | 9 ++++----- 4 files changed, 18 insertions(+), 17 deletions(-)