[PATCH v1 0/6] net/macb: fix for AVR32 and clean up

[PATCH v1 0/6] net/macb: fix for AVR32 and clean up

[Andy Shevchenko]

It seems no one had tested recently the driver on AVR32 platforms such as
ATNGW100. This series bring it back to work.

Andy Shevchenko (6):
  net/macb: improve big endian CPU support
  net/macb: check if macb_config present
  net/macb: use dev_*() when netdev is not yet registered
  net/macb: suppress compiler warnings
  net/macb: replace macb_count_tx_descriptors() by DIV_ROUND_UP()
  net/macb: convert to kernel doc

 drivers/net/ethernet/cadence/macb.c | 125 ++++++++++++++++++++++++------------
 drivers/net/ethernet/cadence/macb.h |  34 ++++------
 include/linux/platform_data/macb.h  |  14 +++-
 3 files changed, 108 insertions(+), 65 deletions(-)

-- 
2.4.6

[PATCH v1 1/6] net/macb: improve big endian CPU support

[Andy Shevchenko]

The commit a50dad355a53 (net: macb: Add big endian CPU support) converted I/O
accessors to readl_relaxed() and writel_relaxed() and consequentially broke
MACB driver on AVR32 platforms such as ATNGW100.

This patch improves I/O access by checking endiannes first and use the
corresponding methods.

Fixes: a50dad355a53 (net: macb: Add big endian CPU support)
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/net/ethernet/cadence/macb.c | 103 ++++++++++++++++++++++++++----------
 drivers/net/ethernet/cadence/macb.h |  28 ++++------
 2 files changed, 87 insertions(+), 44 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index a4e3f86..6980115 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -104,6 +104,57 @@ static void *macb_rx_buffer(struct macb *bp, unsigned int index)
 	return bp->rx_buffers + bp->rx_buffer_size * macb_rx_ring_wrap(index);
 }
 
