[PATCH 1/5] sis900 printk audit

[PATCH 1/5] sis900 printk audit

[Daniele Venzano]

[-- Attachment #1: Type: text/plain, Size: 291 bytes --]



Infrastructure needed for standard netif messages
 - add msg_level to sis900_private
 - define default msg level
 - set default value for sis900_debug
Update module parameter description
Ethtool support for debugging output level
   


Signed-off-by: Daniele Venzano <webvenza@libero.it>


[-- Attachment #2: Type: text/plain, Size: 2495 bytes --]

Index: sis900.c
===================================================================
--- a/drivers/net/sis900.c	(revision 83)
+++ b/drivers/net/sis900.c	(revision 84)
@@ -82,9 +82,14 @@
 static int max_interrupt_work = 40;
 static int multicast_filter_limit = 128;
 
-#define sis900_debug debug
-static int sis900_debug;
+static int sis900_debug = -1; /* Use SIS900_DEF_MSG as value */
 
+#define SIS900_DEF_MSG \
+	(NETIF_MSG_DRV		| \
+	 NETIF_MSG_LINK		| \
+	 NETIF_MSG_RX_ERR	| \
+	 NETIF_MSG_TX_ERR)
+
 /* Time in jiffies before concluding the transmitter is hung. */
 #define TX_TIMEOUT  (4*HZ)
 /* SiS 900 is capable of 32 bits BM DMA */
@@ -160,6 +165,8 @@
 	struct timer_list timer; /* Link status detection timer. */
 	u8 autong_complete; /* 1: auto-negotiate complete  */
 
+	u32 msg_enable;
+
 	unsigned int cur_rx, dirty_rx; /* producer/comsumer pointers for Tx/Rx ring */
 	unsigned int cur_tx, dirty_tx;
 
@@ -182,10 +189,10 @@
 
 module_param(multicast_filter_limit, int, 0444);
 module_param(max_interrupt_work, int, 0444);
-module_param(debug, int, 0444);
+module_param(sis900_debug, int, 0444);
 MODULE_PARM_DESC(multicast_filter_limit, "SiS 900/7016 maximum number of filtered multicast addresses");
 MODULE_PARM_DESC(max_interrupt_work, "SiS 900/7016 maximum events handled per interrupt");
-MODULE_PARM_DESC(debug, "SiS 900/7016 debug level (2-4)");
+MODULE_PARM_DESC(sis900_debug, "SiS 900/7016 bitmapped debugging message level");
 
 static int sis900_open(struct net_device *net_dev);
 static int sis900_mii_probe (struct net_device * net_dev);
@@ -456,6 +463,11 @@
 	net_dev->tx_timeout = sis900_tx_timeout;
 	net_dev->watchdog_timeo = TX_TIMEOUT;
 	net_dev->ethtool_ops = &sis900_ethtool_ops;
+
+	if (sis900_debug > 0)
+		sis_priv->msg_enable = sis900_debug;
+	else
+		sis_priv->msg_enable = SIS900_DEF_MSG;
 	
 	ret = register_netdev(net_dev);
 	if (ret)
@@ -1906,8 +1918,22 @@
 	strcpy (info->bus_info, pci_name(sis_priv->pci_dev));
 }
 
+static u32 sis900_get_msglevel(struct net_device *net_dev)
+{
+	struct sis900_private *sis_priv = net_dev->priv;
+	return sis_priv->msg_enable;
+}
+  
+static void sis900_set_msglevel(struct net_device *net_dev, u32 value)
+{
+	struct sis900_private *sis_priv = net_dev->priv;
+	sis_priv->msg_enable = value;
+}
+
 static struct ethtool_ops sis900_ethtool_ops = {
-	.get_drvinfo =		sis900_get_drvinfo,
+	.get_drvinfo 	= sis900_get_drvinfo,
+	.get_msglevel	= sis900_get_msglevel,
+	.set_msglevel	= sis900_set_msglevel,
 };
 
 /**

[PATCH 2/5] sis900 printk audit

[Daniele Venzano]

[-- Attachment #1: Type: text/plain, Size: 107 bytes --]



Version bump
Remove broken link to documentation



Signed-off-by: Daniele Venzano <webvenza@libero.it>


[-- Attachment #2: Type: text/plain, Size: 1300 bytes --]

Index: sis900.c
===================================================================
--- a/drivers/net/sis900.c	(revision 84)
+++ b/drivers/net/sis900.c	(revision 85)
@@ -1,6 +1,6 @@
 /* sis900.c: A SiS 900/7016 PCI Fast Ethernet driver for Linux.
    Copyright 1999 Silicon Integrated System Corporation 
-   Revision:	1.08.06 Sep. 24 2002
+   Revision:	1.08.08 Jan. 22 2005
    
    Modified from the driver which is originally written by Donald Becker.
    
@@ -16,8 +16,8 @@
    preliminary Rev. 1.0 Nov. 10, 1998
    SiS 7014 Single Chip 100BASE-TX/10BASE-T Physical Layer Solution,
    preliminary Rev. 1.0 Jan. 18, 1998
-   http://www.sis.com.tw/support/databook.htm
 
+   Rev 1.08.08 Jan. 22 2005 Daniele Venzano use netif_msg for debugging messages
    Rev 1.08.07 Nov.  2 2003 Daniele Venzano <webvenza@libero.it> add suspend/resume support
    Rev 1.08.06 Sep. 24 2002 Mufasa Yang bug fix for Tx timeout & add SiS963 support
    Rev 1.08.05 Jun.  6 2002 Mufasa Yang bug fix for read_eeprom & Tx descriptor over-boundary
@@ -74,7 +74,7 @@
 #include "sis900.h"
 
 #define SIS900_MODULE_NAME "sis900"
-#define SIS900_DRV_VERSION "v1.08.07 11/02/2003"
+#define SIS900_DRV_VERSION "v1.08.08 Jan. 22 2005"
 
 static char version[] __devinitdata =
 KERN_INFO "sis900.c: " SIS900_DRV_VERSION "\n";

[PATCH 3/5] sis900 printk audit

[Daniele Venzano]

[-- Attachment #1: Type: text/plain, Size: 140 bytes --]



Change priority of printk where appropriate
Remove two cryptic and useless printk



Signed-off-by: Daniele Venzano <webvenza@libero.it>


[-- Attachment #2: Type: text/plain, Size: 3949 bytes --]

Index: sis900.c
===================================================================
--- a/drivers/net/sis900.c	(revision 85)
+++ b/drivers/net/sis900.c	(revision 86)
@@ -242,7 +242,7 @@
 	/* check to see if we have sane EEPROM */
 	signature = (u16) read_eeprom(ioaddr, EEPROMSignature);    
 	if (signature == 0xffff || signature == 0x0000) {
-		printk (KERN_INFO "%s: Error EERPOM read %x\n", 
+		printk (KERN_WARNING "%s: Error EERPOM read %x\n", 
 			net_dev->name, signature);
 		return 0;
 	}
@@ -275,7 +275,7 @@
 	if (!isa_bridge)
 		isa_bridge = pci_get_device(PCI_VENDOR_ID_SI, 0x0018, isa_bridge);
 	if (!isa_bridge) {
-		printk("%s: Can not find ISA bridge\n", net_dev->name);
+		printk(KERN_WARNING "%s: Can not find ISA bridge\n", net_dev->name);
 		return 0;
 	}
 	pci_read_config_byte(isa_bridge, 0x48, &reg);
@@ -567,7 +567,7 @@
 			continue;
 		
 		if ((mii_phy = kmalloc(sizeof(struct mii_phy), GFP_KERNEL)) == NULL) {
-			printk(KERN_INFO "Cannot allocate mem for struct mii_phy\n");
+			printk(KERN_WARNING "Cannot allocate mem for struct mii_phy\n");
 			mii_phy = sis_priv->first_mii;
 			while (mii_phy) {
 				struct mii_phy *phy;
@@ -1029,7 +1029,7 @@
 		outl(w, ioaddr + rfdr);
 
 		if (sis900_debug > 2) {
-			printk(KERN_INFO "%s: Receive Filter Addrss[%d]=%x\n",
+			printk(KERN_DEBUG "%s: Receive Filter Addrss[%d]=%x\n",
 			       net_dev->name, i, inl(ioaddr + rfdr));
 		}
 	}
@@ -1067,7 +1067,7 @@
 	/* load Transmit Descriptor Register */
 	outl(sis_priv->tx_ring_dma, ioaddr + txdp);
 	if (sis900_debug > 2)
-		printk(KERN_INFO "%s: TX descriptor register loaded with: %8.8x\n",
+		printk(KERN_DEBUG "%s: TX descriptor register loaded with: %8.8x\n",
 		       net_dev->name, inl(ioaddr + txdp));
 }
 
@@ -1121,7 +1121,7 @@
 	/* load Receive Descriptor Register */
 	outl(sis_priv->rx_ring_dma, ioaddr + rxdp);
 	if (sis900_debug > 2)
-		printk(KERN_INFO "%s: RX descriptor register loaded with: %8.8x\n",
+		printk(KERN_DEBUG "%s: RX descriptor register loaded with: %8.8x\n",
 		       net_dev->name, inl(ioaddr + rxdp));
 }
 
@@ -1571,7 +1571,7 @@
 	net_dev->trans_start = jiffies;
 
 	if (sis900_debug > 3)
-		printk(KERN_INFO "%s: Queued Tx packet at %p size %d "
+		printk(KERN_DEBUG "%s: Queued Tx packet at %p size %d "
 		       "to slot %d.\n",
 		       net_dev->name, skb->data, (int)skb->len, entry);
 
@@ -1631,7 +1631,7 @@
 	} while (1);
 
 	if (sis900_debug > 3)
-		printk(KERN_INFO "%s: exiting interrupt, "
+		printk(KERN_DEBUG "%s: exiting interrupt, "
 		       "interrupt status = 0x%#8.8x.\n",
 		       net_dev->name, inl(ioaddr + isr));
 	
@@ -1657,7 +1657,7 @@
 	u32 rx_status = sis_priv->rx_ring[entry].cmdsts;
 
 	if (sis900_debug > 3)
-		printk(KERN_INFO "sis900_rx, cur_rx:%4.4d, dirty_rx:%4.4d "
+		printk(KERN_DEBUG "sis900_rx, cur_rx:%4.4d, dirty_rx:%4.4d "
 		       "status:0x%8.8x\n",
 		       sis_priv->cur_rx, sis_priv->dirty_rx, rx_status);
 
@@ -1669,7 +1669,7 @@
 		if (rx_status & (ABORT|OVERRUN|TOOLONG|RUNT|RXISERR|CRCERR|FAERR)) {
 			/* corrupted packet received */
 			if (sis900_debug > 3)
-				printk(KERN_INFO "%s: Corrupted packet "
+				printk(KERN_DEBUG "%s: Corrupted packet "
 				       "received, buffer status = 0x%8.8x.\n",
 				       net_dev->name, rx_status);
 			sis_priv->stats.rx_errors++;
@@ -1807,7 +1807,7 @@
 		if (tx_status & (ABORT | UNDERRUN | OWCOLL)) {
 			/* packet unsuccessfully transmitted */
 			if (sis900_debug > 3)
-				printk(KERN_INFO "%s: Transmit "
+				printk(KERN_DEBUG "%s: Transmit "
 				       "error, Tx status %8.8x.\n",
 				       net_dev->name, tx_status);
 			sis_priv->stats.tx_errors++;
@@ -2074,12 +2074,10 @@
 		case IF_PORT_AUI: /* AUI */
 		case IF_PORT_100BASEFX: /* 100BaseFx */
                 	/* These Modes are not supported (are they?)*/
-			printk(KERN_INFO "Not supported");
 			return -EOPNOTSUPP;
 			break;
             
 		default:
-			printk(KERN_INFO "Invalid");
 			return -EINVAL;
 		}
 	}

[PATCH 4/5] sis900 printk audit

[Daniele Venzano]

[-- Attachment #1: Type: text/plain, Size: 141 bytes --]



Add some init debugging printk
Use netif_msg macros before printing debug messages



Signed-off-by: Daniele Venzano <webvenza@libero.it>


[-- Attachment #2: Type: text/plain, Size: 8222 bytes --]

Index: sis900.c
===================================================================
--- a/drivers/net/sis900.c	(revision 86)
+++ b/drivers/net/sis900.c	(revision 87)
@@ -475,8 +475,13 @@
 		
 	/* Get Mac address according to the chip revision */
 	pci_read_config_byte(pci_dev, PCI_CLASS_REVISION, &revision);
+
+	if(netif_msg_probe(sis_priv))
+		printk(KERN_DEBUG "%s: detected revision %2.2x, "
+				"trying to get MAC address...\n",
+				net_dev->name, revision);
+	
 	ret = 0;
-
 	if (revision == SIS630E_900_REV)
 		ret = sis630e_get_mac_addr(pci_dev, net_dev);
 	else if ((revision > 0x81) && (revision <= 0x90) )
@@ -487,6 +492,7 @@
 		ret = sis900_get_mac_addr(pci_dev, net_dev);
 
 	if (ret == 0) {
+		printk(KERN_WARNING "%s: Cannot read MAC address.\n", net_dev->name);
 		ret = -ENODEV;
 		goto err_out_unregister;
 	}
@@ -497,6 +503,7 @@
 
 	/* probe for mii transceiver */
 	if (sis900_mii_probe(net_dev) == 0) {
+		printk(KERN_WARNING "%s: Error probing MII device.\n", net_dev->name);
 		ret = -ENODEV;
 		goto err_out_unregister;
 	}
@@ -562,9 +569,13 @@
 		for(i = 0; i < 2; i++)
 			mii_status = mdio_read(net_dev, phy_addr, MII_STATUS);
 
-		if (mii_status == 0xffff || mii_status == 0x0000)
-			/* the mii is not accessible, try next one */
+		if (mii_status == 0xffff || mii_status == 0x0000) {
+			if (netif_msg_probe(sis_priv))
+				printk(KERN_DEBUG "%s: MII at address %d"
+						" not accessible\n",
+						net_dev->name, phy_addr);
 			continue;
+		}
 		
 		if ((mii_phy = kmalloc(sizeof(struct mii_phy), GFP_KERNEL)) == NULL) {
 			printk(KERN_WARNING "Cannot allocate mem for struct mii_phy\n");
@@ -593,9 +604,11 @@
 				if (mii_chip_table[i].phy_types == MIX)
 					mii_phy->phy_types =
 					    (mii_status & (MII_STAT_CAN_TX_FDX | MII_STAT_CAN_TX)) ? LAN : HOME;
-				printk(KERN_INFO "%s: %s transceiver found at address %d.\n",
-				       net_dev->name, mii_chip_table[i].name,
-				       phy_addr);
+				printk(KERN_INFO "%s: %s transceiver found "
+							"at address %d.\n",
+							net_dev->name,
+							mii_chip_table[i].name,
+							phy_addr);
 				break;
 			}
 			
@@ -1011,6 +1024,7 @@
 static void
 sis900_init_rxfilter (struct net_device * net_dev)
 {
+	struct sis900_private *sis_priv = net_dev->priv;
 	long ioaddr = net_dev->base_addr;
 	u32 rfcrSave;
 	u32 i;
@@ -1028,7 +1042,7 @@
 		outl((i << RFADDR_shift), ioaddr + rfcr);
 		outl(w, ioaddr + rfdr);
 
-		if (sis900_debug > 2) {
+		if (netif_msg_hw(sis_priv)) {
 			printk(KERN_DEBUG "%s: Receive Filter Addrss[%d]=%x\n",
 			       net_dev->name, i, inl(ioaddr + rfdr));
 		}
@@ -1066,7 +1080,7 @@
 
 	/* load Transmit Descriptor Register */
 	outl(sis_priv->tx_ring_dma, ioaddr + txdp);
-	if (sis900_debug > 2)
+	if (netif_msg_hw(sis_priv))
 		printk(KERN_DEBUG "%s: TX descriptor register loaded with: %8.8x\n",
 		       net_dev->name, inl(ioaddr + txdp));
 }
@@ -1120,7 +1134,7 @@
 
 	/* load Receive Descriptor Register */
 	outl(sis_priv->rx_ring_dma, ioaddr + rxdp);
-	if (sis900_debug > 2)
+	if (netif_msg_hw(sis_priv))
 		printk(KERN_DEBUG "%s: RX descriptor register loaded with: %8.8x\n",
 		       net_dev->name, inl(ioaddr + rxdp));
 }
@@ -1268,7 +1282,8 @@
 	/* Link ON -> OFF */
                 if (!(status & MII_STAT_LINK)){
                 	netif_carrier_off(net_dev);
-                	printk(KERN_INFO "%s: Media Link Off\n", net_dev->name);
+			if(netif_msg_link(sis_priv))
+                		printk(KERN_INFO "%s: Media Link Off\n", net_dev->name);
 
                 	/* Change mode issue */
                 	if ((mii_phy->phy_id0 == 0x001D) && 
@@ -1383,7 +1398,8 @@
 		status = mdio_read(net_dev, phy_addr, MII_STATUS);
 
 	if (!(status & MII_STAT_LINK)){
-		printk(KERN_INFO "%s: Media Link Off\n", net_dev->name);
+		if(netif_msg_link(sis_priv))
+			printk(KERN_INFO "%s: Media Link Off\n", net_dev->name);
 		sis_priv->autong_complete = 1;
 		netif_carrier_off(net_dev);
 		return;
@@ -1445,7 +1461,8 @@
 			*speed = HW_SPEED_100_MBPS;
 	}
 
-	printk(KERN_INFO "%s: Media Link On %s %s-duplex \n",
+	if(netif_msg_link(sis_priv))
+		printk(KERN_INFO "%s: Media Link On %s %s-duplex \n",
 	       				net_dev->name,
 	       				*speed == HW_SPEED_100_MBPS ?
 	       					"100mbps" : "10mbps",
@@ -1468,8 +1485,9 @@
 	unsigned long flags;
 	int i;
 
-	printk(KERN_INFO "%s: Transmit timeout, status %8.8x %8.8x \n",
-	       net_dev->name, inl(ioaddr + cr), inl(ioaddr + isr));
+	if(netif_msg_tx_err(sis_priv))
+		printk(KERN_INFO "%s: Transmit timeout, status %8.8x %8.8x \n",
+	       		net_dev->name, inl(ioaddr + cr), inl(ioaddr + isr));
 
 	/* Disable interrupts by clearing the interrupt mask. */
 	outl(0x0000, ioaddr + imr);
@@ -1570,7 +1588,7 @@
 
 	net_dev->trans_start = jiffies;
 
-	if (sis900_debug > 3)
+	if (netif_msg_tx_queued(sis_priv))
 		printk(KERN_DEBUG "%s: Queued Tx packet at %p size %d "
 		       "to slot %d.\n",
 		       net_dev->name, skb->data, (int)skb->len, entry);
@@ -1618,19 +1636,21 @@
 
 		/* something strange happened !!! */
 		if (status & HIBERR) {
-			printk(KERN_INFO "%s: Abnormal interrupt,"
-			       "status %#8.8x.\n", net_dev->name, status);
+			if(netif_msg_intr(sis_priv))
+				printk(KERN_INFO "%s: Abnormal interrupt,"
+					"status %#8.8x.\n", net_dev->name, status);
 			break;
 		}
 		if (--boguscnt < 0) {
-			printk(KERN_INFO "%s: Too much work at interrupt, "
-			       "interrupt status = %#8.8x.\n",
-			       net_dev->name, status);
+			if(netif_msg_intr(sis_priv))
+				printk(KERN_INFO "%s: Too much work at interrupt, "
+					"interrupt status = %#8.8x.\n",
+					net_dev->name, status);
 			break;
 		}
 	} while (1);
 
-	if (sis900_debug > 3)
+	if(netif_msg_intr(sis_priv))
 		printk(KERN_DEBUG "%s: exiting interrupt, "
 		       "interrupt status = 0x%#8.8x.\n",
 		       net_dev->name, inl(ioaddr + isr));
@@ -1656,7 +1676,7 @@
 	unsigned int entry = sis_priv->cur_rx % NUM_RX_DESC;
 	u32 rx_status = sis_priv->rx_ring[entry].cmdsts;
 
-	if (sis900_debug > 3)
+	if (netif_msg_rx_status(sis_priv))
 		printk(KERN_DEBUG "sis900_rx, cur_rx:%4.4d, dirty_rx:%4.4d "
 		       "status:0x%8.8x\n",
 		       sis_priv->cur_rx, sis_priv->dirty_rx, rx_status);
@@ -1668,7 +1688,7 @@
 
 		if (rx_status & (ABORT|OVERRUN|TOOLONG|RUNT|RXISERR|CRCERR|FAERR)) {
 			/* corrupted packet received */
-			if (sis900_debug > 3)
+			if (netif_msg_rx_err(sis_priv))
 				printk(KERN_DEBUG "%s: Corrupted packet "
 				       "received, buffer status = 0x%8.8x.\n",
 				       net_dev->name, rx_status);
@@ -1690,9 +1710,10 @@
 			   some unknow bugs, it is possible that
 			   we are working on NULL sk_buff :-( */
 			if (sis_priv->rx_skbuff[entry] == NULL) {
-				printk(KERN_INFO "%s: NULL pointer " 
-				       "encountered in Rx ring, skipping\n",
-				       net_dev->name);
+				if (netif_msg_rx_err(sis_priv))
+					printk(KERN_INFO "%s: NULL pointer " 
+						"encountered in Rx ring, skipping\n",
+						net_dev->name);
 				break;
 			}
 
@@ -1719,9 +1740,10 @@
 				 * "hole" on the buffer ring, it is not clear
 				 * how the hardware will react to this kind
 				 * of degenerated buffer */
-				printk(KERN_INFO "%s: Memory squeeze,"
-				       "deferring packet.\n",
-				       net_dev->name);
+				if (netif_msg_rx_status(sis_priv))
+					printk(KERN_INFO "%s: Memory squeeze,"
+						"deferring packet.\n",
+						net_dev->name);
 				sis_priv->rx_skbuff[entry] = NULL;
 				/* reset buffer descriptor state */
 				sis_priv->rx_ring[entry].cmdsts = 0;
@@ -1755,9 +1777,10 @@
 				 * "hole" on the buffer ring, it is not clear
 				 * how the hardware will react to this kind
 				 * of degenerated buffer */
-				printk(KERN_INFO "%s: Memory squeeze,"
-				       "deferring packet.\n",
-				       net_dev->name);
+				if (netif_msg_rx_err(sis_priv))
+					printk(KERN_INFO "%s: Memory squeeze,"
+						"deferring packet.\n",
+						net_dev->name);
 				sis_priv->stats.rx_dropped++;
 				break;
 			}
@@ -1806,7 +1829,7 @@
 
 		if (tx_status & (ABORT | UNDERRUN | OWCOLL)) {
 			/* packet unsuccessfully transmitted */
-			if (sis900_debug > 3)
+			if (netif_msg_tx_err(sis_priv))
 				printk(KERN_DEBUG "%s: Transmit "
 				       "error, Tx status %8.8x.\n",
 				       net_dev->name, tx_status);

[PATCH 5/5] sis900 printk audit

[Daniele Venzano]

[-- Attachment #1: Type: text/plain, Size: 202 bytes --]



Chip revision is now a member of sis_priv structure
Kill all calls to pci_read_config_byte but one
Change the code to use sis_priv->chipset_rev



Signed-off-by: Daniele Venzano <webvenza@libero.it>


[-- Attachment #2: Type: text/plain, Size: 5115 bytes --]

Index: sis900.c
===================================================================
--- a/drivers/net/sis900.c	(revision 87)
+++ b/drivers/net/sis900.c	(revision 88)
@@ -181,6 +181,7 @@
 
 	unsigned int tx_full; /* The Tx queue is full. */
 	u8 host_bridge_rev;
+	u8 chipset_rev;
 };
 
 MODULE_AUTHOR("Jim Huang <cmhuang@sis.com.tw>, Ollie Lho <ollie@sis.com.tw>");
@@ -393,7 +394,6 @@
 	void *ring_space;
 	long ioaddr;
 	int i, ret;
-	u8 revision;
 	char *card_name = card_names[pci_id->driver_data];
 
 /* when built into the kernel, we only print version if device is found */
@@ -474,19 +474,18 @@
 		goto err_unmap_rx;
 		
 	/* Get Mac address according to the chip revision */
-	pci_read_config_byte(pci_dev, PCI_CLASS_REVISION, &revision);
-
+	pci_read_config_byte(pci_dev, PCI_CLASS_REVISION, &(sis_priv->chipset_rev));
 	if(netif_msg_probe(sis_priv))
 		printk(KERN_DEBUG "%s: detected revision %2.2x, "
 				"trying to get MAC address...\n",
-				net_dev->name, revision);
+				net_dev->name, sis_priv->chipset_rev);
 	
 	ret = 0;
-	if (revision == SIS630E_900_REV)
+	if (sis_priv->chipset_rev == SIS630E_900_REV)
 		ret = sis630e_get_mac_addr(pci_dev, net_dev);
-	else if ((revision > 0x81) && (revision <= 0x90) )
+	else if ((sis_priv->chipset_rev > 0x81) && (sis_priv->chipset_rev <= 0x90) )
 		ret = sis635_get_mac_addr(pci_dev, net_dev);
-	else if (revision == SIS96x_900_REV)
+	else if (sis_priv->chipset_rev == SIS96x_900_REV)
 		ret = sis96x_get_mac_addr(pci_dev, net_dev);
 	else
 		ret = sis900_get_mac_addr(pci_dev, net_dev);
@@ -498,7 +497,7 @@
 	}
 	
 	/* 630ET : set the mii access mode as software-mode */
-	if (revision == SIS630ET_900_REV)
+	if (sis_priv->chipset_rev == SIS630ET_900_REV)
 		outl(ACCESSMODE | inl(ioaddr + cr), ioaddr + cr);
 
 	/* probe for mii transceiver */
@@ -555,7 +554,6 @@
 	u16 poll_bit = MII_STAT_LINK, status = 0;
 	unsigned long timeout = jiffies + 5 * HZ;
 	int phy_addr;
-	u8 revision;
 
 	sis_priv->mii = NULL;
 
@@ -652,8 +650,7 @@
 		}
 	}
 
-	pci_read_config_byte(sis_priv->pci_dev, PCI_CLASS_REVISION, &revision);
-	if (revision == SIS630E_900_REV) {
+	if (sis_priv->chipset_rev == SIS630E_900_REV) {
 		/* SiS 630E has some bugs on default value of PHY registers */
 		mdio_write(net_dev, sis_priv->cur_phy, MII_ANADV, 0x05e1);
 		mdio_write(net_dev, sis_priv->cur_phy, MII_CONFIG1, 0x22);
@@ -968,15 +965,13 @@
 {
 	struct sis900_private *sis_priv = net_dev->priv;
 	long ioaddr = net_dev->base_addr;
-	u8 revision;
 	int ret;
 
 	/* Soft reset the chip. */
 	sis900_reset(net_dev);
 
 	/* Equalizer workaround Rule */
-	pci_read_config_byte(sis_priv->pci_dev, PCI_CLASS_REVISION, &revision);
-	sis630_set_eq(net_dev, revision);
+	sis630_set_eq(net_dev, sis_priv->chipset_rev);
 
 	ret = request_irq(net_dev->irq, &sis900_interrupt, SA_SHIRQ,
 						net_dev->name, net_dev);
@@ -1245,7 +1240,6 @@
 	struct mii_phy *mii_phy = sis_priv->mii;
 	static int next_tick = 5*HZ;
 	u16 status;
-	u8 revision;
 
 	if (!sis_priv->autong_complete){
 		int speed, duplex = 0;
@@ -1253,9 +1247,7 @@
 		sis900_read_mode(net_dev, &speed, &duplex);
 		if (duplex){
 			sis900_set_mode(net_dev->base_addr, speed, duplex);
-			pci_read_config_byte(sis_priv->pci_dev,
-						PCI_CLASS_REVISION, &revision);
-			sis630_set_eq(net_dev, revision);
+			sis630_set_eq(net_dev, sis_priv->chipset_rev);
 			netif_start_queue(net_dev);
 		}
 
@@ -1290,9 +1282,7 @@
 			    ((mii_phy->phy_id1 & 0xFFF0) == 0x8000))
                			sis900_reset_phy(net_dev,  sis_priv->cur_phy);
   
-                	pci_read_config_byte(sis_priv->pci_dev,
-					PCI_CLASS_REVISION, &revision);
-			sis630_set_eq(net_dev, revision);
+			sis630_set_eq(net_dev, sis_priv->chipset_rev);
   
                 	goto LookForLink;
                 }
@@ -2146,11 +2136,10 @@
 	u16 mc_filter[16] = {0};	/* 256/128 bits multicast hash table */
 	int i, table_entries;
 	u32 rx_mode;
-	u8 revision;
 
 	/* 635 Hash Table entires = 256(2^16) */
-	pci_read_config_byte(sis_priv->pci_dev, PCI_CLASS_REVISION, &revision);
-	if((revision >= SIS635A_900_REV) || (revision == SIS900B_900_REV))
+	if((sis_priv->chipset_rev >= SIS635A_900_REV) ||
+			(sis_priv->chipset_rev == SIS900B_900_REV))
 		table_entries = 16;
 	else
 		table_entries = 8;
@@ -2176,7 +2165,7 @@
 			mclist && i < net_dev->mc_count;
 			i++, mclist = mclist->next) {
 			unsigned int bit_nr =
-				sis900_mcast_bitnr(mclist->dmi_addr, revision);
+				sis900_mcast_bitnr(mclist->dmi_addr, sis_priv->chipset_rev);
 			mc_filter[bit_nr >> 4] |= (1 << (bit_nr & 0xf));
 		}
 	}
@@ -2222,7 +2211,6 @@
 	long ioaddr = net_dev->base_addr;
 	int i = 0;
 	u32 status = TxRCMP | RxRCMP;
-	u8  revision;
 
 	outl(0, ioaddr + ier);
 	outl(0, ioaddr + imr);
@@ -2235,8 +2223,8 @@
 		status ^= (inl(isr + ioaddr) & status);
 	}
 
-	pci_read_config_byte(sis_priv->pci_dev, PCI_CLASS_REVISION, &revision);
-	if( (revision >= SIS635A_900_REV) || (revision == SIS900B_900_REV) )
+	if( (sis_priv->chipset_rev >= SIS635A_900_REV) ||
+			(sis_priv->chipset_rev == SIS900B_900_REV) )
 		outl(PESEL | RND_CNT, ioaddr + cfg);
 	else
 		outl(PESEL, ioaddr + cfg);

Re: [PATCH 3/5] sis900 printk audit

[Francois Romieu]

Daniele Venzano <webvenza@libero.it> :
[...]
> Index: sis900.c
> ===================================================================
> --- a/drivers/net/sis900.c	(revision 85)
> +++ b/drivers/net/sis900.c	(revision 86)
[...]
> @@ -275,7 +275,7 @@
>  	if (!isa_bridge)
>  		isa_bridge = pci_get_device(PCI_VENDOR_ID_SI, 0x0018, isa_bridge);
>  	if (!isa_bridge) {
> -		printk("%s: Can not find ISA bridge\n", net_dev->name);
> +		printk(KERN_WARNING "%s: Can not find ISA bridge\n", net_dev->name);
>  		return 0;
>  	}
>  	pci_read_config_byte(isa_bridge, 0x48, &reg);
> @@ -567,7 +567,7 @@
>  			continue;
>  		
>  		if ((mii_phy = kmalloc(sizeof(struct mii_phy), GFP_KERNEL)) == NULL) {
> -			printk(KERN_INFO "Cannot allocate mem for struct mii_phy\n");
> +			printk(KERN_WARNING "Cannot allocate mem for struct mii_phy\n");

-> printk(KERN_WARNING "%s: Cannot allocate mem for struct mii_phy\n", ...) ?

Btw it may make sense to use pci_name() instead of the name associated with
the struct net_device until it is known for sure that the net_device will not
be removed due to a failure of the probe() method. 

--
Ueimor

Re: [PATCH 1/5] sis900 printk audit

[Francois Romieu]

Daniele Venzano <webvenza@libero.it> :
[...]
> Index: sis900.c
> ===================================================================
> --- a/drivers/net/sis900.c	(revision 83)
> +++ b/drivers/net/sis900.c	(revision 84)
[...]
> @@ -1906,8 +1918,22 @@
>  	strcpy (info->bus_info, pci_name(sis_priv->pci_dev));
>  }
>  
> +static u32 sis900_get_msglevel(struct net_device *net_dev)
> +{
> +	struct sis900_private *sis_priv = net_dev->priv;

s/net_dev->priv/netdev_priv/

--
Ueimor

Re: [PATCH 1/5] sis900 printk audit

[Daniele Venzano]

On Sat, Jan 22, 2005 at 11:26:40PM +0100, Francois Romieu wrote:
> Daniele Venzano <webvenza@libero.it> :
> [...]
> > Index: sis900.c
> > ===================================================================
> > --- a/drivers/net/sis900.c	(revision 83)
> > +++ b/drivers/net/sis900.c	(revision 84)
> [...]
> > @@ -1906,8 +1918,22 @@
> >  	strcpy (info->bus_info, pci_name(sis_priv->pci_dev));
> >  }
> >  
> > +static u32 sis900_get_msglevel(struct net_device *net_dev)
> > +{
> > +	struct sis900_private *sis_priv = net_dev->priv;
> 
> s/net_dev->priv/netdev_priv/

I don't understand, what's wrong here ?
Is that a suggestion that I make some global variable or macro to avoid
repeating that declaration in almost every function of the driver ?

-- 
-----------------------------
Daniele Venzano
Web: http://teg.homeunix.org

[PATCH 6/5] sis900 printk audit

[Daniele Venzano]

[-- Attachment #1: Type: text/plain, Size: 909 bytes --]

On Sat, Jan 22, 2005 at 11:26:18PM +0100, Francois Romieu wrote:
> Daniele Venzano <webvenza@libero.it> :
> > -			printk(KERN_INFO "Cannot allocate mem for struct mii_phy\n");
> > +			printk(KERN_WARNING "Cannot allocate mem for struct mii_phy\n");
> 
> -> printk(KERN_WARNING "%s: Cannot allocate mem for struct mii_phy\n", ...) ?
> 
> Btw it may make sense to use pci_name() instead of the name associated with
> the struct net_device until it is known for sure that the net_device will not
> be removed due to a failure of the probe() method. 

Attached a patch on top of the previous ones.

It only fixes the missing prefix. I understand that pci_name could be a
better choice here, but I want to see what other drivers are doing in
their probe() before changing policy.

Signed-off-by: Daniele Venzano <webvenza@linero.it>

-- 
-----------------------------
Daniele Venzano
Web: http://teg.homeunix.org


[-- Attachment #2: sis900_c_89.diff --]
[-- Type: text/plain, Size: 509 bytes --]

Index: sis900.c
===================================================================
--- a/drivers/net/sis900.c	(revision 88)
+++ b/drivers/net/sis900.c	(revision 89)
@@ -576,7 +576,7 @@
 		}
 		
 		if ((mii_phy = kmalloc(sizeof(struct mii_phy), GFP_KERNEL)) == NULL) {
-			printk(KERN_WARNING "Cannot allocate mem for struct mii_phy\n");
+			printk(KERN_WARNING "%s: Cannot allocate mem for struct mii_phy\n", net_dev->name);
 			mii_phy = sis_priv->first_mii;
 			while (mii_phy) {
 				struct mii_phy *phy;

Re: [PATCH 1/5] sis900 printk audit

[Francois Romieu]

Daniele Venzano <webvenza@libero.it> :
[...]
> Is that a suggestion that I make some global variable or macro to avoid
> repeating that declaration in almost every function of the driver ?

It suggests turning:

	struct sis900_private *sis_priv = net_dev->priv;

into:

	struct sis900_private *sis_priv = netdev_priv(net_dev);

--
Ueimor

Re: [PATCH 1/5] sis900 printk audit

[Daniele Venzano]

On Sun, Jan 23, 2005 at 05:16:47PM +0100, Francois Romieu wrote:
> It suggests turning:
> 
> 	struct sis900_private *sis_priv = net_dev->priv;
> 
> into:
> 
> 	struct sis900_private *sis_priv = netdev_priv(net_dev);

Ah, that is indeed a good advice, I didn't even know that netdev_priv()
existed.

I'll put it on my todo list, all the driver should be converted to that
form.

Thanks.

-- 
-----------------------------
Daniele Venzano
Web: http://teg.homeunix.org