[PATCH net-next v3 0/2] net: hisilicon fix some bugs in HNS drivers

[PATCH net-next v3 0/2] net: hisilicon fix some bugs in HNS drivers

[huangdaode]

This patchset fixes the two bugs in HNS driver, one is remove the hnae sysfs interface
according to the review comments from Arnd Bergmann <arnd@arndb.de>, another
is fixing the wrong mac_id judgement bug which is found during internal tests.

change log:
v3:
 remove the hnae sysfs interface.

v2:
  1) remove first bug fix, which is fixed in another patch submitted by
     Arnd Bergmann <arnd@arndb.de>
  2) change the code sytyle according to Joe.

v1:
 initial version.

huangdaode (2):
  net: hisilicon rm hnae sysfs interface
  net: hisilicon fix a bug on Hisilicon Network Subsystem

 drivers/net/ethernet/hisilicon/hns/hnae.c         | 47 -----------------------
 drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c |  2 +-
 2 files changed, 1 insertion(+), 48 deletions(-)

-- 
1.9.1

[PATCH net-next v3 1/2] net: hisilicon rm hnae sysfs interface

[huangdaode]

This patch removes the hns driver hnae sysfs interface according to
Arnd's review comments.

Signed-off-by: huangdaode <huangdaode@hisilicon.com>
---
 drivers/net/ethernet/hisilicon/hns/hnae.c | 47 -------------------------------
 1 file changed, 47 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns/hnae.c b/drivers/net/ethernet/hisilicon/hns/hnae.c
index f52e99a..9d3bb83 100644
--- a/drivers/net/ethernet/hisilicon/hns/hnae.c
+++ b/drivers/net/ethernet/hisilicon/hns/hnae.c
@@ -436,59 +436,12 @@ void hnae_ae_unregister(struct hnae_ae_dev *hdev)
 }
 EXPORT_SYMBOL(hnae_ae_unregister);
 
-static ssize_t handles_show(struct device *dev,
-			    struct device_attribute *attr, char *buf)
-{
-	ssize_t s = 0;
-	struct hnae_ae_dev *hdev = cls_to_ae_dev(dev);
-	struct hnae_handle *h;
-	int i = 0, j;
-
-	list_for_each_entry_rcu(h, &hdev->handle_list, node) {
-		s += sprintf(buf + s, "handle %d (eport_id=%u from %s):\n",
-			    i++, h->eport_id, h->dev->name);
-		for (j = 0; j < h->q_num; j++) {
-			s += sprintf(buf + s, "\tqueue[%d] on %p\n",
-				     j, h->qs[i]->io_base);
-#define HANDEL_TX_MSG "\t\ttx_ring on %p:%u,%u,%u,%u,%u,%llu,%llu\n"
-			s += sprintf(buf + s,
-				     HANDEL_TX_MSG,
-				     h->qs[i]->tx_ring.io_base,
-				     h->qs[i]->tx_ring.buf_size,
-				     h->qs[i]->tx_ring.desc_num,
-				     h->qs[i]->tx_ring.max_desc_num_per_pkt,
-				     h->qs[i]->tx_ring.max_raw_data_sz_per_desc,
-				     h->qs[i]->tx_ring.max_pkt_size,
-				 h->qs[i]->tx_ring.stats.sw_err_cnt,
-				 h->qs[i]->tx_ring.stats.io_err_cnt);
-			s += sprintf(buf + s,
-				"\t\trx_ring on %p:%u,%u,%llu,%llu,%llu\n",
-				h->qs[i]->rx_ring.io_base,
-				h->qs[i]->rx_ring.buf_size,
-				h->qs[i]->rx_ring.desc_num,
-				h->qs[i]->rx_ring.stats.sw_err_cnt,
-				h->qs[i]->rx_ring.stats.io_err_cnt,
-				h->qs[i]->rx_ring.stats.seg_pkt_cnt);
-		}
-	}
-
-	return s;
-}
-
-static DEVICE_ATTR_RO(handles);
-static struct attribute *hnae_class_attrs[] = {
-	&dev_attr_handles.attr,
-	NULL,
-};
-ATTRIBUTE_GROUPS(hnae_class);
-
 static int __init hnae_init(void)
 {
 	hnae_class = class_create(THIS_MODULE, "hnae");
 	if (IS_ERR(hnae_class))
 		return PTR_ERR(hnae_class);
 
-	hnae_class->dev_groups = hnae_class_groups;
 	return 0;
 }
 
-- 
1.9.1

[PATCH net-next v3 2/2] net: hisilicon fix a bug on Hisilicon Network Subsystem

[huangdaode]

This patch fixes the wrong judgement of mac_id when get port num.

Signed-off-by: huangdaode <huangdaode@hisilicon.com>
Signed-off-by: yankejian <yankejian@huawei.com>
---
 drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c
