Skip to contentSkip to conversation

Small fix for the Sibyte Mac driver

PATCH In review Manish Lachwani 3 participants 3 messages Nov 8, 2004 – Nov 9, 2004

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Small fix for the Sibyte Mac driver

Hello !

Attached is a small patch for the Sibyte MAC Driver. This helps print the device name correctly

Thanks
Manish Lachwani

net/sb1250-mac.c renamed from net/sb1250-mac.c.orig +3 -3
@@ -2410,13 +2410,13 @@
 
 	dev->change_mtu         = sb1250_change_mtu;
 
-	/* This is needed for PASS2 for Rx H/W checksum feature */
-	sbmac_set_iphdr_offset(sc);
-
 	err = register_netdev(dev);
 	if (err)
 		goto out_uninit;
 
+	/* This is needed for PASS2 for Rx H/W checksum feature */
+	sbmac_set_iphdr_offset(sc);
+
 	/*
 	 * Display Ethernet address (this is called during the config
 	 * process so we need to finish off the config message that
#1 lore raw

net/sb1250-mac.c line 2419

Quoted 13 lines · Manish Lachwani

Attached is a small patch for the Sibyte MAC Driver. This helps print the device name correctly

net/sb1250-mac.c renamed from net/sb1250-mac.c.orig +3 -3
-	/* This is needed for PASS2 for Rx H/W checksum feature */
-	sbmac_set_iphdr_offset(sc);
-
 	err = register_netdev(dev);
 	if (err)
 		goto out_uninit;
 
+	/* This is needed for PASS2 for Rx H/W checksum feature */
+	sbmac_set_iphdr_offset(sc);
+

Your patch introduces a race condition - the NIC needs to be fully setup before register_netdev. By the time register_netdev returns the driver could possibly already be opened and traffic be flowing. What's usually done is using the PCI device's name as obtained through pci_name(). Which in this case fails, so maybe you should convert the driver to a platform_device() and print platform_device->name instead. The Titan GE driver which I think you're familiar with already use platform_device ;-)

Ralf

#2 lore raw
Quoted 27 lines · Ralf Baechle
Quoted 11 lines · Manish Lachwani

Attached is a small patch for the Sibyte MAC Driver. This helps print the device name correctly

net/sb1250-mac.c renamed from net/sb1250-mac.c.orig +0 -3
-	/* This is needed for PASS2 for Rx H/W checksum feature */
-	sbmac_set_iphdr_offset(sc);
-
	err = register_netdev(dev);
	if (err)
		goto out_uninit;

net/sb1250-mac.c line 2419

Quoted 3 lines
net/sb1250-mac.c renamed from net/sb1250-mac.c.orig +3 -0
+	/* This is needed for PASS2 for Rx H/W checksum feature */
+	sbmac_set_iphdr_offset(sc);
+

Your patch introduces a race condition - the NIC needs to be fully setup before register_netdev. By the time register_netdev returns the driver could possibly already be opened and traffic be flowing. What's usually done is using the PCI device's name as obtained through pci_name(). Which in this case fails, so maybe you should convert the driver to a platform_device() and print platform_device->name instead. The Titan GE driver which I think you're familiar with already use platform_device ;-)

Quoted 2 lines

Ralf

Hi Ralf

Thanks for the response. To make it really simple and avoid lot of changes to the driver code, we can continue to do sbmac_set_iphdr_offset() before the call to register_netdev() and print the "...enabling TCP rcv checksum" after register_netdev().

Since we need the driver to print the device name correctly, this change can keep things really simple :)

Thanks
Manish Lachwani