[PATCH] hp100 -- fixes for new probing.

[PATCH] hp100 -- fixes for new probing.

[Stephen Hemminger]

Fixes to net-drivers-2.5-exp patches for hp100
	* EISA device id table needs a terminating string.
	* if one driver built for all variations (ISA, EISA, PCI)
	  then try to have sane error handling on probe.

diff -Nru a/drivers/net/hp100.c b/drivers/net/hp100.c
--- a/drivers/net/hp100.c	Mon Nov 24 15:34:53 2003
+++ b/drivers/net/hp100.c	Mon Nov 24 15:34:53 2003
@@ -201,6 +201,7 @@
 	{ "HWP1990" }, /* HP J2577 */
 	{ "CPX0301" }, /* ReadyLink ENET100-VG4 */
 	{ "CPX0401" }, /* FreedomLine 100/VG */
+	{ "" }
 };
 MODULE_DEVICE_TABLE(eisa, hp100_eisa_tbl);
 #endif
@@ -3045,10 +3046,16 @@
 	err = hp100_isa_init();
 
 #ifdef CONFIG_EISA
-	err |= eisa_driver_register(&hp100_eisa_driver);
+	if (err && err != -ENODEV) 
+		return err;
+
+	err = eisa_driver_register(&hp100_eisa_driver);
 #endif
 #ifdef CONFIG_PCI
-	err |= pci_module_init(&hp100_pci_driver);
+	if (err && err != -ENODEV)
+		return err;
+
+	err = pci_module_init(&hp100_pci_driver);
 #endif
 	return err;
 }

Re: [PATCH] hp100 -- fixes for new probing.

[Jeff Garzik]

Stephen Hemminger wrote:
> Fixes to net-drivers-2.5-exp patches for hp100
> 	* EISA device id table needs a terminating string.
> 	* if one driver built for all variations (ISA, EISA, PCI)
> 	  then try to have sane error handling on probe.
> 
> diff -Nru a/drivers/net/hp100.c b/drivers/net/hp100.c
> --- a/drivers/net/hp100.c	Mon Nov 24 15:34:53 2003
> +++ b/drivers/net/hp100.c	Mon Nov 24 15:34:53 2003
> @@ -201,6 +201,7 @@
>  	{ "HWP1990" }, /* HP J2577 */
>  	{ "CPX0301" }, /* ReadyLink ENET100-VG4 */
>  	{ "CPX0401" }, /* FreedomLine 100/VG */
> +	{ "" }
>  };
>  MODULE_DEVICE_TABLE(eisa, hp100_eisa_tbl);
>  #endif
> @@ -3045,10 +3046,16 @@
>  	err = hp100_isa_init();
>  
>  #ifdef CONFIG_EISA
> -	err |= eisa_driver_register(&hp100_eisa_driver);
> +	if (err && err != -ENODEV) 
> +		return err;
> +
> +	err = eisa_driver_register(&hp100_eisa_driver);
>  #endif
>  #ifdef CONFIG_PCI
> -	err |= pci_module_init(&hp100_pci_driver);
> +	if (err && err != -ENODEV)
> +		return err;
> +
> +	err = pci_module_init(&hp100_pci_driver);
>  #endif
>  	return err;
>  }


Valid changes... but it looks like there should be some *_unregister_* 
calls in this last patch chunk, to clean up on error...

	Jeff

Re: [PATCH] hp100 -- fixes for new probing.

[Cacophonix]

(resending, copying netdev) 

Hi,
Any idea if any of these patches may have broken hp100 when statically built into the 
kernel (i.e, not as a module)?

Upgrading from 2.6.0-test6 to 2.6.3-mm2 (which does include the latest patchsets for
hp100 from Stephen), the kernel locks up at boot at the point where hp100 
detection/initialization normally occurs (the driver is compiled into the kernel). 
However, changing hp100 to a module, and loading it dynamically works.

Any clues about which of the recent patches between 2.6.0-test6 and 2.6.3-mm2 might
be causing the lockup? Thanks for any pointers.

cheers,
karthik

The /proc/pci data for my hp100 nic:
  Bus  0, device  14, function  0:
    Ethernet controller: Hewlett-Packard Comp J2585B HP 10/100VG P (rev 0).
      IRQ 11.
      Master Capable.  Latency=248.  Min Gnt=8.Max Lat=32.
      I/O at 0xec00 [0xecff].
      Non-prefetchable 32 bit memory at 0xfffac000 [0xfffadfff].





__________________________________
Do you Yahoo!?
Yahoo! Mail SpamGuard - Read only the mail you want.
http://antispam.yahoo.com/tools

Re: [PATCH] hp100 -- fixes for new probing.

[Stephen Hemminger]

On Mon, 23 Feb 2004 00:48:14 -0800 (PST)
Cacophonix <cacophonix@yahoo.com> wrote:

> (resending, copying netdev) 
> 
> Hi,
> Any idea if any of these patches may have broken hp100 when statically built into the 
> kernel (i.e, not as a module)?
> 
> Upgrading from 2.6.0-test6 to 2.6.3-mm2 (which does include the latest patchsets for
> hp100 from Stephen), the kernel locks up at boot at the point where hp100 
> detection/initialization normally occurs (the driver is compiled into the kernel). 
> However, changing hp100 to a module, and loading it dynamically works.
> 
> Any clues about which of the recent patches between 2.6.0-test6 and 2.6.3-mm2 might
> be causing the lockup? Thanks for any pointers.
> 
> cheers,
> karthik

