Re: [PATCH v4 for-next 1/8] RDMA/hns: Fix sparse warnings about hr_reg_write()

[Jason Gunthorpe <jgg@nvidia.com>]

On Fri, Jun 18, 2021 at 11:22:46PM +0800, hhh ching wrote:
> Jason Gunthorpe <jgg@nvidia.com> 于2021年6月18日周五 下午8:49写道：
> >
> > On Fri, Jun 18, 2021 at 06:05:58PM +0800, Weihang Li wrote:
> > > Fix complains from sparse about "dubious: x & !y" when calling
> > > hr_reg_write(ctx, field, !!val) by using "val ? 1 : 0" instead of "!!val".
> > >
> > > Fixes: dc504774408b ("RDMA/hns: Use new interface to set MPT related fields")
> > > Fixes: 495c24808ce7 ("RDMA/hns: Add XRC subtype in QPC and XRC type in SRQC")
> > > Fixes: 782832f25404 ("RDMA/hns: Simplify the function config_eqc()")
> > > Signed-off-by: Weihang Li <liweihang@huawei.com>
> > >  drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 14 +++++++-------
> > >  1 file changed, 7 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
> > > index fbc45b9..6452ccc 100644
> > > +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
> > > @@ -3013,15 +3013,15 @@ static int hns_roce_v2_write_mtpt(struct hns_roce_dev *hr_dev,
> > >       hr_reg_enable(mpt_entry, MPT_L_INV_EN);
> > >
> > >       hr_reg_write(mpt_entry, MPT_BIND_EN,
> > > -                  !!(mr->access & IB_ACCESS_MW_BIND));
> > > +                  mr->access & IB_ACCESS_MW_BIND ? 1 : 0);
> >
> > Err, I'm still confused where the sparse warning is coming from
> 
> Hi, Jason, i found some code in sparse/evaluate.c:
> const unsigned left_not = expr->left->type == EXPR_PREOP &&
> expr->left->op == '!';
> const unsigned right_not = expr->right->type == EXPR_PREOP &&
> expr->right->op == '!';
> if ((op == '&' || op == '|') && (left_not || right_not))
>         warning(expr->pos, "dubious: %sx %c %sy",
> 
> I guess the "dubious" is,  if somebody use "&" or "|",  maybe he want
> to bitwise operate a number instead of a bool.

Oh I see, yes, that does many some sense actually

> > A hr_reg_write_bool() would be cleaner?

Which makes me further think this is the right direction

Jason