Sis900 bug fixes 0/4

I have prepared 4 patches that fix various issues with the sis900 driver in Linux 2.6.6, two of them had some discussion on lkml. The entire patchset has been tested by me, but patches 2 and 3 require testing from the people who reported the bugs (they are CCed).

Patches 2,3,4 are incremental and need to be applied in that order.

Patch summary:
1. change of maintainership for the sis900 driver
2. Add new ISA bridge PCI ID
3. Fix PHY transceiver detection code to fall back to known PHY and not to the last detected.
4. Small cleanup and spelling fixes of sis900.h (much more needed, also in sis900.c, will go through trivial).

Any comment is highly appreciated.

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

Patch 1 of 4

Change of maintainership for the sis900 driver

The URL of a page with some informations is also added.

Any comment is highly appreciated.

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

@@ -1824,10 +1824,11 @@
 S:	Maintained
 
 SIS 900/7016 FAST ETHERNET DRIVER
-P:	Ollie Lho
-M:	[email protected]
+P:	Daniele Venzano
+M:	[email protected]
 L:	[email protected]
-S:	Supported
+W:	http://teg.homeunix.org/sis900.html
+S:	Maintained
 
 SIS FRAMEBUFFER DRIVER
 P:	Thomas Winischhofer

Patch 2 of 4

Add new ISA bridge PCI ID

I added some constants to make the code more readable and modified the if body to check for another ID in case the first check fails.

Any comment is highly appreciated.

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

diff -Naru -X /home/venza/kernel/dontdiff linux-2.6.6/drivers/net/sis900.c linux-sis900/drivers/net/sis900.c
@@ -260,9 +260,13 @@
 	u8 reg;
 	int i;
 
-	if ((isa_bridge = pci_find_device(0x1039, 0x0008, isa_bridge)) == NULL) {
-		printk("%s: Can not find ISA bridge\n", net_dev->name);
-		return 0;
+	isa_bridge = pci_find_device(PCI_VENDOR_ID_SI, SIS630E_ISA_BRIDGE_ID_1, isa_bridge);
+	if (!isa_bridge) {
+		isa_bridge = pci_find_device(PCI_VENDOR_ID_SI, SIS630E_ISA_BRIDGE_ID_2, isa_bridge);
+		if (!isa_bridge) {
+			printk("%s: Can not find ISA bridge\n", net_dev->name);
+			return 0;
+		}
 	}
 	pci_read_config_byte(isa_bridge, 0x48, &reg);
 	pci_write_config_byte(isa_bridge, 0x48, reg | 0x40);

diff -Naru -X /home/venza/kernel/dontdiff linux-2.6.6/drivers/net/sis900.h linux-sis900/drivers/net/sis900.h
@@ -274,6 +274,9 @@
 #define TX_TOTAL_SIZE	NUM_TX_DESC*sizeof(BufferDesc)
 #define RX_TOTAL_SIZE	NUM_RX_DESC*sizeof(BufferDesc)
 
+#define SIS630E_ISA_BRIDGE_ID_1	0x0008	
+#define SIS630E_ISA_BRIDGE_ID_2	0x0018	
+
 /* PCI stuff, should be move to pci.h */
 #define SIS630_VENDOR_ID        0x1039
 #define SIS630_DEVICE_ID        0x0630

Patch 3 of 4

Fix PHY transceiver detection code to fall back to known PHY and not to the last detected.
The code checks every transceiver detected for link status and type, but fails when ghost transceivers are detected, deciding to use the last one detected.

With this patch the driver should choose the correct transceiver even when some ghosts are detected by checking for the type of the tranceiver it is going to use.

This patch needs that patch #2 is applied before.

Any comment is highly appreciated.

Dominik:
Could you please test this patch ? Thanks.

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

Obviously I had to forget at least one attachment... Sorry for the noise.

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

@@ -644,7 +644,7 @@
 static u16 sis900_default_phy(struct net_device * net_dev)
 {
 	struct sis900_private * sis_priv = net_dev->priv;
- 	struct mii_phy *phy = NULL, *phy_home = NULL, *default_phy = NULL;
+ 	struct mii_phy *phy = NULL, *phy_home = NULL, *default_phy = NULL, *phy_lan = NULL;
 	u16 status;
 
         for( phy=sis_priv->first_mii; phy; phy=phy->next ){

@@ -660,12 +660,16 @@
 				status | MII_CNTL_AUTO | MII_CNTL_ISOLATE);
 			if( phy->phy_types == HOME )
 				phy_home = phy;
+			else if (phy->phy_types == LAN)
+				phy_lan = phy;
 		 }
 	}
 
