[PATCH v1 1/7] enc28j60: Use device_get_mac_address()

[PATCH v1 1/7] enc28j60: Use device_get_mac_address()

[Andy Shevchenko]

Replace the DT-specific of_get_mac_address() function with
device_get_mac_address, which works on both DT and ACPI platforms.  This
change makes it easier to add ACPI support.

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

diff --git a/drivers/net/ethernet/microchip/enc28j60.c b/drivers/net/ethernet/microchip/enc28j60.c
index 8f72587b5a2c..fa6d37c86315 100644
--- a/drivers/net/ethernet/microchip/enc28j60.c
+++ b/drivers/net/ethernet/microchip/enc28j60.c
@@ -18,6 +18,7 @@
 #include <linux/types.h>
 #include <linux/fcntl.h>
 #include <linux/interrupt.h>
+#include <linux/property.h>
 #include <linux/string.h>
 #include <linux/errno.h>
 #include <linux/init.h>
@@ -28,7 +29,6 @@
 #include <linux/skbuff.h>
 #include <linux/delay.h>
 #include <linux/spi/spi.h>
-#include <linux/of_net.h>
 
 #include "enc28j60_hw.h"
 
@@ -1554,7 +1554,7 @@ static int enc28j60_probe(struct spi_device *spi)
 {
 	struct net_device *dev;
 	struct enc28j60_net *priv;
-	const void *macaddr;
+	unsigned char macaddr[ETH_ALEN];
 	int ret = 0;
 
 	if (netif_msg_drv(&debug))
@@ -1587,8 +1587,7 @@ static int enc28j60_probe(struct spi_device *spi)
 		goto error_irq;
 	}
 
-	macaddr = of_get_mac_address(spi->dev.of_node);
-	if (macaddr)
+	if (device_get_mac_address(&spi->dev, macaddr, sizeof(macaddr)))
 		ether_addr_copy(dev->dev_addr, macaddr);
 	else
 		eth_hw_addr_random(dev);
-- 
2.20.1

[PATCH v1 2/7] enc28j60: Remove duplicate messaging

[Andy Shevchenko]

The ->probe() and ->remove() stages can be easily debugged with
initcall_debug or function tracer. There is no need to repeat the same
explicitly in the driver.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/net/ethernet/microchip/enc28j60.c | 20 --------------------
 1 file changed, 20 deletions(-)

