netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH RESEND net 0/9] Fixes, cleanup and modernization for some legacy ethernet NIC drivers
@ 2017-10-03  1:07 Finn Thain
  2017-10-03  1:07 ` [PATCH RESEND net 9/9] net/mac8390: Fix log messages Finn Thain
                   ` (9 more replies)
  0 siblings, 10 replies; 12+ messages in thread
From: Finn Thain @ 2017-10-03  1:07 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, linux-kernel

This patch series fixes some logging bugs and adds some missing message
severity levels.

There are also cleanup patches for dead code and some Kconfig cruft.

Custom debug message logging is converted to netif_* calls to reduce
code duplication.

All up, about 150 lines of code are eliminated.

My apologies for duplicated messages. I messed up the addressing.


Finn Thain (9):
  net/smc9194: Remove bogus CONFIG_MAC reference
  net/mac89x0: Remove dead or unreachable code
  net/mac89x0: Fix and modernize log messages
  net/mac89x0: Replace custom debug logging with netif_* calls
  net/macmace: Fix and cleanup log messages
  net/sonic: Cleanup and modernize log messages
  net/sonic: Replace custom debug logging with netif_* calls
  net/8390: Fix redundant code
  net/mac8390: Fix log messages

 drivers/net/ethernet/8390/ax88796.c      |   3 -
 drivers/net/ethernet/8390/axnet_cs.c     |   2 -
 drivers/net/ethernet/8390/etherh.c       |  17 -----
 drivers/net/ethernet/8390/hydra.c        |   4 --
 drivers/net/ethernet/8390/lib8390.c      |   2 +
 drivers/net/ethernet/8390/mac8390.c      |  42 +++++-------
 drivers/net/ethernet/8390/mcf8390.c      |   4 --
 drivers/net/ethernet/8390/pcnet_cs.c     |   4 --
 drivers/net/ethernet/8390/zorro8390.c    |   5 --
 drivers/net/ethernet/apple/macmace.c     |  10 +--
 drivers/net/ethernet/cirrus/mac89x0.c    | 109 ++++++++-----------------------
 drivers/net/ethernet/natsemi/jazzsonic.c |  26 ++------
 drivers/net/ethernet/natsemi/macsonic.c  |  45 ++++---------
 drivers/net/ethernet/natsemi/sonic.c     |  92 +++++++++++++-------------
 drivers/net/ethernet/natsemi/sonic.h     |   2 +
 drivers/net/ethernet/natsemi/xtsonic.c   |  28 ++------
 drivers/net/ethernet/smsc/Kconfig        |   2 +-
 17 files changed, 123 insertions(+), 274 deletions(-)

-- 
2.13.5

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH RESEND net 1/9] net/smc9194: Remove bogus CONFIG_MAC reference
  2017-10-03  1:07 [PATCH RESEND net 0/9] Fixes, cleanup and modernization for some legacy ethernet NIC drivers Finn Thain
                   ` (6 preceding siblings ...)
  2017-10-03  1:07 ` [PATCH RESEND net 4/9] net/mac89x0: Replace custom debug logging with netif_* calls Finn Thain
