[PATCH -next v2 0/4] net: w5100: collection of small changes

[PATCH -next v2 0/4] net: w5100: collection of small changes

[Akinobu Mita]

This patch series is the collection of relatively small changes for
w5100 driver which includes a cleanup with no functional change,
two fixes, and adding a functionality.

* Changes from v1
- Remove the watchdong_timeo assignment to set default tx timeout,
  suggested by David Miller.

Akinobu Mita (4):
  net: w5100: remove unused is_w5200()
  net: w5100: fix MAC filtering for W5500
  net: w5100: increase TX timeout period
  net: w5100-spi: add support to specify MAC address by device tree

 drivers/net/ethernet/wiznet/w5100-spi.c |  4 +++-
 drivers/net/ethernet/wiznet/w5100.c     | 28 ++++++++++++++++------------
 drivers/net/ethernet/wiznet/w5100.h     |  3 ++-
 3 files changed, 21 insertions(+), 14 deletions(-)

Cc: Mike Sinkovsky <msink@permonline.ru>
Cc: David S. Miller <davem@davemloft.net>
-- 
2.7.4

[PATCH -next v2 1/4] net: w5100: remove unused is_w5200()

[Akinobu Mita]

The is_w5200() function is not used anymore by the commit which adds
the W5500 support.

Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: Mike Sinkovsky <msink@permonline.ru>
Cc: David S. Miller <davem@davemloft.net>
---
* No changes from v1

 drivers/net/ethernet/wiznet/w5100.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/net/ethernet/wiznet/w5100.c b/drivers/net/ethernet/wiznet/w5100.c
index 8ed0c77..56ceed9 100644
--- a/drivers/net/ethernet/wiznet/w5100.c
+++ b/drivers/net/ethernet/wiznet/w5100.c
@@ -173,11 +173,6 @@ struct w5100_priv {
 	struct work_struct restart_work;
 };
 
-static inline bool is_w5200(struct w5100_priv *priv)
-{
-	return priv->ops->chip_id == W5200;
-}
-
 /************************************************************************
  *
  *  Lowlevel I/O functions
-- 
2.7.4

[PATCH -next v2 2/4] net: w5100: fix MAC filtering for W5500

[Akinobu Mita]

W5500 has different bit position for MAC filter in Socket n mode
register from W5100 and W5200.

Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: Mike Sinkovsky <msink@permonline.ru>
Cc: David S. Miller <davem@davemloft.net>
---
* No changes from v1

 drivers/net/ethernet/wiznet/w5100.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/wiznet/w5100.c b/drivers/net/ethernet/wiznet/w5100.c
index 56ceed9..c80438c 100644
--- a/drivers/net/ethernet/wiznet/w5100.c
+++ b/drivers/net/ethernet/wiznet/w5100.c
@@ -63,8 +63,9 @@ MODULE_LICENSE("GPL");
 #define S0_REGS(priv)		((priv)->s0_regs)
 
 #define W5100_S0_MR(priv)	(S0_REGS(priv) + W5100_Sn_MR)
-#define   S0_MR_MACRAW		  0x04 /* MAC RAW mode (promiscuous) */
-#define   S0_MR_MACRAW_MF	  0x44 /* MAC RAW mode (filtered) */
+#define   S0_MR_MACRAW		  0x04 /* MAC RAW mode */
+#define   S0_MR_MF		  0x40 /* MAC Filter for W5100 and W5200 */
+#define   W5500_S0_MR_MF	  0x80 /* MAC Filter for W5500 */
 #define W5100_S0_CR(priv)	(S0_REGS(priv) + W5100_Sn_CR)
 #define   S0_CR_OPEN		  0x01 /* OPEN command */
 #define   S0_CR_CLOSE		  0x10 /* CLOSE command */
@@ -702,8 +703,16 @@ static int w5100_hw_reset(struct w5100_priv *priv)
 
 static void w5100_hw_start(struct w5100_priv *priv)
 {
-	w5100_write(priv, W5100_S0_MR(priv), priv->promisc ?
-			  S0_MR_MACRAW : S0_MR_MACRAW_MF);
+	u8 mode = S0_MR_MACRAW;
+
+	if (!priv->promisc) {
+		if (priv->ops->chip_id == W5500)
+			mode |= W5500_S0_MR_MF;
+		else
+			mode |= S0_MR_MF;
+	}
+
+	w5100_write(priv, W5100_S0_MR(priv), mode);
 	w5100_command(priv, S0_CR_OPEN);
 	w5100_enable_intr(priv);
 }