diff --git a/drivers/net/ethernet/microchip/enc28j60.c b/drivers/net/ethernet/microchip/enc28j60.c
index fa6d37c86315..ebf9c0fd5b7d 100644
--- a/drivers/net/ethernet/microchip/enc28j60.c
+++ b/drivers/net/ethernet/microchip/enc28j60.c
@@ -186,9 +186,6 @@ static int spi_write_op(struct enc28j60_net *priv, u8 op,
 
 static void enc28j60_soft_reset(struct enc28j60_net *priv)
 {
-	if (netif_msg_hw(priv))
-		printk(KERN_DEBUG DRV_NAME ": %s() enter\n", __func__);
-
 	spi_write_op(priv, ENC28J60_SOFT_RESET, 0, ENC28J60_SOFT_RESET);
 	/* Errata workaround #1, CLKRDY check is unreliable,
 	 * delay at least 1 mS instead */
@@ -1124,8 +1121,6 @@ static void enc28j60_irq_work_handler(struct work_struct *work)
 	struct net_device *ndev = priv->netdev;
 	int intflags, loop;
 
-	if (netif_msg_intr(priv))
-		printk(KERN_DEBUG DRV_NAME ": %s() enter\n", __func__);
 	/* disable further interrupts */
 	locked_reg_bfclr(priv, EIE, EIE_INTIE);
 
@@ -1228,8 +1223,6 @@ static void enc28j60_irq_work_handler(struct work_struct *work)
 
 	/* re-enable interrupts */
 	locked_reg_bfset(priv, EIE, EIE_INTIE);
-	if (netif_msg_intr(priv))
-		printk(KERN_DEBUG DRV_NAME ": %s() exit\n", __func__);
 }
 
 /*
@@ -1287,9 +1280,6 @@ static netdev_tx_t enc28j60_send_packet(struct sk_buff *skb,
 {
 	struct enc28j60_net *priv = netdev_priv(dev);
 
-	if (netif_msg_tx_queued(priv))
-		printk(KERN_DEBUG DRV_NAME ": %s() enter\n", __func__);
-
 	/* If some error occurs while trying to transmit this
 	 * packet, you should return '1' from this function.
 	 * In such a case you _may not_ do anything to the
@@ -1356,9 +1346,6 @@ static int enc28j60_net_open(struct net_device *dev)
 {
 	struct enc28j60_net *priv = netdev_priv(dev);
 
-	if (netif_msg_drv(priv))
-		printk(KERN_DEBUG DRV_NAME ": %s() enter\n", __func__);
-
 	if (!is_valid_ether_addr(dev->dev_addr)) {
 		if (netif_msg_ifup(priv))
 			dev_err(&dev->dev, "invalid MAC address %pM\n",
@@ -1392,9 +1379,6 @@ static int enc28j60_net_close(struct net_device *dev)
 {
 	struct enc28j60_net *priv = netdev_priv(dev);
 
-	if (netif_msg_drv(priv))
-		printk(KERN_DEBUG DRV_NAME ": %s() enter\n", __func__);
-
 	enc28j60_hw_disable(priv);
 	enc28j60_lowpower(priv, true);
 	netif_stop_queue(dev);
@@ -1619,7 +1603,6 @@ static int enc28j60_probe(struct spi_device *spi)
 				" failed (ret = %d)\n", ret);
 		goto error_register;
 	}
-	dev_info(&dev->dev, DRV_NAME " driver registered\n");
 
 	return 0;
 
@@ -1635,9 +1618,6 @@ static int enc28j60_remove(struct spi_device *spi)
 {
 	struct enc28j60_net *priv = spi_get_drvdata(spi);
 
-	if (netif_msg_drv(priv))
-		printk(KERN_DEBUG DRV_NAME ": remove\n");
-
 	unregister_netdev(priv->netdev);
 	free_irq(spi->irq, priv);
 	free_netdev(priv->netdev);
-- 
2.20.1

[PATCH v1 3/7] enc28j60: Replace dev_*(&netdev->dev, ...) with netdev_*()

[Andy Shevchenko]

Replace open coded netdev_<level>() macros.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/net/ethernet/microchip/enc28j60.c | 42 ++++++++++-------------
 1 file changed, 18 insertions(+), 24 deletions(-)

diff --git a/drivers/net/ethernet/microchip/enc28j60.c b/drivers/net/ethernet/microchip/enc28j60.c
index ebf9c0fd5b7d..fefd0ab068aa 100644
--- a/drivers/net/ethernet/microchip/enc28j60.c
+++ b/drivers/net/ethernet/microchip/enc28j60.c
@@ -790,14 +790,12 @@ enc28j60_setlink(struct net_device *ndev, u8 autoneg, u16 speed, u8 duplex)
 			priv->full_duplex = (duplex == DUPLEX_FULL);
 		else {
 			if (netif_msg_link(priv))
-				dev_warn(&ndev->dev,
-					"unsupported link setting\n");
+				netdev_warn(ndev, "unsupported link setting\n");
 			ret = -EOPNOTSUPP;
 		}
 	} else {
 		if (netif_msg_link(priv))
-			dev_warn(&ndev->dev, "Warning: hw must be disabled "
-				"to set link mode\n");
+			netdev_warn(ndev, "Warning: hw must be disabled to set link mode\n");
 		ret = -EBUSY;
 	}
 	return ret;
@@ -912,9 +910,8 @@ static void enc28j60_hw_rx(struct net_device *ndev)
 
 	if (unlikely(priv->next_pk_ptr > RXEND_INIT)) {
 		if (netif_msg_rx_err(priv))
-			dev_err(&ndev->dev,
-				"%s() Invalid packet address!! 0x%04x\n",
-				__func__, priv->next_pk_ptr);
+			netdev_err(ndev, "%s() Invalid packet address!! 0x%04x\n",
+				   __func__, priv->next_pk_ptr);
 		/* packet address corrupted: reset RX logic */
 		mutex_lock(&priv->lock);
 		nolock_reg_bfclr(priv, ECON1, ECON1_RXEN);
@@ -947,7 +944,7 @@ static void enc28j60_hw_rx(struct net_device *ndev)
 
 	if (!RSV_GETBIT(rxstat, RSV_RXOK) || len > MAX_FRAMELEN) {
 		if (netif_msg_rx_err(priv))
-			dev_err(&ndev->dev, "Rx Error (%04x)\n", rxstat);
+			netdev_err(ndev, "Rx Error (%04x)\n", rxstat);
 		ndev->stats.rx_errors++;
 		if (RSV_GETBIT(rxstat, RSV_CRCERROR))
 			ndev->stats.rx_crc_errors++;
@@ -959,8 +956,7 @@ static void enc28j60_hw_rx(struct net_device *ndev)
 		skb = netdev_alloc_skb(ndev, len + NET_IP_ALIGN);
 		if (!skb) {
 			if (netif_msg_rx_err(priv))
-				dev_err(&ndev->dev,
-					"out of memory for Rx'd frame\n");
+				netdev_err(ndev, "out of memory for Rx'd frame\n");
 			ndev->stats.rx_dropped++;
 		} else {
 			skb_reserve(skb, NET_IP_ALIGN);
@@ -1056,11 +1052,11 @@ static void enc28j60_check_link_status(struct net_device *ndev)
 	if (reg & PHSTAT2_LSTAT) {
 		netif_carrier_on(ndev);
 		if (netif_msg_ifup(priv))
-			dev_info(&ndev->dev, "link up - %s\n",
-				duplex ? "Full duplex" : "Half duplex");
+			netdev_info(ndev, "link up - %s\n",
+				    duplex ? "Full duplex" : "Half duplex");
 	} else {
 		if (netif_msg_ifdown(priv))
-			dev_info(&ndev->dev, "link down\n");
+			netdev_info(ndev, "link down\n");
 		netif_carrier_off(ndev);
 	}
 }
@@ -1156,8 +1152,7 @@ static void enc28j60_irq_work_handler(struct work_struct *work)
 			priv->tx_retry_count = 0;
 			if (locked_regb_read(priv, ESTAT) & ESTAT_TXABRT) {
 				if (netif_msg_tx_err(priv))
-					dev_err(&ndev->dev,
-						"Tx Error (aborted)\n");
+					netdev_err(ndev, "Tx Error (aborted)\n");
 				err = true;
 			}
 			if (netif_msg_tx_done(priv)) {
@@ -1327,7 +1322,7 @@ static void enc28j60_tx_timeout(struct net_device *ndev)
 	struct enc28j60_net *priv = netdev_priv(ndev);
 
 	if (netif_msg_timer(priv))
-		dev_err(&ndev->dev, DRV_NAME " tx timeout\n");
+		netdev_err(ndev, "tx timeout\n");
 
 	ndev->stats.tx_errors++;
 	/* can't restart safely under softirq */
@@ -1348,8 +1343,7 @@ static int enc28j60_net_open(struct net_device *dev)
 
 	if (!is_valid_ether_addr(dev->dev_addr)) {
 		if (netif_msg_ifup(priv))
-			dev_err(&dev->dev, "invalid MAC address %pM\n",
-				dev->dev_addr);
+			netdev_err(dev, "invalid MAC address %pM\n", dev->dev_addr);
 		return -EADDRNOTAVAIL;
 	}
 	/* Reset the hardware here (and take it out of low power mode) */
@@ -1357,7 +1351,7 @@ static int enc28j60_net_open(struct net_device *dev)
 	enc28j60_hw_disable(priv);
 	if (!enc28j60_hw_init(priv)) {
 		if (netif_msg_ifup(priv))
-			dev_err(&dev->dev, "hw_reset() failed\n");
+			netdev_err(dev, "hw_reset() failed\n");
 		return -EINVAL;
 	}
 	/* Update the MAC address (in case user has changed it) */
@@ -1399,16 +1393,16 @@ static void enc28j60_set_multicast_list(struct net_device *dev)
 
 	if (dev->flags & IFF_PROMISC) {
 		if (netif_msg_link(priv))
-			dev_info(&dev->dev, "promiscuous mode\n");
+			netdev_info(dev, "promiscuous mode\n");
 		priv->rxfilter = RXFILTER_PROMISC;
 	} else if ((dev->flags & IFF_ALLMULTI) || !netdev_mc_empty(dev)) {
 		if (netif_msg_link(priv))
-			dev_info(&dev->dev, "%smulticast mode\n",
-				(dev->flags & IFF_ALLMULTI) ? "all-" : "");
+			netdev_info(dev, "%smulticast mode\n",
+				    (dev->flags & IFF_ALLMULTI) ? "all-" : "");
 		priv->rxfilter = RXFILTER_MULTI;
 	} else {
 		if (netif_msg_link(priv))
-			dev_info(&dev->dev, "normal mode\n");
+			netdev_info(dev, "normal mode\n");
 		priv->rxfilter = RXFILTER_NORMAL;
 	}
 
@@ -1452,7 +1446,7 @@ static void enc28j60_restart_work_handler(struct work_struct *work)
 		enc28j60_net_close(ndev);
 		ret = enc28j60_net_open(ndev);
 		if (unlikely(ret)) {
-			dev_info(&ndev->dev, " could not restart %d\n", ret);
+			netdev_info(ndev, "could not restart %d\n", ret);
 			dev_close(ndev);
 		}
 	}
-- 
2.20.1

[PATCH v1 4/7] enc28j60: Drop driver name duplication from messages

[Andy Shevchenko]

When dev_<level>() macros are used against SPI device, the driver's name
is printed as well. No need to duplicate this explicitly.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/net/ethernet/microchip/enc28j60.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/microchip/enc28j60.c b/drivers/net/ethernet/microchip/enc28j60.c
index fefd0ab068aa..c4e098009b83 100644
--- a/drivers/net/ethernet/microchip/enc28j60.c
+++ b/drivers/net/ethernet/microchip/enc28j60.c
@@ -1536,8 +1536,7 @@ static int enc28j60_probe(struct spi_device *spi)
 	int ret = 0;
 
 	if (netif_msg_drv(&debug))
-		dev_info(&spi->dev, DRV_NAME " Ethernet driver %s loaded\n",
-			DRV_VERSION);
+		dev_info(&spi->dev, "Ethernet driver %s loaded\n", DRV_VERSION);
 
 	dev = alloc_etherdev(sizeof(struct enc28j60_net));
 	if (!dev) {
@@ -1560,7 +1559,7 @@ static int enc28j60_probe(struct spi_device *spi)
 
 	if (!enc28j60_chipset_init(dev)) {
 		if (netif_msg_probe(priv))
-			dev_info(&spi->dev, DRV_NAME " chip not found\n");
+			dev_info(&spi->dev, "chip not found\n");
 		ret = -EIO;
 		goto error_irq;
 	}
@@ -1577,8 +1576,8 @@ static int enc28j60_probe(struct spi_device *spi)
 	ret = request_irq(spi->irq, enc28j60_irq, 0, DRV_NAME, priv);
 	if (ret < 0) {
 		if (netif_msg_probe(priv))
-			dev_err(&spi->dev, DRV_NAME ": request irq %d failed "
-				"(ret = %d)\n", spi->irq, ret);
+			dev_err(&spi->dev, "request irq %d failed (ret = %d)\n",
+				spi->irq, ret);
 		goto error_irq;
 	}
 
@@ -1593,8 +1592,8 @@ static int enc28j60_probe(struct spi_device *spi)
 	ret = register_netdev(dev);
 	if (ret) {
 		if (netif_msg_probe(priv))
-			dev_err(&spi->dev, "register netdev " DRV_NAME
-				" failed (ret = %d)\n", ret);
+			dev_err(&spi->dev, "register netdev failed (ret = %d)\n",
+				ret);
 		goto error_register;
 	}
 
-- 
2.20.1

[PATCH v1 5/7] enc28j60: Switch to use module_spi_driver() macro

[Andy Shevchenko]

Eliminate some boilerplate code by using module_spi_driver() instead of
init/exit, moving the salient bits from init into probe.

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

diff --git a/drivers/net/ethernet/microchip/enc28j60.c b/drivers/net/ethernet/microchip/enc28j60.c
index c4e098009b83..ad3748de972a 100644
--- a/drivers/net/ethernet/microchip/enc28j60.c
+++ b/drivers/net/ethernet/microchip/enc28j60.c
@@ -417,11 +417,9 @@ enc28j60_packet_write(struct enc28j60_net *priv, int len, const u8 *data)
 	mutex_unlock(&priv->lock);
 }
 
-static unsigned long msec20_to_jiffies;
-
 static int poll_ready(struct enc28j60_net *priv, u8 reg, u8 mask, u8 val)
 {
-	unsigned long timeout = jiffies + msec20_to_jiffies;
+	unsigned long timeout = jiffies + msecs_to_jiffies(20);
 
 	/* 20 msec timeout read */
 	while ((nolock_regb_read(priv, reg) & mask) != val) {
@@ -1632,22 +1630,7 @@ static struct spi_driver enc28j60_driver = {
 	.probe = enc28j60_probe,
 	.remove = enc28j60_remove,
 };
-
-static int __init enc28j60_init(void)
-{
-	msec20_to_jiffies = msecs_to_jiffies(20);
-
-	return spi_register_driver(&enc28j60_driver);
-}
-
-module_init(enc28j60_init);
-
-static void __exit enc28j60_exit(void)
-{
-	spi_unregister_driver(&enc28j60_driver);
-}
-
-module_exit(enc28j60_exit);
+module_spi_driver(enc28j60_driver);
 
 MODULE_DESCRIPTION(DRV_NAME " ethernet driver");
 MODULE_AUTHOR("Claudio Lanconelli <lanconelli.claudio@eptar.com>");
-- 
2.20.1

[PATCH v1 6/7] enc28j60: Remove linux/init.h

[Andy Shevchenko]

There is no need to include linux/init.h when at the same time
we include linux/module.h.

Remove redundant inclusion.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/net/ethernet/microchip/enc28j60.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/microchip/enc28j60.c b/drivers/net/ethernet/microchip/enc28j60.c
index ad3748de972a..133b5beb3749 100644
--- a/drivers/net/ethernet/microchip/enc28j60.c
+++ b/drivers/net/ethernet/microchip/enc28j60.c
@@ -21,7 +21,6 @@
 #include <linux/property.h>
 #include <linux/string.h>
 #include <linux/errno.h>
-#include <linux/init.h>
 #include <linux/netdevice.h>
 #include <linux/etherdevice.h>
 #include <linux/ethtool.h>
-- 
2.20.1

[PATCH v1 7/7] enc28j60: Convert to use SPDX identifier

[Andy Shevchenko]

Reduce size of duplicated comments by switching to use SPDX identifier.

No functional change.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/net/ethernet/microchip/enc28j60.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/microchip/enc28j60.c b/drivers/net/ethernet/microchip/enc28j60.c
index 133b5beb3749..58cf3ec0f9b3 100644
--- a/drivers/net/ethernet/microchip/enc28j60.c
+++ b/drivers/net/ethernet/microchip/enc28j60.c
@@ -1,3 +1,4 @@
+// SPDX-License-Identifier: GPL-2.0+
 /*
  * Microchip ENC28J60 ethernet driver (MAC + PHY)
  *
@@ -5,11 +6,6 @@
  * Author: Claudio Lanconelli <lanconelli.claudio@eptar.com>
  * based on enc28j60.c written by David Anders for 2.4 kernel version
  *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
  * $Id: enc28j60.c,v 1.22 2007/12/20 10:47:01 claudio Exp $
  */
 
-- 
2.20.1

Re: [PATCH v1 1/7] enc28j60: Use device_get_mac_address()

[David Miller]

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Date: Thu, 28 Feb 2019 23:34:29 +0300

> @@ -1554,7 +1554,7 @@ static int enc28j60_probe(struct spi_device *spi)
>  {
>  	struct net_device *dev;
>  	struct enc28j60_net *priv;
> -	const void *macaddr;
> +	unsigned char macaddr[ETH_ALEN];
>  	int ret = 0;

Preserve the reverse christmas tree here please.

Re: [PATCH v1 1/7] enc28j60: Use device_get_mac_address()

[Joe Perches]

On Fri, 2019-03-01 at 21:46 -0800, David Miller wrote:
> From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Date: Thu, 28 Feb 2019 23:34:29 +0300
> 
> > @@ -1554,7 +1554,7 @@ static int enc28j60_probe(struct spi_device *spi)
> >  {
> >       struct net_device *dev;
> >       struct enc28j60_net *priv;
> > -     const void *macaddr;
> > +     unsigned char macaddr[ETH_ALEN];
> >       int ret = 0;
> 
> Preserve the reverse christmas tree here please.

There is no reverse christmas tree here.