@ 2017-10-03  1:07 ` Finn Thain
  2017-10-03  1:07 ` [PATCH RESEND net 8/9] net/8390: Fix redundant code Finn Thain
  2017-10-04  4:41 ` [PATCH RESEND net 0/9] Fixes, cleanup and modernization for some legacy ethernet NIC drivers David Miller
  9 siblings, 0 replies; 12+ messages in thread
From: Finn Thain @ 2017-10-03  1:07 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, linux-kernel

The only version of smc9194.c with Mac support is the one in the
linux-mac68k CVS repo. AFAIK that driver never made it to the mainline.

Despite that, as of v2.3.45, arch/m68k/config.in listed CONFIG_SMC9194
under CONFIG_MAC. This mistake got carried over into Kconfig in v2.5.55.
(See pre-git era "[PATCH] add m68k dependencies to net driver config".)

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
---
 drivers/net/ethernet/smsc/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/smsc/Kconfig b/drivers/net/ethernet/smsc/Kconfig
index 63aca9f847e1..4c2f612e4414 100644
--- a/drivers/net/ethernet/smsc/Kconfig
+++ b/drivers/net/ethernet/smsc/Kconfig
@@ -20,7 +20,7 @@ if NET_VENDOR_SMSC
 
 config SMC9194
 	tristate "SMC 9194 support"
-	depends on (ISA || MAC && BROKEN)
+	depends on ISA
 	select CRC32
 	---help---
 	  This is support for the SMC9xxx based Ethernet cards. Choose this
-- 
2.13.5

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH RESEND net 8/9] net/8390: Fix redundant code
  2017-10-03  1:07 [PATCH RESEND net 0/9] Fixes, cleanup and modernization for some legacy ethernet NIC drivers Finn Thain
                   ` (7 preceding siblings ...)
  2017-10-03  1:07 ` [PATCH RESEND net 1/9] net/smc9194: Remove bogus CONFIG_MAC reference Finn Thain
@ 2017-10-03  1:07 ` Finn Thain
  2017-10-04  4:41 ` [PATCH RESEND net 0/9] Fixes, cleanup and modernization for some legacy ethernet NIC drivers David Miller
  9 siblings, 0 replies; 12+ messages in thread
From: Finn Thain @ 2017-10-03  1:07 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, linux-kernel, Russell King, linux-arm-kernel

The patch which introduced the 8390 core module parameter 'msg_enable'
failed to do anything useful with it: it merely causes an ancient
version string to be logged.

Remove the other code that logs the same string. Use the msg_enable
module parameter as the default value for ei_local->msg_enable.
Otherwise, some 8390 modules have no way to set ei_local->msg_enable.

Also fix two more issues arising from the same patch: indentation
mistakes and pointless static variables.

Fixes: c45f812f0280 ("8390 : Replace ei_debug with msg_enable/NETIF_MSG_* feature")
Cc: Russell King <linux@armlinux.org.uk>
Cc: linux-arm-kernel@lists.infradead.org
Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
---
 drivers/net/ethernet/8390/ax88796.c   |  3 ---
 drivers/net/ethernet/8390/axnet_cs.c  |  2 --
 drivers/net/ethernet/8390/etherh.c    | 17 -----------------
 drivers/net/ethernet/8390/hydra.c     |  4 ----
 drivers/net/ethernet/8390/lib8390.c   |  2 ++
 drivers/net/ethernet/8390/mac8390.c   |  7 -------
 drivers/net/ethernet/8390/mcf8390.c   |  4 ----
 drivers/net/ethernet/8390/pcnet_cs.c  |  4 ----
 drivers/net/ethernet/8390/zorro8390.c |  5 -----
 9 files changed, 2 insertions(+), 46 deletions(-)

diff --git a/drivers/net/ethernet/8390/ax88796.c b/drivers/net/ethernet/8390/ax88796.c
index 05d9d3e2e92e..28aa79d2f16c 100644
--- a/drivers/net/ethernet/8390/ax88796.c
+++ b/drivers/net/ethernet/8390/ax88796.c
@@ -77,8 +77,6 @@ static unsigned char version[] = "ax88796.c: Copyright 2005,2007 Simtec Electron
 
 #define AX_GPOC_PPDSET	BIT(6)
 
-static u32 ax_msg_enable;
-
 /* device private data */
 
 struct ax_device {
@@ -747,7 +745,6 @@ static int ax_init_dev(struct net_device *dev)
 	ei_local->block_output = &ax_block_output;
 	ei_local->get_8390_hdr = &ax_get_8390_hdr;
 	ei_local->priv = 0;
-	ei_local->msg_enable = ax_msg_enable;
 
 	dev->netdev_ops = &ax_netdev_ops;
 	dev->ethtool_ops = &ax_ethtool_ops;
diff --git a/drivers/net/ethernet/8390/axnet_cs.c b/drivers/net/ethernet/8390/axnet_cs.c
index 3da1fc539ef9..91e76dc1e6e1 100644
--- a/drivers/net/ethernet/8390/axnet_cs.c
+++ b/drivers/net/ethernet/8390/axnet_cs.c
@@ -104,7 +104,6 @@ static void AX88190_init(struct net_device *dev, int startp);
 static int ax_open(struct net_device *dev);
 static int ax_close(struct net_device *dev);
 static irqreturn_t ax_interrupt(int irq, void *dev_id);
-static u32 axnet_msg_enable;
 
 /*====================================================================*/
 
@@ -151,7 +150,6 @@ static int axnet_probe(struct pcmcia_device *link)
 	return -ENOMEM;
 
     ei_local = netdev_priv(dev);
-    ei_local->msg_enable = axnet_msg_enable;
     spin_lock_init(&ei_local->page_lock);
 
     info = PRIV(dev);
diff --git a/drivers/net/ethernet/8390/etherh.c b/drivers/net/ethernet/8390/etherh.c
index 11cbf22ad201..32e9627e3880 100644
--- a/drivers/net/ethernet/8390/etherh.c
+++ b/drivers/net/ethernet/8390/etherh.c
@@ -64,8 +64,6 @@ static char version[] =
 
 #include "lib8390.c"
 
-static u32 etherh_msg_enable;
-
 struct etherh_priv {
 	void __iomem	*ioc_fast;
 	void __iomem	*memc;
@@ -502,18 +500,6 @@ etherh_close(struct net_device *dev)
 }
 
 /*
- * Initialisation
- */
-
-static void __init etherh_banner(void)
-{
-	static int version_printed;
-
-	if ((etherh_msg_enable & NETIF_MSG_DRV) && (version_printed++ == 0))
-		pr_info("%s", version);
-}
-
-/*
  * Read the ethernet address string from the on board rom.
  * This is an ascii string...
  */
@@ -671,8 +657,6 @@ etherh_probe(struct expansion_card *ec, const struct ecard_id *id)
 	struct etherh_priv *eh;
 	int ret;
 
-	etherh_banner();
-
 	ret = ecard_request_resources(ec);
 	if (ret)
 		goto out;
@@ -757,7 +741,6 @@ etherh_probe(struct expansion_card *ec, const struct ecard_id *id)
 	ei_local->block_output  = etherh_block_output;
 	ei_local->get_8390_hdr  = etherh_get_header;
 	ei_local->interface_num = 0;
-	ei_local->msg_enable = etherh_msg_enable;
 
 	etherh_reset(dev);
 	__NS8390_init(dev, 0);
diff --git a/drivers/net/ethernet/8390/hydra.c b/drivers/net/ethernet/8390/hydra.c
index 8ae249195301..941754ea78ec 100644
--- a/drivers/net/ethernet/8390/hydra.c
+++ b/drivers/net/ethernet/8390/hydra.c
@@ -66,7 +66,6 @@ static void hydra_block_input(struct net_device *dev, int count,
 static void hydra_block_output(struct net_device *dev, int count,
 			       const unsigned char *buf, int start_page);
 static void hydra_remove_one(struct zorro_dev *z);
-static u32 hydra_msg_enable;
 
 static struct zorro_device_id hydra_zorro_tbl[] = {
     { ZORRO_PROD_HYDRA_SYSTEMS_AMIGANET },
@@ -119,7 +118,6 @@ static int hydra_init(struct zorro_dev *z)
     int start_page, stop_page;
     int j;
     int err;
-    struct ei_device *ei_local;
 
     static u32 hydra_offsets[16] = {
 	0x00, 0x02, 0x04, 0x06, 0x08, 0x0a, 0x0c, 0x0e,
@@ -138,8 +136,6 @@ static int hydra_init(struct zorro_dev *z)
     start_page = NESM_START_PG;
     stop_page = NESM_STOP_PG;
 
-    ei_local = netdev_priv(dev);
-    ei_local->msg_enable = hydra_msg_enable;
     dev->base_addr = ioaddr;
     dev->irq = IRQ_AMIGA_PORTS;
 
diff --git a/drivers/net/ethernet/8390/lib8390.c b/drivers/net/ethernet/8390/lib8390.c
index 60f8e2c8e726..5d9bbde9fe68 100644
--- a/drivers/net/ethernet/8390/lib8390.c
+++ b/drivers/net/ethernet/8390/lib8390.c
@@ -975,6 +975,8 @@ static void ethdev_setup(struct net_device *dev)
 	ether_setup(dev);
 
 	spin_lock_init(&ei_local->page_lock);
+
+	ei_local->msg_enable = msg_enable;
 }
 
 /**
diff --git a/drivers/net/ethernet/8390/mac8390.c b/drivers/net/ethernet/8390/mac8390.c
index 9497f18eaba0..1bfc66f37971 100644
--- a/drivers/net/ethernet/8390/mac8390.c
+++ b/drivers/net/ethernet/8390/mac8390.c
@@ -167,7 +167,6 @@ static void slow_sane_block_output(struct net_device *dev, int count,
 				   const unsigned char *buf, int start_page);
 static void word_memcpy_tocard(unsigned long tp, const void *fp, int count);
 static void word_memcpy_fromcard(void *tp, unsigned long fp, int count);
-static u32 mac8390_msg_enable;
 
 static enum mac8390_type __init mac8390_ident(struct nubus_dev *dev)
 {
@@ -297,8 +296,6 @@ static bool __init mac8390_init(struct net_device *dev, struct nubus_dev *ndev,
 	int offset;
 	volatile unsigned short *i;
 
-	printk_once(KERN_INFO pr_fmt("%s"), version);
-
 	dev->irq = SLOT2IRQ(ndev->board->slot);
 	/* This is getting to be a habit */
 	dev->base_addr = (ndev->board->slot_addr |
@@ -396,7 +393,6 @@ struct net_device * __init mac8390_probe(int unit)
 	struct net_device *dev;
 	struct nubus_dev *ndev = NULL;
 	int err = -ENODEV;
-	struct ei_device *ei_local;
 
 	static unsigned int slots;
 
@@ -436,9 +432,6 @@ struct net_device * __init mac8390_probe(int unit)
 	if (!ndev)
 		goto out;
 
-	 ei_local = netdev_priv(dev);
-	 ei_local->msg_enable = mac8390_msg_enable;
-
 	err = register_netdev(dev);
 	if (err)
 		goto out;
diff --git a/drivers/net/ethernet/8390/mcf8390.c b/drivers/net/ethernet/8390/mcf8390.c
index 4bb967bc879e..4ad8031ab669 100644
--- a/drivers/net/ethernet/8390/mcf8390.c
+++ b/drivers/net/ethernet/8390/mcf8390.c
@@ -38,7 +38,6 @@ static const char version[] =
 
 #define NESM_START_PG	0x40	/* First page of TX buffer */
 #define NESM_STOP_PG	0x80	/* Last page +1 of RX ring */
-static u32 mcf8390_msg_enable;
 
 #ifdef NE2000_ODDOFFSET
 /*
@@ -407,7 +406,6 @@ static int mcf8390_init(struct net_device *dev)
 static int mcf8390_probe(struct platform_device *pdev)
 {
 	struct net_device *dev;
-	struct ei_device *ei_local;
 	struct resource *mem, *irq;
 	resource_size_t msize;
 	int ret;
@@ -435,8 +433,6 @@ static int mcf8390_probe(struct platform_device *pdev)
 
 	SET_NETDEV_DEV(dev, &pdev->dev);
 	platform_set_drvdata(pdev, dev);
-	ei_local = netdev_priv(dev);
-	ei_local->msg_enable = mcf8390_msg_enable;
 
 	dev->irq = irq->start;
 	dev->base_addr = mem->start;
diff --git a/drivers/net/ethernet/8390/pcnet_cs.c b/drivers/net/ethernet/8390/pcnet_cs.c
index bd0a2a14b649..a81ffe4874e1 100644
--- a/drivers/net/ethernet/8390/pcnet_cs.c
+++ b/drivers/net/ethernet/8390/pcnet_cs.c
@@ -66,7 +66,6 @@
 #define PCNET_RDC_TIMEOUT (2*HZ/100)	/* Max wait in jiffies for Tx RDC */
 
 static const char *if_names[] = { "auto", "10baseT", "10base2"};
-static u32 pcnet_msg_enable;
 
 /*====================================================================*/
 
@@ -556,7 +555,6 @@ static int pcnet_config(struct pcmcia_device *link)
     int start_pg, stop_pg, cm_offset;
     int has_shmem = 0;
     struct hw_info *local_hw_info;
-    struct ei_device *ei_local;
 
     dev_dbg(&link->dev, "pcnet_config\n");
 
@@ -606,8 +604,6 @@ static int pcnet_config(struct pcmcia_device *link)
 	mii_phy_probe(dev);
 
     SET_NETDEV_DEV(dev, &link->dev);
-    ei_local = netdev_priv(dev);
-    ei_local->msg_enable = pcnet_msg_enable;
 
     if (register_netdev(dev) != 0) {
 	pr_notice("register_netdev() failed\n");
diff --git a/drivers/net/ethernet/8390/zorro8390.c b/drivers/net/ethernet/8390/zorro8390.c
index 6d93956b293b..35a500a21521 100644
--- a/drivers/net/ethernet/8390/zorro8390.c
+++ b/drivers/net/ethernet/8390/zorro8390.c
@@ -44,8 +44,6 @@
 static const char version[] =
 	"8390.c:v1.10cvs 9/23/94 Donald Becker (becker@cesdis.gsfc.nasa.gov)\n";
 
-static u32 zorro8390_msg_enable;
-
 #include "lib8390.c"
 
 #define DRV_NAME	"zorro8390"
@@ -296,7 +294,6 @@ static int zorro8390_init(struct net_device *dev, unsigned long board,
 	int err;
 	unsigned char SA_prom[32];
 	int start_page, stop_page;
-	struct ei_device *ei_local = netdev_priv(dev);
 	static u32 zorro8390_offsets[16] = {
 		0x00, 0x02, 0x04, 0x06, 0x08, 0x0a, 0x0c, 0x0e,
 		0x10, 0x12, 0x14, 0x16, 0x18, 0x1a, 0x1c, 0x1e,
@@ -388,8 +385,6 @@ static int zorro8390_init(struct net_device *dev, unsigned long board,
 	dev->netdev_ops = &zorro8390_netdev_ops;
 	__NS8390_init(dev, 0);
 
-	ei_local->msg_enable = zorro8390_msg_enable;
-
 	err = register_netdev(dev);
 	if (err) {
 		free_irq(IRQ_AMIGA_PORTS, dev);
-- 
2.13.5

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH RESEND net 7/9] net/sonic: Replace custom debug logging with netif_* calls
  2017-10-03  1:07 [PATCH RESEND net 0/9] Fixes, cleanup and modernization for some legacy ethernet NIC drivers Finn Thain
                   ` (2 preceding siblings ...)
  2017-10-03  1:07 ` [PATCH RESEND net 3/9] net/mac89x0: Fix and modernize log messages Finn Thain
@ 2017-10-03  1:07 ` Finn Thain
  2017-10-03  1:07 ` [PATCH RESEND net 5/9] net/macmace: Fix and cleanup log messages Finn Thain
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Finn Thain @ 2017-10-03  1:07 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, linux-kernel, Thomas Bogendoerfer, Chris Zankel

Also eliminate duplicated debug code by moving it into the core driver.

Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Chris Zankel <chris@zankel.net>
Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
---
 drivers/net/ethernet/natsemi/jazzsonic.c | 17 ++----
 drivers/net/ethernet/natsemi/macsonic.c  | 21 +-------
 drivers/net/ethernet/natsemi/sonic.c     | 92 +++++++++++++++-----------------
 drivers/net/ethernet/natsemi/sonic.h     |  2 +
 drivers/net/ethernet/natsemi/xtsonic.c   | 17 ++----
 5 files changed, 54 insertions(+), 95 deletions(-)

diff --git a/drivers/net/ethernet/natsemi/jazzsonic.c b/drivers/net/ethernet/natsemi/jazzsonic.c
index 6ec25c27cb73..0fc303901f27 100644
--- a/drivers/net/ethernet/natsemi/jazzsonic.c
+++ b/drivers/net/ethernet/natsemi/jazzsonic.c
@@ -59,14 +59,6 @@ do {									\
 	*((volatile unsigned int *)dev->base_addr+(reg)) = (val);		\
 } while (0)
 
-
-/* use 0 for production, 1 for verification, >1 for debug */
-#ifdef SONIC_DEBUG
-static unsigned int sonic_debug = SONIC_DEBUG;
-#else
-static unsigned int sonic_debug = 1;
-#endif
-
 /*
  * We cannot use station (ethernet) address prefixes to detect the
  * sonic controller since these are board manufacturer depended.
@@ -116,7 +108,6 @@ static const struct net_device_ops sonic_netdev_ops = {
 
 static int sonic_probe1(struct net_device *dev)
 {
-	static unsigned version_printed;
 	unsigned int silicon_revision;
 	unsigned int val;
 	struct sonic_local *lp = netdev_priv(dev);
@@ -143,9 +134,6 @@ static int sonic_probe1(struct net_device *dev)
 		goto out;
 	}
 
-	if (sonic_debug  &&  version_printed++ == 0)
-		printk(version);
-
 	printk(KERN_INFO "%s: Sonic ethernet found at 0x%08lx, ",
 	       dev_name(lp->device), dev->base_addr);
 
@@ -241,6 +229,9 @@ static int jazz_sonic_probe(struct platform_device *pdev)
 	err = sonic_probe1(dev);
 	if (err)
 		goto out;
+
+	sonic_msg_init(dev);
+
 	err = register_netdev(dev);
 	if (err)
 		goto out1;
@@ -258,8 +249,6 @@ static int jazz_sonic_probe(struct platform_device *pdev)
 }
 
 MODULE_DESCRIPTION("Jazz SONIC ethernet driver");
-module_param(sonic_debug, int, 0);
-MODULE_PARM_DESC(sonic_debug, "jazzsonic debug level (1-4)");
 MODULE_ALIAS("platform:jazzsonic");
 
 #include "sonic.c"
diff --git a/drivers/net/ethernet/natsemi/macsonic.c b/drivers/net/ethernet/natsemi/macsonic.c
index 795d71522617..6e5f87e955da 100644
--- a/drivers/net/ethernet/natsemi/macsonic.c
+++ b/drivers/net/ethernet/natsemi/macsonic.c
@@ -71,15 +71,6 @@ static char mac_sonic_string[] = "macsonic";
 #define SONIC_WRITE(reg,val) (nubus_writew(val, dev->base_addr + (reg * 4) \
 	      + lp->reg_offset))
 
-/* use 0 for production, 1 for verification, >1 for debug */
-#ifdef SONIC_DEBUG
-static unsigned int sonic_debug = SONIC_DEBUG;
-#else
-static unsigned int sonic_debug = 1;
-#endif
-
-static int sonic_version_printed;
-
 /* For onboard SONIC */
 #define ONBOARD_SONIC_REGISTERS	0x50F0A000
 #define ONBOARD_SONIC_PROM_BASE	0x50f08000
@@ -341,10 +332,6 @@ static int mac_onboard_sonic_probe(struct net_device *dev)
 	else
 		dev->irq = IRQ_NUBUS_9;
 
-	if (!sonic_version_printed) {
-		printk(KERN_INFO "%s", version);
-		sonic_version_printed = 1;
-	}
 	printk(KERN_INFO "%s: onboard / comm-slot SONIC at 0x%08lx\n",
 	       dev_name(lp->device), dev->base_addr);
 
@@ -527,10 +514,6 @@ static int mac_nubus_sonic_probe(struct net_device *dev)
 	lp->dma_bitmode = dma_bitmode;
 	dev->irq = SLOT2IRQ(ndev->board->slot);
 
-	if (!sonic_version_printed) {
-		printk(KERN_INFO "%s", version);
-		sonic_version_printed = 1;
-	}
 	printk(KERN_INFO "%s: %s in slot %X\n",
 	       dev_name(lp->device), ndev->board->name, ndev->board->slot);
 	printk(KERN_INFO "%s: revision 0x%04x, using %d bit DMA and register offset %d\n",
@@ -586,6 +569,8 @@ static int mac_sonic_probe(struct platform_device *pdev)
 	if (err)
 		goto out;
 found:
+	sonic_msg_init(dev);
+
 	err = register_netdev(dev);
 	if (err)
 		goto out;
@@ -601,8 +586,6 @@ static int mac_sonic_probe(struct platform_device *pdev)
 }
 
 MODULE_DESCRIPTION("Macintosh SONIC ethernet driver");
-module_param(sonic_debug, int, 0);
-MODULE_PARM_DESC(sonic_debug, "macsonic debug level (1-4)");
 MODULE_ALIAS("platform:macsonic");
 
 #include "sonic.c"
diff --git a/drivers/net/ethernet/natsemi/sonic.c b/drivers/net/ethernet/natsemi/sonic.c
index 612c7a44b26c..c0bae615238f 100644
--- a/drivers/net/ethernet/natsemi/sonic.c
+++ b/drivers/net/ethernet/natsemi/sonic.c
@@ -33,7 +33,18 @@
  * the NetBSD file "sys/arch/mac68k/dev/if_sn.c".
  */
 
+static int sonic_debug = -1;
+module_param(sonic_debug, int, 0);
+MODULE_PARM_DESC(sonic_debug, "debug message level");
 
+static void sonic_msg_init(struct net_device *dev)
+{
+	struct sonic_local *lp = netdev_priv(dev);
+
+	lp->msg_enable = netif_msg_init(sonic_debug, 0);
+
+	netif_dbg(lp, drv, dev, "%s", version);
+}
 
 /*
  * Open/initialize the SONIC controller.
@@ -47,8 +58,7 @@ static int sonic_open(struct net_device *dev)
 	struct sonic_local *lp = netdev_priv(dev);
 	int i;
 
-	if (sonic_debug > 2)
-		printk("sonic_open: initializing sonic driver.\n");
+	netif_dbg(lp, ifup, dev, "%s\n", __func__);
 
 	for (i = 0; i < SONIC_NUM_RRS; i++) {
 		struct sk_buff *skb = netdev_alloc_skb(dev, SONIC_RBSIZE + 2);
@@ -95,8 +105,7 @@ static int sonic_open(struct net_device *dev)
 
 	netif_start_queue(dev);
 
-	if (sonic_debug > 2)
-		printk("sonic_open: Initialization done.\n");
+	netif_dbg(lp, ifup, dev, "%s: done\n", __func__);
 
 	return 0;
 }
@@ -110,8 +119,7 @@ static int sonic_close(struct net_device *dev)
 	struct sonic_local *lp = netdev_priv(dev);
 	int i;
 
-	if (sonic_debug > 2)
-		printk("sonic_close\n");
+	netif_dbg(lp, ifdown, dev, "%s\n", __func__);
 
 	netif_stop_queue(dev);
 
@@ -205,8 +213,7 @@ static int sonic_send_packet(struct sk_buff *skb, struct net_device *dev)
 	int length;
 	int entry = lp->next_tx;
 
-	if (sonic_debug > 2)
-		printk("sonic_send_packet: skb=%p, dev=%p\n", skb, dev);
+	netif_dbg(lp, tx_queued, dev, "%s: skb=%p\n", __func__, skb);
 
 	length = skb->len;
 	if (length < ETH_ZLEN) {
@@ -252,14 +259,12 @@ static int sonic_send_packet(struct sk_buff *skb, struct net_device *dev)
 	lp->next_tx = (entry + 1) & SONIC_TDS_MASK;
 	if (lp->tx_skb[lp->next_tx] != NULL) {
 		/* The ring is full, the ISR has yet to process the next TD. */
-		if (sonic_debug > 3)
-			printk("%s: stopping queue\n", dev->name);
+		netif_dbg(lp, tx_queued, dev, "%s: stop queue\n", __func__);
 		netif_stop_queue(dev);
 		/* after this packet, wait for ISR to free up some TDAs */
 	} else netif_start_queue(dev);
 
-	if (sonic_debug > 2)
-		printk("sonic_send_packet: issuing Tx command\n");
+	netif_dbg(lp, tx_queued, dev, "%s: issuing tx command\n", __func__);
 
 	SONIC_WRITE(SONIC_CMD, SONIC_CR_TXP);
 
@@ -281,8 +286,7 @@ static irqreturn_t sonic_interrupt(int irq, void *dev_id)
 
 	do {
 		if (status & SONIC_INT_PKTRX) {
-			if (sonic_debug > 2)
-				printk("%s: packet rx\n", dev->name);
+			netif_dbg(lp, intr, dev, "%s: packet rx\n", __func__);
 			sonic_rx(dev);	/* got packet(s) */
 			SONIC_WRITE(SONIC_ISR, SONIC_INT_PKTRX); /* clear the interrupt */
 		}
@@ -299,8 +303,7 @@ static irqreturn_t sonic_interrupt(int irq, void *dev_id)
 			 *   still being allocated by sonic_send_packet (status clear & tx_skb[entry] clear)
 			 */
 
-			if (sonic_debug > 2)
-				printk("%s: tx done\n", dev->name);
+			netif_dbg(lp, intr, dev, "%s: tx done\n", __func__);
 
 			while (lp->tx_skb[entry] != NULL) {
 				if ((td_status = sonic_tda_get(dev, entry, SONIC_TD_STATUS)) == 0)
@@ -346,20 +349,20 @@ static irqreturn_t sonic_interrupt(int irq, void *dev_id)
 		 * check error conditions
 		 */
 		if (status & SONIC_INT_RFO) {
-			if (sonic_debug > 1)
-				printk("%s: rx fifo overrun\n", dev->name);
+			netif_dbg(lp, rx_err, dev, "%s: rx fifo overrun\n",
+			          __func__);
 			lp->stats.rx_fifo_errors++;
 			SONIC_WRITE(SONIC_ISR, SONIC_INT_RFO); /* clear the interrupt */
 		}
 		if (status & SONIC_INT_RDE) {
-			if (sonic_debug > 1)
-				printk("%s: rx descriptors exhausted\n", dev->name);
+			netif_dbg(lp, rx_err, dev, "%s: rx descriptors exhausted\n",
+			          __func__);
 			lp->stats.rx_dropped++;
 			SONIC_WRITE(SONIC_ISR, SONIC_INT_RDE); /* clear the interrupt */
 		}
 		if (status & SONIC_INT_RBAE) {
-			if (sonic_debug > 1)
-				printk("%s: rx buffer area exceeded\n", dev->name);
+			netif_dbg(lp, rx_err, dev, "%s: rx buffer area exceeded\n",
+			          __func__);
 			lp->stats.rx_dropped++;
 			SONIC_WRITE(SONIC_ISR, SONIC_INT_RBAE); /* clear the interrupt */
 		}
@@ -380,8 +383,9 @@ static irqreturn_t sonic_interrupt(int irq, void *dev_id)
 
 		/* transmit error */
 		if (status & SONIC_INT_TXER) {
-			if ((SONIC_READ(SONIC_TCR) & SONIC_TCR_FU) && (sonic_debug > 2))
-				printk(KERN_ERR "%s: tx fifo underrun\n", dev->name);
+			if (SONIC_READ(SONIC_TCR) & SONIC_TCR_FU)
+				netif_dbg(lp, tx_err, dev, "%s: tx fifo underrun\n",
+				          __func__);
 			SONIC_WRITE(SONIC_ISR, SONIC_INT_TXER); /* clear the interrupt */
 		}
 
@@ -475,8 +479,8 @@ static void sonic_rx(struct net_device *dev)
 			if (lp->cur_rwp >= lp->rra_end) lp->cur_rwp = lp->rra_laddr & 0xffff;
 			SONIC_WRITE(SONIC_RWP, lp->cur_rwp);
 			if (SONIC_READ(SONIC_ISR) & SONIC_INT_RBE) {
-				if (sonic_debug > 2)
-					printk("%s: rx buffer exhausted\n", dev->name);
+				netif_dbg(lp, rx_err, dev, "%s: rx buffer exhausted\n",
+				          __func__);
 				SONIC_WRITE(SONIC_ISR, SONIC_INT_RBE); /* clear the flag */
 			}
 		} else
@@ -542,9 +546,8 @@ static void sonic_multicast_list(struct net_device *dev)
 		    (netdev_mc_count(dev) > 15)) {
 			rcr |= SONIC_RCR_AMC;
 		} else {
-			if (sonic_debug > 2)
-				printk("sonic_multicast_list: mc_count %d\n",
-				       netdev_mc_count(dev));
+			netif_dbg(lp, ifup, dev, "%s: mc_count %d\n", __func__,
+			          netdev_mc_count(dev));
 			sonic_set_cam_enable(dev, 1);  /* always enable our own address */
 			i = 1;
 			netdev_for_each_mc_addr(ha, dev) {
@@ -562,8 +565,7 @@ static void sonic_multicast_list(struct net_device *dev)
 		}
 	}
 
-	if (sonic_debug > 2)
-		printk("sonic_multicast_list: setting RCR=%x\n", rcr);
+	netif_dbg(lp, ifup, dev, "%s: rcr=%x\n", __func__, rcr);
 
 	SONIC_WRITE(SONIC_RCR, rcr);
 }
@@ -596,8 +598,7 @@ static int sonic_init(struct net_device *dev)
 	/*
 	 * initialize the receive resource area
 	 */
-	if (sonic_debug > 2)
-		printk("sonic_init: initialize receive resource area\n");
+	netif_dbg(lp, ifup, dev, "%s: receive resource area\n", __func__);
 
 	for (i = 0; i < SONIC_NUM_RRS; i++) {
 		u16 bufadr_l = (unsigned long)lp->rx_laddr[i] & 0xffff;
@@ -622,8 +623,7 @@ static int sonic_init(struct net_device *dev)
 	SONIC_WRITE(SONIC_EOBC, (SONIC_RBSIZE >> 1) - (lp->dma_bitmode ? 2 : 1));
 
 	/* load the resource pointers */
-	if (sonic_debug > 3)
-		printk("sonic_init: issuing RRRA command\n");
+	netif_dbg(lp, ifup, dev, "%s: issuing RRRA command\n", __func__);
 
 	SONIC_WRITE(SONIC_CMD, SONIC_CR_RRRA);
 	i = 0;
@@ -632,16 +632,16 @@ static int sonic_init(struct net_device *dev)
 			break;
 	}
 
-	if (sonic_debug > 2)
-		printk("sonic_init: status=%x i=%d\n", SONIC_READ(SONIC_CMD), i);
+	netif_dbg(lp, ifup, dev, "%s: status=%x, i=%d\n", __func__,
+	          SONIC_READ(SONIC_CMD), i);
 
 	/*
 	 * Initialize the receive descriptors so that they
 	 * become a circular linked list, ie. let the last
 	 * descriptor point to the first again.
 	 */
-	if (sonic_debug > 2)
-		printk("sonic_init: initialize receive descriptors\n");
+	netif_dbg(lp, ifup, dev, "%s: receive descriptors\n", __func__);
+
 	for (i=0; i<SONIC_NUM_RDS; i++) {
 		sonic_rda_put(dev, i, SONIC_RD_STATUS, 0);
 		sonic_rda_put(dev, i, SONIC_RD_PKTLEN, 0);
@@ -664,8 +664,8 @@ static int sonic_init(struct net_device *dev)
 	/*
 	 * initialize transmit descriptors
 	 */
-	if (sonic_debug > 2)
-		printk("sonic_init: initialize transmit descriptors\n");
+	netif_dbg(lp, ifup, dev, "%s: transmit descriptors\n", __func__);
+
 	for (i = 0; i < SONIC_NUM_TDS; i++) {
 		sonic_tda_put(dev, i, SONIC_TD_STATUS, 0);
 		sonic_tda_put(dev, i, SONIC_TD_CONFIG, 0);
@@ -712,10 +712,8 @@ static int sonic_init(struct net_device *dev)
 		if (SONIC_READ(SONIC_ISR) & SONIC_INT_LCD)
 			break;
 	}
-	if (sonic_debug > 2) {
-		printk("sonic_init: CMD=%x, ISR=%x\n, i=%d",
-		       SONIC_READ(SONIC_CMD), SONIC_READ(SONIC_ISR), i);
-	}
+	netif_dbg(lp, ifup, dev, "%s: CMD=%x, ISR=%x, i=%d\n", __func__,
+	          SONIC_READ(SONIC_CMD), SONIC_READ(SONIC_ISR), i);
 
 	/*
 	 * enable receiver, disable loopback
@@ -731,9 +729,7 @@ static int sonic_init(struct net_device *dev)
 	if ((cmd & SONIC_CR_RXEN) == 0 || (cmd & SONIC_CR_STP) == 0)
 		printk(KERN_ERR "sonic_init: failed, status=%x\n", cmd);
 
-	if (sonic_debug > 2)
-		printk("sonic_init: new status=%x\n",
-		       SONIC_READ(SONIC_CMD));
+	netif_dbg(lp, ifup, dev, "%s: cmd=%x\n", __func__, cmd);
 
 	return 0;
 }
diff --git a/drivers/net/ethernet/natsemi/sonic.h b/drivers/net/ethernet/natsemi/sonic.h
index 7b0a8db57af9..2f18229b17cf 100644
--- a/drivers/net/ethernet/natsemi/sonic.h
+++ b/drivers/net/ethernet/natsemi/sonic.h
@@ -318,6 +318,7 @@ struct sonic_local {
 	unsigned int eol_rx;
 	unsigned int eol_tx;           /* last unacked transmit packet */
 	unsigned int next_tx;          /* next free TD */
+	int msg_enable;
 	struct device *device;         /* generic device */
 	struct net_device_stats stats;
 };
@@ -335,6 +336,7 @@ static struct net_device_stats *sonic_get_stats(struct net_device *dev);
 static void sonic_multicast_list(struct net_device *dev);
 static int sonic_init(struct net_device *dev);
 static void sonic_tx_timeout(struct net_device *dev);
+static void sonic_msg_init(struct net_device *dev);
 
 /* Internal inlines for reading/writing DMA buffers.  Note that bus
    size and endianness matter here, whereas they don't for registers,
diff --git a/drivers/net/ethernet/natsemi/xtsonic.c b/drivers/net/ethernet/natsemi/xtsonic.c
index bed89e8105e9..1f601eb8a828 100644
--- a/drivers/net/ethernet/natsemi/xtsonic.c
+++ b/drivers/net/ethernet/natsemi/xtsonic.c
@@ -72,14 +72,6 @@ extern void xtboard_get_ether_addr(unsigned char *buf);
 #define SONIC_WRITE(reg,val) \
 	*((volatile unsigned int *)dev->base_addr+reg) = val
 
-
-/* Use 0 for production, 1 for verification, and >2 for debug */
-#ifdef SONIC_DEBUG
-static unsigned int sonic_debug = SONIC_DEBUG;
-#else
-static unsigned int sonic_debug = 1;
-#endif
-
 /*
  * We cannot use station (ethernet) address prefixes to detect the
  * sonic controller since these are board manufacturer depended.
@@ -129,7 +121,6 @@ static const struct net_device_ops xtsonic_netdev_ops = {
 
 static int __init sonic_probe1(struct net_device *dev)
 {
-	static unsigned version_printed = 0;
 	unsigned int silicon_revision;
 	struct sonic_local *lp = netdev_priv(dev);
 	unsigned int base_addr = dev->base_addr;
@@ -156,9 +147,6 @@ static int __init sonic_probe1(struct net_device *dev)
 		return -ENODEV;
 	}
 
-	if (sonic_debug  &&  version_printed++ == 0)
-		printk(version);
-
 	/*
 	 * Put the sonic into software reset, then retrieve ethernet address.
 	 * Note: we are assuming that the boot-loader has initialized the cam.
@@ -269,6 +257,9 @@ int xtsonic_probe(struct platform_device *pdev)
 
 	if ((err = sonic_probe1(dev)))
 		goto out;
+
+	sonic_msg_init(dev);
+
 	if ((err = register_netdev(dev)))
 		goto out1;
 
@@ -286,8 +277,6 @@ int xtsonic_probe(struct platform_device *pdev)
 }
 
 MODULE_DESCRIPTION("Xtensa XT2000 SONIC ethernet driver");
-module_param(sonic_debug, int, 0);
-MODULE_PARM_DESC(sonic_debug, "xtsonic debug level (1-4)");
 
 #include "sonic.c"
 
-- 
2.13.5

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH RESEND net 9/9] net/mac8390: Fix log messages
  2017-10-03  1:07 [PATCH RESEND net 0/9] Fixes, cleanup and modernization for some legacy ethernet NIC drivers Finn Thain
@ 2017-10-03  1:07 ` Finn Thain
  2017-10-03  1:07 ` [PATCH RESEND net 2/9] net/mac89x0: Remove dead or unreachable code Finn Thain
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Finn Thain @ 2017-10-03  1:07 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, linux-kernel

Before expansion, dev->name is "eth%d", so log the slot number instead.
Log the MAC address after the interface gets a meaningful name.
Disambiguate the two identical "Card type %s is unsupported" messages.
Fix the duplicated driver name in the pr_warn() message.

Fixes: 3a3a7f3b7fbd ("net: mac8390: Allow modular build")
Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
---
 drivers/net/ethernet/8390/mac8390.c | 35 ++++++++++++++++++-----------------
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/drivers/net/ethernet/8390/mac8390.c b/drivers/net/ethernet/8390/mac8390.c
index 1bfc66f37971..45d724d8333a 100644
--- a/drivers/net/ethernet/8390/mac8390.c
+++ b/drivers/net/ethernet/8390/mac8390.c
@@ -307,14 +307,15 @@ static bool __init mac8390_init(struct net_device *dev, struct nubus_dev *ndev,
 	 */
 
 	if (nubus_get_func_dir(ndev, &dir) == -1) {
-		pr_err("%s: Unable to get Nubus functional directory for slot %X!\n",
-		       dev->name, ndev->board->slot);
+		pr_err("Unable to get Nubus functional directory for slot %X!\n",
+		       ndev->board->slot);
 		return false;
 	}
 
 	/* Get the MAC address */
 	if (nubus_find_rsrc(&dir, NUBUS_RESID_MAC_ADDRESS, &ent) == -1) {
-		pr_info("%s: Couldn't get MAC address!\n", dev->name);
+		pr_info("MAC address resource for slot %X not found!\n",
+		        ndev->board->slot);
 		return false;
 	}
 
@@ -324,8 +325,8 @@ static bool __init mac8390_init(struct net_device *dev, struct nubus_dev *ndev,
 		nubus_rewinddir(&dir);
 		if (nubus_find_rsrc(&dir, NUBUS_RESID_MINOR_BASEOS,
 				    &ent) == -1) {
-			pr_err("%s: Memory offset resource for slot %X not found!\n",
-			       dev->name, ndev->board->slot);
+			pr_err("Memory offset resource for slot %X not found!\n",
+			       ndev->board->slot);
 			return false;
 		}
 		nubus_get_rsrc_mem(&offset, &ent, 4);
@@ -335,8 +336,8 @@ static bool __init mac8390_init(struct net_device *dev, struct nubus_dev *ndev,
 		nubus_rewinddir(&dir);
 		if (nubus_find_rsrc(&dir, NUBUS_RESID_MINOR_LENGTH,
 				    &ent) == -1) {
-			pr_info("%s: Memory length resource for slot %X not found, probing\n",
-				dev->name, ndev->board->slot);
+			pr_info("Memory length resource for slot %X not found, probing\n",
+				ndev->board->slot);
 			offset = mac8390_memsize(dev->mem_start);
 		} else {
 			nubus_get_rsrc_mem(&offset, &ent, 4);
@@ -379,8 +380,8 @@ static bool __init mac8390_init(struct net_device *dev, struct nubus_dev *ndev,
 			break;
 
 		default:
-			pr_err("Card type %s is unsupported, sorry\n",
-			       ndev->board->name);
+			pr_err("No known base address for %s card in slot %X\n",
+			       ndev->board->name, ndev->board->slot);
 			return false;
 		}
 	}
@@ -435,6 +436,9 @@ struct net_device * __init mac8390_probe(int unit)
 	err = register_netdev(dev);
 	if (err)
 		goto out;
+
+	netdev_info(dev, "MAC %pM, IRQ %d\n", dev->dev_addr, dev->irq);
+
 	return dev;
 
 out:
@@ -453,7 +457,7 @@ int __init init_module(void)
 {
 	dev_mac8390 = mac8390_probe(-1);
 	if (IS_ERR(dev_mac8390)) {
-		pr_warn("mac8390: No card found\n");
+		pr_warn("No card found\n");
 		return PTR_ERR(dev_mac8390);
 	}
 	return 0;
@@ -600,19 +604,16 @@ static int __init mac8390_initdev(struct net_device *dev,
 		break;
 
 	default:
-		pr_err("Card type %s is unsupported, sorry\n",
-		       ndev->board->name);
+		pr_err("%s card in slot %X is unsupported, sorry\n",
+		       ndev->board->name, ndev->board->slot);
 		return -ENODEV;
 	}
 
 	__NS8390_init(dev, 0);
 
 	/* Good, done, now spit out some messages */
-	pr_info("%s: %s in slot %X (type %s)\n",
-		dev->name, ndev->board->name, ndev->board->slot,
-		cardname[type]);
-	pr_info("MAC %pM IRQ %d, %d KB shared memory at %#lx, %d-bit access.\n",
-		dev->dev_addr, dev->irq,
+	pr_info("%s in slot %X has type %s, %d KB shared memory at %#lx, %d-bit access\n",
+		ndev->board->name, ndev->board->slot, cardname[type],
 		(unsigned int)(dev->mem_end - dev->mem_start) >> 10,
 		dev->mem_start, access_bitmode ? 32 : 16);
 	return 0;
-- 
2.13.5

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH RESEND net 5/9] net/macmace: Fix and cleanup log messages
  2017-10-03  1:07 [PATCH RESEND net 0/9] Fixes, cleanup and modernization for some legacy ethernet NIC drivers Finn Thain
                   ` (3 preceding siblings ...)
  2017-10-03  1:07 ` [PATCH RESEND net 7/9] net/sonic: Replace custom debug logging with netif_* calls Finn Thain
@ 2017-10-03  1:07 ` Finn Thain
  2017-10-03  1:07 ` [PATCH RESEND net 6/9] net/sonic: Cleanup and modernize " Finn Thain
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Finn Thain @ 2017-10-03  1:07 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, linux-kernel

Log the MAC address after the interface gets a meaningful name.
Drop redundant debug messages for FIFO events recorded in the
interface statistics (consistent with mace.c).

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
---
 drivers/net/ethernet/apple/macmace.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/apple/macmace.c b/drivers/net/ethernet/apple/macmace.c
index f17a160dbff2..3ff53c3ef732 100644
--- a/drivers/net/ethernet/apple/macmace.c
+++ b/drivers/net/ethernet/apple/macmace.c
@@ -247,13 +247,12 @@ static int mace_probe(struct platform_device *pdev)
 	dev->netdev_ops		= &mace_netdev_ops;
 	dev->watchdog_timeo	= TX_TIMEOUT;
 
-	printk(KERN_INFO "%s: 68K MACE, hardware address %pM\n",
-	       dev->name, dev->dev_addr);
-
 	err = register_netdev(dev);
 	if (!err)
 		return 0;
 
+	netdev_info(dev, "MAC %pM\n", dev->dev_addr);
+
 	free_netdev(dev);
 	return err;
 }
@@ -589,7 +588,6 @@ static irqreturn_t mace_interrupt(int irq, void *dev_id)
 			else if (fs & (UFLO|LCOL|RTRY)) {
 				++dev->stats.tx_aborted_errors;
 				if (mb->xmtfs & UFLO) {
-					printk(KERN_ERR "%s: DMA underrun.\n", dev->name);
 					dev->stats.tx_fifo_errors++;
 					mace_txdma_reset(dev);
 				}
@@ -644,10 +642,8 @@ static void mace_dma_rx_frame(struct net_device *dev, struct mace_frame *mf)
 
 	if (frame_status & (RS_OFLO | RS_CLSN | RS_FRAMERR | RS_FCSERR)) {
 		dev->stats.rx_errors++;
-		if (frame_status & RS_OFLO) {
-			printk(KERN_DEBUG "%s: fifo overflow.\n", dev->name);
+		if (frame_status & RS_OFLO)
 			dev->stats.rx_fifo_errors++;
-		}
 		if (frame_status & RS_CLSN)
 			dev->stats.collisions++;
 		if (frame_status & RS_FRAMERR)
-- 
2.13.5

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH RESEND net 6/9] net/sonic: Cleanup and modernize log messages
  2017-10-03  1:07 [PATCH RESEND net 0/9] Fixes, cleanup and modernization for some legacy ethernet NIC drivers Finn Thain
                   ` (4 preceding siblings ...)
  2017-10-03  1:07 ` [PATCH RESEND net 5/9] net/macmace: Fix and cleanup log messages Finn Thain
@ 2017-10-03  1:07 ` Finn Thain
  2017-10-03  1:07 ` [PATCH RESEND net 4/9] net/mac89x0: Replace custom debug logging with netif_* calls Finn Thain
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Finn Thain @ 2017-10-03  1:07 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, linux-kernel, Thomas Bogendoerfer, Chris Zankel

Add missing printk severity levels.
Don't print the valid silicon revision number (it's in the source).
Avoid KERN_CONT usage as per advice from checkpatch.
Avoid ifdef around printk calls.

Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Chris Zankel <chris@zankel.net>
Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
---
 drivers/net/ethernet/natsemi/jazzsonic.c |  9 +++------
 drivers/net/ethernet/natsemi/macsonic.c  | 24 ++++++++++--------------
 drivers/net/ethernet/natsemi/xtsonic.c   | 11 ++++-------
 3 files changed, 17 insertions(+), 27 deletions(-)

diff --git a/drivers/net/ethernet/natsemi/jazzsonic.c b/drivers/net/ethernet/natsemi/jazzsonic.c
index a6caeb567c0d..6ec25c27cb73 100644
--- a/drivers/net/ethernet/natsemi/jazzsonic.c
+++ b/drivers/net/ethernet/natsemi/jazzsonic.c
@@ -132,17 +132,14 @@ static int sonic_probe1(struct net_device *dev)
 	 * the expected location.
 	 */
 	silicon_revision = SONIC_READ(SONIC_SR);
-	if (sonic_debug > 1)
-		printk("SONIC Silicon Revision = 0x%04x\n",silicon_revision);
-
 	i = 0;
 	while (known_revisions[i] != 0xffff &&
 	       known_revisions[i] != silicon_revision)
 		i++;
 
 	if (known_revisions[i] == 0xffff) {
-		printk("SONIC ethernet controller not found (0x%4x)\n",
-		       silicon_revision);
+		pr_info("SONIC ethernet controller not found (0x%4x)\n",
+		        silicon_revision);
 		goto out;
 	}
 
@@ -248,7 +245,7 @@ static int jazz_sonic_probe(struct platform_device *pdev)
 	if (err)
 		goto out1;
 
-	printk("%s: MAC %pM IRQ %d\n", dev->name, dev->dev_addr, dev->irq);
+	netdev_info(dev, "MAC %pM, IRQ %d\n", dev->dev_addr, dev->irq);
 
 	return 0;
 
diff --git a/drivers/net/ethernet/natsemi/macsonic.c b/drivers/net/ethernet/natsemi/macsonic.c
index 3ca6ae7caf55..795d71522617 100644
--- a/drivers/net/ethernet/natsemi/macsonic.c
+++ b/drivers/net/ethernet/natsemi/macsonic.c
@@ -315,8 +315,6 @@ static int mac_onboard_sonic_probe(struct net_device *dev)
 	if (!MACH_IS_MAC)
 		return -ENODEV;
 
-	printk(KERN_INFO "Checking for internal Macintosh ethernet (SONIC).. ");
-
 	/* Bogus probing, on the models which may or may not have
 	   Ethernet (BTW, the Ethernet *is* always at the same
 	   address, and nothing else lives there, at least if Apple's
@@ -329,14 +327,12 @@ static int mac_onboard_sonic_probe(struct net_device *dev)
 
 		card_present = hwreg_present((void*)ONBOARD_SONIC_REGISTERS);
 		if (!card_present) {
-			printk("none.\n");
+			pr_info("No onboard or comm-slot SONIC was detected\n");
 			return -ENODEV;
 		}
 		commslot = 1;
 	}
 
-	printk("yes\n");
-
 	/* Danger!  My arms are flailing wildly!  You *must* set lp->reg_offset
 	 * and dev->base_addr before using SONIC_READ() or SONIC_WRITE() */
 	dev->base_addr = ONBOARD_SONIC_REGISTERS;
@@ -385,10 +381,10 @@ static int mac_onboard_sonic_probe(struct net_device *dev)
 	       "%s: revision 0x%04x, using %d bit DMA and register offset %d\n",
 	       dev_name(lp->device), sr, lp->dma_bitmode?32:16, lp->reg_offset);
 
-#if 0 /* This is sometimes useful to find out how MacOS configured the card. */
-	printk(KERN_INFO "%s: DCR: 0x%04x, DCR2: 0x%04x\n", dev_name(lp->device),
-	       SONIC_READ(SONIC_DCR) & 0xffff, SONIC_READ(SONIC_DCR2) & 0xffff);
-#endif
+	/* This is sometimes useful to find out how MacOS configured the card */
+	pr_debug("%s: DCR=0x%04x, DCR2=0x%04x\n", __func__,
+	         SONIC_READ(SONIC_DCR) & 0xffff,
+	         SONIC_READ(SONIC_DCR2) & 0xffff);
 
 	/* Software reset, then initialize control registers. */
 	SONIC_WRITE(SONIC_CMD, SONIC_CR_RST);
@@ -540,10 +536,10 @@ static int mac_nubus_sonic_probe(struct net_device *dev)
 	printk(KERN_INFO "%s: revision 0x%04x, using %d bit DMA and register offset %d\n",
 	       dev_name(lp->device), SONIC_READ(SONIC_SR), dma_bitmode?32:16, reg_offset);
 
-#if 0 /* This is sometimes useful to find out how MacOS configured the card. */
-	printk(KERN_INFO "%s: DCR: 0x%04x, DCR2: 0x%04x\n", dev_name(lp->device),
-	       SONIC_READ(SONIC_DCR) & 0xffff, SONIC_READ(SONIC_DCR2) & 0xffff);
-#endif
+	/* This is sometimes useful to find out how MacOS configured the card */
+	pr_debug("%s: DCR=0x%04x, DCR2=0x%04x\n", __func__,
+	         SONIC_READ(SONIC_DCR) & 0xffff,
+	         SONIC_READ(SONIC_DCR2) & 0xffff);
 
 	/* Software reset, then initialize control registers. */
 	SONIC_WRITE(SONIC_CMD, SONIC_CR_RST);
@@ -594,7 +590,7 @@ static int mac_sonic_probe(struct platform_device *pdev)
 	if (err)
 		goto out;
 
-	printk("%s: MAC %pM IRQ %d\n", dev->name, dev->dev_addr, dev->irq);
+	netdev_info(dev, "MAC %pM, IRQ %d\n", dev->dev_addr, dev->irq);
 
 	return 0;
 
diff --git a/drivers/net/ethernet/natsemi/xtsonic.c b/drivers/net/ethernet/natsemi/xtsonic.c
index 9ee0f69a83c0..bed89e8105e9 100644
--- a/drivers/net/ethernet/natsemi/xtsonic.c
+++ b/drivers/net/ethernet/natsemi/xtsonic.c
@@ -145,17 +145,14 @@ static int __init sonic_probe1(struct net_device *dev)
 	 * the expected location.
 	 */
 	silicon_revision = SONIC_READ(SONIC_SR);
-	if (sonic_debug > 1)
-		printk("SONIC Silicon Revision = 0x%04x\n",silicon_revision);
-
 	i = 0;
 	while ((known_revisions[i] != 0xffff) &&
 			(known_revisions[i] != silicon_revision))
 		i++;
 
 	if (known_revisions[i] == 0xffff) {
-		printk("SONIC ethernet controller not found (0x%4x)\n",
-				silicon_revision);
+		pr_info("SONIC ethernet controller not found (0x%4x)\n",
+		        silicon_revision);
 		return -ENODEV;
 	}
 
@@ -275,8 +272,8 @@ int xtsonic_probe(struct platform_device *pdev)
 	if ((err = register_netdev(dev)))
 		goto out1;
 
-	printk("%s: SONIC ethernet @%08lx, MAC %pM, IRQ %d\n", dev->name,
-	       dev->base_addr, dev->dev_addr, dev->irq);
+	netdev_info(dev, "SONIC ethernet @%08lx, MAC %pM, IRQ %d\n",
+	            dev->base_addr, dev->dev_addr, dev->irq);
 
 	return 0;
 
-- 
2.13.5

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH RESEND net 3/9] net/mac89x0: Fix and modernize log messages
  2017-10-03  1:07 [PATCH RESEND net 0/9] Fixes, cleanup and modernization for some legacy ethernet NIC drivers Finn Thain
  2017-10-03  1:07 ` [PATCH RESEND net 9/9] net/mac8390: Fix log messages Finn Thain
  2017-10-03  1:07 ` [PATCH RESEND net 2/9] net/mac89x0: Remove dead or unreachable code Finn Thain
@ 2017-10-03  1:07 ` Finn Thain
  2017-10-03  1:07 ` [PATCH RESEND net 7/9] net/sonic: Replace custom debug logging with netif_* calls Finn Thain
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Finn Thain @ 2017-10-03  1:07 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, linux-kernel

Fix misplaced newlines in conditional log messages.
Add missing printk severity levels.
Log the MAC address after the interface gets a meaningful name.
Drop deprecated "out of memory" message as per checkpatch advice.

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
---
 drivers/net/ethernet/cirrus/mac89x0.c | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/cirrus/mac89x0.c b/drivers/net/ethernet/cirrus/mac89x0.c
index 8fc43c865621..4393d9b89f33 100644
--- a/drivers/net/ethernet/cirrus/mac89x0.c
+++ b/drivers/net/ethernet/cirrus/mac89x0.c
@@ -56,6 +56,8 @@
   local_irq_{dis,en}able()
 */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 static const char version[] =
 "cs89x0.c:v1.02 11/26/96 Russell Nelson <nelson@crynwr.com>\n";
 
@@ -243,16 +245,14 @@ struct net_device * __init mac89x0_probe(int unit)
 	if (net_debug && version_printed++ == 0)
 		printk(version);
 
-	printk(KERN_INFO "%s: cs89%c0%s rev %c found at %#8lx",
-	       dev->name,
-	       lp->chip_type==CS8900?'0':'2',
-	       lp->chip_type==CS8920M?"M":"",
-	       lp->chip_revision,
-	       dev->base_addr);
+	pr_info("CS89%c0%s rev %c found at %#8lx\n",
+	        lp->chip_type == CS8900 ? '0' : '2',
+	        lp->chip_type == CS8920M ? "M" : "",
+	        lp->chip_revision, dev->base_addr);
 
 	/* Try to read the MAC address */
 	if ((readreg(dev, PP_SelfST) & (EEPROM_PRESENT | EEPROM_OK)) == 0) {
-		printk("\nmac89x0: No EEPROM, giving up now.\n");
+		pr_info("No EEPROM, giving up now\n");
 		goto out1;
         } else {
                 for (i = 0; i < ETH_ALEN; i += 2) {
@@ -265,15 +265,14 @@ struct net_device * __init mac89x0_probe(int unit)
 
 	dev->irq = SLOT2IRQ(slot);
 
-	/* print the IRQ and ethernet address. */
-
-	printk(" IRQ %d ADDR %pM\n", dev->irq, dev->dev_addr);
-
 	dev->netdev_ops		= &mac89x0_netdev_ops;
 
 	err = register_netdev(dev);
 	if (err)
 		goto out1;
+
+	netdev_info(dev, "MAC %pM, IRQ %d\n", dev->dev_addr, dev->irq);
+
 	return NULL;
 out1:
 	nubus_writew(0, dev->base_addr + ADD_PORT);
@@ -468,7 +467,6 @@ net_rx(struct net_device *dev)
 	/* Malloc up new buffer. */
 	skb = alloc_skb(length, GFP_ATOMIC);
 	if (skb == NULL) {
-		printk("%s: Memory squeeze, dropping packet.\n", dev->name);
 		dev->stats.rx_dropped++;
 		return;
 	}
@@ -556,7 +554,7 @@ static int set_mac_address(struct net_device *dev, void *addr)
 		return -EADDRNOTAVAIL;
 
 	memcpy(dev->dev_addr, saddr->sa_data, ETH_ALEN);
-	printk("%s: Setting MAC address to %pM\n", dev->name, dev->dev_addr);
+	netdev_info(dev, "Setting MAC address to %pM\n", dev->dev_addr);
 
 	/* set the Ethernet address */
 	for (i=0; i < ETH_ALEN/2; i++)
@@ -580,7 +578,7 @@ init_module(void)
 	net_debug = debug;
         dev_cs89x0 = mac89x0_probe(-1);
 	if (IS_ERR(dev_cs89x0)) {
-                printk(KERN_WARNING "mac89x0.c: No card found\n");
+		pr_warn("No card found\n");
 		return PTR_ERR(dev_cs89x0);
 	}
 	return 0;
-- 
2.13.5

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH RESEND net 4/9] net/mac89x0: Replace custom debug logging with netif_* calls
  2017-10-03  1:07 [PATCH RESEND net 0/9] Fixes, cleanup and modernization for some legacy ethernet NIC drivers Finn Thain
                   ` (5 preceding siblings ...)
  2017-10-03  1:07 ` [PATCH RESEND net 6/9] net/sonic: Cleanup and modernize " Finn Thain
@ 2017-10-03  1:07 ` Finn Thain
  2017-10-03  1:07 ` [PATCH RESEND net 1/9] net/smc9194: Remove bogus CONFIG_MAC reference Finn Thain
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Finn Thain @ 2017-10-03  1:07 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, linux-kernel

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
---
 drivers/net/ethernet/cirrus/mac89x0.c | 47 ++++++++++++-----------------------
 1 file changed, 16 insertions(+), 31 deletions(-)

diff --git a/drivers/net/ethernet/cirrus/mac89x0.c b/drivers/net/ethernet/cirrus/mac89x0.c
index 4393d9b89f33..278df230c5f6 100644
--- a/drivers/net/ethernet/cirrus/mac89x0.c
+++ b/drivers/net/ethernet/cirrus/mac89x0.c
@@ -61,18 +61,6 @@
 static const char version[] =
 "cs89x0.c:v1.02 11/26/96 Russell Nelson <nelson@crynwr.com>\n";
 
-/* ======================= configure the driver here ======================= */
-
-/* use 0 for production, 1 for verification, >2 for debug */
-#ifndef NET_DEBUG
-#define NET_DEBUG 0
-#endif
-
-/* ======================= end of configuration ======================= */
-
-
-/* Always include 'config.h' first in case the user wants to turn on
-   or override something. */
 #include <linux/module.h>
 
 /*
@@ -107,10 +95,13 @@ static const char version[] =
 
 #include "cs89x0.h"
 
-static unsigned int net_debug = NET_DEBUG;
+static int debug = -1;
+module_param(debug, int, 0);
+MODULE_PARM_DESC(debug, "debug message level");
 
 /* Information that need to be kept for each board. */
 struct net_local {
+	int msg_enable;
 	int chip_type;		/* one of: CS8900, CS8920, CS8920M */
 	char chip_revision;	/* revision letter of the chip ('A'...) */
 	int send_cmd;		/* the propercommand used to send a packet. */
@@ -174,7 +165,6 @@ struct net_device * __init mac89x0_probe(int unit)
 {
 	struct net_device *dev;
 	struct net_local *lp;
-	static unsigned version_printed;
 	int i, slot;
 	unsigned rev_type = 0;
 	unsigned long ioaddr;
@@ -220,6 +210,8 @@ struct net_device * __init mac89x0_probe(int unit)
 	/* Initialize the net_device structure. */
 	lp = netdev_priv(dev);
 
+	lp->msg_enable = netif_msg_init(debug, 0);
+
 	/* Fill in the 'dev' fields. */
 	dev->base_addr = ioaddr;
 	dev->mem_start = (unsigned long)
@@ -242,8 +234,7 @@ struct net_device * __init mac89x0_probe(int unit)
 	if (lp->chip_type != CS8900 && lp->chip_revision >= 'C')
 		lp->send_cmd = TX_NOW;
 
-	if (net_debug && version_printed++ == 0)
-		printk(version);
+	netif_dbg(lp, drv, dev, "%s", version);
 
 	pr_info("CS89%c0%s rev %c found at %#8lx\n",
 	        lp->chip_type == CS8900 ? '0' : '2',
@@ -340,11 +331,9 @@ net_send_packet(struct sk_buff *skb, struct net_device *dev)
 	struct net_local *lp = netdev_priv(dev);
 	unsigned long flags;
 
-	if (net_debug > 3)
-		printk("%s: sent %d byte packet of type %x\n",
-		       dev->name, skb->len,
-		       (skb->data[ETH_ALEN+ETH_ALEN] << 8)
-		       | skb->data[ETH_ALEN+ETH_ALEN+1]);
+	netif_dbg(lp, tx_queued, dev, "sent %d byte packet of type %x\n",
+	          skb->len, skb->data[ETH_ALEN + ETH_ALEN] << 8 |
+	                    skb->data[ETH_ALEN + ETH_ALEN + 1]);
 
 	/* keep the upload from being interrupted, since we
 	   ask the chip to start transmitting before the
@@ -393,7 +382,7 @@ static irqreturn_t net_interrupt(int irq, void *dev_id)
            faster than you can read them off, you're screwed.  Hasta la
            vista, baby!  */
 	while ((status = swab16(nubus_readw(dev->base_addr + ISQ_PORT)))) {
-		if (net_debug > 4)printk("%s: event=%04x\n", dev->name, status);
+		netif_dbg(lp, intr, dev, "status=%04x\n", status);
 		switch(status & ISQ_EVENT_MASK) {
 		case ISQ_RECEIVER_EVENT:
 			/* Got a packet(s). */
@@ -423,7 +412,7 @@ static irqreturn_t net_interrupt(int irq, void *dev_id)
 				netif_wake_queue(dev);
 			}
 			if (status & TX_UNDERRUN) {
-				if (net_debug > 0) printk("%s: transmit underrun\n", dev->name);
+				netif_dbg(lp, tx_err, dev, "transmit underrun\n");
                                 lp->send_underrun++;
                                 if (lp->send_underrun == 3) lp->send_cmd = TX_AFTER_381;
                                 else if (lp->send_underrun == 6) lp->send_cmd = TX_AFTER_ALL;
@@ -444,6 +433,7 @@ static irqreturn_t net_interrupt(int irq, void *dev_id)
 static void
 net_rx(struct net_device *dev)
 {
+	struct net_local *lp = netdev_priv(dev);
 	struct sk_buff *skb;
 	int status, length;
 
@@ -475,10 +465,9 @@ net_rx(struct net_device *dev)
 	skb_copy_to_linear_data(skb, (void *)(dev->mem_start + PP_RxFrame),
 				length);
 
-	if (net_debug > 3)printk("%s: received %d byte packet of type %x\n",
-                                 dev->name, length,
-                                 (skb->data[ETH_ALEN+ETH_ALEN] << 8)
-				 | skb->data[ETH_ALEN+ETH_ALEN+1]);
+	netif_dbg(lp, rx_status, dev, "received %d byte packet of type %x\n",
+	          length, skb->data[ETH_ALEN + ETH_ALEN] << 8 |
+	                  skb->data[ETH_ALEN + ETH_ALEN + 1]);
 
         skb->protocol=eth_type_trans(skb,dev);
 	netif_rx(skb);
@@ -566,16 +555,12 @@ static int set_mac_address(struct net_device *dev, void *addr)
 #ifdef MODULE
 
 static struct net_device *dev_cs89x0;
-static int debug;
 
-module_param(debug, int, 0);
-MODULE_PARM_DESC(debug, "CS89[02]0 debug level (0-5)");
 MODULE_LICENSE("GPL");
 
 int __init
 init_module(void)
 {
-	net_debug = debug;
         dev_cs89x0 = mac89x0_probe(-1);
 	if (IS_ERR(dev_cs89x0)) {
 		pr_warn("No card found\n");
-- 
2.13.5

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH RESEND net 2/9] net/mac89x0: Remove dead or unreachable code
  2017-10-03  1:07 [PATCH RESEND net 0/9] Fixes, cleanup and modernization for some legacy ethernet NIC drivers Finn Thain
  2017-10-03  1:07 ` [PATCH RESEND net 9/9] net/mac8390: Fix log messages Finn Thain
