Re: [rdma-next v1 13/22] RDMA/hns: Remove empty functions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Aug 21, 2017 at 10:09:10AM +0530, Selvin Xavier wrote:
> On Fri, Aug 18, 2017 at 9:10 PM, Leon Romanovsky <leon@xxxxxxxxxx> wrote:
>
> >>
> >> The following patch helps. But it is like a workaround to solve the problem.
> >>
> >> diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c
> >> index d5ca101..59911dd 100644
> >> --- a/drivers/infiniband/core/cm.c
> >> +++ b/drivers/infiniband/core/cm.c
> >> @@ -4189,7 +4189,7 @@ static void cm_add_one(struct ib_device *ib_device)
> >>                         goto error2;
> >>
> >>                 ret = ib_modify_port(ib_device, i, 0, &port_modify);
> >> -               if (ret)
> >> +               if (ret && ret != -ENOSYS)
> >>                         goto error3;
> >>
> >>                 count++;
> >>
> >> Or should we have the modify_port hook with some basic checks?
> >
> > I think that your proposal is right thing to do. The driver should
> > properly return the status of its callbacks (-ENOSYS) and it is
> > responsibility of the caller to decide what to do in such case.
> >
>
> > The dummy function (return 0) hides the fact that modify_port do
> > nothing and it can be wrong for some callers.
> >
> I am bit confused. You support the proposal to have a modify_port with
> some basic check.. right?

Sorry for not being clear,

Yes, I fully support your suggestion to improve modify_port function.

At least that common modify_port function should check the type of port
and return success for RoCE (see mlx4_ib_modify_port), but need to check
that mlx5_ib_modify_port won't break after that.

Thanks

> > Thanks
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux