netdev - Re: [PATCH 2/2] net: dsa: introduce MICREL KSZ8893MQL/BL ethernet switch chip support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1279730427.1714.34.camel@Joe-Laptop.home>
Date:	Wed, 21 Jul 2010 09:40:27 -0700
From:	Joe Perches <joe@...ches.com>
To:	Eric Dumazet <eric.dumazet@...il.com>
Cc:	Lennert Buytenhek <buytenh@...tstofly.org>,
	Mike Frysinger <vapier@...too.org>, netdev@...r.kernel.org,
	"David S. Miller" <davem@...emloft.net>,
	uclinux-dist-devel@...ckfin.uclinux.org,
	Karl Beldan <karl.beldan@...il.com>,
	Graf Yang <graf.yang@...log.com>,
	Bryan Wu <cooloney@...nel.org>
Subject: Re: [PATCH 2/2] net: dsa: introduce MICREL KSZ8893MQL/BL ethernet
 switch chip support

On Wed, 2010-07-21 at 17:33 +0200, Eric Dumazet wrote:
> It is used by all pr_err() friends

Shouldn't these drivers be moved into
something like drivers/net/dsa/ rather
than be kept in net?

The #define pr_fmt would be more standard as
	#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
The printks could be netdev_<level>

Suggested patches:

diff --git a/net/dsa/ksz8893m.c b/net/dsa/ksz8893m.c
index 98cce04..80fb0ac 100644
--- a/net/dsa/ksz8893m.c
+++ b/net/dsa/ksz8893m.c
@@ -8,7 +8,7 @@
  * Licensed under the GPL-2 or later.
  */
 
-#define pr_fmt(fmt) "ksz8893m: " fmt
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
 #include <linux/list.h>
 #include <linux/netdevice.h>
@@ -42,11 +42,14 @@ static int ksz8893m_read(unsigned char *din, unsigned char reg, int len)
 	spi_message_init(&message);
 	spi_message_add_tail(t, &message);
 	ret = spi_sync(sw.dev, &message);
-	if (!ret)
-		return message.status;
+	if (ret) {
+		pr_err("read reg%d failed, ret=%d\n", reg, ret);
+		return ret;
+	}
+	if (message.status)
+		pr_err("read reg%d failed, status=%d\n", reg, message.status);
 
-	pr_err("read reg%d failed, ret=%d\n", reg, ret);
-	return ret;
+	return message.status;
 }
 
 static int ksz8893m_write(unsigned char *dout, unsigned char reg, int len)
@@ -65,11 +68,14 @@ static int ksz8893m_write(unsigned char *dout, unsigned char reg, int len)
 	spi_message_init(&message);
 	spi_message_add_tail(t, &message);
 	ret = spi_sync(sw.dev, &message);
-	if (!ret)
-		return message.status;
+	if (ret) {
+		pr_err("write reg%d failed, ret=%d\n", reg, ret);
+		return ret;
+	}
+	if (message.status)
+		pr_err("write reg%d failed, status=%d\n", reg, message.status);
 
-	pr_err("write reg%d failed, ret=%d\n", reg, ret);
-	return ret;
+	return message.status;
 }
 
 static char *ksz8893m_probe(struct mii_bus *bus, int sw_addr)
@@ -168,8 +174,8 @@ static int ksz8893m_setup_port(struct dsa_switch *ds, int p)
 	if (val < 0)
 		return val;
 	val |= AN_ENABLE | FORCE_100 | FORCE_FULL_DUPLEX;
-	val &= ~(POWER_DOWN | DISABLE_MDIX | DIS_FAR_END_FAULT |\
-			DISABLE_TRANSMIT | DISABLE_LED);
+	val &= ~(POWER_DOWN | DISABLE_MDIX | DIS_FAR_END_FAULT |
+		 DISABLE_TRANSMIT | DISABLE_LED);
 	ret = mdiobus_write(ds->master_mii_bus, p, MII_BMCR, val);
 	if (ret < 0)
 		return ret;
@@ -228,8 +234,7 @@ ksz8893m_phy_read(struct dsa_switch *ds, int port, int regnum)
 }
 
 static int
-ksz8893m_phy_write(struct dsa_switch *ds,
-			      int port, int regnum, u16 val)
+ksz8893m_phy_write(struct dsa_switch *ds, int port, int regnum, u16 val)
 {
 	int phy_addr = ksz8893m_port_to_phy_addr(port);
 	return mdiobus_write(ds->master_mii_bus, phy_addr, regnum, val);
@@ -263,7 +268,7 @@ static void ksz8893m_poll_link(struct dsa_switch *ds)
 
 		if (!link) {
 			if (netif_carrier_ok(dev)) {
-				printk(KERN_INFO "%s: link down\n", dev->name);
+				netdev_info(dev, "link down\n");
 				netif_carrier_off(dev);
 			}
 			continue;
@@ -274,8 +279,8 @@ static void ksz8893m_poll_link(struct dsa_switch *ds)
 		val = mdiobus_read(ds->master_mii_bus, i, MII_BMSR);
 		if (val < 0)
 			continue;
-		val &= HALF_10_CAPABLE | FULL_10_CAPABLE |\
-		       HALF_100_CAPABLE | FULL_100_CAPABLE;
+		val &= (HALF_10_CAPABLE | FULL_10_CAPABLE |
+			HALF_100_CAPABLE | FULL_100_CAPABLE);
 		if (val & FULL_100_CAPABLE) {
 			speed = 100;
 			duplex = 1;
@@ -288,8 +293,8 @@ static void ksz8893m_poll_link(struct dsa_switch *ds)
 		}
 
 		if (!netif_carrier_ok(dev)) {
-			printk(KERN_INFO "%s: link up, %d Mb/s, %s duplex\n",
-					dev->name, speed, duplex ? "full" : "half");
+			netdev_info(dev, "link up, %d Mb/s, %s duplex\n",
+				    speed, duplex ? "full" : "half");
 			netif_carrier_on(dev);
 		}
 	}



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html