On Sat, Nov 29, 2003 at 12:24:01PM -0500, Ron Gage wrote:
> On Saturday 29 November 2003 11:59 am, Domen Puncer wrote:
> > Second:
> > Patch is wrong
> > check_region returns !=0 when fails, request_region returns 0 on failure.
> > Also, you missed already existing request_region and release_region.
> >
> Updated patch to account for differences in the return value from
> check_region
> to request_region...
Right, but you didn't account for the existing request_region() calls.
This driver needs a complete overhaul anyway; it needs to be converted to the
"new" pci probe mechanism and it needs to stop calling pci_read_config_foo()
so much; we already cached copies of this information in pdev. But
here's an untested patch that fixes some of the worst abuses.
Index: drivers/net/skfp/skfddi.c
===================================================================
RCS file: /var/cvs/linux-2.6/drivers/net/skfp/skfddi.c,v
retrieving revision 1.4
diff -u -p -r1.4 skfddi.c
--- drivers/net/skfp/skfddi.c 28 Sep 2003 04:05:40 -0000 1.4
+++ drivers/net/skfp/skfddi.c 29 Nov 2003 21:18:13 -0000
@@ -265,15 +265,12 @@ int skfp_probe(struct net_device *dev)
struct pci_dev *pdev = NULL; /* PCI device structure */
#ifndef MEM_MAPPED_IO
u16 port; /* temporary I/O (port) address */
- int port_len; /* length of port address range (in bytes) */
#else
unsigned long port;
#endif
- u16 command; /* PCI Configuration space Command register val */
struct s_smc *smc; /* board pointer */
struct net_device *tmp = dev;
u8 first_dev_used = 0;
- u16 SubSysId;
PRINTK(KERN_INFO "entering skfp_probe\n");
@@ -308,53 +305,17 @@ int skfp_probe(struct net_device *dev)
}
if (pci_enable_device(pdev))
continue;
+ pci_set_master(pdev);
-#ifndef MEM_MAPPED_IO
- /* Verify that I/O enable bit is set (PCI slot is enabled) */
- pci_read_config_word(pdev, PCI_COMMAND, &command);
- if ((command & PCI_COMMAND_IO) == 0) {
- PRINTK("I/O enable bit not set!");
- PRINTK(" Verify that slot is enabled\n");
+ if (pci_request_regions(pdev, dev->name)) {
+ printk(KERN_ERR "Cannot allocate resources for "
+ "adapter %s\n", pci_name(pdev));
continue;
}
- /* Turn off memory mapped space and enable mastering */
-
- PRINTK(KERN_INFO "Command Reg: %04x\n", command);
- command |= PCI_COMMAND_MASTER;
- command &= ~PCI_COMMAND_MEMORY;
- pci_write_config_word(pdev, PCI_COMMAND, command);
-
- /* Read I/O base address from PCI Configuration Space */
-
- pci_read_config_word(pdev, PCI_BASE_ADDRESS_1, &port);
- port &= PCI_BASE_ADDRESS_IO_MASK; // clear I/O bit (bit 0)
-
- /* Verify port address range is not already being used */
-
- port_len = FP_IO_LEN;
- if (check_region(port, port_len) != 0) {
- printk("I/O range allocated to adapter");
- printk(" (0x%X-0x%X) is already being used!\n", port,
- (port + port_len - 1));
- continue;
- }
+#ifndef MEM_MAPPED_IO
+ port = pci_resource_start(pdev, 1);
#else
- /* Verify that MEM enable bit is set (PCI slot is enabled) */
- pci_read_config_word(pdev, PCI_COMMAND, &command);
- if ((command & PCI_COMMAND_MEMORY) == 0) {
- PRINTK("MEMORY-I/O enable bit not set!");
- PRINTK(" Verify that slot is enabled\n");
- continue;
- }
-
- /* Turn off IO mapped space and enable mastering */
-
- PRINTK(KERN_INFO "Command Reg: %04x\n", command);
- command |= PCI_COMMAND_MASTER;
- command &= ~PCI_COMMAND_IO;
- pci_write_config_word(pdev, PCI_COMMAND, command);
-
port = pci_resource_start(pdev, 0);
port = (unsigned long)ioremap(port, 0x4000);
@@ -371,8 +332,6 @@ int skfp_probe(struct net_device *dev)
}
first_dev_used = 1; // only significant first time
- pci_read_config_word(pdev, PCI_SUBSYSTEM_ID, &SubSysId);
-
if (tmp != NULL) {
if (loading_module)
link_modules(dev, tmp);
@@ -394,19 +353,18 @@ int skfp_probe(struct net_device *dev)
skb_queue_head_init(&smc->os.SendSkbQueue);
if (skfp_driver_init(dev) == 0) {
+ int SubSysId = pdev->subsystem_device;
// only increment global board
// count on success
num_boards++;
- request_region(dev->base_addr,
- FP_IO_LEN, dev->name);
if ((SubSysId & 0xff00) == 0x5500 ||
(SubSysId & 0xff00) == 0x5800) {
printk("%s: SysKonnect FDDI PCI adapter"
" found (SK-%04X)\n", dev->name,
SubSysId);
} else {
- printk("%s: FDDI PCI adapter found\n",
- dev->name);
+ printk("%s: FDDI PCI adapter found\n",
+ dev->name);
}
} else {
kfree(dev);
@@ -2581,8 +2539,10 @@ static struct net_device *unlink_modules
lp->os.LocalRxBufferDMA);
lp->os.LocalRxBuffer = NULL;
}
- release_region(p->base_addr,
- (lp->os.bus_type == SK_BUS_TYPE_PCI ? FP_IO_LEN : 0));
+#ifdef MEM_MAPPED_IO
+ iounmap((void *)p->base_addr);
+#endif
+ pci_release_regions(&lp->os.pdev);
}
unregister_netdev(p);
printk("%s: unloaded\n", p->name);
--
"Next the statesmen will invent cheap lies, putting the blame upon
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince
himself that the war is just, and will thank God for the better sleep
he enjoys after this process of grotesque self-deception." -- Mark Twain
|
