diff mbox series

[v2] RDMA/qedr: Remove enumerated type qed_roce_ll2_tx_dest

Message ID 20180927205557.32026-1-natechancellor@gmail.com (mailing list archive)
State Accepted
Delegated to: Jason Gunthorpe
Headers show
Series [v2] RDMA/qedr: Remove enumerated type qed_roce_ll2_tx_dest | expand

Commit Message

Nathan Chancellor Sept. 27, 2018, 8:55 p.m. UTC
Clang warns when one enumerated type is explicitly converted to another.

drivers/infiniband/hw/qedr/qedr_roce_cm.c:198:28: warning: implicit
conversion from enumeration type 'enum qed_roce_ll2_tx_dest' to
different enumeration type 'enum qed_ll2_tx_dest' [-Wenum-conversion]
        ll2_tx_pkt.tx_dest = pkt->tx_dest;
                           ~ ~~~~~^~~~~~~
1 warning generated.

Turns out that QED_ROCE_LL2_TX_DEST_NW and QED_ROCE_LL2_TX_DEST_LB are
only used once in the whole tree and QED_ROCE_LL2_TX_DEST_MAX is used
nowhere. Remove them and use the equivalent values from qed_ll2_tx_dest
in their place.

Reported-by: Nick Desaulniers <ndesaulniers@google.com>
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---

v1 -> v2:

* Rather than using an explicit cast, just convert the uses to the
  appropriate values and delete the duplicated enum.

 drivers/infiniband/hw/qedr/qedr_roce_cm.c |  4 ++--
 include/linux/qed/qed_rdma_if.h           | 11 +----------
 2 files changed, 3 insertions(+), 12 deletions(-)

Comments

Nick Desaulniers Sept. 27, 2018, 10:29 p.m. UTC | #1
On Thu, Sep 27, 2018 at 1:57 PM Nathan Chancellor
<natechancellor@gmail.com> wrote:
>
> Clang warns when one enumerated type is explicitly converted to another.
>
> drivers/infiniband/hw/qedr/qedr_roce_cm.c:198:28: warning: implicit
> conversion from enumeration type 'enum qed_roce_ll2_tx_dest' to
> different enumeration type 'enum qed_ll2_tx_dest' [-Wenum-conversion]
>         ll2_tx_pkt.tx_dest = pkt->tx_dest;
>                            ~ ~~~~~^~~~~~~
> 1 warning generated.
>
> Turns out that QED_ROCE_LL2_TX_DEST_NW and QED_ROCE_LL2_TX_DEST_LB are
> only used once in the whole tree and QED_ROCE_LL2_TX_DEST_MAX is used
> nowhere. Remove them and use the equivalent values from qed_ll2_tx_dest
> in their place.
>
> Reported-by: Nick Desaulniers <ndesaulniers@google.com>
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> ---
>
> v1 -> v2:
>
> * Rather than using an explicit cast, just convert the uses to the
>   appropriate values and delete the duplicated enum.
>
>  drivers/infiniband/hw/qedr/qedr_roce_cm.c |  4 ++--
>  include/linux/qed/qed_rdma_if.h           | 11 +----------
>  2 files changed, 3 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/infiniband/hw/qedr/qedr_roce_cm.c b/drivers/infiniband/hw/qedr/qedr_roce_cm.c
> index 85578887421b..e1ac2fd60bb1 100644
> --- a/drivers/infiniband/hw/qedr/qedr_roce_cm.c
> +++ b/drivers/infiniband/hw/qedr/qedr_roce_cm.c
> @@ -519,9 +519,9 @@ static inline int qedr_gsi_build_packet(struct qedr_dev *dev,
>         }
>
>         if (ether_addr_equal(udh.eth.smac_h, udh.eth.dmac_h))
> -               packet->tx_dest = QED_ROCE_LL2_TX_DEST_LB;
> +               packet->tx_dest = QED_LL2_TX_DEST_LB;
>         else
> -               packet->tx_dest = QED_ROCE_LL2_TX_DEST_NW;
> +               packet->tx_dest = QED_LL2_TX_DEST_NW;

Yep, looks like these were the only two sites using enum
qed_roce_ll2_tx_dest in the whole kernel.  The new enum values match
the previous enum values, so this is no functional change while
simplifying the code (one less enum to get wrong).
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