-	if( (!default_phy) && phy_home )
+	if( !default_phy && phy_home )
 		default_phy = phy_home;
-	else if(!default_phy)
+	else if( !default_phy && phy_lan )
+		default_phy = phy_lan;
+	else if ( !default_phy )
 		default_phy = sis_priv->first_mii;
 
 	if( sis_priv->mii != default_phy ){

After further testing with Dominik, I made a new patch that actually works. So, please disregard patch number 3 I sent before and use this one instead. The other patches need no changes and can be found here:

http://teg.homeunix.org/sis900.html

They are:
sis900-maintainers
sis900-isa-bridge-id
sis900-phy-detection (attached below)
sis900-header-cleanups

This patch has been tested on a previously working system (keeps working) and on a before-not-working one (now works as expected).

Please, consider this patchset for inclusion.

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

@@ -116,6 +116,7 @@
 #define	HOME 	0x0001
 #define LAN	0x0002
 #define MIX	0x0003
+#define UNKNOWN	0x0
 } mii_chip_table[] = {
 	{ "SiS 900 Internal MII PHY", 		0x001d, 0x8000, LAN },
 	{ "SiS 7014 Physical Layer Solution", 	0x0016, 0xf830, LAN },

@@ -577,9 +578,11 @@
 				break;
 			}
 			
-		if( !mii_chip_table[i].phy_id1 )
+		if( !mii_chip_table[i].phy_id1 ) {
 			printk(KERN_INFO "%s: Unknown PHY transceiver found at address %d.\n",
-			       net_dev->name, phy_addr);			
+			       net_dev->name, phy_addr);
+			mii_phy->phy_types = UNKNOWN;
+		}
 	}
 	
 	if (sis_priv->mii == NULL) {

@@ -644,15 +647,15 @@
 static u16 sis900_default_phy(struct net_device * net_dev)
 {
 	struct sis900_private * sis_priv = net_dev->priv;
- 	struct mii_phy *phy = NULL, *phy_home = NULL, *default_phy = NULL;
+ 	struct mii_phy *phy = NULL, *phy_home = NULL, *default_phy = NULL, *phy_lan = NULL;
 	u16 status;
 
         for( phy=sis_priv->first_mii; phy; phy=phy->next ){
 		status = mdio_read(net_dev, phy->phy_addr, MII_STATUS);
 		status = mdio_read(net_dev, phy->phy_addr, MII_STATUS);
 
-		/* Link ON & Not select deafalut PHY */
-		 if ( (status & MII_STAT_LINK) && !(default_phy) )
+		/* Link ON & Not select default PHY & not ghost PHY */
+		 if ( (status & MII_STAT_LINK) && !default_phy && (phy->phy_types != UNKNOWN) )
 		 	default_phy = phy;
 		 else{
 			status = mdio_read(net_dev, phy->phy_addr, MII_CONTROL);

@@ -660,12 +663,16 @@
 				status | MII_CNTL_AUTO | MII_CNTL_ISOLATE);
 			if( phy->phy_types == HOME )
 				phy_home = phy;
+			else if (phy->phy_types == LAN)
+				phy_lan = phy;
 		 }
 	}
 
-	if( (!default_phy) && phy_home )
+	if( !default_phy && phy_home )
 		default_phy = phy_home;
-	else if(!default_phy)
+	else if( !default_phy && phy_lan )
+		default_phy = phy_lan;
+	else if ( !default_phy )
 		default_phy = sis_priv->first_mii;
 
 	if( sis_priv->mii != default_phy ){

Let's hold off on this for now... we can commit this patch anytime, and there is no harm in being "unofficial maintainer" for a while.

applied, with modifications, to netdev-2.6

I'd like to wait until my netdev-2.6 changes (just pushed to their public BK home, bk://gkernel.bkbits.net/netdev-2.6) are pulled by Andrew, and incorporated into the update of his -mm tree.

This patch will likely need some rediffing.

applied, with modifications, to netdev-2.6

Also I applied the sis900-detach found on the web page.

Jeff

Patch 4 of 4

Small cleanup and spelling fixes of sis900.h (much more needed, also in sis900.c, will go through Rusty's trivial patch monkey if my maintainership is accepted).

Any comment is highly appreciated.

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

@@ -77,7 +77,7 @@
 	IE = 0x00000001
 };
 
-/* maximum dma burst fro transmission and receive*/
+/* maximum dma burst for transmission and receive*/
 #define MAX_DMA_RANGE	7	/* actually 0 means MAXIMUM !! */
 #define TxMXDMA_shift   	20
 #define RxMXDMA_shift    20

@@ -86,7 +86,7 @@
 	DMA_BURST_512 = 0,	DMA_BURST_64 = 5
 };
 
-/* transmit FIFO threshholds */
+/* transmit FIFO thresholds */
 #define TX_FILL_THRESH   16	/* 1/4 FIFO size */
 #define TxFILLT_shift   	8
 #define TxDRNT_shift    	0

@@ -140,7 +140,7 @@
 	EEREQ = 0x00000400, EEDONE = 0x00000200, EEGNT = 0x00000100
 };
 
