Re: [PATCH for-next V3 1/4] RDMA: Introduce ib_port_phys_state enum

[Kamal Heib <kamalheib1@gmail.com>]

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@gmail.com>
> > Reviewed-by: Andrew Boyer <aboyer@tobark.org>
> > ---
> >  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