Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1248317 > unrolled thread

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

Back to article view | Back to linux.kernel

Contents

  [PATCH net-next v3 0/2] net: hisilicon fix some bugs in HNS drivers huangdaode <huangdaode@hisilicon.com> - 2015-10-16 05:40 +0200
    [PATCH net-next v3 1/2] net: hisilicon rm hnae sysfs interface huangdaode <huangdaode@hisilicon.com> - 2015-10-16 05:40 +0200
      Re: [PATCH net-next v3 1/2] net: hisilicon rm hnae sysfs interface Arnd Bergmann <arnd@arndb.de> - 2015-10-16 11:30 +0200
    [PATCH] net: hisilicon: include linux/vmalloc.h in dsaf Arnd Bergmann <arnd@arndb.de> - 2015-10-16 11:40 +0200
    [PATCH] net: hisilicon: add OF dependency Arnd Bergmann <arnd@arndb.de> - 2015-10-16 11:40 +0200
      Re: [PATCH] net: hisilicon: add OF dependency kbuild test robot <lkp@intel.com> - 2015-10-16 19:10 +0200
      Re: [PATCH] net: hisilicon: add OF dependency kbuild test robot <lkp@intel.com> - 2015-10-16 20:20 +0200
        Re: [PATCH] net: hisilicon: add OF dependency Joe Perches <joe@perches.com> - 2015-10-16 20:30 +0200
    Re: [PATCH net-next v3 0/2] net: hisilicon fix some bugs in HNS drivers Arnd Bergmann <arnd@arndb.de> - 2015-10-16 11:40 +0200
    [PATCH] net: hix5hd2_gmac: avoid integer overload warning Arnd Bergmann <arnd@arndb.de> - 2015-10-16 12:10 +0200
      RE: [PATCH] net: hix5hd2_gmac: avoid integer overload warning David Laight <David.Laight@ACULAB.COM> - 2015-10-16 13:20 +0200
        Re: [PATCH] net: hix5hd2_gmac: avoid integer overload warning Arnd Bergmann <arnd@arndb.de> - 2015-10-16 13:30 +0200
          Re: [PATCH] net: hix5hd2_gmac: avoid integer overload warning Joe Perches <joe@perches.com> - 2015-10-16 20:10 +0200
            Re: [PATCH] net: hix5hd2_gmac: avoid integer overload warning Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2015-10-16 21:00 +0200
              Re: [PATCH] net: hix5hd2_gmac: avoid integer overload warning Joe Perches <joe@perches.com> - 2015-10-16 23:30 +0200
                Re: [PATCH] net: hix5hd2_gmac: avoid integer overload warning Arnd Bergmann <arnd@arndb.de> - 2015-10-16 23:50 +0200

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

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

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

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

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

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

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

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"

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1248522 — [PATCH] net: hisilicon: add OF dependency

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.
 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1248990 — Re: [PATCH] net: hisilicon: add OF dependency

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1249041 — Re: [PATCH] net: hisilicon: add OF dependency

[Multipart message — attachments visible in raw view] — view raw

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

[toc] | [prev] | [next] | [standalone]

#1249048 — Re: [PATCH] net: hisilicon: add OF dependency

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



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1248524

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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1248551 — [PATCH] net: hix5hd2_gmac: avoid integer overload warning

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);
 }
 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1248650 — RE: [PATCH] net: hix5hd2_gmac: avoid integer overload warning

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1248657 — Re: [PATCH] net: hix5hd2_gmac: avoid integer overload warning

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1249038 — Re: [PATCH] net: hix5hd2_gmac: avoid integer overload warning

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.



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1249072 — Re: [PATCH] net: hix5hd2_gmac: avoid integer overload warning

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1249155 — Re: [PATCH] net: hix5hd2_gmac: avoid integer overload warning

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)


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1249170 — Re: [PATCH] net: hix5hd2_gmac: avoid integer overload warning

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web