>
>         packet->roce_mode = roce_mode;
>         memcpy(packet->header.vaddr, ud_header_buffer, header_size);
> diff --git a/include/linux/qed/qed_rdma_if.h b/include/linux/qed/qed_rdma_if.h
> index df4d13f7e191..d15f8e4815e3 100644
> --- a/include/linux/qed/qed_rdma_if.h
> +++ b/include/linux/qed/qed_rdma_if.h
> @@ -39,15 +39,6 @@
>  #include <linux/qed/qed_ll2_if.h>
>  #include <linux/qed/rdma_common.h>
>
> -enum qed_roce_ll2_tx_dest {
> -       /* Light L2 TX Destination to the Network */
> -       QED_ROCE_LL2_TX_DEST_NW,
> -
> -       /* Light L2 TX Destination to the Loopback */
> -       QED_ROCE_LL2_TX_DEST_LB,
> -       QED_ROCE_LL2_TX_DEST_MAX
> -};
> -
>  #define QED_RDMA_MAX_CNQ_SIZE               (0xFFFF)
>
>  /* rdma interface */
> @@ -581,7 +572,7 @@ struct qed_roce_ll2_packet {
>         int n_seg;
>         struct qed_roce_ll2_buffer payload[RDMA_MAX_SGE_PER_SQ_WQE];
>         int roce_mode;
> -       enum qed_roce_ll2_tx_dest tx_dest;
> +       enum qed_ll2_tx_dest tx_dest;
>  };
>
>  enum qed_rdma_type {
> --
> 2.19.0
>
Kalderon, Michal Sept. 28, 2018, 8:06 a.m. UTC | #2
> From: Nick Desaulniers <ndesaulniers@google.com>
> Sent: Friday, September 28, 2018 1:30 AM
> 
> External Email
> 
> On Thu, Sep 27, 2018 at 1:57 PM Nathan Chancellor
> <natechancellor@gmail.com> wrote:
> >
> > Clang warns when one enumerated type is explicitly converted to another.
> >
> > drivers/infiniband/hw/qedr/qedr_roce_cm.c:198:28: warning: implicit
> > conversion from enumeration type 'enum qed_roce_ll2_tx_dest' to
> > different enumeration type 'enum qed_ll2_tx_dest' [-Wenum-conversion]
> >         ll2_tx_pkt.tx_dest = pkt->tx_dest;
> >                            ~ ~~~~~^~~~~~~
> > 1 warning generated.
> >
> > Turns out that QED_ROCE_LL2_TX_DEST_NW and
> QED_ROCE_LL2_TX_DEST_LB are
> > only used once in the whole tree and QED_ROCE_LL2_TX_DEST_MAX is
> used
> > nowhere. Remove them and use the equivalent values from
> > qed_ll2_tx_dest in their place.
> >
> > Reported-by: Nick Desaulniers <ndesaulniers@google.com>
> > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> > ---
> >
> > v1 -> v2:
> >
> > * Rather than using an explicit cast, just convert the uses to the
> >   appropriate values and delete the duplicated enum.
> >
> >  drivers/infiniband/hw/qedr/qedr_roce_cm.c |  4 ++--
> >  include/linux/qed/qed_rdma_if.h           | 11 +----------
> >  2 files changed, 3 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/infiniband/hw/qedr/qedr_roce_cm.c
> > b/drivers/infiniband/hw/qedr/qedr_roce_cm.c
> > index 85578887421b..e1ac2fd60bb1 100644
> > --- a/drivers/infiniband/hw/qedr/qedr_roce_cm.c
> > +++ b/drivers/infiniband/hw/qedr/qedr_roce_cm.c
> > @@ -519,9 +519,9 @@ static inline int qedr_gsi_build_packet(struct
> qedr_dev *dev,
> >         }
> >
> >         if (ether_addr_equal(udh.eth.smac_h, udh.eth.dmac_h))
> > -               packet->tx_dest = QED_ROCE_LL2_TX_DEST_LB;
> > +               packet->tx_dest = QED_LL2_TX_DEST_LB;
> >         else
> > -               packet->tx_dest = QED_ROCE_LL2_TX_DEST_NW;
> > +               packet->tx_dest = QED_LL2_TX_DEST_NW;
> 
> Yep, looks like these were the only two sites using enum
> qed_roce_ll2_tx_dest in the whole kernel.  The new enum values match the
> previous enum values, so this is no functional change while simplifying the
> code (one less enum to get wrong).
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> 
> >
> >         packet->roce_mode = roce_mode;
> >         memcpy(packet->header.vaddr, ud_header_buffer, header_size);
> > diff --git a/include/linux/qed/qed_rdma_if.h
> > b/include/linux/qed/qed_rdma_if.h index df4d13f7e191..d15f8e4815e3
> > 100644
> > --- a/include/linux/qed/qed_rdma_if.h
> > +++ b/include/linux/qed/qed_rdma_if.h
> > @@ -39,15 +39,6 @@
> >  #include <linux/qed/qed_ll2_if.h>
> >  #include <linux/qed/rdma_common.h>
> >
> > -enum qed_roce_ll2_tx_dest {
> > -       /* Light L2 TX Destination to the Network */
> > -       QED_ROCE_LL2_TX_DEST_NW,
> > -
> > -       /* Light L2 TX Destination to the Loopback */
> > -       QED_ROCE_LL2_TX_DEST_LB,
> > -       QED_ROCE_LL2_TX_DEST_MAX
> > -};
> > -
> >  #define QED_RDMA_MAX_CNQ_SIZE               (0xFFFF)
> >
> >  /* rdma interface */
> > @@ -581,7 +572,7 @@ struct qed_roce_ll2_packet {
> >         int n_seg;
> >         struct qed_roce_ll2_buffer payload[RDMA_MAX_SGE_PER_SQ_WQE];
> >         int roce_mode;
> > -       enum qed_roce_ll2_tx_dest tx_dest;
> > +       enum qed_ll2_tx_dest tx_dest;
> >  };
> >
> >  enum qed_rdma_type {
> > --
> > 2.19.0
> >
> 
> 
> --
> Thanks,
> ~Nick Desaulniers

Thanks, definitely  looks cleaner. 
Acked-by: Michal Kalderon <michal.kalderon@cavium.com>
Jason Gunthorpe Sept. 28, 2018, 4:14 p.m. UTC | #3
On Thu, Sep 27, 2018 at 01:55:58PM -0700, Nathan Chancellor wrote:
> Clang warns when one enumerated type is explicitly converted to another.
> 
> drivers/infiniband/hw/qedr/qedr_roce_cm.c:198:28: warning: implicit
> conversion from enumeration type 'enum qed_roce_ll2_tx_dest' to
> different enumeration type 'enum qed_ll2_tx_dest' [-Wenum-conversion]
>         ll2_tx_pkt.tx_dest = pkt->tx_dest;
>                            ~ ~~~~~^~~~~~~
> 1 warning generated.
> 
> Turns out that QED_ROCE_LL2_TX_DEST_NW and QED_ROCE_LL2_TX_DEST_LB are
> only used once in the whole tree and QED_ROCE_LL2_TX_DEST_MAX is used
> nowhere. Remove them and use the equivalent values from qed_ll2_tx_dest
> in their place.
> 
> Reported-by: Nick Desaulniers <ndesaulniers@google.com>
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> Acked-by: Michal Kalderon <michal.kalderon@cavium.com>
> ---
> 
> v1 -> v2:
> 
> * Rather than using an explicit cast, just convert the uses to the
>   appropriate values and delete the duplicated enum.

Applied to for-next, thanks

Jason
diff mbox series

Patch

diff --git a/drivers/infiniband/hw/qedr/qedr_roce_cm.c b/drivers/infiniband/hw/qedr/qedr_roce_cm.c
index 85578887421b..e1ac2fd60bb1 100644
--- a/drivers/infiniband/hw/qedr/qedr_roce_cm.c
+++ b/drivers/infiniband/hw/qedr/qedr_roce_cm.c
@@ -519,9 +519,9 @@  static inline int qedr_gsi_build_packet(struct qedr_dev *dev,
 	}
 
 	if (ether_addr_equal(udh.eth.smac_h, udh.eth.dmac_h))
-		packet->tx_dest = QED_ROCE_LL2_TX_DEST_LB;
+		packet->tx_dest = QED_LL2_TX_DEST_LB;
 	else
-		packet->tx_dest = QED_ROCE_LL2_TX_DEST_NW;
+		packet->tx_dest = QED_LL2_TX_DEST_NW;
 
 	packet->roce_mode = roce_mode;
 	memcpy(packet->header.vaddr, ud_header_buffer, header_size);
diff --git a/include/linux/qed/qed_rdma_if.h b/include/linux/qed/qed_rdma_if.h
index df4d13f7e191..d15f8e4815e3 100644
--- a/include/linux/qed/qed_rdma_if.h
+++ b/include/linux/qed/qed_rdma_if.h
@@ -39,15 +39,6 @@ 
 #include <linux/qed/qed_ll2_if.h>
 #include <linux/qed/rdma_common.h>
 
-enum qed_roce_ll2_tx_dest {
-	/* Light L2 TX Destination to the Network */
-	QED_ROCE_LL2_TX_DEST_NW,
-
-	/* Light L2 TX Destination to the Loopback */
-	QED_ROCE_LL2_TX_DEST_LB,
-	QED_ROCE_LL2_TX_DEST_MAX
-};
-
 #define QED_RDMA_MAX_CNQ_SIZE               (0xFFFF)
 
 /* rdma interface */
@@ -581,7 +572,7 @@  struct qed_roce_ll2_packet {
 	int n_seg;
 	struct qed_roce_ll2_buffer payload[RDMA_MAX_SGE_PER_SQ_WQE];
 	int roce_mode;
-	enum qed_roce_ll2_tx_dest tx_dest;
+	enum qed_ll2_tx_dest tx_dest;
 };
 
 enum qed_rdma_type {