* 53052feb6 (PNP: remove pnp_mem_flags() as an lvalue) breaks my ALSA intel8x0 sound card regression
@ 2008-06-01 14:42 Avuton Olrich
2008-06-01 16:20 ` Rene Herman
0 siblings, 1 reply; 15+ messages in thread
From: Avuton Olrich @ 2008-06-01 14:42 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Rene Herman, Len Brown, Linux Kernel Mailing List,
Rafael J. Wysocki
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/d7ae0a1490b7d142?lnk=st
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
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: 53052feb6 (PNP: remove pnp_mem_flags() as an lvalue) breaks my ALSA intel8x0 sound card regression 2008-06-01 14:42 53052feb6 (PNP: remove pnp_mem_flags() as an lvalue) breaks my ALSA intel8x0 sound card regression Avuton Olrich @ 2008-06-01 16:20 ` Rene Herman 2008-06-02 3:25 ` Avuton Olrich 0 siblings, 1 reply; 15+ messages in thread From: Rene Herman @ 2008-06-01 16:20 UTC (permalink / raw) To: Avuton Olrich Cc: Bjorn Helgaas, Rene Herman, Len Brown, Linux Kernel Mailing List, Rafael J. Wysocki On 01-06-08 16:42, Avuton Olrich wrote: > 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/d7ae0a1490b7d142?lnk=st > > http://avuton.googlepages.com/intel8x0-config.gz > http://avuton.googlepages.com/intel8x0-cpuinfo > http://avuton.googlepages.com/intel8x0-dmesg This dmesg seems to be 6-byte file consisting of "dmesg\n" ... > 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> > 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. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: 53052feb6 (PNP: remove pnp_mem_flags() as an lvalue) breaks my ALSA intel8x0 sound card regression 2008-06-01 16:20 ` Rene Herman @ 2008-06-02 3:25 ` Avuton Olrich 2008-06-02 19:06 ` Rene Herman 0 siblings, 1 reply; 15+ messages in thread From: Avuton Olrich @ 2008-06-02 3:25 UTC (permalink / raw) To: Rene Herman Cc: Bjorn Helgaas, Rene Herman, Len Brown, Linux Kernel Mailing List, Rafael J. Wysocki On Sun, Jun 1, 2008 at 9:20 AM, Rene Herman <rene.herman@keyaccess.nl> wrote: > On 01-06-08 16:42, Avuton Olrich wrote: > >> 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/d7ae0a1490b7d142?lnk=st >> >> http://avuton.googlepages.com/intel8x0-config.gz >> http://avuton.googlepages.com/intel8x0-cpuinfo >> http://avuton.googlepages.com/intel8x0-dmesg > > This dmesg seems to be 6-byte file consisting of "dmesg\n" ... Corrected: http://avuton.googlepages.com/dmesg.txt >> 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> >> > > 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?) 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: 53052feb6 (PNP: remove pnp_mem_flags() as an lvalue) breaks my ALSA intel8x0 sound card regression 2008-06-02 3:25 ` Avuton Olrich @ 2008-06-02 19:06 ` Rene Herman 2008-06-02 22:05 ` Bjorn Helgaas 0 siblings, 1 reply; 15+ messages in thread From: Rene Herman @ 2008-06-02 19:06 UTC (permalink / raw) To: Avuton Olrich Cc: Bjorn Helgaas, Rene Herman, Len Brown, Linux Kernel Mailing List, Rafael J. Wysocki On 02-06-08 05:25, Avuton Olrich wrote: > On Sun, Jun 1, 2008 at 9:20 AM, Rene Herman <rene.herman@keyaccess.nl> wrote: >> On 01-06-08 16:42, Avuton Olrich wrote: >> >>> 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/d7ae0a1490b7d142?lnk=st >>> >>> http://avuton.googlepages.com/intel8x0-config.gz >>> http://avuton.googlepages.com/intel8x0-cpuinfo >>> http://avuton.googlepages.com/intel8x0-dmesg >> This dmesg seems to be 6-byte file consisting of "dmesg\n" ... > > Corrected: > http://avuton.googlepages.com/dmesg.txt 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. I suppose/gather this worked at _some_ point, so over to author I guess. >>> http://avuton.googlepages.com/intel8x0-ioports 7>>> 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> >>> >> 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?) > > I will confirm in the next 24 hours that the previous revision works. Rene. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: 53052feb6 (PNP: remove pnp_mem_flags() as an lvalue) breaks my ALSA intel8x0 sound card regression 2008-06-02 19:06 ` Rene Herman @ 2008-06-02 22:05 ` Bjorn Helgaas 2008-06-02 22:23 ` Avuton Olrich 0 siblings, 1 reply; 15+ messages in thread From: Bjorn Helgaas @ 2008-06-02 22:05 UTC (permalink / raw) To: Rene Herman Cc: Avuton Olrich, Rene Herman, Len Brown, Linux Kernel Mailing List, Rafael J. Wysocki On Monday 02 June 2008 01:06:12 pm Rene Herman wrote: > On 02-06-08 05:25, Avuton Olrich wrote: > > > On Sun, Jun 1, 2008 at 9:20 AM, Rene Herman <rene.herman@keyaccess.nl> wrote: > >> On 01-06-08 16:42, Avuton Olrich wrote: > >> > >>> My intel8x0 card stops working due to a regression; bisection and > >>> information below. 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); ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: 53052feb6 (PNP: remove pnp_mem_flags() as an lvalue) breaks my ALSA intel8x0 sound card regression 2008-06-02 22:05 ` Bjorn Helgaas @ 2008-06-02 22:23 ` Avuton Olrich 2008-06-02 22:42 ` Bjorn Helgaas 0 siblings, 1 reply; 15+ messages in thread From: Avuton Olrich @ 2008-06-02 22:23 UTC (permalink / raw) To: Bjorn Helgaas Cc: Rene Herman, Rene Herman, Len Brown, Linux Kernel Mailing List, Rafael J. Wysocki On Mon, Jun 2, 2008 at 3:05 PM, Bjorn Helgaas <bjorn.helgaas@hp.com> wrote: > On Monday 02 June 2008 01:06:12 pm Rene Herman wrote: >> On 02-06-08 05:25, Avuton Olrich wrote: >> >> > On Sun, Jun 1, 2008 at 9:20 AM, Rene Herman <rene.herman@keyaccess.nl> wrote: >> >> On 01-06-08 16:42, Avuton Olrich wrote: >> >> >> >>> My intel8x0 card stops working due to a regression; bisection and >> >>> information below. > > 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: 53052feb6 (PNP: remove pnp_mem_flags() as an lvalue) breaks my ALSA intel8x0 sound card regression 2008-06-02 22:23 ` Avuton Olrich @ 2008-06-02 22:42 ` Bjorn Helgaas 2008-06-02 23:49 ` Andrew Morton 2008-06-04 23:38 ` Tony Luck 0 siblings, 2 replies; 15+ messages in thread From: Bjorn Helgaas @ 2008-06-02 22:42 UTC (permalink / raw) To: Andrew Morton, Linus Torvalds Cc: Avuton Olrich, Rene Herman, Rene Herman, Len Brown, Linux Kernel Mailing List, Rafael J. Wysocki On Monday 02 June 2008 04:23:28 pm Avuton Olrich wrote: > On Mon, Jun 2, 2008 at 3:05 PM, Bjorn Helgaas <bjorn.helgaas@hp.com> wrote: > > On Monday 02 June 2008 01:06:12 pm Rene Herman wrote: > >> On 02-06-08 05:25, Avuton Olrich wrote: > >> > >> > On Sun, Jun 1, 2008 at 9:20 AM, Rene Herman <rene.herman@keyaccess.nl> wrote: > >> >> On 01-06-08 16:42, Avuton Olrich wrote: > >> >> > >> >>> My intel8x0 card stops working due to a regression; bisection and > >> >>> information below. > > > > 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? > > ... > > Sweet! That works, thanks for the quick fix. Feel free to add me as Tested-by:. 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); ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: 53052feb6 (PNP: remove pnp_mem_flags() as an lvalue) breaks my ALSA intel8x0 sound card regression 2008-06-02 22:42 ` Bjorn Helgaas @ 2008-06-02 23:49 ` Andrew Morton 2008-06-02 23:58 ` Rene Herman 2008-06-03 18:40 ` Bjorn Helgaas 2008-06-04 23:38 ` Tony Luck 1 sibling, 2 replies; 15+ messages in thread From: Andrew Morton @ 2008-06-02 23:49 UTC (permalink / raw) To: Bjorn Helgaas Cc: torvalds, avuton, rene.herman, rene.herman, len.brown, linux-kernel, rjw On Mon, 2 Jun 2008 16:42:49 -0600 Bjorn Helgaas <bjorn.helgaas@hp.com> wrote: > 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); 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. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: 53052feb6 (PNP: remove pnp_mem_flags() as an lvalue) breaks my ALSA intel8x0 sound card regression 2008-06-02 23:49 ` Andrew Morton @ 2008-06-02 23:58 ` Rene Herman 2008-06-03 0:03 ` Andrew Morton 2008-06-03 0:15 ` Rene Herman 2008-06-03 18:40 ` Bjorn Helgaas 1 sibling, 2 replies; 15+ messages in thread From: Rene Herman @ 2008-06-02 23:58 UTC (permalink / raw) To: Andrew Morton Cc: Bjorn Helgaas, torvalds, avuton, rene.herman, len.brown, linux-kernel, rjw On 03-06-08 01:49, Andrew Morton wrote: > This broke > pnp-replace-pnp_resource_table-with-dynamically-allocated-resources.patch: 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: 53052feb6 (PNP: remove pnp_mem_flags() as an lvalue) breaks my ALSA intel8x0 sound card regression 2008-06-02 23:58 ` Rene Herman @ 2008-06-03 0:03 ` Andrew Morton 2008-06-03 0:31 ` Rene Herman 2008-06-03 0:15 ` Rene Herman 1 sibling, 1 reply; 15+ messages in thread From: Andrew Morton @ 2008-06-03 0:03 UTC (permalink / raw) To: Rene Herman Cc: bjorn.helgaas, torvalds, avuton, rene.herman, len.brown, linux-kernel, rjw On Tue, 03 Jun 2008 01:58:42 +0200 Rene Herman <rene.herman@keyaccess.nl> wrote: > On 03-06-08 01:49, Andrew Morton wrote: > > > This broke > > pnp-replace-pnp_resource_table-with-dynamically-allocated-resources.patch: > > I expect the patch was meant solely for 2.6.26... 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 broke, as I described here. > 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. "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. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: 53052feb6 (PNP: remove pnp_mem_flags() as an lvalue) breaks my ALSA intel8x0 sound card regression 2008-06-03 0:03 ` Andrew Morton @ 2008-06-03 0:31 ` Rene Herman 0 siblings, 0 replies; 15+ messages in thread From: Rene Herman @ 2008-06-03 0:31 UTC (permalink / raw) To: Andrew Morton Cc: bjorn.helgaas, torvalds, avuton, rene.herman, len.brown, linux-kernel, rjw On 03-06-08 02:03, Andrew Morton wrote: > On Tue, 03 Jun 2008 01:58:42 +0200 > Rene Herman <rene.herman@keyaccess.nl> wrote: > >> On 03-06-08 01:49, Andrew Morton wrote: >> >>> This broke >>> pnp-replace-pnp_resource_table-with-dynamically-allocated-resources.patch: >> I expect the patch was meant solely for 2.6.26... > > 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 > broke, as I described here. Err, yes, got them mixed up. There are just so many of them -- after .27 Bjorn will have basically rewritten all of PnP... >> 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. > > "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. 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. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: 53052feb6 (PNP: remove pnp_mem_flags() as an lvalue) breaks my ALSA intel8x0 sound card regression 2008-06-02 23:58 ` Rene Herman 2008-06-03 0:03 ` Andrew Morton @ 2008-06-03 0:15 ` Rene Herman 1 sibling, 0 replies; 15+ messages in thread From: Rene Herman @ 2008-06-03 0:15 UTC (permalink / raw) To: Bjorn Helgaas Cc: Andrew Morton, torvalds, avuton, rene.herman, len.brown, linux-kernel, rjw On 03-06-08 01:58, Rene Herman wrote: > 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. 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 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 Dependent: 03 - Priority functional port 0x330-0x330, align 0x0, size 0x2, 10-bit address decoding Dependent: 04 - Priority functional port 0x300-0x300, align 0x0, size 0x2, 10-bit address decoding Dependent: 05 - Priority functional port 0x100-0x3fe, align 0x1, size 0x2, 10-bit address decoding As you see, no need for 4, 5 and 6. So no need fior the cloning. This also means functi9ons can be folded back in but I'll do that later if you prefer. Unfortunately, the optional thing doesn't seem to work at all at the moment (this is post your series): ad1816a 01:01.01: pnp_assign_resources, try dependent set 0 ad1816a 01:01.01: couldn't assign irq 0 ad1816a 01:01.01: pnp_assign_resources failed (-16) ad1816a 01:01.01: pnp_assign_resources, try dependent set 1 ad1816a 01:01.01: couldn't assign irq 0 ad1816a 01:01.01: pnp_assign_resources failed (-16) ad1816a 01:01.01: pnp_assign_resources, try dependent set 2 ad1816a 01:01.01: couldn't assign irq 0 ad1816a 01:01.01: pnp_assign_resources failed (-16) ad1816a 01:01.01: pnp_assign_resources, try dependent set 3 ad1816a 01:01.01: couldn't assign irq 0 ad1816a 01:01.01: pnp_assign_resources failed (-16) ad1816a 01:01.01: pnp_assign_resources, try dependent set 4 ad1816a 01:01.01: couldn't assign irq 0 ad1816a 01:01.01: pnp_assign_resources failed (-16) ad1816a 01:01.01: pnp_assign_resources, try dependent set 5 ad1816a 01:01.01: couldn't assign irq 0 ad1816a 01:01.01: pnp_assign_resources failed (-16) ad1816a 01:01.01: unable to assign resources ad1816a: MPU401 PnP configure failure but I ran out of day again. Will look. Rene. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: 53052feb6 (PNP: remove pnp_mem_flags() as an lvalue) breaks my ALSA intel8x0 sound card regression 2008-06-02 23:49 ` Andrew Morton 2008-06-02 23:58 ` Rene Herman @ 2008-06-03 18:40 ` Bjorn Helgaas 1 sibling, 0 replies; 15+ messages in thread From: Bjorn Helgaas @ 2008-06-03 18:40 UTC (permalink / raw) To: Andrew Morton Cc: torvalds, avuton, rene.herman, rene.herman, len.brown, linux-kernel, rjw On Monday 02 June 2008 05:49:54 pm Andrew Morton wrote: > On Mon, 2 Jun 2008 16:42:49 -0600 > Bjorn Helgaas <bjorn.helgaas@hp.com> wrote: > > > PNP: mark resources that conflict with PCI devices "disabled" > > > > ... > > 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? 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 marked IORESOURCE_AUTO are reinitialized and marked as IORESOURCE_UNSET by pnp_clean_resource_table() in these cases: - before we attempt to assign resources automatically, - if we fail to assign resources automatically, - after disabling a device IORESOURCE_DISABLED Set by pnp_assign_{io,mem,etc}() when automatic assignment fails. Also set by PNPBIOS and PNPACPI for: - invalid IRQs or GSI registration failures - invalid DMA channels - I/O ports above 0x10000 - mem ranges with negative length After this patch, there is no pnp_resource_table, and the resource list entries use the flags like this: IORESOURCE_UNSET This flag is no longer used in PNP. Instead of keeping IORESOURCE_UNSET entries in the resource list, we remove entries from the list and free them. IORESOURCE_AUTO No change in meaning: it still means the resource was assigned automatically by pnp_assign_{port,mem,etc}(), but these functions now set the bit explicitly. We still "clean" a device's resource list in the same places, but rather than reinitializing IORESOURCE_AUTO entries, we just remove them from the list. Note that IORESOURCE_AUTO entries are always at the end of the list, so removing them doesn't reorder other list entries. This is because non-IORESOURCE_AUTO entries are added by the ISAPNP, PNPBIOS, or PNPACPI "get resources" methods and by the sysfs "set" command. In each of these cases, we completely free the resource list first. IORESOURCE_DISABLED In addition to the cases where we used to set this flag, ISAPNP now adds an IORESOURCE_DISABLED resource when it reads a configuration register with a "disabled" value. Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com> --- drivers/pnp/base.h | 18 --- drivers/pnp/core.c | 25 +++-- drivers/pnp/interface.c | 60 +++++------- drivers/pnp/isapnp/core.c | 12 -- drivers/pnp/manager.c | 218 +++++++++++----------------------------------- drivers/pnp/quirks.c | 3 drivers/pnp/resource.c | 86 +++--------------- drivers/pnp/support.c | 72 +++++---------- drivers/pnp/system.c | 8 - include/linux/pnp.h | 13 +- 10 files changed, 158 insertions(+), 357 deletions(-) Index: work10/drivers/pnp/base.h =================================================================== --- work10.orig/drivers/pnp/base.h 2008-06-03 12:20:22.000000000 -0600 +++ work10/drivers/pnp/base.h 2008-06-03 12:21:12.000000000 -0600 @@ -46,27 +46,15 @@ int pnp_check_dma(struct pnp_dev *dev, s char *pnp_resource_type_name(struct resource *res); void dbg_pnp_show_resources(struct pnp_dev *dev, char *desc); -void pnp_init_resource(struct resource *res); +void pnp_free_resources(struct pnp_dev *dev); int pnp_resource_type(struct resource *res); -struct pnp_resource *pnp_get_pnp_resource(struct pnp_dev *dev, - unsigned int type, unsigned int num); - -#define PNP_MAX_PORT 40 -#define PNP_MAX_MEM 24 -#define PNP_MAX_IRQ 2 -#define PNP_MAX_DMA 2 - struct pnp_resource { + struct list_head list; struct resource res; }; -struct pnp_resource_table { - struct pnp_resource port[PNP_MAX_PORT]; - struct pnp_resource mem[PNP_MAX_MEM]; - struct pnp_resource dma[PNP_MAX_DMA]; - struct pnp_resource irq[PNP_MAX_IRQ]; -}; +void pnp_free_resource(struct pnp_resource *pnp_res); struct pnp_resource *pnp_add_irq_resource(struct pnp_dev *dev, int irq, int flags); Index: work10/drivers/pnp/core.c =================================================================== --- work10.orig/drivers/pnp/core.c 2008-06-03 12:11:31.000000000 -0600 +++ work10/drivers/pnp/core.c 2008-06-03 12:21:12.000000000 -0600 @@ -99,6 +99,21 @@ static void pnp_free_ids(struct pnp_dev } } +void pnp_free_resource(struct pnp_resource *pnp_res) +{ + list_del(&pnp_res->list); + kfree(pnp_res); +} + +void pnp_free_resources(struct pnp_dev *dev) +{ + struct pnp_resource *pnp_res, *tmp; + + list_for_each_entry_safe(pnp_res, tmp, &dev->resources, list) { + pnp_free_resource(pnp_res); + } +} + static void pnp_release_device(struct device *dmdev) { struct pnp_dev *dev = to_pnp_dev(dmdev); @@ -106,7 +121,7 @@ static void pnp_release_device(struct de pnp_free_option(dev->independent); pnp_free_option(dev->dependent); pnp_free_ids(dev); - kfree(dev->res); + pnp_free_resources(dev); kfree(dev); } @@ -119,12 +134,7 @@ struct pnp_dev *pnp_alloc_dev(struct pnp if (!dev) return NULL; - dev->res = kzalloc(sizeof(struct pnp_resource_table), GFP_KERNEL); - if (!dev->res) { - kfree(dev); - return NULL; - } - + INIT_LIST_HEAD(&dev->resources); dev->protocol = protocol; dev->number = id; dev->dma_mask = DMA_24BIT_MASK; @@ -140,7 +150,6 @@ struct pnp_dev *pnp_alloc_dev(struct pnp dev_id = pnp_add_id(dev, pnpid); if (!dev_id) { - kfree(dev->res); kfree(dev); return NULL; } Index: work10/drivers/pnp/interface.c =================================================================== --- work10.orig/drivers/pnp/interface.c 2008-06-03 12:16:37.000000000 -0600 +++ work10/drivers/pnp/interface.c 2008-06-03 12:34:32.000000000 -0600 @@ -269,46 +269,38 @@ static ssize_t pnp_show_current_resource pnp_printf(buffer, "disabled\n"); for (i = 0; (res = pnp_get_resource(dev, IORESOURCE_IO, i)); i++) { - if (pnp_resource_valid(res)) { - pnp_printf(buffer, "io"); - if (res->flags & IORESOURCE_DISABLED) - pnp_printf(buffer, " disabled\n"); - else - pnp_printf(buffer, " 0x%llx-0x%llx\n", - (unsigned long long) res->start, - (unsigned long long) res->end); - } + pnp_printf(buffer, "io"); + if (res->flags & IORESOURCE_DISABLED) + pnp_printf(buffer, " disabled\n"); + else + pnp_printf(buffer, " 0x%llx-0x%llx\n", + (unsigned long long) res->start, + (unsigned long long) res->end); } for (i = 0; (res = pnp_get_resource(dev, IORESOURCE_MEM, i)); i++) { - if (pnp_resource_valid(res)) { - pnp_printf(buffer, "mem"); - if (res->flags & IORESOURCE_DISABLED) - pnp_printf(buffer, " disabled\n"); - else - pnp_printf(buffer, " 0x%llx-0x%llx\n", - (unsigned long long) res->start, - (unsigned long long) res->end); - } + pnp_printf(buffer, "mem"); + if (res->flags & IORESOURCE_DISABLED) + pnp_printf(buffer, " disabled\n"); + else + pnp_printf(buffer, " 0x%llx-0x%llx\n", + (unsigned long long) res->start, + (unsigned long long) res->end); } for (i = 0; (res = pnp_get_resource(dev, IORESOURCE_IRQ, i)); i++) { - if (pnp_resource_valid(res)) { - pnp_printf(buffer, "irq"); - if (res->flags & IORESOURCE_DISABLED) - pnp_printf(buffer, " disabled\n"); - else - pnp_printf(buffer, " %lld\n", - (unsigned long long) res->start); - } + pnp_printf(buffer, "irq"); + if (res->flags & IORESOURCE_DISABLED) + pnp_printf(buffer, " disabled\n"); + else + pnp_printf(buffer, " %lld\n", + (unsigned long long) res->start); } for (i = 0; (res = pnp_get_resource(dev, IORESOURCE_DMA, i)); i++) { - if (pnp_resource_valid(res)) { - pnp_printf(buffer, "dma"); - if (res->flags & IORESOURCE_DISABLED) - pnp_printf(buffer, " disabled\n"); - else - pnp_printf(buffer, " %lld\n", - (unsigned long long) res->start); - } + pnp_printf(buffer, "dma"); + if (res->flags & IORESOURCE_DISABLED) + pnp_printf(buffer, " disabled\n"); + else + pnp_printf(buffer, " %lld\n", + (unsigned long long) res->start); } ret = (buffer->curr - buf); kfree(buffer); Index: work10/drivers/pnp/isapnp/core.c =================================================================== --- work10.orig/drivers/pnp/isapnp/core.c 2008-06-03 12:16:37.000000000 -0600 +++ work10/drivers/pnp/isapnp/core.c 2008-06-03 12:21:12.000000000 -0600 @@ -973,8 +973,7 @@ static int isapnp_set_resources(struct p dev->active = 1; for (tmp = 0; tmp < ISAPNP_MAX_PORT; tmp++) { res = pnp_get_resource(dev, IORESOURCE_IO, tmp); - if (res && pnp_resource_valid(res) && - !(res->flags & IORESOURCE_DISABLED)) { + if (res && !(res->flags & IORESOURCE_DISABLED)) { dev_dbg(&dev->dev, " set io %d to %#llx\n", tmp, (unsigned long long) res->start); isapnp_write_word(ISAPNP_CFG_PORT + (tmp << 1), @@ -983,8 +982,7 @@ static int isapnp_set_resources(struct p } for (tmp = 0; tmp < ISAPNP_MAX_IRQ; tmp++) { res = pnp_get_resource(dev, IORESOURCE_IRQ, tmp); - if (res && pnp_resource_valid(res) && - !(res->flags & IORESOURCE_DISABLED)) { + if (res && !(res->flags & IORESOURCE_DISABLED)) { int irq = res->start; if (irq == 2) irq = 9; @@ -994,8 +992,7 @@ static int isapnp_set_resources(struct p } for (tmp = 0; tmp < ISAPNP_MAX_DMA; tmp++) { res = pnp_get_resource(dev, IORESOURCE_DMA, tmp); - if (res && pnp_resource_valid(res) && - !(res->flags & IORESOURCE_DISABLED)) { + if (res && !(res->flags & IORESOURCE_DISABLED)) { dev_dbg(&dev->dev, " set dma %d to %lld\n", tmp, (unsigned long long) res->start); isapnp_write_byte(ISAPNP_CFG_DMA + tmp, res->start); @@ -1003,8 +1000,7 @@ static int isapnp_set_resources(struct p } for (tmp = 0; tmp < ISAPNP_MAX_MEM; tmp++) { res = pnp_get_resource(dev, IORESOURCE_MEM, tmp); - if (res && pnp_resource_valid(res) && - !(res->flags & IORESOURCE_DISABLED)) { + if (res && !(res->flags & IORESOURCE_DISABLED)) { dev_dbg(&dev->dev, " set mem %d to %#llx\n", tmp, (unsigned long long) res->start); isapnp_write_word(ISAPNP_CFG_MEM + (tmp << 3), Index: work10/drivers/pnp/manager.c =================================================================== --- work10.orig/drivers/pnp/manager.c 2008-06-03 12:16:37.000000000 -0600 +++ work10/drivers/pnp/manager.c 2008-06-03 12:21:12.000000000 -0600 @@ -19,40 +19,30 @@ DEFINE_MUTEX(pnp_res_mutex); static int pnp_assign_port(struct pnp_dev *dev, struct pnp_port *rule, int idx) { - struct pnp_resource *pnp_res; - struct resource *res; + struct resource *res, local_res; - pnp_res = pnp_get_pnp_resource(dev, IORESOURCE_IO, idx); - if (!pnp_res) { - dev_err(&dev->dev, "too many I/O port resources\n"); - /* pretend we were successful so at least the manager won't try again */ - return 1; - } - - res = &pnp_res->res; - - /* check if this resource has been manually set, if so skip */ - if (!(res->flags & IORESOURCE_AUTO)) { + res = pnp_get_resource(dev, IORESOURCE_IO, idx); + if (res) { dev_dbg(&dev->dev, " io %d already set to %#llx-%#llx " "flags %#lx\n", idx, (unsigned long long) res->start, (unsigned long long) res->end, res->flags); return 1; } - /* set the initial values */ - res->flags |= rule->flags | IORESOURCE_IO; - res->flags &= ~IORESOURCE_UNSET; + res = &local_res; + res->flags = rule->flags | IORESOURCE_AUTO; + res->start = 0; + res->end = 0; if (!rule->size) { res->flags |= IORESOURCE_DISABLED; dev_dbg(&dev->dev, " io %d disabled\n", idx); - return 1; /* skip disabled resource requests */ + goto __add; } res->start = rule->min; res->end = res->start + rule->size - 1; - /* run through until pnp_check_port is happy */ while (!pnp_check_port(dev, res)) { res->start += rule->align; res->end = res->start + rule->size - 1; @@ -61,38 +51,29 @@ static int pnp_assign_port(struct pnp_de return 0; } } - dev_dbg(&dev->dev, " assign io %d %#llx-%#llx\n", idx, - (unsigned long long) res->start, (unsigned long long) res->end); + +__add: + pnp_add_io_resource(dev, res->start, res->end, res->flags); return 1; } static int pnp_assign_mem(struct pnp_dev *dev, struct pnp_mem *rule, int idx) { - struct pnp_resource *pnp_res; - struct resource *res; - - pnp_res = pnp_get_pnp_resource(dev, IORESOURCE_MEM, idx); - if (!pnp_res) { - dev_err(&dev->dev, "too many memory resources\n"); - /* pretend we were successful so at least the manager won't try again */ - return 1; - } - - res = &pnp_res->res; + struct resource *res, local_res; - /* check if this resource has been manually set, if so skip */ - if (!(res->flags & IORESOURCE_AUTO)) { + res = pnp_get_resource(dev, IORESOURCE_MEM, idx); + if (res) { dev_dbg(&dev->dev, " mem %d already set to %#llx-%#llx " "flags %#lx\n", idx, (unsigned long long) res->start, (unsigned long long) res->end, res->flags); return 1; } - /* set the initial values */ - res->flags |= rule->flags | IORESOURCE_MEM; - res->flags &= ~IORESOURCE_UNSET; + res = &local_res; + res->flags = rule->flags | IORESOURCE_AUTO; + res->start = 0; + res->end = 0; - /* convert pnp flags to standard Linux flags */ if (!(rule->flags & IORESOURCE_MEM_WRITEABLE)) res->flags |= IORESOURCE_READONLY; if (rule->flags & IORESOURCE_MEM_CACHEABLE) @@ -105,13 +86,12 @@ static int pnp_assign_mem(struct pnp_dev if (!rule->size) { res->flags |= IORESOURCE_DISABLED; dev_dbg(&dev->dev, " mem %d disabled\n", idx); - return 1; /* skip disabled resource requests */ + goto __add; } res->start = rule->min; res->end = res->start + rule->size - 1; - /* run through until pnp_check_mem is happy */ while (!pnp_check_mem(dev, res)) { res->start += rule->align; res->end = res->start + rule->size - 1; @@ -120,15 +100,15 @@ static int pnp_assign_mem(struct pnp_dev return 0; } } - dev_dbg(&dev->dev, " assign mem %d %#llx-%#llx\n", idx, - (unsigned long long) res->start, (unsigned long long) res->end); + +__add: + pnp_add_mem_resource(dev, res->start, res->end, res->flags); return 1; } static int pnp_assign_irq(struct pnp_dev *dev, struct pnp_irq *rule, int idx) { - struct pnp_resource *pnp_res; - struct resource *res; + struct resource *res, local_res; int i; /* IRQ priority: this table is good for i386 */ @@ -136,58 +116,48 @@ static int pnp_assign_irq(struct pnp_dev 5, 10, 11, 12, 9, 14, 15, 7, 3, 4, 13, 0, 1, 6, 8, 2 }; - pnp_res = pnp_get_pnp_resource(dev, IORESOURCE_IRQ, idx); - if (!pnp_res) { - dev_err(&dev->dev, "too many IRQ resources\n"); - /* pretend we were successful so at least the manager won't try again */ - return 1; - } - - res = &pnp_res->res; - - /* check if this resource has been manually set, if so skip */ - if (!(res->flags & IORESOURCE_AUTO)) { + res = pnp_get_resource(dev, IORESOURCE_IRQ, idx); + if (res) { dev_dbg(&dev->dev, " irq %d already set to %d flags %#lx\n", idx, (int) res->start, res->flags); return 1; } - /* set the initial values */ - res->flags |= rule->flags | IORESOURCE_IRQ; - res->flags &= ~IORESOURCE_UNSET; + res = &local_res; + res->flags = rule->flags | IORESOURCE_AUTO; + res->start = -1; + res->end = -1; if (bitmap_empty(rule->map, PNP_IRQ_NR)) { res->flags |= IORESOURCE_DISABLED; dev_dbg(&dev->dev, " irq %d disabled\n", idx); - return 1; /* skip disabled resource requests */ + goto __add; } /* TBD: need check for >16 IRQ */ res->start = find_next_bit(rule->map, PNP_IRQ_NR, 16); if (res->start < PNP_IRQ_NR) { res->end = res->start; - dev_dbg(&dev->dev, " assign irq %d %d\n", idx, - (int) res->start); - return 1; + goto __add; } for (i = 0; i < 16; i++) { if (test_bit(xtab[i], rule->map)) { res->start = res->end = xtab[i]; - if (pnp_check_irq(dev, res)) { - dev_dbg(&dev->dev, " assign irq %d %d\n", idx, - (int) res->start); - return 1; - } + if (pnp_check_irq(dev, res)) + goto __add; } } dev_dbg(&dev->dev, " couldn't assign irq %d\n", idx); return 0; + +__add: + pnp_add_irq_resource(dev, res->start, res->flags); + return 1; } static void pnp_assign_dma(struct pnp_dev *dev, struct pnp_dma *rule, int idx) { - struct pnp_resource *pnp_res; - struct resource *res; + struct resource *res, local_res; int i; /* DMA priority: this table is good for i386 */ @@ -195,127 +165,47 @@ static void pnp_assign_dma(struct pnp_de 1, 3, 5, 6, 7, 0, 2, 4 }; - pnp_res = pnp_get_pnp_resource(dev, IORESOURCE_DMA, idx); - if (!pnp_res) { - dev_err(&dev->dev, "too many DMA resources\n"); - return; - } - - res = &pnp_res->res; - - /* check if this resource has been manually set, if so skip */ - if (!(res->flags & IORESOURCE_AUTO)) { + res = pnp_get_resource(dev, IORESOURCE_DMA, idx); + if (res) { dev_dbg(&dev->dev, " dma %d already set to %d flags %#lx\n", idx, (int) res->start, res->flags); return; } - /* set the initial values */ - res->flags |= rule->flags | IORESOURCE_DMA; - res->flags &= ~IORESOURCE_UNSET; + res = &local_res; + res->flags = rule->flags | IORESOURCE_AUTO; + res->start = -1; + res->end = -1; for (i = 0; i < 8; i++) { if (rule->map & (1 << xtab[i])) { res->start = res->end = xtab[i]; - if (pnp_check_dma(dev, res)) { - dev_dbg(&dev->dev, " assign dma %d %d\n", idx, - (int) res->start); - return; - } + if (pnp_check_dma(dev, res)) + goto __add; } } #ifdef MAX_DMA_CHANNELS res->start = res->end = MAX_DMA_CHANNELS; #endif - res->flags |= IORESOURCE_UNSET | IORESOURCE_DISABLED; + res->flags |= IORESOURCE_DISABLED; dev_dbg(&dev->dev, " disable dma %d\n", idx); -} -void pnp_init_resource(struct resource *res) -{ - unsigned long type; - - type = res->flags & (IORESOURCE_IO | IORESOURCE_MEM | - IORESOURCE_IRQ | IORESOURCE_DMA); - - res->name = NULL; - res->flags = type | IORESOURCE_AUTO | IORESOURCE_UNSET; - if (type == IORESOURCE_IRQ || type == IORESOURCE_DMA) { - res->start = -1; - res->end = -1; - } else { - res->start = 0; - res->end = 0; - } +__add: + pnp_add_dma_resource(dev, res->start, res->flags); } -/** - * pnp_init_resources - Resets a resource table to default values. - * @table: pointer to the desired resource table - */ void pnp_init_resources(struct pnp_dev *dev) { - struct resource *res; - int idx; - - for (idx = 0; idx < PNP_MAX_IRQ; idx++) { - res = &dev->res->irq[idx].res; - res->flags = IORESOURCE_IRQ; - pnp_init_resource(res); - } - for (idx = 0; idx < PNP_MAX_DMA; idx++) { - res = &dev->res->dma[idx].res; - res->flags = IORESOURCE_DMA; - pnp_init_resource(res); - } - for (idx = 0; idx < PNP_MAX_PORT; idx++) { - res = &dev->res->port[idx].res; - res->flags = IORESOURCE_IO; - pnp_init_resource(res); - } - for (idx = 0; idx < PNP_MAX_MEM; idx++) { - res = &dev->res->mem[idx].res; - res->flags = IORESOURCE_MEM; - pnp_init_resource(res); - } + pnp_free_resources(dev); } -/** - * pnp_clean_resources - clears resources that were not manually set - * @res: the resources to clean - */ static void pnp_clean_resource_table(struct pnp_dev *dev) { - struct resource *res; - int idx; + struct pnp_resource *pnp_res, *tmp; - for (idx = 0; idx < PNP_MAX_IRQ; idx++) { - res = &dev->res->irq[idx].res; - if (res->flags & IORESOURCE_AUTO) { - res->flags = IORESOURCE_IRQ; - pnp_init_resource(res); - } - } - for (idx = 0; idx < PNP_MAX_DMA; idx++) { - res = &dev->res->dma[idx].res; - if (res->flags & IORESOURCE_AUTO) { - res->flags = IORESOURCE_DMA; - pnp_init_resource(res); - } - } - for (idx = 0; idx < PNP_MAX_PORT; idx++) { - res = &dev->res->port[idx].res; - if (res->flags & IORESOURCE_AUTO) { - res->flags = IORESOURCE_IO; - pnp_init_resource(res); - } - } - for (idx = 0; idx < PNP_MAX_MEM; idx++) { - res = &dev->res->mem[idx].res; - if (res->flags & IORESOURCE_AUTO) { - res->flags = IORESOURCE_MEM; - pnp_init_resource(res); - } + list_for_each_entry_safe(pnp_res, tmp, &dev->resources, list) { + if (pnp_res->res.flags & IORESOURCE_AUTO) + pnp_free_resource(pnp_res); } } Index: work10/drivers/pnp/quirks.c =================================================================== --- work10.orig/drivers/pnp/quirks.c 2008-06-03 12:11:31.000000000 -0600 +++ work10/drivers/pnp/quirks.c 2008-06-03 12:21:12.000000000 -0600 @@ -248,8 +248,7 @@ static void quirk_system_pci_resources(s for (j = 0; (res = pnp_get_resource(dev, IORESOURCE_MEM, j)); j++) { - if (res->flags & IORESOURCE_UNSET || - (res->start == 0 && res->end == 0)) + if (res->start == 0 && res->end == 0) continue; pnp_start = res->start; Index: work10/drivers/pnp/resource.c =================================================================== --- work10.orig/drivers/pnp/resource.c 2008-06-03 12:17:35.000000000 -0600 +++ work10/drivers/pnp/resource.c 2008-06-03 12:34:33.000000000 -0600 @@ -237,7 +237,7 @@ void pnp_free_option(struct pnp_option * !((*(enda) < *(startb)) || (*(endb) < *(starta))) #define cannot_compare(flags) \ -((flags) & (IORESOURCE_UNSET | IORESOURCE_DISABLED)) +((flags) & IORESOURCE_DISABLED) int pnp_check_port(struct pnp_dev *dev, struct resource *res) { @@ -505,81 +505,31 @@ int pnp_resource_type(struct resource *r IORESOURCE_IRQ | IORESOURCE_DMA); } -struct pnp_resource *pnp_get_pnp_resource(struct pnp_dev *dev, - unsigned int type, unsigned int num) -{ - struct pnp_resource_table *res = dev->res; - - switch (type) { - case IORESOURCE_IO: - if (num >= PNP_MAX_PORT) - return NULL; - return &res->port[num]; - case IORESOURCE_MEM: - if (num >= PNP_MAX_MEM) - return NULL; - return &res->mem[num]; - case IORESOURCE_IRQ: - if (num >= PNP_MAX_IRQ) - return NULL; - return &res->irq[num]; - case IORESOURCE_DMA: - if (num >= PNP_MAX_DMA) - return NULL; - return &res->dma[num]; - } - return NULL; -} - struct resource *pnp_get_resource(struct pnp_dev *dev, unsigned int type, unsigned int num) { struct pnp_resource *pnp_res; + struct resource *res; - pnp_res = pnp_get_pnp_resource(dev, type, num); - if (pnp_res) - return &pnp_res->res; - + list_for_each_entry(pnp_res, &dev->resources, list) { + res = &pnp_res->res; + if (pnp_resource_type(res) == type && num-- == 0) + return res; + } return NULL; } EXPORT_SYMBOL(pnp_get_resource); -static struct pnp_resource *pnp_new_resource(struct pnp_dev *dev, int type) +static struct pnp_resource *pnp_new_resource(struct pnp_dev *dev) { struct pnp_resource *pnp_res; - int i; - switch (type) { - case IORESOURCE_IO: - for (i = 0; i < PNP_MAX_PORT; i++) { - pnp_res = pnp_get_pnp_resource(dev, IORESOURCE_IO, i); - if (pnp_res && !pnp_resource_valid(&pnp_res->res)) - return pnp_res; - } - break; - case IORESOURCE_MEM: - for (i = 0; i < PNP_MAX_MEM; i++) { - pnp_res = pnp_get_pnp_resource(dev, IORESOURCE_MEM, i); - if (pnp_res && !pnp_resource_valid(&pnp_res->res)) - return pnp_res; - } - break; - case IORESOURCE_IRQ: - for (i = 0; i < PNP_MAX_IRQ; i++) { - pnp_res = pnp_get_pnp_resource(dev, IORESOURCE_IRQ, i); - if (pnp_res && !pnp_resource_valid(&pnp_res->res)) - return pnp_res; - } - break; - case IORESOURCE_DMA: - for (i = 0; i < PNP_MAX_DMA; i++) { - pnp_res = pnp_get_pnp_resource(dev, IORESOURCE_DMA, i); - if (pnp_res && !pnp_resource_valid(&pnp_res->res)) - return pnp_res; - } - break; - } - return NULL; + pnp_res = kzalloc(sizeof(struct pnp_resource), GFP_KERNEL); + if (!pnp_res) + return NULL; + + list_add_tail(&pnp_res->list, &dev->resources); + return pnp_res; } struct pnp_resource *pnp_add_irq_resource(struct pnp_dev *dev, int irq, @@ -589,7 +539,7 @@ struct pnp_resource *pnp_add_irq_resourc struct resource *res; static unsigned char warned; - pnp_res = pnp_new_resource(dev, IORESOURCE_IRQ); + pnp_res = pnp_new_resource(dev); if (!pnp_res) { if (!warned) { dev_err(&dev->dev, "can't add resource for IRQ %d\n", @@ -615,7 +565,7 @@ struct pnp_resource *pnp_add_dma_resourc struct resource *res; static unsigned char warned; - pnp_res = pnp_new_resource(dev, IORESOURCE_DMA); + pnp_res = pnp_new_resource(dev); if (!pnp_res) { if (!warned) { dev_err(&dev->dev, "can't add resource for DMA %d\n", @@ -642,7 +592,7 @@ struct pnp_resource *pnp_add_io_resource struct resource *res; static unsigned char warned; - pnp_res = pnp_new_resource(dev, IORESOURCE_IO); + pnp_res = pnp_new_resource(dev); if (!pnp_res) { if (!warned) { dev_err(&dev->dev, "can't add resource for IO " @@ -671,7 +621,7 @@ struct pnp_resource *pnp_add_mem_resourc struct resource *res; static unsigned char warned; - pnp_res = pnp_new_resource(dev, IORESOURCE_MEM); + pnp_res = pnp_new_resource(dev); if (!pnp_res) { if (!warned) { dev_err(&dev->dev, "can't add resource for MEM " Index: work10/drivers/pnp/support.c =================================================================== --- work10.orig/drivers/pnp/support.c 2008-06-03 12:20:22.000000000 -0600 +++ work10/drivers/pnp/support.c 2008-06-03 12:21:12.000000000 -0600 @@ -16,6 +16,10 @@ */ int pnp_is_active(struct pnp_dev *dev) { + /* + * I don't think this is very reliable because pnp_disable_dev() + * only clears out auto-assigned resources. + */ if (!pnp_port_start(dev, 0) && pnp_port_len(dev, 0) <= 1 && !pnp_mem_start(dev, 0) && pnp_mem_len(dev, 0) <= 1 && pnp_irq(dev, 0) == -1 && pnp_dma(dev, 0) == -1) @@ -70,54 +74,34 @@ char *pnp_resource_type_name(struct reso void dbg_pnp_show_resources(struct pnp_dev *dev, char *desc) { #ifdef DEBUG + struct pnp_resource *pnp_res; struct resource *res; - int i; dev_dbg(&dev->dev, "current resources: %s\n", desc); + list_for_each_entry(pnp_res, &dev->resources, list) { + res = &pnp_res->res; - for (i = 0; i < PNP_MAX_IRQ; i++) { - res = pnp_get_resource(dev, IORESOURCE_IRQ, i); - if (res && !(res->flags & IORESOURCE_UNSET)) - dev_dbg(&dev->dev, " irq %lld flags %#lx%s%s\n", - (unsigned long long) res->start, res->flags, - res->flags & IORESOURCE_DISABLED ? - " DISABLED" : "", - res->flags & IORESOURCE_AUTO ? - " AUTO" : ""); - } - for (i = 0; i < PNP_MAX_DMA; i++) { - res = pnp_get_resource(dev, IORESOURCE_DMA, i); - if (res && !(res->flags & IORESOURCE_UNSET)) - dev_dbg(&dev->dev, " dma %lld flags %#lx%s%s\n", - (unsigned long long) res->start, res->flags, - res->flags & IORESOURCE_DISABLED ? - " DISABLED" : "", - res->flags & IORESOURCE_AUTO ? - " AUTO" : ""); - } - for (i = 0; i < PNP_MAX_PORT; i++) { - res = pnp_get_resource(dev, IORESOURCE_IO, i); - if (res && !(res->flags & IORESOURCE_UNSET)) - dev_dbg(&dev->dev, " io %#llx-%#llx flags %#lx" - "%s%s\n", - (unsigned long long) res->start, - (unsigned long long) res->end, res->flags, - res->flags & IORESOURCE_DISABLED ? - " DISABLED" : "", - res->flags & IORESOURCE_AUTO ? - " AUTO" : ""); - } - for (i = 0; i < PNP_MAX_MEM; i++) { - res = pnp_get_resource(dev, IORESOURCE_MEM, i); - if (res && !(res->flags & IORESOURCE_UNSET)) - dev_dbg(&dev->dev, " mem %#llx-%#llx flags %#lx" - "%s%s\n", - (unsigned long long) res->start, - (unsigned long long) res->end, res->flags, - res->flags & IORESOURCE_DISABLED ? - " DISABLED" : "", - res->flags & IORESOURCE_AUTO ? - " AUTO" : ""); + dev_dbg(&dev->dev, " %-3s ", pnp_resource_type_name(res)); + + if (res->flags & IORESOURCE_DISABLED) { + printk("disabled\n"); + continue; + } + + switch (pnp_resource_type(res)) { + case IORESOURCE_IO: + case IORESOURCE_MEM: + printk("%#llx-%#llx flags %#lx", + (unsigned long long) res->start, + (unsigned long long) res->end, res->flags); + break; + case IORESOURCE_IRQ: + case IORESOURCE_DMA: + printk("%lld flags %#lx", + (unsigned long long) res->start, res->flags); + break; + } + printk("\n"); } #endif } Index: work10/drivers/pnp/system.c =================================================================== --- work10.orig/drivers/pnp/system.c 2008-06-03 12:11:31.000000000 -0600 +++ work10/drivers/pnp/system.c 2008-06-03 12:21:12.000000000 -0600 @@ -60,8 +60,6 @@ static void reserve_resources_of_dev(str int i; for (i = 0; (res = pnp_get_resource(dev, IORESOURCE_IO, i)); i++) { - if (res->flags & IORESOURCE_UNSET) - continue; if (res->start == 0) continue; /* disabled */ if (res->start < 0x100) Index: work10/include/linux/pnp.h =================================================================== --- work10.orig/include/linux/pnp.h 2008-06-03 12:20:27.000000000 -0600 +++ work10/include/linux/pnp.h 2008-06-03 12:21:12.000000000 -0600 @@ -15,7 +15,6 @@ struct pnp_protocol; struct pnp_dev; -struct pnp_resource_table; /* * Resource Management @@ -24,7 +23,7 @@ struct resource *pnp_get_resource(struct static inline int pnp_resource_valid(struct resource *res) { - if (res && !(res->flags & IORESOURCE_UNSET)) + if (res) return 1; return 0; } @@ -64,7 +63,7 @@ static inline unsigned long pnp_port_fla if (pnp_resource_valid(res)) return res->flags; - return IORESOURCE_IO | IORESOURCE_AUTO | IORESOURCE_UNSET; + return IORESOURCE_IO | IORESOURCE_AUTO; } static inline int pnp_port_valid(struct pnp_dev *dev, unsigned int bar) @@ -109,7 +108,7 @@ static inline unsigned long pnp_mem_flag if (pnp_resource_valid(res)) return res->flags; - return IORESOURCE_MEM | IORESOURCE_AUTO | IORESOURCE_UNSET; + return IORESOURCE_MEM | IORESOURCE_AUTO; } static inline int pnp_mem_valid(struct pnp_dev *dev, unsigned int bar) @@ -143,7 +142,7 @@ static inline unsigned long pnp_irq_flag if (pnp_resource_valid(res)) return res->flags; - return IORESOURCE_IRQ | IORESOURCE_AUTO | IORESOURCE_UNSET; + return IORESOURCE_IRQ | IORESOURCE_AUTO; } static inline int pnp_irq_valid(struct pnp_dev *dev, unsigned int bar) @@ -167,7 +166,7 @@ static inline unsigned long pnp_dma_flag if (pnp_resource_valid(res)) return res->flags; - return IORESOURCE_DMA | IORESOURCE_AUTO | IORESOURCE_UNSET; + return IORESOURCE_DMA | IORESOURCE_AUTO; } static inline int pnp_dma_valid(struct pnp_dev *dev, unsigned int bar) @@ -296,7 +295,7 @@ struct pnp_dev { int capabilities; struct pnp_option *independent; struct pnp_option *dependent; - struct pnp_resource_table *res; + struct list_head resources; char name[PNP_NAME_LEN]; /* contains a human-readable name */ int flags; /* used by protocols */ ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: 53052feb6 (PNP: remove pnp_mem_flags() as an lvalue) breaks my ALSA intel8x0 sound card regression 2008-06-02 22:42 ` Bjorn Helgaas 2008-06-02 23:49 ` Andrew Morton @ 2008-06-04 23:38 ` Tony Luck 2008-06-05 16:18 ` Bjorn Helgaas 1 sibling, 1 reply; 15+ messages in thread From: Tony Luck @ 2008-06-04 23:38 UTC (permalink / raw) To: Bjorn Helgaas Cc: Andrew Morton, Linus Torvalds, Avuton Olrich, Rene Herman, Rene Herman, Len Brown, Linux Kernel Mailing List, Rafael J. Wysocki This is now in Linus tree (commit 4b34fe156455d26ee6ed67b61539f136bf4e439c) > --- 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) 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: 53052feb6 (PNP: remove pnp_mem_flags() as an lvalue) breaks my ALSA intel8x0 sound card regression 2008-06-04 23:38 ` Tony Luck @ 2008-06-05 16:18 ` Bjorn Helgaas 0 siblings, 0 replies; 15+ messages in thread From: Bjorn Helgaas @ 2008-06-05 16:18 UTC (permalink / raw) To: Tony Luck Cc: Andrew Morton, Linus Torvalds, Avuton Olrich, Rene Herman, Rene Herman, Len Brown, Linux Kernel Mailing List, Rafael J. Wysocki On Wednesday 04 June 2008 05:38:53 pm Tony Luck wrote: > This is now in Linus tree (commit 4b34fe156455d26ee6ed67b61539f136bf4e439c) > > > --- 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) > > 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 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); ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2008-06-05 16:19 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-06-01 14:42 53052feb6 (PNP: remove pnp_mem_flags() as an lvalue) breaks my ALSA intel8x0 sound card regression Avuton Olrich 2008-06-01 16:20 ` Rene Herman 2008-06-02 3:25 ` Avuton Olrich 2008-06-02 19:06 ` Rene Herman 2008-06-02 22:05 ` Bjorn Helgaas 2008-06-02 22:23 ` Avuton Olrich 2008-06-02 22:42 ` Bjorn Helgaas 2008-06-02 23:49 ` Andrew Morton 2008-06-02 23:58 ` Rene Herman 2008-06-03 0:03 ` Andrew Morton 2008-06-03 0:31 ` Rene Herman 2008-06-03 0:15 ` Rene Herman 2008-06-03 18:40 ` Bjorn Helgaas 2008-06-04 23:38 ` Tony Luck 2008-06-05 16:18 ` Bjorn Helgaas
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox