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

[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 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 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 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 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

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

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

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

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

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

[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

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

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

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

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