[PATCH 0/9] network driver fixes

[PATCH 0/9] network driver fixes

[Arnd Bergmann]

These are all fixes for relatively harmless bugs that showed up
in my randconfig testing, so they should not be needed for v4.5
but get merged into net-next.

I've managed to address all 'uninitialized variable' warnings that
I get in ARM randconfig kernels now, this series includes the
last five I got in network drivers. They are often really annoying
warnings but when we get new ones, they often are about actual
bugs in corner cases, so I'm trying hard to eliminate the false
positives here to get people to pay attention to added warnings.
I've recently tried building with an older gcc and found tons more
that are all bogus, this series only addresses the ones that
gcc-5.2 finds.

Arnd Bergmann (9):
  net: davinci_cpdma: use dma_addr_t for DMA address
  net: hp100: remove unnecessary #ifdefs
  net: bgmac: clarify CONFIG_BCMA dependency
  net: moxart: use correct accessors for DMA memory
  net: fddi/defxx: avoid warning about uninitialized variable use
  net: vxge: avoid unused function warnings
  net: macb: avoid uninitialized variables
  net: nb8800: avoid uninitialized variable warning
  net: tg3: avoid uninitialized variable warning

 drivers/net/ethernet/aurora/nb8800.c           |  4 +--
 drivers/net/ethernet/broadcom/Kconfig          |  5 ++-
 drivers/net/ethernet/broadcom/tg3.c            |  2 +-
 drivers/net/ethernet/cadence/macb.c            |  1 +
 drivers/net/ethernet/hp/hp100.c                | 18 -----------
 drivers/net/ethernet/moxa/moxart_ether.c       | 42 ++++++++++++++++----------
 drivers/net/ethernet/moxa/moxart_ether.h       |  4 +--
 drivers/net/ethernet/neterion/vxge/vxge-main.c | 31 ++++++++-----------
 drivers/net/ethernet/ti/davinci_cpdma.c        | 12 ++++----
 drivers/net/fddi/defxx.c                       |  5 +++
 10 files changed, 59 insertions(+), 65 deletions(-)

-- 
2.7.0

[PATCH 0/9] network driver fixes

[Arnd Bergmann]

These are all fixes for relatively harmless bugs that showed up
in my randconfig testing, so they should not be needed for v4.5
but get merged into net-next.

I've managed to address all 'uninitialized variable' warnings that
I get in ARM randconfig kernels now, this series includes the
last five I got in network drivers. They are often really annoying
warnings but when we get new ones, they often are about actual
bugs in corner cases, so I'm trying hard to eliminate the false
positives here to get people to pay attention to added warnings.
I've recently tried building with an older gcc and found tons more
that are all bogus, this series only addresses the ones that
gcc-5.2 finds.

Arnd Bergmann (9):
  net: davinci_cpdma: use dma_addr_t for DMA address
  net: hp100: remove unnecessary #ifdefs
  net: bgmac: clarify CONFIG_BCMA dependency
  net: moxart: use correct accessors for DMA memory
  net: fddi/defxx: avoid warning about uninitialized variable use
  net: vxge: avoid unused function warnings
  net: macb: avoid uninitialized variables
  net: nb8800: avoid uninitialized variable warning
  net: tg3: avoid uninitialized variable warning

 drivers/net/ethernet/aurora/nb8800.c           |  4 +--
 drivers/net/ethernet/broadcom/Kconfig          |  5 ++-
 drivers/net/ethernet/broadcom/tg3.c            |  2 +-
 drivers/net/ethernet/cadence/macb.c            |  1 +
 drivers/net/ethernet/hp/hp100.c                | 18 -----------
 drivers/net/ethernet/moxa/moxart_ether.c       | 42 ++++++++++++++++----------
 drivers/net/ethernet/moxa/moxart_ether.h       |  4 +--
 drivers/net/ethernet/neterion/vxge/vxge-main.c | 31 ++++++++-----------
 drivers/net/ethernet/ti/davinci_cpdma.c        | 12 ++++----
 drivers/net/fddi/defxx.c                       |  5 +++
 10 files changed, 59 insertions(+), 65 deletions(-)

-- 
2.7.0

[PATCH 1/9] net: davinci_cpdma: use dma_addr_t for DMA address

[Arnd Bergmann]

The davinci_cpdma mixes up physical addresses as seen from the CPU
and DMA addresses as seen from a DMA master, since it can operate
on both normal memory or an on-chip buffer. If dma_addr_t is
different from phys_addr_t, this means we get a compile-time warning
about the type mismatch:

