Skip to contentSkip to conversation

PNP: have quirk_system_pci_resources() include io resources.

PATCH In review Rene Herman 2 participants 4 messages Jun 25, 2008 Pnp support

lore
Share & tools
lore

Rene Herman

author
#0 lore raw

PATCH 1/1 PNP: have quirk_system_pci_resources() include io resources.

Hi Andrew.

quirk_system_pci_resources() disables a PnP mem resource that overlaps a PCI BAR so as to not keep the PCI driver from claiming the resource. Have it do the same for io resources.

Here, ACPI claims ports that overlap with my soundcard causing the soundcard driver to fail to load. It's unknown why my ACPI BIOS claims those ports; it did not use to but this is not a (kernel) regression. Some odd BIOS reconfig triggered by temporarily removing the card seems to have brought this on.

This is not urgent -- I now all of a sudden need this on any kernel anyway. It applies on top of Bjorn's PNP patches you have in the tree. Strictly, Bjorn's ACK was on another version of this that rolled its own private pnp_resource_type_name(), an interface introduced by those new PNP patches. I took the liberty to consider that trivial enough of a change to keep it though.

Rene.

Quoted 1 line

From 4c6191498771b0e2b087db988f74e4b902b99d32 Mon Sep 17 00:00:00 2001

From: Rene Herman <[email protected]>
Date: Wed, 25 Jun 2008 19:45:41 +0200
Subject: [PATCH] PNP: have quirk_system_pci_resources() include io resources.

quirk_system_pci_resources() disables a PnP mem resource that overlaps a PCI BAR so as to not keep the PCI driver from claiming the resource. Have it do the same for io resources.

Here, ACPI claims ports that overlap with my soundcard causing the soundcard driver to fail to load. It's unknown why my ACPI BIOS claims those ports; it did not use to but this is not a (kernel) regression. Some odd BIOS reconfig triggered by temporarily removing the card seems to have brought this on.

Signed-off-by Rene Herman <[email protected]>
Acked-by Bjorn Helgaas <[email protected]>
---
 drivers/pnp/quirks.c |   13 ++++++++-----
 1 files changed, 8 insertions(+), 5 deletions(-)
drivers/pnp/quirks.c +8 -5
index 55f55ed..0bdf9b8 100644
@@ -245,15 +245,17 @@ static void quirk_system_pci_resources(struct pnp_dev *dev)
 	 */
 	for_each_pci_dev(pdev) {
 		for (i = 0; i < DEVICE_COUNT_RESOURCE; i++) {
-			if (!(pci_resource_flags(pdev, i) & IORESOURCE_MEM) ||
-			    pci_resource_len(pdev, i) == 0)
+			unsigned int type;
+
+			type = pci_resource_flags(pdev, i) &
+					(IORESOURCE_IO | IORESOURCE_MEM);
+			if (!type || pci_resource_len(pdev, i) == 0)
 				continue;
 
 			pci_start = pci_resource_start(pdev, i);
 			pci_end = pci_resource_end(pdev, i);
 			for (j = 0;
-			     (res = pnp_get_resource(dev, IORESOURCE_MEM, j));
-			     j++) {
+			     (res = pnp_get_resource(dev, type, j)); j++) {
 				if (res->start == 0 && res->end == 0)
 					continue;
 

@@ -283,9 +285,10 @@ static void quirk_system_pci_resources(struct pnp_dev *dev)
 				 * the PCI region, and that might prevent a PCI
 				 * driver from requesting its resources.
 				 */
-				dev_warn(&dev->dev, "mem resource "
+				dev_warn(&dev->dev, "%s resource "
 					"(0x%llx-0x%llx) overlaps %s BAR %d "
 					"(0x%llx-0x%llx), disabling\n",
+					pnp_resource_type_name(res),
 					(unsigned long long) pnp_start,
 					(unsigned long long) pnp_end,
 					pci_name(pdev), i,
Signature · 1 line
1.5.5
#1 lore raw

drivers/pnp/quirks.c static void quirk_system_pci_resources(struct pnp_dev *dev) · line 251

Quoted 4 lines · On Wed, 25 Jun 2008 19:58:20 +0200 Rene Herman <[email protected]> wrote:
drivers/pnp/quirks.c +4 -0
+			unsigned int type;
+
+			type = pci_resource_flags(pdev, i) &
+					(IORESOURCE_IO | IORESOURCE_MEM);

pci_resource_flags() returns an unsigned long.

But I think you're right and resource.flags is wrong. Because there's no way in which we'll ever be able to use bits 32-63 of that field anyway.

I suppose that Someone(tm) should convert resource.flags to a u32 or something. Minor issue.

author
#2 lore raw
Quoted 10 lines · Andrew Morton

drivers/pnp/quirks.c static void quirk_system_pci_resources(struct pnp_dev *dev) · line 251

Quoted 4 lines · On Wed, 25 Jun 2008 19:58:20 +0200 Rene Herman <[email protected]> wrote:
drivers/pnp/quirks.c +4 -0
+			unsigned int type;
+
+			type = pci_resource_flags(pdev, i) &
+					(IORESOURCE_IO | IORESOURCE_MEM);

pci_resource_flags() returns an unsigned long.

Quoted 6 lines

But I think you're right and resource.flags is wrong. Because there's no way in which we'll ever be able to use bits 32-63 of that field anyway.

I suppose that Someone(tm) should convert resource.flags to a u32 or something. Minor issue.

Yes, did notice but pnp_get_resource() then takes those same flags as an unsigned int -- had to pick one or the other and "the smallest one" then seemed best...

Rene.

author
#3 lore raw
Quoted 12 lines · Rene Herman
Quoted 10 lines · Andrew Morton

Review

Quoted 4 lines · On Wed, 25 Jun 2008 19:58:20 +0200 Rene Herman <[email protected]> wrote:
(quoted diff) +4 -0
+            unsigned int type;
+
+            type = pci_resource_flags(pdev, i) &
+                    (IORESOURCE_IO | IORESOURCE_MEM);

pci_resource_flags() returns an unsigned long.

Quoted 8 lines
Quoted 6 lines

But I think you're right and resource.flags is wrong. Because there's no way in which we'll ever be able to use bits 32-63 of that field anyway.

I suppose that Someone(tm) should convert resource.flags to a u32 or something. Minor issue.

Yes, did notice but pnp_get_resource() then takes those same flags as an unsigned int -- had to pick one or the other and "the smallest one" then seemed best...

semi-arguably, the pnp_get_resource() is simply wrong though, so maybe this patch. After this, the quirks one should indeed be an unsigned long also. Bjorn?

Rene.

Quoted 1 line

From c54f1a8a4d82a5b2891c3b15c9935ba5f8ece531 Mon Sep 17 00:00:00 2001

From: Rene Herman <[email protected]>
Date: Thu, 26 Jun 2008 00:14:08 +0200
Subject: [PATCH] PNP: make the resource type an unsigned long

PnP encodes the resource type directly as it's struct resource->flags value which is an unsigned long. Make it so...

Signed-off-by Rene Herman <[email protected]>
---
 drivers/pnp/base.h     |    2 +-
 drivers/pnp/resource.c |    4 ++--
 include/linux/pnp.h    |    2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)
drivers/pnp/base.h +1 -1
index e3fa9a2..45690ff 100644
@@ -148,7 +148,7 @@ char *pnp_resource_type_name(struct resource *res);
 void dbg_pnp_show_resources(struct pnp_dev *dev, char *desc);
 
 void pnp_free_resources(struct pnp_dev *dev);
-int pnp_resource_type(struct resource *res);
+unsigned long pnp_resource_type(struct resource *res);
 
 struct pnp_resource {
 	struct list_head list;
drivers/pnp/resource.c +2 -2
index e0e853d..5d27df1 100644
@@ -468,14 +468,14 @@ int pnp_check_dma(struct pnp_dev *dev, struct resource *res)
 #endif
 }
 
-int pnp_resource_type(struct resource *res)
+unsigned long pnp_resource_type(struct resource *res)
 {
 	return res->flags & (IORESOURCE_IO  | IORESOURCE_MEM |
 			     IORESOURCE_IRQ | IORESOURCE_DMA);
 }
 
 struct resource *pnp_get_resource(struct pnp_dev *dev,
-				  unsigned int type, unsigned int num)
+				  unsigned long type, unsigned int num)
 {
 	struct pnp_resource *pnp_res;
 	struct resource *res;
include/linux/pnp.h +1 -1
index 116abdc..c2e844c 100644
@@ -21,7 +21,7 @@ struct pnp_dev;
 /*
  * Resource Management
  */
-struct resource *pnp_get_resource(struct pnp_dev *, unsigned int, unsigned int);
+struct resource *pnp_get_resource(struct pnp_dev *, unsigned long, unsigned int);
 
 static inline int pnp_resource_valid(struct resource *res)
 {
Signature · 1 line
1.5.5