[PATCH] Hamachi not doing pci_enable before reading resources

[PATCH] Hamachi not doing pci_enable before reading resources

[davej]

Hi Alan,

 Another driver not doing pci_enable_device() early enough.

Dave.

-- 
| Dave Jones.        http://www.suse.de/~davej
| SuSE Labs

diff -urN --exclude-from=/home/davej/.exclude linux/drivers/net/hamachi.c linux-dj/drivers/net/hamachi.c
--- linux/drivers/net/hamachi.c	Wed Feb  7 12:42:39 2001
+++ linux-dj/drivers/net/hamachi.c	Wed Feb  7 19:09:31 2001
@@ -562,13 +562,14 @@
 	if (hamachi_debug > 0  &&  did_version++ == 0)
 		printk(version);

+	if (pci_enable_device(pdev))
+		return -EIO;
+
 	ioaddr = pci_resource_start(pdev, 0);
 #ifdef __alpha__				/* Really "64 bit addrs" */
 	ioaddr |= (pci_resource_start(pdev, 1) << 32);
 #endif

-	if (pci_enable_device(pdev))
-		return -EIO;
 	pci_set_master(pdev);

 	ioaddr = (long) ioremap(ioaddr, 0x400);


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: [PATCH] Hamachi not doing pci_enable before reading resources

[Jeff Garzik]

Applied locally: pci_device_enable() cleanups for hamachi, eepro100,
starfire

I'll have to look at ne2k-pci, I think a patch in -ac may be spurious.

	Jeff


-- 
Jeff Garzik       | "You see, in this world there's two kinds of
Building 1024     |  people, my friend: Those with loaded guns
MandrakeSoft      |  and those who dig. You dig."  --Blondie
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: [PATCH] Hamachi not doing pci_enable before reading resources

[Richard B. Johnson]

On Wed, 7 Feb 2001 davej@suse.de wrote:

> 
> Hi Alan,
> 
>  Another driver not doing pci_enable_device() early enough.
> 
> Dave.
> 

A PCI device does not and should not be enabled to probe for resources!
It is only devices that have BIOS that require the device to be enabled
for memory I/O prior to downloading the BIOS into RAM. The BARs are
read/writable (and are required to be), even when the Mem/I/O bits
in the cmd/status register are clear.

This is a required condition!  You certainly don't want to write all
ones to a decode (to find the resource length) of a live, on-line chip!
If the chip hickups (think network chips connected to networks, on a
warm-boot), you will trash lots of stuff in memory.

It looks as though you are "fixing" drivers that are not broken and,
in fact, are trying to do the right thing. Maybe the PCI code in the
kernel is preventing access to resources unless the device has been
enabled??? If so, it's broken and should be fixed, instead of all
the drivers.

Cheers,
Dick Johnson

Penguin : Linux version 2.4.1 on an i686 machine (799.53 BogoMips).

"Memory is like gasoline. You use it up when you are running. Of
course you get it all back when you reboot..."; Actual explanation
obtained from the Micro$oft help desk.


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: [PATCH] Hamachi not doing pci_enable before reading resources

[Jeff Garzik]

"Richard B. Johnson" wrote:
> A PCI device does not and should not be enabled to probe for resources!

Some PCI devices do not -have- resources until pci_enable_device() is
called, hence the rule.


> It is only devices that have BIOS that require the device to be enabled
> for memory I/O prior to downloading the BIOS into RAM. The BARs are
> read/writable (and are required to be), even when the Mem/I/O bits
> in the cmd/status register are clear.
> 
> This is a required condition!  You certainly don't want to write all
> ones to a decode (to find the resource length) of a live, on-line chip!
> If the chip hickups (think network chips connected to networks, on a
> warm-boot), you will trash lots of stuff in memory.

When writing 0xFFFFFFFF to a BAR to find its length, you must disable IO
and MEM decoding.  This is a ideally what we should be doing anyway.. 
But you can re-enabling decoding once region size detection is
complete.  AND.  Region sizing only occurs once, and the value is cached
in dev->resource[], so it should not be occurring all over again, even
if pci_enable_device() is called.


> It looks as though you are "fixing" drivers that are not broken and,
> in fact, are trying to do the right thing. Maybe the PCI code in the
> kernel is preventing access to resources unless the device has been
> enabled??? If so, it's broken and should be fixed, instead of all
> the drivers.

wrong, you just missed this new "rule"...  

	Jeff