-/* Manamgement Data I/O (mdio) frame */
+/* Management Data I/O (mdio) frame */
 #define MIIread         0x6000
 #define MIIwrite        0x5002
 #define MIIpmdShift     7

@@ -277,6 +277,3 @@
 #define SIS630E_ISA_BRIDGE_ID_1	0x0008	
 #define SIS630E_ISA_BRIDGE_ID_2	0x0018	
 
-/* PCI stuff, should be move to pci.h */
-#define SIS630_VENDOR_ID        0x1039
-#define SIS630_DEVICE_ID        0x0630

I applied all 4 patches, but the wrong PHY transceiver is used now again. Here is the dmesg output:

sis900.c: v1.08.07 11/02/2003
eth0: Unknown PHY transceiver found at address 0.
eth0: Realtek RTL8201 PHY transceiver found at address 1.
eth0: Unknown PHY transceiver found at address 2.
eth0: Unknown PHY transceiver found at address 3.
eth0: Unknown PHY transceiver found at address 4.
eth0: Unknown PHY transceiver found at address 5.
eth0: Unknown PHY transceiver found at address 6.
eth0: Unknown PHY transceiver found at address 7.
eth0: Unknown PHY transceiver found at address 8.
eth0: Unknown PHY transceiver found at address 9.
eth0: Unknown PHY transceiver found at address 10.
eth0: Unknown PHY transceiver found at address 11.
eth0: Unknown PHY transceiver found at address 12.
eth0: Unknown PHY transceiver found at address 13.
eth0: Unknown PHY transceiver found at address 14.
eth0: Unknown PHY transceiver found at address 15.
eth0: Unknown PHY transceiver found at address 16.
eth0: Unknown PHY transceiver found at address 17.
eth0: Unknown PHY transceiver found at address 18.
eth0: Unknown PHY transceiver found at address 19.
eth0: Unknown PHY transceiver found at address 20.
eth0: Unknown PHY transceiver found at address 21.
eth0: Unknown PHY transceiver found at address 22.
eth0: Unknown PHY transceiver found at address 23.
eth0: Unknown PHY transceiver found at address 24.
eth0: Unknown PHY transceiver found at address 25.
eth0: Unknown PHY transceiver found at address 26.
eth0: Unknown PHY transceiver found at address 27.
eth0: Unknown PHY transceiver found at address 28.
eth0: Unknown PHY transceiver found at address 29.
eth0: Unknown PHY transceiver found at address 30.
eth0: Unknown PHY transceiver found at address 31.
eth0: Using transceiver found at address 31 as default
eth0: SiS 900 PCI Fast Ethernet at 0xdc00, IRQ 19, 00:10:dc:8f:a9:ac.

