RE: [PATCH v2] RDMA/qedr: Remove enumerated type qed_roce_ll2_tx_dest
From: Kalderon, Michal
Date: Fri Sep 28 2018 - 04:06:11 EST
> From: Nick Desaulniers <ndesaulniers@xxxxxxxxxx>
> Sent: Friday, September 28, 2018 1:30 AM
>
> External Email
>
> On Thu, Sep 27, 2018 at 1:57 PM Nathan Chancellor
> <natechancellor@xxxxxxxxx> 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@xxxxxxxxxx>
> > Signed-off-by: Nathan Chancellor <natechancellor@xxxxxxxxx>
> > ---
> >
> > 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@xxxxxxxxxx>
>
> >
> > 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@xxxxxxxxxx>