[PATCH] Small fix for the Sibyte Mac driver

[PATCH] Small fix for the Sibyte Mac driver

[Manish Lachwani]

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

Hello !

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

Thanks
Manish Lachwani

[-- Attachment #2: patch-sb1250-mac --]
[-- Type: text/plain, Size: 590 bytes --]

--- drivers/net/sb1250-mac.c.orig	2004-11-08 15:36:45.000000000 -0800
+++ drivers/net/sb1250-mac.c	2004-11-08 15:44:33.000000000 -0800
@@ -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

Re: [PATCH] Small fix for the Sibyte Mac driver

[Ralf Baechle]

On Mon, Nov 08, 2004 at 03:51:48PM -0800, Manish Lachwani wrote:

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

> -	/* 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

Re: [PATCH] Small fix for the Sibyte Mac driver

[Manish Lachwani]

Ralf Baechle wrote:
> On Mon, Nov 08, 2004 at 03:51:48PM -0800, Manish Lachwani wrote:
> 
> 
>>Attached is a small patch for the Sibyte MAC Driver. This helps
>>print the device name correctly
> 
> 
>>-	/* 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
> 
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