On Sun, May 19, 2019 at 01:07:34PM +0300, Kamal Heib wrote: > On Fri, 2019-05-17 at 15:01 -0700, Ira Weiny wrote: > > On Thu, May 16, 2019 at 12:19:44PM -0300, Jason Gunthorpe wrote: > > > On Thu, May 16, 2019 at 02:52:48PM +0300, Kamal Heib wrote: > > > > On Thu, 2019-05-16 at 08:37 -0300, Jason Gunthorpe wrote: > > > > > On Thu, May 16, 2019 at 02:28:40PM +0300, Kamal Heib wrote: > > > > > > On Thu, 2019-05-16 at 08:16 -0300, Jason Gunthorpe wrote: > > > > > > > On Thu, May 16, 2019 at 01:53:08PM +0300, Kamal Heib wrote: > > > > > > > > For RoCE ports the call for ib_modify_port is not > > > > > > > > meaningful, > > > > > > > > so > > > > > > > > simplify the providers of RoCE by return OK in ib_core. > > > > > > > > > > > > > > > > Signed-off-by: Kamal Heib <kamalheib1@xxxxxxxxx> > > > > > > > > drivers/infiniband/core/device.c | 23 > > > > > > > > ++++++----- > > > > > > > > drivers/infiniband/hw/hns/hns_roce_main.c | 7 ---- > > > > > > > > drivers/infiniband/hw/mlx4/main.c | 8 ---- > > > > > > > > drivers/infiniband/hw/mlx5/main.c | 6 --- > > > > > > > > drivers/infiniband/hw/ocrdma/ocrdma_main.c | 1 - > > > > > > > > drivers/infiniband/hw/ocrdma/ocrdma_verbs.c | 6 --- > > > > > > > > drivers/infiniband/hw/ocrdma/ocrdma_verbs.h | 2 - > > > > > > > > drivers/infiniband/hw/qedr/main.c | 1 - > > > > > > > > drivers/infiniband/hw/qedr/verbs.c | 6 --- > > > > > > > > drivers/infiniband/hw/qedr/verbs.h | 2 - > > > > > > > > .../infiniband/hw/vmw_pvrdma/pvrdma_main.c | 1 - > > > > > > > > .../infiniband/hw/vmw_pvrdma/pvrdma_verbs.c | 38 --- > > > > > > > > .../infiniband/hw/vmw_pvrdma/pvrdma_verbs.h | 2 - > > > > > > > > drivers/infiniband/sw/rxe/rxe_verbs.c | 18 --- > > > > > > > > 14 files changed, 14 insertions(+), 107 deletions(-) > > > > > > > > > > > > > > We have more roce only drivers than this, why isn't > > > > > > > everything > > > > > > > changed? > > > > > > > > > > > > > > Jason > > > > > > > > > > > > Not all of them implements modify_port(). > > > > > > > > > > Then why didn't we just delete modify port from the other > > > > > drivers? > > > > > > > > > > Jason > > > > > > > > This patch is doing that for all roce drivers that implement > > > > modify > > > > port, unless you mean none-roce drivers? > > > > > > I mean just delete it without any change to the core code.. Here we > > > are now changing some roce drivers to have a working modify_port > > > > > > It is confusing what the intention is > > > > I see what Jason is saying here. If ib_modify_port() is meaningless > > then lets > > remove the call and let it return -EOPNOTSUPP. > > > > Please see below the original implementation of ib_modify_port(), if > modify_port() callback is implemented then it will be called, otherwise > for RoCE driver the return value will be "ok" and for none RoCE driver > it will be "-ENOSYS". > > So, what I tried to do in this patch is to return "ok" (like how it is > done for hns, mlx4, mlx5, ocrdma, and qedr) in the ib_core instead in > the drivers, and changed rxe and vmw_pvrdma to be aligned with this > change. Well, you should explain this in the commit message And explain in the commit why one return code is more correct than the other Jason