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