-- 
Jeff Garzik       | "You see, in this world there's two kinds of
Building 1024     |  people, my friend: Those with loaded guns
MandrakeSoft      |  and those who dig. You dig."  --Blondie
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: [PATCH] Hamachi not doing pci_enable before reading resources

[Richard B. Johnson]

On Wed, 7 Feb 2001, Jeff Garzik wrote:

> "Richard B. Johnson" wrote:
> > A PCI device does not and should not be enabled to probe for resources!
> 
> Some PCI devices do not -have- resources until pci_enable_device() is
> called, hence the rule.
> 

I stand by my statement. PCI devices that require resources are
required to provide read/write registers indicating these resources
whether or not the enable bits are set. This is mandatory. 


Cheers,
Dick Johnson

Penguin : Linux version 2.4.1 on an i686 machine (799.53 BogoMips).

"Memory is like gasoline. You use it up when you are running. Of
course you get it all back when you reboot..."; Actual explanation
obtained from the Micro$oft help desk.


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: [PATCH] Hamachi not doing pci_enable before reading resources

[Alan Cox]

> I stand by my statement. PCI devices that require resources are
> required to provide read/write registers indicating these resources
> whether or not the enable bits are set. This is mandatory. 

And the assignment of those resources is done by pci_enable_device. So
looking at the irq data or the BAR assignment until pci_enable_device
has done its work doesnt tell you anything meaningful

It doesnt just flip the IO and MEM bits

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: [PATCH] Hamachi not doing pci_enable before reading resources

[Gérard Roudier]

You missed the newer statements about every piece of hardware being
assumed to be hot-pluggable and all the hardware being under full control
by CPU.

You also missed the well known point that only device drivers are broken
under Linux and that all the generic O/S code is just perfect. :-)

  Gérard.

On Wed, 7 Feb 2001, Richard B. Johnson wrote:

> On Wed, 7 Feb 2001 davej@suse.de wrote:
> 
> > 
> > Hi Alan,
> > 
> >  Another driver not doing pci_enable_device() early enough.
> > 
> > Dave.
> > 
> 
> A PCI device does not and should not be enabled to probe for resources!
> It is only devices that have BIOS that require the device to be enabled
> for memory I/O prior to downloading the BIOS into RAM. The BARs are
> read/writable (and are required to be), even when the Mem/I/O bits
> in the cmd/status register are clear.
> 
> This is a required condition!  You certainly don't want to write all
> ones to a decode (to find the resource length) of a live, on-line chip!
> If the chip hickups (think network chips connected to networks, on a
> warm-boot), you will trash lots of stuff in memory.
> 
> It looks as though you are "fixing" drivers that are not broken and,
> in fact, are trying to do the right thing. Maybe the PCI code in the
> kernel is preventing access to resources unless the device has been
> enabled??? If so, it's broken and should be fixed, instead of all
> the drivers.
> 
> Cheers,
> Dick Johnson
> 
> Penguin : Linux version 2.4.1 on an i686 machine (799.53 BogoMips).
> 
> "Memory is like gasoline. You use it up when you are running. Of
> course you get it all back when you reboot..."; Actual explanation
> obtained from the Micro$oft help desk.
> 
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> Please read the FAQ at http://www.tux.org/lkml/
> 

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: [PATCH] Hamachi not doing pci_enable before reading resources

[Richard B. Johnson]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: TEXT/PLAIN; charset=US-ASCII, Size: 930 bytes --]

On Wed, 7 Feb 2001, [ISO-8859-1] Gérard Roudier wrote:

> 
> You missed the newer statements about every piece of hardware being
> assumed to be hot-pluggable and all the hardware being under full control
> by CPU.
> 
> You also missed the well known point that only device drivers are broken
> under Linux and that all the generic O/S code is just perfect. :-)
> 
>   Gérard.
> 

Yep. I missed a lot. When the next 'release' comes out, I'll have
to rewrite all my drivers again --sigh...

Cheers,
Dick Johnson

Penguin : Linux version 2.4.1 on an i686 machine (799.53 BogoMips).

"Memory is like gasoline. You use it up when you are running. Of
course you get it all back when you reboot..."; Actual explanation
obtained from the Micro$oft help desk.


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: [PATCH] Hamachi not doing pci_enable before reading resources

[davej]

Jeff Garzik wrote..

> rejected -- 'irq' assigned a value before pci_enable_device called.
> better patch installed locally.

Ugh, yep missed that one.
Will look more carefully for those assignments.

Dave.

-- 
| Dave Jones.        http://www.suse.de/~davej
| SuSE Labs

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/