login
Search
My intel8x0 card stops working due to a regression; bisection and information below. May have relationship to this thread http://groups.google.com/group/linux.kernel/browse_thread/thread/d5857287a36e71af/d7ae... http://avuton.googlepages.com/intel8x0-config.gz http://avuton.googlepages.com/intel8x0-cpuinfo http://avuton.googlepages.com/intel8x0-dmesg http://avuton.googlepages.com/intel8x0-ioports http://avuton.googlepages.com/intel8x0-lspci-vvv http://avuton.googlepages.com/intel8x0-modules http://avuton.googlepages.com/intel8x0-ver-linux http://avuton.googlepages.com/intel8x0-version commit 53052feb6ddd05cb2b5c6e89fb489bf83bbb6803 Author: Bjorn Helgaas <bjorn.helgaas@hp.com> Date: Mon Apr 28 16:34:15 2008 -0600 PNP: remove pnp_mem_flags() as an lvalue A future change will change pnp_mem_flags() from a "#define that simplifies to an lvalue" to "an inline function that returns the flags value." Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com> Acked-By: Rene Herman <rene.herman@gmail.com> Signed-off-by: Len Brown <len.brown@intel.com> -- avuton -- "I've got a fever. And the only prescription is more cowbell." -- Christopher Walken --
I'm probably just really blind but I don't see how that specific commit may have made any difference. It _is_ in the exact spot which would fix that overlap problem of yours but this should be an identity change as far as fuctionality goes. Are you _really_ sure it's this one? (Is this racing with anything?) Rene. --
Corrected: I will confirm in the next 24 hours that the previous revision works. Thanks, -- avuton -- "I've got a fever. And the only prescription is more cowbell." -- Christopher Walken --
Thanks. Important lines: [ 0.000000] Linux version 2.6.26-rc4 (sbh@rocket) (gcc version 4.2.4 (Gentoo 4.2.4 p1.0)) #4 SMP PREEMPT Sun Jun 1 17:41:35 PDT 2008 <snip> [ 0.205750] Linux Plug and Play Support v0.97 (c) Adam Belay [ 0.205891] pnp: PnP ACPI init [ 0.205945] ACPI: bus type pnp registered [ 0.206968] pnp 00:07: mem resource (0xfebfa000-0xfebfac00) overlaps 0000:00:1b.0 BAR 0 (0xfebf8000-0xfebfbfff), disabling [ 0.208685] pnp: PnP ACPI: found 13 devices [ 0.208737] ACPI: ACPI bus type pnp unregistered <snip> [ 0.231865] system 00:07: ioport range 0x4d0-0x4d1 has been reserved [ 0.231923] system 00:07: ioport range 0x800-0x87f has been reserved [ 0.231990] system 00:07: ioport range 0x480-0x4bf has been reserved [ 0.232045] system 00:07: iomem range 0xffafe000-0xffb0cbff could not be reserved [ 0.232114] system 00:07: iomem range 0xffb00000-0xffbfffff could not be reserved [ 0.232182] system 00:07: iomem range 0xfed1c000-0xfed1ffff has been reserved [ 0.232188] system 00:07: iomem range 0xfed20000-0xfed3ffff has been reserved [ 0.232188] system 00:07: iomem range 0xfed45000-0xfed89fff has been reserved [ 0.232188] system 00:07: iomem range 0xfff00000-0xfffffffe could not be reserved [ 0.232188] system 00:07: iomem range 0xfebfe000-0xfebfec00 has been reserved [ 0.232188] system 00:07: iomem range 0xfebfa000-0xfebfac00 has been reserved (*) <snip> [ 7.432374] ACPI: PCI Interrupt 0000:00:1b.0[A] -> GSI 22 (level, low) -> IRQ 22 [ 7.432374] PCI: Unable to reserve mem region #1:4000@febf8000 for device 0000:00:1b.0 [ 7.432374] ACPI: PCI interrupt for device 0000:00:1b.0 disabled [ 7.432374] HDA Intel: probe of 0000:00:1b.0 failed with error -16 So it's first saying it's disabling the region, then grabbing it at (*) anyway. Rene. --
Thank you very much for doing all the work to bisect this and write
up such a complete report.
Can you try the patch below, please?
PNP: mark resources that conflict with PCI devices "disabled"
Both the PNP/PCI conflict detection quirk and the PNP system
driver must use the same mechanism to mark resources as disabled.
I think it's best to keep the resource and to keep the type bit
(IORESOURCE_MEM, etc), so that we match the list from firmware
as closely as possible.
Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
Index: work11/drivers/pnp/quirks.c
===================================================================
--- work11.orig/drivers/pnp/quirks.c 2008-06-02 14:59:03.000000000 -0600
+++ work11/drivers/pnp/quirks.c 2008-06-02 15:42:35.000000000 -0600
@@ -286,7 +286,7 @@ static void quirk_system_pci_resources(s
pci_name(pdev), i,
(unsigned long long) pci_start,
(unsigned long long) pci_end);
- res->flags = 0;
+ res->flags |= IORESOURCE_DISABLED;
}
}
}
Index: work11/drivers/pnp/system.c
===================================================================
--- work11.orig/drivers/pnp/system.c 2008-06-02 14:58:56.000000000 -0600
+++ work11/drivers/pnp/system.c 2008-06-02 15:44:36.000000000 -0600
@@ -81,7 +81,7 @@ static void reserve_resources_of_dev(str
}
for (i = 0; (res = pnp_get_resource(dev, IORESOURCE_MEM, i)); i++) {
- if (res->flags & IORESOURCE_UNSET)
+ if (res->flags & IORESOURCE_DISABLED)
continue;
reserve_range(dev, res->start, res->end, 0);
--
Sweet! That works, thanks for the quick fix. Feel free to add me as Tested-by:. -- avuton -- "I've got a fever. And the only prescription is more cowbell." -- Christopher Walken --
The patch below fixes this regression from 2.6.25 and should go in 2.6.26. PNP: mark resources that conflict with PCI devices "disabled" Both the PNP/PCI conflict detection quirk and the PNP system driver must use the same mechanism to mark resources as disabled. I think it's best to keep the resource and to keep the type bit (IORESOURCE_MEM, etc), so that we match the list from firmware as closely as possible. Fixes this regression from 2.6.25: http://lkml.org/lkml/2008/6/1/82 Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com> Tested-by: Avuton Olrich <avuton@gmail.com> Index: work11/drivers/pnp/quirks.c =================================================================== --- work11.orig/drivers/pnp/quirks.c 2008-06-02 14:59:03.000000000 -0600 +++ work11/drivers/pnp/quirks.c 2008-06-02 15:42:35.000000000 -0600 @@ -286,7 +286,7 @@ static void quirk_system_pci_resources(s pci_name(pdev), i, (unsigned long long) pci_start, (unsigned long long) pci_end); - res->flags = 0; + res->flags |= IORESOURCE_DISABLED; } } } Index: work11/drivers/pnp/system.c =================================================================== --- work11.orig/drivers/pnp/system.c 2008-06-02 14:58:56.000000000 -0600 +++ work11/drivers/pnp/system.c 2008-06-02 15:44:36.000000000 -0600 @@ -81,7 +81,7 @@ static void reserve_resources_of_dev(str } for (i = 0; (res = pnp_get_resource(dev, IORESOURCE_MEM, i)); i++) { - if (res->flags & IORESOURCE_UNSET) + if (res->flags & IORESOURCE_DISABLED) continue; reserve_range(dev, res->start, res->end, 0); --
On Mon, 2 Jun 2008 16:42:49 -0600
This broke
pnp-replace-pnp_resource_table-with-dynamically-allocated-resources.patch:
***************
*** 80,91 ****
reserve_range(dev, res->start, res->end, 1);
}
- for (i = 0; (res = pnp_get_resource(dev, IORESOURCE_MEM, i)); i++) {
- if (res->flags & IORESOURCE_UNSET)
- continue;
-
reserve_range(dev, res->start, res->end, 0);
- }
}
static int system_pnp_probe(struct pnp_dev *dev,
--- 78,85 ----
reserve_range(dev, res->start, res->end, 1);
}
+ for (i = 0; (res = pnp_get_resource(dev, IORESOURCE_MEM, i)); i++)
reserve_range(dev, res->start, res->end, 0);
}
static int system_pnp_probe(struct pnp_dev *dev,
Which I fixed thusly:
static void reserve_resources_of_dev(struct pnp_dev *dev)
{
struct resource *res;
int i;
for (i = 0; (res = pnp_get_resource(dev, IORESOURCE_IO, i)); i++) {
if (res->start == 0)
continue; /* disabled */
if (res->start < 0x100)
/*
* Below 0x100 is only standard PC hardware
* (pics, kbd, timer, dma, ...)
* We should not get resource conflicts there,
* and the kernel reserves these anyway
* (see arch/i386/kernel/setup.c).
* So, do nothing
*/
continue;
if (res->end < res->start)
continue; /* invalid */
reserve_range(dev, res->start, res->end, 1);
}
for (i = 0; (res = pnp_get_resource(dev, IORESOURCE_MEM, i)); i++)
reserve_range(dev, res->start, res->end, 0);
}
Is it still correct?
Thanks.
--
I expect the patch was meant solely for 2.6.26... Bjorn, before you repost the option series due to this, wait a minute. Am now looking at/testing 14/15 and see some stuff that needs changing as well. Rene --
On Tue, 03 Jun 2008 01:58:42 +0200 Sure. When this patch (pnp-mark-resources-that-conflict-with-pci-devices-disabled.patch) is applied to 2.6.26, the for-2.6.27 pnp-replace-pnp_resource_table-with-dynamically-allocated-resources.patch "the option series" == "PNP: convert resource options to unified dynamic list, v1". I haven't got onto that yet. Strategic lagginess is working out nicely. --
Err, yes, got them mixed up. There are just so many of them -- after .27 My bloody ISA test machine decided to up and die on me during a BIOS flash. I'm sitting right smack in the middle of huge heap of old ISA gunk hardware, cases and motherboards here -- and now I have to try and dig out my bed ... :-( Rene. --
Well, or not if you're in a hurry. Just see it's 02:00+ here again so
I'm off. But at least this bit from 14/15 isn't right:
===
@ -176,33 +184,10 @@ static void quirk_ad1815_mpu_resources(s
if (!irq || irq->next)
return;
- res = dev->dependent;
- if (!res)
- return;
-
- while (1) {
- struct pnp_irq *copy;
-
- copy = pnp_alloc(sizeof *copy);
- if (!copy)
- break;
-
- bitmap_copy(copy->map.bits, irq->map.bits, PNP_IRQ_NR);
- copy->flags = irq->flags;
-
- copy->next = res->irq; /* Yes, this is NULL */
- res->irq = copy;
-
- if (!res->next)
- break;
- res = res->next;
- }
- kfree(irq);
+ irq->flags |= IORESOURCE_IRQ_OPTIONAL;
+ dev_info(&dev->dev, "made independent IRQ optional\n");
res->next = quirk_isapnp_mpu_options(dev);
-
- res = dev->independent;
- res->irq = NULL;
}
===
The deleted while loop traversedf the dependent optiosn so that at the
end res->next= added to the end of teh dependent chain. Now this adds to
the independent optiion.
Fortunately fix is simple; just delete the res->next = line completely.
This previously distributed the independent IRQ over the dependents so I
_could_ have an IRQ-less option by adding IRQ-less dependents but with
an OPTIONAL flag this is no longer needed.
Specifically, ad1815 MPU401 starts out as:
rene@ax6bc:~$ cat /sys/devices/pnp1/01\:01/01\:01.01/options
irq 5,7,2/9,11,12 High-Edge
Dependent: 00 - Priority preferred
port 0x330-0x330, align 0x0, size 0x2, 10-bit address decoding
Dependent: 01 - Priority acceptable
port 0x300-0x300, align 0x0, size 0x2, 10-bit address decoding
Dependent: 02 - Priority functional
port 0x100-0x3fe, align 0x1, size 0x2, 10-bit address decoding
and with your current code ends up as:
rene@ax6bc:~$ cat /sys/devices/pnp1/01\:01/01\:01.01/options
irq 5,7,2/9,11,12 High-Edge
Dependent: 00 - Priority preferred
port 0x330-0x330, align 0x0, size 0x2, 10-bit address decoding
Dependent: 01 - Priority acceptable
port ...Not quite. We need this instead:
for (i = 0; (res = pnp_get_resource(dev, IORESOURCE_MEM, i)); i++) {
if (res->flags & IORESOURCE_DISABLED)
continue;
reserve_range(dev, res->start, res->end, 0);
}
You can either just edit it by hand, or replace your
pnp-replace-pnp_resource_table-with-dynamically-allocated-resources.patch
with the attached. This hunk should be the only change compared to what
you currently have in mmotm.
Bjorn
PNP: replace pnp_resource_table with dynamically allocated resources
PNP used to have a fixed-size pnp_resource_table for tracking the
resources used by a device. This table often overflowed, so we've
had to increase the table size, which wastes memory because most
devices have very few resources.
This patch replaces the table with a linked list of resources where
the entries are allocated on demand.
This removes messages like these:
pnpacpi: exceeded the max number of IO resources
00:01: too many I/O port resources
References:
http://bugzilla.kernel.org/show_bug.cgi?id=9535
http://bugzilla.kernel.org/show_bug.cgi?id=9740
http://lkml.org/lkml/2007/11/30/110
This patch also changes the way PNP uses the IORESOURCE_UNSET,
IORESOURCE_AUTO, and IORESOURCE_DISABLED flags.
Prior to this patch, the pnp_resource_table entries used the flags
like this:
IORESOURCE_UNSET
This table entry is unused and available for use. When this flag
is set, we shouldn't look at anything else in the resource structure.
This flag is set when a resource table entry is initialized.
IORESOURCE_AUTO
This resource was assigned automatically by pnp_assign_{io,mem,etc}().
This flag is set when a resource table entry is initialized and
cleared whenever we discover a resource setting by reading an ISAPNP
config register, parsing a PNPBIOS resource data stream, parsing an
ACPI _CRS list, or interpreting a sysfs "set" command.
Resources ...but this change is responsible for a slew of console messages during boot like this on my Tiger4: system 00:01: iomem range 0x0-0x0 has been reserved system 00:01: iomem range 0x0-0x0 has been reserved system 00:01: iomem range 0x0-0x0 has been reserved system 00:01: iomem range 0x0-0x0 has been reserved system 00:01: iomem range 0x0-0x0 has been reserved system 00:01: iomem range 0x0-0x0 has been reserved system 00:01: iomem range 0x0-0x0 has been reserved system 00:01: iomem range 0x0-0x0 has been reserved system 00:01: iomem range 0x0-0x0 has been reserved ... continues for total of 46 lines -Tony --
I just sent Linus the patch to fix this (below).
In Linus' current tree, PNP_MAX_MEM is 24, so I would expect that
you would see 24 iomem messages (either successful or unsuccessful
reservations) for each system device (PNP0C01 or PNP0C02).
Hmm... the other report (on LKML) was for "iomem range 0x0-0x0
could not be reserved", while your messages were for successful
reservations. I don't know why they're different. But I think
this patch should fix both.
PNP: skip UNSET MEM resources as well as DISABLED ones
We don't need to reserve "unset" resources. Trying to reserve
them results in messages like this, which are ugly but harmless:
system 00:08: iomem range 0x0-0x0 could not be reserved
Future PNP patches will remove use of IORESOURCE_UNSET, but
we still need it for now.
Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
Index: work11/drivers/pnp/system.c
===================================================================
--- work11.orig/drivers/pnp/system.c 2008-06-05 09:46:33.000000000 -0600
+++ work11/drivers/pnp/system.c 2008-06-05 09:48:09.000000000 -0600
@@ -81,7 +81,8 @@ static void reserve_resources_of_dev(str
}
for (i = 0; (res = pnp_get_resource(dev, IORESOURCE_MEM, i)); i++) {
- if (res->flags & IORESOURCE_DISABLED)
+ if (res->flags & IORESOURCE_UNSET ||
+ res->flags & IORESOURCE_DISABLED)
continue;
reserve_range(dev, res->start, res->end, 0);
--