There probably is some bug introduced with the recent changes.  The driver tries
to handle isa, eisa, and pci all with the same infrastructure.  There are two different
probe paths, one for the module and one for the non-module ISA case.  Could you add some
printk's and see if the problem in the hp100_isa_probe code (happens first), or the
PCI probe that happens from: 
	hp100_module_init -> pci_module_init -> hp100_pci_probe...

Re: [PATCH] hp100 -- fixes for new probing.

[Cacophonix]

Thanks for the suggestion.

It appears to be locking up at line 309 in hp100.c, i.e,

    hp100_page(ID_MAC_ADDR);
    (called from hp100_isa_probe1() and hp100_read_id() )

thanks,
karthik

--- Stephen Hemminger <shemminger@osdl.org> wrote:

> 
> There probably is some bug introduced with the recent changes.  The driver tries
> to handle isa, eisa, and pci all with the same infrastructure.  There are two different
> probe paths, one for the module and one for the non-module ISA case.  Could you add
> some
> printk's and see if the problem in the hp100_isa_probe code (happens first), or the
> PCI probe that happens from: 
> 	hp100_module_init -> pci_module_init -> hp100_pci_probe...
> 


__________________________________
Do you Yahoo!?
Yahoo! Mail SpamGuard - Read only the mail you want.
http://antispam.yahoo.com/tools

Re: [PATCH] hp100 -- fixes for new probing.

[Stephen Hemminger]

Does fix the problem?  it checks for card before proceeding in hp100_isa_probe

diff -Nru a/drivers/net/hp100.c b/drivers/net/hp100.c
--- a/drivers/net/hp100.c	Tue Feb 24 17:05:54 2004
+++ b/drivers/net/hp100.c	Tue Feb 24 17:05:54 2004
@@ -333,6 +333,11 @@
 
 	if (!request_region(addr, HP100_REGION_SIZE, "hp100"))
 		goto err;
+	
+	if (hp100_inw(HW_ID) != HP100_HW_ID_CASCADE) {
+		release_region(addr, HP100_REGION_SIZE);
+		goto err;
+	}
 
 	sig = hp100_read_id(addr);
 	release_region(addr, HP100_REGION_SIZE);

Re: [PATCH] hp100 -- fixes for new probing.

[Cacophonix]

Yes, this fixes the lockup issue (once I renamed addr to ioaddr in hp100_isa_probe1()
to get it to compile. Thanks for the patch!

Next order of business for me is to figure out why the ordering of device recognition
of e100 and hp100 changed between 2.6.0-test* and 2.6.3-mm2 (in the former, hp100 
started up first, while in the latter kernel e100 get's to load up first). I guess I 
could work around this in other ways, but it's a tad disconcerting when eth0, eth1 
and eth2 decide to trade places every so often ;)

Thanks again for the patch.
cheers,
karthik




--- Stephen Hemminger <shemminger@osdl.org> wrote:
> Does fix the problem?  it checks for card before proceeding in hp100_isa_probe
> 
> diff -Nru a/drivers/net/hp100.c b/drivers/net/hp100.c
> --- a/drivers/net/hp100.c	Tue Feb 24 17:05:54 2004
> +++ b/drivers/net/hp100.c	Tue Feb 24 17:05:54 2004
> @@ -333,6 +333,11 @@
>  
>  	if (!request_region(addr, HP100_REGION_SIZE, "hp100"))
>  		goto err;
> +	
> +	if (hp100_inw(HW_ID) != HP100_HW_ID_CASCADE) {
> +		release_region(addr, HP100_REGION_SIZE);
> +		goto err;
> +	}
>  
>  	sig = hp100_read_id(addr);
>  	release_region(addr, HP100_REGION_SIZE);


__________________________________
Do you Yahoo!?
Yahoo! Mail SpamGuard - Read only the mail you want.
http://antispam.yahoo.com/tools

Re: [PATCH] hp100 -- fixes for new probing.

[Stephen Hemminger]

On Wed, 25 Feb 2004 13:33:47 -0800 (PST)
Cacophonix <cacophonix@yahoo.com> wrote:

> 
> Yes, this fixes the lockup issue (once I renamed addr to ioaddr in hp100_isa_probe1()
> to get it to compile. Thanks for the patch!
> 
> Next order of business for me is to figure out why the ordering of device recognition
> of e100 and hp100 changed between 2.6.0-test* and 2.6.3-mm2 (in the former, hp100 
> started up first, while in the latter kernel e100 get's to load up first). I guess I 
> could work around this in other ways, but it's a tad disconcerting when eth0, eth1 
> and eth2 decide to trade places every so often ;)
> 

The naming is a function of initialization order which used to be fixed by a long list
of legacy devices.  Since the hp100 pci now probes like a proper PCI device, it will
happen in a order defined by the load order (ie not well controlled). 

Learn to use nameif it will make your life simpler.