Skip to contentSkip to conversation

kni : fix build errors for gcc --version >= 6.1

PATCH In review Anupam Kapoor 2 participants 3 messages Jun 21, 2016

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 kni : fix build errors for gcc --version >= 6.1

This commit fixes build errors triggered due misleading indentation.

Fixes 366113dbfb696 (e1000: suppress misleading indentation warning)
---
 lib/librte_eal/linuxapp/kni/ethtool/igb/e1000_phy.c     | 12 ++++++++----
 lib/librte_eal/linuxapp/kni/ethtool/ixgbe/ixgbe_82599.c |  6 +++---
 2 files changed, 11 insertions(+), 7 deletions(-)
lib/librte_eal/linuxapp/kni/ethtool/igb/e1000_phy.c +8 -4
index df224702ed7d..26352da15101 100644
@@ -3299,13 +3299,15 @@ s32 e1000_read_phy_reg_mphy(struct e1000_hw *hw, u32 address, u32 *data)
                return -E1000_ERR_PHY;
        *data = E1000_READ_REG(hw, E1000_MPHY_DATA);
 
-       /* Disable access to mPHY if it was originally disabled */
-       if (locked)
+       /* Disable access to mPHY if it was originally enabled */
+       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);
+       }
 
        return E1000_SUCCESS;
 }
@@ -3364,13 +3366,15 @@ s32 e1000_write_phy_reg_mphy(struct e1000_hw *hw, u32 address, u32 data,
                return -E1000_ERR_PHY;
        E1000_WRITE_REG(hw, E1000_MPHY_DATA, data);
 
-       /* Disable access to mPHY if it was originally disabled */
-       if (locked)
+       /* Disable access to mPHY if it was originally enabled */
+       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);
+       }
 
        return E1000_SUCCESS;
 }
lib/librte_eal/linuxapp/kni/ethtool/ixgbe/ixgbe_82599.c +3 -3
index 017dfe16c73f..dc2a4fb61c25 100644
@@ -870,9 +870,9 @@ s32 ixgbe_setup_mac_link_82599(struct ixgbe_hw *hw,
                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))
-                               autoc |= IXGBE_AUTOC_KR_SUPP;
+               if ((orig_autoc & IXGBE_AUTOC_KR_SUPP) &&
+                   (hw->phy.smart_speed_active == false))
+                       autoc |= IXGBE_AUTOC_KR_SUPP;
                if (speed & IXGBE_LINK_SPEED_1GB_FULL)
                        autoc |= IXGBE_AUTOC_KX_SUPP;
        } else if ((pma_pmd_1g == IXGBE_AUTOC_1G_SFI) &&

--
2.9.0

#1 lore raw

Hi Anupam,

Thank you for the patch.

Quoted 3 lines · Anupam Kapoor

This commit fixes build errors triggered due misleading indentation.

Fixes 366113dbfb696 (e1000: suppress misleading indentation warning)

This looks like wrong commit id that fixed, can you please double check Also you may need two fixes lines, since fixing two different driver files

Quoted 13 lines
---
 lib/librte_eal/linuxapp/kni/ethtool/igb/e1000_phy.c     | 12 ++++++++----
 lib/librte_eal/linuxapp/kni/ethtool/ixgbe/ixgbe_82599.c |  6 +++---
 2 files changed, 11 insertions(+), 7 deletions(-)
lib/librte_eal/linuxapp/kni/ethtool/igb/e1000_phy.c +0 -0
index df224702ed7d..26352da15101 100644

lib/librte_eal/linuxapp/kni/ethtool/igb/e1000_phy.c s32 e1000_read_phy_reg_mphy(struct e1000_hw *hw, u32 address, u32 *data) · line 3302

Quoted 6 lines
lib/librte_eal/linuxapp/kni/ethtool/igb/e1000_phy.c +1 -2
@@ -3299,13 +3299,15 @@ s32 e1000_read_phy_reg_mphy(struct e1000_hw *hw, u32 address, u32 *data)
                return -E1000_ERR_PHY;
        *data = E1000_READ_REG(hw, E1000_MPHY_DATA);
 
-       /* Disable access to mPHY if it was originally disabled */
-       if (locked)
+       /* Disable access to mPHY if it was originally enabled */

I think original comment is correct.
As far as I can see, if access disabled in the beginning of the function, it is enabled and here disabled back. Original state saved to locked variable.

Quoted 9 lines
lib/librte_eal/linuxapp/kni/ethtool/igb/e1000_phy.c +3 -0
+       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);
+       }
 

...

Quoted 5 lines
lib/librte_eal/linuxapp/kni/ethtool/ixgbe/ixgbe_82599.c +0 -0
index 017dfe16c73f..dc2a4fb61c25 100644

lib/librte_eal/linuxapp/kni/ethtool/ixgbe/ixgbe_82599.c s32 ixgbe_setup_mac_link_82599(struct ixgbe_hw *hw, · line 875

Quoted 9 lines
lib/librte_eal/linuxapp/kni/ethtool/ixgbe/ixgbe_82599.c +3 -3
@@ -870,9 +870,9 @@ s32 ixgbe_setup_mac_link_82599(struct ixgbe_hw *hw,
                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))