@ 2017-10-03  1:07 ` Finn Thain
  2017-10-03  1:07 ` [PATCH RESEND net 3/9] net/mac89x0: Fix and modernize log messages Finn Thain
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Finn Thain @ 2017-10-03  1:07 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, linux-kernel

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
---
 drivers/net/ethernet/cirrus/mac89x0.c | 36 -----------------------------------
 1 file changed, 36 deletions(-)

diff --git a/drivers/net/ethernet/cirrus/mac89x0.c b/drivers/net/ethernet/cirrus/mac89x0.c
index f910f0f386d6..8fc43c865621 100644
--- a/drivers/net/ethernet/cirrus/mac89x0.c
+++ b/drivers/net/ethernet/cirrus/mac89x0.c
@@ -119,10 +119,6 @@ struct net_local {
 };
 
 /* Index to functions, as function prototypes. */
-
-#if 0
-extern void reset_chip(struct net_device *dev);
-#endif
 static int net_open(struct net_device *dev);
 static int net_send_packet(struct sk_buff *skb, struct net_device *dev);
 static irqreturn_t net_interrupt(int irq, void *dev_id);
@@ -132,10 +128,6 @@ static int net_close(struct net_device *dev);
 static struct net_device_stats *net_get_stats(struct net_device *dev);
 static int set_mac_address(struct net_device *dev, void *addr);
 