-- 
2.7.4

[PATCH -next v2 3/4] net: w5100: increase TX timeout period

[Akinobu Mita]

This increases TX timeout period from one second to 5 seconds which is
the default value if the driver doesn't explicitly set
net_device->watchdog_timeo.

The one second timeout is too short for W5100 with SPI interface mode
which doesn't support burst READ/WRITE processing in the SPI transfer.
If the packet is transmitted while RX packets are being received at a
very high rate, the TX transmittion work in the workqueue is delayed
and the watchdog timer is expired.

Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: Mike Sinkovsky <msink@permonline.ru>
Cc: David S. Miller <davem@davemloft.net>
---
* v2
- Remove the watchdong_timeo assignment to set default tx timeout,
  suggested by David Miller.

 drivers/net/ethernet/wiznet/w5100.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/wiznet/w5100.c b/drivers/net/ethernet/wiznet/w5100.c
index c80438c..43fdf88 100644
--- a/drivers/net/ethernet/wiznet/w5100.c
+++ b/drivers/net/ethernet/wiznet/w5100.c
@@ -1142,7 +1142,6 @@ int w5100_probe(struct device *dev, const struct w5100_ops *ops,
 
 	ndev->netdev_ops = &w5100_netdev_ops;
 	ndev->ethtool_ops = &w5100_ethtool_ops;
-	ndev->watchdog_timeo = HZ;
 	netif_napi_add(ndev, &priv->napi, w5100_napi_poll, 16);
 
 	/* This chip doesn't support VLAN packets with normal MTU,
-- 
2.7.4

RE: [PATCH -next v2 3/4] net: w5100: increase TX timeout period

[David Laight]

From: netdev-owner@vger.kernel.org Akinobu Mita
> Sent: 14 May 2016 06:56
> This increases TX timeout period from one second to 5 seconds which is
> the default value if the driver doesn't explicitly set
> net_device->watchdog_timeo.
> 
> The one second timeout is too short for W5100 with SPI interface mode
> which doesn't support burst READ/WRITE processing in the SPI transfer.
> If the packet is transmitted while RX packets are being received at a
> very high rate, the TX transmittion work in the workqueue is delayed
> and the watchdog timer is expired.

ISTM that if RX traffic can cause a 1 second timeout to expire I can
see no reason why it shouldn't also cause a 5 second timer to expire.

I'm guessing that something needs to be done to cause the SPI
block to discard receive frames (I guess the hardware will be discarding
them as well) in order to allow frames to be transmitted.

Given the 'damp string' nature of SPI, you might even want to give
transmits priority over receives.

	David

Re: [PATCH -next v2 3/4] net: w5100: increase TX timeout period

[Akinobu Mita]

2016-05-16 18:33 GMT+09:00 David Laight <David.Laight@aculab.com>:
> From: netdev-owner@vger.kernel.org Akinobu Mita
>> Sent: 14 May 2016 06:56
>> This increases TX timeout period from one second to 5 seconds which is
>> the default value if the driver doesn't explicitly set
>> net_device->watchdog_timeo.
>>
>> The one second timeout is too short for W5100 with SPI interface mode
>> which doesn't support burst READ/WRITE processing in the SPI transfer.
>> If the packet is transmitted while RX packets are being received at a
>> very high rate, the TX transmittion work in the workqueue is delayed
>> and the watchdog timer is expired.
>
> ISTM that if RX traffic can cause a 1 second timeout to expire I can
> see no reason why it shouldn't also cause a 5 second timer to expire.
>
> I'm guessing that something needs to be done to cause the SPI
> block to discard receive frames (I guess the hardware will be discarding
> them as well) in order to allow frames to be transmitted.

You are right.  In SPI interface mode, the interrupt of W5100 is masked
as far as receiving packets in w5100_rx_work(). (Ideally, RECV interrupt
should only be masked as W5200 and W5500 have different interrupt mask
bits for RECV and SENDOK, but W5100 doesn't have)

So this problem should be fixed by limiting the maximum execution time
of w5100_rx_skb() in w5100_rx_work() by maximum loop count or querying
jiffies.  I'll try it out.

> Given the 'damp string' nature of SPI, you might even want to give
> transmits priority over receives.

How can we do this?

[PATCH -next v2 4/4] net: w5100-spi: add support to specify MAC address by device tree

[Akinobu Mita]

This adds support to specify the MAC address by 'mac-address' or
'local-mac-address' properties in the device tree.  These are common
properties for the Ethernet controller.

Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: Mike Sinkovsky <msink@permonline.ru>
Cc: David S. Miller <davem@davemloft.net>
---
* No changes from v1

 drivers/net/ethernet/wiznet/w5100-spi.c | 4 +++-
 drivers/net/ethernet/wiznet/w5100.c     | 5 +++--
 drivers/net/ethernet/wiznet/w5100.h     | 3 ++-
 3 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/wiznet/w5100-spi.c b/drivers/net/ethernet/wiznet/w5100-spi.c
index b868e45..93a2d3c 100644
--- a/drivers/net/ethernet/wiznet/w5100-spi.c
+++ b/drivers/net/ethernet/wiznet/w5100-spi.c
@@ -15,6 +15,7 @@
 #include <linux/module.h>
 #include <linux/delay.h>
 #include <linux/netdevice.h>
+#include <linux/of_net.h>
 #include <linux/spi/spi.h>
 
 #include "w5100.h"
@@ -414,6 +415,7 @@ static int w5100_spi_probe(struct spi_device *spi)
 	const struct spi_device_id *id = spi_get_device_id(spi);
 	const struct w5100_ops *ops;
 	int priv_size;
+	const void *mac = of_get_mac_address(spi->dev.of_node);
 
 	switch (id->driver_data) {
 	case W5100:
@@ -432,7 +434,7 @@ static int w5100_spi_probe(struct spi_device *spi)
 		return -EINVAL;
 	}
 
-	return w5100_probe(&spi->dev, ops, priv_size, NULL, spi->irq, -EINVAL);
+	return w5100_probe(&spi->dev, ops, priv_size, mac, spi->irq, -EINVAL);
 }
 
 static int w5100_spi_remove(struct spi_device *spi)
diff --git a/drivers/net/ethernet/wiznet/w5100.c b/drivers/net/ethernet/wiznet/w5100.c
index 43fdf88..e9ae7e6 100644
--- a/drivers/net/ethernet/wiznet/w5100.c
+++ b/drivers/net/ethernet/wiznet/w5100.c
@@ -1052,7 +1052,7 @@ static const struct net_device_ops w5100_netdev_ops = {
 static int w5100_mmio_probe(struct platform_device *pdev)
 {
 	struct wiznet_platform_data *data = dev_get_platdata(&pdev->dev);
-	u8 *mac_addr = NULL;
+	const void *mac_addr = NULL;
 	struct resource *mem;
 	const struct w5100_ops *ops;
 	int irq;
@@ -1087,7 +1087,8 @@ void *w5100_ops_priv(const struct net_device *ndev)
 EXPORT_SYMBOL_GPL(w5100_ops_priv);
 
 int w5100_probe(struct device *dev, const struct w5100_ops *ops,
-		int sizeof_ops_priv, u8 *mac_addr, int irq, int link_gpio)
+		int sizeof_ops_priv, const void *mac_addr, int irq,
+		int link_gpio)
 {
 	struct w5100_priv *priv;
 	struct net_device *ndev;
diff --git a/drivers/net/ethernet/wiznet/w5100.h b/drivers/net/ethernet/wiznet/w5100.h
index f8a16fa..17983a3 100644
--- a/drivers/net/ethernet/wiznet/w5100.h
+++ b/drivers/net/ethernet/wiznet/w5100.h
@@ -30,7 +30,8 @@ struct w5100_ops {
 void *w5100_ops_priv(const struct net_device *ndev);
 
 int w5100_probe(struct device *dev, const struct w5100_ops *ops,
-		int sizeof_ops_priv, u8 *mac_addr, int irq, int link_gpio);
+		int sizeof_ops_priv, const void *mac_addr, int irq,
+		int link_gpio);
 int w5100_remove(struct device *dev);
 
 extern const struct dev_pm_ops w5100_pm_ops;
-- 
2.7.4

Re: [PATCH -next v2 0/4] net: w5100: collection of small changes

[David Miller]

From: Akinobu Mita <akinobu.mita@gmail.com>
Date: Sat, 14 May 2016 14:55:46 +0900

> This patch series is the collection of relatively small changes for
> w5100 driver which includes a cleanup with no functional change,
> two fixes, and adding a functionality.
> 
> * Changes from v1
> - Remove the watchdong_timeo assignment to set default tx timeout,
>   suggested by David Miller.

Series applied, thanks.