-                               autoc |= IXGBE_AUTOC_KR_SUPP;
+               if ((orig_autoc & IXGBE_AUTOC_KR_SUPP) &&
+                   (hw->phy.smart_speed_active == false))
+                       autoc |= IXGBE_AUTOC_KR_SUPP;

Can you please check following commit:
https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/commit/?id=55461ddbc

Quoted 6 lines
lib/librte_eal/linuxapp/kni/ethtool/ixgbe/ixgbe_82599.c +0 -1
                if (speed & IXGBE_LINK_SPEED_1GB_FULL)
                        autoc |= IXGBE_AUTOC_KX_SUPP;
        } else if ((pma_pmd_1g == IXGBE_AUTOC_1G_SFI) &&
--

2.9.0

Would you mind sending a new version of patch according above comments?

Thanks,
ferruh

#2 lore raw
Quoted 5 lines · Ferruh Yigit

Hi Anupam,

Thank you for the patch.

Quoted 7 lines
Quoted 3 lines · Anupam Kapoor

This commit fixes build errors triggered due misleading indentation.

Fixes 366113dbfb696 (e1000: suppress misleading indentation warning)

This looks like wrong commit id that fixed, can you please double check Also you may need two fixes lines, since fixing two different driver files

​yes, you are right, i erroneously looked at the commit-id ​

​which included the '-Wno-misleading-indentation' and used that.​

Quoted 19 lines
Quoted 5 lines
---
 lib/librte_eal/linuxapp/kni/ethtool/igb/e1000_phy.c     | 12

++++++++----

Review

Quoted 4 lines
 lib/librte_eal/linuxapp/kni/ethtool/ixgbe/ixgbe_82599.c |  6 +++---
 2 files changed, 11 insertions(+), 7 deletions(-)
(quoted diff) +0 -0

b/lib/librte_eal/linuxapp/kni/ethtool/igb/e1000_phy.c

lib/librte_eal/linuxapp/kni/ethtool/igb/e1000_phy.c s32 e1000_read_phy_reg_mphy(struct e1000_hw *hw,

Quoted 4 lines

index df224702ed7d..26352da15101 100644

lib/librte_eal/linuxapp/kni/ethtool/igb/e1000_phy.c +0 -0
@@ -3299,13 +3299,15 @@ s32 e1000_read_phy_reg_mphy(struct e1000_hw *hw,

u32 address, u32 *data)

Quoted 3 lines
lib/librte_eal/linuxapp/kni/ethtool/igb/e1000_phy.c +0 -0
                return -E1000_ERR_PHY;
        *data = E1000_READ_REG(hw, E1000_MPHY_DATA);
 
Quoted 8 lines

lib/librte_eal/linuxapp/kni/ethtool/igb/e1000_phy.c s32 e1000_read_phy_reg_mphy(struct e1000_hw *hw, u32 address, u32 *data) · line 3302

Quoted 3 lines
lib/librte_eal/linuxapp/kni/ethtool/igb/e1000_phy.c +1 -2
-       /* Disable access to mPHY if it was originally disabled */
-       if (locked)
+       /* Disable access to mPHY if it was originally enabled */

I think original comment is correct.
As far as I can see, if access disabled in the beginning of the function, it is enabled and here disabled back. Original state saved to locked variable.

​yes, the locked variable keeps that state...
​

Quoted 29 lines
Quoted 9 lines
lib/librte_eal/linuxapp/kni/ethtool/igb/e1000_phy.c +3 -0
+       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);
+       }
 

...

Review

Quoted 1 line
(quoted diff) +0 -0

b/lib/librte_eal/linuxapp/kni/ethtool/ixgbe/ixgbe_82599.c

lib/librte_eal/linuxapp/kni/ethtool/ixgbe/ixgbe_82599.c s32 ixgbe_setup_mac_link_82599(struct ixgbe_hw *hw, · line 875

Quoted 13 lines

index 017dfe16c73f..dc2a4fb61c25 100644

lib/librte_eal/linuxapp/kni/ethtool/ixgbe/ixgbe_82599.c +3 -3
@@ -870,9 +870,9 @@ s32 ixgbe_setup_mac_link_82599(struct ixgbe_hw *hw,
                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))
-                               autoc |= IXGBE_AUTOC_KR_SUPP;
+               if ((orig_autoc & IXGBE_AUTOC_KR_SUPP) &&
+                   (hw->phy.smart_speed_active == false))
+                       autoc |= IXGBE_AUTOC_KR_SUPP;

Can you please check following commit:

Quoted 1 line

https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/commit/?id=55461ddbc

​cool thanks for the information.
​

Quoted 10 lines
Quoted 7 lines
lib/librte_eal/linuxapp/kni/ethtool/ixgbe/ixgbe_82599.c +0 -1
                if (speed & IXGBE_LINK_SPEED_1GB_FULL)
                        autoc |= IXGBE_AUTOC_KX_SUPP;
        } else if ((pma_pmd_1g == IXGBE_AUTOC_1G_SFI) &&
--

2.9.0

Would you mind sending a new version of patch according above comments?

​yes sure. in a few minutes...​

Quoted 4 lines

Thanks,
ferruh

Signature · 2 lines
​thanks
anupam​