index f8f7347..026b386 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c
@@ -179,7 +179,7 @@ static int hns_mac_get_inner_port_num(struct hns_mac_cb *mac_cb,
 			return -EINVAL;
 		}
 	} else if (mac_cb->dsaf_dev->dsaf_mode < DSAF_MODE_MAX) {
-		if (mac_cb->mac_id <= DSAF_MAX_PORT_NUM_PER_CHIP) {
+		if (mac_cb->mac_id >= DSAF_MAX_PORT_NUM_PER_CHIP) {
 			dev_err(mac_cb->dev,
 				"input invalid,%s mac%d vmid%d!\n",
 				mac_cb->dsaf_dev->ae_dev.name,
-- 
1.9.1

Re: [PATCH net-next v3 1/2] net: hisilicon rm hnae sysfs interface

[Arnd Bergmann]

On Friday 16 October 2015 11:54:16 huangdaode wrote:
> This patch removes the hns driver hnae sysfs interface according to
> Arnd's review comments.
> 
> Signed-off-by: huangdaode <huangdaode@hisilicon.com>
> 

Reviewed-by: Arnd Bergmann <arnd@arndb.de>

Thanks!

	Arnd

Re: [PATCH net-next v3 0/2] net: hisilicon fix some bugs in HNS drivers

[Arnd Bergmann]

On Friday 16 October 2015 11:54:15 huangdaode wrote:
> This patchset fixes the two bugs in HNS driver, one is remove the hnae sysfs interface
> according to the review comments from Arnd Bergmann <arnd@arndb.de>, another
> is fixing the wrong mac_id judgement bug which is found during internal tests.
> 
> change log:
> v3:
>  remove the hnae sysfs interface.
> 
> v2:
>   1) remove first bug fix, which is fixed in another patch submitted by
>      Arnd Bergmann <arnd@arndb.de>
>   2) change the code sytyle according to Joe.
> 
> v1:
>  initial version.
> 
> huangdaode (2):
>   net: hisilicon rm hnae sysfs interface
>   net: hisilicon fix a bug on Hisilicon Network Subsystem
> 
>  drivers/net/ethernet/hisilicon/hns/hnae.c         | 47 -----------------------
>  drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c |  2 +-
>  2 files changed, 1 insertion(+), 48 deletions(-)
> 
> 

I found two more bugs in build testing, will follow up with patches in this
thread

	Arnd.

[PATCH] net: hisilicon: include linux/vmalloc.h in dsaf

[Arnd Bergmann]

Some configurations fail to build the hns dsaf code because of
a missing header file:

ethernet/hisilicon/hns/hns_dsaf_main.c: In function 'hns_dsaf_init':
ethernet/hisilicon/hns/hns_dsaf_main.c:1096:2: error: implicit declaration of function 'vzalloc' [-Werror=implicit-function-declaration]
  priv->soft_mac_tbl = vzalloc(sizeof(*priv->soft_mac_tbl)

This adds the correct #include.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>

diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c
index 26ae6c64d74c..473a860baf1d 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c
@@ -17,6 +17,8 @@
 #include <linux/of_address.h>
 #include <linux/of_irq.h>
 #include <linux/device.h>
+#include <linux/vmalloc.h>
+
 #include "hns_dsaf_main.h"
 #include "hns_dsaf_rcb.h"
 #include "hns_dsaf_ppe.h"

[PATCH] net: hisilicon: add OF dependency

[Arnd Bergmann]

The HNS MDIO driver fails to build on older ARM machines that are not
yet converted to CONFIG_OF:

drivers/net/ethernet/hisilicon/hns_mdio.c: In function 'hns_mdio_bus_name':
drivers/net/ethernet/hisilicon/hns_mdio.c:405:14: error: 'OF_BAD_ADDR' undeclared (first use in this function)
  u64 taddr = OF_BAD_ADDR;
              ^
drivers/net/ethernet/hisilicon/hns_mdio.c:405:14: note: each undeclared identifier is reported only once for each function it appears in
drivers/net/ethernet/hisilicon/hns_mdio.c:409:11: error: implicit declaration of function 'of_translate_address' [-Werror=implicit-function-declaration]
   taddr = of_translate_address(np, addr);
           ^

This clarifies the dependency to ensure we don't attempt to build these
drivers without CONFIG_OF, but also adds a COMPILE_TEST alternative to
give us better build coverage testing.

Build-tested on x86 as well to ensure this actually works.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>

diff --git a/drivers/net/ethernet/hisilicon/Kconfig b/drivers/net/ethernet/hisilicon/Kconfig
index 8d12b587809e..f250dec488fd 100644
--- a/drivers/net/ethernet/hisilicon/Kconfig
+++ b/drivers/net/ethernet/hisilicon/Kconfig
@@ -5,7 +5,7 @@
 config NET_VENDOR_HISILICON
 	bool "Hisilicon devices"
 	default y
-	depends on ARM || ARM64
+	depends on OF && (ARM || ARM64 || COMPILE_TEST)
 	---help---
 	  If you have a network (Ethernet) card belonging to this class, say Y.
 

[PATCH] net: hix5hd2_gmac: avoid integer overload warning

[Arnd Bergmann]

BITS_RX_EN is an 'unsigned long' constant, so the ones complement of that
has bits set that do not fit into a 32-bit variable on 64-bit architectures,
which causes a harmless gcc warning:

drivers/net/ethernet/hisilicon/hix5hd2_gmac.c: In function 'hix5hd2_port_disable':
drivers/net/ethernet/hisilicon/hix5hd2_gmac.c:374:2: warning: large integer implicitly truncated to unsigned type [-Woverflow]
  writel_relaxed(~(BITS_RX_EN | BITS_TX_EN), priv->base + PORT_EN);

This adds a cast to (u32) to tell gcc that the code is indeed fine.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>

diff --git a/drivers/net/ethernet/hisilicon/hix5hd2_gmac.c b/drivers/net/ethernet/hisilicon/hix5hd2_gmac.c
index a5e077eac99a..e51892d518ff 100644
--- a/drivers/net/ethernet/hisilicon/hix5hd2_gmac.c
+++ b/drivers/net/ethernet/hisilicon/hix5hd2_gmac.c
@@ -371,7 +371,7 @@ static void hix5hd2_port_enable(struct hix5hd2_priv *priv)
 
 static void hix5hd2_port_disable(struct hix5hd2_priv *priv)
 {
-	writel_relaxed(~(BITS_RX_EN | BITS_TX_EN), priv->base + PORT_EN);
+	writel_relaxed(~(u32)(BITS_RX_EN | BITS_TX_EN), priv->base + PORT_EN);
 	writel_relaxed(0, priv->base + DESC_WR_RD_ENA);
 }
 

RE: [PATCH] net: hix5hd2_gmac: avoid integer overload warning

[David Laight]

From: Arnd Bergmann
> Sent: 16 October 2015 11:01
> BITS_RX_EN is an 'unsigned long' constant, so the ones complement of that
> has bits set that do not fit into a 32-bit variable on 64-bit architectures,
> which causes a harmless gcc warning:
...
>  static void hix5hd2_port_disable(struct hix5hd2_priv *priv)
>  {
> -	writel_relaxed(~(BITS_RX_EN | BITS_TX_EN), priv->base + PORT_EN);
> +	writel_relaxed(~(u32)(BITS_RX_EN | BITS_TX_EN), priv->base + PORT_EN);
>  	writel_relaxed(0, priv->base + DESC_WR_RD_ENA);

ISTM that just means that the constants shouldn't be 'long'.

	David

Re: [PATCH] net: hix5hd2_gmac: avoid integer overload warning

[Arnd Bergmann]

On Friday 16 October 2015 11:14:44 David Laight wrote:
> From: Arnd Bergmann
> > Sent: 16 October 2015 11:01
> > BITS_RX_EN is an 'unsigned long' constant, so the ones complement of that
> > has bits set that do not fit into a 32-bit variable on 64-bit architectures,
> > which causes a harmless gcc warning:
> ...
> >  static void hix5hd2_port_disable(struct hix5hd2_priv *priv)
> >  {
> > -     writel_relaxed(~(BITS_RX_EN | BITS_TX_EN), priv->base + PORT_EN);
> > +     writel_relaxed(~(u32)(BITS_RX_EN | BITS_TX_EN), priv->base + PORT_EN);
> >       writel_relaxed(0, priv->base + DESC_WR_RD_ENA);
> 
> ISTM that just means that the constants shouldn't be 'long'.

Right, but that would probably mean changing the BIT() macro or not using it
here. In the past I've argued against using that macro, but I've given
up that fight.

	Arnd

Re: [PATCH] net: hisilicon: add OF dependency

[kbuild test robot]

Hi Arnd,

[auto build test ERROR on net-next/master -- if it's inappropriate base, please suggest rules for selecting the more suitable base]

url:    https://github.com/0day-ci/linux/commits/Arnd-Bergmann/net-hisilicon-add-OF-dependency/20151016-173818
config: x86_64-allmodconfig
reproduce:
        make ARCH=x86_64  allmodconfig
        make ARCH=x86_64 

All error/warnings (new ones prefixed by >>):


vim +/vzalloc +1123 drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c

511e6bc0 huangdaode 2015-09-17  1117  
511e6bc0 huangdaode 2015-09-17  1118  	ret = hns_dsaf_init_hw(dsaf_dev);
511e6bc0 huangdaode 2015-09-17  1119  	if (ret)
511e6bc0 huangdaode 2015-09-17  1120  		return ret;
511e6bc0 huangdaode 2015-09-17  1121  
511e6bc0 huangdaode 2015-09-17  1122  	/* malloc mem for tcam mac key(vlan+mac) */
511e6bc0 huangdaode 2015-09-17 @1123  	priv->soft_mac_tbl = vzalloc(sizeof(*priv->soft_mac_tbl)
511e6bc0 huangdaode 2015-09-17  1124  		  * DSAF_TCAM_SUM);
511e6bc0 huangdaode 2015-09-17  1125  	if (!priv->soft_mac_tbl) {
511e6bc0 huangdaode 2015-09-17  1126  		ret = -ENOMEM;
511e6bc0 huangdaode 2015-09-17  1127  		goto remove_hw;
511e6bc0 huangdaode 2015-09-17  1128  	}
511e6bc0 huangdaode 2015-09-17  1129  
511e6bc0 huangdaode 2015-09-17  1130  	/*all entry invall */
511e6bc0 huangdaode 2015-09-17  1131  	for (i = 0; i < DSAF_TCAM_SUM; i++)
511e6bc0 huangdaode 2015-09-17  1132  		(priv->soft_mac_tbl + i)->index = DSAF_INVALID_ENTRY_IDX;
511e6bc0 huangdaode 2015-09-17  1133  
511e6bc0 huangdaode 2015-09-17  1134  	return 0;
511e6bc0 huangdaode 2015-09-17  1135  
511e6bc0 huangdaode 2015-09-17  1136  remove_hw:
511e6bc0 huangdaode 2015-09-17  1137  	hns_dsaf_remove_hw(dsaf_dev);
511e6bc0 huangdaode 2015-09-17  1138  	return ret;
511e6bc0 huangdaode 2015-09-17  1139  }
511e6bc0 huangdaode 2015-09-17  1140  
511e6bc0 huangdaode 2015-09-17  1141  /**
511e6bc0 huangdaode 2015-09-17  1142   * hns_dsaf_free - free dsa fabric
511e6bc0 huangdaode 2015-09-17  1143   * @dsaf_dev: dsa fabric device struct pointer
511e6bc0 huangdaode 2015-09-17  1144   */
511e6bc0 huangdaode 2015-09-17  1145  static void hns_dsaf_free(struct dsaf_device *dsaf_dev)
511e6bc0 huangdaode 2015-09-17  1146  {
511e6bc0 huangdaode 2015-09-17  1147  	struct dsaf_drv_priv *priv =
511e6bc0 huangdaode 2015-09-17  1148  	    (struct dsaf_drv_priv *)hns_dsaf_dev_priv(dsaf_dev);
511e6bc0 huangdaode 2015-09-17  1149  
511e6bc0 huangdaode 2015-09-17  1150  	hns_dsaf_remove_hw(dsaf_dev);
511e6bc0 huangdaode 2015-09-17  1151  
511e6bc0 huangdaode 2015-09-17  1152  	/* free all mac mem */
511e6bc0 huangdaode 2015-09-17 @1153  	vfree(priv->soft_mac_tbl);
511e6bc0 huangdaode 2015-09-17  1154  	priv->soft_mac_tbl = NULL;
511e6bc0 huangdaode 2015-09-17  1155  }
511e6bc0 huangdaode 2015-09-17  1156  

:::::: The code at line 1123 was first introduced by commit
:::::: 511e6bc071db1484d1a3d1d0bd4c244cf33910ff net: add Hisilicon Network Subsystem DSAF support

:::::: TO: huangdaode <huangdaode@hisilicon.com>
:::::: CC: David S. Miller <davem@davemloft.net>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Re: [PATCH] net: hix5hd2_gmac: avoid integer overload warning

[Joe Perches]

On Fri, 2015-10-16 at 13:28 +0200, Arnd Bergmann wrote:
> On Friday 16 October 2015 11:14:44 David Laight wrote:
> > From: Arnd Bergmann
> > > Sent: 16 October 2015 11:01
> > > BITS_RX_EN is an 'unsigned long' constant, so the ones complement of that
> > > has bits set that do not fit into a 32-bit variable on 64-bit architectures,
> > > which causes a harmless gcc warning:
> > ...
> > >  static void hix5hd2_port_disable(struct hix5hd2_priv *priv)
> > >  {
> > > -     writel_relaxed(~(BITS_RX_EN | BITS_TX_EN), priv->base + PORT_EN);
> > > +     writel_relaxed(~(u32)(BITS_RX_EN | BITS_TX_EN), priv->base + PORT_EN);
> > >       writel_relaxed(0, priv->base + DESC_WR_RD_ENA);
> > 
> > ISTM that just means that the constants shouldn't be 'long'.
> 
> Right, but that would probably mean changing the BIT() macro or not using it
> here. In the past I've argued against using that macro, but I've given
> up that fight.

Fight on... (Somebody must have gone to USC here)

There might be value in a BIT_U32 macro.
Maybe BIT_U64 too.

Re: [PATCH] net: hisilicon: add OF dependency

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2848 bytes --]

Hi Arnd,

[auto build test ERROR on net-next/master -- if it's inappropriate base, please suggest rules for selecting the more suitable base]

url:    https://github.com/0day-ci/linux/commits/Arnd-Bergmann/net-hisilicon-add-OF-dependency/20151016-173818
config: um-allyesconfig (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=um 

All errors (new ones prefixed by >>):

   drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c: In function 'hns_dsaf_get_cfg':
>> drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c:151:3: error: implicit declaration of function 'iounmap' [-Werror=implicit-function-declaration]
      iounmap(dsaf_dev->io_base);
      ^
   cc1: some warnings being treated as errors

vim +/iounmap +151 drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c

511e6bc0 huangdaode 2015-09-17  135  	dsaf_dev->buf_size_type = hns_rcb_buf_size2type(buf_size);
511e6bc0 huangdaode 2015-09-17  136  	if (dsaf_dev->buf_size_type < 0) {
511e6bc0 huangdaode 2015-09-17  137  		dev_err(dsaf_dev->dev,
511e6bc0 huangdaode 2015-09-17  138  			"buf_size(%d) is wrong!\n", buf_size);
511e6bc0 huangdaode 2015-09-17  139  		goto unmap_base_addr;
511e6bc0 huangdaode 2015-09-17  140  	}
511e6bc0 huangdaode 2015-09-17  141  
511e6bc0 huangdaode 2015-09-17  142  	if (!dma_set_mask_and_coherent(dsaf_dev->dev, DMA_BIT_MASK(64ULL)))
511e6bc0 huangdaode 2015-09-17  143  		dev_dbg(dsaf_dev->dev, "set mask to 64bit\n");
511e6bc0 huangdaode 2015-09-17  144  	else
511e6bc0 huangdaode 2015-09-17  145  		dev_err(dsaf_dev->dev, "set mask to 64bit fail!\n");
511e6bc0 huangdaode 2015-09-17  146  
511e6bc0 huangdaode 2015-09-17  147  	return 0;
511e6bc0 huangdaode 2015-09-17  148  
511e6bc0 huangdaode 2015-09-17  149  unmap_base_addr:
511e6bc0 huangdaode 2015-09-17  150  	if (dsaf_dev->io_base)
511e6bc0 huangdaode 2015-09-17 @151  		iounmap(dsaf_dev->io_base);
511e6bc0 huangdaode 2015-09-17  152  	if (dsaf_dev->ppe_base)
511e6bc0 huangdaode 2015-09-17  153  		iounmap(dsaf_dev->ppe_base);
511e6bc0 huangdaode 2015-09-17  154  	if (dsaf_dev->sds_base)
511e6bc0 huangdaode 2015-09-17  155  		iounmap(dsaf_dev->sds_base);
511e6bc0 huangdaode 2015-09-17  156  	if (dsaf_dev->sc_base)
511e6bc0 huangdaode 2015-09-17  157  		iounmap(dsaf_dev->sc_base);
511e6bc0 huangdaode 2015-09-17  158  	if (dsaf_dev->cpld_base)
511e6bc0 huangdaode 2015-09-17  159  		iounmap(dsaf_dev->cpld_base);

:::::: The code at line 151 was first introduced by commit
:::::: 511e6bc071db1484d1a3d1d0bd4c244cf33910ff net: add Hisilicon Network Subsystem DSAF support

:::::: TO: huangdaode <huangdaode@hisilicon.com>
:::::: CC: David S. Miller <davem@davemloft.net>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 16799 bytes --]

Re: [PATCH] net: hisilicon: add OF dependency

[Joe Perches]

On Sat, 2015-10-17 at 02:16 +0800, kbuild test robot wrote:
> All errors (new ones prefixed by >>):
>    drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c: In function 'hns_dsaf_get_cfg':
> >> drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c:151:3: error: implicit declaration of function 'iounmap' [-Werror=implicit-function-declaration]
>       iounmap(dsaf_dev->io_base);
>       ^
>    cc1: some warnings being treated as errors
> 
> vim +/iounmap +151 drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c
[]
> 511e6bc0 huangdaode 2015-09-17  142  	if (!dma_set_mask_and_coherent(dsaf_dev->dev, DMA_BIT_MASK(64ULL)))

btw: this should be DMA_BIT_MASK(64) without the ULL

Re: [PATCH] net: hisilicon: add OF dependency

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 3596 bytes --]

Hi Arnd,

[auto build test WARNING on net-next/master -- if it's inappropriate base, please suggest rules for selecting the more suitable base]

url:    https://github.com/0day-ci/linux/commits/Arnd-Bergmann/net-hisilicon-add-OF-dependency/20151016-173818
config: ia64-allmodconfig (attached as .config)
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=ia64 

All warnings (new ones prefixed by >>):

   In file included from arch/ia64/include/asm/smp.h:20:0,
                    from include/linux/smp.h:59,
                    from include/linux/topology.h:33,
                    from include/linux/gfp.h:8,
                    from include/linux/kmod.h:22,
                    from include/linux/module.h:13,
                    from drivers/net/ethernet/hisilicon/hix5hd2_gmac.c:10:
   drivers/net/ethernet/hisilicon/hix5hd2_gmac.c: In function 'hix5hd2_port_disable':
   arch/ia64/include/asm/io.h:398:38: warning: large integer implicitly truncated to unsigned type [-Woverflow]
    #define writel_relaxed(v,a) __writel((v), (a))
                                         ^
>> drivers/net/ethernet/hisilicon/hix5hd2_gmac.c:374:2: note: in expansion of macro 'writel_relaxed'
     writel_relaxed(~(BITS_RX_EN | BITS_TX_EN), priv->base + PORT_EN);
     ^

vim +/writel_relaxed +374 drivers/net/ethernet/hisilicon/hix5hd2_gmac.c

57c5bc9a Zhangfei Gao 2014-06-03  358  	writel_relaxed(DEF_INT_MASK, priv->base + ENA_PMU_INT);
57c5bc9a Zhangfei Gao 2014-06-03  359  }
57c5bc9a Zhangfei Gao 2014-06-03  360  
57c5bc9a Zhangfei Gao 2014-06-03  361  static void hix5hd2_irq_disable(struct hix5hd2_priv *priv)
57c5bc9a Zhangfei Gao 2014-06-03  362  {
57c5bc9a Zhangfei Gao 2014-06-03  363  	writel_relaxed(0, priv->base + ENA_PMU_INT);
57c5bc9a Zhangfei Gao 2014-06-03  364  }
57c5bc9a Zhangfei Gao 2014-06-03  365  
57c5bc9a Zhangfei Gao 2014-06-03  366  static void hix5hd2_port_enable(struct hix5hd2_priv *priv)
57c5bc9a Zhangfei Gao 2014-06-03  367  {
57c5bc9a Zhangfei Gao 2014-06-03  368  	writel_relaxed(0xf, priv->base + DESC_WR_RD_ENA);
57c5bc9a Zhangfei Gao 2014-06-03  369  	writel_relaxed(BITS_RX_EN | BITS_TX_EN, priv->base + PORT_EN);
57c5bc9a Zhangfei Gao 2014-06-03  370  }
57c5bc9a Zhangfei Gao 2014-06-03  371  
57c5bc9a Zhangfei Gao 2014-06-03  372  static void hix5hd2_port_disable(struct hix5hd2_priv *priv)
57c5bc9a Zhangfei Gao 2014-06-03  373  {
57c5bc9a Zhangfei Gao 2014-06-03 @374  	writel_relaxed(~(BITS_RX_EN | BITS_TX_EN), priv->base + PORT_EN);
57c5bc9a Zhangfei Gao 2014-06-03  375  	writel_relaxed(0, priv->base + DESC_WR_RD_ENA);
57c5bc9a Zhangfei Gao 2014-06-03  376  }
57c5bc9a Zhangfei Gao 2014-06-03  377  
57c5bc9a Zhangfei Gao 2014-06-03  378  static void hix5hd2_hw_set_mac_addr(struct net_device *dev)
57c5bc9a Zhangfei Gao 2014-06-03  379  {
57c5bc9a Zhangfei Gao 2014-06-03  380  	struct hix5hd2_priv *priv = netdev_priv(dev);
57c5bc9a Zhangfei Gao 2014-06-03  381  	unsigned char *mac = dev->dev_addr;
57c5bc9a Zhangfei Gao 2014-06-03  382  	u32 val;

:::::: The code at line 374 was first introduced by commit
:::::: 57c5bc9ad7d799e9507ba6e993398d2c55f03fab net: hisilicon: add hix5hd2 mac driver

:::::: TO: Zhangfei Gao <zhangfei.gao@linaro.org>
:::::: CC: David S. Miller <davem@davemloft.net>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 41107 bytes --]

Re: [PATCH] net: hix5hd2_gmac: avoid integer overload warning

[Sergei Shtylyov]

On 10/16/2015 09:04 PM, Joe Perches wrote:

>>>> BITS_RX_EN is an 'unsigned long' constant, so the ones complement of that
>>>> has bits set that do not fit into a 32-bit variable on 64-bit architectures,
>>>> which causes a harmless gcc warning:
>>> ...
>>>>   static void hix5hd2_port_disable(struct hix5hd2_priv *priv)
>>>>   {
>>>> -     writel_relaxed(~(BITS_RX_EN | BITS_TX_EN), priv->base + PORT_EN);
>>>> +     writel_relaxed(~(u32)(BITS_RX_EN | BITS_TX_EN), priv->base + PORT_EN);
>>>>        writel_relaxed(0, priv->base + DESC_WR_RD_ENA);
>>>
>>> ISTM that just means that the constants shouldn't be 'long'.
>>
>> Right, but that would probably mean changing the BIT() macro or not using it
>> here. In the past I've argued against using that macro, but I've given
>> up that fight.
>
> Fight on... (Somebody must have gone to USC here)
>
> There might be value in a BIT_U32 macro.
> Maybe BIT_U64 too.

    There's BIT_ULL() already.

MBR, Sergei

Re: [PATCH] net: hisilicon: add OF dependency

[Arnd Bergmann]

On Saturday 17 October 2015 02:39:51 kbuild test robot wrote:
> 
> [auto build test WARNING on net-next/master -- if it's inappropriate base, please suggest rules for selecting the more suitable base]
> 
> url:    https://github.com/0day-ci/linux/commits/Arnd-Bergmann/net-hisilicon-add-OF-dependency/20151016-173818
> config: ia64-allmodconfig (attached as .config)
> reproduce:
>         wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # save the attached .config to linux build tree
>         make.cross ARCH=ia64 
> 

I guess I should have sent my three patches as a series. The build bot
found the same two bugs I had found myself and sent fixes for, but
applying the 'add OF dependency' without the other two makes them appear
in more configurations.

For all I can see, the two reports here are fixed if all three of my
patches are applied.

	Arnd

Re: [PATCH] net: hix5hd2_gmac: avoid integer overload warning

[Joe Perches]

On Fri, 2015-10-16 at 21:50 +0300, Sergei Shtylyov wrote:
> On 10/16/2015 09:04 PM, Joe Perches wrote:
> 
> >>>> BITS_RX_EN is an 'unsigned long' constant, so the ones complement of that
> >>>> has bits set that do not fit into a 32-bit variable on 64-bit architectures,
> >>>> which causes a harmless gcc warning:
> >>> ...
> >>>>   static void hix5hd2_port_disable(struct hix5hd2_priv *priv)
> >>>>   {
> >>>> -     writel_relaxed(~(BITS_RX_EN | BITS_TX_EN), priv->base + PORT_EN);
> >>>> +     writel_relaxed(~(u32)(BITS_RX_EN | BITS_TX_EN), priv->base + PORT_EN);
> >>>>        writel_relaxed(0, priv->base + DESC_WR_RD_ENA);
> >>>
> >>> ISTM that just means that the constants shouldn't be 'long'.
> >>
> >> Right, but that would probably mean changing the BIT() macro or not using it
> >> here. In the past I've argued against using that macro, but I've given
> >> up that fight.
> >
> > Fight on... (Somebody must have gone to USC here)
> >
> > There might be value in aefin BIT_U32 macro.
> > Maybe BIT_U64 too.
> 
>     There's BIT_ULL() already.

I know, but symmetry is good.
I think there'd be no harm in adding it.
Perhaps adding all the sized variants would be useful.

Something like:

#define BIT_OF_TYPE(type, nr)						\
({									\
	typeof(type) rtn;						\
	BUILD_BUG_ON(__builtin_constant_p(nr) &&			\
		     ((nr) < 0 ||					\
		      (nr) >= sizeof(type) * BITS_PER_BYTE));		\
	rtn = ((type)1) << (nr);					\
	rtn;								\
})

#define BIT_U8(nr)	BIT_OF_TYPE(u8, nr)
#define BIT_U16(nr)	BIT_OF_TYPE(u16, nr)
#define BIT_U32(nr)	BIT_OF_TYPE(u32, nr)
#define BIT_U64(nr)	BIT_OF_TYPE(u64, nr)

Re: [PATCH] net: hix5hd2_gmac: avoid integer overload warning

[Arnd Bergmann]

On Friday 16 October 2015 14:22:15 Joe Perches wrote:
> On Fri, 2015-10-16 at 21:50 +0300, Sergei Shtylyov wrote:
> > On 10/16/2015 09:04 PM, Joe Perches wrote:
> > 
> > >>>> BITS_RX_EN is an 'unsigned long' constant, so the ones complement of that
> > >>>> has bits set that do not fit into a 32-bit variable on 64-bit architectures,
> > >>>> which causes a harmless gcc warning:
> > >>> ...
> > >>>>   static void hix5hd2_port_disable(struct hix5hd2_priv *priv)
> > >>>>   {
> > >>>> -     writel_relaxed(~(BITS_RX_EN | BITS_TX_EN), priv->base + PORT_EN);
> > >>>> +     writel_relaxed(~(u32)(BITS_RX_EN | BITS_TX_EN), priv->base + PORT_EN);
> > >>>>        writel_relaxed(0, priv->base + DESC_WR_RD_ENA);
> > >>>
> > >>> ISTM that just means that the constants shouldn't be 'long'.
> > >>
> > >> Right, but that would probably mean changing the BIT() macro or not using it
> > >> here. In the past I've argued against using that macro, but I've given
> > >> up that fight.
> > >
> > > Fight on... (Somebody must have gone to USC here)

Ok, I'll try:

Please stop this nonsense!

;-)

> > > There might be value in aefin BIT_U32 macro.
> > > Maybe BIT_U64 too.
> > 
> >     There's BIT_ULL() already.
> 
> I know, but symmetry is good.
> I think there'd be no harm in adding it.
> Perhaps adding all the sized variants would be useful.
> 
> Something like:
> 
> #define BIT_OF_TYPE(type, nr)						\
> ({									\
> 	typeof(type) rtn;						\
> 	BUILD_BUG_ON(__builtin_constant_p(nr) &&			\
> 		     ((nr) < 0 ||					\
> 		      (nr) >= sizeof(type) * BITS_PER_BYTE));		\
> 	rtn = ((type)1) << (nr);					\
> 	rtn;								\
> })
> 
> #define BIT_U8(nr)	BIT_OF_TYPE(u8, nr)
> #define BIT_U16(nr)	BIT_OF_TYPE(u16, nr)
> #define BIT_U32(nr)	BIT_OF_TYPE(u32, nr)
> #define BIT_U64(nr)	BIT_OF_TYPE(u64, nr)

As I said, I'd rather see less uses of BIT() instead of more. While
using 'BIT(23)' is often than the open-coded '1 << 23', I wish more
people would write that as '0x00800000' instead. It's easier to
match with data sheets, and to compare to printk output, plus
it's non-ambiguous if you are dealing with data sheets that use
the IBM convention of counting the bits from the other end.

	Arnd

Re: [PATCH net-next v3 0/2] net: hisilicon fix some bugs in HNS drivers

[huangdaode]

On 2015/10/16 17:30, Arnd Bergmann wrote:
> On Friday 16 October 2015 11:54:15 huangdaode wrote:
>> This patchset fixes the two bugs in HNS driver, one is remove the hnae sysfs interface
>> according to the review comments from Arnd Bergmann <arnd@arndb.de>, another
>> is fixing the wrong mac_id judgement bug which is found during internal tests.
>>
>> change log:
>> v3:
>>   remove the hnae sysfs interface.
>>
>> v2:
>>    1) remove first bug fix, which is fixed in another patch submitted by
>>       Arnd Bergmann <arnd@arndb.de>
>>    2) change the code sytyle according to Joe.
>>
>> v1:
>>   initial version.
>>
>> huangdaode (2):
>>    net: hisilicon rm hnae sysfs interface
>>    net: hisilicon fix a bug on Hisilicon Network Subsystem
>>
>>   drivers/net/ethernet/hisilicon/hns/hnae.c         | 47 -----------------------
>>   drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c |  2 +-
>>   2 files changed, 1 insertion(+), 48 deletions(-)
>>
>>
> I found two more bugs in build testing, will follow up with patches in this
> thread
>
> 	Arnd.
>
> .

Hi Arnd, thanks for your comments. i see you adding  three patches in 
this thread to fix related bugs.
should  I need to re-send a new version patch which contains your other 
three patches  or just wait for
David Miller to merge this thread?

thanks
Daode Huang
   .


-- 
Best Regards
Daode Huang

Re: [PATCH net-next v3 0/2] net: hisilicon fix some bugs in HNS drivers

[David Miller]

From: huangdaode <huangdaode@hisilicon.com>
Date: Fri, 16 Oct 2015 11:54:15 +0800

> This patchset fixes the two bugs in HNS driver, one is remove the hnae sysfs interface
> according to the review comments from Arnd Bergmann <arnd@arndb.de>, another
> is fixing the wrong mac_id judgement bug which is found during internal tests.

Series applied, tanks.

Re: [PATCH] net: hisilicon: include linux/vmalloc.h in dsaf

[David Miller]

From: Arnd Bergmann <arnd@arndb.de>
Date: Fri, 16 Oct 2015 11:30:56 +0200

> Some configurations fail to build the hns dsaf code because of
> a missing header file:
> 
> ethernet/hisilicon/hns/hns_dsaf_main.c: In function 'hns_dsaf_init':
> ethernet/hisilicon/hns/hns_dsaf_main.c:1096:2: error: implicit declaration of function 'vzalloc' [-Werror=implicit-function-declaration]
>   priv->soft_mac_tbl = vzalloc(sizeof(*priv->soft_mac_tbl)
> 
> This adds the correct #include.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Applied, thank you.

Re: [PATCH] net: hisilicon: add OF dependency

[David Miller]

From: Arnd Bergmann <arnd@arndb.de>
Date: Fri, 16 Oct 2015 11:33:49 +0200

> The HNS MDIO driver fails to build on older ARM machines that are not
> yet converted to CONFIG_OF:
> 
> drivers/net/ethernet/hisilicon/hns_mdio.c: In function 'hns_mdio_bus_name':
> drivers/net/ethernet/hisilicon/hns_mdio.c:405:14: error: 'OF_BAD_ADDR' undeclared (first use in this function)
>   u64 taddr = OF_BAD_ADDR;
>               ^
> drivers/net/ethernet/hisilicon/hns_mdio.c:405:14: note: each undeclared identifier is reported only once for each function it appears in
> drivers/net/ethernet/hisilicon/hns_mdio.c:409:11: error: implicit declaration of function 'of_translate_address' [-Werror=implicit-function-declaration]
>    taddr = of_translate_address(np, addr);
>            ^
> 
> This clarifies the dependency to ensure we don't attempt to build these
> drivers without CONFIG_OF, but also adds a COMPILE_TEST alternative to
> give us better build coverage testing.
> 
> Build-tested on x86 as well to ensure this actually works.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Applied.

Re: [PATCH] net: hix5hd2_gmac: avoid integer overload warning

[David Miller]

From: Arnd Bergmann <arnd@arndb.de>
Date: Fri, 16 Oct 2015 12:00:51 +0200

> BITS_RX_EN is an 'unsigned long' constant, so the ones complement of that
> has bits set that do not fit into a 32-bit variable on 64-bit architectures,
> which causes a harmless gcc warning:
> 
> drivers/net/ethernet/hisilicon/hix5hd2_gmac.c: In function 'hix5hd2_port_disable':
> drivers/net/ethernet/hisilicon/hix5hd2_gmac.c:374:2: warning: large integer implicitly truncated to unsigned type [-Woverflow]
>   writel_relaxed(~(BITS_RX_EN | BITS_TX_EN), priv->base + PORT_EN);
> 
> This adds a cast to (u32) to tell gcc that the code is indeed fine.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Applied.