login
Search
Hi Bart, i broke ide-floppy for Iomega ZIP drives with the last round of generic patches and now it works only sometimes during write requests. The reason for it is that the command issue path is not being delayed with a 50msec timeout, for details see the comment in idefloppy_start_pc(). Anyway, attached is a fix that should go into the -stable kernel too since the driver is now broken in 2.6.26. On a different note, the current pata tree on top of v2.6.25-2125-g50515af blows up here with the following error: [ 4.296729] Uniform Multi-Platform E-IDE driver [ 4.297905] ICH4: IDE controller (0x8086:0x24cb rev 0x02) at PCI slot 0000:00:1f.1 [ 4.297986] ACPI: PCI Interrupt 0000:00:1f.1[A] -> GSI 18 (level, low) -> IRQ 18 [ 4.298153] ICH4: not 100% native mode: will probe irqs later [ 4.298213] ide0: BM-DMA at 0xfc00-0xfc07 [ 4.298282] ide1: BM-DMA at 0xfc08-0xfc0f [ 4.561768] hda: QUANTUM FIREBALLlct10 20, ATA DISK drive [ 4.816724] hdb: SAMSUNG SP2014N, ATA DISK drive [ 4.867959] hda: drive side 80-wire cable detection failed, limiting max speed to UDMA33 [ 4.868027] hda: UDMA/33 mode selected [ 4.868441] hdb: UDMA/100 mode selected [ 5.540683] hdc: IOMEGA ZIP 100 ATAPI, ATAPI FLOPPY drive [ 5.795564] hdd: IC35L120AVV207-0, ATA DISK drive [ 5.847295] hdd: host side 80-wire cable detection failed, limiting max speed to UDMA33 [ 5.847362] hdd: UDMA/33 mode selected [ 5.847715] ide0 at 0x1f0-0x1f7,0x3f6 on irq 14 [ 5.855487] ide1 at 0x170-0x177,0x376 on irq 15 [ 5.875927] ide_generic: please use "probe_mask=0x3f" module parameter for probing all legacy ISA IDE ports [ 5.876012] ide_generic: I/O resource 0x1F0-0x1F7 not free. [ 5.876074] ide_generic: I/O resource 0x170-0x177 not free. [ 11.342504] hde: no response (status = 0xa1), resetting drive [ 17.206535] hdf: no response (status = 0xa1), resetting drive [ 17.614474] ------------[ cut here ]------------ [ 17.614528] WARNING: at lib/kref.c:43 ...
Hi, On Tuesday 15 July 2008, Borislav Petkov wrote: hde? hdf? Unfortunately I couldn't reproduce this problem here (2.6.26 + pata tree) Thanks, I folded the fix into original patch (->dev_flags is not yet upstream so -stable fix shouldn't be necessary). While on it: I later noticed that there will be also need for common ATA/ATAPI ->dev_flags in the future so I wonder whether current ->dev_flags should be renamed to ->atapi_flags (& s/*DFLAG*/*AFLAG*/). If there is agreement on this I'll fix it in pata tree. --
... or if there's room, use a single ->dev_flags for all possible flag settings?
--
Regards/Gruß,
Boris.
--
Close, it is related to MAX_HWIFS being upper bound on hws[] in ide_generic.c but now we loop for ARRAY_SIZE(legacy_bases). IOW there is a memset(hws, 0, MAX_HWIFS) missing at the top of ide_generic_init() (please re-test after adding it). Unfortunately there isn't enough room left (27 bits are occupied ATM) and having u64 ->dev_flags sucks... --
I'm pretty sure some of the 27 will be removed later but yeah, u64 flags is
kinda bad since it has to be always atomically updated and this has to be
explicitly staged on 32bit cpus due to the wordsize. I guess two flags members
are the easiest thing to do for now, you might add some comments to both so we
know which is which.
--
Regards/Gruß,
Boris.
--
I fixed this in pata tree now with: Sure, I will remember about this when it comes time for ->dev_flags. Thanks, Bart --
Hi Bart,
here's the root cause for the problem: I had both Intel ICH chipset
(BLK_DEV_PIIX) und generic ide (BLK_DEV_GENERIC) selected in Kconfig and since
ICH4 uses the generic driver detection routine, the second(!) generic detection after the
ICH4 one failed and died. This is why request_region()-resources are shown as
One of the possible fixes is adding
depends on !BLK_DEV_GENERIC
after each IDE chipset driver using the generic detection in drivers/ide/Kconfig
but it's a not-that-elegant one. Another thing would be using a dummy one like
BLK_DEV_IDEDMA_PCI, but I'm not that sure. Will look into it. I'm pretty sure
--
Regards/Gruß,
Boris.
--
I also use BLK_DEV_PIIX + BLK_DEV_GENERIC configuration
(for testing purposes) and it works fine here.
[ Besides it shouldn't result in phantom hde & hdf devices
and ide_generic blowing up on failure. ]
Have you tried the memset() fix that I proposed
pata_legacy.c has a proper fix which needs porting into ide-generic.c
(it should be pretty easy thing to do).
[ The fix is to skip automatic-probing of primary port and/or secondary
one if a PCI controller using legacy I/O bases is detected:
for_each_pci_dev(p) {
int r;
/* Check for any overlap of the system ATA mappings. Native
mode controllers stuck on these addresses or some devices
in 'raid' mode won't be found by the storage class test */
for (r = 0; r < 6; r++) {
if (pci_resource_start(p, r) == 0x1f0)
primary = 1;
if (pci_resource_start(p, r) == 0x170)
secondary = 1;
}
/* Check for special cases */
legacy_check_special_cases(p, &primary, &secondary); ]
Thanks,
Bart
--
Well, I added some more debug printk's to see how the hosts get initialized and here's what it looks like here: [ 4.297031] netconsole: network logging started [ 4.297086] Uniform Multi-Platform E-IDE driver [ 4.297457] ICH4: IDE controller (0x8086:0x24cb rev 0x02) at PCI slot 0000:00:1f.1 [ 4.297547] pci 0000:00:1f.1: PCI INT A -> GSI 18 (level, low) -> IRQ 18 [ 4.297658] ICH4: not 100% native mode: will probe irqs later [ 4.297714] ide_host_register: loop0: i=0, hwif=dfa19000 [ 4.297774] ide0: BM-DMA at 0xfc00-0xfc07 [ 4.297835] ide_host_register: loop0: i=1, hwif=dfa19800 [ 4.298041] ide1: BM-DMA at 0xfc08-0xfc0f [ 4.298102] ide_host_register: loop0: i=2, hwif=00000000 [ 4.298157] ide_host_register: loop0: i=3, hwif=00000000 [ 4.298211] ide_host_register: loop0: i=4, hwif=00000000 [ 4.298267] ide_host_register: loop0: i=5, hwif=00000000 [ 4.298321] ide_host_register: loop0: i=6, hwif=00000000 [ 4.298375] ide_host_register: loop0: i=7, hwif=00000000 [ 4.298429] ide_host_register: loop0: i=8, hwif=00000000 [ 4.298483] ide_host_register: loop0: i=9, hwif=00000000 [ 4.298538] ide_host_register: loop1: i=0, hwif=dfa19000 [ 4.562086] hda: QUANTUM FIREBALLlct10 20, ATA DISK drive [ 4.817043] hdb: SAMSUNG SP2014N, ATA DISK drive [ 4.867892] ide_host_register: hwif=dfa19000 [ 4.868350] hda: drive side 80-wire cable detection failed, limiting max speed to UDMA33 [ 4.868419] hda: UDMA/33 mode selected [ 4.868827] hdb: UDMA/100 mode selected [ 4.868974] ide_host_register: loop1: i=1, hwif=dfa19800 [ 5.541001] hdc: IOMEGA ZIP 100 ATAPI, ATAPI FLOPPY drive [ 5.795879] hdd: IC35L120AVV207-0, ATA DISK drive [ 5.846729] ide_host_register: hwif=dfa19800 [ 5.847673] hdd: host side 80-wire cable detection failed, limiting max speed to UDMA33 [ 5.847756] hdd: UDMA/33 mode selected [ 5.848002] ide_host_register: loop1: i=2, hwif=00000000 [ 5.848056] ide_host_register: loop1: i=3, ...
Hi,
On Tuesday 22 July 2008, Borislav Petkov wrote:
*sigh*
The previous fix was garbage and contained brown-paper-bag bug:
diff -u b/drivers/ide/ide-generic.c b/drivers/ide/ide-generic.c
--- b/drivers/ide/ide-generic.c
+++ b/drivers/ide/ide-generic.c
@@ -114,7 +114,7 @@
printk(KERN_INFO DRV_NAME ": please use \"probe_mask=0x3f\" module "
"parameter for probing all legacy ISA IDE ports\n");
- memset(hws, 0, MAX_HWIFS);
+ memset(hws, 0, sizeof(hw_regs_t *) * MAX_HWIFS);
for (i = 0; i < ARRAY_SIZE(legacy_bases); i++) {
io_addr = legacy_bases[i];
It has *HINT* written all over it. ;)
Thanks,
Bart
--
Hm, let's see whether there's time during the weekend. I already have something
stolen from pata_legacy but I'll do some more testing first. By the way, what
are the chances of exporting those pieces of code from drivers/ata/pata_legacy.c
and adding the function def into some header instead of duplicating the code into
ide_generic.c?
--
Regards/Gruß,
Boris.
--
On Wednesday 23 July 2008, Borislav Petkov wrote: Good idea (<linux/ata.h> sounds like a perfect spot). Thanks, Bart --
[ removed stable@kernel.org from the CC-list ] Hi Bart, i finally found some time to work on the iobase-exclusion. Actually, i dropped the original idea of reusing pata_legacy code without duplicating it since this got the whole SATA pulled in in Kconfig, which, imo, outweighs the savings from not duplicating one function. I ended up refitting the pata_legacy iobase checks into ide-generic. As a result, i have now a new bool-Kconfig option BLK_DEV_GENERIC_ONLY which gets reverse-selected only when no pci ide controller which is using the generic ide_host_register() from within ide_pci_init_one() is selected in Kconfig. This is tested both with and without a pci ide driver selected in addition to ide-generic. --- From: Borislav Petkov <petkovbb@gmail.com> Date: Fri, 1 Aug 2008 07:33:13 +0200 Subject: [PATCH] ide-generic: skip automatic probing of legacy iobases A number of pci ide controllers use legacy IO bases for their primary and secondary ports. Skip probing those when both a specific host driver _and_ ide-generic are enabled. The checking code originates from drivers/ata/pata_legacy.c and is only reorganized into ide-generic. Signed-off-by: Borislav Petkov <petkovbb@gmail.com> --- drivers/ide/Kconfig | 4 +++ drivers/ide/ide-generic.c | 49 +++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/drivers/ide/Kconfig b/drivers/ide/Kconfig index 611319b..f103f5f 100644 --- a/drivers/ide/Kconfig +++ b/drivers/ide/Kconfig @@ -386,10 +386,14 @@ config BLK_DEV_OFFBOARD config BLK_DEV_GENERIC tristate "Generic PCI IDE Chipset Support" select BLK_DEV_IDEPCI + select BLK_DEV_GENERIC_ONLY if !(BLK_DEV_AEC62XX || BLK_DEV_ALI15X3 || BLK_DEV_AMD74XX || BLK_DEV_ATIIXP || BLK_DEV_CMD64X || BLK_DEV_CS5530 || BLK_DEV_CS5535 || BLK_DEV_HPT34X || BLK_DEV_HPT366 || BLK_DEV_IT821X || BLK_DEV_IT8213 || BLK_DEV_JMICRON || BLK_DEV_NS87415 || BLK_DEV_OPTI621 || BLK_DEV_PDC202XX_OLD || BLK_DEV_PDC202XX_NEW || ...
Hi, Why not try <linux/ata.h> + inline trick instead? [ <linux/ata.h> is shared by both stacks so by moving the function there + making it inline it can also be shared without the need for dependency How's about just leaving the final decision up to the user with changing probe_mask in ide_generic from 0x3 to 0x0 and automatically probing for ports 0-1 iff there is no IDE PCI controller present (otherwise check probe_mask). This is should remove the need for Kconfig magic and is a sane default since a lot of people get caught using ide_generic by mistake and not by intent (IOW they forgot to enable the right IDE PCI host driver). [ The small minority which may use it by intent (I don't see any practical reasons for doing it though) would still be able to override the default --
On Sat, Aug 02, 2008 at 07:02:12PM +0200, Bartlomiej Zolnierkiewicz wrote:
Wait, let me get this straight: you want to set probe_mask to 0x0 as a default,
which skips probing of the primary and secondary ports, and to do the checking
whether the IDE PCI controller uses legacy iobases only when the user has
enforced it by setting probe_mask to 0x3? At least this is how i understand
[.. ]
--
Regards/Gruss,
Boris.
--
Nope, always do the checking and if there is no IDE PCI controller do the probing (& if there is IDE PCI controller present check probe_mask bits). --
Ok, here's a definitely better solution:
---
From: Borislav Petkov <petkovbb@gmail.com>
Date: Sun, 3 Aug 2008 08:31:20 +0200
Subject: [PATCH 1/2] pata_legacy: export functionality to ide
export the legacy iobases checking code to other
users (ide) by pushing it up into the header.
CC: Alan Cox <alan@redhat.com>
Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
drivers/ata/pata_legacy.c | 63 +-----------------------------------------
include/linux/ata.h | 67 +++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 68 insertions(+), 62 deletions(-)
diff --git a/drivers/ata/pata_legacy.c b/drivers/ata/pata_legacy.c
index bc037ff..14d187e 100644
--- a/drivers/ata/pata_legacy.c
+++ b/drivers/ata/pata_legacy.c
@@ -50,7 +50,6 @@
#include <linux/kernel.h>
#include <linux/module.h>
-#include <linux/pci.h>
#include <linux/init.h>
#include <linux/blkdev.h>
#include <linux/delay.h>
@@ -1040,47 +1039,6 @@ fail:
return ret;
}
-/**
- * legacy_check_special_cases - ATA special cases
- * @p: PCI device to check
- * @master: set this if we find an ATA master
- * @master: set this if we find an ATA secondary
- *
- * A small number of vendors implemented early PCI ATA interfaces
- * on bridge logic without the ATA interface being PCI visible.
- * Where we have a matching PCI driver we must skip the relevant
- * device here. If we don't know about it then the legacy driver
- * is the right driver anyway.
- */
-
-static void __init legacy_check_special_cases(struct pci_dev *p, int *primary,
- int *secondary)
-{
- /* Cyrix CS5510 pre SFF MWDMA ATA on the bridge */
- if (p->vendor == 0x1078 && p->device == 0x0000) {
- *primary = *secondary = 1;
- return;
- }
- /* Cyrix CS5520 pre SFF MWDMA ATA on the bridge */
- if (p->vendor == 0x1078 && p->device == 0x0002) {
- *primary = *secondary = 1;
- return;
- }
- /* Intel MPIIX - PIO ATA on non PCI side of bridge */
- if (p->vendor == 0x8086 && p->device == 0x1234) {
- u16 ...On Sun, 3 Aug 2008 09:37:56 +0200 Please don't stuff large important pieces of code in header files where they will be overlooked NAK this. I'm happy to have a shared library directory for ATA stuff, containing useful C code, but hiding stuff in headers like that is just plain wrong. --
The code in question is 65 LOC total (43 LOC without counting comments) so having a shared library just for it sounds like an overkill and we may just copy that one function from pata_legacy to ide_generic instead. Jeff, what is your stance here? --
People expect code in C files, so in headers it gets missed as well as If you are going to #include two copies you might as well just copy it. Alan --
That's pretty much my feeling... just copy the code. If the shared code grows larger, create a kernel module with the stuff shared by both libata and drivers/ide. liblibata? libata-core-core? :) Jeff --
What do you mean by "overlooked"? If you're looking for the function defintion,
any sensible code indexing tool will point you to the right place.
And linux/ata.h already contains several c one liners/helpers. What is the
difference between the two new functions and the ones already present there?
Although the solution i propose is not adhering to some header/c file
conventions, it is still the best one considering the other possibilities:
a) code duplication: dumb idea, bloated kernel for no reason
b) evil Kconfig SELECT pulling in core libata just so that ide might be calling
a function or two.
[.. ]
--
Regards/Gruss,
Boris.
--
I think you might want to start somewhere else if that worries you. Its also a mostly bogus reasoning as almost nobody builds with both, in fact you have to be pretty careful if you do that or it all falls over in a heap. For previous cases (eg ide timing) it has actually made more sense to split the code. The moment you get future different behaviour between libata and old IDE on any of these devices the sharing will just break again (eg if one or the other drops chipset support for one of those That would be stunningly dumb. I happen to think we have developers whose minds extent to adding libata-common.c and pulling in a single file if we do that. Yes people have the past done stupid stuff like pulling all of CONFIG_IDE in for a single USB device but that was because nobody noticed and fixed it promptly not because it was a good idea. Alan --
[shortened up CC list]
Ok then, so we duplicate. This seems like the easiest solution. Sharing code
between libata and IDE is not that smart in case the two development directions
divert, as you said. Bart, here's v3:
---
From: Borislav Petkov <petkovbb@gmail.com>
Date: Sun, 3 Aug 2008 18:46:35 +0200
Subject: [PATCH] ide-generic: handle probing of legacy io-ports v3
Avoid probing the io-ports in case an IDE PCI controller is present and it uses
the legacy iobases. If we still want to enforce the probing, we do
ide_generic.probe_mask=0x3f
on the kernel command line. The iobase checking code is adapted from
drivers/ata/pata_legacy.c
Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
drivers/ide/ide-generic.c | 60 ++++++++++++++++++++++++++++++++++++++++++---
1 files changed, 56 insertions(+), 4 deletions(-)
diff --git a/drivers/ide/ide-generic.c b/drivers/ide/ide-generic.c
index 8fe8b5b..e9b7b69 100644
--- a/drivers/ide/ide-generic.c
+++ b/drivers/ide/ide-generic.c
@@ -27,7 +27,7 @@
#define DRV_NAME "ide_generic"
-static int probe_mask = 0x03;
+static int probe_mask;
module_param(probe_mask, int, 0);
MODULE_PARM_DESC(probe_mask, "probe mask for legacy ISA IDE ports");
@@ -100,19 +100,71 @@ static const u16 legacy_bases[] = { 0x1f0, 0x170, 0x1e8, 0x168, 0x1e0, 0x160 };
static const int legacy_irqs[] = { 14, 15, 11, 10, 8, 12 };
#endif
+
+static void ide_generic_check_pci_legacy_iobases(int *primary, int *secondary)
+{
+ struct pci_dev *p = NULL;
+ u16 val;
+
+ for_each_pci_dev(p) {
+ int r;
+
+ for (r = 0; r < 6; r++) {
+ if (pci_resource_start(p, r) == 0x1f0)
+ *primary = 1;
+ if (pci_resource_start(p, r) == 0x170)
+ *secondary = 1;
+ }
+
+ /* Cyrix CS5510 pre SFF MWDMA ATA on the bridge */
+ if (p->vendor == 0x1078 && p->device == 0x0000)
+ *primary = *secondary = 1;
+
+ /* Cyrix CS5520 pre SFF MWDMA ATA on the bridge */
+ if (p->vendor == 0x1078 && p->device == 0x0002)
+ *primary = *secondary = ...Hello.
Also, perhaps it makes sense to #include <linux/pci_ids.h> and use the
macros defined there...
MBR, Sergei
--
This is code is actually from the pata_legacy.c but yep, you're right, Will look into it later and redo the patch, thanks for reviewing. I still haven't heard from Bart, though, whether he's OK with the code duplication...? -- Regards/Gruß, Boris --
On Tue, Aug 5, 2008 at 4:32 PM, Boris Petkov <petkovbb@googlemail.com> wrote: Given Alan & Jeff concerns about moving it to <linux/ata.h>, just copying the function to ide_generic seems like a best solution for now. --
---
From: Borislav Petkov <petkovbb@gmail.com>
Date: Sun, 3 Aug 2008 18:46:35 +0200
Subject: [PATCH] ide-generic: handle probing of legacy io-ports v4
Avoid probing the io-ports in case an IDE PCI controller is present and it uses
the legacy iobases. If we still want to enforce the probing, we do
ide_generic.probe_mask=0x3f
on the kernel command line. The iobase checking code is adapted from
drivers/ata/pata_legacy.c after converting hex pci ids into their corresponding
macros in <linux/pci_ids.h>.
CC: Sergei Shtylyov <sshtylyov@ru.mvista.com>
Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
drivers/ide/ide-generic.c | 59 +++++++++++++++++++++++++++++++++++++++++---
1 files changed, 55 insertions(+), 4 deletions(-)
diff --git a/drivers/ide/ide-generic.c b/drivers/ide/ide-generic.c
index 8fe8b5b..efce159 100644
--- a/drivers/ide/ide-generic.c
+++ b/drivers/ide/ide-generic.c
@@ -19,6 +19,7 @@
#include <linux/init.h>
#include <linux/module.h>
#include <linux/ide.h>
+#include <linux/pci_ids.h>
/* FIXME: convert m32r to use ide_platform host driver */
#ifdef CONFIG_M32R
@@ -27,7 +28,7 @@
#define DRV_NAME "ide_generic"
-static int probe_mask = 0x03;
+static int probe_mask;
module_param(probe_mask, int, 0);
MODULE_PARM_DESC(probe_mask, "probe mask for legacy ISA IDE ports");
@@ -100,19 +101,69 @@ static const u16 legacy_bases[] = { 0x1f0, 0x170, 0x1e8, 0x168, 0x1e0, 0x160 };
static const int legacy_irqs[] = { 14, 15, 11, 10, 8, 12 };
#endif
+static void ide_generic_check_pci_legacy_iobases(int *primary, int *secondary)
+{
+ struct pci_dev *p = NULL;
+ u16 val;
+
+ for_each_pci_dev(p) {
+ int r;
+
+ for (r = 0; r < 6; r++) {
+ if (pci_resource_start(p, r) == 0x1f0)
+ *primary = 1;
+ if (pci_resource_start(p, r) == 0x170)
+ *secondary = 1;
+ }
+
+ /* Cyrix CS55{1,2}0 pre SFF MWDMA ATA on the bridge */
+ if (p->vendor == PCI_VENDOR_ID_CYRIX &&
+ (p->device == PCI_DEVICE_ID_CYRIX_5510 ||
+ ...Hello. Would have been probably enough to test only BAR0/2, don't you think? MBR, Sergei --
I assume you're referring to the legacy ioports fixup in drivers/pci/probe.c:pci_setup_device(). Yes, there's no need to go all the way to BAR5 since those are guaranteed unused in compatibility mode, so actually the loop should go till 4. Bart, can you please change that when applying? -- Regards/Gruß, Boris --
And to the fact that the value 0x1f0 should only be ever seen in BAR0 and 0x170 in BAR2 even if they would have been read off the chips (some chips have these values reading back even in legacy mode, and even could malfunction if other values are written there), not fixed up there, and certainly not in BAR1 or BAR3, so it's quite pointless to look in these BARs too. WBR, Sergei --
So the comment in there saying that in some cases BAR0-3 could contain junk is a
bogus? In other words, can we assume that one will always read 0x1f0 from BAR0
and 0x170 from BAR2 in compatibility mode. If so, the check is even simpler:
if (pci_resource_start(p, 0) == 0x1f0)
*primary = 1;
if (pci_resource_start(p, 2) == 0x170)
*secondary = 1;
--
Regards/Gruss,
Boris.
--
In theory, but for the sake of about 30 bytes of code in an obscure 'last resort work everywhere' driver it seemed a bit pointless. Alan --
Don't know. The PCI IDE spec. said that those should be 0 in compatibility
mode but I know that some controllers require the standard values of 0x1[f7]0
to be in BAR0/2 in compatibility mode. If my memory serves, NatSemi PC8741x
Certainly not. We can only rely on the workaround putting 0x1[f7]0 in the
Yes, I think that should be enough...
WBR, Sergei
--
On Thu, Aug 07, 2008 at 12:04:19AM +0400, Sergei Shtylyov wrote:
Here's v5:
---
From: Borislav Petkov <petkovbb@gmail.com>
Date: Sun, 3 Aug 2008 18:46:35 +0200
Subject: [PATCH] ide-generic: handle probing of legacy io-ports v5
Avoid probing the io-ports in case an IDE PCI controller is present and it
uses the legacy iobases. If we still want to enforce the probing, we do
ide_generic.probe_mask=0x3f
on the kernel command line. The iobase checking code is
adapted from drivers/ata/pata_legacy.c after converting hex
pci ids into their corresponding macros in <linux/pci_ids.h>.
Also, check only BAR0/2 resources since those are guaranteed
by the workaround in drivers/pci/probe.c:pci_setup_device().
CC: Sergei Shtylyov <sshtylyov@ru.mvista.com>
Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
drivers/ide/ide-generic.c | 56 +++++++++++++++++++++++++++++++++++++++++---
1 files changed, 52 insertions(+), 4 deletions(-)
diff --git a/drivers/ide/ide-generic.c b/drivers/ide/ide-generic.c
index 8fe8b5b..608f353 100644
--- a/drivers/ide/ide-generic.c
+++ b/drivers/ide/ide-generic.c
@@ -19,6 +19,7 @@
#include <linux/init.h>
#include <linux/module.h>
#include <linux/ide.h>
+#include <linux/pci_ids.h>
/* FIXME: convert m32r to use ide_platform host driver */
#ifdef CONFIG_M32R
@@ -27,7 +28,7 @@
#define DRV_NAME "ide_generic"
-static int probe_mask = 0x03;
+static int probe_mask;
module_param(probe_mask, int, 0);
MODULE_PARM_DESC(probe_mask, "probe mask for legacy ISA IDE ports");
@@ -100,19 +101,66 @@ static const u16 legacy_bases[] = { 0x1f0, 0x170, 0x1e8, 0x168, 0x1e0, 0x160 };
static const int legacy_irqs[] = { 14, 15, 11, 10, 8, 12 };
#endif
+static void ide_generic_check_pci_legacy_iobases(int *primary, int *secondary)
+{
+ struct pci_dev *p = NULL;
+ u16 val;
+
+ for_each_pci_dev(p) {
+
+ if (pci_resource_start(p, 0) == 0x1f0)
+ *primary = 1;
+ if (pci_resource_start(p, 2) == 0x170)
+ *secondary = 1;
+
+ /* ...On Thursday 07 August 2008, Borislav Petkov wrote: applied, thanks --
Hello. Acked-by: Sergei Shtylyov <sshtylyov@ru.mvista.com> MBR, Sergei --
From: Borislav Petkov <petkovbb@gmail.com>
Date: Sun, 3 Aug 2008 09:28:53 +0200
Subject: [PATCH 2/2] ide-generic: handle probing of legacy io-ports
Avoid probing the io-ports in case an IDE PCI controller is present and it uses
the legacy iobases. If we still want to enforce the probing, we do
ide_generic.probe_mask=0x3f
on the kernel command line.
Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
drivers/ide/ide-generic.c | 23 ++++++++++++++++++-----
1 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/drivers/ide/ide-generic.c b/drivers/ide/ide-generic.c
index 8fe8b5b..7d79616 100644
--- a/drivers/ide/ide-generic.c
+++ b/drivers/ide/ide-generic.c
@@ -27,7 +27,7 @@
#define DRV_NAME "ide_generic"
-static int probe_mask = 0x03;
+static int probe_mask = 0x00;
module_param(probe_mask, int, 0);
MODULE_PARM_DESC(probe_mask, "probe mask for legacy ISA IDE ports");
@@ -105,18 +105,31 @@ static int __init ide_generic_init(void)
hw_regs_t hw[MAX_HWIFS], *hws[MAX_HWIFS];
struct ide_host *host;
unsigned long io_addr;
- int i, rc;
+ int i, rc, dummy, primary = 0, secondary = 0;
#ifdef CONFIG_MIPS
if (!ide_probe_legacy())
return -ENODEV;
#endif
- printk(KERN_INFO DRV_NAME ": please use \"probe_mask=0x3f\" module "
- "parameter for probing all legacy ISA IDE ports\n");
+ ata_legacy_check_iobases(&primary, &secondary, &dummy);
+
+ if (primary) {
+ if (probe_mask) {
+ printk(KERN_WARNING "%s: enforcing probing of io ports "
+ "upon user request.\n", DRV_NAME);
+ primary = 0;
+ secondary = 0;
+ } else
+ printk(KERN_INFO DRV_NAME ": please use "
+ \"probe_mask=0x3f\" module parameter for probing"
+ "all legacy ISA IDE ports\n");
+
+ } else
+ probe_mask = 0x3;
memset(hws, 0, sizeof(hw_regs_t *) * MAX_HWIFS);
- for (i = 0; i < ARRAY_SIZE(legacy_bases); i++) {
+ for (i = primary + secondary; i < ARRAY_SIZE(legacy_bases); i++) {
io_addr = legacy_bases[i];
hws[i] = ...Thanks for reworking the patch, looks much better now.
Help message is no longer printed for !primary
No need for primary/secondary checking here now as everything
is controlled by probe_mask.
Thus we can check for probe_mask first in the previous chunk
and it can be rewritten/simplified to something like:
ata_legacy_check_iobases(&primary, &secondary, &dummy);
if (probe_mask == 0) {
printk(KERN_INFO DRV_NAME ": please use "
\"probe_mask=0x3f\" module parameter for probing"
"all legacy ISA IDE ports\n");
if (primary == 0)
probe_mask |= 1;
if (secondary == 0)
probe_mask |= 2;
}
--
How about moving that info to Documentation/ide/ide.txt instead? Do we want to
issue that on every boot, seems like a too unimportant message to be in the
True, this version is more readable. Reworked one coming up :) ...
--
Regards/Gruss,
Boris.
--
crap, forget what i said here ^ :(.
--
Regards/Gruss,
Boris.
--
On Sunday 03 August 2008, Borislav Petkov wrote: Some time ago we've changed the default from probing all ports to probe only safe ones (we had to do it because probing all ports may break other ISA devices) so there may be very unlikely cases when this bugfix caused regression and we would like people to be able to deduce the workaround from just reading kernel messages. --
On Sun, Aug 03, 2008 at 04:11:10PM +0200, Bartlomiej Zolnierkiewicz wrote:
Issues addressed. Here's v2:
---
From: Borislav Petkov <petkovbb@gmail.com>
Date: Sun, 3 Aug 2008 09:28:53 +0200
Subject: [PATCH] ide-generic: handle probing of legacy io-ports v2
Avoid probing the io-ports in case an IDE PCI controller is present and it uses
the legacy iobases. If we still want to enforce the probing, we do
ide_generic.probe_mask=0x3f
on the kernel command line.
Signed-off-by: Borislav Petkov <petkovbb@gmail.com>
---
drivers/ide/ide-generic.c | 21 +++++++++++++++++----
1 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/drivers/ide/ide-generic.c b/drivers/ide/ide-generic.c
index 8fe8b5b..1beb51b 100644
--- a/drivers/ide/ide-generic.c
+++ b/drivers/ide/ide-generic.c
@@ -27,7 +27,7 @@
#define DRV_NAME "ide_generic"
-static int probe_mask = 0x03;
+static int probe_mask;
module_param(probe_mask, int, 0);
MODULE_PARM_DESC(probe_mask, "probe mask for legacy ISA IDE ports");
@@ -105,14 +105,27 @@ static int __init ide_generic_init(void)
hw_regs_t hw[MAX_HWIFS], *hws[MAX_HWIFS];
struct ide_host *host;
unsigned long io_addr;
- int i, rc;
+ int i, rc, dummy, primary = 0, secondary = 0;
#ifdef CONFIG_MIPS
if (!ide_probe_legacy())
return -ENODEV;
#endif
- printk(KERN_INFO DRV_NAME ": please use \"probe_mask=0x3f\" module "
- "parameter for probing all legacy ISA IDE ports\n");
+ ata_legacy_check_iobases(&primary, &secondary, &dummy);
+
+ if (!probe_mask) {
+ printk(KERN_INFO DRV_NAME ": please use \"probe_mask=0x3f\" "
+ "module parameter for probing all legacy ISA IDE ports\n");
+
+ if (primary == 0)
+ probe_mask |= 0x1;
+
+ if (secondary == 0)
+ probe_mask |= 0x2;
+ } else {
+ printk(KERN_WARNING "%s: enforcing probing of io ports upon "
+ "user request.\n", DRV_NAME);
+ }
memset(hws, 0, sizeof(hw_regs_t *) * MAX_HWIFS);
--
1.5.5.4
--
Regards/Gruss,
Boris.
--
