Bug 4940
Summary: | Repeatable Kernel Panic on Adaptec 2015S I20 device on bootup | ||
---|---|---|---|
Product: | Drivers | Reporter: | Joshua McClintock (joshuam) |
Component: | I2O | Assignee: | Alan (alan) |
Status: | CLOSED CODE_FIX | ||
Severity: | high | CC: | aacraid, akpm, bunk, Markus.Lidel |
Priority: | P2 | ||
Hardware: | i386 | ||
OS: | Linux | ||
Kernel Version: | 2.6.12.3 | Subsystem: | |
Regression: | --- | Bisected commit-id: | |
Attachments: |
Added request_region() call to find out if device is already in use
Use pci_request_regions() instead of request_region() move pci_request_regions() just behind pci_enable_device() If the PCI device is enabled already don't disable it at failure |
Description
Joshua McClintock
2005-07-25 14:13:06 UTC
A kind person on the list told me that I have 2 drivers trying to load and this is what's causing my problem. I removed I2O support while leaving support for the card itself and viola! I'm not sure if there is anyway around it, but maybe having some code in there to check for the existance of a previously loaded driver and skip if it's already loaded? I spoke to Markus Lidel about this and I think we agreed that yes, the i2o drivers should be fixed up so they don't both diddle with the same hardware. That means that both of them need to be taught about request_region() so the second-to-be-loaded driver will fail to allocate its I/O resources and will correctly bale out. I'll ask Mark Salyzyn to create a bugzilla account and to take a look at this. Markus what do you suggest as a means to check for the other driver's existence? So, tentatively based on request_region: --- dpt_i2o.c.orig 2005-08-05 07:52:21.057255479 -0400 +++ dpt_i2o.c 2005-08-05 07:51:47.471030222 -0400 @@ -1228,6 +1228,10 @@ hba_map0_area_size = 524288; } + if (!request_region(base_addr0_phys, hba_map0_area_size, DPT_DRIVER)) { + PERROR("dpti: adpt_config_hba: BAR0 region already claimed\n"); + return -ENODEV; + } base_addr_virt = ioremap(base_addr0_phys,hba_map0_area_size); if(!base_addr_virt) { PERROR("dpti: adpt_config_hba: io remap failed\n"); @@ -1235,10 +1239,17 @@ } if(raptorFlag == 1) { + if (!request_region(base_addr1_phys, hba_map1_area_size, DPT_DRIVER)) { + PERROR("dpti: adpt_config_hba: BAR1 region already claimed\n"); + iounmap(base_addr_virt); + release_region(base_addr0_phys,hba_map0_area_size); + return -ENODEV; + } msg_addr_virt = ioremap(base_addr1_phys, hba_map1_area_size ); if(!msg_addr_virt) { PERROR("dpti: adpt_config_hba: io remap failed on BAR1 \n"); iounmap(base_addr_virt); + release_region(base_addr0_phys,hba_map0_area_size); return -EINVAL; } } else { @@ -1250,8 +1261,10 @@ if( pHba == NULL) { if(msg_addr_virt != base_addr_virt){ iounmap(msg_addr_virt); + release_region(base_addr1_phys,hba_map1_area_size); } iounmap(base_addr_virt); + release_region(base_addr0_phys,hba_map0_area_size); return -ENOMEM; } memset(pHba, 0, sizeof(adpt_hba)); @@ -1321,7 +1334,11 @@ int j; struct adpt_device* pDev; struct adpt_device* pNext; - + ulong base_addr0_phys = 0; + ulong base_addr1_phys = 0; + u32 hba_map0_area_size = 0; + u32 hba_map1_area_size = 0; + int raptorFlag = 0; down(&adpt_configuration_lock); // scsi_unregister calls our adpt_release which @@ -1344,9 +1364,40 @@ hba_count--; up(&adpt_configuration_lock); + base_addr0_phys = pci_resource_start(pHba->pDev,0); + hba_map0_area_size = pci_resource_len(pHba->pDev,0); + + // Check if standard PCI card or single BAR Raptor + if(pHba->pDev->device == PCI_DPT_DEVICE_ID){ + if(pHba->pDev->subsystem_device >=0xc032 && pHba->pDev- >subsystem_device <= 0xc03b){ + // Raptor card with this device id needs 4M + hba_map0_area_size = 0x400000; + } else { // Not Raptor - it is a PCI card + if(hba_map0_area_size > 0x100000 ){ + hba_map0_area_size = 0x100000; + } + } + } else {// Raptor split BAR config + // Use BAR1 in this configuration + base_addr1_phys = pci_resource_start(pHba->pDev,1); + hba_map1_area_size = pci_resource_len(pHba->pDev,1); + raptorFlag = 1; + } + /* x86_64 machines need more optimal mappings */ + if(raptorFlag == 1) { + if (hba_map0_area_size > 128) + hba_map0_area_size = 128; + if (hba_map1_area_size > 524288) + hba_map1_area_size = 524288; + } else { + if (hba_map0_area_size > 524288) + hba_map0_area_size = 524288; + } iounmap(pHba->base_addr_virt); + release_region(base_addr0_phys,hba_map0_area_size); if(pHba->msg_addr_virt != pHba->base_addr_virt){ iounmap(pHba->msg_addr_virt); + release_region(base_addr1_phys,hba_map1_area_size); } if(pHba->hrt_va) { pci_free_consistent(pHba->pDev, le32_to_cpu(pHba->hrt_va- >num_entries) * le32_to_cpu(pHba->hrt_va->entry_len) << 2, pHba->hrt_va, pHba- >hrt_pa); sweet, thanks Mark. Could you please email me that patch when you're happy with it so I can add it to my collection? Created attachment 5546 [details]
Added request_region() call to find out if device is already in use
Patch to avoid that dpt_i2o and I2O subsystem both claim the same controller at
the same time.
Comments from Alan and Markus simplifies and fixes this patch to: --- dpt_i2o.c.orig 2005-08-08 08:25:46.888900616 -0400 +++ dpt_i2o.c 2005-08-08 08:30:48.062596580 -0400 @@ -1228,8 +1228,13 @@ hba_map0_area_size = 524288; } + if (pci_request_regions(pDev)) { + PERROR("dpti: adpt_config_hba: pci request region failed\n"); + return -EINVAL; + } base_addr_virt = ioremap(base_addr0_phys,hba_map0_area_size); if(!base_addr_virt) { + pci_release_regions(pDev); PERROR("dpti: adpt_config_hba: io remap failed\n"); return -EINVAL; } @@ -1239,6 +1244,7 @@ if(!msg_addr_virt) { PERROR("dpti: adpt_config_hba: io remap failed on BAR1 \n"); iounmap(base_addr_virt); + pci_release_regions(pDev); return -EINVAL; } } else { @@ -1252,6 +1258,7 @@ iounmap(msg_addr_virt); } iounmap(base_addr_virt); + pci_release_regions(pDev); return -ENOMEM; } memset(pHba, 0, sizeof(adpt_hba)); @@ -1345,6 +1352,7 @@ up(&adpt_configuration_lock); iounmap(pHba->base_addr_virt); + pci_release_regions(pHba->pDev); if(pHba->msg_addr_virt != pHba->base_addr_virt){ iounmap(pHba->msg_addr_virt); } Created attachment 5548 [details]
Use pci_request_regions() instead of request_region()
Use pci_request_regions() instead of request_region(), which is much easier to
use (thanks to Alan Cox and Mark Salyzyn).
Thanks, Markus. Could you please email me that patch when it's ready to go? And cc Mark? This patch is now in Linus' tree and will therefore be included in 2.6.13-rc7. I have a problem with 2.6.14.3 and the dpti driver with an Adaptec 2100S card. During boot I get: iop0: controller found PCI: Unable to reserv mem region ... for device ...(iop0) iop0: device already claimed iop0: DMA/IO allocation fopr I2O controller failed ... ... dpti0: Trying to abort cmd=56 The system hangs here, bootup with 2.6.8 debian stable works fine, I will now try older kernel versions. The raid array and disks where detected correctly prior to the messages above. Hi Felix, you seem to have a different problem. Please open a new bug for it. Adrian is partially right in that the dpt_i2o driver is failing even despite the code that prevents both drivers from loading to attach to the Adapter (iop0: device already claimed; meaning the dpt_i2o driver loaded first). However, I would like to ask if anyone *tested* this code to make sure that the i2o_block driver failing to load did not somehow result in some interference with the previously loaded dpt_i2o driver? Please remove the i2o_block driver from your 2.6.14.3 configuration and see if the dpt_i2o driver functions correctly. If it continues to be dysfunctional, then open a different bug report. If it springs to life, then we need to look further into this. Mark, I haven't tested as you sad but already opened a new report. I will try to test other configurations/kernels tomorrow. Created attachment 6995 [details]
move pci_request_regions() just behind pci_enable_device()
In linux 2.6.15, data transfer does hang when both drivers are loaded.
It seems that location of pci_request_regions() are wrong.
I moved it just behind pci_enable_device() like other drivers,
and it becomes fine.
whee, we fixed a bug! Could the reporters please test this? Kamei, could you please mail that bug to myself and to Mark Salyzyn <aacraid@adaptec.com> with the abvove description and a Signed-off-by: as per section 11 of Documentation/SubmittingPatches? Thanks. Hello, probably the reason why this works, is because in the "old" version disables the PCI device if it fails to load, independent if the device is enabled or not. So IMHO it would be better to not disable the device if it is enabled before, or? Thanks you very much! Created attachment 7009 [details]
If the PCI device is enabled already don't disable it at failure
|