ethernet/ti/davinci_cpdma.c: In function 'cpdma_desc_pool_create':
ethernet/ti/davinci_cpdma.c:182:48: error: passing argument 3 of 'dma_alloc_coherent' from incompatible pointer type [-Werror=incompatible-pointer-types]
   pool->cpumap = dma_alloc_coherent(dev, size, &pool->phys,
In file included from ethernet/ti/davinci_cpdma.c:21:0:
dma-mapping.h:398:21: note: expected 'dma_addr_t * {aka long long unsigned int *}' but argument is of type 'phys_addr_t * {aka unsigned int *}'
 static inline void *dma_alloc_coherent(struct device *dev, size_t size,

This slightly restructures the code so the address we use for
mapping RAM into a DMA address is always a dma_addr_t, avoiding
the warning. The code is correct even if both types are 32-bit
because the DMA master in this device only supports 32-bit addressing
anyway, independent of the types that are used.

We still assign this value to pool->phys, and that is wrong if
the driver is ever used with an IOMMU, but that value appears to
be never used, so there is no problem really. I've added a couple
of comments about where we do things that are slightly violating
the API.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/ethernet/ti/davinci_cpdma.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/ti/davinci_cpdma.c b/drivers/net/ethernet/ti/davinci_cpdma.c
index 657b65bf5cac..18bf3a8fdc50 100644
--- a/drivers/net/ethernet/ti/davinci_cpdma.c
+++ b/drivers/net/ethernet/ti/davinci_cpdma.c
@@ -82,7 +82,7 @@ struct cpdma_desc {
 
 struct cpdma_desc_pool {
 	phys_addr_t		phys;
-	u32			hw_addr;
+	dma_addr_t		hw_addr;
 	void __iomem		*iomap;		/* ioremap map */
 	void			*cpumap;	/* dma_alloc map */
 	int			desc_size, mem_size;
@@ -152,7 +152,7 @@ struct cpdma_chan {
  * abstract out these details
  */
 static struct cpdma_desc_pool *
-cpdma_desc_pool_create(struct device *dev, u32 phys, u32 hw_addr,
+cpdma_desc_pool_create(struct device *dev, u32 phys, dma_addr_t hw_addr,
 				int size, int align)
 {
 	int bitmap_size;
@@ -176,13 +176,13 @@ cpdma_desc_pool_create(struct device *dev, u32 phys, u32 hw_addr,
 
 	if (phys) {
 		pool->phys  = phys;
-		pool->iomap = ioremap(phys, size);
+		pool->iomap = ioremap(phys, size); /* should be memremap? */
 		pool->hw_addr = hw_addr;
 	} else {
-		pool->cpumap = dma_alloc_coherent(dev, size, &pool->phys,
+		pool->cpumap = dma_alloc_coherent(dev, size, &pool->hw_addr,
 						  GFP_KERNEL);
-		pool->iomap = pool->cpumap;
-		pool->hw_addr = pool->phys;
+		pool->iomap = (void __iomem __force *)pool->cpumap;
+		pool->phys = pool->hw_addr; /* assumes no IOMMU, don't use this value */
 	}
 
 	if (pool->iomap)
-- 
2.7.0

[PATCH 1/9] net: davinci_cpdma: use dma_addr_t for DMA address

[Arnd Bergmann]

The davinci_cpdma mixes up physical addresses as seen from the CPU
and DMA addresses as seen from a DMA master, since it can operate
on both normal memory or an on-chip buffer. If dma_addr_t is
different from phys_addr_t, this means we get a compile-time warning
about the type mismatch:

ethernet/ti/davinci_cpdma.c: In function 'cpdma_desc_pool_create':
ethernet/ti/davinci_cpdma.c:182:48: error: passing argument 3 of 'dma_alloc_coherent' from incompatible pointer type [-Werror=incompatible-pointer-types]
   pool->cpumap = dma_alloc_coherent(dev, size, &pool->phys,
In file included from ethernet/ti/davinci_cpdma.c:21:0:
dma-mapping.h:398:21: note: expected 'dma_addr_t * {aka long long unsigned int *}' but argument is of type 'phys_addr_t * {aka unsigned int *}'
 static inline void *dma_alloc_coherent(struct device *dev, size_t size,

This slightly restructures the code so the address we use for
mapping RAM into a DMA address is always a dma_addr_t, avoiding
the warning. The code is correct even if both types are 32-bit
because the DMA master in this device only supports 32-bit addressing
anyway, independent of the types that are used.

We still assign this value to pool->phys, and that is wrong if
the driver is ever used with an IOMMU, but that value appears to
be never used, so there is no problem really. I've added a couple
of comments about where we do things that are slightly violating
the API.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/ethernet/ti/davinci_cpdma.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/ti/davinci_cpdma.c b/drivers/net/ethernet/ti/davinci_cpdma.c
index 657b65bf5cac..18bf3a8fdc50 100644
--- a/drivers/net/ethernet/ti/davinci_cpdma.c
+++ b/drivers/net/ethernet/ti/davinci_cpdma.c
@@ -82,7 +82,7 @@ struct cpdma_desc {
 
 struct cpdma_desc_pool {
 	phys_addr_t		phys;
-	u32			hw_addr;
+	dma_addr_t		hw_addr;
 	void __iomem		*iomap;		/* ioremap map */
 	void			*cpumap;	/* dma_alloc map */
 	int			desc_size, mem_size;
@@ -152,7 +152,7 @@ struct cpdma_chan {
  * abstract out these details
  */
 static struct cpdma_desc_pool *
-cpdma_desc_pool_create(struct device *dev, u32 phys, u32 hw_addr,
+cpdma_desc_pool_create(struct device *dev, u32 phys, dma_addr_t hw_addr,
 				int size, int align)
 {
 	int bitmap_size;
@@ -176,13 +176,13 @@ cpdma_desc_pool_create(struct device *dev, u32 phys, u32 hw_addr,
 
 	if (phys) {
 		pool->phys  = phys;
-		pool->iomap = ioremap(phys, size);
+		pool->iomap = ioremap(phys, size); /* should be memremap? */
 		pool->hw_addr = hw_addr;
 	} else {
-		pool->cpumap = dma_alloc_coherent(dev, size, &pool->phys,
+		pool->cpumap = dma_alloc_coherent(dev, size, &pool->hw_addr,
 						  GFP_KERNEL);
-		pool->iomap = pool->cpumap;
-		pool->hw_addr = pool->phys;
+		pool->iomap = (void __iomem __force *)pool->cpumap;
+		pool->phys = pool->hw_addr; /* assumes no IOMMU, don't use this value */
 	}
 
 	if (pool->iomap)
-- 
2.7.0

[PATCH 2/9] net: hp100: remove unnecessary #ifdefs

[Arnd Bergmann]

Building the hp100 ethernet driver causes warnings when both the PCI
and EISA drivers are disabled:

ethernet/hp/hp100.c: In function 'hp100_module_init':
ethernet/hp/hp100.c:3047:2: warning: label 'out3' defined but not used [-Wunused-label]
ethernet/hp/hp100.c: At top level:
ethernet/hp/hp100.c:2828:13: warning: 'cleanup_dev' defined but not used [-Wunused-function]

We can easily avoid the warnings and make the driver look slightly
nicer by removing the #ifdefs that check for the CONFIG_PCI and
CONFIG_EISA, as all the registration functions are designed to
have no effect when the buses are disabled.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/ethernet/hp/hp100.c | 18 ------------------
 1 file changed, 18 deletions(-)

diff --git a/drivers/net/ethernet/hp/hp100.c b/drivers/net/ethernet/hp/hp100.c
index 1d5c3e16d8f4..3daf2d4a7ca0 100644
--- a/drivers/net/ethernet/hp/hp100.c
+++ b/drivers/net/ethernet/hp/hp100.c
@@ -194,7 +194,6 @@ static const char *hp100_isa_tbl[] = {
 };
 #endif
 
-#ifdef CONFIG_EISA
 static struct eisa_device_id hp100_eisa_tbl[] = {
 	{ "HWPF180" }, /* HP J2577 rev A */
 	{ "HWP1920" }, /* HP 27248B */
@@ -205,9 +204,7 @@ static struct eisa_device_id hp100_eisa_tbl[] = {
 	{ "" }	       /* Mandatory final entry ! */
 };
 MODULE_DEVICE_TABLE(eisa, hp100_eisa_tbl);
-#endif
 
-#ifdef CONFIG_PCI
 static const struct pci_device_id hp100_pci_tbl[] = {
 	{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_J2585A, PCI_ANY_ID, PCI_ANY_ID,},
 	{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_J2585B, PCI_ANY_ID, PCI_ANY_ID,},
@@ -219,7 +216,6 @@ static const struct pci_device_id hp100_pci_tbl[] = {
 	{}			/* Terminating entry */
 };
 MODULE_DEVICE_TABLE(pci, hp100_pci_tbl);
-#endif
 
 static int hp100_rx_ratio = HP100_DEFAULT_RX_RATIO;
 static int hp100_priority_tx = HP100_DEFAULT_PRIORITY_TX;
@@ -2842,7 +2838,6 @@ static void cleanup_dev(struct net_device *d)
 	free_netdev(d);
 }
 
-#ifdef CONFIG_EISA
 static int hp100_eisa_probe(struct device *gendev)
 {
 	struct net_device *dev = alloc_etherdev(sizeof(struct hp100_private));
@@ -2884,9 +2879,7 @@ static struct eisa_driver hp100_eisa_driver = {
 		.remove  = hp100_eisa_remove,
         }
 };
-#endif
 
-#ifdef CONFIG_PCI
 static int hp100_pci_probe(struct pci_dev *pdev,
 			   const struct pci_device_id *ent)
 {
@@ -2955,7 +2948,6 @@ static struct pci_driver hp100_pci_driver = {
 	.probe		= hp100_pci_probe,
 	.remove		= hp100_pci_remove,
 };
-#endif
 
 /*
  *  module section
@@ -3032,23 +3024,17 @@ static int __init hp100_module_init(void)
 	err = hp100_isa_init();
 	if (err && err != -ENODEV)
 		goto out;
-#ifdef CONFIG_EISA
 	err = eisa_driver_register(&hp100_eisa_driver);
 	if (err && err != -ENODEV)
 		goto out2;
-#endif
-#ifdef CONFIG_PCI
 	err = pci_register_driver(&hp100_pci_driver);
 	if (err && err != -ENODEV)
 		goto out3;
-#endif
  out:
 	return err;
  out3:
-#ifdef CONFIG_EISA
 	eisa_driver_unregister (&hp100_eisa_driver);
  out2:
-#endif
 	hp100_isa_cleanup();
 	goto out;
 }
@@ -3057,12 +3043,8 @@ static int __init hp100_module_init(void)
 static void __exit hp100_module_exit(void)
 {
 	hp100_isa_cleanup();
-#ifdef CONFIG_EISA
 	eisa_driver_unregister (&hp100_eisa_driver);
-#endif
-#ifdef CONFIG_PCI
 	pci_unregister_driver (&hp100_pci_driver);
-#endif
 }
 
 module_init(hp100_module_init)
-- 
2.7.0

[PATCH 2/9] net: hp100: remove unnecessary #ifdefs

[Arnd Bergmann]

Building the hp100 ethernet driver causes warnings when both the PCI
and EISA drivers are disabled:

ethernet/hp/hp100.c: In function 'hp100_module_init':
ethernet/hp/hp100.c:3047:2: warning: label 'out3' defined but not used [-Wunused-label]
ethernet/hp/hp100.c: At top level:
ethernet/hp/hp100.c:2828:13: warning: 'cleanup_dev' defined but not used [-Wunused-function]

We can easily avoid the warnings and make the driver look slightly
nicer by removing the #ifdefs that check for the CONFIG_PCI and
CONFIG_EISA, as all the registration functions are designed to
have no effect when the buses are disabled.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/ethernet/hp/hp100.c | 18 ------------------
 1 file changed, 18 deletions(-)

diff --git a/drivers/net/ethernet/hp/hp100.c b/drivers/net/ethernet/hp/hp100.c
index 1d5c3e16d8f4..3daf2d4a7ca0 100644
--- a/drivers/net/ethernet/hp/hp100.c
+++ b/drivers/net/ethernet/hp/hp100.c
@@ -194,7 +194,6 @@ static const char *hp100_isa_tbl[] = {
 };
 #endif
 
-#ifdef CONFIG_EISA
 static struct eisa_device_id hp100_eisa_tbl[] = {
 	{ "HWPF180" }, /* HP J2577 rev A */
 	{ "HWP1920" }, /* HP 27248B */
@@ -205,9 +204,7 @@ static struct eisa_device_id hp100_eisa_tbl[] = {
 	{ "" }	       /* Mandatory final entry ! */
 };
 MODULE_DEVICE_TABLE(eisa, hp100_eisa_tbl);
-#endif
 
-#ifdef CONFIG_PCI
 static const struct pci_device_id hp100_pci_tbl[] = {
 	{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_J2585A, PCI_ANY_ID, PCI_ANY_ID,},
 	{PCI_VENDOR_ID_HP, PCI_DEVICE_ID_HP_J2585B, PCI_ANY_ID, PCI_ANY_ID,},
@@ -219,7 +216,6 @@ static const struct pci_device_id hp100_pci_tbl[] = {
 	{}			/* Terminating entry */
 };
 MODULE_DEVICE_TABLE(pci, hp100_pci_tbl);
-#endif
 
 static int hp100_rx_ratio = HP100_DEFAULT_RX_RATIO;
 static int hp100_priority_tx = HP100_DEFAULT_PRIORITY_TX;
@@ -2842,7 +2838,6 @@ static void cleanup_dev(struct net_device *d)
 	free_netdev(d);
 }
 
-#ifdef CONFIG_EISA
 static int hp100_eisa_probe(struct device *gendev)
 {
 	struct net_device *dev = alloc_etherdev(sizeof(struct hp100_private));
@@ -2884,9 +2879,7 @@ static struct eisa_driver hp100_eisa_driver = {
 		.remove  = hp100_eisa_remove,
         }
 };
-#endif
 
-#ifdef CONFIG_PCI
 static int hp100_pci_probe(struct pci_dev *pdev,
 			   const struct pci_device_id *ent)
 {
@@ -2955,7 +2948,6 @@ static struct pci_driver hp100_pci_driver = {
 	.probe		= hp100_pci_probe,
 	.remove		= hp100_pci_remove,
 };
-#endif
 
 /*
  *  module section
@@ -3032,23 +3024,17 @@ static int __init hp100_module_init(void)
 	err = hp100_isa_init();
 	if (err && err != -ENODEV)
 		goto out;
-#ifdef CONFIG_EISA
 	err = eisa_driver_register(&hp100_eisa_driver);
 	if (err && err != -ENODEV)
 		goto out2;
-#endif
-#ifdef CONFIG_PCI
 	err = pci_register_driver(&hp100_pci_driver);
 	if (err && err != -ENODEV)
 		goto out3;
-#endif
  out:
 	return err;
  out3:
-#ifdef CONFIG_EISA
 	eisa_driver_unregister (&hp100_eisa_driver);
  out2:
-#endif
 	hp100_isa_cleanup();
 	goto out;
 }
@@ -3057,12 +3043,8 @@ static int __init hp100_module_init(void)
 static void __exit hp100_module_exit(void)
 {
 	hp100_isa_cleanup();
-#ifdef CONFIG_EISA
 	eisa_driver_unregister (&hp100_eisa_driver);
-#endif
-#ifdef CONFIG_PCI
 	pci_unregister_driver (&hp100_pci_driver);
-#endif
 }
 
 module_init(hp100_module_init)
-- 
2.7.0

[PATCH 3/9] net: bgmac: clarify CONFIG_BCMA dependency

[Arnd Bergmann]

The bgmac driver depends on BCMA_HOST_SOC, which is only used
when CONFIG_BCMA is enabled. However, it is a bool option and can
be set when CONFIG_BCMA=m, and then bgmac can be built-in, leading
to an obvious link error:

drivers/built-in.o: In function `bgmac_init':
:(.init.text+0x7f2c): undefined reference to `__bcma_driver_register'
drivers/built-in.o: In function `bgmac_exit':
:(.exit.text+0x110a): undefined reference to `bcma_driver_unregister'

To avoid this case, we need to depend on both BCMA and BCMA_SOC,
as this patch does. I'm also trying to make the dependency more
readable by splitting it into three lines, and adding a COMPILE_TEST
alternative so we can test-build it in all configurations that
support BCMA.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/ethernet/broadcom/Kconfig | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/broadcom/Kconfig b/drivers/net/ethernet/broadcom/Kconfig
index 8550df189ceb..19f7cd02e085 100644
--- a/drivers/net/ethernet/broadcom/Kconfig
+++ b/drivers/net/ethernet/broadcom/Kconfig
@@ -151,8 +151,11 @@ config BNX2X_VXLAN
 
 config BGMAC
 	tristate "BCMA bus GBit core support"
-	depends on BCMA_HOST_SOC && HAS_DMA && (BCM47XX || ARCH_BCM_5301X)
+	depends on BCMA && BCMA_HOST_SOC
+	depends on HAS_DMA
+	depends on BCM47XX || ARCH_BCM_5301X || COMPILE_TEST
 	select PHYLIB
+	select FIXED_PHY
 	---help---
 	  This driver supports GBit MAC and BCM4706 GBit MAC cores on BCMA bus.
 	  They can be found on BCM47xx SoCs and provide gigabit ethernet.
-- 
2.7.0

[PATCH 3/9] net: bgmac: clarify CONFIG_BCMA dependency

[Arnd Bergmann]

The bgmac driver depends on BCMA_HOST_SOC, which is only used
when CONFIG_BCMA is enabled. However, it is a bool option and can
be set when CONFIG_BCMA=m, and then bgmac can be built-in, leading
to an obvious link error:

drivers/built-in.o: In function `bgmac_init':
:(.init.text+0x7f2c): undefined reference to `__bcma_driver_register'
drivers/built-in.o: In function `bgmac_exit':
:(.exit.text+0x110a): undefined reference to `bcma_driver_unregister'

To avoid this case, we need to depend on both BCMA and BCMA_SOC,
as this patch does. I'm also trying to make the dependency more
readable by splitting it into three lines, and adding a COMPILE_TEST
alternative so we can test-build it in all configurations that
support BCMA.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/ethernet/broadcom/Kconfig | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/broadcom/Kconfig b/drivers/net/ethernet/broadcom/Kconfig
index 8550df189ceb..19f7cd02e085 100644
--- a/drivers/net/ethernet/broadcom/Kconfig
+++ b/drivers/net/ethernet/broadcom/Kconfig
@@ -151,8 +151,11 @@ config BNX2X_VXLAN
 
 config BGMAC
 	tristate "BCMA bus GBit core support"
-	depends on BCMA_HOST_SOC && HAS_DMA && (BCM47XX || ARCH_BCM_5301X)
+	depends on BCMA && BCMA_HOST_SOC
+	depends on HAS_DMA
+	depends on BCM47XX || ARCH_BCM_5301X || COMPILE_TEST
 	select PHYLIB
+	select FIXED_PHY
 	---help---
 	  This driver supports GBit MAC and BCM4706 GBit MAC cores on BCMA bus.
 	  They can be found on BCM47xx SoCs and provide gigabit ethernet.
-- 
2.7.0

[PATCH 4/9] net: moxart: use correct accessors for DMA memory

[Arnd Bergmann]

The moxart ethernet driver confuses coherent DMA buffers with
MMIO registers.

moxart_ether.c: In function 'moxart_mac_setup_desc_ring':
moxart_ether.c:146:428: error: passing argument 1 of '__fswab32' makes integer from pointer without a cast [-Werror=int-conversion]
moxart_ether.c:74:39: warning: incorrect type in argument 3 (different address spaces)
moxart_ether.c:74:39:    expected void *cpu_addr
moxart_ether.c:74:39:    got void [noderef] <asn:2>*tx_desc_base

This leaves the basic logic alone and uses normal pointers for
the virtual address of the descriptor. As we cannot use readl/writel
to access them, we also introduce our own moxart_desc_read
moxart_desc_write helpers that perform the same endianess swap
as the original code, but without the extra barriers and address
space conversion.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/ethernet/moxa/moxart_ether.c | 42 ++++++++++++++++++++------------
 drivers/net/ethernet/moxa/moxart_ether.h |  4 +--
 2 files changed, 28 insertions(+), 18 deletions(-)

diff --git a/drivers/net/ethernet/moxa/moxart_ether.c b/drivers/net/ethernet/moxa/moxart_ether.c
index a10c928bbd6b..b2f1c0db9445 100644
--- a/drivers/net/ethernet/moxa/moxart_ether.c
+++ b/drivers/net/ethernet/moxa/moxart_ether.c
@@ -28,6 +28,16 @@
 
 #include "moxart_ether.h"
 
+static inline void moxart_desc_write(u32 data, u32 *desc)
+{
+	*desc = cpu_to_le32(data);
+}
+
+static inline u32 moxart_desc_read(u32 *desc)
+{
+	return le32_to_cpu(*desc);
+}
+
 static inline void moxart_emac_write(struct net_device *ndev,
 				     unsigned int reg, unsigned long value)
 {
@@ -112,7 +122,7 @@ static void moxart_mac_enable(struct net_device *ndev)
 static void moxart_mac_setup_desc_ring(struct net_device *ndev)
 {
 	struct moxart_mac_priv_t *priv = netdev_priv(ndev);
-	void __iomem *desc;
+	void *desc;
 	int i;
 
 	for (i = 0; i < TX_DESC_NUM; i++) {
@@ -121,7 +131,7 @@ static void moxart_mac_setup_desc_ring(struct net_device *ndev)
 
 		priv->tx_buf[i] = priv->tx_buf_base + priv->tx_buf_size * i;
 	}
-	writel(TX_DESC1_END, desc + TX_REG_OFFSET_DESC1);
+	moxart_desc_write(TX_DESC1_END, desc + TX_REG_OFFSET_DESC1);
 
 	priv->tx_head = 0;
 	priv->tx_tail = 0;
@@ -129,8 +139,8 @@ static void moxart_mac_setup_desc_ring(struct net_device *ndev)
 	for (i = 0; i < RX_DESC_NUM; i++) {
 		desc = priv->rx_desc_base + i * RX_REG_DESC_SIZE;
 		memset(desc, 0, RX_REG_DESC_SIZE);
-		writel(RX_DESC0_DMA_OWN, desc + RX_REG_OFFSET_DESC0);
-		writel(RX_BUF_SIZE & RX_DESC1_BUF_SIZE_MASK,
+		moxart_desc_write(RX_DESC0_DMA_OWN, desc + RX_REG_OFFSET_DESC0);
+		moxart_desc_write(RX_BUF_SIZE & RX_DESC1_BUF_SIZE_MASK,
 		       desc + RX_REG_OFFSET_DESC1);
 
 		priv->rx_buf[i] = priv->rx_buf_base + priv->rx_buf_size * i;
@@ -141,12 +151,12 @@ static void moxart_mac_setup_desc_ring(struct net_device *ndev)
 		if (dma_mapping_error(&ndev->dev, priv->rx_mapping[i]))
 			netdev_err(ndev, "DMA mapping error\n");
 
-		writel(priv->rx_mapping[i],
+		moxart_desc_write(priv->rx_mapping[i],
 		       desc + RX_REG_OFFSET_DESC2 + RX_DESC2_ADDRESS_PHYS);
-		writel(priv->rx_buf[i],
+		moxart_desc_write((uintptr_t)priv->rx_buf[i],
 		       desc + RX_REG_OFFSET_DESC2 + RX_DESC2_ADDRESS_VIRT);
 	}
-	writel(RX_DESC1_END, desc + RX_REG_OFFSET_DESC1);
+	moxart_desc_write(RX_DESC1_END, desc + RX_REG_OFFSET_DESC1);
 
 	priv->rx_head = 0;
 
@@ -201,14 +211,14 @@ static int moxart_rx_poll(struct napi_struct *napi, int budget)
 						      napi);
 	struct net_device *ndev = priv->ndev;
 	struct sk_buff *skb;
-	void __iomem *desc;
+	void *desc;
 	unsigned int desc0, len;
 	int rx_head = priv->rx_head;
 	int rx = 0;
 
 	while (rx < budget) {
 		desc = priv->rx_desc_base + (RX_REG_DESC_SIZE * rx_head);
-		desc0 = readl(desc + RX_REG_OFFSET_DESC0);
+		desc0 = moxart_desc_read(desc + RX_REG_OFFSET_DESC0);
 
 		if (desc0 & RX_DESC0_DMA_OWN)
 			break;
@@ -250,7 +260,7 @@ static int moxart_rx_poll(struct napi_struct *napi, int budget)
 			priv->stats.multicast++;
 
 rx_next:
-		writel(RX_DESC0_DMA_OWN, desc + RX_REG_OFFSET_DESC0);
+		moxart_desc_write(RX_DESC0_DMA_OWN, desc + RX_REG_OFFSET_DESC0);
 
 		rx_head = RX_NEXT(rx_head);
 		priv->rx_head = rx_head;
@@ -310,7 +320,7 @@ static irqreturn_t moxart_mac_interrupt(int irq, void *dev_id)
 static int moxart_mac_start_xmit(struct sk_buff *skb, struct net_device *ndev)
 {
 	struct moxart_mac_priv_t *priv = netdev_priv(ndev);
-	void __iomem *desc;
+	void *desc;
 	unsigned int len;
 	unsigned int tx_head = priv->tx_head;
 	u32 txdes1;
@@ -319,7 +329,7 @@ static int moxart_mac_start_xmit(struct sk_buff *skb, struct net_device *ndev)
 	desc = priv->tx_desc_base + (TX_REG_DESC_SIZE * tx_head);
 
 	spin_lock_irq(&priv->txlock);
-	if (readl(desc + TX_REG_OFFSET_DESC0) & TX_DESC0_DMA_OWN) {
+	if (moxart_desc_read(desc + TX_REG_OFFSET_DESC0) & TX_DESC0_DMA_OWN) {
 		net_dbg_ratelimited("no TX space for packet\n");
 		priv->stats.tx_dropped++;
 		goto out_unlock;
@@ -337,9 +347,9 @@ static int moxart_mac_start_xmit(struct sk_buff *skb, struct net_device *ndev)
 	priv->tx_len[tx_head] = len;
 	priv->tx_skb[tx_head] = skb;
 
-	writel(priv->tx_mapping[tx_head],
+	moxart_desc_write(priv->tx_mapping[tx_head],
 	       desc + TX_REG_OFFSET_DESC2 + TX_DESC2_ADDRESS_PHYS);
-	writel(skb->data,
+	moxart_desc_write((uintptr_t)skb->data,
 	       desc + TX_REG_OFFSET_DESC2 + TX_DESC2_ADDRESS_VIRT);
 
 	if (skb->len < ETH_ZLEN) {
@@ -354,8 +364,8 @@ static int moxart_mac_start_xmit(struct sk_buff *skb, struct net_device *ndev)
 	txdes1 = TX_DESC1_LTS | TX_DESC1_FTS | (len & TX_DESC1_BUF_SIZE_MASK);
 	if (tx_head == TX_DESC_NUM_MASK)
 		txdes1 |= TX_DESC1_END;
-	writel(txdes1, desc + TX_REG_OFFSET_DESC1);
-	writel(TX_DESC0_DMA_OWN, desc + TX_REG_OFFSET_DESC0);
+	moxart_desc_write(txdes1, desc + TX_REG_OFFSET_DESC1);
+	moxart_desc_write(TX_DESC0_DMA_OWN, desc + TX_REG_OFFSET_DESC0);
 
 	/* start to send packet */
 	writel(0xffffffff, priv->base + REG_TX_POLL_DEMAND);
diff --git a/drivers/net/ethernet/moxa/moxart_ether.h b/drivers/net/ethernet/moxa/moxart_ether.h
index 2be9280d608c..93a9563ac7c6 100644
--- a/drivers/net/ethernet/moxa/moxart_ether.h
+++ b/drivers/net/ethernet/moxa/moxart_ether.h
@@ -300,7 +300,7 @@ struct moxart_mac_priv_t {
 
 	dma_addr_t rx_base;
 	dma_addr_t rx_mapping[RX_DESC_NUM];
-	void __iomem *rx_desc_base;
+	void *rx_desc_base;
 	unsigned char *rx_buf_base;
 	unsigned char *rx_buf[RX_DESC_NUM];
 	unsigned int rx_head;
@@ -308,7 +308,7 @@ struct moxart_mac_priv_t {
 
 	dma_addr_t tx_base;
 	dma_addr_t tx_mapping[TX_DESC_NUM];
-	void __iomem *tx_desc_base;
+	void *tx_desc_base;
 	unsigned char *tx_buf_base;
 	unsigned char *tx_buf[RX_DESC_NUM];
 	unsigned int tx_head;
-- 
2.7.0

[PATCH 4/9] net: moxart: use correct accessors for DMA memory

[Arnd Bergmann]

The moxart ethernet driver confuses coherent DMA buffers with
MMIO registers.

moxart_ether.c: In function 'moxart_mac_setup_desc_ring':
moxart_ether.c:146:428: error: passing argument 1 of '__fswab32' makes integer from pointer without a cast [-Werror=int-conversion]
moxart_ether.c:74:39: warning: incorrect type in argument 3 (different address spaces)
moxart_ether.c:74:39:    expected void *cpu_addr
moxart_ether.c:74:39:    got void [noderef] <asn:2>*tx_desc_base

This leaves the basic logic alone and uses normal pointers for
the virtual address of the descriptor. As we cannot use readl/writel
to access them, we also introduce our own moxart_desc_read
moxart_desc_write helpers that perform the same endianess swap
as the original code, but without the extra barriers and address
space conversion.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/ethernet/moxa/moxart_ether.c | 42 ++++++++++++++++++++------------
 drivers/net/ethernet/moxa/moxart_ether.h |  4 +--
 2 files changed, 28 insertions(+), 18 deletions(-)

diff --git a/drivers/net/ethernet/moxa/moxart_ether.c b/drivers/net/ethernet/moxa/moxart_ether.c
index a10c928bbd6b..b2f1c0db9445 100644
--- a/drivers/net/ethernet/moxa/moxart_ether.c
+++ b/drivers/net/ethernet/moxa/moxart_ether.c
@@ -28,6 +28,16 @@
 
 #include "moxart_ether.h"
 
+static inline void moxart_desc_write(u32 data, u32 *desc)
+{
+	*desc = cpu_to_le32(data);
+}
+
+static inline u32 moxart_desc_read(u32 *desc)
+{
+	return le32_to_cpu(*desc);
+}
+
 static inline void moxart_emac_write(struct net_device *ndev,
 				     unsigned int reg, unsigned long value)
 {
@@ -112,7 +122,7 @@ static void moxart_mac_enable(struct net_device *ndev)
 static void moxart_mac_setup_desc_ring(struct net_device *ndev)
 {
 	struct moxart_mac_priv_t *priv = netdev_priv(ndev);
-	void __iomem *desc;
+	void *desc;
 	int i;
 
 	for (i = 0; i < TX_DESC_NUM; i++) {
@@ -121,7 +131,7 @@ static void moxart_mac_setup_desc_ring(struct net_device *ndev)
 
 		priv->tx_buf[i] = priv->tx_buf_base + priv->tx_buf_size * i;
 	}
-	writel(TX_DESC1_END, desc + TX_REG_OFFSET_DESC1);
+	moxart_desc_write(TX_DESC1_END, desc + TX_REG_OFFSET_DESC1);
 
 	priv->tx_head = 0;
 	priv->tx_tail = 0;
@@ -129,8 +139,8 @@ static void moxart_mac_setup_desc_ring(struct net_device *ndev)
 	for (i = 0; i < RX_DESC_NUM; i++) {
 		desc = priv->rx_desc_base + i * RX_REG_DESC_SIZE;
 		memset(desc, 0, RX_REG_DESC_SIZE);
-		writel(RX_DESC0_DMA_OWN, desc + RX_REG_OFFSET_DESC0);
-		writel(RX_BUF_SIZE & RX_DESC1_BUF_SIZE_MASK,
+		moxart_desc_write(RX_DESC0_DMA_OWN, desc + RX_REG_OFFSET_DESC0);
+		moxart_desc_write(RX_BUF_SIZE & RX_DESC1_BUF_SIZE_MASK,
 		       desc + RX_REG_OFFSET_DESC1);
 
 		priv->rx_buf[i] = priv->rx_buf_base + priv->rx_buf_size * i;
@@ -141,12 +151,12 @@ static void moxart_mac_setup_desc_ring(struct net_device *ndev)
 		if (dma_mapping_error(&ndev->dev, priv->rx_mapping[i]))
 			netdev_err(ndev, "DMA mapping error\n");
 
-		writel(priv->rx_mapping[i],
+		moxart_desc_write(priv->rx_mapping[i],
 		       desc + RX_REG_OFFSET_DESC2 + RX_DESC2_ADDRESS_PHYS);
-		writel(priv->rx_buf[i],
+		moxart_desc_write((uintptr_t)priv->rx_buf[i],
 		       desc + RX_REG_OFFSET_DESC2 + RX_DESC2_ADDRESS_VIRT);
 	}
-	writel(RX_DESC1_END, desc + RX_REG_OFFSET_DESC1);
+	moxart_desc_write(RX_DESC1_END, desc + RX_REG_OFFSET_DESC1);
 
 	priv->rx_head = 0;
 
@@ -201,14 +211,14 @@ static int moxart_rx_poll(struct napi_struct *napi, int budget)
 						      napi);
 	struct net_device *ndev = priv->ndev;
 	struct sk_buff *skb;
-	void __iomem *desc;
+	void *desc;
 	unsigned int desc0, len;
 	int rx_head = priv->rx_head;
 	int rx = 0;
 
 	while (rx < budget) {
 		desc = priv->rx_desc_base + (RX_REG_DESC_SIZE * rx_head);
-		desc0 = readl(desc + RX_REG_OFFSET_DESC0);
+		desc0 = moxart_desc_read(desc + RX_REG_OFFSET_DESC0);
 
 		if (desc0 & RX_DESC0_DMA_OWN)
 			break;
@@ -250,7 +260,7 @@ static int moxart_rx_poll(struct napi_struct *napi, int budget)
 			priv->stats.multicast++;
 
 rx_next:
-		writel(RX_DESC0_DMA_OWN, desc + RX_REG_OFFSET_DESC0);
+		moxart_desc_write(RX_DESC0_DMA_OWN, desc + RX_REG_OFFSET_DESC0);
 
 		rx_head = RX_NEXT(rx_head);
 		priv->rx_head = rx_head;
@@ -310,7 +320,7 @@ static irqreturn_t moxart_mac_interrupt(int irq, void *dev_id)
 static int moxart_mac_start_xmit(struct sk_buff *skb, struct net_device *ndev)
 {
 	struct moxart_mac_priv_t *priv = netdev_priv(ndev);
-	void __iomem *desc;
+	void *desc;
 	unsigned int len;
 	unsigned int tx_head = priv->tx_head;
 	u32 txdes1;
@@ -319,7 +329,7 @@ static int moxart_mac_start_xmit(struct sk_buff *skb, struct net_device *ndev)
 	desc = priv->tx_desc_base + (TX_REG_DESC_SIZE * tx_head);
 
 	spin_lock_irq(&priv->txlock);
-	if (readl(desc + TX_REG_OFFSET_DESC0) & TX_DESC0_DMA_OWN) {
+	if (moxart_desc_read(desc + TX_REG_OFFSET_DESC0) & TX_DESC0_DMA_OWN) {
 		net_dbg_ratelimited("no TX space for packet\n");
 		priv->stats.tx_dropped++;
 		goto out_unlock;
@@ -337,9 +347,9 @@ static int moxart_mac_start_xmit(struct sk_buff *skb, struct net_device *ndev)
 	priv->tx_len[tx_head] = len;
 	priv->tx_skb[tx_head] = skb;
 
-	writel(priv->tx_mapping[tx_head],
+	moxart_desc_write(priv->tx_mapping[tx_head],
 	       desc + TX_REG_OFFSET_DESC2 + TX_DESC2_ADDRESS_PHYS);
-	writel(skb->data,
+	moxart_desc_write((uintptr_t)skb->data,
 	       desc + TX_REG_OFFSET_DESC2 + TX_DESC2_ADDRESS_VIRT);
 
 	if (skb->len < ETH_ZLEN) {
@@ -354,8 +364,8 @@ static int moxart_mac_start_xmit(struct sk_buff *skb, struct net_device *ndev)
 	txdes1 = TX_DESC1_LTS | TX_DESC1_FTS | (len & TX_DESC1_BUF_SIZE_MASK);
 	if (tx_head == TX_DESC_NUM_MASK)
 		txdes1 |= TX_DESC1_END;
-	writel(txdes1, desc + TX_REG_OFFSET_DESC1);
-	writel(TX_DESC0_DMA_OWN, desc + TX_REG_OFFSET_DESC0);
+	moxart_desc_write(txdes1, desc + TX_REG_OFFSET_DESC1);
+	moxart_desc_write(TX_DESC0_DMA_OWN, desc + TX_REG_OFFSET_DESC0);
 
 	/* start to send packet */
 	writel(0xffffffff, priv->base + REG_TX_POLL_DEMAND);
diff --git a/drivers/net/ethernet/moxa/moxart_ether.h b/drivers/net/ethernet/moxa/moxart_ether.h
index 2be9280d608c..93a9563ac7c6 100644
--- a/drivers/net/ethernet/moxa/moxart_ether.h
+++ b/drivers/net/ethernet/moxa/moxart_ether.h
@@ -300,7 +300,7 @@ struct moxart_mac_priv_t {
 
 	dma_addr_t rx_base;
 	dma_addr_t rx_mapping[RX_DESC_NUM];
-	void __iomem *rx_desc_base;
+	void *rx_desc_base;
 	unsigned char *rx_buf_base;
 	unsigned char *rx_buf[RX_DESC_NUM];
 	unsigned int rx_head;
@@ -308,7 +308,7 @@ struct moxart_mac_priv_t {
 
 	dma_addr_t tx_base;
 	dma_addr_t tx_mapping[TX_DESC_NUM];
-	void __iomem *tx_desc_base;
+	void *tx_desc_base;
 	unsigned char *tx_buf_base;
 	unsigned char *tx_buf[RX_DESC_NUM];
 	unsigned int tx_head;
-- 
2.7.0

[PATCH 5/9] net: fddi/defxx: avoid warning about uninitialized variable use

[Arnd Bergmann]

The defxx driver can be configured for different kinds of buses,
and appears to be handling this correctly, but the compiler cannot
see how it always initializes the bar_start and bar_length
fields it uses depending on the configured bus, so we get a warning
with recent gcc versions:

fddi/defxx.c: In function 'dfx_pci_unregister':
fddi/defxx.c:3726:3: warning: 'bar_len' may be used uninitialized in this function [-Wmaybe-uninitialized]
   release_mem_region(bar_start[0], bar_len[0]);
fddi/defxx.c:3701:18: note: 'bar_len' was declared here
  resource_size_t bar_len[3];  /* resource lengths */
fddi/defxx.c:3726:3: warning: 'bar_start' may be used uninitialized in this function [-Wmaybe-uninitialized]
   release_mem_region(bar_start[0], bar_len[0]);
fddi/defxx.c:3700:18: note: 'bar_start' was declared here
  resource_size_t bar_start[3];  /* pointers to ports */
                  ^
fddi/defxx.c: In function 'dfx_pci_register':
fddi/defxx.c:617:18: warning: 'bar_len' may be used uninitialized in this function [-Wmaybe-uninitialized]
   bp->base.mem = ioremap_nocache(bar_start[0], bar_len[0]);
fddi/defxx.c:537:18: note: 'bar_len' was declared here
  resource_size_t bar_len[3];  /* resource length */
fddi/defxx.c:1125:2: warning: 'bar_start' may be used uninitialized in this function [-Wmaybe-uninitialized]
  pr_info("%s: %s at %s addr = 0x%llx, IRQ = %d, Hardware addr = %pMF\n",
fddi/defxx.c:536:18: note: 'bar_start' was declared here
  resource_size_t bar_start[3];  /* pointers to ports */

This adds code to ensure that the BAR values are initialized
even in the impossible case when a device gets probed that
does not belong to any bus. This shuts up the warning.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/fddi/defxx.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/fddi/defxx.c b/drivers/net/fddi/defxx.c
index 7f975a2c8990..e7cdd1226d39 100644
--- a/drivers/net/fddi/defxx.c
+++ b/drivers/net/fddi/defxx.c
@@ -484,6 +484,11 @@ static void dfx_get_bars(struct device *bdev,
 		bar_start[2] = bar_start[1] = 0;
 		bar_len[2] = bar_len[1] = 0;
 	}
+	if (!(dfx_bus_pci || dfx_bus_eisa || dfx_bus_tc)) {
+		dev_err(bdev, "invalid bus configuration\n");
+		bar_start[2] = bar_start[1] = bar_start[0] = 0;
+		bar_len[2] = bar_len[1] = bar_len[0] = 0;
+	}
 }
 
 static const struct net_device_ops dfx_netdev_ops = {
-- 
2.7.0

[PATCH 5/9] net: fddi/defxx: avoid warning about uninitialized variable use

[Arnd Bergmann]

The defxx driver can be configured for different kinds of buses,
and appears to be handling this correctly, but the compiler cannot
see how it always initializes the bar_start and bar_length
fields it uses depending on the configured bus, so we get a warning
with recent gcc versions:

fddi/defxx.c: In function 'dfx_pci_unregister':
fddi/defxx.c:3726:3: warning: 'bar_len' may be used uninitialized in this function [-Wmaybe-uninitialized]
   release_mem_region(bar_start[0], bar_len[0]);
fddi/defxx.c:3701:18: note: 'bar_len' was declared here
  resource_size_t bar_len[3];  /* resource lengths */
fddi/defxx.c:3726:3: warning: 'bar_start' may be used uninitialized in this function [-Wmaybe-uninitialized]
   release_mem_region(bar_start[0], bar_len[0]);
fddi/defxx.c:3700:18: note: 'bar_start' was declared here
  resource_size_t bar_start[3];  /* pointers to ports */
                  ^
fddi/defxx.c: In function 'dfx_pci_register':
fddi/defxx.c:617:18: warning: 'bar_len' may be used uninitialized in this function [-Wmaybe-uninitialized]
   bp->base.mem = ioremap_nocache(bar_start[0], bar_len[0]);
fddi/defxx.c:537:18: note: 'bar_len' was declared here
  resource_size_t bar_len[3];  /* resource length */
fddi/defxx.c:1125:2: warning: 'bar_start' may be used uninitialized in this function [-Wmaybe-uninitialized]
  pr_info("%s: %s at %s addr = 0x%llx, IRQ = %d, Hardware addr = %pMF\n",
fddi/defxx.c:536:18: note: 'bar_start' was declared here
  resource_size_t bar_start[3];  /* pointers to ports */

This adds code to ensure that the BAR values are initialized
even in the impossible case when a device gets probed that
does not belong to any bus. This shuts up the warning.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/fddi/defxx.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/fddi/defxx.c b/drivers/net/fddi/defxx.c
index 7f975a2c8990..e7cdd1226d39 100644
--- a/drivers/net/fddi/defxx.c
+++ b/drivers/net/fddi/defxx.c
@@ -484,6 +484,11 @@ static void dfx_get_bars(struct device *bdev,
 		bar_start[2] = bar_start[1] = 0;
 		bar_len[2] = bar_len[1] = 0;
 	}
+	if (!(dfx_bus_pci || dfx_bus_eisa || dfx_bus_tc)) {
+		dev_err(bdev, "invalid bus configuration\n");
+		bar_start[2] = bar_start[1] = bar_start[0] = 0;
+		bar_len[2] = bar_len[1] = bar_len[0] = 0;
+	}
 }
 
 static const struct net_device_ops dfx_netdev_ops = {
-- 
2.7.0

[PATCH 6/9] net: vxge: avoid unused function warnings

[Arnd Bergmann]

When CONFIG_PCI_MSI is disabled, we get warnings about unused functions
in the vxge driver:

drivers/net/ethernet/neterion/vxge/vxge-main.c:2121:13: warning: 'adaptive_coalesce_tx_interrupts' defined but not used [-Wunused-function]
drivers/net/ethernet/neterion/vxge/vxge-main.c:2149:13: warning: 'adaptive_coalesce_rx_interrupts' defined but not used [-Wunused-function]

We could add another #ifdef here, but it's nicer to avoid those warnings
for good by converting the existing #ifdef to if(IS_ENABLED()), which has
the same effect but provides better compile-time coverage in general,
and lets the compiler understand better when the function is intentionally
unused.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/ethernet/neterion/vxge/vxge-main.c | 31 ++++++++++----------------
 1 file changed, 12 insertions(+), 19 deletions(-)

diff --git a/drivers/net/ethernet/neterion/vxge/vxge-main.c b/drivers/net/ethernet/neterion/vxge/vxge-main.c
index 50d5604833ed..e0993eba5df3 100644
--- a/drivers/net/ethernet/neterion/vxge/vxge-main.c
+++ b/drivers/net/ethernet/neterion/vxge/vxge-main.c
@@ -2223,8 +2223,6 @@ static irqreturn_t vxge_isr_napi(int irq, void *dev_id)
 	return IRQ_NONE;
 }
 
-#ifdef CONFIG_PCI_MSI
-
 static irqreturn_t vxge_tx_msix_handle(int irq, void *dev_id)
 {
 	struct vxge_fifo *fifo = (struct vxge_fifo *)dev_id;
@@ -2442,16 +2440,13 @@ static void vxge_rem_msix_isr(struct vxgedev *vdev)
 	if (vdev->config.intr_type == MSI_X)
 		pci_disable_msix(vdev->pdev);
 }
-#endif
 
 static void vxge_rem_isr(struct vxgedev *vdev)
 {
-#ifdef CONFIG_PCI_MSI
-	if (vdev->config.intr_type == MSI_X) {
+	if (IS_ENABLED(CONFIG_PCI_MSI) &&
+	    vdev->config.intr_type == MSI_X) {
 		vxge_rem_msix_isr(vdev);
-	} else
-#endif
-	if (vdev->config.intr_type == INTA) {
+	} else if (vdev->config.intr_type == INTA) {
 			synchronize_irq(vdev->pdev->irq);
 			free_irq(vdev->pdev->irq, vdev);
 	}
@@ -2460,11 +2455,10 @@ static void vxge_rem_isr(struct vxgedev *vdev)
 static int vxge_add_isr(struct vxgedev *vdev)
 {
 	int ret = 0;
-#ifdef CONFIG_PCI_MSI
 	int vp_idx = 0, intr_idx = 0, intr_cnt = 0, msix_idx = 0, irq_req = 0;
 	int pci_fun = PCI_FUNC(vdev->pdev->devfn);
 
-	if (vdev->config.intr_type == MSI_X)
+	if (IS_ENABLED(CONFIG_PCI_MSI) && vdev->config.intr_type == MSI_X)
 		ret = vxge_enable_msix(vdev);
 
 	if (ret) {
@@ -2475,7 +2469,7 @@ static int vxge_add_isr(struct vxgedev *vdev)
 		vdev->config.intr_type = INTA;
 	}
 
-	if (vdev->config.intr_type == MSI_X) {
+	if (IS_ENABLED(CONFIG_PCI_MSI) && vdev->config.intr_type == MSI_X) {
 		for (intr_idx = 0;
 		     intr_idx < (vdev->no_of_vpath *
 			VXGE_HW_VPATH_MSIX_ACTIVE); intr_idx++) {
@@ -2576,9 +2570,8 @@ static int vxge_add_isr(struct vxgedev *vdev)
 		vdev->vxge_entries[intr_cnt].in_use = 1;
 		vdev->vxge_entries[intr_cnt].arg = &vdev->vpaths[0];
 	}
-INTA_MODE:
-#endif
 
+INTA_MODE:
 	if (vdev->config.intr_type == INTA) {
 		snprintf(vdev->desc[0], VXGE_INTR_STRLEN,
 			"%s:vxge:INTA", vdev->ndev->name);
@@ -3889,12 +3882,12 @@ static void vxge_device_config_init(struct vxge_hw_device_config *device_config,
 	if (max_mac_vpath > VXGE_MAX_MAC_ADDR_COUNT)
 		max_mac_vpath = VXGE_MAX_MAC_ADDR_COUNT;
 
-#ifndef CONFIG_PCI_MSI
-	vxge_debug_init(VXGE_ERR,
-		"%s: This Kernel does not support "
-		"MSI-X. Defaulting to INTA", VXGE_DRIVER_NAME);
-	*intr_type = INTA;
-#endif
+	if (!IS_ENABLED(CONFIG_PCI_MSI)) {
+		vxge_debug_init(VXGE_ERR,
+			"%s: This Kernel does not support "
+			"MSI-X. Defaulting to INTA", VXGE_DRIVER_NAME);
+		*intr_type = INTA;
+	}
 
 	/* Configure whether MSI-X or IRQL. */
 	switch (*intr_type) {
-- 
2.7.0

[PATCH 6/9] net: vxge: avoid unused function warnings

[Arnd Bergmann]

When CONFIG_PCI_MSI is disabled, we get warnings about unused functions
in the vxge driver:

drivers/net/ethernet/neterion/vxge/vxge-main.c:2121:13: warning: 'adaptive_coalesce_tx_interrupts' defined but not used [-Wunused-function]
drivers/net/ethernet/neterion/vxge/vxge-main.c:2149:13: warning: 'adaptive_coalesce_rx_interrupts' defined but not used [-Wunused-function]

We could add another #ifdef here, but it's nicer to avoid those warnings
for good by converting the existing #ifdef to if(IS_ENABLED()), which has
the same effect but provides better compile-time coverage in general,
and lets the compiler understand better when the function is intentionally
unused.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/ethernet/neterion/vxge/vxge-main.c | 31 ++++++++++----------------
 1 file changed, 12 insertions(+), 19 deletions(-)

diff --git a/drivers/net/ethernet/neterion/vxge/vxge-main.c b/drivers/net/ethernet/neterion/vxge/vxge-main.c
index 50d5604833ed..e0993eba5df3 100644
--- a/drivers/net/ethernet/neterion/vxge/vxge-main.c
+++ b/drivers/net/ethernet/neterion/vxge/vxge-main.c
@@ -2223,8 +2223,6 @@ static irqreturn_t vxge_isr_napi(int irq, void *dev_id)
 	return IRQ_NONE;
 }
 
-#ifdef CONFIG_PCI_MSI
-
 static irqreturn_t vxge_tx_msix_handle(int irq, void *dev_id)
 {
 	struct vxge_fifo *fifo = (struct vxge_fifo *)dev_id;
@@ -2442,16 +2440,13 @@ static void vxge_rem_msix_isr(struct vxgedev *vdev)
 	if (vdev->config.intr_type == MSI_X)
 		pci_disable_msix(vdev->pdev);
 }
-#endif
 
 static void vxge_rem_isr(struct vxgedev *vdev)
 {
-#ifdef CONFIG_PCI_MSI
-	if (vdev->config.intr_type == MSI_X) {
+	if (IS_ENABLED(CONFIG_PCI_MSI) &&
+	    vdev->config.intr_type == MSI_X) {
 		vxge_rem_msix_isr(vdev);
-	} else
-#endif
-	if (vdev->config.intr_type == INTA) {
+	} else if (vdev->config.intr_type == INTA) {
 			synchronize_irq(vdev->pdev->irq);
 			free_irq(vdev->pdev->irq, vdev);
 	}
@@ -2460,11 +2455,10 @@ static void vxge_rem_isr(struct vxgedev *vdev)
 static int vxge_add_isr(struct vxgedev *vdev)
 {
 	int ret = 0;
-#ifdef CONFIG_PCI_MSI
 	int vp_idx = 0, intr_idx = 0, intr_cnt = 0, msix_idx = 0, irq_req = 0;
 	int pci_fun = PCI_FUNC(vdev->pdev->devfn);
 
-	if (vdev->config.intr_type == MSI_X)
+	if (IS_ENABLED(CONFIG_PCI_MSI) && vdev->config.intr_type == MSI_X)
 		ret = vxge_enable_msix(vdev);
 
 	if (ret) {
@@ -2475,7 +2469,7 @@ static int vxge_add_isr(struct vxgedev *vdev)
 		vdev->config.intr_type = INTA;
 	}
 
-	if (vdev->config.intr_type == MSI_X) {
+	if (IS_ENABLED(CONFIG_PCI_MSI) && vdev->config.intr_type == MSI_X) {
 		for (intr_idx = 0;
 		     intr_idx < (vdev->no_of_vpath *
 			VXGE_HW_VPATH_MSIX_ACTIVE); intr_idx++) {
@@ -2576,9 +2570,8 @@ static int vxge_add_isr(struct vxgedev *vdev)
 		vdev->vxge_entries[intr_cnt].in_use = 1;
 		vdev->vxge_entries[intr_cnt].arg = &vdev->vpaths[0];
 	}
-INTA_MODE:
-#endif
 
+INTA_MODE:
 	if (vdev->config.intr_type == INTA) {
 		snprintf(vdev->desc[0], VXGE_INTR_STRLEN,
 			"%s:vxge:INTA", vdev->ndev->name);
@@ -3889,12 +3882,12 @@ static void vxge_device_config_init(struct vxge_hw_device_config *device_config,
 	if (max_mac_vpath > VXGE_MAX_MAC_ADDR_COUNT)
 		max_mac_vpath = VXGE_MAX_MAC_ADDR_COUNT;
 
-#ifndef CONFIG_PCI_MSI
-	vxge_debug_init(VXGE_ERR,
-		"%s: This Kernel does not support "
-		"MSI-X. Defaulting to INTA", VXGE_DRIVER_NAME);
-	*intr_type = INTA;
-#endif
+	if (!IS_ENABLED(CONFIG_PCI_MSI)) {
+		vxge_debug_init(VXGE_ERR,
+			"%s: This Kernel does not support "
+			"MSI-X. Defaulting to INTA", VXGE_DRIVER_NAME);
+		*intr_type = INTA;
+	}
 
 	/* Configure whether MSI-X or IRQL. */
 	switch (*intr_type) {
-- 
2.7.0

[PATCH 7/9] net: macb: avoid uninitialized variables

[Arnd Bergmann]

The macb_clk_init function returns three clock pointers, unless
the it fails to get the first ones. We correctly handle the
failure case by propagating the error from macb_probe, but
gcc does not realize this and incorrectly warns about a later
use of those:

In file included from /git/arm-soc/drivers/net/ethernet/cadence/macb.c:12:0:
drivers/net/ethernet/cadence/macb.c: In function 'macb_probe':
include/linux/clk.h:484:2: error: 'tx_clk' may be used uninitialized in this function [-Werror=maybe-uninitialized]
  clk_disable(clk);
  ^
drivers/net/ethernet/cadence/macb.c:2822:28: note: 'tx_clk' was declared here
  struct clk *pclk, *hclk, *tx_clk;
                            ^
In file included from /git/arm-soc/drivers/net/ethernet/cadence/macb.c:12:0:
include/linux/clk.h:484:2: error: 'hclk' may be used uninitialized in this function [-Werror=maybe-uninitialized]
  clk_disable(clk);
  ^
drivers/net/ethernet/cadence/macb.c:2822:21: note: 'hclk' was declared here
  struct clk *pclk, *hclk, *tx_clk;
                     ^

This shuts up the misleading warnings by ensuring that the
macb_clk_init() always stores something into all three pointers.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/ethernet/cadence/macb.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index 9d9984a87d42..d3aa74f9db79 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -2268,6 +2268,7 @@ static int macb_clk_init(struct platform_device *pdev, struct clk **pclk,
 {
 	int err;
 
+	*tx_clk = *hclk = NULL;
 	*pclk = devm_clk_get(&pdev->dev, "pclk");
 	if (IS_ERR(*pclk)) {
 		err = PTR_ERR(*pclk);
-- 
2.7.0

[PATCH 7/9] net: macb: avoid uninitialized variables

[Arnd Bergmann]

The macb_clk_init function returns three clock pointers, unless
the it fails to get the first ones. We correctly handle the
failure case by propagating the error from macb_probe, but
gcc does not realize this and incorrectly warns about a later
use of those:

In file included from /git/arm-soc/drivers/net/ethernet/cadence/macb.c:12:0:
drivers/net/ethernet/cadence/macb.c: In function 'macb_probe':
include/linux/clk.h:484:2: error: 'tx_clk' may be used uninitialized in this function [-Werror=maybe-uninitialized]
  clk_disable(clk);
  ^
drivers/net/ethernet/cadence/macb.c:2822:28: note: 'tx_clk' was declared here
  struct clk *pclk, *hclk, *tx_clk;
                            ^
In file included from /git/arm-soc/drivers/net/ethernet/cadence/macb.c:12:0:
include/linux/clk.h:484:2: error: 'hclk' may be used uninitialized in this function [-Werror=maybe-uninitialized]
  clk_disable(clk);
  ^
drivers/net/ethernet/cadence/macb.c:2822:21: note: 'hclk' was declared here
  struct clk *pclk, *hclk, *tx_clk;
                     ^

This shuts up the misleading warnings by ensuring that the
macb_clk_init() always stores something into all three pointers.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/ethernet/cadence/macb.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index 9d9984a87d42..d3aa74f9db79 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -2268,6 +2268,7 @@ static int macb_clk_init(struct platform_device *pdev, struct clk **pclk,
 {
 	int err;
 
+	*tx_clk = *hclk = NULL;
 	*pclk = devm_clk_get(&pdev->dev, "pclk");
 	if (IS_ERR(*pclk)) {
 		err = PTR_ERR(*pclk);
-- 
2.7.0

[PATCH 8/9] net: nb8800: avoid uninitialized variable warning

[Arnd Bergmann]

The nb8800_poll() function initializes the 'next' variable in the
loop looking for new input data. We know this will be called at
least once because 'budget' is a guaranteed to be a positive number
when we enter the function, but the compiler doesn't know that
and warns when the variable is used later:

drivers/net/ethernet/aurora/nb8800.c: In function 'nb8800_poll':
drivers/net/ethernet/aurora/nb8800.c:350:21: warning: 'next' may be used uninitialized in this function [-Wmaybe-uninitialized]

Changing the 'while() {}' loop to 'do {} while()' makes it obvious
to the compiler what is going on so it no longer warns.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/ethernet/aurora/nb8800.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/aurora/nb8800.c b/drivers/net/ethernet/aurora/nb8800.c
index ecc4a334c507..f71ab2647a3b 100644
--- a/drivers/net/ethernet/aurora/nb8800.c
+++ b/drivers/net/ethernet/aurora/nb8800.c
@@ -302,7 +302,7 @@ static int nb8800_poll(struct napi_struct *napi, int budget)
 	nb8800_tx_done(dev);
 
 again:
-	while (work < budget) {
+	do {
 		struct nb8800_rx_buf *rxb;
 		unsigned int len;
 
@@ -330,7 +330,7 @@ again:
 		rxd->report = 0;
 		last = next;
 		work++;
-	}
+	} while (work < budget);
 
 	if (work) {
 		priv->rx_descs[last].desc.config |= DESC_EOC;
-- 
2.7.0

[PATCH 8/9] net: nb8800: avoid uninitialized variable warning

[Arnd Bergmann]

The nb8800_poll() function initializes the 'next' variable in the
loop looking for new input data. We know this will be called at
least once because 'budget' is a guaranteed to be a positive number
when we enter the function, but the compiler doesn't know that
and warns when the variable is used later:

drivers/net/ethernet/aurora/nb8800.c: In function 'nb8800_poll':
drivers/net/ethernet/aurora/nb8800.c:350:21: warning: 'next' may be used uninitialized in this function [-Wmaybe-uninitialized]

Changing the 'while() {}' loop to 'do {} while()' makes it obvious
to the compiler what is going on so it no longer warns.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/ethernet/aurora/nb8800.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/aurora/nb8800.c b/drivers/net/ethernet/aurora/nb8800.c
index ecc4a334c507..f71ab2647a3b 100644
--- a/drivers/net/ethernet/aurora/nb8800.c
+++ b/drivers/net/ethernet/aurora/nb8800.c
@@ -302,7 +302,7 @@ static int nb8800_poll(struct napi_struct *napi, int budget)
 	nb8800_tx_done(dev);
 
 again:
-	while (work < budget) {
+	do {
 		struct nb8800_rx_buf *rxb;
 		unsigned int len;
 
@@ -330,7 +330,7 @@ again:
 		rxd->report = 0;
 		last = next;
 		work++;
-	}
+	} while (work < budget);
 
 	if (work) {
 		priv->rx_descs[last].desc.config |= DESC_EOC;
-- 
2.7.0

[PATCH 9/9] net: tg3: avoid uninitialized variable warning

[Arnd Bergmann]

The tg3_set_eeprom() function correctly initializes the 'start' variable,
but gcc generates a false warning:

drivers/net/ethernet/broadcom/tg3.c: In function 'tg3_set_eeprom':
drivers/net/ethernet/broadcom/tg3.c:12057:4: warning: 'start' may be used uninitialized in this function [-Wmaybe-uninitialized]

I have not come up with a way to restructure the code in a way that
avoids the warning without making it less readable, so this adds an
initialization for the declaration to shut up that warning.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/ethernet/broadcom/tg3.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index 9293675df7ba..49eea8981332 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -12016,7 +12016,7 @@ static int tg3_set_eeprom(struct net_device *dev, struct ethtool_eeprom *eeprom,
 	int ret;
 	u32 offset, len, b_offset, odd_len;
 	u8 *buf;
-	__be32 start, end;
+	__be32 start = 0, end;
 
 	if (tg3_flag(tp, NO_NVRAM) ||
 	    eeprom->magic != TG3_EEPROM_MAGIC)
-- 
2.7.0

[PATCH 9/9] net: tg3: avoid uninitialized variable warning

[Arnd Bergmann]

The tg3_set_eeprom() function correctly initializes the 'start' variable,
but gcc generates a false warning:

drivers/net/ethernet/broadcom/tg3.c: In function 'tg3_set_eeprom':
drivers/net/ethernet/broadcom/tg3.c:12057:4: warning: 'start' may be used uninitialized in this function [-Wmaybe-uninitialized]

I have not come up with a way to restructure the code in a way that
avoids the warning without making it less readable, so this adds an
initialization for the declaration to shut up that warning.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/ethernet/broadcom/tg3.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index 9293675df7ba..49eea8981332 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -12016,7 +12016,7 @@ static int tg3_set_eeprom(struct net_device *dev, struct ethtool_eeprom *eeprom,
 	int ret;
 	u32 offset, len, b_offset, odd_len;
 	u8 *buf;
-	__be32 start, end;
+	__be32 start = 0, end;
 
 	if (tg3_flag(tp, NO_NVRAM) ||
 	    eeprom->magic != TG3_EEPROM_MAGIC)
-- 
2.7.0

Re: [PATCH 8/9] net: nb8800: avoid uninitialized variable warning

[Måns Rullgård]

Arnd Bergmann <arnd@arndb.de> writes:

> The nb8800_poll() function initializes the 'next' variable in the
> loop looking for new input data. We know this will be called at
> least once because 'budget' is a guaranteed to be a positive number
> when we enter the function, but the compiler doesn't know that
> and warns when the variable is used later:
>
> drivers/net/ethernet/aurora/nb8800.c: In function 'nb8800_poll':
> drivers/net/ethernet/aurora/nb8800.c:350:21: warning: 'next' may be used uninitialized in this function [-Wmaybe-uninitialized]

Which gcc version is this?  4.9 doesn't warn here, presumably because
it's clever enough to notice that the offending use of 'next' is under a
condition that can only be true if the first one was.  Of course fixing
the code so older compilers don't warn is a good idea.

> Changing the 'while() {}' loop to 'do {} while()' makes it obvious
> to the compiler what is going on so it no longer warns.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Acked-by: Mans Rullgard <mans@mansr.com>

> ---
>  drivers/net/ethernet/aurora/nb8800.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/aurora/nb8800.c b/drivers/net/ethernet/aurora/nb8800.c
> index ecc4a334c507..f71ab2647a3b 100644
> --- a/drivers/net/ethernet/aurora/nb8800.c
> +++ b/drivers/net/ethernet/aurora/nb8800.c
> @@ -302,7 +302,7 @@ static int nb8800_poll(struct napi_struct *napi, int budget)
>  	nb8800_tx_done(dev);
>
>  again:
> -	while (work < budget) {
> +	do {
>  		struct nb8800_rx_buf *rxb;
>  		unsigned int len;
>
> @@ -330,7 +330,7 @@ again:
>  		rxd->report = 0;
>  		last = next;
>  		work++;
> -	}
> +	} while (work < budget);
>
>  	if (work) {
>  		priv->rx_descs[last].desc.config |= DESC_EOC;
> -- 
> 2.7.0
>

-- 
Måns Rullgård

[PATCH 8/9] net: nb8800: avoid uninitialized variable warning

[Måns Rullgård]

Arnd Bergmann <arnd@arndb.de> writes:

> The nb8800_poll() function initializes the 'next' variable in the
> loop looking for new input data. We know this will be called at
> least once because 'budget' is a guaranteed to be a positive number
> when we enter the function, but the compiler doesn't know that
> and warns when the variable is used later:
>
> drivers/net/ethernet/aurora/nb8800.c: In function 'nb8800_poll':
> drivers/net/ethernet/aurora/nb8800.c:350:21: warning: 'next' may be used uninitialized in this function [-Wmaybe-uninitialized]

Which gcc version is this?  4.9 doesn't warn here, presumably because
it's clever enough to notice that the offending use of 'next' is under a
condition that can only be true if the first one was.  Of course fixing
the code so older compilers don't warn is a good idea.

> Changing the 'while() {}' loop to 'do {} while()' makes it obvious
> to the compiler what is going on so it no longer warns.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Acked-by: Mans Rullgard <mans@mansr.com>

> ---
>  drivers/net/ethernet/aurora/nb8800.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/aurora/nb8800.c b/drivers/net/ethernet/aurora/nb8800.c
> index ecc4a334c507..f71ab2647a3b 100644
> --- a/drivers/net/ethernet/aurora/nb8800.c
> +++ b/drivers/net/ethernet/aurora/nb8800.c
> @@ -302,7 +302,7 @@ static int nb8800_poll(struct napi_struct *napi, int budget)
>  	nb8800_tx_done(dev);
>
>  again:
> -	while (work < budget) {
> +	do {
>  		struct nb8800_rx_buf *rxb;
>  		unsigned int len;
>
> @@ -330,7 +330,7 @@ again:
>  		rxd->report = 0;
>  		last = next;
>  		work++;
> -	}
> +	} while (work < budget);
>
>  	if (work) {
>  		priv->rx_descs[last].desc.config |= DESC_EOC;
> -- 
> 2.7.0
>

-- 
M?ns Rullg?rd

Re: [PATCH 5/9] net: fddi/defxx: avoid warning about uninitialized variable use

[Maciej W. Rozycki]

On Wed, 27 Jan 2016, Arnd Bergmann wrote:

> This adds code to ensure that the BAR values are initialized
> even in the impossible case when a device gets probed that
> does not belong to any bus. This shuts up the warning.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/net/fddi/defxx.c | 5 +++++
>  1 file changed, 5 insertions(+)

 NAK, fixed already, commit 62f2aaabcf41 ("defxx: fix build warning").

 Thanks for looking into this problem though, always welcome!

  Maciej

[PATCH 5/9] net: fddi/defxx: avoid warning about uninitialized variable use

[Maciej W. Rozycki]

On Wed, 27 Jan 2016, Arnd Bergmann wrote:

> This adds code to ensure that the BAR values are initialized
> even in the impossible case when a device gets probed that
> does not belong to any bus. This shuts up the warning.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/net/fddi/defxx.c | 5 +++++
>  1 file changed, 5 insertions(+)

 NAK, fixed already, commit 62f2aaabcf41 ("defxx: fix build warning").

 Thanks for looking into this problem though, always welcome!

  Maciej

Re: [PATCH 8/9] net: nb8800: avoid uninitialized variable warning

[Arnd Bergmann]

On Wednesday 27 January 2016 14:13:16 Måns Rullgård wrote:
> Arnd Bergmann <arnd@arndb.de> writes:
> 
> > The nb8800_poll() function initializes the 'next' variable in the
> > loop looking for new input data. We know this will be called at
> > least once because 'budget' is a guaranteed to be a positive number
> > when we enter the function, but the compiler doesn't know that
> > and warns when the variable is used later:
> >
> > drivers/net/ethernet/aurora/nb8800.c: In function 'nb8800_poll':
> > drivers/net/ethernet/aurora/nb8800.c:350:21: warning: 'next' may be used uninitialized in this function [-Wmaybe-uninitialized]
> 
> Which gcc version is this?  4.9 doesn't warn here, presumably because
> it's clever enough to notice that the offending use of 'next' is under a
> condition that can only be true if the first one was.  Of course fixing
> the code so older compilers don't warn is a good idea.

This was with gcc-5.2.1, but possibly in an unusual kernel configuration.
The only time I see it in my logs was with CONFIG_ARCH_IXP4XX=y, which
has its own mach/io.h and other headers that sometimes override generic
functions with a more elaborate version.

I have another patch for ixp4xx that simplifies its mach/io.h in order to
get rid of other false 'uninitialized use' warnings, but this one still
shows up with that other patch.

> > Changing the 'while() {}' loop to 'do {} while()' makes it obvious
> > to the compiler what is going on so it no longer warns.
> >
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> 
> Acked-by: Mans Rullgard <mans@mansr.com>

Thanks,

	Arnd

[PATCH 8/9] net: nb8800: avoid uninitialized variable warning

[Arnd Bergmann]

On Wednesday 27 January 2016 14:13:16 M?ns Rullg?rd wrote:
> Arnd Bergmann <arnd@arndb.de> writes:
> 
> > The nb8800_poll() function initializes the 'next' variable in the
> > loop looking for new input data. We know this will be called at
> > least once because 'budget' is a guaranteed to be a positive number
> > when we enter the function, but the compiler doesn't know that
> > and warns when the variable is used later:
> >
> > drivers/net/ethernet/aurora/nb8800.c: In function 'nb8800_poll':
> > drivers/net/ethernet/aurora/nb8800.c:350:21: warning: 'next' may be used uninitialized in this function [-Wmaybe-uninitialized]
> 
> Which gcc version is this?  4.9 doesn't warn here, presumably because
> it's clever enough to notice that the offending use of 'next' is under a
> condition that can only be true if the first one was.  Of course fixing
> the code so older compilers don't warn is a good idea.

This was with gcc-5.2.1, but possibly in an unusual kernel configuration.
The only time I see it in my logs was with CONFIG_ARCH_IXP4XX=y, which
has its own mach/io.h and other headers that sometimes override generic
functions with a more elaborate version.

I have another patch for ixp4xx that simplifies its mach/io.h in order to
get rid of other false 'uninitialized use' warnings, but this one still
shows up with that other patch.

> > Changing the 'while() {}' loop to 'do {} while()' makes it obvious
> > to the compiler what is going on so it no longer warns.
> >
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> 
> Acked-by: Mans Rullgard <mans@mansr.com>

Thanks,

	Arnd

Re: [PATCH 7/9] net: macb: avoid uninitialized variables

[Nicolas Ferre]

Le 27/01/2016 15:04, Arnd Bergmann a écrit :
> The macb_clk_init function returns three clock pointers, unless
> the it fails to get the first ones. We correctly handle the
> failure case by propagating the error from macb_probe, but
> gcc does not realize this and incorrectly warns about a later
> use of those:
> 
> In file included from /git/arm-soc/drivers/net/ethernet/cadence/macb.c:12:0:
> drivers/net/ethernet/cadence/macb.c: In function 'macb_probe':
> include/linux/clk.h:484:2: error: 'tx_clk' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>   clk_disable(clk);
>   ^
> drivers/net/ethernet/cadence/macb.c:2822:28: note: 'tx_clk' was declared here
>   struct clk *pclk, *hclk, *tx_clk;
>                             ^
> In file included from /git/arm-soc/drivers/net/ethernet/cadence/macb.c:12:0:
> include/linux/clk.h:484:2: error: 'hclk' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>   clk_disable(clk);
>   ^
> drivers/net/ethernet/cadence/macb.c:2822:21: note: 'hclk' was declared here
>   struct clk *pclk, *hclk, *tx_clk;
>                      ^
> 
> This shuts up the misleading warnings by ensuring that the
> macb_clk_init() always stores something into all three pointers.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Okay Arnd, thanks!

Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>

> ---
>  drivers/net/ethernet/cadence/macb.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
> index 9d9984a87d42..d3aa74f9db79 100644
> --- a/drivers/net/ethernet/cadence/macb.c
> +++ b/drivers/net/ethernet/cadence/macb.c
> @@ -2268,6 +2268,7 @@ static int macb_clk_init(struct platform_device *pdev, struct clk **pclk,
>  {
>  	int err;
>  
> +	*tx_clk = *hclk = NULL;
>  	*pclk = devm_clk_get(&pdev->dev, "pclk");
>  	if (IS_ERR(*pclk)) {
>  		err = PTR_ERR(*pclk);
> 


-- 
Nicolas Ferre

[PATCH 7/9] net: macb: avoid uninitialized variables

[Nicolas Ferre]

Le 27/01/2016 15:04, Arnd Bergmann a ?crit :
> The macb_clk_init function returns three clock pointers, unless
> the it fails to get the first ones. We correctly handle the
> failure case by propagating the error from macb_probe, but
> gcc does not realize this and incorrectly warns about a later
> use of those:
> 
> In file included from /git/arm-soc/drivers/net/ethernet/cadence/macb.c:12:0:
> drivers/net/ethernet/cadence/macb.c: In function 'macb_probe':
> include/linux/clk.h:484:2: error: 'tx_clk' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>   clk_disable(clk);
>   ^
> drivers/net/ethernet/cadence/macb.c:2822:28: note: 'tx_clk' was declared here
>   struct clk *pclk, *hclk, *tx_clk;
>                             ^
> In file included from /git/arm-soc/drivers/net/ethernet/cadence/macb.c:12:0:
> include/linux/clk.h:484:2: error: 'hclk' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>   clk_disable(clk);
>   ^
> drivers/net/ethernet/cadence/macb.c:2822:21: note: 'hclk' was declared here
>   struct clk *pclk, *hclk, *tx_clk;
>                      ^
> 
> This shuts up the misleading warnings by ensuring that the
> macb_clk_init() always stores something into all three pointers.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Okay Arnd, thanks!

Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>

> ---
>  drivers/net/ethernet/cadence/macb.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
> index 9d9984a87d42..d3aa74f9db79 100644
> --- a/drivers/net/ethernet/cadence/macb.c
> +++ b/drivers/net/ethernet/cadence/macb.c
> @@ -2268,6 +2268,7 @@ static int macb_clk_init(struct platform_device *pdev, struct clk **pclk,
>  {
>  	int err;
>  
> +	*tx_clk = *hclk = NULL;
>  	*pclk = devm_clk_get(&pdev->dev, "pclk");
>  	if (IS_ERR(*pclk)) {
>  		err = PTR_ERR(*pclk);
> 


-- 
Nicolas Ferre

Re: [PATCH 7/9] net: macb: avoid uninitialized variables

[Nicolas Ferre]

Le 27/01/2016 16:51, Nicolas Ferre a écrit :
> Le 27/01/2016 15:04, Arnd Bergmann a écrit :
>> The macb_clk_init function returns three clock pointers, unless
>> the it fails to get the first ones. We correctly handle the
>> failure case by propagating the error from macb_probe, but
>> gcc does not realize this and incorrectly warns about a later
>> use of those:
>>
>> In file included from /git/arm-soc/drivers/net/ethernet/cadence/macb.c:12:0:
>> drivers/net/ethernet/cadence/macb.c: In function 'macb_probe':
>> include/linux/clk.h:484:2: error: 'tx_clk' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>>   clk_disable(clk);
>>   ^
>> drivers/net/ethernet/cadence/macb.c:2822:28: note: 'tx_clk' was declared here
>>   struct clk *pclk, *hclk, *tx_clk;
>>                             ^
>> In file included from /git/arm-soc/drivers/net/ethernet/cadence/macb.c:12:0:
>> include/linux/clk.h:484:2: error: 'hclk' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>>   clk_disable(clk);
>>   ^
>> drivers/net/ethernet/cadence/macb.c:2822:21: note: 'hclk' was declared here
>>   struct clk *pclk, *hclk, *tx_clk;
>>                      ^
>>
>> This shuts up the misleading warnings by ensuring that the
>> macb_clk_init() always stores something into all three pointers.
>>
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> 
> Okay Arnd, thanks!
> 
> Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>

Oh, crap: actually this warning has just been fixed by Sudip Mukherjee
and is already queued by David here:
https://patchwork.ozlabs.org/patch/572610/

So, sorry, I've shot too fast: NACK...

Bye,

>> ---
>>  drivers/net/ethernet/cadence/macb.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
>> index 9d9984a87d42..d3aa74f9db79 100644
>> --- a/drivers/net/ethernet/cadence/macb.c
>> +++ b/drivers/net/ethernet/cadence/macb.c
>> @@ -2268,6 +2268,7 @@ static int macb_clk_init(struct platform_device *pdev, struct clk **pclk,
>>  {
>>  	int err;
>>  
>> +	*tx_clk = *hclk = NULL;
>>  	*pclk = devm_clk_get(&pdev->dev, "pclk");
>>  	if (IS_ERR(*pclk)) {
>>  		err = PTR_ERR(*pclk);
>>
> 
> 


-- 
Nicolas Ferre

[PATCH 7/9] net: macb: avoid uninitialized variables

[Nicolas Ferre]

Le 27/01/2016 16:51, Nicolas Ferre a ?crit :
> Le 27/01/2016 15:04, Arnd Bergmann a ?crit :
>> The macb_clk_init function returns three clock pointers, unless
>> the it fails to get the first ones. We correctly handle the
>> failure case by propagating the error from macb_probe, but
>> gcc does not realize this and incorrectly warns about a later
>> use of those:
>>
>> In file included from /git/arm-soc/drivers/net/ethernet/cadence/macb.c:12:0:
>> drivers/net/ethernet/cadence/macb.c: In function 'macb_probe':
>> include/linux/clk.h:484:2: error: 'tx_clk' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>>   clk_disable(clk);
>>   ^
>> drivers/net/ethernet/cadence/macb.c:2822:28: note: 'tx_clk' was declared here
>>   struct clk *pclk, *hclk, *tx_clk;
>>                             ^
>> In file included from /git/arm-soc/drivers/net/ethernet/cadence/macb.c:12:0:
>> include/linux/clk.h:484:2: error: 'hclk' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>>   clk_disable(clk);
>>   ^
>> drivers/net/ethernet/cadence/macb.c:2822:21: note: 'hclk' was declared here
>>   struct clk *pclk, *hclk, *tx_clk;
>>                      ^
>>
>> This shuts up the misleading warnings by ensuring that the
>> macb_clk_init() always stores something into all three pointers.
>>
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> 
> Okay Arnd, thanks!
> 
> Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>

Oh, crap: actually this warning has just been fixed by Sudip Mukherjee
and is already queued by David here:
https://patchwork.ozlabs.org/patch/572610/

So, sorry, I've shot too fast: NACK...

Bye,

>> ---
>>  drivers/net/ethernet/cadence/macb.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
>> index 9d9984a87d42..d3aa74f9db79 100644
>> --- a/drivers/net/ethernet/cadence/macb.c
>> +++ b/drivers/net/ethernet/cadence/macb.c
>> @@ -2268,6 +2268,7 @@ static int macb_clk_init(struct platform_device *pdev, struct clk **pclk,
>>  {
>>  	int err;
>>  
>> +	*tx_clk = *hclk = NULL;
>>  	*pclk = devm_clk_get(&pdev->dev, "pclk");
>>  	if (IS_ERR(*pclk)) {
>>  		err = PTR_ERR(*pclk);
>>
> 
> 


-- 
Nicolas Ferre

Re: [PATCH 3/9] net: bgmac: clarify CONFIG_BCMA dependency

[Paul Gortmaker]

[[PATCH 3/9] net: bgmac: clarify CONFIG_BCMA dependency] On 27/01/2016 (Wed 15:04) Arnd Bergmann wrote:

> The bgmac driver depends on BCMA_HOST_SOC, which is only used
> when CONFIG_BCMA is enabled. However, it is a bool option and can
> be set when CONFIG_BCMA=m, and then bgmac can be built-in, leading
> to an obvious link error:
> 
> drivers/built-in.o: In function `bgmac_init':
> :(.init.text+0x7f2c): undefined reference to `__bcma_driver_register'
> drivers/built-in.o: In function `bgmac_exit':
> :(.exit.text+0x110a): undefined reference to `bcma_driver_unregister'
> 
> To avoid this case, we need to depend on both BCMA and BCMA_SOC,
> as this patch does. I'm also trying to make the dependency more
> readable by splitting it into three lines, and adding a COMPILE_TEST
> alternative so we can test-build it in all configurations that
> support BCMA.

It wasn't immediately clear to me from the above why you added the
select on FIXED_PHY.

P.
--

> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/net/ethernet/broadcom/Kconfig | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/Kconfig b/drivers/net/ethernet/broadcom/Kconfig
> index 8550df189ceb..19f7cd02e085 100644
> --- a/drivers/net/ethernet/broadcom/Kconfig
> +++ b/drivers/net/ethernet/broadcom/Kconfig
> @@ -151,8 +151,11 @@ config BNX2X_VXLAN
>  
>  config BGMAC
>  	tristate "BCMA bus GBit core support"
> -	depends on BCMA_HOST_SOC && HAS_DMA && (BCM47XX || ARCH_BCM_5301X)
> +	depends on BCMA && BCMA_HOST_SOC
> +	depends on HAS_DMA
> +	depends on BCM47XX || ARCH_BCM_5301X || COMPILE_TEST
>  	select PHYLIB
> +	select FIXED_PHY
>  	---help---
>  	  This driver supports GBit MAC and BCM4706 GBit MAC cores on BCMA bus.
>  	  They can be found on BCM47xx SoCs and provide gigabit ethernet.
> -- 
> 2.7.0
> 

Re: [PATCH 3/9] net: bgmac: clarify CONFIG_BCMA dependency

[Paul Gortmaker]

[[PATCH 3/9] net: bgmac: clarify CONFIG_BCMA dependency] On 27/01/2016 (Wed 15:04) Arnd Bergmann wrote:

> The bgmac driver depends on BCMA_HOST_SOC, which is only used
> when CONFIG_BCMA is enabled. However, it is a bool option and can
> be set when CONFIG_BCMA=m, and then bgmac can be built-in, leading
> to an obvious link error:
> 
> drivers/built-in.o: In function `bgmac_init':
> :(.init.text+0x7f2c): undefined reference to `__bcma_driver_register'
> drivers/built-in.o: In function `bgmac_exit':
> :(.exit.text+0x110a): undefined reference to `bcma_driver_unregister'
> 
> To avoid this case, we need to depend on both BCMA and BCMA_SOC,
> as this patch does. I'm also trying to make the dependency more
> readable by splitting it into three lines, and adding a COMPILE_TEST
> alternative so we can test-build it in all configurations that
> support BCMA.

It wasn't immediately clear to me from the above why you added the
select on FIXED_PHY.

P.

[PATCH 3/9] net: bgmac: clarify CONFIG_BCMA dependency

[Paul Gortmaker]

[[PATCH 3/9] net: bgmac: clarify CONFIG_BCMA dependency] On 27/01/2016 (Wed 15:04) Arnd Bergmann wrote:

> The bgmac driver depends on BCMA_HOST_SOC, which is only used
> when CONFIG_BCMA is enabled. However, it is a bool option and can
> be set when CONFIG_BCMA=m, and then bgmac can be built-in, leading
> to an obvious link error:
> 
> drivers/built-in.o: In function `bgmac_init':
> :(.init.text+0x7f2c): undefined reference to `__bcma_driver_register'
> drivers/built-in.o: In function `bgmac_exit':
> :(.exit.text+0x110a): undefined reference to `bcma_driver_unregister'
> 
> To avoid this case, we need to depend on both BCMA and BCMA_SOC,
> as this patch does. I'm also trying to make the dependency more
> readable by splitting it into three lines, and adding a COMPILE_TEST
> alternative so we can test-build it in all configurations that
> support BCMA.

It wasn't immediately clear to me from the above why you added the
select on FIXED_PHY.

P.
--

> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/net/ethernet/broadcom/Kconfig | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/Kconfig b/drivers/net/ethernet/broadcom/Kconfig
> index 8550df189ceb..19f7cd02e085 100644
> --- a/drivers/net/ethernet/broadcom/Kconfig
> +++ b/drivers/net/ethernet/broadcom/Kconfig
> @@ -151,8 +151,11 @@ config BNX2X_VXLAN
>  
>  config BGMAC
>  	tristate "BCMA bus GBit core support"
> -	depends on BCMA_HOST_SOC && HAS_DMA && (BCM47XX || ARCH_BCM_5301X)
> +	depends on BCMA && BCMA_HOST_SOC
> +	depends on HAS_DMA
> +	depends on BCM47XX || ARCH_BCM_5301X || COMPILE_TEST
>  	select PHYLIB
> +	select FIXED_PHY
>  	---help---
>  	  This driver supports GBit MAC and BCM4706 GBit MAC cores on BCMA bus.
>  	  They can be found on BCM47xx SoCs and provide gigabit ethernet.
> -- 
> 2.7.0
> 

Re: [PATCH 3/9] net: bgmac: clarify CONFIG_BCMA dependency

[Arnd Bergmann]

On Wednesday 27 January 2016 11:11:10 Paul Gortmaker wrote:
> [[PATCH 3/9] net: bgmac: clarify CONFIG_BCMA dependency] On 27/01/2016 (Wed 15:04) Arnd Bergmann wrote:
> 
> > The bgmac driver depends on BCMA_HOST_SOC, which is only used
> > when CONFIG_BCMA is enabled. However, it is a bool option and can
> > be set when CONFIG_BCMA=m, and then bgmac can be built-in, leading
> > to an obvious link error:
> > 
> > drivers/built-in.o: In function `bgmac_init':
> > :(.init.text+0x7f2c): undefined reference to `__bcma_driver_register'
> > drivers/built-in.o: In function `bgmac_exit':
> > :(.exit.text+0x110a): undefined reference to `bcma_driver_unregister'
> > 
> > To avoid this case, we need to depend on both BCMA and BCMA_SOC,
> > as this patch does. I'm also trying to make the dependency more
> > readable by splitting it into three lines, and adding a COMPILE_TEST
> > alternative so we can test-build it in all configurations that
> > support BCMA.
> 
> It wasn't immediately clear to me from the above why you added the
> select on FIXED_PHY.
> 

Right, I'll resend the patch with improved changelog. This series
is mostly patches for old and rare randconfig bugs, so I had built
thousands of configurations and fixed up everything until new warnings
or errors kept coming up. The FIXED_PHY error came up in the same
driver so I merged the two patches but did not notice how the changelog
failed to explain it.

Thanks,

	Arnd

[PATCH 3/9] net: bgmac: clarify CONFIG_BCMA dependency

[Arnd Bergmann]

On Wednesday 27 January 2016 11:11:10 Paul Gortmaker wrote:
> [[PATCH 3/9] net: bgmac: clarify CONFIG_BCMA dependency] On 27/01/2016 (Wed 15:04) Arnd Bergmann wrote:
> 
> > The bgmac driver depends on BCMA_HOST_SOC, which is only used
> > when CONFIG_BCMA is enabled. However, it is a bool option and can
> > be set when CONFIG_BCMA=m, and then bgmac can be built-in, leading
> > to an obvious link error:
> > 
> > drivers/built-in.o: In function `bgmac_init':
> > :(.init.text+0x7f2c): undefined reference to `__bcma_driver_register'
> > drivers/built-in.o: In function `bgmac_exit':
> > :(.exit.text+0x110a): undefined reference to `bcma_driver_unregister'
> > 
> > To avoid this case, we need to depend on both BCMA and BCMA_SOC,
> > as this patch does. I'm also trying to make the dependency more
> > readable by splitting it into three lines, and adding a COMPILE_TEST
> > alternative so we can test-build it in all configurations that
> > support BCMA.
> 
> It wasn't immediately clear to me from the above why you added the
> select on FIXED_PHY.
> 

Right, I'll resend the patch with improved changelog. This series
is mostly patches for old and rare randconfig bugs, so I had built
thousands of configurations and fixed up everything until new warnings
or errors kept coming up. The FIXED_PHY error came up in the same
driver so I merged the two patches but did not notice how the changelog
failed to explain it.

Thanks,

	Arnd

RE: [PATCH 4/9] net: moxart: use correct accessors for DMA memory

[David Laight]

From: Arnd Bergmann
> Sent: 27 January 2016 14:05
> The moxart ethernet driver confuses coherent DMA buffers with
> MMIO registers.
> 
> moxart_ether.c: In function 'moxart_mac_setup_desc_ring':
> moxart_ether.c:146:428: error: passing argument 1 of '__fswab32' makes integer from pointer without a
> cast [-Werror=int-conversion]
> moxart_ether.c:74:39: warning: incorrect type in argument 3 (different address spaces)
> moxart_ether.c:74:39:    expected void *cpu_addr
> moxart_ether.c:74:39:    got void [noderef] <asn:2>*tx_desc_base
> 
> This leaves the basic logic alone and uses normal pointers for
> the virtual address of the descriptor. As we cannot use readl/writel
> to access them, we also introduce our own moxart_desc_read
> moxart_desc_write helpers that perform the same endianess swap
> as the original code, but without the extra barriers and address
> space conversion.

I'm pretty sure you need to add some explicit barriers:

> @@ -354,8 +364,8 @@ static int moxart_mac_start_xmit(struct sk_buff *skb, struct net_device *ndev)
>  	txdes1 = TX_DESC1_LTS | TX_DESC1_FTS | (len & TX_DESC1_BUF_SIZE_MASK);
>  	if (tx_head == TX_DESC_NUM_MASK)
>  		txdes1 |= TX_DESC1_END;
> -	writel(txdes1, desc + TX_REG_OFFSET_DESC1);
> -	writel(TX_DESC0_DMA_OWN, desc + TX_REG_OFFSET_DESC0);
> +	moxart_desc_write(txdes1, desc + TX_REG_OFFSET_DESC1);
> +	moxart_desc_write(TX_DESC0_DMA_OWN, desc + TX_REG_OFFSET_DESC0);

Those last two writes must happen in that order.
There may be others.

	David

[PATCH 4/9] net: moxart: use correct accessors for DMA memory

[David Laight]

From: Arnd Bergmann
> Sent: 27 January 2016 14:05
> The moxart ethernet driver confuses coherent DMA buffers with
> MMIO registers.
> 
> moxart_ether.c: In function 'moxart_mac_setup_desc_ring':
> moxart_ether.c:146:428: error: passing argument 1 of '__fswab32' makes integer from pointer without a
> cast [-Werror=int-conversion]
> moxart_ether.c:74:39: warning: incorrect type in argument 3 (different address spaces)
> moxart_ether.c:74:39:    expected void *cpu_addr
> moxart_ether.c:74:39:    got void [noderef] <asn:2>*tx_desc_base
> 
> This leaves the basic logic alone and uses normal pointers for
> the virtual address of the descriptor. As we cannot use readl/writel
> to access them, we also introduce our own moxart_desc_read
> moxart_desc_write helpers that perform the same endianess swap
> as the original code, but without the extra barriers and address
> space conversion.

I'm pretty sure you need to add some explicit barriers:

> @@ -354,8 +364,8 @@ static int moxart_mac_start_xmit(struct sk_buff *skb, struct net_device *ndev)
>  	txdes1 = TX_DESC1_LTS | TX_DESC1_FTS | (len & TX_DESC1_BUF_SIZE_MASK);
>  	if (tx_head == TX_DESC_NUM_MASK)
>  		txdes1 |= TX_DESC1_END;
> -	writel(txdes1, desc + TX_REG_OFFSET_DESC1);
> -	writel(TX_DESC0_DMA_OWN, desc + TX_REG_OFFSET_DESC0);
> +	moxart_desc_write(txdes1, desc + TX_REG_OFFSET_DESC1);
> +	moxart_desc_write(TX_DESC0_DMA_OWN, desc + TX_REG_OFFSET_DESC0);

Those last two writes must happen in that order.
There may be others.

	David

Re: [PATCH 7/9] net: macb: avoid uninitialized variables

[Sergei Shtylyov]

Hello.

On 1/27/2016 5:04 PM, Arnd Bergmann wrote:

> The macb_clk_init function returns three clock pointers, unless
> the it fails to get the first ones. We correctly handle the

    s/the//.

> failure case by propagating the error from macb_probe, but
> gcc does not realize this and incorrectly warns about a later
> use of those:
>
> In file included from /git/arm-soc/drivers/net/ethernet/cadence/macb.c:12:0:
> drivers/net/ethernet/cadence/macb.c: In function 'macb_probe':

    Hm, didn't these 2 lines get swapped by chance?

> include/linux/clk.h:484:2: error: 'tx_clk' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>    clk_disable(clk);
>    ^
> drivers/net/ethernet/cadence/macb.c:2822:28: note: 'tx_clk' was declared here
>    struct clk *pclk, *hclk, *tx_clk;
>                              ^
> In file included from /git/arm-soc/drivers/net/ethernet/cadence/macb.c:12:0:
> include/linux/clk.h:484:2: error: 'hclk' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>    clk_disable(clk);
>    ^
> drivers/net/ethernet/cadence/macb.c:2822:21: note: 'hclk' was declared here
>    struct clk *pclk, *hclk, *tx_clk;
>                       ^
>
> This shuts up the misleading warnings by ensuring that the
> macb_clk_init() always stores something into all three pointers.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

[...]

MBR, Sergei

[PATCH 7/9] net: macb: avoid uninitialized variables

[Sergei Shtylyov]

Hello.

On 1/27/2016 5:04 PM, Arnd Bergmann wrote:

> The macb_clk_init function returns three clock pointers, unless
> the it fails to get the first ones. We correctly handle the

    s/the//.

> failure case by propagating the error from macb_probe, but
> gcc does not realize this and incorrectly warns about a later
> use of those:
>
> In file included from /git/arm-soc/drivers/net/ethernet/cadence/macb.c:12:0:
> drivers/net/ethernet/cadence/macb.c: In function 'macb_probe':

    Hm, didn't these 2 lines get swapped by chance?

> include/linux/clk.h:484:2: error: 'tx_clk' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>    clk_disable(clk);
>    ^
> drivers/net/ethernet/cadence/macb.c:2822:28: note: 'tx_clk' was declared here
>    struct clk *pclk, *hclk, *tx_clk;
>                              ^
> In file included from /git/arm-soc/drivers/net/ethernet/cadence/macb.c:12:0:
> include/linux/clk.h:484:2: error: 'hclk' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>    clk_disable(clk);
>    ^
> drivers/net/ethernet/cadence/macb.c:2822:21: note: 'hclk' was declared here
>    struct clk *pclk, *hclk, *tx_clk;
>                       ^
>
> This shuts up the misleading warnings by ensuring that the
> macb_clk_init() always stores something into all three pointers.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

[...]

MBR, Sergei

Re: [PATCH 7/9] net: macb: avoid uninitialized variables

[Arnd Bergmann]

On Wednesday 27 January 2016 17:04:47 Nicolas Ferre wrote:
> > 
> > Okay Arnd, thanks!
> > 
> > Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> 
> Oh, crap: actually this warning has just been fixed by Sudip Mukherjee
> and is already queued by David here:
> https://patchwork.ozlabs.org/patch/572610/
> 
> So, sorry, I've shot too fast: NACK...

Ok, thanks for pointing this out.

	Arnd

[PATCH 7/9] net: macb: avoid uninitialized variables

[Arnd Bergmann]

On Wednesday 27 January 2016 17:04:47 Nicolas Ferre wrote:
> > 
> > Okay Arnd, thanks!
> > 
> > Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> 
> Oh, crap: actually this warning has just been fixed by Sudip Mukherjee
> and is already queued by David here:
> https://patchwork.ozlabs.org/patch/572610/
> 
> So, sorry, I've shot too fast: NACK...

Ok, thanks for pointing this out.

	Arnd

Re: [PATCH 4/9] net: moxart: use correct accessors for DMA memory

[Arnd Bergmann]

On Thursday 28 January 2016 12:36:19 David Laight wrote:
> From: Arnd Bergmann
> > Sent: 27 January 2016 14:05
> > The moxart ethernet driver confuses coherent DMA buffers with
> > MMIO registers.
> > 
> > moxart_ether.c: In function 'moxart_mac_setup_desc_ring':
> > moxart_ether.c:146:428: error: passing argument 1 of '__fswab32' makes integer from pointer without a
> > cast [-Werror=int-conversion]
> > moxart_ether.c:74:39: warning: incorrect type in argument 3 (different address spaces)
> > moxart_ether.c:74:39:    expected void *cpu_addr
> > moxart_ether.c:74:39:    got void [noderef] <asn:2>*tx_desc_base
> > 
> > This leaves the basic logic alone and uses normal pointers for
> > the virtual address of the descriptor. As we cannot use readl/writel
> > to access them, we also introduce our own moxart_desc_read
> > moxart_desc_write helpers that perform the same endianess swap
> > as the original code, but without the extra barriers and address
> > space conversion.
> 
> I'm pretty sure you need to add some explicit barriers:
> 
> > @@ -354,8 +364,8 @@ static int moxart_mac_start_xmit(struct sk_buff *skb, struct net_device *ndev)
> >       txdes1 = TX_DESC1_LTS | TX_DESC1_FTS | (len & TX_DESC1_BUF_SIZE_MASK);
> >       if (tx_head == TX_DESC_NUM_MASK)
> >               txdes1 |= TX_DESC1_END;
> > -     writel(txdes1, desc + TX_REG_OFFSET_DESC1);
> > -     writel(TX_DESC0_DMA_OWN, desc + TX_REG_OFFSET_DESC0);
> > +     moxart_desc_write(txdes1, desc + TX_REG_OFFSET_DESC1);
> > +     moxart_desc_write(TX_DESC0_DMA_OWN, desc + TX_REG_OFFSET_DESC0);
> 
> Those last two writes must happen in that order.
> There may be others.

Makes sense. I looked at the ftmac100 driver, which is another driver
for the same hardware, and it's also missing barriers. We should
probably add them for both then.

I think for the SoC that uses this, a barrier() would be sufficient
because of the page flags that dma_alloc_coherent() uses on ARM for
non-coherent platforms, but to be on the safe side we need a full
rmb()/wmb(). Sending a version 2 now.

	Arnd

[PATCH 4/9] net: moxart: use correct accessors for DMA memory

[Arnd Bergmann]

On Thursday 28 January 2016 12:36:19 David Laight wrote:
> From: Arnd Bergmann
> > Sent: 27 January 2016 14:05
> > The moxart ethernet driver confuses coherent DMA buffers with
> > MMIO registers.
> > 
> > moxart_ether.c: In function 'moxart_mac_setup_desc_ring':
> > moxart_ether.c:146:428: error: passing argument 1 of '__fswab32' makes integer from pointer without a
> > cast [-Werror=int-conversion]
> > moxart_ether.c:74:39: warning: incorrect type in argument 3 (different address spaces)
> > moxart_ether.c:74:39:    expected void *cpu_addr
> > moxart_ether.c:74:39:    got void [noderef] <asn:2>*tx_desc_base
> > 
> > This leaves the basic logic alone and uses normal pointers for
> > the virtual address of the descriptor. As we cannot use readl/writel
> > to access them, we also introduce our own moxart_desc_read
> > moxart_desc_write helpers that perform the same endianess swap
> > as the original code, but without the extra barriers and address
> > space conversion.
> 
> I'm pretty sure you need to add some explicit barriers:
> 
> > @@ -354,8 +364,8 @@ static int moxart_mac_start_xmit(struct sk_buff *skb, struct net_device *ndev)
> >       txdes1 = TX_DESC1_LTS | TX_DESC1_FTS | (len & TX_DESC1_BUF_SIZE_MASK);
> >       if (tx_head == TX_DESC_NUM_MASK)
> >               txdes1 |= TX_DESC1_END;
> > -     writel(txdes1, desc + TX_REG_OFFSET_DESC1);
> > -     writel(TX_DESC0_DMA_OWN, desc + TX_REG_OFFSET_DESC0);
> > +     moxart_desc_write(txdes1, desc + TX_REG_OFFSET_DESC1);
> > +     moxart_desc_write(TX_DESC0_DMA_OWN, desc + TX_REG_OFFSET_DESC0);
> 
> Those last two writes must happen in that order.
> There may be others.

Makes sense. I looked at the ftmac100 driver, which is another driver
for the same hardware, and it's also missing barriers. We should
probably add them for both then.

I think for the SoC that uses this, a barrier() would be sufficient
because of the page flags that dma_alloc_coherent() uses on ARM for
non-coherent platforms, but to be on the safe side we need a full
rmb()/wmb(). Sending a version 2 now.

	Arnd

Re: [PATCH 4/9] net: moxart: use correct accessors for DMA memory

[Arnd Bergmann]

On Thursday 28 January 2016 17:53:43 Arnd Bergmann wrote:
> 
> Makes sense. I looked at the ftmac100 driver, which is another driver
> for the same hardware, and it's also missing barriers. We should
> probably add them for both then.
> 

Nevermind. That one has write barriers, but no read barriers. I'm assuming
that this intentional and won't follow up with another patch for ftmac100.

	Arnd

[PATCH 4/9] net: moxart: use correct accessors for DMA memory

[Arnd Bergmann]

On Thursday 28 January 2016 17:53:43 Arnd Bergmann wrote:
> 
> Makes sense. I looked at the ftmac100 driver, which is another driver
> for the same hardware, and it's also missing barriers. We should
> probably add them for both then.
> 

Nevermind. That one has write barriers, but no read barriers. I'm assuming
that this intentional and won't follow up with another patch for ftmac100.

	Arnd

Re: [PATCH 0/9] network driver fixes

[David Miller]

From: Arnd Bergmann <arnd@arndb.de>
Date: Wed, 27 Jan 2016 15:04:50 +0100

> These are all fixes for relatively harmless bugs that showed up
> in my randconfig testing, so they should not be needed for v4.5
> but get merged into net-next.
> 
> I've managed to address all 'uninitialized variable' warnings that
> I get in ARM randconfig kernels now, this series includes the
> last five I got in network drivers. They are often really annoying
> warnings but when we get new ones, they often are about actual
> bugs in corner cases, so I'm trying hard to eliminate the false
> positives here to get people to pay attention to added warnings.
> I've recently tried building with an older gcc and found tons more
> that are all bogus, this series only addresses the ones that
> gcc-5.2 finds.

Arnd I'm expecting a respin of this series to address with the
feedback you've been given.

Thanks.

[PATCH 0/9] network driver fixes

[David Miller]

From: Arnd Bergmann <arnd@arndb.de>
Date: Wed, 27 Jan 2016 15:04:50 +0100

> These are all fixes for relatively harmless bugs that showed up
> in my randconfig testing, so they should not be needed for v4.5
> but get merged into net-next.
> 
> I've managed to address all 'uninitialized variable' warnings that
> I get in ARM randconfig kernels now, this series includes the
> last five I got in network drivers. They are often really annoying
> warnings but when we get new ones, they often are about actual
> bugs in corner cases, so I'm trying hard to eliminate the false
> positives here to get people to pay attention to added warnings.
> I've recently tried building with an older gcc and found tons more
> that are all bogus, this series only addresses the ones that
> gcc-5.2 finds.

Arnd I'm expecting a respin of this series to address with the
feedback you've been given.

Thanks.

Re: [PATCH 0/9] network driver fixes

[Arnd Bergmann]

On Thursday 28 January 2016 16:14:13 David Miller wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> Date: Wed, 27 Jan 2016 15:04:50 +0100
> 
> > These are all fixes for relatively harmless bugs that showed up
> > in my randconfig testing, so they should not be needed for v4.5
> > but get merged into net-next.
> > 
> > I've managed to address all 'uninitialized variable' warnings that
> > I get in ARM randconfig kernels now, this series includes the
> > last five I got in network drivers. They are often really annoying
> > warnings but when we get new ones, they often are about actual
> > bugs in corner cases, so I'm trying hard to eliminate the false
> > positives here to get people to pay attention to added warnings.
> > I've recently tried building with an older gcc and found tons more
> > that are all bogus, this series only addresses the ones that
> > gcc-5.2 finds.
> 
> Arnd I'm expecting a respin of this series to address with the
> feedback you've been given.
> 

Done. I wasn't sure if you planned to pick up the patches
individually, thanks for the clarification.

	Arnd

[PATCH 0/9] network driver fixes

[Arnd Bergmann]

On Thursday 28 January 2016 16:14:13 David Miller wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> Date: Wed, 27 Jan 2016 15:04:50 +0100
> 
> > These are all fixes for relatively harmless bugs that showed up
> > in my randconfig testing, so they should not be needed for v4.5
> > but get merged into net-next.
> > 
> > I've managed to address all 'uninitialized variable' warnings that
> > I get in ARM randconfig kernels now, this series includes the
> > last five I got in network drivers. They are often really annoying
> > warnings but when we get new ones, they often are about actual
> > bugs in corner cases, so I'm trying hard to eliminate the false
> > positives here to get people to pay attention to added warnings.
> > I've recently tried building with an older gcc and found tons more
> > that are all bogus, this series only addresses the ones that
> > gcc-5.2 finds.
> 
> Arnd I'm expecting a respin of this series to address with the
> feedback you've been given.
> 

Done. I wasn't sure if you planned to pick up the patches
individually, thanks for the clarification.

	Arnd