[PATCH net 0/7] net: hns: fix some bugs in HNS driver
3797d
Revision v1 of 2 in this series.
9 messages,
2 authors,
2016-03-24 · open the first message on its own page
Here are some bug fixed patches for HNS driver.
They are:
from Kejian, fix for the warning of passing zero to 'PTR_ERR' from qianqian, four fixes for inappropriate operation in hns driver from Sheng, one fix for optimization of irq proccess in hns driver, and one fix for hilink status for hns driver.
For more details, please see individual patches.
MBR.
Kejian Yan (1):
net: hns: fix warning of passing zero to 'PTR_ERR'
Qianqian Xie (4):
net: hns: fix a bug for cycle index
net: hns: optimizate fmt of snprintf()
net: hns: bug fix for return values
net: hns: remove useless variable assignment and comment
Sheng Li (2):
net: hns: optimizate irq proccess for HNS V2
net: hns: bug fix about getting hilink status for HNS v2
drivers/net/ethernet/hisilicon/hns/hns_dsaf_gmac.c | 3 +-
drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c | 12 +++----
drivers/net/ethernet/hisilicon/hns/hns_dsaf_misc.c | 40 ++++++++++++----------
drivers/net/ethernet/hisilicon/hns/hns_dsaf_reg.h | 2 ++
drivers/net/ethernet/hisilicon/hns/hns_enet.c | 16 ++++-----
drivers/net/ethernet/hisilicon/hns/hns_ethtool.c | 4 +--
6 files changed, 41 insertions(+), 36 deletions(-)
--
1.9.1
From: Qianqian Xie <redacted>
The cycle index should be varied while the variable j is a fixed value.
The patch will fix this bug.
Signed-off-by: Qianqian Xie <redacted>
Signed-off-by: Yisen Zhuang <redacted>
---
drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c
index 5c1ac9b..5978a5c 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c @@ -2219,17 +2219,17 @@ void hns_dsaf_get_regs(struct dsaf_device *ddev, u32 port, void *data)
/* dsaf onode registers */
for ( i = 0 ; i < DSAF_XOD_NUM ; i ++ ) {
p [ 311 + i ] = dsaf_read_dev ( ddev ,
- DSAF_XOD_ETS_TSA_TC0_TC3_CFG_0_REG + j * 0x90 ); + DSAF_XOD_ETS_TSA_TC0_TC3_CFG_0_REG + i * 0x90 ); p [ 319 + i ] = dsaf_read_dev ( ddev ,
- DSAF_XOD_ETS_TSA_TC4_TC7_CFG_0_REG + j * 0x90 ); + DSAF_XOD_ETS_TSA_TC4_TC7_CFG_0_REG + i * 0x90 ); p [ 327 + i ] = dsaf_read_dev ( ddev ,
- DSAF_XOD_ETS_BW_TC0_TC3_CFG_0_REG + j * 0x90 ); + DSAF_XOD_ETS_BW_TC0_TC3_CFG_0_REG + i * 0x90 ); p [ 335 + i ] = dsaf_read_dev ( ddev ,
- DSAF_XOD_ETS_BW_TC4_TC7_CFG_0_REG + j * 0x90 ); + DSAF_XOD_ETS_BW_TC4_TC7_CFG_0_REG + i * 0x90 ); p [ 343 + i ] = dsaf_read_dev ( ddev ,
- DSAF_XOD_ETS_BW_OFFSET_CFG_0_REG + j * 0x90 ); + DSAF_XOD_ETS_BW_OFFSET_CFG_0_REG + i * 0x90 ); p [ 351 + i ] = dsaf_read_dev ( ddev ,
- DSAF_XOD_ETS_TOKEN_CFG_0_REG + j * 0x90 ); + DSAF_XOD_ETS_TOKEN_CFG_0_REG + i * 0x90 ); }
p [ 359 ] = dsaf_read_dev ( ddev , DSAF_XOD_PFS_CFG_0_0_REG + port * 0x90 ); --
1.9.1
From: Sheng Li <redacted>
The hilink status reg in HNS V2 is different from HNS v1. In HNS V2, It
distinguishes differnt lane status according to the bit-field of the reg.
As is shown below:
[0:0] ---> lane0
[1:1] ---> lane1
...
But the current driver reads the reg to get the hilink status ONLY
concidering HNS V1 situation. Here is a patch to support both of them.
Signed-off-by: Sheng Li <redacted>
Signed-off-by: Daode Huang <redacted>
---
drivers/net/ethernet/hisilicon/hns/hns_dsaf_misc.c | 40 ++++++++++++----------
drivers/net/ethernet/hisilicon/hns/hns_dsaf_reg.h | 2 ++
2 files changed, 24 insertions(+), 18 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_misc.c b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_misc.c
index 607c3be..e69b022 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_misc.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_misc.c @@ -244,31 +244,35 @@ void hns_ppe_com_srst(struct ppe_common_cb *ppe_common, u32 val)
*/
phy_interface_t hns_mac_get_phy_if ( struct hns_mac_cb * mac_cb )
{
- u32 hilink3_mode ; - u32 hilink4_mode ; + u32 mode ; + u32 reg ; + u32 shift ; + bool is_ver1 = AE_IS_VER1 ( mac_cb -> dsaf_dev -> dsaf_ver ); void __iomem * sys_ctl_vaddr = mac_cb -> sys_ctl_vaddr ;
- int dev_id = mac_cb -> mac_id ; + int mac_id = mac_cb -> mac_id ; phy_interface_t phy_if = PHY_INTERFACE_MODE_NA ;
- hilink3_mode = dsaf_read_reg ( sys_ctl_vaddr , HNS_MAC_HILINK3_REG ); - hilink4_mode = dsaf_read_reg ( sys_ctl_vaddr , HNS_MAC_HILINK4_REG ); - if ( dev_id >= 0 && dev_id <= 3 ) { - if ( hilink4_mode == 0 ) - phy_if = PHY_INTERFACE_MODE_SGMII ; - else + if ( is_ver1 && ( mac_id >= 6 && mac_id <= 7 )) { + phy_if = PHY_INTERFACE_MODE_SGMII ; + } else if ( mac_id >= 0 && mac_id <= 3 ) { + reg = is_ver1 ? HNS_MAC_HILINK4_REG : HNS_MAC_HILINK4V2_REG ; + mode = dsaf_read_reg ( sys_ctl_vaddr , reg ); + /* mac_id 0, 1, 2, 3 ---> hilink4 lane 0, 1, 2, 3 */ + shift = is_ver1 ? 0 : mac_id ; + if ( dsaf_get_bit ( mode , shift )) phy_if = PHY_INTERFACE_MODE_XGMII ;
- } else if ( dev_id >= 4 && dev_id <= 5 ) { - if ( hilink3_mode == 0 ) - phy_if = PHY_INTERFACE_MODE_SGMII ; else
+ phy_if = PHY_INTERFACE_MODE_SGMII ; + } else if ( mac_id >= 4 && mac_id <= 7 ) { + reg = is_ver1 ? HNS_MAC_HILINK3_REG : HNS_MAC_HILINK3V2_REG ; + mode = dsaf_read_reg ( sys_ctl_vaddr , reg ); + /* mac_id 4, 5, 6, 7 ---> hilink3 lane 2, 3, 0, 1 */ + shift = is_ver1 ? 0 : mac_id <= 5 ? mac_id - 2 : mac_id - 6 ; + if ( dsaf_get_bit ( mode , shift )) phy_if = PHY_INTERFACE_MODE_XGMII ;
- } else { - phy_if = PHY_INTERFACE_MODE_SGMII ; + else + phy_if = PHY_INTERFACE_MODE_SGMII ; }
- - dev_dbg ( mac_cb -> dev , - "hilink3_mode=%d, hilink4_mode=%d dev_id=%d, phy_if=%d \n " , - hilink3_mode , hilink4_mode , dev_id , phy_if ); return phy_if ;
}
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_reg.h b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_reg.h
index bf62687..e2206f9 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_reg.h
+++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_reg.h @@ -103,6 +103,8 @@
/*serdes offset**/
#define HNS_MAC_HILINK3_REG DSAF_SUB_SC_HILINK3_CRG_CTRL0_REG
#define HNS_MAC_HILINK4_REG DSAF_SUB_SC_HILINK4_CRG_CTRL0_REG
+ #define HNS_MAC_HILINK3V2_REG DSAF_SUB_SC_HILINK3_CRG_CTRL1_REG + #define HNS_MAC_HILINK4V2_REG DSAF_SUB_SC_HILINK4_CRG_CTRL1_REG #define HNS_MAC_LANE0_CTLEDFE_REG 0x000BFFCCULL
#define HNS_MAC_LANE1_CTLEDFE_REG 0x000BFFBCULL
#define HNS_MAC_LANE2_CTLEDFE_REG 0x000BFFACULL --
1.9.1
From: Qianqian Xie <redacted>
The return values in the first two functions mdiobus_write()
are ignored. The patch will fix it.
Signed-off-by: Qianqian Xie <redacted>
Signed-off-by: Yisen Zhuang <redacted>
---
drivers/net/ethernet/hisilicon/hns/hns_ethtool.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_ethtool.c b/drivers/net/ethernet/hisilicon/hns/hns_ethtool.c
index 9c3ba65..0e7da3f 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_ethtool.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_ethtool.c @@ -1013,8 +1013,8 @@ int hns_phy_led_set(struct net_device *netdev, int value)
struct phy_device * phy_dev = priv -> phy ;
retval = phy_write ( phy_dev , HNS_PHY_PAGE_REG , HNS_PHY_PAGE_LED );
- retval = phy_write ( phy_dev , HNS_LED_FC_REG , value ); - retval = phy_write ( phy_dev , HNS_PHY_PAGE_REG , HNS_PHY_PAGE_COPPER ); + retval |= phy_write ( phy_dev , HNS_LED_FC_REG , value ); + retval |= phy_write ( phy_dev , HNS_PHY_PAGE_REG , HNS_PHY_PAGE_COPPER ); if ( retval ) {
netdev_err ( netdev , "mdiobus_write fail ! \n " );
return retval ; --
1.9.1
From: Qianqian Xie <redacted>
It misses string format in function snprintf(), as below:
snprintf(buff, ETH_GSTRING_LEN, g_gmac_stats_string[i].desc);
It needs to add "%s" to fix it as below:
snprintf(buff, ETH_GSTRING_LEN, "%s", g_gmac_stats_string[i].desc);
Signed-off-by: Qianqian Xie <redacted>
Signed-off-by: Kejian Yan <redacted>
Signed-off-by: Yisen Zhuang <redacted>
---
drivers/net/ethernet/hisilicon/hns/hns_dsaf_gmac.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_gmac.c b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_gmac.c
index 6e2b76e..44abb08 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_gmac.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_gmac.c @@ -664,7 +664,8 @@ static void hns_gmac_get_strings(u32 stringset, u8 *data)
return ;
for ( i = 0 ; i < ARRAY_SIZE ( g_gmac_stats_string ); i ++ ) {
- snprintf ( buff , ETH_GSTRING_LEN , g_gmac_stats_string [ i ]. desc ); + snprintf ( buff , ETH_GSTRING_LEN , "%s" , + g_gmac_stats_string [ i ]. desc ); buff = buff + ETH_GSTRING_LEN ;
}
} --
1.9.1
From: Qianqian Xie <redacted>
The variable head in hns_nic_tx_fini_pro has read a value, but it is
obviously no use. The patch will fix it.
And the comment is nothing to do with the routine, so it has to be removed
Signed-off-by: Qianqian Xie <redacted>
Signed-off-by: Yisen Zhuang <redacted>
---
drivers/net/ethernet/hisilicon/hns/hns_enet.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_enet.c b/drivers/net/ethernet/hisilicon/hns/hns_enet.c
index 71aa37b..f0c9a41 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_enet.c @@ -913,10 +913,7 @@ static int hns_nic_tx_poll_one(struct hns_nic_ring_data *ring_data,
static void hns_nic_tx_fini_pro ( struct hns_nic_ring_data * ring_data )
{
struct hnae_ring * ring = ring_data -> ring ;
- int head = ring -> next_to_clean ; - - /* for hardware bug fixed */ - head = readl_relaxed ( ring -> io_base + RCB_REG_HEAD ); + int head = readl_relaxed ( ring -> io_base + RCB_REG_HEAD );
if ( head != ring -> next_to_clean ) {
ring_data -> ring -> q -> handle -> dev -> ops -> toggle_ring_irq ( --
1.9.1
From: Sheng Li <redacted>
In hns V1, common_poll should check and clean fbd pkts, because it
can not pend irq to clean them if there is no new pkt comes in.
But hns V2 hw fixes this bug, and will pend irq itself to do this.
So, for hns V2, we set ring_data->fini_process to NULL.
Signed-off-by: Sheng Li <redacted>
Signed-off-by: Yisen Zhuang <redacted>
---
drivers/net/ethernet/hisilicon/hns/hns_enet.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_enet.c b/drivers/net/ethernet/hisilicon/hns/hns_enet.c
index f0c9a41..66d1652 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_enet.c @@ -956,8 +956,8 @@ static int hns_nic_common_poll(struct napi_struct *napi, int budget)
napi_complete ( napi );
ring_data -> ring -> q -> handle -> dev -> ops -> toggle_ring_irq (
ring_data -> ring , 0 );
- - ring_data -> fini_process ( ring_data ); + if ( ring_data -> fini_process ) + ring_data -> fini_process ( ring_data ); return 0 ;
}
@@ -1720,6 +1720,7 @@ static int hns_nic_init_ring_data(struct hns_nic_priv *priv)
{
struct hnae_handle * h = priv -> ae_handle ;
struct hns_nic_ring_data * rd ;
+ bool is_ver1 = AE_IS_VER1 ( priv -> enet_ver ); int i ;
if ( h -> q_num > NIC_MAX_Q_PER_VF ) { @@ -1737,7 +1738,7 @@ static int hns_nic_init_ring_data(struct hns_nic_priv *priv)
rd -> queue_index = i ;
rd -> ring = & h -> qs [ i ] -> tx_ring ;
rd -> poll_one = hns_nic_tx_poll_one ;
- rd -> fini_process = hns_nic_tx_fini_pro ; + rd -> fini_process = is_ver1 ? hns_nic_tx_fini_pro : NULL ;
netif_napi_add ( priv -> netdev , & rd -> napi ,
hns_nic_common_poll , NIC_TX_CLEAN_MAX_NUM ); @@ -1749,7 +1750,7 @@ static int hns_nic_init_ring_data(struct hns_nic_priv *priv)
rd -> ring = & h -> qs [ i - h -> q_num ] -> rx_ring ;
rd -> poll_one = hns_nic_rx_poll_one ;
rd -> ex_process = hns_nic_rx_up_pro ;
- rd -> fini_process = hns_nic_rx_fini_pro ; + rd -> fini_process = is_ver1 ? hns_nic_rx_fini_pro : NULL ;
netif_napi_add ( priv -> netdev , & rd -> napi ,
hns_nic_common_poll , NIC_RX_CLEAN_MAX_NUM ); --
1.9.1
From: Kejian Yan <redacted>
There is a misuse of PTR as shown below:
ae_node = (void *)of_parse_phandle(dev->of_node,
"ae-handle",
0);
if (IS_ERR_OR_NULL(ae_node)) {
ret = PTR_ERR(ae_node);
dev_err(dev, "not find ae-handle\n");
goto out_read_prop_fail;
}
if the ae_node is NULL, PTR_ERR(ae_node) means it returns success. And the
return value should be -ENODEV.
Signed-off-by: Kejian Yan <redacted>
Signed-off-by: Yisen Zhuang <redacted>
---
drivers/net/ethernet/hisilicon/hns/hns_enet.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/hisilicon/hns/hns_enet.c b/drivers/net/ethernet/hisilicon/hns/hns_enet.c
index 66d1652..687204b 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_enet.c @@ -1814,7 +1814,7 @@ static int hns_nic_try_get_ae(struct net_device *ndev)
h = hnae_get_handle ( & priv -> netdev -> dev ,
priv -> ae_node , priv -> port_id , NULL );
if ( IS_ERR_OR_NULL ( h )) {
- ret = PTR_ERR ( h ); + ret = - ENODEV ; dev_dbg ( priv -> dev , "has not handle, register notifier! \n " );
goto out ;
} --
1.9.1
From: Yisen Zhuang <redacted>
Date: Thu, 24 Mar 2016 19:07:59 +0800
Here are some bug fixed patches for HNS driver.
They are:
quoted from Kejian, fix for the warning of passing zero to 'PTR_ERR' quoted from qianqian, four fixes for inappropriate operation in hns driver quoted from Sheng, one fix for optimization of irq proccess in hns driver, and one fix for hilink status for hns driver.
For more details, please see individual patches.
Series applied, thanks.