On Tue, Aug 06, 2019 at 12:56:38PM +0300, Leon Romanovsky wrote:
> On Fri, Aug 02, 2019 at 12:32:07PM +0300, Kamal Heib wrote:
> > In order to improve readability, add ib_port_phys_state enum to replace
> > the use of magic numbers.
> >
> > Signed-off-by: Kamal Heib <kamalheib1@xxxxxxxxx>
> > Reviewed-by: Andrew Boyer <aboyer@xxxxxxxxxx>
> > ---
> > drivers/infiniband/core/sysfs.c | 24 +++++++++++++-------
> > drivers/infiniband/hw/bnxt_re/ib_verbs.c | 4 ++--
> > drivers/infiniband/hw/efa/efa_verbs.c | 2 +-
> > drivers/infiniband/hw/hns/hns_roce_device.h | 10 --------
> > drivers/infiniband/hw/hns/hns_roce_main.c | 3 ++-
> > drivers/infiniband/hw/mlx4/main.c | 3 ++-
> > drivers/infiniband/hw/mlx5/main.c | 4 ++--
> > drivers/infiniband/hw/ocrdma/ocrdma_verbs.c | 4 ++--
> > drivers/infiniband/hw/qedr/verbs.c | 4 ++--
> > drivers/infiniband/hw/usnic/usnic_ib_verbs.c | 7 +++---
> > drivers/infiniband/sw/rxe/rxe.h | 4 ----
> > drivers/infiniband/sw/rxe/rxe_param.h | 2 +-
> > drivers/infiniband/sw/rxe/rxe_verbs.c | 6 ++---
> > drivers/infiniband/sw/siw/siw_verbs.c | 3 ++-
> > include/rdma/ib_verbs.h | 10 ++++++++
> > 15 files changed, 49 insertions(+), 41 deletions(-)
> >
> > diff --git a/drivers/infiniband/core/sysfs.c b/drivers/infiniband/core/sysfs.c
> > index b477295a96c2..46722e04f6e1 100644
> > --- a/drivers/infiniband/core/sysfs.c
> > +++ b/drivers/infiniband/core/sysfs.c
> > @@ -301,14 +301,22 @@ static ssize_t phys_state_show(struct ib_port *p, struct port_attribute *unused,
> > return ret;
> >
> > switch (attr.phys_state) {
> > - case 1: return sprintf(buf, "1: Sleep\n");
> > - case 2: return sprintf(buf, "2: Polling\n");
> > - case 3: return sprintf(buf, "3: Disabled\n");
> > - case 4: return sprintf(buf, "4: PortConfigurationTraining\n");
> > - case 5: return sprintf(buf, "5: LinkUp\n");
> > - case 6: return sprintf(buf, "6: LinkErrorRecovery\n");
> > - case 7: return sprintf(buf, "7: Phy Test\n");
> > - default: return sprintf(buf, "%d: <unknown>\n", attr.phys_state);
> > + case IB_PORT_PHYS_STATE_SLEEP:
> > + return sprintf(buf, "1: Sleep\n");
> > + case IB_PORT_PHYS_STATE_POLLING:
> > + return sprintf(buf, "2: Polling\n");
> > + case IB_PORT_PHYS_STATE_DISABLED:
> > + return sprintf(buf, "3: Disabled\n");
> > + case IB_PORT_PHYS_STATE_PORT_CONFIGURATION_TRAINING:
> > + return sprintf(buf, "4: PortConfigurationTraining\n");
> > + case IB_PORT_PHYS_STATE_LINK_UP:
> > + return sprintf(buf, "5: LinkUp\n");
> > + case IB_PORT_PHYS_STATE_LINK_ERROR_RECOVERY:
> > + return sprintf(buf, "6: LinkErrorRecovery\n");
> > + case IB_PORT_PHYS_STATE_PHY_TEST:
> > + return sprintf(buf, "7: Phy Test\n");
> > + default:
> > + return sprintf(buf, "%d: <unknown>\n", attr.phys_state);
> > }
>
> If you touch that function, the better way to write it will be like here (without OPA)
> https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/tree/rdma/link.c#n209
>
> sprintf(buf, "%d: %s\n", attr.phys_state, phys_state_to_str(attr.phys_state));
>
> Thanks
OK, I'll send v4 soon.
Thanks,
Kamal
