Messages in this thread | | | Date | Thu, 4 Oct 2018 09:27:36 -0700 | From | Nathan Chancellor <> | Subject | Re: [PATCH] qed: Remove unneeded enumerated type core_tx_dest |
| |
On Thu, Oct 04, 2018 at 04:23:43PM +0000, Tayar, Tomer wrote: > From: Nathan Chancellor <natechancellor@gmail.com> > Sent: Thursday, October 04, 2018 3:09 AM > > > Clang warns when one enumerated type is implicitly converted to another. > > > > drivers/net/ethernet/qlogic/qed/qed_ll2.c:799:32: warning: implicit > > conversion from enumeration type 'enum core_tx_dest' to different > > enumeration type 'enum qed_ll2_tx_dest' [-Wenum-conversion] > > tx_pkt.tx_dest = p_ll2_conn->tx_dest; > > ~ ~~~~~~~~~~~~^~~~~~~ > > 1 warning generated. > > > > These enumerated types are not 1 to 1: > > > > /* Light L2 TX Destination */ > > enum core_tx_dest { > > CORE_TX_DEST_NW, > > CORE_TX_DEST_LB, > > CORE_TX_DEST_RESERVED, > > CORE_TX_DEST_DROP, > > MAX_CORE_TX_DEST > > }; > > > > enum qed_ll2_tx_dest { > > QED_LL2_TX_DEST_NW, /* Light L2 TX Destination to the Network */ > > QED_LL2_TX_DEST_LB, /* Light L2 TX Destination to the Loopback */ > > QED_LL2_TX_DEST_DROP, /* Light L2 Drop the TX packet */ > > QED_LL2_TX_DEST_MAX > > }; > > > > Fix this conversion warning by adding CORE_TX_DEST_DROP to > > qed_ll2_tx_dest and converting all values of core_tx_dest to > > the equivalent value in qed_ll2_tx_dest so that there is no > > conversion warning or functional change. > > > > Link: https://github.com/ClangBuiltLinux/linux/issues/125 > > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> > > [...] > > > -/* Light L2 TX Destination */ > > -enum core_tx_dest { > > - CORE_TX_DEST_NW, > > - CORE_TX_DEST_LB, > > - CORE_TX_DEST_RESERVED, > > - CORE_TX_DEST_DROP, > > - MAX_CORE_TX_DEST > > -}; > > [...] > > > QED_LL2_TX_DEST_NW, /* Light L2 TX Destination to the Network */ > > QED_LL2_TX_DEST_LB, /* Light L2 TX Destination to the Loopback */ > > QED_LL2_TX_DEST_DROP, /* Light L2 Drop the TX packet */ > > + QED_LL2_TX_DEST_DROP_CORE, /* CORE_TX_DEST_DROP value */ > > QED_LL2_TX_DEST_MAX > > }; > > Thanks Nathan for finding this issue. > "enum core_tx_dest" is for the interface with the device FW, while "enum qed_ll2_tx_dest" is for the interface with other qed* kernel modules. > The distinction is on purpose, so "enum core_tx_dest" shouldn't be deleted. > Maybe an explicit switch/case would be better as a fix? > E.g. - > - tx_pkt.tx_dest = p_ll2_conn->tx_dest; > + switch (p_ll2_conn->tx_dest) { > + case CORE_TX_DEST_NW: > + tx_pkt.tx_dest = QED_LL2_TX_DEST_NW; > + break; > + case CORE_TX_DEST_LB: > + tx_pkt.tx_dest = QED_LL2_TX_DEST_LB; > + break; > + case CORE_TX_DEST_DROP: > + tx_pkt.tx_dest = QED_LL2_TX_DEST_DROP; > + break; > + default: > + tx_pkt.tx_dest = QED_LL2_TX_DEST_DROP; > + } >
Hi Tomer,
Yes, that should work and it would match the rest of the driver's handling of this distinction. I will go ahead and spin up a v2 here shortly for review, thank you.
Nathan
| |