-
-/* Example routines you must write ;->. */
-#define tx_done(dev) 1
-
 /* For reading/writing registers ISA-style */
 static inline int
 readreg_io(struct net_device *dev, int portno)
@@ -179,7 +171,6 @@ static const struct net_device_ops mac89x0_netdev_ops = {
 struct net_device * __init mac89x0_probe(int unit)
 {
 	struct net_device *dev;
-	static int once_is_enough;
 	struct net_local *lp;
 	static unsigned version_printed;
 	int i, slot;
@@ -200,10 +191,6 @@ struct net_device * __init mac89x0_probe(int unit)
 		netdev_boot_setup_check(dev);
 	}
 
-	if (once_is_enough)
-		goto out;
-	once_is_enough = 1;
-
 	/* We might have to parameterize this later */
 	slot = 0xE;
 	/* Get out now if there's a real NuBus card in slot E */
@@ -295,24 +282,6 @@ struct net_device * __init mac89x0_probe(int unit)
 	return ERR_PTR(err);
 }
 
-#if 0
-/* This is useful for something, but I don't know what yet. */
-void __init reset_chip(struct net_device *dev)
-{
-	int reset_start_time;
-
-	writereg(dev, PP_SelfCTL, readreg(dev, PP_SelfCTL) | POWER_ON_RESET);
-
-	/* wait 30 ms */
-	msleep_interruptible(30);
-
-	/* Wait until the chip is reset */
-	reset_start_time = jiffies;
-	while( (readreg(dev, PP_SelfST) & INIT_DONE) == 0 && jiffies - reset_start_time < 2)
-		;
-}
-#endif
-
 /* Open/initialize the board.  This is called (in the current kernel)
    sometime after booting when the 'ifconfig' program is run.
 
@@ -414,11 +383,6 @@ static irqreturn_t net_interrupt(int irq, void *dev_id)
 	struct net_local *lp;
 	int ioaddr, status;
 
-	if (dev == NULL) {
-		printk ("net_interrupt(): irq %d for unknown device.\n", irq);
-		return IRQ_NONE;
-	}
-
 	ioaddr = dev->base_addr;
 	lp = netdev_priv(dev);
 
-- 
2.13.5

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH RESEND net 0/9] Fixes, cleanup and modernization for some legacy ethernet NIC drivers
  2017-10-03  1:07 [PATCH RESEND net 0/9] Fixes, cleanup and modernization for some legacy ethernet NIC drivers Finn Thain
                   ` (8 preceding siblings ...)
  2017-10-03  1:07 ` [PATCH RESEND net 8/9] net/8390: Fix redundant code Finn Thain
@ 2017-10-04  4:41 ` David Miller
  2017-10-04  6:16   ` Finn Thain
  9 siblings, 1 reply; 12+ messages in thread
From: David Miller @ 2017-10-04  4:41 UTC (permalink / raw)
  To: fthain; +Cc: netdev, linux-kernel

From: Finn Thain <fthain@telegraphics.com.au>
Date: Mon,  2 Oct 2017 21:07:17 -0400 (EDT)

> This patch series fixes some logging bugs and adds some missing message
> severity levels.
> 
> There are also cleanup patches for dead code and some Kconfig cruft.
> 
> Custom debug message logging is converted to netif_* calls to reduce
> code duplication.
> 
> All up, about 150 lines of code are eliminated.
> 
> My apologies for duplicated messages. I messed up the addressing.

Finn, I'm finding real bugs in this series and seriously if you cannot
test these changes in some way please leave this code alone.

For example, you're removing the "once_is_enough" logic from
mac89x0_probe().

But you can't do that.  The probe function can in fact be called
multiple times, from drivers/net/Space.c  It gets called in a loop
iterating over different 'unit' argument values.

Unless you're making stylistic changes where you can prove the object
code resulting is still the same, you really should not be playing
with fire by trying to remove "dead code" like this in legacy drivers
you cannot fully test.

Thank you.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH RESEND net 0/9] Fixes, cleanup and modernization for some legacy ethernet NIC drivers
  2017-10-04  4:41 ` [PATCH RESEND net 0/9] Fixes, cleanup and modernization for some legacy ethernet NIC drivers David Miller
@ 2017-10-04  6:16   ` Finn Thain
  0 siblings, 0 replies; 12+ messages in thread
From: Finn Thain @ 2017-10-04  6:16 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-kernel

On Tue, 3 Oct 2017, David Miller wrote:

> From: Finn Thain <fthain@telegraphics.com.au>
> Date: Mon,  2 Oct 2017 21:07:17 -0400 (EDT)
> 
> > This patch series fixes some logging bugs and adds some missing 
> > message severity levels.
> > 
> > There are also cleanup patches for dead code and some Kconfig cruft.
> > 
> > Custom debug message logging is converted to netif_* calls to reduce 
> > code duplication.
> > 
> > All up, about 150 lines of code are eliminated.
> > 
> > My apologies for duplicated messages. I messed up the addressing.
> 
> Finn, I'm finding real bugs in this series and seriously if you cannot 
> test these changes in some way please leave this code alone.
> 

Well, this series has been tested on SONIC, MACE and 8390 devices. I do 
have 89x0 hardware but it is stored elsewhere and I can't get to it right 
now.

> For example, you're removing the "once_is_enough" logic from 
> mac89x0_probe().
> 
> But you can't do that.

I take your point. I overlooked the Space.c logic. But that's not the 
whole story here.

> The probe function can in fact be called multiple times, from 
> drivers/net/Space.c It gets called in a loop iterating over different 
> 'unit' argument values.
> 

Testing shows that mac8390 doesn't probe any devices unless it is modular. 
In the CONFIG_MAC8390=m case, the mac8390_probe() call in Space.c is 
elided by '#ifdef CONFIG_MAC8390' and the device gets probed by 
init_module() instead. (And the binaries published in the Debian archive 
and the mac68k project on sourceforge all use CONFIG_MAC8390=m and 
CONFIG_MAC89x0=m.)

In the CONFIG_MAC8390=y case, no device is probed (for some reason). This 
test result suggests that the probe function is not actually called. Its 
safe to assume that mac89x0 has the same problem as mac8390, and it 
follows that the once_is_enough logic is redundant. At least, that was my 
thinking when I made the change; that is, init_module() must be the only 
caller of the probe function.

BTW, mac8390 does this,

                if (slots & (1 << ndev->board->slot))
                        continue;

which is equivalent to the once_is_enough test, and I went looking for 
some way to avoid this too.

Anyway, I see now that changing the once_is_enough logic is wrong. First 
the bug has to be found or else bus matching implemented for nubus 
drivers.

> Unless you're making stylistic changes where you can prove the object 
> code resulting is still the same, you really should not be playing with 
> fire by trying to remove "dead code" like this in legacy drivers you 
> cannot fully test.
> 

I agree, and that's why I've been careful with any code I cannot test 
(like xtsonic) by making only changes that I can test (with macsonic).
I should have added tested-by tags...

> Thank you.
> 

Thanks for your review. I will revert the once_is_enough change, describe 
the testing better and re-submit.

-- 

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2017-10-04  6:16 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-03  1:07 [PATCH RESEND net 0/9] Fixes, cleanup and modernization for some legacy ethernet NIC drivers Finn Thain
2017-10-03  1:07 ` [PATCH RESEND net 9/9] net/mac8390: Fix log messages Finn Thain
2017-10-03  1:07 ` [PATCH RESEND net 2/9] net/mac89x0: Remove dead or unreachable code Finn Thain
2017-10-03  1:07 ` [PATCH RESEND net 3/9] net/mac89x0: Fix and modernize log messages Finn Thain
2017-10-03  1:07 ` [PATCH RESEND net 7/9] net/sonic: Replace custom debug logging with netif_* calls Finn Thain
2017-10-03  1:07 ` [PATCH RESEND net 5/9] net/macmace: Fix and cleanup log messages Finn Thain
2017-10-03  1:07 ` [PATCH RESEND net 6/9] net/sonic: Cleanup and modernize " Finn Thain
2017-10-03  1:07 ` [PATCH RESEND net 4/9] net/mac89x0: Replace custom debug logging with netif_* calls Finn Thain
2017-10-03  1:07 ` [PATCH RESEND net 1/9] net/smc9194: Remove bogus CONFIG_MAC reference Finn Thain
2017-10-03  1:07 ` [PATCH RESEND net 8/9] net/8390: Fix redundant code Finn Thain
2017-10-04  4:41 ` [PATCH RESEND net 0/9] Fixes, cleanup and modernization for some legacy ethernet NIC drivers David Miller
2017-10-04  6:16   ` Finn Thain

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).