+/* I/O accessors */
+static u32 hw_readl_native(struct macb *bp, int offset)
+{
+	return __raw_readl(bp->regs + offset);
+}
+
+static void hw_writel_native(struct macb *bp, int offset, u32 value)
+{
+	__raw_writel(value, bp->regs + offset);
+}
+
+static u32 hw_readl(struct macb *bp, int offset)
+{
+	return readl_relaxed(bp->regs + offset);
+}
+
+static void hw_writel(struct macb *bp, int offset, u32 value)
+{
+	writel_relaxed(value, bp->regs + offset);
+}
+
+/*
+ * Find the CPU endianness by using the loopback bit of NCR register. When the
+ * CPU is in big endian we need to program swaped mode for management
+ * descriptor access.
+ */
+static bool hw_is_native_io(void __iomem *addr)
+{
+	u32 value = MACB_BIT(LLB);
+
+	__raw_writel(value, addr + MACB_NCR);
+	value = __raw_readl(addr + MACB_NCR);
+
+	/* Write 0 back to disable everything */
+	__raw_writel(0, addr + MACB_NCR);
+
+	return value == MACB_BIT(LLB);
+}
+
+static bool hw_is_gem(void __iomem *addr, bool native_io)
+{
+	u32 id;
+
+	if (native_io)
+		id = __raw_readl(addr + MACB_MID);
+	else
+		id = readl_relaxed(addr + MACB_MID);
+
+	return MACB_BFEXT(IDNUM, id) >= 0x2;
+}
+
 static void macb_set_hwaddr(struct macb *bp)
 {
 	u32 bottom;
@@ -449,14 +500,14 @@ err_out:
 
 static void macb_update_stats(struct macb *bp)
 {
-	u32 __iomem *reg = bp->regs + MACB_PFR;
 	u32 *p = &bp->hw_stats.macb.rx_pause_frames;
 	u32 *end = &bp->hw_stats.macb.tx_pause_frames + 1;
+	int offset = MACB_PFR;
 
 	WARN_ON((unsigned long)(end - p - 1) != (MACB_TPF - MACB_PFR) / 4);
 
-	for(; p < end; p++, reg++)
-		*p += readl_relaxed(reg);
+	for(; p < end; p++, offset += 4)
+		*p += bp->readl(bp, offset);
 }
 
 static int macb_halt_tx(struct macb *bp)
@@ -1603,7 +1654,6 @@ static u32 macb_dbw(struct macb *bp)
 static void macb_configure_dma(struct macb *bp)
 {
 	u32 dmacfg;
-	u32 tmp, ncr;
 
 	if (macb_is_gem(bp)) {
 		dmacfg = gem_readl(bp, DMACFG) & ~GEM_BF(RXBS, -1L);
@@ -1613,22 +1663,11 @@ static void macb_configure_dma(struct macb *bp)
 		dmacfg |= GEM_BIT(TXPBMS) | GEM_BF(RXBMS, -1L);
 		dmacfg &= ~GEM_BIT(ENDIA_PKT);
 
-		/* Find the CPU endianness by using the loopback bit of net_ctrl
-		 * register. save it first. When the CPU is in big endian we
-		 * need to program swaped mode for management descriptor access.
-		 */
-		ncr = macb_readl(bp, NCR);
-		__raw_writel(MACB_BIT(LLB), bp->regs + MACB_NCR);
-		tmp =  __raw_readl(bp->regs + MACB_NCR);
-
-		if (tmp == MACB_BIT(LLB))
+		if (bp->native_io)
 			dmacfg &= ~GEM_BIT(ENDIA_DESC);
 		else
 			dmacfg |= GEM_BIT(ENDIA_DESC); /* CPU in big endian */
 
-		/* Restore net_ctrl */
-		macb_writel(bp, NCR, ncr);
-
 		if (bp->dev->features & NETIF_F_HW_CSUM)
 			dmacfg |= GEM_BIT(TXCOEN);
 		else
@@ -1902,14 +1941,14 @@ static void gem_update_stats(struct macb *bp)
 
 	for (i = 0; i < GEM_STATS_LEN; ++i, ++p) {
 		u32 offset = gem_statistics[i].offset;
-		u64 val = readl_relaxed(bp->regs + offset);
+		u64 val = bp->readl(bp, offset);
 
 		bp->ethtool_stats[i] += val;
 		*p += val;
 
 		if (offset == GEM_OCTTXL || offset == GEM_OCTRXL) {
 			/* Add GEM_OCTTXH, GEM_OCTRXH */
-			val = readl_relaxed(bp->regs + offset + 4);
+			val = bp->readl(bp, offset + 4);
 			bp->ethtool_stats[i] += ((u64)val) << 32;
 			*(++p) += val;
 		}
@@ -2190,7 +2229,7 @@ static void macb_configure_caps(struct macb *bp, const struct macb_config *dt_co
 	if (dt_conf)
 		bp->caps = dt_conf->caps;
 
-	if (macb_is_gem_hw(bp->regs)) {
+	if (hw_is_gem(bp->regs, bp->native_io)) {
 		bp->caps |= MACB_CAPS_MACB_IS_GEM;
 
 		dcfg = gem_readl(bp, DCFG1);
@@ -2205,6 +2244,7 @@ static void macb_configure_caps(struct macb *bp, const struct macb_config *dt_co
 }
 
 static void macb_probe_queues(void __iomem *mem,
+			      bool native_io,
 			      unsigned int *queue_mask,
 			      unsigned int *num_queues)
 {
@@ -2219,7 +2259,7 @@ static void macb_probe_queues(void __iomem *mem,
 	 * we are early in the probe process and don't have the
 	 * MACB_CAPS_MACB_IS_GEM flag positioned
 	 */
-	if (!macb_is_gem_hw(mem))
+	if (!hw_is_gem(mem, native_io))
 		return;
 
 	/* bit 0 is never set but queue 0 always exists */
@@ -2784,6 +2824,7 @@ static int macb_probe(struct platform_device *pdev)
 	struct clk *pclk, *hclk, *tx_clk;
 	unsigned int queue_mask, num_queues;
 	struct macb_platform_data *pdata;
+	bool native_io;
 	struct phy_device *phydev;
 	struct net_device *dev;
 	struct resource *regs;
@@ -2792,6 +2833,11 @@ static int macb_probe(struct platform_device *pdev)
 	struct macb *bp;
 	int err;
 
+	regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	mem = devm_ioremap_resource(&pdev->dev, regs);
+	if (IS_ERR(mem))
+		return PTR_ERR(mem);
+
 	if (np) {
 		const struct of_device_id *match;
 
@@ -2807,14 +2853,9 @@ static int macb_probe(struct platform_device *pdev)
 	if (err)
 		return err;
 
-	regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	mem = devm_ioremap_resource(&pdev->dev, regs);
-	if (IS_ERR(mem)) {
-		err = PTR_ERR(mem);
-		goto err_disable_clocks;
-	}
+	native_io = hw_is_native_io(mem);
 
-	macb_probe_queues(mem, &queue_mask, &num_queues);
+	macb_probe_queues(mem, native_io, &queue_mask, &num_queues);
 	dev = alloc_etherdev_mq(sizeof(*bp), num_queues);
 	if (!dev) {
 		err = -ENOMEM;
@@ -2829,6 +2870,14 @@ static int macb_probe(struct platform_device *pdev)
 	bp->pdev = pdev;
 	bp->dev = dev;
 	bp->regs = mem;
+	bp->native_io = native_io;
+	if (native_io) {
+		bp->readl = hw_readl_native;
+		bp->writel = hw_writel_native;
+	} else {
+		bp->readl = hw_readl;
+		bp->writel = hw_writel;
+	}
 	bp->num_queues = num_queues;
 	bp->queue_mask = queue_mask;
 	if (macb_config)
diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index d746559..f245340 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -429,18 +429,12 @@
 	 | GEM_BF(name, value))
 
 /* Register access macros */
-#define macb_readl(port,reg)				\
-	readl_relaxed((port)->regs + MACB_##reg)
-#define macb_writel(port,reg,value)			\
-	writel_relaxed((value), (port)->regs + MACB_##reg)
-#define gem_readl(port, reg)				\
-	readl_relaxed((port)->regs + GEM_##reg)
-#define gem_writel(port, reg, value)			\
-	writel_relaxed((value), (port)->regs + GEM_##reg)
-#define queue_readl(queue, reg)				\
-	readl_relaxed((queue)->bp->regs + (queue)->reg)
-#define queue_writel(queue, reg, value)			\
-	writel_relaxed((value), (queue)->bp->regs + (queue)->reg)
+#define macb_readl(port, reg)		(port)->readl((port), MACB_##reg)
+#define macb_writel(port, reg, value)	(port)->writel((port), MACB_##reg, (value))
+#define gem_readl(port, reg)		(port)->readl((port), GEM_##reg)
+#define gem_writel(port, reg, value)	(port)->writel((port), GEM_##reg, (value))
+#define queue_readl(queue, reg)		(queue)->bp->readl((queue)->bp, (queue)->reg)
+#define queue_writel(queue, reg, value)	(queue)->bp->writel((queue)->bp, (queue)->reg, (value))
 
 /* Conditional GEM/MACB macros.  These perform the operation to the correct
  * register dependent on whether the device is a GEM or a MACB.  For registers
@@ -785,6 +779,11 @@ struct macb_queue {
 
 struct macb {
 	void __iomem		*regs;
+	bool			native_io;
+
+	/* hardware IO accessors */
+	u32	(*readl)(struct macb *bp, int offset);
+	void	(*writel)(struct macb *bp, int offset, u32 value);
 
 	unsigned int		rx_tail;
 	unsigned int		rx_prepared_head;
@@ -843,9 +842,4 @@ static inline bool macb_is_gem(struct macb *bp)
 	return !!(bp->caps & MACB_CAPS_MACB_IS_GEM);
 }
 
-static inline bool macb_is_gem_hw(void __iomem *addr)
-{
-	return !!(MACB_BFEXT(IDNUM, readl_relaxed(addr + MACB_MID)) >= 0x2);
-}
-
 #endif /* _MACB_H */
-- 
2.4.6

[PATCH v1 2/6] net/macb: check if macb_config present

[Andy Shevchenko]

The commit 98b5a0f4a228 introduces jumbo frame support, but also it assumes
that macb_config present which is not always true.

The configuration without macb_config fails to boot.

 Unable to handle kernel NULL pointer dereference at virtual address 00000010
 ptbr = 90350000 pgd = 00000000
 Oops: Kernel access of bad area, sig: 11 [#1]
 FRAME_POINTER chip: 0x01f:0x1e82 rev 2
 Modules linked in:
 CPU: 0 PID: 1 Comm: swapper Not tainted 4.2.0-rc3-next-20150723+ #13
 task: 91c26000 ti: 91c28000 task.ti: 91c28000
 PC is at macb_probe+0x140/0x61c

Fixes: 98b5a0f4a228 (net: macb: Add support for jumbo frames)
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/net/ethernet/cadence/macb.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index 6980115..7986778 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -2885,9 +2885,8 @@ static int macb_probe(struct platform_device *pdev)
 	bp->pclk = pclk;
 	bp->hclk = hclk;
 	bp->tx_clk = tx_clk;
-	if (macb_config->jumbo_max_len) {
+	if (macb_config)
 		bp->jumbo_max_len = macb_config->jumbo_max_len;
-	}
 
 	spin_lock_init(&bp->lock);
 
-- 
2.4.6

[PATCH v1 3/6] net/macb: use dev_*() when netdev is not yet registered

[Andy Shevchenko]

To avoid messages like

macb macb.0 (unnamed net_device) (uninitialized): Cadence caps 0x00000000
macb macb.0 (unnamed net_device) (uninitialized): invalid hw address, using random

let's use dev_*() macros.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/net/ethernet/cadence/macb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index 7986778..367fc9d 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -211,7 +211,7 @@ static void macb_get_hwaddr(struct macb *bp)
 		}
 	}
 
-	netdev_info(bp->dev, "invalid hw address, using random\n");
+	dev_info(&bp->pdev->dev, "invalid hw address, using random\n");
 	eth_hw_addr_random(bp->dev);
 }
 
@@ -2240,7 +2240,7 @@ static void macb_configure_caps(struct macb *bp, const struct macb_config *dt_co
 			bp->caps |= MACB_CAPS_FIFO_MODE;
 	}
 
-	netdev_dbg(bp->dev, "Cadence caps 0x%08x\n", bp->caps);
+	dev_dbg(&bp->pdev->dev, "Cadence caps 0x%08x\n", bp->caps);
 }
 
 static void macb_probe_queues(void __iomem *mem,
-- 
2.4.6

[PATCH v1 4/6] net/macb: suppress compiler warnings

[Andy Shevchenko]

This patch fixes the following warnings:
drivers/net/ethernet/cadence/macb.c: In function ‘macb_handle_link_change’:
drivers/net/ethernet/cadence/macb.c:266: warning: comparison between signed and unsigned
drivers/net/ethernet/cadence/macb.c:267: warning: comparison between signed and unsigned
drivers/net/ethernet/cadence/macb.c:291: warning: comparison between signed and unsigned
drivers/net/ethernet/cadence/macb.c: In function ‘gem_update_stats’:
drivers/net/ethernet/cadence/macb.c:1908: warning: comparison between signed and unsigned
drivers/net/ethernet/cadence/macb.c: In function ‘gem_get_ethtool_strings’:
drivers/net/ethernet/cadence/macb.c:1988: warning: comparison between signed and unsigned

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/net/ethernet/cadence/macb.c | 5 ++---
 drivers/net/ethernet/cadence/macb.h | 6 +++---
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index 367fc9d..13d7e96 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -303,7 +303,6 @@ static void macb_handle_link_change(struct net_device *dev)
 	struct macb *bp = netdev_priv(dev);
 	struct phy_device *phydev = bp->phy_dev;
 	unsigned long flags;
-
 	int status_change = 0;
 
 	spin_lock_irqsave(&bp->lock, flags);
@@ -1936,7 +1935,7 @@ static int macb_change_mtu(struct net_device *dev, int new_mtu)
 
 static void gem_update_stats(struct macb *bp)
 {
-	int i;
+	unsigned int i;
 	u32 *p = &bp->hw_stats.gem.tx_octets_31_0;
 
 	for (i = 0; i < GEM_STATS_LEN; ++i, ++p) {
@@ -2015,7 +2014,7 @@ static int gem_get_sset_count(struct net_device *dev, int sset)
 
 static void gem_get_ethtool_strings(struct net_device *dev, u32 sset, u8 *p)
 {
-	int i;
+	unsigned int i;
 
 	switch (sset) {
 	case ETH_SS_STATS:
diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index f245340..2aa102e 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -816,9 +816,9 @@ struct macb {
 
 	struct mii_bus		*mii_bus;
 	struct phy_device	*phy_dev;
-	unsigned int 		link;
-	unsigned int 		speed;
-	unsigned int 		duplex;
+	int 			link;
+	int 			speed;
+	int 			duplex;
 
 	u32			caps;
 	unsigned int		dma_burst_length;
-- 
2.4.6

[PATCH v1 5/6] net/macb: replace macb_count_tx_descriptors() by DIV_ROUND_UP()

[Andy Shevchenko]

macb_count_tx_descriptors() repeats the generic macro DIV_ROUND_UP(). The patch
does a replacement.

There is no functional change.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/net/ethernet/cadence/macb.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index 13d7e96..5818c04 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -1157,12 +1157,6 @@ static void macb_poll_controller(struct net_device *dev)
 }
 #endif
 
-static inline unsigned int macb_count_tx_descriptors(struct macb *bp,
-						     unsigned int len)
-{
-	return (len + bp->max_tx_length - 1) / bp->max_tx_length;
-}
-
 static unsigned int macb_tx_map(struct macb *bp,
 				struct macb_queue *queue,
 				struct sk_buff *skb)
@@ -1313,11 +1307,11 @@ static int macb_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	 * socket buffer: skb fragments of jumbo frames may need to be
 	 * splitted into many buffer descriptors.
 	 */
-	count = macb_count_tx_descriptors(bp, skb_headlen(skb));
+	count = DIV_ROUND_UP(skb_headlen(skb), bp->max_tx_length);
 	nr_frags = skb_shinfo(skb)->nr_frags;
 	for (f = 0; f < nr_frags; f++) {
 		frag_size = skb_frag_size(&skb_shinfo(skb)->frags[f]);
-		count += macb_count_tx_descriptors(bp, frag_size);
+		count += DIV_ROUND_UP(frag_size, bp->max_tx_length);
 	}
 
 	spin_lock_irqsave(&bp->lock, flags);
-- 
2.4.6

[PATCH v1 6/6] net/macb: convert to kernel doc

[Andy Shevchenko]

This patch coverts struct description to the kernel doc format. There is no
functional change.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 include/linux/platform_data/macb.h | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/include/linux/platform_data/macb.h b/include/linux/platform_data/macb.h
index 044a124..21b15f6 100644
--- a/include/linux/platform_data/macb.h
+++ b/include/linux/platform_data/macb.h
@@ -8,11 +8,19 @@
 #ifndef __MACB_PDATA_H__
 #define __MACB_PDATA_H__
 
+/**
+ * struct macb_platform_data - platform data for MACB Ethernet
+ * @phy_mask:		phy mask passed when register the MDIO bus
+ *			within the driver
+ * @phy_irq_pin:	PHY IRQ
+ * @is_rmii:		using RMII interface?
+ * @rev_eth_addr:	reverse Ethernet address byte order
+ */
 struct macb_platform_data {
 	u32		phy_mask;
-	int		phy_irq_pin;	/* PHY IRQ */
-	u8		is_rmii;	/* using RMII interface? */
-	u8		rev_eth_addr;	/* reverse Ethernet address byte order */
+	int		phy_irq_pin;
+	u8		is_rmii;
+	u8		rev_eth_addr;
 };
 
 #endif /* __MACB_PDATA_H__ */
-- 
2.4.6

Re: [PATCH v1 0/6] net/macb: fix for AVR32 and clean up

[David Miller]

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Date: Fri, 24 Jul 2015 21:23:58 +0300

> It seems no one had tested recently the driver on AVR32 platforms
> such as ATNGW100. This series bring it back to work.

Series applied, thanks.

Re: [PATCH v1 0/6] net/macb: fix for AVR32 and clean up

[Nicolas Ferre]

Le 27/07/2015 10:10, David Miller a écrit :
> From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Date: Fri, 24 Jul 2015 21:23:58 +0300
> 
>> It seems no one had tested recently the driver on AVR32 platforms
>> such as ATNGW100. This series bring it back to work.
> 
> Series applied, thanks.

David, I would have liked to have more time to go through this series:
can you hold on a little bit?

Bye,
-- 
Nicolas Ferre

Re: [PATCH v1 0/6] net/macb: fix for AVR32 and clean up

[David Miller]

From: Nicolas Ferre <nicolas.ferre@atmel.com>
Date: Mon, 27 Jul 2015 10:18:22 +0200

> Le 27/07/2015 10:10, David Miller a écrit :
>> From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> Date: Fri, 24 Jul 2015 21:23:58 +0300
>> 
>>> It seems no one had tested recently the driver on AVR32 platforms
>>> such as ATNGW100. This series bring it back to work.
>> 
>> Series applied, thanks.
> 
> David, I would have liked to have more time to go through this series:
> can you hold on a little bit?

Too late it's already pushed out to my tree.  When I say "applied" it really
is committed to the permanent irreversable record in my public GIT tree,
so you must send me relative changes if you want anything fixes at this
point.