RE: [PATCH] qed: Remove unneeded enumerated type core_tx_dest

From: Tayar, Tomer
Date: Thu Oct 04 2018 - 12:23:50 EST


From: Nathan Chancellor <natechancellor@xxxxxxxxx>
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@xxxxxxxxx>

[...]

> -/* 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;
+ }