[dpdk-dev] [PATCH] Fix misleading indentation in ethtool
De Lara Guarch, Pablo
pablo.de.lara.guarch at intel.com
Fri Jul 1 13:29:05 CEST 2016
Hi Pengyu,
> -----Original Message-----
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Pengyu Ma
> Sent: Friday, July 01, 2016 9:53 AM
> To: thomas.monjalon at 6wind.com; dev at dpdk.org
> Subject: [dpdk-dev] [PATCH] Fix misleading indentation in ethtool
You need to add the library that you are fixing in the title:
i.e. kni: fix misleading identation in ethtool
>
> gcc complains about:
> build/lib/librte_eal/linuxapp/kni/e1000_phy.c:3303:2:
> error: this 'if' clause does not guard... [-Werror=misleading-indentation]
Could you tell me which gcc version you are using?
>
> Code indentation is misleadingly indented as whether
> the following content is guarded by if or not.
> With the reference of the context, add the curly braces.
>
> Remove unused const variables too.
You are fixing part of a library, so you should add a fixes line here.
Please refer to the following document to know how to do it:
http://dpdk.readthedocs.io/en/v16.04/contributing/patches.html
>
> Signed-off-by: Pengyu Ma <pengyu.ma at windriver.com>
> ---
> lib/librte_eal/linuxapp/kni/ethtool/igb/e1000_phy.c | 9 ++++++---
> lib/librte_eal/linuxapp/kni/ethtool/ixgbe/ixgbe_82599.c | 8 +++++---
> lib/librte_eal/linuxapp/kni/ethtool/ixgbe/ixgbe_main.c | 5 +++++
> 3 files changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/lib/librte_eal/linuxapp/kni/ethtool/igb/e1000_phy.c
> b/lib/librte_eal/linuxapp/kni/ethtool/igb/e1000_phy.c
> index df22470..120e57b 100644
> --- a/lib/librte_eal/linuxapp/kni/ethtool/igb/e1000_phy.c
> +++ b/lib/librte_eal/linuxapp/kni/ethtool/igb/e1000_phy.c
> @@ -3300,12 +3300,13 @@ s32 e1000_read_phy_reg_mphy(struct e1000_hw
> *hw, u32 address, u32 *data)
> *data = E1000_READ_REG(hw, E1000_MPHY_DATA);
>
> /* Disable access to mPHY if it was originally disabled */
> - if (locked)
> + if (locked) {
> ready = e1000_is_mphy_ready(hw);
> if (!ready)
> return -E1000_ERR_PHY;
> E1000_WRITE_REG(hw, E1000_MPHY_ADDR_CTRL,
> E1000_MPHY_DIS_ACCESS);
> + }
You are actually fixing the code flow, right?
So, not sure if you should modify the title and body of the patch.
>
> return E1000_SUCCESS;
> }
> @@ -3365,12 +3366,14 @@ s32 e1000_write_phy_reg_mphy(struct
> e1000_hw *hw, u32 address, u32 data,
> E1000_WRITE_REG(hw, E1000_MPHY_DATA, data);
>
> /* Disable access to mPHY if it was originally disabled */
> - if (locked)
> + if (locked) {
> ready = e1000_is_mphy_ready(hw);
> - if (!ready)
> + if (!ready) {
> return -E1000_ERR_PHY;
> + }
Not needed it, but harmless :)
> E1000_WRITE_REG(hw, E1000_MPHY_ADDR_CTRL,
> E1000_MPHY_DIS_ACCESS);
> + }
>
> return E1000_SUCCESS;
> }
> diff --git a/lib/librte_eal/linuxapp/kni/ethtool/ixgbe/ixgbe_82599.c
> b/lib/librte_eal/linuxapp/kni/ethtool/ixgbe/ixgbe_82599.c
> index 017dfe1..71cbf1a 100644
> --- a/lib/librte_eal/linuxapp/kni/ethtool/ixgbe/ixgbe_82599.c
> +++ b/lib/librte_eal/linuxapp/kni/ethtool/ixgbe/ixgbe_82599.c
> @@ -867,14 +867,16 @@ s32 ixgbe_setup_mac_link_82599(struct ixgbe_hw
> *hw,
> link_mode == IXGBE_AUTOC_LMS_KX4_KX_KR_SGMII) {
> /* Set KX4/KX/KR support according to speed requested */
> autoc &= ~(IXGBE_AUTOC_KX4_KX_SUPP_MASK |
> IXGBE_AUTOC_KR_SUPP);
> - if (speed & IXGBE_LINK_SPEED_10GB_FULL)
> + if (speed & IXGBE_LINK_SPEED_10GB_FULL) {
> if (orig_autoc & IXGBE_AUTOC_KX4_SUPP)
> autoc |= IXGBE_AUTOC_KX4_SUPP;
> if ((orig_autoc & IXGBE_AUTOC_KR_SUPP) &&
> - (hw->phy.smart_speed_active == false))
> + (hw->phy.smart_speed_active == false))
> autoc |= IXGBE_AUTOC_KR_SUPP;
> - if (speed & IXGBE_LINK_SPEED_1GB_FULL)
> + }
> + if (speed & IXGBE_LINK_SPEED_1GB_FULL) {
> autoc |= IXGBE_AUTOC_KX_SUPP;
> + }
> } else if ((pma_pmd_1g == IXGBE_AUTOC_1G_SFI) &&
> (link_mode == IXGBE_AUTOC_LMS_1G_LINK_NO_AN ||
> link_mode == IXGBE_AUTOC_LMS_1G_AN)) {
> diff --git a/lib/librte_eal/linuxapp/kni/ethtool/ixgbe/ixgbe_main.c
> b/lib/librte_eal/linuxapp/kni/ethtool/ixgbe/ixgbe_main.c
> index 8c1d2fe..1e9f9d1 100644
> --- a/lib/librte_eal/linuxapp/kni/ethtool/ixgbe/ixgbe_main.c
> +++ b/lib/librte_eal/linuxapp/kni/ethtool/ixgbe/ixgbe_main.c
> @@ -59,8 +59,11 @@
> #undef CONFIG_DCA_MODULE
>
> char ixgbe_driver_name[] = "ixgbe";
> +/*
> static const char ixgbe_driver_string[] =
> "Intel(R) 10 Gigabit PCI Express Network Driver";
> +*/
This line and the one below that you are commenting out have been removed in this patch
(http://dpdk.org/dev/patchwork/patch/14276/) and it has been applied, so no need to touch this file :)
Make sure that you rebase your patch against head before submitting a v2, to avoid this issue.
> +
> #define DRV_HW_PERF
>
> #ifndef CONFIG_IXGBE_NAPI
> @@ -79,8 +82,10 @@ static const char ixgbe_driver_string[] =
> #define DRV_VERSION __stringify(MAJ) "." __stringify(MIN) "." \
> __stringify(BUILD) DRIVERNAPI DRV_HW_PERF FPGA
> VMDQ_TAG
> const char ixgbe_driver_version[] = DRV_VERSION;
> +/*
> static const char ixgbe_copyright[] =
> "Copyright (c) 1999-2012 Intel Corporation.";
> +*/
>
> /* ixgbe_pci_tbl - PCI Device ID Table
> *
> --
> 2.7.4
More information about the dev
mailing list