Patchwork [45/86] pata_legacy: do not probe extra ports automatically if PCI is not present

login
register
mail settings
Submitter Bartlomiej Zolnierkiewicz
Date Nov. 25, 2009, 5:07 p.m.
Message ID <20091125170740.5446.67217.sendpatchset@localhost>
Download mbox | patch
Permalink /patch/39339/
State Not Applicable
Delegated to: David Miller
Headers show

Comments

Bartlomiej Zolnierkiewicz - Nov. 25, 2009, 5:07 p.m.
From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Subject: [PATCH] pata_legacy: do not probe extra ports automatically if PCI is not present

It can be problematic if there are other ISA/VLB devices using
those ports.

This is a forward port of bugfix from IDE ide-generic host driver.

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/ata/pata_legacy.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alan Cox - Nov. 25, 2009, 5:32 p.m.
On Wed, 25 Nov 2009 18:07:40 +0100
Bartlomiej Zolnierkiewicz <bzolnier@gmail.com> wrote:

> From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> Subject: [PATCH] pata_legacy: do not probe extra ports automatically if PCI is not present
> 
> It can be problematic if there are other ISA/VLB devices using
> those ports.

NAK. You made a rather peculiar set of changes in the IDE layer but the
PCI handling in pata_legacy includes knowledge of the various half-pci
devices and on a non PCI system 0x1E8/0x168 are reserved for extra IDE.

We've had no complaints at all, ever about this.

> This is a forward port of bugfix from IDE ide-generic host driver.

Of a bug.

Alan
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bartlomiej Zolnierkiewicz - Nov. 25, 2009, 5:45 p.m.
On Wednesday 25 November 2009 06:32:15 pm Alan Cox wrote:
> On Wed, 25 Nov 2009 18:07:40 +0100
> Bartlomiej Zolnierkiewicz <bzolnier@gmail.com> wrote:
> 
> > From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> > Subject: [PATCH] pata_legacy: do not probe extra ports automatically if PCI is not present
> > 
> > It can be problematic if there are other ISA/VLB devices using
> > those ports.
> 
> NAK. You made a rather peculiar set of changes in the IDE layer but the

Please give commit numbers and describe such 'peculiar set of changes in
the IDE layer' in more details or skip such baseless, unjust and pointless
comments from your mails.

> PCI handling in pata_legacy includes knowledge of the various half-pci
> devices and on a non PCI system 0x1E8/0x168 are reserved for extra IDE.
> 
> We've had no complaints at all, ever about this.

OTOH We had. :)

--
Bartlomiej Zolnierkiewicz
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alan Cox - Nov. 25, 2009, 6:12 p.m.
> > NAK. You made a rather peculiar set of changes in the IDE layer but the
> 
> Please give commit numbers and describe such 'peculiar set of changes in
> the IDE layer' in more details or skip such baseless, unjust and pointless

Disabling what can be a safe probe

> comments from your mails.

The old IDE code didn't do the full PCI check that libata did at the time
there were public complaints. Libata legacy also doesn't fall for the bug
where legacy loads and blocks PCI adapters as it has a smart check in it,
including for weirdness like the CS55x0 and the MPIIX.

So I'm firmly of the opinion that
- We should leave it as is
- If it causes a problem we should look at the exact case involved.
  Possibly it would need a CONFIG_X86 but thats sort of implied as
  embedded use pata_platform in preference.

The fact it just works by default is very very important.
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bartlomiej Zolnierkiewicz - Nov. 25, 2009, 6:20 p.m.
On Wednesday 25 November 2009 07:12:05 pm Alan Cox wrote:
> > > NAK. You made a rather peculiar set of changes in the IDE layer but the
> > 
> > Please give commit numbers and describe such 'peculiar set of changes in
> > the IDE layer' in more details or skip such baseless, unjust and pointless
> 
> Disabling what can be a safe probe

In many of your comments recent you're referring to IDE state from five or
more years ago.

> > comments from your mails.
> 
> The old IDE code didn't do the full PCI check that libata did at the time
> there were public complaints. Libata legacy also doesn't fall for the bug
> where legacy loads and blocks PCI adapters as it has a smart check in it,
> including for weirdness like the CS55x0 and the MPIIX.

This was fixed long time ago and the needed checks were added
(despite complete lack of code input from you).

> So I'm firmly of the opinion that
> - We should leave it as is

We may as well decide to do it before we have more data or more recent
bug-report at hand.  No big deal.

> - If it causes a problem we should look at the exact case involved.

While it was being fixed a year or two ago you were on cc: for original
patch (+ asked whether the same should be needed for pata_legacy) and
the bug-report from mikpe was on linux-ide ML.

IOW You were not interested into it the issue back when it was reported.

>   Possibly it would need a CONFIG_X86 but thats sort of implied as
>   embedded use pata_platform in preference.
> 
> The fact it just works by default is very very important.

Agreed on this one.

--
Bartlomiej Zolnierkiewicz
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

Index: b/drivers/ata/pata_legacy.c
===================================================================
--- a/drivers/ata/pata_legacy.c
+++ b/drivers/ata/pata_legacy.c
@@ -1241,7 +1241,7 @@  static __init int legacy_init(void)
 	if (secondary == 0 || all)
 		legacy_probe_add(0x170, 15, UNKNOWN, 0);
 
-	if (probe_all || !pci_present) {
+	if (probe_all) {
 		/* ISA/VLB extra ports */
 		legacy_probe_add(0x1E8, 11, UNKNOWN, 0);
 		legacy_probe_add(0x168, 10, UNKNOWN, 0);