PATCH: trident , pci_enable_device moved
Hi Alan, linux-kernel,
This moves pci_enable_device() in trident.c before any PCI resource access. Everything else appears to be ok in regards to 2.4 PCI API and return values.
Ciao, Marcus
Index: trident.c
===================================================================
RCS file: /build/mm/work/repository/linux-mm/drivers/sound/trident.c,v retrieving revision 1.12
trident.c +3 -3
@@ -3309,6 +3309,9 @@struct trident_card *card;u8 revision; +if (pci_enable_device (pci_dev)) +return -ENODEV ; +if (pci_set_dma_mask (pci_dev,TRIDENT_DMA_MASK )) {printk (KERN_ERR "trident: architecture does not support" " 30bit PCI busmaster DMA\n" );
@@ -3322,9 +3325,6 @@ iobase);return -ENODEV ; } - -if (pci_enable_device (pci_dev)) -return -ENODEV ;if ((card =kmalloc (sizeof (struct trident_card ),GFP_KERNEL )) ==NULL ) {printk (KERN_ERR "trident: out of memory\n" );
Quoted 6 lines · Marcus Meissner
Hi Alan, linux-kernel,
This moves
pci_enable_device()intrident.cbefore any PCI resource access. Everything else appears to be ok in regards to 2.4 PCI API and return values.Ciao, Marcus
Argh, actually the return value of pci_enable_device*() should be returned.
Ciao, Marcus
Index: drivers/sound/trident.c
===================================================================
RCS file: /build/mm/work/repository/linux-mm/drivers/sound/trident.c,v retrieving revision 1.12
sound/trident.c +4 -3
@@ -3308,7 +3308,11 @@unsigned long iobase;struct trident_card *card;u8 revision; +int ret; +if ((ret=pci_enable_device (pci_dev))) +return ret; +if (pci_set_dma_mask (pci_dev,TRIDENT_DMA_MASK )) {printk (KERN_ERR "trident: architecture does not support" " 30bit PCI busmaster DMA\n" );
@@ -3322,9 +3326,6 @@ iobase);return -ENODEV ; } - -if (pci_enable_device (pci_dev)) -return -ENODEV ;if ((card =kmalloc (sizeof (struct trident_card ),GFP_KERNEL )) ==NULL ) {printk (KERN_ERR "trident: out of memory\n" );
Just a warning; I was informed by Alan that doing this for video drivers was unnecessary, since video devices were already enabled during bootup.
Quoted 13 lines · Marcus Meissner
Quoted 6 lines · Marcus Meissner
Hi Alan, linux-kernel,
This moves
pci_enable_device()intrident.cbefore any PCI resource access. Everything else appears to be ok in regards to 2.4 PCI API and return values.Ciao, Marcus
Argh, actually the return value of pci_enable_device*() should be returned.
Ciao, Marcus
Signature · 6 lines
"... being a Linux user is sort of like living in a house inhabited
by a large family of carpenters and architects. Every morning when
you wake up, the house is a little different. Maybe there is a new
turret, or some walls have moved. Or perhaps someone has temporarily
removed the floor under your bed." - Unix for Dummies, 2nd Edition
-- found in the .sig of Rob Riggs, [email protected]Quoted 3 lines · Andres Salomon
Just a warning; I was informed by Alan that doing this for video drivers was unnecessary, since video devices were already enabled during bootup.
To clarify: the primary display device is enabled and initialized, and its video BIOS executed, when during BIOS startup and before the Linux kernel gets control. All other display devices are not only not initialized, but they are disabled as well.
Marcus is doing sound ATM so I doubt this matters to him...
Signature · 4 lines
Jeff Garzik | The difference between America and England is that
Building 1024 | the English think 100 miles is a long distance and
MandrakeSoft | the Americans think 100 years is a long time.
| (random fortune)Oops, I saw "trident" and thought video. Sorry, marcus. :)
This is what I was told (it was only needed for secondary video devices). From that, I would expect that all video devices would need it, just in case they happened to be the second card. Am I missing some subtlety in some of the video driers/chipsets that wouldn't allow them to be used as a second video device (therefore not requiring pci_enable_device)?
Quoted 19 lines · Jeff Garzik
Quoted 3 lines · Andres Salomon
Just a warning; I was informed by Alan that doing this for video drivers was unnecessary, since video devices were already enabled during bootup.
To clarify: the primary display device is enabled and initialized, and its video BIOS executed, when during BIOS startup and before the Linux kernel gets control. All other display devices are not only not initialized, but they are disabled as well.
Marcus is doing sound ATM so I doubt this matters to him...
Signature · 4 lines
Jeff Garzik | The difference between America and England is that Building 1024 | the English think 100 miles is a long distance and MandrakeSoft | the Americans think 100 years is a long time. | (random fortune)
Signature · 6 lines
"... being a Linux user is sort of like living in a house inhabited
by a large family of carpenters and architects. Every morning when
you wake up, the house is a little different. Maybe there is a new
turret, or some walls have moved. Or perhaps someone has temporarily
removed the floor under your bed." - Unix for Dummies, 2nd Edition
-- found in the .sig of Rob Riggs, [email protected]Quoted 6 lines · Andres Salomon
This is what I was told (it was only needed for secondary video devices). From that, I would expect that all video devices would need it, just in case they happened to be the second card. Am I missing some subtlety in some of the video driers/chipsets that wouldn't allow them to be used as a second video device (therefore not requiring
pci_enable_device)?
They do need pci_enable_device, both primary and secondary displays. For the primary display its safe to call pci_enable_device. For secondary displays, you have to first disable I/O decoding for all VGA devices before you can enable a secondary display. You don't want more than one device decoding the legacy VGA region at any one time.
Some cards have the capability to relocate the VGA region, which is nice. The bigger problem is initializing secondary displays; every video card has a proprietary video BIOS initialization sequence that is run by main BIOS on startup. You can either duplicate this sequence with C code, which is sometimes difficult due to lack of docs or variety of boards, or you can execute the video BIOS with an x86 emulator.
Signature · 4 lines
Jeff Garzik | The difference between America and England is that
Building 1024 | the English think 100 miles is a long distance and
MandrakeSoft | the Americans think 100 years is a long time.
| (random fortune)Quoted 14 lines · Jeff Garzik
Quoted 6 lines · Andres Salomon
This is what I was told (it was only needed for secondary video devices). From that, I would expect that all video devices would need it, just in case they happened to be the second card. Am I missing some subtlety in some of the video driers/chipsets that wouldn't allow them to be used as a second video device (therefore not requiring
pci_enable_device)?They do need
pci_enable_device, both primary and secondary displays. For the primary display its safe to callpci_enable_device. For secondary displays, you have to first disable I/O decoding for all VGA devices before you can enable a secondary display. You don't want more than one device decoding the legacy VGA region at any one time.
Quoted 6 lines
Some cards have the capability to relocate the VGA region, which is nice. The bigger problem is initializing secondary displays; every video card has a proprietary video BIOS initialization sequence that is run by main BIOS on startup. You can either duplicate this sequence with C code, which is sometimes difficult due to lack of docs or variety of boards, or you can execute the video BIOS with an x86 emulator.
Note: With linuxBIOS (and some other embedded linux setups) even a primary display doesn't get initialized until you start linux so if you can properly initialize your display please do it.
Eric