[dpdk-dev,v2,02/18] ixgbe: Clean up IXGBE base codes
Commit Message
This patch cleans up some IXGBE base codes, such as remove unnecessary return statement,
and reduce goto statement etc.
Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
---
lib/librte_pmd_ixgbe/ixgbe/ixgbe_82598.c | 2 --
lib/librte_pmd_ixgbe/ixgbe/ixgbe_82599.c | 7 +++----
lib/librte_pmd_ixgbe/ixgbe/ixgbe_common.c | 2 +-
lib/librte_pmd_ixgbe/ixgbe/ixgbe_common.h | 2 --
lib/librte_pmd_ixgbe/ixgbe/ixgbe_dcb_82598.c | 2 ++
lib/librte_pmd_ixgbe/ixgbe/ixgbe_dcb_82599.c | 1 +
lib/librte_pmd_ixgbe/ixgbe/ixgbe_phy.c | 3 ++-
7 files changed, 9 insertions(+), 10 deletions(-)
Comments
On Mon, Sep 29, 2014 at 03:16:10PM +0800, Ouyang Changchun wrote:
> This patch cleans up some IXGBE base codes, such as remove unnecessary return statement,
> and reduce goto statement etc.
>
> Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
> ---
> lib/librte_pmd_ixgbe/ixgbe/ixgbe_82598.c | 2 --
> lib/librte_pmd_ixgbe/ixgbe/ixgbe_82599.c | 7 +++----
> lib/librte_pmd_ixgbe/ixgbe/ixgbe_common.c | 2 +-
> lib/librte_pmd_ixgbe/ixgbe/ixgbe_common.h | 2 --
> lib/librte_pmd_ixgbe/ixgbe/ixgbe_dcb_82598.c | 2 ++
> lib/librte_pmd_ixgbe/ixgbe/ixgbe_dcb_82599.c | 1 +
> lib/librte_pmd_ixgbe/ixgbe/ixgbe_phy.c | 3 ++-
> 7 files changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/lib/librte_pmd_ixgbe/ixgbe/ixgbe_82598.c b/lib/librte_pmd_ixgbe/ixgbe/ixgbe_82598.c
> index ee2217d..c8ce893 100644
> --- a/lib/librte_pmd_ixgbe/ixgbe/ixgbe_82598.c
> +++ b/lib/librte_pmd_ixgbe/ixgbe/ixgbe_82598.c
> @@ -1417,8 +1417,6 @@ STATIC void ixgbe_set_rxpba_82598(struct ixgbe_hw *hw, int num_pb,
> /* Setup Tx packet buffer sizes */
> for (i = 0; i < IXGBE_MAX_PACKET_BUFFERS; i++)
> IXGBE_WRITE_REG(hw, IXGBE_TXPBSIZE(i), IXGBE_TXPBSIZE_40KB);
> -
> - return;
> }
>
> /**
> diff --git a/lib/librte_pmd_ixgbe/ixgbe/ixgbe_82599.c b/lib/librte_pmd_ixgbe/ixgbe/ixgbe_82599.c
> index 835331b..046a35e 100644
> --- a/lib/librte_pmd_ixgbe/ixgbe/ixgbe_82599.c
> +++ b/lib/librte_pmd_ixgbe/ixgbe/ixgbe_82599.c
> @@ -2103,7 +2103,7 @@ s32 ixgbe_identify_phy_82599(struct ixgbe_hw *hw)
> if (status != IXGBE_SUCCESS) {
> /* 82599 10GBASE-T requires an external PHY */
> if (hw->mac.ops.get_media_type(hw) == ixgbe_media_type_copper)
> - goto out;
> + return status;
> else
> status = ixgbe_identify_module_generic(hw);
> }
> @@ -2111,14 +2111,13 @@ s32 ixgbe_identify_phy_82599(struct ixgbe_hw *hw)
> /* Set PHY type none if no PHY detected */
> if (hw->phy.type == ixgbe_phy_unknown) {
> hw->phy.type = ixgbe_phy_none;
> - status = IXGBE_SUCCESS;
> + return IXGBE_SUCCESS;
> }
>
> /* Return error if SFP module has been detected but is not supported */
> if (hw->phy.type == ixgbe_phy_sfp_unsupported)
> - status = IXGBE_ERR_SFP_NOT_SUPPORTED;
> + return IXGBE_ERR_SFP_NOT_SUPPORTED;
>
> -out:
> return status;
> }
>
How is this a cleanup? I understand that you've removed a set of goto
statements from this function, and, while I don't think gotos are a problem, its
fine that you did. But there are literally dozens of other goto statements that
could be cleaned up in a simmilar fashion that you ignored in this patch. Why
just this one location?
Also, isn't this code lifted directly from the linux ixgbe driver? Wouldn't it
be prudent to just keep it in line with that driver as much as possible?
Neil
@@ -1417,8 +1417,6 @@ STATIC void ixgbe_set_rxpba_82598(struct ixgbe_hw *hw, int num_pb,
/* Setup Tx packet buffer sizes */
for (i = 0; i < IXGBE_MAX_PACKET_BUFFERS; i++)
IXGBE_WRITE_REG(hw, IXGBE_TXPBSIZE(i), IXGBE_TXPBSIZE_40KB);
-
- return;
}
/**
@@ -2103,7 +2103,7 @@ s32 ixgbe_identify_phy_82599(struct ixgbe_hw *hw)
if (status != IXGBE_SUCCESS) {
/* 82599 10GBASE-T requires an external PHY */
if (hw->mac.ops.get_media_type(hw) == ixgbe_media_type_copper)
- goto out;
+ return status;
else
status = ixgbe_identify_module_generic(hw);
}
@@ -2111,14 +2111,13 @@ s32 ixgbe_identify_phy_82599(struct ixgbe_hw *hw)
/* Set PHY type none if no PHY detected */
if (hw->phy.type == ixgbe_phy_unknown) {
hw->phy.type = ixgbe_phy_none;
- status = IXGBE_SUCCESS;
+ return IXGBE_SUCCESS;
}
/* Return error if SFP module has been detected but is not supported */
if (hw->phy.type == ixgbe_phy_sfp_unsupported)
- status = IXGBE_ERR_SFP_NOT_SUPPORTED;
+ return IXGBE_ERR_SFP_NOT_SUPPORTED;
-out:
return status;
}
@@ -1060,7 +1060,7 @@ s32 ixgbe_stop_adapter_generic(struct ixgbe_hw *hw)
hw->adapter_stopped = true;
/* Disable the receive unit */
- IXGBE_WRITE_REG(hw, IXGBE_RXCTRL, 0);
+ ixgbe_disable_rx(hw);
/* Clear interrupt mask to stop interrupts from being generated */
IXGBE_WRITE_REG(hw, IXGBE_EIMC, IXGBE_IRQ_CLEAR_MASK);
@@ -41,9 +41,7 @@ POSSIBILITY OF SUCH DAMAGE.
IXGBE_WRITE_REG(hw, reg, (u32) value); \
IXGBE_WRITE_REG(hw, reg + 4, (u32) (value >> 32)); \
} while (0)
-#ifndef IXGBE_REMOVED
#define IXGBE_REMOVED(a) (0)
-#endif /* IXGBE_REMOVED */
struct ixgbe_pba {
u16 word[2];
u16 *pba_block;
@@ -347,6 +347,8 @@ s32 ixgbe_dcb_hw_config_82598(struct ixgbe_hw *hw, int link_speed,
u16 *refill, u16 *max, u8 *bwg_id,
u8 *tsa)
{
+ UNREFERENCED_1PARAMETER(link_speed);
+
ixgbe_dcb_config_rx_arbiter_82598(hw, refill, max, tsa);
ixgbe_dcb_config_tx_desc_arbiter_82598(hw, refill, max, bwg_id,
tsa);
@@ -580,6 +580,7 @@ s32 ixgbe_dcb_hw_config_82599(struct ixgbe_hw *hw, int link_speed,
u16 *refill, u16 *max, u8 *bwg_id, u8 *tsa,
u8 *map)
{
+ UNREFERENCED_1PARAMETER(link_speed);
ixgbe_dcb_config_rx_arbiter_82599(hw, refill, max, bwg_id, tsa,
map);
@@ -1804,7 +1804,7 @@ STATIC s32 ixgbe_get_i2c_ack(struct ixgbe_hw *hw)
ack = ixgbe_get_i2c_data(&i2cctl);
usec_delay(1);
- if (ack == 0)
+ if (!ack)
break;
}
@@ -1886,6 +1886,7 @@ STATIC s32 ixgbe_clock_out_i2c_bit(struct ixgbe_hw *hw, bool data)
return status;
}
+
/**
* ixgbe_raise_i2c_clk - Raises the I2C SCL clock
* @hw: pointer to hardware structure