greets,
dominik

<snip>

Hi

I had the same messages about unknown PHY transceivers and very poor transfer rates. I figured that all but one of the transceivers had the status bit MII_STAT_FAULT set. Selecting that one solved my problems.

I posted this and the patch already a while ago but didn't get any response. It would be nice if a few people could check whether this fixes/breaks it for them.

I just checked it with 2.6.6-rc3 and the problem is still there and gets solved by the attached patch.

Grischa

@@ -18,6 +18,7 @@
    preliminary Rev. 1.0 Jan. 18, 1998
    http://www.sis.com.tw/support/databook.htm
 
+               Jan.  7 2004 Grischa Jacobs - Skip mii's with MII_STAT_FAULT set
    Rev 1.08.07 Nov.  2 2003 Daniele Venzano <[email protected]> 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

@@ -541,6 +542,10 @@
 			/* the mii is not accessible, try next one */
 			continue;
 		
+		if (mii_status & MII_STAT_FAULT)
+			/* ignore mii with the fault bit set */
+			continue;
+
 		if ((mii_phy = kmalloc(sizeof(struct mii_phy), GFP_KERNEL)) == NULL) {
 			printk(KERN_INFO "Cannot allocate mem for struct mii_phy\n");
 			mii_phy = sis_priv->first_mii;

I did some tries on patching the driver. When I did following changes it works here, but I don't know if it isn't broken on other machines?

Following changes I did in the 3rd patch created code:

       if( !default_phy && phy_home )
                default_phy = phy_home;
       else if( !default_phy && phy_lan )
               default_phy = phy_lan;
       else if ( !default_phy )
                default_phy = sis_priv->first_mii;

changed to

if( phy_lan )
default_phy = phy_lan;
else if( phy_home )
default_phy = phy_home;
else if ( !default_phy )
default_phy = sis_priv->first_mii;

dmesg output:
sis900.c: v1.08.07 11/02/2003
eth0: Unknown PHY transceiver found at address 0.
eth0: Realtek RTL8201 PHY transceiver found at address 1.
eth0: Unknown PHY transceiver found at address 2.
eth0: Unknown PHY transceiver found at address 3.
eth0: Unknown PHY transceiver found at address 4.
eth0: Unknown PHY transceiver found at address 5.
eth0: Unknown PHY transceiver found at address 6.
eth0: Unknown PHY transceiver found at address 7.
eth0: Unknown PHY transceiver found at address 8.
eth0: Unknown PHY transceiver found at address 9.
eth0: Unknown PHY transceiver found at address 10.
eth0: Unknown PHY transceiver found at address 11.
eth0: Unknown PHY transceiver found at address 12.
eth0: Unknown PHY transceiver found at address 13.
eth0: Unknown PHY transceiver found at address 14.
eth0: Unknown PHY transceiver found at address 15.
eth0: Unknown PHY transceiver found at address 16.
eth0: Unknown PHY transceiver found at address 17.
eth0: Unknown PHY transceiver found at address 18.
eth0: Unknown PHY transceiver found at address 19.
eth0: Unknown PHY transceiver found at address 20.
eth0: Unknown PHY transceiver found at address 21.
eth0: Unknown PHY transceiver found at address 22.
eth0: Unknown PHY transceiver found at address 23.
eth0: Unknown PHY transceiver found at address 24.
eth0: Unknown PHY transceiver found at address 25.
eth0: Unknown PHY transceiver found at address 26.
eth0: Unknown PHY transceiver found at address 27.
eth0: Unknown PHY transceiver found at address 28.
eth0: Unknown PHY transceiver found at address 29.
eth0: Unknown PHY transceiver found at address 30.
eth0: Unknown PHY transceiver found at address 31.
eth0: Using transceiver found at address 1 as default
eth0: SiS 900 PCI Fast Ethernet at 0xdc00, IRQ 19, 00:10:dc:8f:a9:ac.

Maybe this information can help you!

greets,
dominik