Skip to contentSkip to conversation

PNP resource_table cleanups

PATCH In review Bjorn Helgaas 3 participants 49 messages Mar 26, 2008 – Apr 1, 2008

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

cover PNP resource_table cleanups

This series of patches does some PNP housecleaning and consolidation.

PNP currently uses a fixed-size table (pnp_resource_table) to track the IO, MMIO, IRQ, and DMA resources used by a device. Some motherboard devices have many resources, so we've been plagued by table overflows and we've had to drastically increase the table size, which wastes a lot of memory.

The end goal is to replace that fixed-size table with something more dynamic. These patches don't go that far, but they do make pnp_resource_table private to the PNP core and centralize all references to it in a small set of shared functions.

In addition, this series contains a number of related cleanups, like centralized allocation of struct pnp_dev, conversion to dev_printk when possible, removing many PNP core internal functions from the public interface, and alignment of the ISAPNP, PNPBIOS, and PNPACPI backends.

Bjorn

#1 lore raw

PATCH 1/37 ISAPNP: move config register addresses out of isapnp.h

These are used only in drivers/pnp/isapnp/core.c, so no need to expose them to the world.

drivers/pnp/isapnp/core.c +8 -0
Index: work7/drivers/pnp/isapnp/core.c
===================================================================
@@ -88,6 +88,14 @@
 #define _LTAG_MEM32RANGE	0x85
 #define _LTAG_FIXEDMEM32RANGE	0x86
 
+/* Logical device control and configuration registers */
+
+#define ISAPNP_CFG_ACTIVATE	0x30	/* byte */
+#define ISAPNP_CFG_MEM		0x40	/* 4 * dword */
+#define ISAPNP_CFG_PORT		0x60	/* 8 * word */
+#define ISAPNP_CFG_IRQ		0x70	/* 2 * word */
+#define ISAPNP_CFG_DMA		0x74	/* 2 * byte */
+
 /*
  * Sizes of ISAPNP logical device configuration register sets.
  * See PNP-ISA-v1.0a.pdf, Appendix A.
include/linux/isapnp.h +0 -10
Index: work7/include/linux/isapnp.h
===================================================================
@@ -26,16 +26,6 @@
 #include <linux/pnp.h>
 
 /*
- *  Configuration registers (TODO: change by specification)
- */ 
-
-#define ISAPNP_CFG_ACTIVATE		0x30	/* byte */
-#define ISAPNP_CFG_MEM			0x40	/* 4 * dword */
-#define ISAPNP_CFG_PORT			0x60	/* 8 * word */
-#define ISAPNP_CFG_IRQ			0x70	/* 2 * word */
-#define ISAPNP_CFG_DMA			0x74	/* 2 * byte */
-
-/*
  *
  */
 
#2 lore raw

PATCH 2/37 PNPACPI: continue after _CRS and _PRS errors

In reply to Bjorn Helgaas

Keep going and register the device even if we have trouble parsing _CRS or _PRS. A parsing problem might mean we ignore some resources the device is using, or we might not be able to change its resources. But we should still take note of anything we *could* parse correctly.

Also remove reference to dev_id because I plan to remove it soon.

drivers/pnp/pnpacpi/core.c +2 -6
Index: work7/drivers/pnp/pnpacpi/core.c
===================================================================
@@ -213,8 +213,7 @@
 							  &dev->res);
 		if (ACPI_FAILURE(status) && (status != AE_NOT_FOUND)) {
 			pnp_err("PnPACPI: METHOD_NAME__CRS failure for %s",
-				dev_id->id);
-			goto err1;
+				acpi_device_hid(device));
 		}
 	}
 
@@ -223,8 +222,7 @@
 							    dev);
 		if (ACPI_FAILURE(status) && (status != AE_NOT_FOUND)) {
 			pnp_err("PnPACPI: METHOD_NAME__PRS failure for %s",
-				dev_id->id);
-			goto err1;
+				acpi_device_hid(device));
 		}
 	}
 
@@ -252,8 +250,6 @@
 	num++;
 
 	return AE_OK;
-err1:
-	kfree(dev_id);
 err:
 	kfree(dev);
 	return -EINVAL;
#3 lore raw

PATCH 3/37 PNP: make pnp_add_id() internal to PNP core

In reply to Bjorn Helgaas

pnp_add_id() doesn't need to be exposed outside the PNP core, so move the declaration to an internal header file.

include/linux/pnp.h +0 -2
Index: work7/include/linux/pnp.h
===================================================================
@@ -403,7 +403,6 @@
 /* protocol helpers */
 int pnp_is_active(struct pnp_dev *dev);
 int compare_pnp_id(struct pnp_id *pos, const char *id);
-int pnp_add_id(struct pnp_id *id, struct pnp_dev *dev);
 int pnp_register_driver(struct pnp_driver *drv);
 void pnp_unregister_driver(struct pnp_driver *drv);
 
@@ -450,7 +449,6 @@
 /* protocol helpers */
 static inline int pnp_is_active(struct pnp_dev *dev) { return 0; }
 static inline int compare_pnp_id(struct pnp_id *pos, const char *id) { return -ENODEV; }
-static inline int pnp_add_id(struct pnp_id *id, struct pnp_dev *dev) { return -ENODEV; }
 static inline int pnp_register_driver(struct pnp_driver *drv) { return -ENODEV; }
 static inline void pnp_unregister_driver(struct pnp_driver *drv) { }
 
drivers/pnp/base.h +1 -0
Index: work7/drivers/pnp/base.h
===================================================================
@@ -1,5 +1,6 @@
 extern spinlock_t pnp_lock;
 void *pnp_alloc(long size);
+int pnp_add_id(struct pnp_id *id, struct pnp_dev *dev);
 int pnp_interface_attach_device(struct pnp_dev *dev);
 void pnp_fixup_device(struct pnp_dev *dev);
 void pnp_free_option(struct pnp_option *option);
drivers/pnp/pnpacpi/core.c +1 -0
Index: work7/drivers/pnp/pnpacpi/core.c
===================================================================
@@ -25,6 +25,7 @@
 #include <acpi/acpi_bus.h>
 #include <acpi/actypes.h>
 
+#include "../base.h"
 #include "pnpacpi.h"
 
 static int num = 0;
drivers/pnp/pnpbios/core.c +1 -0
Index: work7/drivers/pnp/pnpbios/core.c
===================================================================
@@ -69,6 +69,7 @@
 #include <asm/system.h>
 #include <asm/byteorder.h>
 
+#include "../base.h"
 #include "pnpbios.h"
 
 /*
#4 lore raw

PATCH 4/37 PNP: change pnp_add_id() to allocate its own pnp_id structures

In reply to Bjorn Helgaas

This moves some of the pnp_id knowledge out of the backends and into the PNP core.

---
 drivers/pnp/base.h             |    2 +-
 drivers/pnp/driver.c           |   28 +++++++++++++++++++++-------
 drivers/pnp/isapnp/core.c      |   24 +++++++++++-------------
 drivers/pnp/pnpacpi/core.c     |   27 +++------------------------
 drivers/pnp/pnpbios/core.c     |   10 +---------
 drivers/pnp/pnpbios/rsparser.c |    7 +------
 6 files changed, 38 insertions(+), 60 deletions(-)
drivers/pnp/base.h +1 -1
Index: work7/drivers/pnp/base.h
===================================================================
@@ -1,6 +1,6 @@
 extern spinlock_t pnp_lock;
 void *pnp_alloc(long size);
-int pnp_add_id(struct pnp_id *id, struct pnp_dev *dev);
+struct pnp_id *pnp_add_id(struct pnp_dev *dev, char *id);
 int pnp_interface_attach_device(struct pnp_dev *dev);
 void pnp_fixup_device(struct pnp_dev *dev);
 void pnp_free_option(struct pnp_option *option);
drivers/pnp/driver.c +21 -7
Index: work7/drivers/pnp/driver.c
===================================================================
@@ -226,22 +226,36 @@
 
 /**
  * pnp_add_id - adds an EISA id to the specified device
- * @id: pointer to a pnp_id structure
  * @dev: pointer to the desired device
+ * @id: pointer to an EISA id string
  */
-int pnp_add_id(struct pnp_id *id, struct pnp_dev *dev)
+struct pnp_id *pnp_add_id(struct pnp_dev *dev, char *id)
 {
-	struct pnp_id *ptr;
+	struct pnp_id *dev_id, *ptr;
 
-	id->next = NULL;
+	dev_id = kzalloc(sizeof(struct pnp_id), GFP_KERNEL);
+	if (!dev_id)
+		return NULL;
+
+	dev_id->id[0] = id[0];
+	dev_id->id[1] = id[1];
+	dev_id->id[2] = id[2];
+	dev_id->id[3] = tolower(id[3]);
+	dev_id->id[4] = tolower(id[4]);
+	dev_id->id[5] = tolower(id[5]);
+	dev_id->id[6] = tolower(id[6]);
+	dev_id->id[7] = '\0';
+
+	dev_id->next = NULL;
 	ptr = dev->id;
 	while (ptr && ptr->next)
 		ptr = ptr->next;
 	if (ptr)
-		ptr->next = id;
+		ptr->next = dev_id;
 	else
-		dev->id = id;
-	return 0;
+		dev->id = dev_id;
+
+	return dev_id;
 }
 
 EXPORT_SYMBOL(pnp_register_driver);
drivers/pnp/isapnp/core.c +13 -13
Index: work7/drivers/pnp/isapnp/core.c
===================================================================
@@ -44,6 +44,8 @@
 #include <linux/mutex.h>
 #include <asm/io.h>
 
+#include "../base.h"
+
 #if 0
 #define ISAPNP_REGION_OK
 #endif
@@ -401,20 +403,18 @@
 static void isapnp_parse_id(struct pnp_dev *dev, unsigned short vendor,
 			    unsigned short device)
 {
-	struct pnp_id *id;
+	char id[8];
 
-	if (!dev)
-		return;
-	id = kzalloc(sizeof(struct pnp_id), GFP_KERNEL);
-	if (!id)
-		return;
-	sprintf(id->id, "%c%c%c%x%x%x%x",
-		'A' + ((vendor >> 2) & 0x3f) - 1,
-		'A' + (((vendor & 3) << 3) | ((vendor >> 13) & 7)) - 1,
-		'A' + ((vendor >> 8) & 0x1f) - 1,
-		(device >> 4) & 0x0f,
-		device & 0x0f, (device >> 12) & 0x0f, (device >> 8) & 0x0f);
-	pnp_add_id(id, dev);
+	id[0] = 'A' + ((vendor >> 2) & 0x3f) - 1;
+	id[1] = 'A' + (((vendor & 3) << 3) | ((vendor >> 13) & 7)) - 1;
+	id[2] = 'A' + ((vendor >> 8) & 0x1f) - 1;
+	id[3] = '0' + ((device >> 4) & 0x0f);
+	id[4] = '0' + (device & 0x0f);
+	id[5] = '0' + ((device >> 12) & 0x0f);
+	id[6] = '0' + ((device >> 8) & 0x0f);
+	id[7] = '\0';
+
+	pnp_add_id(dev, id);
 }
 
 /*
drivers/pnp/pnpacpi/core.c +2 -23
Index: work7/drivers/pnp/pnpacpi/core.c
===================================================================
@@ -73,18 +73,6 @@
 	return 1;
 }
 
-static void __init pnpidacpi_to_pnpid(char *id, char *str)
-{
-	str[0] = id[0];
-	str[1] = id[1];
-	str[2] = id[2];
-	str[3] = tolower(id[3]);
-	str[4] = tolower(id[4]);
-	str[5] = tolower(id[5]);
-	str[6] = tolower(id[6]);
-	str[7] = '\0';
-}
-
 static int pnpacpi_get_resources(struct pnp_dev *dev,
 				 struct pnp_resource_table *res)
 {
@@ -201,12 +189,9 @@
 
 	dev->number = num;
 
-	/* set the initial values for the PnP device */
-	dev_id = kzalloc(sizeof(struct pnp_id), GFP_KERNEL);
+	dev_id = pnp_add_id(dev, acpi_device_hid(device));
 	if (!dev_id)
 		goto err;
-	pnpidacpi_to_pnpid(acpi_device_hid(device), dev_id->id);
-	pnp_add_id(dev_id, dev);
 
 	if (dev->active) {
 		/* parse allocated resource */
@@ -227,7 +212,6 @@
 		}
 	}
 
-	/* parse compatible ids */
 	if (device->flags.compatible_ids) {
 		struct acpi_compatible_id_list *cid_list = device->pnp.cid_list;
 		int i;
@@ -235,12 +219,7 @@
 		for (i = 0; i < cid_list->count; i++) {
 			if (!ispnpidacpi(cid_list->id[i].value))
 				continue;
-			dev_id = kzalloc(sizeof(struct pnp_id), GFP_KERNEL);
-			if (!dev_id)
-				continue;
-
-			pnpidacpi_to_pnpid(cid_list->id[i].value, dev_id->id);
-			pnp_add_id(dev_id, dev);
+			pnp_add_id(dev, cid_list->id[i].value);
 		}
 	}
 
drivers/pnp/pnpbios/core.c +2 -4
Index: work7/drivers/pnp/pnpbios/core.c
===================================================================
@@ -332,16 +332,14 @@
 	if (!dev)
 		return -1;
 
-	dev_id = kzalloc(sizeof(struct pnp_id), GFP_KERNEL);
+	pnpid32_to_pnpid(node->eisa_id, id);
+	dev_id = pnp_add_id(dev, id);
 	if (!dev_id) {
 		kfree(dev);
 		return -1;
 	}
 
 	dev->number = node->handle;
-	pnpid32_to_pnpid(node->eisa_id, id);
-	memcpy(dev_id->id, id, 7);
-	pnp_add_id(dev_id, dev);
 	pnpbios_parse_data_stream(dev, node);
 	dev->active = pnp_is_active(dev);
 	dev->flags = node->flags;
drivers/pnp/pnpbios/rsparser.c +4 -5
Index: work7/drivers/pnp/pnpbios/rsparser.c
===================================================================
@@ -16,6 +16,7 @@
 }
 #endif				/* CONFIG_PCI */
 
+#include "../base.h"
 #include "pnpbios.h"
 
 /* standard resource tags */
@@ -548,13 +549,11 @@
 		case SMALL_TAG_COMPATDEVID:	/* compatible ID */
 			if (len != 4)
 				goto len_err;
-			dev_id = kzalloc(sizeof(struct pnp_id), GFP_KERNEL);
-			if (!dev_id)
-				return NULL;
 			pnpid32_to_pnpid(p[1] | p[2] << 8 | p[3] << 16 | p[4] <<
 					 24, id);
-			memcpy(&dev_id->id, id, 7);
-			pnp_add_id(dev_id, dev);
+			dev_id = pnp_add_id(dev, id);
+			if (!dev_id)
+				return NULL;
 			break;
 
 		case SMALL_TAG_END:
#5 lore raw

PATCH 5/37 ISAPNP: pull pnp_add_id() out of isapnp_parse_id()

In reply to Bjorn Helgaas

Split the pnp_add_id() part from the PNPID conversion part so we can move the initial add_id() into the pnp_dev allocation.

drivers/pnp/isapnp/core.c +9 -9
Index: work7/drivers/pnp/isapnp/core.c
===================================================================
@@ -400,11 +400,9 @@
 /*
  *  Parse EISA id.
  */
-static void isapnp_parse_id(struct pnp_dev *dev, unsigned short vendor,
-			    unsigned short device)
+static void isapnp_to_pnpid(unsigned short vendor, unsigned short device,
+			    char *id)
 {
-	char id[8];
-
 	id[0] = 'A' + ((vendor >> 2) & 0x3f) - 1;
 	id[1] = 'A' + (((vendor & 3) << 3) | ((vendor >> 13) & 7)) - 1;
 	id[2] = 'A' + ((vendor >> 8) & 0x1f) - 1;
@@ -413,8 +411,6 @@
 	id[5] = '0' + ((device >> 12) & 0x0f);
 	id[6] = '0' + ((device >> 8) & 0x0f);
 	id[7] = '\0';
-
-	pnp_add_id(dev, id);
 }
 
 /*
@@ -424,6 +420,7 @@
 						  int size, int number)
 {
 	unsigned char tmp[6];
+	char id[8];
 	struct pnp_dev *dev;
 
 	isapnp_peek(tmp, size);
@@ -431,7 +428,8 @@
 	if (!dev)
 		return NULL;
 	dev->number = number;
-	isapnp_parse_id(dev, (tmp[1] << 8) | tmp[0], (tmp[3] << 8) | tmp[2]);
+	isapnp_to_pnpid((tmp[1] << 8) | tmp[0], (tmp[3] << 8) | tmp[2], id);
+	pnp_add_id(dev, id);
 	dev->regs = tmp[4];
 	dev->card = card;
 	if (size > 5)
@@ -621,6 +619,7 @@
 	unsigned char type, tmp[17];
 	struct pnp_option *option;
 	struct pnp_dev *dev;
+	char id[8];
 
 	if ((dev = isapnp_parse_device(card, size, number++)) == NULL)
 		return 1;
@@ -660,8 +659,9 @@
 		case _STAG_COMPATDEVID:
 			if (size == 4 && compat < DEVICE_COUNT_COMPATIBLE) {
 				isapnp_peek(tmp, 4);
-				isapnp_parse_id(dev, (tmp[1] << 8) | tmp[0],
-						(tmp[3] << 8) | tmp[2]);
+				isapnp_to_pnpid((tmp[1] << 8) | tmp[0],
+						(tmp[3] << 8) | tmp[2], id);
+				pnp_add_id(dev, id);
 				compat++;
 				size = 0;
 			}
#6 lore raw

PATCH 6/37 PNP: add pnp_alloc_dev()

In reply to Bjorn Helgaas

Add pnp_alloc_dev() to allocate a struct pnp_dev and fill in the protocol, instance number, and initial PNP ID. Now it is always valid to use dev_printk() on any pnp_dev pointer.

---
 drivers/pnp/base.h         |    1 +
 drivers/pnp/core.c         |   28 +++++++++++++++++++++++++---
 drivers/pnp/isapnp/core.c  |    9 ++++-----
 drivers/pnp/pnpacpi/core.c |   19 +++----------------
 drivers/pnp/pnpbios/core.c |   16 ++++------------
 5 files changed, 37 insertions(+), 36 deletions(-)
drivers/pnp/base.h +1 -0
Index: work7/drivers/pnp/base.h
===================================================================
@@ -1,5 +1,6 @@
 extern spinlock_t pnp_lock;
 void *pnp_alloc(long size);
+struct pnp_dev *pnp_alloc_dev(struct pnp_protocol *, int id, char *pnpid);
 struct pnp_id *pnp_add_id(struct pnp_dev *dev, char *id);
 int pnp_interface_attach_device(struct pnp_dev *dev);
 void pnp_fixup_device(struct pnp_dev *dev);
drivers/pnp/core.c +25 -3
Index: work7/drivers/pnp/core.c
===================================================================
@@ -109,6 +109,31 @@
 	kfree(dev);
 }
 
+struct pnp_dev *pnp_alloc_dev(struct pnp_protocol *protocol, int id, char *pnpid)
+{
+	struct pnp_dev *dev;
+	struct pnp_id *dev_id;
+
+	dev = kzalloc(sizeof(struct pnp_dev), GFP_KERNEL);
+	if (!dev)
+		return NULL;
+
+	dev->protocol = protocol;
+	dev->number = id;
+
+	dev->dev.parent = &dev->protocol->dev;
+	sprintf(dev->dev.bus_id, "%02x:%02x", dev->protocol->number,
+		dev->number);
+
+	dev_id = pnp_add_id(dev, pnpid);
+	if (!dev_id) {
+		kfree(dev);
+		return NULL;
+	}
+
+	return dev;
+}
+
 int __pnp_add_device(struct pnp_dev *dev)
 {
 	int ret;
@@ -145,9 +170,6 @@
 	if (dev->card)
 		return -EINVAL;
 
-	dev->dev.parent = &dev->protocol->dev;
-	sprintf(dev->dev.bus_id, "%02x:%02x", dev->protocol->number,
-		dev->number);
 	ret = __pnp_add_device(dev);
 	if (ret)
 		return ret;
drivers/pnp/isapnp/core.c +4 -5
Index: work7/drivers/pnp/isapnp/core.c
===================================================================
@@ -424,17 +424,16 @@
 	struct pnp_dev *dev;
 
 	isapnp_peek(tmp, size);
-	dev = kzalloc(sizeof(struct pnp_dev), GFP_KERNEL);
+	isapnp_to_pnpid((tmp[1] << 8) | tmp[0], (tmp[3] << 8) | tmp[2], id);
+
+	dev = pnp_alloc_dev(&isapnp_protocol, number, id);
 	if (!dev)
 		return NULL;
-	dev->number = number;
-	isapnp_to_pnpid((tmp[1] << 8) | tmp[0], (tmp[3] << 8) | tmp[2], id);
-	pnp_add_id(dev, id);
+
 	dev->regs = tmp[4];
 	dev->card = card;
 	if (size > 5)
 		dev->regs |= tmp[5] << 8;
-	dev->protocol = &isapnp_protocol;
 	dev->capabilities |= PNP_CONFIGURABLE;
 	dev->capabilities |= PNP_READ;
 	dev->capabilities |= PNP_WRITE;
drivers/pnp/pnpacpi/core.c +3 -16
Index: work7/drivers/pnp/pnpacpi/core.c
===================================================================
@@ -152,7 +152,6 @@
 {
 	acpi_handle temp = NULL;
 	acpi_status status;
-	struct pnp_id *dev_id;
 	struct pnp_dev *dev;
 
 	status = acpi_get_handle(device->handle, "_CRS", &temp);
@@ -160,11 +159,10 @@
 	    is_exclusive_device(device))
 		return 0;
 
-	dev = kzalloc(sizeof(struct pnp_dev), GFP_KERNEL);
-	if (!dev) {
-		pnp_err("Out of memory");
+	dev = pnp_alloc_dev(&pnpacpi_protocol, num, acpi_device_hid(device));
+	if (!dev)
 		return -ENOMEM;
-	}
+
 	dev->data = device->handle;
 	/* .enabled means the device can decode the resources */
 	dev->active = device->status.enabled;
@@ -180,19 +178,11 @@
 	if (ACPI_SUCCESS(status))
 		dev->capabilities |= PNP_DISABLE;
 
-	dev->protocol = &pnpacpi_protocol;
-
 	if (strlen(acpi_device_name(device)))
 		strncpy(dev->name, acpi_device_name(device), sizeof(dev->name));
 	else
 		strncpy(dev->name, acpi_device_bid(device), sizeof(dev->name));
 
-	dev->number = num;
-
-	dev_id = pnp_add_id(dev, acpi_device_hid(device));
-	if (!dev_id)
-		goto err;
-
 	if (dev->active) {
 		/* parse allocated resource */
 		status = pnpacpi_parse_allocated_resource(device->handle,
@@ -230,9 +220,6 @@
 	num++;
 
 	return AE_OK;
-err:
-	kfree(dev);
-	return -EINVAL;
 }
 
 static acpi_status __init pnpacpi_add_device_handler(acpi_handle handle,
drivers/pnp/pnpbios/core.c +4 -12
Index: work7/drivers/pnp/pnpbios/core.c
===================================================================
@@ -318,7 +318,6 @@
 {
 	struct list_head *pos;
 	struct pnp_dev *dev;
-	struct pnp_id *dev_id;
 	char id[8];
 
 	/* check if the device is already added */
@@ -328,18 +327,12 @@
 			return -1;
 	}
 
-	dev = kzalloc(sizeof(struct pnp_dev), GFP_KERNEL);
-	if (!dev)
-		return -1;
-
 	pnpid32_to_pnpid(node->eisa_id, id);
-	dev_id = pnp_add_id(dev, id);
-	if (!dev_id) {
-		kfree(dev);
-		return -1;
-	}
 
-	dev->number = node->handle;
+	dev = pnp_alloc_dev(&pnpbios_protocol, node->handle, id);
+	if (!dev)
+		return -ENOMEM;
+
 	pnpbios_parse_data_stream(dev, node);
 	dev->active = pnp_is_active(dev);
 	dev->flags = node->flags;
@@ -352,7 +345,6 @@
 		dev->capabilities |= PNP_WRITE;
 	if (dev->flags & PNPBIOS_REMOVABLE)
 		dev->capabilities |= PNP_REMOVABLE;
-	dev->protocol = &pnpbios_protocol;
 
 	/* clear out the damaged flags */
 	if (!dev->active)
#7 lore raw

PATCH 7/37 PNP: make pnp_add_card_id() internal to PNP core

In reply to Bjorn Helgaas

pnp_add_card_id() doesn't need to be exposed outside the PNP core, so move the declaration to an internal header file.

drivers/pnp/base.h +1 -0
Index: work7/drivers/pnp/base.h
===================================================================
@@ -2,6 +2,7 @@
 void *pnp_alloc(long size);
 struct pnp_dev *pnp_alloc_dev(struct pnp_protocol *, int id, char *pnpid);
 struct pnp_id *pnp_add_id(struct pnp_dev *dev, char *id);
+int pnp_add_card_id(struct pnp_id *id, struct pnp_card *card);
 int pnp_interface_attach_device(struct pnp_dev *dev);
 void pnp_fixup_device(struct pnp_dev *dev);
 void pnp_free_option(struct pnp_option *option);
include/linux/pnp.h +0 -2
Index: work7/include/linux/pnp.h
===================================================================
@@ -371,7 +371,6 @@
 void pnp_remove_card(struct pnp_card *card);
 int pnp_add_card_device(struct pnp_card *card, struct pnp_dev *dev);
 void pnp_remove_card_device(struct pnp_dev *dev);
-int pnp_add_card_id(struct pnp_id *id, struct pnp_card *card);
 struct pnp_dev *pnp_request_card_device(struct pnp_card_link *clink,
 					const char *id, struct pnp_dev *from);
 void pnp_release_card_device(struct pnp_dev *dev);
@@ -423,7 +422,6 @@
 static inline void pnp_remove_card(struct pnp_card *card) { }
 static inline int pnp_add_card_device(struct pnp_card *card, struct pnp_dev *dev) { return -ENODEV; }
 static inline void pnp_remove_card_device(struct pnp_dev *dev) { }
-static inline int pnp_add_card_id(struct pnp_id *id, struct pnp_card *card) { return -ENODEV; }
 static inline struct pnp_dev *pnp_request_card_device(struct pnp_card_link *clink, const char *id, struct pnp_dev *from) { return NULL; }
 static inline void pnp_release_card_device(struct pnp_dev *dev) { }
 static inline int pnp_register_card_driver(struct pnp_card_driver *drv) { return -ENODEV; }
#8 lore raw

PATCH 8/37 PNP: change pnp_add_card_id() to allocate its own pnp_id structures

In reply to Bjorn Helgaas

This moves some of the pnp_id knowledge out of the backends and into the PNP core.

drivers/pnp/base.h +1 -1
Index: work7/drivers/pnp/base.h
===================================================================
@@ -2,7 +2,7 @@
 void *pnp_alloc(long size);
 struct pnp_dev *pnp_alloc_dev(struct pnp_protocol *, int id, char *pnpid);
 struct pnp_id *pnp_add_id(struct pnp_dev *dev, char *id);
-int pnp_add_card_id(struct pnp_id *id, struct pnp_card *card);
+struct pnp_id *pnp_add_card_id(struct pnp_card *card, char *id);
 int pnp_interface_attach_device(struct pnp_dev *dev);
 void pnp_fixup_device(struct pnp_dev *dev);
 void pnp_free_option(struct pnp_option *option);
drivers/pnp/card.c +21 -6
Index: work7/drivers/pnp/card.c
===================================================================
@@ -5,6 +5,7 @@
  */
 
 #include <linux/module.h>
+#include <linux/ctype.h>
 #include <linux/slab.h>
 #include <linux/pnp.h>
 #include "base.h"
@@ -100,19 +101,33 @@
  * @id: pointer to a pnp_id structure
  * @card: pointer to the desired card
  */
-int pnp_add_card_id(struct pnp_id *id, struct pnp_card *card)
+struct pnp_id *pnp_add_card_id(struct pnp_card *card, char *id)
 {
-	struct pnp_id *ptr;
+	struct pnp_id *dev_id, *ptr;
 
-	id->next = NULL;
+	dev_id = kzalloc(sizeof(struct pnp_id), GFP_KERNEL);
+	if (!dev_id)
+		return NULL;
+
+	dev_id->id[0] = id[0];
+	dev_id->id[1] = id[1];
+	dev_id->id[2] = id[2];
+	dev_id->id[3] = tolower(id[3]);
+	dev_id->id[4] = tolower(id[4]);
+	dev_id->id[5] = tolower(id[5]);
+	dev_id->id[6] = tolower(id[6]);
+	dev_id->id[7] = '\0';
+
+	dev_id->next = NULL;
 	ptr = card->id;
 	while (ptr && ptr->next)
 		ptr = ptr->next;
 	if (ptr)
-		ptr->next = id;
+		ptr->next = dev_id;
 	else
-		card->id = id;
-	return 0;
+		card->id = dev_id;
+
+	return dev_id;
 }
 
 static void pnp_free_card_ids(struct pnp_card *card)
drivers/pnp/isapnp/core.c +11 -10
Index: work7/drivers/pnp/isapnp/core.c
===================================================================
@@ -834,17 +834,18 @@
 static void isapnp_parse_card_id(struct pnp_card *card, unsigned short vendor,
 				 unsigned short device)
 {
-	struct pnp_id *id = kzalloc(sizeof(struct pnp_id), GFP_KERNEL);
+	char id[8];
 
-	if (!id)
-		return;
-	sprintf(id->id, "%c%c%c%x%x%x%x",
-		'A' + ((vendor >> 2) & 0x3f) - 1,
-		'A' + (((vendor & 3) << 3) | ((vendor >> 13) & 7)) - 1,
-		'A' + ((vendor >> 8) & 0x1f) - 1,
-		(device >> 4) & 0x0f,
-		device & 0x0f, (device >> 12) & 0x0f, (device >> 8) & 0x0f);
-	pnp_add_card_id(id, card);
+	id[0] = 'A' + ((vendor >> 2) & 0x3f) - 1;
+	id[1] = 'A' + (((vendor & 3) << 3) | ((vendor >> 13) & 7)) - 1;
+	id[2] = 'A' + ((vendor >> 8) & 0x1f) - 1;
+	id[3] = '0' + ((device >> 4) & 0x0f);
+	id[4] = '0' + (device & 0x0f);
+	id[5] = '0' + ((device >> 12) & 0x0f);
+	id[6] = '0' + ((device >> 8) & 0x0f);
+	id[7] = '\0';
+
+	pnp_add_card_id(card, id);
 }
 
 /*
#9 lore raw

PATCH 9/37 ISAPNP: pull pnp_add_card_id() out of isapnp_parse_card_id()

In reply to Bjorn Helgaas

Split the pnp_add_card_id() part from the PNPID conversion part so we can move the initial add_id() into the pnp_card allocation.

This makes the PNPID conversion generic so we can use the same one for both devices and cards.

drivers/pnp/isapnp/core.c +4 -22
Index: work7/drivers/pnp/isapnp/core.c
===================================================================
@@ -829,26 +829,6 @@
 }
 
 /*
- *  Parse EISA id for ISA PnP card.
- */
-static void isapnp_parse_card_id(struct pnp_card *card, unsigned short vendor,
-				 unsigned short device)
-{
-	char id[8];
-
-	id[0] = 'A' + ((vendor >> 2) & 0x3f) - 1;
-	id[1] = 'A' + (((vendor & 3) << 3) | ((vendor >> 13) & 7)) - 1;
-	id[2] = 'A' + ((vendor >> 8) & 0x1f) - 1;
-	id[3] = '0' + ((device >> 4) & 0x0f);
-	id[4] = '0' + (device & 0x0f);
-	id[5] = '0' + ((device >> 12) & 0x0f);
-	id[6] = '0' + ((device >> 8) & 0x0f);
-	id[7] = '\0';
-
-	pnp_add_card_id(card, id);
-}
-
-/*
  *  Build device list for all present ISA PnP devices.
  */
 static int __init isapnp_build_device_list(void)
@@ -856,6 +836,7 @@
 	int csn;
 	unsigned char header[9], checksum;
 	struct pnp_card *card;
+	char id[8];
 
 	isapnp_wait();
 	isapnp_key();
@@ -876,8 +857,9 @@
 
 		card->number = csn;
 		INIT_LIST_HEAD(&card->devices);
-		isapnp_parse_card_id(card, (header[1] << 8) | header[0],
-				     (header[3] << 8) | header[2]);
+		isapnp_to_pnpid((header[1] << 8) | header[0],
+				(header[3] << 8) | header[2], id);
+		pnp_add_card_id(card, id);
 		card->serial =
 		    (header[7] << 24) | (header[6] << 16) | (header[5] << 8) |
 		    header[4];
#10 lore raw

PATCH 10/37 PNP: add pnp_alloc_card()

In reply to Bjorn Helgaas

Add pnp_alloc_card() to allocate a struct pnp_card and fill in the protocol, instance number, and initial PNP ID. Now it is always valid to use dev_printk() on any pnp_card pointer.

---
 drivers/pnp/base.h        |    1 +
 drivers/pnp/card.c        |   28 +++++++++++++++++++++++++---
 drivers/pnp/isapnp/core.c |   11 ++++-------
 3 files changed, 30 insertions(+), 10 deletions(-)
drivers/pnp/base.h +1 -0
Index: work7/drivers/pnp/base.h
===================================================================
@@ -1,6 +1,7 @@
 extern spinlock_t pnp_lock;
 void *pnp_alloc(long size);
 struct pnp_dev *pnp_alloc_dev(struct pnp_protocol *, int id, char *pnpid);
+struct pnp_card *pnp_alloc_card(struct pnp_protocol *, int id, char *pnpid);
 struct pnp_id *pnp_add_id(struct pnp_dev *dev, char *id);
 struct pnp_id *pnp_add_card_id(struct pnp_card *card, char *id);
 int pnp_interface_attach_device(struct pnp_dev *dev);
drivers/pnp/card.c +25 -3
Index: work7/drivers/pnp/card.c
===================================================================
@@ -151,6 +151,31 @@
 	kfree(card);
 }
 
+struct pnp_card *pnp_alloc_card(struct pnp_protocol *protocol, int id, char *pnpid)
+{
+	struct pnp_card *card;
+	struct pnp_id *dev_id;
+
+	card = kzalloc(sizeof(struct pnp_card), GFP_KERNEL);
+	if (!card)
+		return NULL;
+
+	card->protocol = protocol;
+	card->number = id;
+
+	card->dev.parent = &card->protocol->dev;
+	sprintf(card->dev.bus_id, "%02x:%02x", card->protocol->number,
+		card->number);
+
+	dev_id = pnp_add_card_id(card, pnpid);
+	if (!dev_id) {
+		kfree(card);
+		return NULL;
+	}
+
+	return card;
+}
+
 static ssize_t pnp_show_card_name(struct device *dmdev,
 				  struct device_attribute *attr, char *buf)
 {
@@ -206,9 +231,6 @@
 	int error;
 	struct list_head *pos, *temp;
 
-	sprintf(card->dev.bus_id, "%02x:%02x", card->protocol->number,
-		card->number);
-	card->dev.parent = &card->protocol->dev;
 	card->dev.bus = NULL;
 	card->dev.release = &pnp_release_card;
 	error = device_register(&card->dev);
drivers/pnp/isapnp/core.c +4 -7
Index: work7/drivers/pnp/isapnp/core.c
===================================================================
@@ -851,15 +851,13 @@
 		       header[5], header[6], header[7], header[8]);
 		printk(KERN_DEBUG "checksum = 0x%x\n", checksum);
 #endif
-		if ((card =
-		     kzalloc(sizeof(struct pnp_card), GFP_KERNEL)) == NULL)
+		isapnp_to_pnpid((header[1] << 8) | header[0],
+				(header[3] << 8) | header[2], id);
+		card = pnp_alloc_card(&isapnp_protocol, csn, id);
+		if (!card)
 			continue;
 
-		card->number = csn;
 		INIT_LIST_HEAD(&card->devices);
-		isapnp_to_pnpid((header[1] << 8) | header[0],
-				(header[3] << 8) | header[2], id);
-		pnp_add_card_id(card, id);
 		card->serial =
 		    (header[7] << 24) | (header[6] << 16) | (header[5] << 8) |
 		    header[4];
@@ -870,7 +868,6 @@
 			       "isapnp: checksum for device %i is not valid (0x%x)\n",
 			       csn, isapnp_checksum_value);
 		card->checksum = isapnp_checksum_value;
-		card->protocol = &isapnp_protocol;
 
 		pnp_add_card(card);
 	}
#11 lore raw

PATCH 11/37 PNPACPI: pnpacpi_encode_ext_irq() wrongly set "irq" instead of "extended_irq"

In reply to Bjorn Helgaas

This has been wrong since at least 2.6.12. I haven't seen any bug reports, but it's clearly incorrect.

drivers/pnp/pnpacpi/rsparser.c +2 -2
Index: work7/drivers/pnp/pnpacpi/rsparser.c
===================================================================
@@ -818,9 +818,9 @@
 	resource->data.extended_irq.triggering = triggering;
 	resource->data.extended_irq.polarity = polarity;
 	if (triggering == ACPI_EDGE_SENSITIVE)
-		resource->data.irq.sharable = ACPI_EXCLUSIVE;
+		resource->data.extended_irq.sharable = ACPI_EXCLUSIVE;
 	else
-		resource->data.irq.sharable = ACPI_SHARED;
+		resource->data.extended_irq.sharable = ACPI_SHARED;
 	resource->data.extended_irq.interrupt_count = 1;
 	resource->data.extended_irq.interrupts[0] = p->start;
 }
#12 lore raw

PATCH 12/37 PNPACPI: use temporaries to reduce repetition

In reply to Bjorn Helgaas

No functional change, just fewer words and fewer chances for transcription errors.

drivers/pnp/pnpacpi/rsparser.c +103 -73
Index: work7/drivers/pnp/pnpacpi/rsparser.c
===================================================================
@@ -278,6 +278,14 @@
 					      void *data)
 {
 	struct pnp_resource_table *res_table = data;
+	struct acpi_resource_irq *irq;
+	struct acpi_resource_dma *dma;
+	struct acpi_resource_io *io;
+	struct acpi_resource_fixed_io *fixed_io;
+	struct acpi_resource_memory24 *memory24;
+	struct acpi_resource_memory32 *memory32;
+	struct acpi_resource_fixed_memory32 *fixed_memory32;
+	struct acpi_resource_extended_irq *extended_irq;
 	int i;
 
 	switch (res->type) {
@@ -286,29 +294,32 @@
 		 * Per spec, only one interrupt per descriptor is allowed in
 		 * _CRS, but some firmware violates this, so parse them all.
 		 */
-		for (i = 0; i < res->data.irq.interrupt_count; i++) {
+		irq = &res->data.irq;
+		for (i = 0; i < irq->interrupt_count; i++) {
 			pnpacpi_parse_allocated_irqresource(res_table,
-				res->data.irq.interrupts[i],
-				res->data.irq.triggering,
-				res->data.irq.polarity,
-				res->data.irq.sharable);
+				irq->interrupts[i],
+				irq->triggering,
+				irq->polarity,
+				irq->sharable);
 		}
 		break;
 
 	case ACPI_RESOURCE_TYPE_DMA:
-		if (res->data.dma.channel_count > 0)
+		dma = &res->data.dma;
+		if (dma->channel_count > 0)
 			pnpacpi_parse_allocated_dmaresource(res_table,
-				res->data.dma.channels[0],
-				res->data.dma.type,
-				res->data.dma.bus_master,
-				res->data.dma.transfer);
+				dma->channels[0],
+				dma->type,
+				dma->bus_master,
+				dma->transfer);
 		break;
 
 	case ACPI_RESOURCE_TYPE_IO:
+		io = &res->data.io;
 		pnpacpi_parse_allocated_ioresource(res_table,
-			res->data.io.minimum,
-			res->data.io.address_length,
-			res->data.io.io_decode);
+			io->minimum,
+			io->address_length,
+			io->io_decode);
 		break;
 
 	case ACPI_RESOURCE_TYPE_START_DEPENDENT:
@@ -316,9 +327,10 @@
 		break;
 
 	case ACPI_RESOURCE_TYPE_FIXED_IO:
+		fixed_io = &res->data.fixed_io;
 		pnpacpi_parse_allocated_ioresource(res_table,
-			res->data.fixed_io.address,
-			res->data.fixed_io.address_length,
+			fixed_io->address,
+			fixed_io->address_length,
 			ACPI_DECODE_10);
 		break;
 
@@ -329,22 +341,25 @@
 		break;
 
 	case ACPI_RESOURCE_TYPE_MEMORY24:
+		memory24 = &res->data.memory24;
 		pnpacpi_parse_allocated_memresource(res_table,
-			res->data.memory24.minimum,
-			res->data.memory24.address_length,
-			res->data.memory24.write_protect);
+			memory24->minimum,
+			memory24->address_length,
+			memory24->write_protect);
 		break;
 	case ACPI_RESOURCE_TYPE_MEMORY32:
+		memory32 = &res->data.memory32;
 		pnpacpi_parse_allocated_memresource(res_table,
-			res->data.memory32.minimum,
-			res->data.memory32.address_length,
-			res->data.memory32.write_protect);
+			memory32->minimum,
+			memory32->address_length,
+			memory32->write_protect);
 		break;
 	case ACPI_RESOURCE_TYPE_FIXED_MEMORY32:
+		fixed_memory32 = &res->data.fixed_memory32;
 		pnpacpi_parse_allocated_memresource(res_table,
-			res->data.fixed_memory32.address,
-			res->data.fixed_memory32.address_length,
-			res->data.fixed_memory32.write_protect);
+			fixed_memory32->address,
+			fixed_memory32->address_length,
+			fixed_memory32->write_protect);
 		break;
 	case ACPI_RESOURCE_TYPE_ADDRESS16:
 	case ACPI_RESOURCE_TYPE_ADDRESS32:
@@ -358,15 +373,16 @@
 		break;
 
 	case ACPI_RESOURCE_TYPE_EXTENDED_IRQ:
-		if (res->data.extended_irq.producer_consumer == ACPI_PRODUCER)
+		extended_irq = &res->data.extended_irq;
+		if (extended_irq->producer_consumer == ACPI_PRODUCER)
 			return AE_OK;
 
-		for (i = 0; i < res->data.extended_irq.interrupt_count; i++) {
+		for (i = 0; i < extended_irq->interrupt_count; i++) {
 			pnpacpi_parse_allocated_irqresource(res_table,
-				res->data.extended_irq.interrupts[i],
-				res->data.extended_irq.triggering,
-				res->data.extended_irq.polarity,
-				res->data.extended_irq.sharable);
+				extended_irq->interrupts[i],
+				extended_irq->triggering,
+				extended_irq->polarity,
+				extended_irq->sharable);
 		}
 		break;
 
@@ -795,122 +811,136 @@
 static void pnpacpi_encode_irq(struct acpi_resource *resource,
 			       struct resource *p)
 {
+	struct acpi_resource_irq *irq = &resource->data.irq;
 	int triggering, polarity;
 
 	decode_irq_flags(p->flags & IORESOURCE_BITS, &triggering, &polarity);
-	resource->data.irq.triggering = triggering;
-	resource->data.irq.polarity = polarity;
+	irq->triggering = triggering;
+	irq->polarity = polarity;
 	if (triggering == ACPI_EDGE_SENSITIVE)
-		resource->data.irq.sharable = ACPI_EXCLUSIVE;
+		irq->sharable = ACPI_EXCLUSIVE;
 	else
-		resource->data.irq.sharable = ACPI_SHARED;
-	resource->data.irq.interrupt_count = 1;
-	resource->data.irq.interrupts[0] = p->start;
+		irq->sharable = ACPI_SHARED;
+	irq->interrupt_count = 1;
+	irq->interrupts[0] = p->start;
 }
 
 static void pnpacpi_encode_ext_irq(struct acpi_resource *resource,
 				   struct resource *p)
 {
+	struct acpi_resource_extended_irq *extended_irq = &resource->data.extended_irq;
 	int triggering, polarity;
 
 	decode_irq_flags(p->flags & IORESOURCE_BITS, &triggering, &polarity);
-	resource->data.extended_irq.producer_consumer = ACPI_CONSUMER;
-	resource->data.extended_irq.triggering = triggering;
-	resource->data.extended_irq.polarity = polarity;
+	extended_irq->producer_consumer = ACPI_CONSUMER;
+	extended_irq->triggering = triggering;
+	extended_irq->polarity = polarity;
 	if (triggering == ACPI_EDGE_SENSITIVE)
-		resource->data.extended_irq.sharable = ACPI_EXCLUSIVE;
+		extended_irq->sharable = ACPI_EXCLUSIVE;
 	else
-		resource->data.extended_irq.sharable = ACPI_SHARED;
-	resource->data.extended_irq.interrupt_count = 1;
-	resource->data.extended_irq.interrupts[0] = p->start;
+		extended_irq->sharable = ACPI_SHARED;
+	extended_irq->interrupt_count = 1;
+	extended_irq->interrupts[0] = p->start;
 }
 
 static void pnpacpi_encode_dma(struct acpi_resource *resource,
 			       struct resource *p)
 {
+	struct acpi_resource_dma *dma = &resource->data.dma;
+
 	/* Note: pnp_assign_dma will copy pnp_dma->flags into p->flags */
 	switch (p->flags & IORESOURCE_DMA_SPEED_MASK) {
 	case IORESOURCE_DMA_TYPEA:
-		resource->data.dma.type = ACPI_TYPE_A;
+		dma->type = ACPI_TYPE_A;
 		break;
 	case IORESOURCE_DMA_TYPEB:
-		resource->data.dma.type = ACPI_TYPE_B;
+		dma->type = ACPI_TYPE_B;
 		break;
 	case IORESOURCE_DMA_TYPEF:
-		resource->data.dma.type = ACPI_TYPE_F;
+		dma->type = ACPI_TYPE_F;
 		break;
 	default:
-		resource->data.dma.type = ACPI_COMPATIBILITY;
+		dma->type = ACPI_COMPATIBILITY;
 	}
 
 	switch (p->flags & IORESOURCE_DMA_TYPE_MASK) {
 	case IORESOURCE_DMA_8BIT:
-		resource->data.dma.transfer = ACPI_TRANSFER_8;
+		dma->transfer = ACPI_TRANSFER_8;
 		break;
 	case IORESOURCE_DMA_8AND16BIT:
-		resource->data.dma.transfer = ACPI_TRANSFER_8_16;
+		dma->transfer = ACPI_TRANSFER_8_16;
 		break;
 	default:
-		resource->data.dma.transfer = ACPI_TRANSFER_16;
+		dma->transfer = ACPI_TRANSFER_16;
 	}
 
-	resource->data.dma.bus_master = !!(p->flags & IORESOURCE_DMA_MASTER);
-	resource->data.dma.channel_count = 1;
-	resource->data.dma.channels[0] = p->start;
+	dma->bus_master = !!(p->flags & IORESOURCE_DMA_MASTER);
+	dma->channel_count = 1;
+	dma->channels[0] = p->start;
 }
 
 static void pnpacpi_encode_io(struct acpi_resource *resource,
 			      struct resource *p)
 {
+	struct acpi_resource_io *io = &resource->data.io;
+
 	/* Note: pnp_assign_port will copy pnp_port->flags into p->flags */
-	resource->data.io.io_decode = (p->flags & PNP_PORT_FLAG_16BITADDR) ?
+	io->io_decode = (p->flags & PNP_PORT_FLAG_16BITADDR) ?
 	    ACPI_DECODE_16 : ACPI_DECODE_10;
-	resource->data.io.minimum = p->start;
-	resource->data.io.maximum = p->end;
-	resource->data.io.alignment = 0;	/* Correct? */
-	resource->data.io.address_length = p->end - p->start + 1;
+	io->minimum = p->start;
+	io->maximum = p->end;
+	io->alignment = 0;	/* Correct? */
+	io->address_length = p->end - p->start + 1;
 }
 
 static void pnpacpi_encode_fixed_io(struct acpi_resource *resource,
 				    struct resource *p)
 {
-	resource->data.fixed_io.address = p->start;
-	resource->data.fixed_io.address_length = p->end - p->start + 1;
+	struct acpi_resource_fixed_io *fixed_io = &resource->data.fixed_io;
+
+	fixed_io->address = p->start;
+	fixed_io->address_length = p->end - p->start + 1;
 }
 
 static void pnpacpi_encode_mem24(struct acpi_resource *resource,
 				 struct resource *p)
 {
+	struct acpi_resource_memory24 *memory24 = &resource->data.memory24;
+
 	/* Note: pnp_assign_mem will copy pnp_mem->flags into p->flags */
-	resource->data.memory24.write_protect =
+	memory24->write_protect =
 	    (p->flags & IORESOURCE_MEM_WRITEABLE) ?
 	    ACPI_READ_WRITE_MEMORY : ACPI_READ_ONLY_MEMORY;
-	resource->data.memory24.minimum = p->start;
-	resource->data.memory24.maximum = p->end;
-	resource->data.memory24.alignment = 0;
-	resource->data.memory24.address_length = p->end - p->start + 1;
+	memory24->minimum = p->start;
+	memory24->maximum = p->end;
+	memory24->alignment = 0;
+	memory24->address_length = p->end - p->start + 1;
 }
 
 static void pnpacpi_encode_mem32(struct acpi_resource *resource,
 				 struct resource *p)
 {
-	resource->data.memory32.write_protect =
+	struct acpi_resource_memory32 *memory32 = &resource->data.memory32;
+
+	memory32->write_protect =
 	    (p->flags & IORESOURCE_MEM_WRITEABLE) ?
 	    ACPI_READ_WRITE_MEMORY : ACPI_READ_ONLY_MEMORY;
-	resource->data.memory32.minimum = p->start;
-	resource->data.memory32.maximum = p->end;
-	resource->data.memory32.alignment = 0;
-	resource->data.memory32.address_length = p->end - p->start + 1;
+	memory32->minimum = p->start;
+	memory32->maximum = p->end;
+	memory32->alignment = 0;
+	memory32->address_length = p->end - p->start + 1;
 }
 
 static void pnpacpi_encode_fixed_mem32(struct acpi_resource *resource,
 				       struct resource *p)
 {
-	resource->data.fixed_memory32.write_protect =
+	struct acpi_resource_fixed_memory32 *fixed_memory32 = &resource->data.fixed_memory32;
+
+	fixed_memory32->write_protect =
 	    (p->flags & IORESOURCE_MEM_WRITEABLE) ?
 	    ACPI_READ_WRITE_MEMORY : ACPI_READ_ONLY_MEMORY;
-	resource->data.fixed_memory32.address = p->start;
-	resource->data.fixed_memory32.address_length = p->end - p->start + 1;
+	fixed_memory32->address = p->start;
+	fixed_memory32->address_length = p->end - p->start + 1;
 }
 
 int pnpacpi_encode_resources(struct pnp_resource_table *res_table,
#13 lore raw

PATCH 13/37 PNPACPI: hoist dma_flags() out of pnpacpi_parse_allocated_dmaresource()

In reply to Bjorn Helgaas

Hoist dma_flags() out of pnpacpi_parse_allocated_dmaresource() into its caller. This makes pnpacpi_parse_allocated_dmaresource() more similar to pnpbios_parse_allocated_dmaresource().

drivers/pnp/pnpacpi/rsparser.c +4 -7
Index: work7/drivers/pnp/pnpacpi/rsparser.c
===================================================================
@@ -167,8 +167,7 @@
 }
 
 static void pnpacpi_parse_allocated_dmaresource(struct pnp_resource_table *res,
-						u32 dma, int type,
-						int bus_master, int transfer)
+						u32 dma, int flags)
 {
 	int i = 0;
 	static unsigned char warned;
@@ -178,8 +177,7 @@
 		i++;
 	if (i < PNP_MAX_DMA) {
 		res->dma_resource[i].flags = IORESOURCE_DMA;	// Also clears _UNSET flag
-		res->dma_resource[i].flags |=
-		    dma_flags(type, bus_master, transfer);
+		res->dma_resource[i].flags |= flags;
 		if (dma == -1) {
 			res->dma_resource[i].flags |= IORESOURCE_DISABLED;
 			return;
@@ -309,9 +307,8 @@
 		if (dma->channel_count > 0)
 			pnpacpi_parse_allocated_dmaresource(res_table,
 				dma->channels[0],
-				dma->type,
-				dma->bus_master,
-				dma->transfer);
+				dma_flags(dma->type, dma->bus_master,
+					  dma->transfer));
 		break;
 
 	case ACPI_RESOURCE_TYPE_IO:
#14 lore raw

PATCH 14/37 PNPACPI: extend irq_flags() to set IORESOURCE_IRQ_SHAREABLE when appropriate

In reply to Bjorn Helgaas

This simplifies IRQ resource parsing slightly by computing all the IORESOURCE_IRQ_* flags at the same time.

This also keeps track of shareability information when parsing options from _PRS. Previously we ignored shareability in _PRS.

drivers/pnp/pnpacpi/rsparser.c +15 -11
Index: work7/drivers/pnp/pnpacpi/rsparser.c
===================================================================
@@ -32,19 +32,26 @@
 /*
  * Allocated Resources
  */
-static int irq_flags(int triggering, int polarity)
+static int irq_flags(int triggering, int polarity, int shareable)
 {
+	int flags;
+
 	if (triggering == ACPI_LEVEL_SENSITIVE) {
 		if (polarity == ACPI_ACTIVE_LOW)
-			return IORESOURCE_IRQ_LOWLEVEL;
+			flags = IORESOURCE_IRQ_LOWLEVEL;
 		else
-			return IORESOURCE_IRQ_HIGHLEVEL;
+			flags = IORESOURCE_IRQ_HIGHLEVEL;
 	} else {
 		if (polarity == ACPI_ACTIVE_LOW)
-			return IORESOURCE_IRQ_LOWEDGE;
+			flags = IORESOURCE_IRQ_LOWEDGE;
 		else
-			return IORESOURCE_IRQ_HIGHEDGE;
+			flags = IORESOURCE_IRQ_HIGHEDGE;
 	}
+
+	if (shareable)
+		flags |= IORESOURCE_IRQ_SHAREABLE;
+
+	return flags;
 }
 
 static void decode_irq_flags(int flag, int *triggering, int *polarity)
@@ -108,16 +115,13 @@
 	}
 
 	res->irq_resource[i].flags = IORESOURCE_IRQ;	// Also clears _UNSET flag
-	res->irq_resource[i].flags |= irq_flags(triggering, polarity);
+	res->irq_resource[i].flags |= irq_flags(triggering, polarity, shareable);
 	irq = acpi_register_gsi(gsi, triggering, polarity);
 	if (irq < 0) {
 		res->irq_resource[i].flags |= IORESOURCE_DISABLED;
 		return;
 	}
 
-	if (shareable)
-		res->irq_resource[i].flags |= IORESOURCE_IRQ_SHAREABLE;
-
 	res->irq_resource[i].start = irq;
 	res->irq_resource[i].end = irq;
 	pcibios_penalize_isa_irq(irq, 1);
@@ -439,7 +443,7 @@
 	for (i = 0; i < p->interrupt_count; i++)
 		if (p->interrupts[i])
 			__set_bit(p->interrupts[i], irq->map);
-	irq->flags = irq_flags(p->triggering, p->polarity);
+	irq->flags = irq_flags(p->triggering, p->polarity, p->sharable);
 
 	pnp_register_irq_resource(option, irq);
 }
@@ -459,7 +463,7 @@
 	for (i = 0; i < p->interrupt_count; i++)
 		if (p->interrupts[i])
 			__set_bit(p->interrupts[i], irq->map);
-	irq->flags = irq_flags(p->triggering, p->polarity);
+	irq->flags = irq_flags(p->triggering, p->polarity, p->sharable);
 
 	pnp_register_irq_resource(option, irq);
 }
#15 lore raw

PATCH 15/37 PNPACPI: pass pnp_dev instead of acpi_handle

In reply to Bjorn Helgaas

Pass the pnp_dev pointer when possible instead of the acpi_handle. This allows better error messages and reduces the chance of error in the caller.

drivers/pnp/pnpacpi/core.c +2 -2
Index: work7/drivers/pnp/pnpacpi/core.c
===================================================================
@@ -88,10 +88,10 @@
 {
 	acpi_handle handle = dev->data;
 	struct acpi_buffer buffer;
-	int ret = 0;
+	int ret;
 	acpi_status status;
 
-	ret = pnpacpi_build_resource_template(handle, &buffer);
+	ret = pnpacpi_build_resource_template(dev, &buffer);
 	if (ret)
 		return ret;
 	ret = pnpacpi_encode_resources(res, &buffer);
drivers/pnp/pnpacpi/pnpacpi.h +1 -1
Index: work7/drivers/pnp/pnpacpi/pnpacpi.h
===================================================================
@@ -8,5 +8,5 @@
 acpi_status pnpacpi_parse_allocated_resource(acpi_handle, struct pnp_resource_table*);
 acpi_status pnpacpi_parse_resource_option_data(acpi_handle, struct pnp_dev*);
 int pnpacpi_encode_resources(struct pnp_resource_table *, struct acpi_buffer *);
-int pnpacpi_build_resource_template(acpi_handle, struct acpi_buffer*);
+int pnpacpi_build_resource_template(struct pnp_dev *, struct acpi_buffer *);
 #endif
drivers/pnp/pnpacpi/rsparser.c +2 -1
Index: work7/drivers/pnp/pnpacpi/rsparser.c
===================================================================
@@ -775,9 +775,10 @@
 	return AE_OK;
 }
 
-int pnpacpi_build_resource_template(acpi_handle handle,
+int pnpacpi_build_resource_template(struct pnp_dev *dev,
 				    struct acpi_buffer *buffer)
 {
+	acpi_handle handle = dev->data;
 	struct acpi_resource *resource;
 	int res_cnt = 0;
 	acpi_status status;
#16 lore raw

PATCH 16/37 PNP: remove pnp_resource_table from internal get/set interfaces

In reply to Bjorn Helgaas

When we call protocol->get() and protocol->set() methods, we currently supply pointers to both the pnp_dev and the pnp_resource_table even though the pnp_resource_table should always be the one associated with the pnp_dev.

This removes the pnp_resource_table arguments to make it clear that these methods only operate on the specified pnp_dev.

---
 include/linux/pnp.h        |    4 ++--
 drivers/pnp/interface.c    |    2 +-
 drivers/pnp/manager.c      |    2 +-
 drivers/pnp/isapnp/core.c  |   11 +++++------
 drivers/pnp/pnpacpi/core.c |    8 +++-----
 drivers/pnp/pnpbios/core.c |   10 ++++------
 6 files changed, 16 insertions(+), 21 deletions(-)
include/linux/pnp.h +2 -2
Index: work7/include/linux/pnp.h
===================================================================
@@ -328,8 +328,8 @@
 	char *name;
 
 	/* resource control functions */
-	int (*get) (struct pnp_dev *dev, struct pnp_resource_table *res);
-	int (*set) (struct pnp_dev *dev, struct pnp_resource_table *res);
+	int (*get) (struct pnp_dev *dev);
+	int (*set) (struct pnp_dev *dev);
 	int (*disable) (struct pnp_dev *dev);
 
 	/* protocol specific suspend/resume */
drivers/pnp/interface.c +1 -1
Index: work7/drivers/pnp/interface.c
===================================================================
@@ -364,7 +364,7 @@
 	if (!strnicmp(buf, "get", 3)) {
 		mutex_lock(&pnp_res_mutex);
 		if (pnp_can_read(dev))
-			dev->protocol->get(dev, &dev->res);
+			dev->protocol->get(dev);
 		mutex_unlock(&pnp_res_mutex);
 		goto done;
 	}
drivers/pnp/manager.c +1 -1
Index: work7/drivers/pnp/manager.c
===================================================================
@@ -473,7 +473,7 @@
 		return -EINVAL;
 	}
 
-	if (dev->protocol->set(dev, &dev->res) < 0) {
+	if (dev->protocol->set(dev) < 0) {
 		dev_err(&dev->dev, "activation failed\n");
 		return -EIO;
 	}
drivers/pnp/isapnp/core.c +5 -6
Index: work7/drivers/pnp/isapnp/core.c
===================================================================
@@ -978,21 +978,20 @@
 	return 0;
 }
 
-static int isapnp_get_resources(struct pnp_dev *dev,
-				struct pnp_resource_table *res)
+static int isapnp_get_resources(struct pnp_dev *dev)
 {
 	int ret;
 
-	pnp_init_resource_table(res);
+	pnp_init_resource_table(&dev->res);
 	isapnp_cfg_begin(dev->card->number, dev->number);
-	ret = isapnp_read_resources(dev, res);
+	ret = isapnp_read_resources(dev, &dev->res);
 	isapnp_cfg_end();
 	return ret;
 }
 
-static int isapnp_set_resources(struct pnp_dev *dev,
-				struct pnp_resource_table *res)
+static int isapnp_set_resources(struct pnp_dev *dev)
 {
+	struct pnp_resource_table *res = &dev->res;
 	int tmp;
 
 	isapnp_cfg_begin(dev->card->number, dev->number);
drivers/pnp/pnpacpi/core.c +3 -5
Index: work7/drivers/pnp/pnpacpi/core.c
===================================================================
@@ -73,8 +73,7 @@
 	return 1;
 }
 
-static int pnpacpi_get_resources(struct pnp_dev *dev,
-				 struct pnp_resource_table *res)
+static int pnpacpi_get_resources(struct pnp_dev *dev)
 {
 	acpi_status status;
 
@@ -83,8 +82,7 @@
 	return ACPI_FAILURE(status) ? -ENODEV : 0;
 }
 
-static int pnpacpi_set_resources(struct pnp_dev *dev,
-				 struct pnp_resource_table *res)
+static int pnpacpi_set_resources(struct pnp_dev *dev)
 {
 	acpi_handle handle = dev->data;
 	struct acpi_buffer buffer;
@@ -94,7 +92,7 @@
 	ret = pnpacpi_build_resource_template(dev, &buffer);
 	if (ret)
 		return ret;
-	ret = pnpacpi_encode_resources(res, &buffer);
+	ret = pnpacpi_encode_resources(&dev->res, &buffer);
 	if (ret) {
 		kfree(buffer.pointer);
 		return ret;
drivers/pnp/pnpbios/core.c +4 -6
Index: work7/drivers/pnp/pnpbios/core.c
===================================================================
@@ -204,8 +204,7 @@
 
 #endif				/* CONFIG_HOTPLUG */
 
-static int pnpbios_get_resources(struct pnp_dev *dev,
-				 struct pnp_resource_table *res)
+static int pnpbios_get_resources(struct pnp_dev *dev)
 {
 	u8 nodenum = dev->number;
 	struct pnp_bios_node *node;
@@ -220,14 +219,13 @@
 		kfree(node);
 		return -ENODEV;
 	}
-	pnpbios_read_resources_from_node(res, node);
+	pnpbios_read_resources_from_node(&dev->res, node);
 	dev->active = pnp_is_active(dev);
 	kfree(node);
 	return 0;
 }
 
-static int pnpbios_set_resources(struct pnp_dev *dev,
-				 struct pnp_resource_table *res)
+static int pnpbios_set_resources(struct pnp_dev *dev)
 {
 	u8 nodenum = dev->number;
 	struct pnp_bios_node *node;
@@ -243,7 +241,7 @@
 		kfree(node);
 		return -ENODEV;
 	}
-	if (pnpbios_write_resources_to_node(res, node) < 0) {
+	if (pnpbios_write_resources_to_node(&dev->res, node) < 0) {
 		kfree(node);
 		return -1;
 	}
#17 lore raw

PATCH 17/37 PNP: remove more pnp_resource_table arguments

In reply to Bjorn Helgaas

Stop passing around struct pnp_resource_table pointers. In most cases, the caller doesn't need to know how the resources are stored inside the struct pnp_dev.

---
 drivers/pnp/isapnp/core.c      |    6 ++--
 drivers/pnp/pnpacpi/core.c     |   11 ++-----
 drivers/pnp/pnpacpi/pnpacpi.h  |    6 ++--
 drivers/pnp/pnpacpi/rsparser.c |   55 +++++++++++++++++++++-----------------
 drivers/pnp/pnpbios/core.c     |    4 +-
 drivers/pnp/pnpbios/pnpbios.h  |    4 +-
 drivers/pnp/pnpbios/rsparser.c |   58 ++++++++++++++++++++---------------------
 7 files changed, 73 insertions(+), 71 deletions(-)
drivers/pnp/isapnp/core.c +3 -3
Index: work7/drivers/pnp/isapnp/core.c
===================================================================
@@ -934,9 +934,9 @@
 EXPORT_SYMBOL(isapnp_cfg_end);
 EXPORT_SYMBOL(isapnp_write_byte);
 
-static int isapnp_read_resources(struct pnp_dev *dev,
-				 struct pnp_resource_table *res)
+static int isapnp_read_resources(struct pnp_dev *dev)
 {
+	struct pnp_resource_table *res = &dev->res;
 	int tmp, ret;
 
 	dev->active = isapnp_read_byte(ISAPNP_CFG_ACTIVATE);
@@ -984,7 +984,7 @@
 
 	pnp_init_resource_table(&dev->res);
 	isapnp_cfg_begin(dev->card->number, dev->number);
-	ret = isapnp_read_resources(dev, &dev->res);
+	ret = isapnp_read_resources(dev);
 	isapnp_cfg_end();
 	return ret;
 }
drivers/pnp/pnpacpi/core.c +4 -7
Index: work7/drivers/pnp/pnpacpi/core.c
===================================================================
@@ -77,8 +77,7 @@
 {
 	acpi_status status;
 
-	status = pnpacpi_parse_allocated_resource((acpi_handle) dev->data,
-						  &dev->res);
+	status = pnpacpi_parse_allocated_resource(dev);
 	return ACPI_FAILURE(status) ? -ENODEV : 0;
 }
 
@@ -92,7 +91,7 @@
 	ret = pnpacpi_build_resource_template(dev, &buffer);
 	if (ret)
 		return ret;
-	ret = pnpacpi_encode_resources(&dev->res, &buffer);
+	ret = pnpacpi_encode_resources(dev, &buffer);
 	if (ret) {
 		kfree(buffer.pointer);
 		return ret;
@@ -183,8 +182,7 @@
 
 	if (dev->active) {
 		/* parse allocated resource */
-		status = pnpacpi_parse_allocated_resource(device->handle,
-							  &dev->res);
+		status = pnpacpi_parse_allocated_resource(dev);
 		if (ACPI_FAILURE(status) && (status != AE_NOT_FOUND)) {
 			pnp_err("PnPACPI: METHOD_NAME__CRS failure for %s",
 				acpi_device_hid(device));
@@ -192,8 +190,7 @@
 	}
 
 	if (dev->capabilities & PNP_CONFIGURABLE) {
-		status = pnpacpi_parse_resource_option_data(device->handle,
-							    dev);
+		status = pnpacpi_parse_resource_option_data(dev);
 		if (ACPI_FAILURE(status) && (status != AE_NOT_FOUND)) {
 			pnp_err("PnPACPI: METHOD_NAME__PRS failure for %s",
 				acpi_device_hid(device));
drivers/pnp/pnpacpi/pnpacpi.h +3 -3
Index: work7/drivers/pnp/pnpacpi/pnpacpi.h
===================================================================
@@ -5,8 +5,8 @@
 #include <linux/acpi.h>
 #include <linux/pnp.h>
 
-acpi_status pnpacpi_parse_allocated_resource(acpi_handle, struct pnp_resource_table*);
-acpi_status pnpacpi_parse_resource_option_data(acpi_handle, struct pnp_dev*);
-int pnpacpi_encode_resources(struct pnp_resource_table *, struct acpi_buffer *);
+acpi_status pnpacpi_parse_allocated_resource(struct pnp_dev *);
+acpi_status pnpacpi_parse_resource_option_data(struct pnp_dev *);
+int pnpacpi_encode_resources(struct pnp_dev *, struct acpi_buffer *);
 int pnpacpi_build_resource_template(struct pnp_dev *, struct acpi_buffer *);
 #endif
drivers/pnp/pnpacpi/rsparser.c +30 -25

188 lines not on this page · raw message

drivers/pnp/pnpbios/core.c +2 -2
Index: work7/drivers/pnp/pnpbios/core.c
===================================================================
@@ -219,7 +219,7 @@
 		kfree(node);
 		return -ENODEV;
 	}
-	pnpbios_read_resources_from_node(&dev->res, node);
+	pnpbios_read_resources_from_node(dev, node);
 	dev->active = pnp_is_active(dev);
 	kfree(node);
 	return 0;
@@ -241,7 +241,7 @@
 		kfree(node);
 		return -ENODEV;
 	}
-	if (pnpbios_write_resources_to_node(&dev->res, node) < 0) {
+	if (pnpbios_write_resources_to_node(dev, node) < 0) {
 		kfree(node);
 		return -1;
 	}
drivers/pnp/pnpbios/pnpbios.h +2 -2
Index: work7/drivers/pnp/pnpbios/pnpbios.h
===================================================================
@@ -28,8 +28,8 @@
 extern int  pnpbios_dont_use_current_config;
 
 extern int pnpbios_parse_data_stream(struct pnp_dev *dev, struct pnp_bios_node * node);
-extern int pnpbios_read_resources_from_node(struct pnp_resource_table *res, struct pnp_bios_node * node);
-extern int pnpbios_write_resources_to_node(struct pnp_resource_table *res, struct pnp_bios_node * node);
+extern int pnpbios_read_resources_from_node(struct pnp_dev *dev, struct pnp_bios_node *node);
+extern int pnpbios_write_resources_to_node(struct pnp_dev *dev, struct pnp_bios_node *node);
 extern void pnpid32_to_pnpid(u32 id, char *str);
 
 extern void pnpbios_print_status(const char * module, u16 status);
drivers/pnp/pnpbios/rsparser.c +29 -29

176 lines not on this page · raw message

#18 lore raw

PATCH 18/37 PNP: add pnp_init_resources(struct pnp_dev *) interface

In reply to Bjorn Helgaas

Add pnp_init_resources(struct pnp_dev *), which will replace pnp_init_resource_table(), which takes a pointer to the pnp_resource_table itself. Passing only the pnp_dev * reduces the possibility for error in the caller and removes the pnp_resource_table implementation detail from the interface.

Even though pnp_init_resource_table() is exported, I did not export pnp_init_resources() because it is used only by the PNP core.

---
 drivers/pnp/interface.c        |    6 +++---
 drivers/pnp/isapnp/core.c      |    4 ++--
 drivers/pnp/manager.c          |    5 +++++
 drivers/pnp/pnpacpi/core.c     |    2 +-
 drivers/pnp/pnpacpi/rsparser.c |    3 +--
 drivers/pnp/pnpbios/core.c     |    2 +-
 drivers/pnp/pnpbios/rsparser.c |    3 +--
 include/linux/pnp.h            |    2 ++
 8 files changed, 16 insertions(+), 11 deletions(-)
include/linux/pnp.h +2 -0
Index: work7/include/linux/pnp.h
===================================================================
@@ -388,6 +388,7 @@
 			       struct pnp_port *data);
 int pnp_register_mem_resource(struct pnp_option *option, struct pnp_mem *data);
 void pnp_init_resource_table(struct pnp_resource_table *table);
+void pnp_init_resources(struct pnp_dev *dev);
 int pnp_manual_config_dev(struct pnp_dev *dev, struct pnp_resource_table *res,
 			  int mode);
 int pnp_auto_config_dev(struct pnp_dev *dev);
@@ -435,6 +436,7 @@
 static inline int pnp_register_port_resource(struct pnp_option *option, struct pnp_port *data) { return -ENODEV; }
 static inline int pnp_register_mem_resource(struct pnp_option *option, struct pnp_mem *data) { return -ENODEV; }
 static inline void pnp_init_resource_table(struct pnp_resource_table *table) { }
+static inline void pnp_init_resources(struct pnp_dev *dev) { }
 static inline int pnp_manual_config_dev(struct pnp_dev *dev, struct pnp_resource_table *res, int mode) { return -ENODEV; }
 static inline int pnp_auto_config_dev(struct pnp_dev *dev) { return -ENODEV; }
 static inline int pnp_validate_config(struct pnp_dev *dev) { return -ENODEV; }
drivers/pnp/manager.c +5 -0
Index: work7/drivers/pnp/manager.c
===================================================================
@@ -238,6 +238,11 @@
 	}
 }
 
+void pnp_init_resources(struct pnp_dev *dev)
+{
+	pnp_init_resource_table(&dev->res);
+}
+
 /**
  * pnp_clean_resources - clears resources that were not manually set
  * @res: the resources to clean
drivers/pnp/interface.c +3 -3
Index: work7/drivers/pnp/interface.c
===================================================================
@@ -351,14 +351,14 @@
 	if (!strnicmp(buf, "auto", 4)) {
 		if (dev->active)
 			goto done;
-		pnp_init_resource_table(&dev->res);
+		pnp_init_resources(dev);
 		retval = pnp_auto_config_dev(dev);
 		goto done;
 	}
 	if (!strnicmp(buf, "clear", 5)) {
 		if (dev->active)
 			goto done;
-		pnp_init_resource_table(&dev->res);
+		pnp_init_resources(dev);
 		goto done;
 	}
 	if (!strnicmp(buf, "get", 3)) {
@@ -373,7 +373,7 @@
 		if (dev->active)
 			goto done;
 		buf += 3;
-		pnp_init_resource_table(&dev->res);
+		pnp_init_resources(dev);
 		mutex_lock(&pnp_res_mutex);
 		while (1) {
 			while (isspace(*buf))
drivers/pnp/isapnp/core.c +2 -2
Index: work7/drivers/pnp/isapnp/core.c
===================================================================
@@ -438,7 +438,7 @@
 	dev->capabilities |= PNP_READ;
 	dev->capabilities |= PNP_WRITE;
 	dev->capabilities |= PNP_DISABLE;
-	pnp_init_resource_table(&dev->res);
+	pnp_init_resources(dev);
 	return dev;
 }
 
@@ -982,7 +982,7 @@
 {
 	int ret;
 
-	pnp_init_resource_table(&dev->res);
+	pnp_init_resources(dev);
 	isapnp_cfg_begin(dev->card->number, dev->number);
 	ret = isapnp_read_resources(dev);
 	isapnp_cfg_end();
drivers/pnp/pnpacpi/core.c +1 -1
Index: work7/drivers/pnp/pnpacpi/core.c
===================================================================
@@ -210,7 +210,7 @@
 
 	/* clear out the damaged flags */
 	if (!dev->active)
-		pnp_init_resource_table(&dev->res);
+		pnp_init_resources(dev);
 	pnp_add_device(dev);
 	num++;
 
drivers/pnp/pnpbios/core.c +1 -1
Index: work7/drivers/pnp/pnpbios/core.c
===================================================================
@@ -346,7 +346,7 @@
 
 	/* clear out the damaged flags */
 	if (!dev->active)
-		pnp_init_resource_table(&dev->res);
+		pnp_init_resources(dev);
 
 	pnp_add_device(dev);
 	pnpbios_interface_attach_device(node);
drivers/pnp/pnpacpi/rsparser.c +1 -2
Index: work7/drivers/pnp/pnpacpi/rsparser.c
===================================================================
@@ -406,8 +406,7 @@
 {
 	acpi_handle handle = dev->data;
 
-	/* Blank the resource table values */
-	pnp_init_resource_table(&dev->res);
+	pnp_init_resources(dev);
 
 	return acpi_walk_resources(handle, METHOD_NAME__CRS,
 				   pnpacpi_allocated_resource, dev);
drivers/pnp/pnpbios/rsparser.c +1 -2
Index: work7/drivers/pnp/pnpbios/rsparser.c
===================================================================
@@ -143,8 +143,7 @@
 	if (!p)
 		return NULL;
 
-	/* Blank the resource table values */
-	pnp_init_resource_table(&dev->res);
+	pnp_init_resources(dev);
 
 	while ((char *)p < (char *)end) {
 
#19 lore raw

PATCH 19/37 PNP: remove pnp_resource_table from internal pnp_clean_resource_table interface

In reply to Bjorn Helgaas

This changes pnp_clean_resource_table() to take a pnp_dev pointer rather than a pnp_resource_table pointer. This reduces the visibility of pnp_resource_table and removes an opportunity for error in the caller.

drivers/pnp/manager.c +5 -4
Index: work7/drivers/pnp/manager.c
===================================================================
@@ -247,8 +247,9 @@
  * pnp_clean_resources - clears resources that were not manually set
  * @res: the resources to clean
  */
-static void pnp_clean_resource_table(struct pnp_resource_table *res)
+static void pnp_clean_resource_table(struct pnp_dev *dev)
 {
+	struct pnp_resource_table *res = &dev->res;
 	int idx;
 
 	for (idx = 0; idx < PNP_MAX_IRQ; idx++) {
@@ -304,7 +305,7 @@
 		return -ENODEV;
 
 	mutex_lock(&pnp_res_mutex);
-	pnp_clean_resource_table(&dev->res);	/* start with a fresh slate */
+	pnp_clean_resource_table(dev);
 	if (dev->independent) {
 		port = dev->independent->port;
 		mem = dev->independent->mem;
@@ -376,7 +377,7 @@
 	return 1;
 
 fail:
-	pnp_clean_resource_table(&dev->res);
+	pnp_clean_resource_table(dev);
 	mutex_unlock(&pnp_res_mutex);
 	return 0;
 }
@@ -554,7 +555,7 @@
 
 	/* release the resources so that other devices can use them */
 	mutex_lock(&pnp_res_mutex);
-	pnp_clean_resource_table(&dev->res);
+	pnp_clean_resource_table(dev);
 	mutex_unlock(&pnp_res_mutex);
 
 	return 0;
#20 lore raw

PATCH 20/37 PNP: make generic pnp_add_irq_resource()

In reply to Bjorn Helgaas

Add a pnp_add_irq_resource() that can be used by all the PNP backends. This consolidates a little more pnp_resource_table knowledge into one place.

---
 drivers/pnp/base.h             |    2 ++
 drivers/pnp/interface.c        |   14 ++++----------
 drivers/pnp/isapnp/core.c      |    4 +---
 drivers/pnp/pnpacpi/rsparser.c |   28 +++++-----------------------
 drivers/pnp/pnpbios/rsparser.c |   23 ++---------------------
 drivers/pnp/resource.c         |   26 ++++++++++++++++++++++++++
 6 files changed, 40 insertions(+), 57 deletions(-)
drivers/pnp/base.h +2 -0
Index: work7/drivers/pnp/base.h
===================================================================
@@ -14,3 +14,5 @@
 int pnp_check_mem(struct pnp_dev * dev, int idx);
 int pnp_check_irq(struct pnp_dev * dev, int idx);
 int pnp_check_dma(struct pnp_dev * dev, int idx);
+
+int pnp_add_irq_resource(struct pnp_dev *dev, int irq, int flags);
drivers/pnp/resource.c +26 -0
Index: work7/drivers/pnp/resource.c
===================================================================
@@ -461,6 +461,32 @@
 #endif
 }
 
+#define set(flags)	((flags & IORESOURCE_UNSET) == 0)
+
+int pnp_add_irq_resource(struct pnp_dev *dev, int irq, int flags)
+{
+	struct pnp_resource_table *res = &dev->res;
+	int i = 0;
+	static unsigned char warned;
+
+	while (set(res->irq_resource[i].flags) && i < PNP_MAX_IRQ)
+		i++;
+	if (i >= PNP_MAX_IRQ && !warned) {
+		dev_err(&dev->dev, "too many IRQs (max %d)\n", PNP_MAX_IRQ);
+		warned = 1;
+		return -ENOSPC;
+	}
+
+	res->irq_resource[i].flags = IORESOURCE_IRQ | flags;
+	if (irq < 0) {
+		res->irq_resource[i].flags |= IORESOURCE_DISABLED;
+		return -EINVAL;
+	}
+	res->irq_resource[i].start = irq;
+	res->irq_resource[i].end = irq;
+	return 0;
+}
+
 /* format is: pnp_reserve_irq=irq1[,irq2] .... */
 static int __init pnp_setup_reserve_irq(char *str)
 {
drivers/pnp/isapnp/core.c +1 -3
Index: work7/drivers/pnp/isapnp/core.c
===================================================================
@@ -962,9 +962,7 @@
 			     8);
 			if (!ret)
 				continue;
-			res->irq_resource[tmp].start =
-			    res->irq_resource[tmp].end = ret;
-			res->irq_resource[tmp].flags = IORESOURCE_IRQ;
+			pnp_add_irq_resource(dev, ret, 0);
 		}
 		for (tmp = 0; tmp < ISAPNP_MAX_DMA; tmp++) {
 			ret = isapnp_read_byte(ISAPNP_CFG_DMA + tmp);
drivers/pnp/pnpacpi/rsparser.c +5 -23
Index: work7/drivers/pnp/pnpacpi/rsparser.c
===================================================================
@@ -22,6 +22,7 @@
 #include <linux/acpi.h>
 #include <linux/pci.h>
 #include "pnpacpi.h"
+#include "../base.h"
 
 #ifdef CONFIG_IA64
 #define valid_IRQ(i) (1)
@@ -80,24 +81,12 @@
 						u32 gsi, int triggering,
 						int polarity, int shareable)
 {
-	struct pnp_resource_table *res = &dev->res;
-	int i = 0;
-	int irq;
+	int irq, flags;
 	int p, t;
-	static unsigned char warned;
 
 	if (!valid_IRQ(gsi))
 		return;
 
-	while (!(res->irq_resource[i].flags & IORESOURCE_UNSET) &&
-	       i < PNP_MAX_IRQ)
-		i++;
-	if (i >= PNP_MAX_IRQ && !warned) {
-		printk(KERN_ERR "pnpacpi: exceeded the max number of IRQ "
-				"resources: %d \n", PNP_MAX_IRQ);
-		warned = 1;
-		return;
-	}
 	/*
 	 * in IO-APIC mode, use overrided attribute. Two reasons:
 	 * 1. BIOS bug in DSDT
@@ -114,18 +103,11 @@
 			polarity = p;
 		}
 	}
+	flags = irq_flags(triggering, polarity, shareable);
 
-	res->irq_resource[i].flags = IORESOURCE_IRQ;	// Also clears _UNSET flag
-	res->irq_resource[i].flags |= irq_flags(triggering, polarity, shareable);
 	irq = acpi_register_gsi(gsi, triggering, polarity);
-	if (irq < 0) {
-		res->irq_resource[i].flags |= IORESOURCE_DISABLED;
-		return;
-	}
-
-	res->irq_resource[i].start = irq;
-	res->irq_resource[i].end = irq;
-	pcibios_penalize_isa_irq(irq, 1);
+	if (pnp_add_irq_resource(dev, irq, flags) == 0)
+		pcibios_penalize_isa_irq(irq, 1);
 }
 
 static int dma_flags(int type, int bus_master, int transfer)
drivers/pnp/pnpbios/rsparser.c +2 -21
Index: work7/drivers/pnp/pnpbios/rsparser.c
===================================================================
@@ -54,26 +54,6 @@
  * Allocated Resources
  */
 
-static void pnpbios_parse_allocated_irqresource(struct pnp_dev *dev, int irq)
-{
-	struct pnp_resource_table *res = &dev->res;
-	int i = 0;
-
-	while (!(res->irq_resource[i].flags & IORESOURCE_UNSET)
-	       && i < PNP_MAX_IRQ)
-		i++;
-	if (i < PNP_MAX_IRQ) {
-		res->irq_resource[i].flags = IORESOURCE_IRQ;	// Also clears _UNSET flag
-		if (irq == -1) {
-			res->irq_resource[i].flags |= IORESOURCE_DISABLED;
-			return;
-		}
-		res->irq_resource[i].start =
-		    res->irq_resource[i].end = (unsigned long)irq;
-		pcibios_penalize_isa_irq(irq, 1);
-	}
-}
-
 static void pnpbios_parse_allocated_dmaresource(struct pnp_dev *dev, int dma)
 {
 	struct pnp_resource_table *res = &dev->res;
@@ -198,7 +178,8 @@
 			for (i = 0; i < 16; i++, mask = mask >> 1)
 				if (mask & 0x01)
 					io = i;
-			pnpbios_parse_allocated_irqresource(dev, io);
+			if (pnp_add_irq_resource(dev, io, 0) == 0)
+				pcibios_penalize_isa_irq(io, 1);
 			break;
 
 		case SMALL_TAG_DMA:
drivers/pnp/interface.c +4 -9
Index: work7/drivers/pnp/interface.c
===================================================================
@@ -325,6 +325,7 @@
 	struct pnp_dev *dev = to_pnp_dev(dmdev);
 	char *buf = (void *)ubuf;
 	int retval = 0;
+	resource_size_t start;
 
 	if (dev->status & PNP_ATTACHED) {
 		retval = -EBUSY;
@@ -369,7 +370,7 @@
 		goto done;
 	}
 	if (!strnicmp(buf, "set", 3)) {
-		int nport = 0, nmem = 0, nirq = 0, ndma = 0;
+		int nport = 0, nmem = 0, ndma = 0;
 		if (dev->active)
 			goto done;
 		buf += 3;
@@ -430,14 +431,8 @@
 				buf += 3;
 				while (isspace(*buf))
 					++buf;
-				dev->res.irq_resource[nirq].start =
-				    dev->res.irq_resource[nirq].end =
-				    simple_strtoul(buf, &buf, 0);
-				dev->res.irq_resource[nirq].flags =
-				    IORESOURCE_IRQ;
-				nirq++;
-				if (nirq >= PNP_MAX_IRQ)
-					break;
+				start = simple_strtoul(buf, &buf, 0);
+				pnp_add_irq_resource(dev, start, 0);
 				continue;
 			}
 			if (!strnicmp(buf, "dma", 3)) {
#21 lore raw

PATCH 21/37 PNP: make generic pnp_add_dma_resource()

In reply to Bjorn Helgaas

Add a pnp_add_dma_resource() that can be used by all the PNP backends. This consolidates a little more pnp_resource_table knowledge into one place.

---
 drivers/pnp/base.h             |    1 +
 drivers/pnp/isapnp/core.c      |    4 +---
 drivers/pnp/pnpacpi/rsparser.c |   34 ++++------------------------------
 drivers/pnp/pnpbios/rsparser.c |   21 +--------------------
 drivers/pnp/resource.c         |   24 ++++++++++++++++++++++++
 5 files changed, 31 insertions(+), 53 deletions(-)
drivers/pnp/base.h +1 -0
Index: work7/drivers/pnp/base.h
===================================================================
@@ -16,3 +16,4 @@
 int pnp_check_dma(struct pnp_dev * dev, int idx);
 
 int pnp_add_irq_resource(struct pnp_dev *dev, int irq, int flags);
+int pnp_add_dma_resource(struct pnp_dev *dev, int dma, int flags);
drivers/pnp/resource.c +24 -0
Index: work7/drivers/pnp/resource.c
===================================================================
@@ -487,6 +487,30 @@
 	return 0;
 }
 
+int pnp_add_dma_resource(struct pnp_dev *dev, int dma, int flags)
+{
+	struct pnp_resource_table *res = &dev->res;
+	int i = 0;
+	static unsigned char warned;
+
+	while (set(res->dma_resource[i].flags) && i < PNP_MAX_DMA)
+		i++;
+	if (i >= PNP_MAX_DMA && !warned) {
+		dev_err(&dev->dev, "too many DMAs (max %d)\n", PNP_MAX_DMA);
+		warned = 1;
+		return -ENOSPC;
+	}
+
+	res->dma_resource[i].flags = IORESOURCE_DMA | flags;
+	if (dma < 0) {
+		res->dma_resource[i].flags |= IORESOURCE_DISABLED;
+		return -EINVAL;
+	}
+	res->dma_resource[i].start = dma;
+	res->dma_resource[i].end = dma;
+	return 0;
+}
+
 /* format is: pnp_reserve_irq=irq1[,irq2] .... */
 static int __init pnp_setup_reserve_irq(char *str)
 {
drivers/pnp/isapnp/core.c +1 -3
Index: work7/drivers/pnp/isapnp/core.c
===================================================================
@@ -968,9 +968,7 @@
 			ret = isapnp_read_byte(ISAPNP_CFG_DMA + tmp);
 			if (ret == 4)
 				continue;
-			res->dma_resource[tmp].start =
-			    res->dma_resource[tmp].end = ret;
-			res->dma_resource[tmp].flags = IORESOURCE_DMA;
+			pnp_add_dma_resource(dev, ret, 0);
 		}
 	}
 	return 0;
drivers/pnp/pnpacpi/rsparser.c +4 -30
Index: work7/drivers/pnp/pnpacpi/rsparser.c
===================================================================
@@ -153,32 +153,6 @@
 	return flags;
 }
 
-static void pnpacpi_parse_allocated_dmaresource(struct pnp_dev *dev,
-						u32 dma, int flags)
-{
-	struct pnp_resource_table *res = &dev->res;
-	int i = 0;
-	static unsigned char warned;
-
-	while (i < PNP_MAX_DMA &&
-	       !(res->dma_resource[i].flags & IORESOURCE_UNSET))
-		i++;
-	if (i < PNP_MAX_DMA) {
-		res->dma_resource[i].flags = IORESOURCE_DMA;	// Also clears _UNSET flag
-		res->dma_resource[i].flags |= flags;
-		if (dma == -1) {
-			res->dma_resource[i].flags |= IORESOURCE_DISABLED;
-			return;
-		}
-		res->dma_resource[i].start = dma;
-		res->dma_resource[i].end = dma;
-	} else if (!warned) {
-		printk(KERN_ERR "pnpacpi: exceeded the max number of DMA "
-				"resources: %d \n", PNP_MAX_DMA);
-		warned = 1;
-	}
-}
-
 static void pnpacpi_parse_allocated_ioresource(struct pnp_dev *dev,
 					       u64 io, u64 len, int io_decode)
 {
@@ -295,10 +269,10 @@
 	case ACPI_RESOURCE_TYPE_DMA:
 		dma = &res->data.dma;
 		if (dma->channel_count > 0)
-			pnpacpi_parse_allocated_dmaresource(dev,
-				dma->channels[0],
-				dma_flags(dma->type, dma->bus_master,
-					  dma->transfer));
+			pnp_add_dma_resource(dev, dma->channels[0],
+					     dma_flags(dma->type,
+						       dma->bus_master,
+						       dma->transfer));
 		break;
 
 	case ACPI_RESOURCE_TYPE_IO:
drivers/pnp/pnpbios/rsparser.c +1 -20
Index: work7/drivers/pnp/pnpbios/rsparser.c
===================================================================
@@ -54,25 +54,6 @@
  * Allocated Resources
  */
 
-static void pnpbios_parse_allocated_dmaresource(struct pnp_dev *dev, int dma)
-{
-	struct pnp_resource_table *res = &dev->res;
-	int i = 0;
-
-	while (i < PNP_MAX_DMA &&
-	       !(res->dma_resource[i].flags & IORESOURCE_UNSET))
-		i++;
-	if (i < PNP_MAX_DMA) {
-		res->dma_resource[i].flags = IORESOURCE_DMA;	// Also clears _UNSET flag
-		if (dma == -1) {
-			res->dma_resource[i].flags |= IORESOURCE_DISABLED;
-			return;
-		}
-		res->dma_resource[i].start =
-		    res->dma_resource[i].end = (unsigned long)dma;
-	}
-}
-
 static void pnpbios_parse_allocated_ioresource(struct pnp_dev *dev,
 					       int io, int len)
 {
@@ -190,7 +171,7 @@
 			for (i = 0; i < 8; i++, mask = mask >> 1)
 				if (mask & 0x01)
 					io = i;
-			pnpbios_parse_allocated_dmaresource(dev, io);
+			pnp_add_dma_resource(dev, io, 0);
 			break;
 
 		case SMALL_TAG_PORT:
drivers/pnp/interface.c +3 -9
Index: work7/drivers/pnp/interface.c
===================================================================
@@ -370,7 +370,7 @@
 		goto done;
 	}
 	if (!strnicmp(buf, "set", 3)) {
-		int nport = 0, nmem = 0, ndma = 0;
+		int nport = 0, nmem = 0;
 		if (dev->active)
 			goto done;
 		buf += 3;
@@ -439,14 +439,8 @@
 				buf += 3;
 				while (isspace(*buf))
 					++buf;
-				dev->res.dma_resource[ndma].start =
-				    dev->res.dma_resource[ndma].end =
-				    simple_strtoul(buf, &buf, 0);
-				dev->res.dma_resource[ndma].flags =
-				    IORESOURCE_DMA;
-				ndma++;
-				if (ndma >= PNP_MAX_DMA)
-					break;
+				start = simple_strtoul(buf, &buf, 0);
+				pnp_add_dma_resource(dev, start, 0);
 				continue;
 			}
 			break;
#22 lore raw

PATCH 22/37 PNP: make generic pnp_add_io_resource()

In reply to Bjorn Helgaas

Add a pnp_add_io_resource() that can be used by all the PNP backends. This consolidates a little more pnp_resource_table knowledge into one place.

---
 drivers/pnp/base.h             |    1 
 drivers/pnp/interface.c        |   20 ++++++------------
 drivers/pnp/isapnp/core.c      |    3 --
 drivers/pnp/pnpacpi/rsparser.c |   45 ++++++-----------------------------------
 drivers/pnp/pnpbios/rsparser.c |   24 +--------------------
 drivers/pnp/resource.c         |   24 +++++++++++++++++++++
 6 files changed, 42 insertions(+), 75 deletions(-)
drivers/pnp/base.h +1 -0
Index: work7/drivers/pnp/base.h
===================================================================
@@ -17,3 +17,4 @@
 
 int pnp_add_irq_resource(struct pnp_dev *dev, int irq, int flags);
 int pnp_add_dma_resource(struct pnp_dev *dev, int dma, int flags);
+int pnp_add_io_resource(struct pnp_dev *dev, resource_size_t start, resource_size_t len, int flags);
drivers/pnp/resource.c +25 -0
Index: work7/drivers/pnp/resource.c
===================================================================
@@ -511,6 +511,31 @@
 	return 0;
 }
 
+int pnp_add_io_resource(struct pnp_dev *dev, resource_size_t start, resource_size_t len, int flags)
+{
+	struct pnp_resource_table *res = &dev->res;
+	resource_size_t end = start + len - 1;
+	int i = 0;
+	static unsigned char warned;
+
+	while (set(res->port_resource[i].flags) && i < PNP_MAX_PORT)
+		i++;
+	if (i >= PNP_MAX_PORT && !warned) {
+		dev_err(&dev->dev, "too many PORTs (max %d)\n", PNP_MAX_PORT);
+		warned = 1;
+		return -ENOSPC;
+	}
+
+	res->port_resource[i].flags = IORESOURCE_IO | flags;
+	if (len <= 0 || end >= 0x10003) {
+		res->port_resource[i].flags |= IORESOURCE_DISABLED;
+		return -EINVAL;
+	}
+	res->port_resource[i].start = start;
+	res->port_resource[i].end = end;
+	return 0;
+}
+
 /* format is: pnp_reserve_irq=irq1[,irq2] .... */
 static int __init pnp_setup_reserve_irq(char *str)
 {
drivers/pnp/isapnp/core.c +1 -2
Index: work7/drivers/pnp/isapnp/core.c
===================================================================
@@ -945,8 +945,7 @@
 			ret = isapnp_read_word(ISAPNP_CFG_PORT + (tmp << 1));
 			if (!ret)
 				continue;
-			res->port_resource[tmp].start = ret;
-			res->port_resource[tmp].flags = IORESOURCE_IO;
+			pnp_add_io_resource(dev, ret, 1, 0);
 		}
 		for (tmp = 0; tmp < ISAPNP_MAX_MEM; tmp++) {
 			ret =
drivers/pnp/pnpacpi/rsparser.c +7 -38

70 lines not on this page · raw message

drivers/pnp/pnpbios/rsparser.c +2 -22
Index: work7/drivers/pnp/pnpbios/rsparser.c
===================================================================
@@ -54,26 +54,6 @@
  * Allocated Resources
  */
 
-static void pnpbios_parse_allocated_ioresource(struct pnp_dev *dev,
-					       int io, int len)
-{
-	struct pnp_resource_table *res = &dev->res;
-	int i = 0;
-
-	while (!(res->port_resource[i].flags & IORESOURCE_UNSET)
-	       && i < PNP_MAX_PORT)
-		i++;
-	if (i < PNP_MAX_PORT) {
-		res->port_resource[i].flags = IORESOURCE_IO;	// Also clears _UNSET flag
-		if (len <= 0 || (io + len - 1) >= 0x10003) {
-			res->port_resource[i].flags |= IORESOURCE_DISABLED;
-			return;
-		}
-		res->port_resource[i].start = (unsigned long)io;
-		res->port_resource[i].end = (unsigned long)(io + len - 1);
-	}
-}
-
 static void pnpbios_parse_allocated_memresource(struct pnp_dev *dev,
 						int mem, int len)
 {
@@ -179,7 +159,7 @@
 				goto len_err;
 			io = p[2] + p[3] * 256;
 			size = p[7];
-			pnpbios_parse_allocated_ioresource(dev, io, size);
+			pnp_add_io_resource(dev, io, size, 0);
 			break;
 
 		case SMALL_TAG_VENDOR:
@@ -191,7 +171,7 @@
 				goto len_err;
 			io = p[1] + p[2] * 256;
 			size = p[3];
-			pnpbios_parse_allocated_ioresource(dev, io, size);
+			pnp_add_io_resource(dev, io, size, 0);
 			break;
 
 		case SMALL_TAG_END:
drivers/pnp/interface.c +7 -13
Index: work7/drivers/pnp/interface.c
===================================================================
@@ -325,7 +325,7 @@
 	struct pnp_dev *dev = to_pnp_dev(dmdev);
 	char *buf = (void *)ubuf;
 	int retval = 0;
-	resource_size_t start;
+	resource_size_t start, end, length;
 
 	if (dev->status & PNP_ATTACHED) {
 		retval = -EBUSY;
@@ -370,7 +370,7 @@
 		goto done;
 	}
 	if (!strnicmp(buf, "set", 3)) {
-		int nport = 0, nmem = 0;
+		int nmem = 0;
 		if (dev->active)
 			goto done;
 		buf += 3;
@@ -383,24 +383,18 @@
 				buf += 2;
 				while (isspace(*buf))
 					++buf;
-				dev->res.port_resource[nport].start =
-				    simple_strtoul(buf, &buf, 0);
+				start = simple_strtoul(buf, &buf, 0);
 				while (isspace(*buf))
 					++buf;
 				if (*buf == '-') {
 					buf += 1;
 					while (isspace(*buf))
 						++buf;
-					dev->res.port_resource[nport].end =
-					    simple_strtoul(buf, &buf, 0);
+					end = simple_strtoul(buf, &buf, 0);
 				} else
-					dev->res.port_resource[nport].end =
-					    dev->res.port_resource[nport].start;
-				dev->res.port_resource[nport].flags =
-				    IORESOURCE_IO;
-				nport++;
-				if (nport >= PNP_MAX_PORT)
-					break;
+					end = start;
+				length = end - start + 1;
+				pnp_add_io_resource(dev, start, length, 0);
 				continue;
 			}
 			if (!strnicmp(buf, "mem", 3)) {
#23 lore raw

PATCH 23/37 PNP: make generic pnp_add_mem_resource()

In reply to Bjorn Helgaas

Add a pnp_add_mem_resource() that can be used by all the PNP backends. This consolidates a little more pnp_resource_table knowledge into one place.

---
 drivers/pnp/base.h             |    1 
 drivers/pnp/interface.c        |   17 ++++---------
 drivers/pnp/isapnp/core.c      |    4 ---
 drivers/pnp/pnpacpi/rsparser.c |   50 +++++++++--------------------------------
 drivers/pnp/pnpbios/rsparser.c |   26 ++-------------------
 drivers/pnp/resource.c         |   24 +++++++++++++++++++
 6 files changed, 46 insertions(+), 76 deletions(-)
drivers/pnp/base.h +1 -0
Index: work7/drivers/pnp/base.h
===================================================================
@@ -18,3 +18,4 @@
 int pnp_add_irq_resource(struct pnp_dev *dev, int irq, int flags);
 int pnp_add_dma_resource(struct pnp_dev *dev, int dma, int flags);
 int pnp_add_io_resource(struct pnp_dev *dev, resource_size_t start, resource_size_t len, int flags);
+int pnp_add_mem_resource(struct pnp_dev *dev, resource_size_t start, resource_size_t len, int flags);
drivers/pnp/resource.c +24 -0
Index: work7/drivers/pnp/resource.c
===================================================================
@@ -536,6 +536,30 @@
 	return 0;
 }
 
+int pnp_add_mem_resource(struct pnp_dev *dev, resource_size_t start, resource_size_t len, int flags)
+{
+	struct pnp_resource_table *res = &dev->res;
+	int i = 0;
+	static unsigned char warned;
+
+	while (set(res->mem_resource[i].flags) && i < PNP_MAX_MEM)
+		i++;
+	if (i >= PNP_MAX_MEM && !warned) {
+		dev_err(&dev->dev, "too many MEMs (max %d)\n", PNP_MAX_MEM);
+		warned = 1;
+		return -ENOSPC;
+	}
+
+	res->mem_resource[i].flags = IORESOURCE_MEM | flags;
+	if (len <= 0) {
+		res->mem_resource[i].flags |= IORESOURCE_DISABLED;
+		return -EINVAL;
+	}
+	res->mem_resource[i].start = start;
+	res->mem_resource[i].end = start + len - 1;
+	return 0;
+}
+
 /* format is: pnp_reserve_irq=irq1[,irq2] .... */
 static int __init pnp_setup_reserve_irq(char *str)
 {
drivers/pnp/isapnp/core.c +1 -3
Index: work7/drivers/pnp/isapnp/core.c
===================================================================
@@ -936,7 +936,6 @@
 
 static int isapnp_read_resources(struct pnp_dev *dev)
 {
-	struct pnp_resource_table *res = &dev->res;
 	int tmp, ret;
 
 	dev->active = isapnp_read_byte(ISAPNP_CFG_ACTIVATE);
@@ -952,8 +951,7 @@
 			    isapnp_read_word(ISAPNP_CFG_MEM + (tmp << 3)) << 8;
 			if (!ret)
 				continue;
-			res->mem_resource[tmp].start = ret;
-			res->mem_resource[tmp].flags = IORESOURCE_MEM;
+			pnp_add_mem_resource(dev, ret, 1, 0);
 		}
 		for (tmp = 0; tmp < ISAPNP_MAX_IRQ; tmp++) {
 			ret =
drivers/pnp/pnpacpi/rsparser.c +12 -38

80 lines not on this page · raw message

drivers/pnp/pnpbios/rsparser.c +3 -23
Index: work7/drivers/pnp/pnpbios/rsparser.c
===================================================================
@@ -54,26 +54,6 @@
  * Allocated Resources
  */
 
-static void pnpbios_parse_allocated_memresource(struct pnp_dev *dev,
-						int mem, int len)
-{
-	struct pnp_resource_table *res = &dev->res;
-	int i = 0;
-
-	while (!(res->mem_resource[i].flags & IORESOURCE_UNSET)
-	       && i < PNP_MAX_MEM)
-		i++;
-	if (i < PNP_MAX_MEM) {
-		res->mem_resource[i].flags = IORESOURCE_MEM;	// Also clears _UNSET flag
-		if (len <= 0) {
-			res->mem_resource[i].flags |= IORESOURCE_DISABLED;
-			return;
-		}
-		res->mem_resource[i].start = (unsigned long)mem;
-		res->mem_resource[i].end = (unsigned long)(mem + len - 1);
-	}
-}
-
 static unsigned char *pnpbios_parse_allocated_resource_data(struct pnp_dev *dev,
 							    unsigned char *p,
 							    unsigned char *end)
@@ -104,7 +84,7 @@
 				goto len_err;
 			io = *(short *)&p[4];
 			size = *(short *)&p[10];
-			pnpbios_parse_allocated_memresource(dev, io, size);
+			pnp_add_mem_resource(dev, io, size, 0);
 			break;
 
 		case LARGE_TAG_ANSISTR:
@@ -120,7 +100,7 @@
 				goto len_err;
 			io = *(int *)&p[4];
 			size = *(int *)&p[16];
-			pnpbios_parse_allocated_memresource(dev, io, size);
+			pnp_add_mem_resource(dev, io, size, 0);
 			break;
 
 		case LARGE_TAG_FIXEDMEM32:
@@ -128,7 +108,7 @@
 				goto len_err;
 			io = *(int *)&p[4];
 			size = *(int *)&p[8];
-			pnpbios_parse_allocated_memresource(dev, io, size);
+			pnp_add_mem_resource(dev, io, size, 0);
 			break;
 
 		case SMALL_TAG_IRQ:
drivers/pnp/interface.c +5 -12
Index: work7/drivers/pnp/interface.c
===================================================================
@@ -370,7 +370,6 @@
 		goto done;
 	}
 	if (!strnicmp(buf, "set", 3)) {
-		int nmem = 0;
 		if (dev->active)
 			goto done;
 		buf += 3;
@@ -401,24 +400,18 @@
 				buf += 3;
 				while (isspace(*buf))
 					++buf;
-				dev->res.mem_resource[nmem].start =
-				    simple_strtoul(buf, &buf, 0);
+				start = simple_strtoul(buf, &buf, 0);
 				while (isspace(*buf))
 					++buf;
 				if (*buf == '-') {
 					buf += 1;
 					while (isspace(*buf))
 						++buf;
-					dev->res.mem_resource[nmem].end =
-					    simple_strtoul(buf, &buf, 0);
+					end = simple_strtoul(buf, &buf, 0);
 				} else
-					dev->res.mem_resource[nmem].end =
-					    dev->res.mem_resource[nmem].start;
-				dev->res.mem_resource[nmem].flags =
-				    IORESOURCE_MEM;
-				nmem++;
-				if (nmem >= PNP_MAX_MEM)
-					break;
+					end = start;
+				length = end - start + 1;
+				pnp_add_mem_resource(dev, start, length, 0);
 				continue;
 			}
 			if (!strnicmp(buf, "irq", 3)) {
#24 lore raw

PATCH 24/37 PNP: use dev_printk when possible

In reply to Bjorn Helgaas

Use dev_printk() when possible for more informative error messages.

---
 drivers/pnp/isapnp/core.c      |   38 ++++++++++++++++++--------------------
 drivers/pnp/pnpacpi/rsparser.c |   20 ++++++++++++--------
 drivers/pnp/pnpbios/rsparser.c |   36 ++++++++++++++----------------------
 3 files changed, 44 insertions(+), 50 deletions(-)
drivers/pnp/isapnp/core.c +18 -20
Index: work7/drivers/pnp/isapnp/core.c
===================================================================
@@ -741,9 +741,8 @@
 				isapnp_skip_bytes(size);
 			return 1;
 		default:
-			printk(KERN_ERR
-			       "isapnp: unexpected or unknown tag type 0x%x for logical device %i (device %i), ignored\n",
-			       type, dev->number, card->number);
+			dev_err(&dev->dev, "unknown tag 0x%x (card %i), "
+				"ignored\n", type, card->number);
 		}
 __skip:
 		if (size > 0)
@@ -796,9 +795,8 @@
 				isapnp_skip_bytes(size);
 			return;
 		default:
-			printk(KERN_ERR
-			       "isapnp: unexpected or unknown tag type 0x%x for device %i, ignored\n",
-			       type, card->number);
+			dev_err(&card->dev, "unknown tag 0x%x, ignored\n",
+			       type);
 		}
 __skip:
 		if (size > 0)
@@ -844,19 +842,20 @@
 		isapnp_wake(csn);
 		isapnp_peek(header, 9);
 		checksum = isapnp_checksum(header);
-#if 0
-		printk(KERN_DEBUG
-		       "vendor: %02x:%02x:%02x:%02x:%02x:%02x:%02x:%02x:%02x\n",
-		       header[0], header[1], header[2], header[3], header[4],
-		       header[5], header[6], header[7], header[8]);
-		printk(KERN_DEBUG "checksum = 0x%x\n", checksum);
-#endif
 		isapnp_to_pnpid((header[1] << 8) | header[0],
 				(header[3] << 8) | header[2], id);
 		card = pnp_alloc_card(&isapnp_protocol, csn, id);
 		if (!card)
 			continue;
 
+#if 0
+		dev_info(&card->dev,
+		       "vendor: %02x:%02x:%02x:%02x:%02x:%02x:%02x:%02x:%02x\n",
+		       header[0], header[1], header[2], header[3], header[4],
+		       header[5], header[6], header[7], header[8]);
+		dev_info(&card->dev, "checksum = 0x%x\n", checksum);
+#endif
+
 		INIT_LIST_HEAD(&card->devices);
 		card->serial =
 		    (header[7] << 24) | (header[6] << 16) | (header[5] << 8) |
@@ -864,9 +863,8 @@
 		isapnp_checksum_value = 0x00;
 		isapnp_parse_resource_map(card);
 		if (isapnp_checksum_value != 0x00)
-			printk(KERN_ERR
-			       "isapnp: checksum for device %i is not valid (0x%x)\n",
-			       csn, isapnp_checksum_value);
+			dev_err(&card->dev, "invalid checksum 0x%x\n",
+				isapnp_checksum_value);
 		card->checksum = isapnp_checksum_value;
 
 		pnp_add_card(card);
@@ -1117,13 +1115,13 @@
 	protocol_for_each_card(&isapnp_protocol, card) {
 		cards++;
 		if (isapnp_verbose) {
-			printk(KERN_INFO "isapnp: Card '%s'\n",
-			       card->name[0] ? card->name : "Unknown");
+			dev_info(&card->dev, "card '%s'\n",
+			       card->name[0] ? card->name : "unknown");
 			if (isapnp_verbose < 2)
 				continue;
 			card_for_each_dev(card, dev) {
-				printk(KERN_INFO "isapnp:   Device '%s'\n",
-				       dev->name[0] ? dev->name : "Unknown");
+				dev_info(&card->dev, "device '%s'\n",
+				       dev->name[0] ? dev->name : "unknown");
 			}
 		}
 	}
drivers/pnp/pnpacpi/rsparser.c +12 -8
Index: work7/drivers/pnp/pnpacpi/rsparser.c
===================================================================
@@ -97,7 +97,7 @@
 		p = p ? ACPI_ACTIVE_LOW : ACPI_ACTIVE_HIGH;
 
 		if (triggering != t || polarity != p) {
-			pnp_warn("IRQ %d override to %s, %s",
+			dev_warn(&dev->dev, "IRQ %d override to %s, %s\n",
 				gsi, t ? "edge":"level", p ? "low":"high");
 			triggering = t;
 			polarity = p;
@@ -161,7 +161,7 @@
 
 	status = acpi_resource_to_address64(res, p);
 	if (!ACPI_SUCCESS(status)) {
-		pnp_warn("PnPACPI: failed to convert resource type %d",
+		dev_warn(&dev->dev, "failed to convert resource type %d\n",
 			 res->type);
 		return;
 	}
@@ -294,7 +294,8 @@
 		break;
 
 	default:
-		pnp_warn("PnPACPI: unknown resource type %d", res->type);
+		dev_warn(&dev->dev, "unknown resource type %d in _CRS\n",
+			 res->type);
 		return AE_ERROR;
 	}
 
@@ -560,7 +561,8 @@
 	case ACPI_RESOURCE_TYPE_END_DEPENDENT:
 		/*only one EndDependentFn is allowed */
 		if (!parse_data->option_independent) {
-			pnp_warn("PnPACPI: more than one EndDependentFn");
+			dev_warn(&dev->dev, "more than one EndDependentFn "
+				 "in _PRS\n");
 			return AE_ERROR;
 		}
 		parse_data->option = parse_data->option_independent;
@@ -609,7 +611,8 @@
 		break;
 
 	default:
-		pnp_warn("PnPACPI: unknown resource type %d", res->type);
+		dev_warn(&dev->dev, "unknown resource type %d in _PRS\n",
+			 res->type);
 		return AE_ERROR;
 	}
 
@@ -689,7 +692,7 @@
 	status = acpi_walk_resources(handle, METHOD_NAME__CRS,
 				     pnpacpi_count_resources, &res_cnt);
 	if (ACPI_FAILURE(status)) {
-		pnp_err("Evaluate _CRS failed");
+		dev_err(&dev->dev, "can't evaluate _CRS\n");
 		return -EINVAL;
 	}
 	if (!res_cnt)
@@ -704,7 +707,7 @@
 				     pnpacpi_type_resources, &resource);
 	if (ACPI_FAILURE(status)) {
 		kfree(buffer->pointer);
-		pnp_err("Evaluate _CRS failed");
+		dev_err(&dev->dev, "can't evaluate _CRS\n");
 		return -EINVAL;
 	}
 	/* resource will pointer the end resource now */
@@ -921,7 +924,8 @@
 		case ACPI_RESOURCE_TYPE_EXTENDED_ADDRESS64:
 		case ACPI_RESOURCE_TYPE_GENERIC_REGISTER:
 		default:	/* other type */
-			pnp_warn("unknown resource type %d", resource->type);
+			dev_warn(&dev->dev, "can't encode unknown resource "
+				 "type %d\n", resource->type);
 			return -EINVAL;
 		}
 		resource++;
drivers/pnp/pnpbios/rsparser.c +14 -22
Index: work7/drivers/pnp/pnpbios/rsparser.c
===================================================================
@@ -161,9 +161,8 @@
 
 		default:	/* an unkown tag */
 len_err:
-			printk(KERN_ERR
-			       "PnPBIOS: Unknown tag '0x%x', length '%d'.\n",
-			       tag, len);
+			dev_err(&dev->dev, "unknown tag 0x%x length %d\n",
+				tag, len);
 			break;
 		}
 
@@ -174,8 +173,7 @@
 			p += len + 1;
 	}
 
-	printk(KERN_ERR
-	       "PnPBIOS: Resource structure does not contain an end tag.\n");
+	dev_err(&dev->dev, "no end tag in resource structure\n");
 
 	return NULL;
 }
@@ -382,8 +380,8 @@
 			if (len != 0)
 				goto len_err;
 			if (option_independent == option)
-				printk(KERN_WARNING
-				       "PnPBIOS: Missing SMALL_TAG_STARTDEP tag\n");
+				dev_warn(&dev->dev, "missing "
+					 "SMALL_TAG_STARTDEP tag\n");
 			option = option_independent;
 			break;
 
@@ -392,9 +390,8 @@
 
 		default:	/* an unkown tag */
 len_err:
-			printk(KERN_ERR
-			       "PnPBIOS: Unknown tag '0x%x', length '%d'.\n",
-			       tag, len);
+			dev_err(&dev->dev, "unknown tag 0x%x length %d\n",
+				tag, len);
 			break;
 		}
 
@@ -405,8 +402,7 @@
 			p += len + 1;
 	}
 
-	printk(KERN_ERR
-	       "PnPBIOS: Resource structure does not contain an end tag.\n");
+	dev_err(&dev->dev, "no end tag in resource structure\n");
 
 	return NULL;
 }
@@ -484,9 +480,8 @@
 
 		default:	/* an unkown tag */
 len_err:
-			printk(KERN_ERR
-			       "PnPBIOS: Unknown tag '0x%x', length '%d'.\n",
-			       tag, len);
+			dev_err(&dev->dev, "unknown tag 0x%x length %d\n",
+				tag, len);
 			break;
 		}
 
@@ -497,8 +492,7 @@
 			p += len + 1;
 	}
 
-	printk(KERN_ERR
-	       "PnPBIOS: Resource structure does not contain an end tag.\n");
+	dev_err(&dev->dev, "no end tag in resource structure\n");
 
 	return NULL;
 }
@@ -678,9 +672,8 @@
 
 		default:	/* an unkown tag */
 len_err:
-			printk(KERN_ERR
-			       "PnPBIOS: Unknown tag '0x%x', length '%d'.\n",
-			       tag, len);
+			dev_err(&dev->dev, "unknown tag 0x%x length %d\n",
+				tag, len);
 			break;
 		}
 
@@ -691,8 +684,7 @@
 			p += len + 1;
 	}
 
-	printk(KERN_ERR
-	       "PnPBIOS: Resource structure does not contain an end tag.\n");
+	dev_err(&dev->dev, "no end tag in resource structure\n");
 
 	return NULL;
 }
#25 lore raw

PATCH 25/37 PNPACPI: remove redundant warnings about _CRS/_PRS failures

In reply to Bjorn Helgaas

Any errors should already be reported inside
pnpacpi_parse_allocated_resource() and
pnpacpi_parse_resource_option_data(), so no
need to report them again.

drivers/pnp/pnpacpi/core.c +4 -15
Index: work7/drivers/pnp/pnpacpi/core.c
===================================================================
@@ -180,22 +180,11 @@
 	else
 		strncpy(dev->name, acpi_device_bid(device), sizeof(dev->name));
 
-	if (dev->active) {
-		/* parse allocated resource */
-		status = pnpacpi_parse_allocated_resource(dev);
-		if (ACPI_FAILURE(status) && (status != AE_NOT_FOUND)) {
-			pnp_err("PnPACPI: METHOD_NAME__CRS failure for %s",
-				acpi_device_hid(device));
-		}
-	}
+	if (dev->active)
+		pnpacpi_parse_allocated_resource(dev);
 
-	if (dev->capabilities & PNP_CONFIGURABLE) {
-		status = pnpacpi_parse_resource_option_data(dev);
-		if (ACPI_FAILURE(status) && (status != AE_NOT_FOUND)) {
-			pnp_err("PnPACPI: METHOD_NAME__PRS failure for %s",
-				acpi_device_hid(device));
-		}
-	}
+	if (dev->capabilities & PNP_CONFIGURABLE)
+		pnpacpi_parse_resource_option_data(dev);
 
 	if (device->flags.compatible_ids) {
 		struct acpi_compatible_id_list *cid_list = device->pnp.cid_list;
#26 lore raw

PATCH 26/37 PNPACPI: remove some pnp_dbg calls

In reply to Bjorn Helgaas

These are turned off by default and don't print much useful information anyway. Anybody debugging this area will likely need to add his or her own printks anyway.

drivers/pnp/pnpacpi/rsparser.c +0 -10
Index: work7/drivers/pnp/pnpacpi/rsparser.c
===================================================================
@@ -701,7 +701,6 @@
 	buffer->pointer = kzalloc(buffer->length - 1, GFP_KERNEL);
 	if (!buffer->pointer)
 		return -ENOMEM;
-	pnp_dbg("Res cnt %d", res_cnt);
 	resource = (struct acpi_resource *)buffer->pointer;
 	status = acpi_walk_resources(handle, METHOD_NAME__CRS,
 				     pnpacpi_type_resources, &resource);
@@ -860,56 +859,47 @@
 	struct acpi_resource *resource = buffer->pointer;
 	int port = 0, irq = 0, dma = 0, mem = 0;
 
-	pnp_dbg("res cnt %d", res_cnt);
 	while (i < res_cnt) {
 		switch (resource->type) {
 		case ACPI_RESOURCE_TYPE_IRQ:
-			pnp_dbg("Encode irq");
 			pnpacpi_encode_irq(resource,
 					   &res_table->irq_resource[irq]);
 			irq++;
 			break;
 
 		case ACPI_RESOURCE_TYPE_DMA:
-			pnp_dbg("Encode dma");
 			pnpacpi_encode_dma(resource,
 					   &res_table->dma_resource[dma]);
 			dma++;
 			break;
 		case ACPI_RESOURCE_TYPE_IO:
-			pnp_dbg("Encode io");
 			pnpacpi_encode_io(resource,
 					  &res_table->port_resource[port]);
 			port++;
 			break;
 		case ACPI_RESOURCE_TYPE_FIXED_IO:
-			pnp_dbg("Encode fixed io");
 			pnpacpi_encode_fixed_io(resource,
 						&res_table->
 						port_resource[port]);
 			port++;
 			break;
 		case ACPI_RESOURCE_TYPE_MEMORY24:
-			pnp_dbg("Encode mem24");
 			pnpacpi_encode_mem24(resource,
 					     &res_table->mem_resource[mem]);
 			mem++;
 			break;
 		case ACPI_RESOURCE_TYPE_MEMORY32:
-			pnp_dbg("Encode mem32");
 			pnpacpi_encode_mem32(resource,
 					     &res_table->mem_resource[mem]);
 			mem++;
 			break;
 		case ACPI_RESOURCE_TYPE_FIXED_MEMORY32:
-			pnp_dbg("Encode fixed mem32");
 			pnpacpi_encode_fixed_mem32(resource,
 						   &res_table->
 						   mem_resource[mem]);
 			mem++;
 			break;
 		case ACPI_RESOURCE_TYPE_EXTENDED_IRQ:
-			pnp_dbg("Encode ext irq");
 			pnpacpi_encode_ext_irq(resource,
 					       &res_table->irq_resource[irq]);
 			irq++;
#27 lore raw

PATCH 27/37 PNP: use conventional "i" for loop indices

In reply to Bjorn Helgaas

Cosmetic only: just use "i" instead of "tmp".

drivers/pnp/resource.c +46 -46
Index: work7/drivers/pnp/resource.c
===================================================================
@@ -215,7 +215,7 @@
 
 int pnp_check_port(struct pnp_dev *dev, int idx)
 {
-	int tmp;
+	int i;
 	struct pnp_dev *tdev;
 	resource_size_t *port, *end, *tport, *tend;
 
@@ -234,18 +234,18 @@
 	}
 
 	/* check if the resource is reserved */
-	for (tmp = 0; tmp < 8; tmp++) {
-		int rport = pnp_reserve_io[tmp << 1];
-		int rend = pnp_reserve_io[(tmp << 1) + 1] + rport - 1;
+	for (i = 0; i < 8; i++) {
+		int rport = pnp_reserve_io[i << 1];
+		int rend = pnp_reserve_io[(i << 1) + 1] + rport - 1;
 		if (ranged_conflict(port, end, &rport, &rend))
 			return 0;
 	}
 
 	/* check for internal conflicts */
-	for (tmp = 0; tmp < PNP_MAX_PORT && tmp != idx; tmp++) {
-		if (dev->res.port_resource[tmp].flags & IORESOURCE_IO) {
-			tport = &dev->res.port_resource[tmp].start;
-			tend = &dev->res.port_resource[tmp].end;
+	for (i = 0; i < PNP_MAX_PORT && i != idx; i++) {
+		if (dev->res.port_resource[i].flags & IORESOURCE_IO) {
+			tport = &dev->res.port_resource[i].start;
+			tend = &dev->res.port_resource[i].end;
 			if (ranged_conflict(port, end, tport, tend))
 				return 0;
 		}
@@ -255,13 +255,13 @@
 	pnp_for_each_dev(tdev) {
 		if (tdev == dev)
 			continue;
-		for (tmp = 0; tmp < PNP_MAX_PORT; tmp++) {
-			if (tdev->res.port_resource[tmp].flags & IORESOURCE_IO) {
+		for (i = 0; i < PNP_MAX_PORT; i++) {
+			if (tdev->res.port_resource[i].flags & IORESOURCE_IO) {
 				if (cannot_compare
-				    (tdev->res.port_resource[tmp].flags))
+				    (tdev->res.port_resource[i].flags))
 					continue;
-				tport = &tdev->res.port_resource[tmp].start;
-				tend = &tdev->res.port_resource[tmp].end;
+				tport = &tdev->res.port_resource[i].start;
+				tend = &tdev->res.port_resource[i].end;
 				if (ranged_conflict(port, end, tport, tend))
 					return 0;
 			}
@@ -273,7 +273,7 @@
 
 int pnp_check_mem(struct pnp_dev *dev, int idx)
 {
-	int tmp;
+	int i;
 	struct pnp_dev *tdev;
 	resource_size_t *addr, *end, *taddr, *tend;
 
@@ -292,18 +292,18 @@
 	}
 
 	/* check if the resource is reserved */
-	for (tmp = 0; tmp < 8; tmp++) {
-		int raddr = pnp_reserve_mem[tmp << 1];
-		int rend = pnp_reserve_mem[(tmp << 1) + 1] + raddr - 1;
+	for (i = 0; i < 8; i++) {
+		int raddr = pnp_reserve_mem[i << 1];
+		int rend = pnp_reserve_mem[(i << 1) + 1] + raddr - 1;
 		if (ranged_conflict(addr, end, &raddr, &rend))
 			return 0;
 	}
 
 	/* check for internal conflicts */
-	for (tmp = 0; tmp < PNP_MAX_MEM && tmp != idx; tmp++) {
-		if (dev->res.mem_resource[tmp].flags & IORESOURCE_MEM) {
-			taddr = &dev->res.mem_resource[tmp].start;
-			tend = &dev->res.mem_resource[tmp].end;
+	for (i = 0; i < PNP_MAX_MEM && i != idx; i++) {
+		if (dev->res.mem_resource[i].flags & IORESOURCE_MEM) {
+			taddr = &dev->res.mem_resource[i].start;
+			tend = &dev->res.mem_resource[i].end;
 			if (ranged_conflict(addr, end, taddr, tend))
 				return 0;
 		}
@@ -313,13 +313,13 @@
 	pnp_for_each_dev(tdev) {
 		if (tdev == dev)
 			continue;
-		for (tmp = 0; tmp < PNP_MAX_MEM; tmp++) {
-			if (tdev->res.mem_resource[tmp].flags & IORESOURCE_MEM) {
+		for (i = 0; i < PNP_MAX_MEM; i++) {
+			if (tdev->res.mem_resource[i].flags & IORESOURCE_MEM) {
 				if (cannot_compare
-				    (tdev->res.mem_resource[tmp].flags))
+				    (tdev->res.mem_resource[i].flags))
 					continue;
-				taddr = &tdev->res.mem_resource[tmp].start;
-				tend = &tdev->res.mem_resource[tmp].end;
+				taddr = &tdev->res.mem_resource[i].start;
+				tend = &tdev->res.mem_resource[i].end;
 				if (ranged_conflict(addr, end, taddr, tend))
 					return 0;
 			}
@@ -336,7 +336,7 @@
 
 int pnp_check_irq(struct pnp_dev *dev, int idx)
 {
-	int tmp;
+	int i;
 	struct pnp_dev *tdev;
 	resource_size_t *irq = &dev->res.irq_resource[idx].start;
 
@@ -349,15 +349,15 @@
 		return 0;
 
 	/* check if the resource is reserved */
-	for (tmp = 0; tmp < 16; tmp++) {
-		if (pnp_reserve_irq[tmp] == *irq)
+	for (i = 0; i < 16; i++) {
+		if (pnp_reserve_irq[i] == *irq)
 			return 0;
 	}
 
 	/* check for internal conflicts */
-	for (tmp = 0; tmp < PNP_MAX_IRQ && tmp != idx; tmp++) {
-		if (dev->res.irq_resource[tmp].flags & IORESOURCE_IRQ) {
-			if (dev->res.irq_resource[tmp].start == *irq)
+	for (i = 0; i < PNP_MAX_IRQ && i != idx; i++) {
+		if (dev->res.irq_resource[i].flags & IORESOURCE_IRQ) {
+			if (dev->res.irq_resource[i].start == *irq)
 				return 0;
 		}
 	}
@@ -388,12 +388,12 @@
 	pnp_for_each_dev(tdev) {
 		if (tdev == dev)
 			continue;
-		for (tmp = 0; tmp < PNP_MAX_IRQ; tmp++) {
-			if (tdev->res.irq_resource[tmp].flags & IORESOURCE_IRQ) {
+		for (i = 0; i < PNP_MAX_IRQ; i++) {
+			if (tdev->res.irq_resource[i].flags & IORESOURCE_IRQ) {
 				if (cannot_compare
-				    (tdev->res.irq_resource[tmp].flags))
+				    (tdev->res.irq_resource[i].flags))
 					continue;
-				if ((tdev->res.irq_resource[tmp].start == *irq))
+				if ((tdev->res.irq_resource[i].start == *irq))
 					return 0;
 			}
 		}
@@ -405,7 +405,7 @@
 int pnp_check_dma(struct pnp_dev *dev, int idx)
 {
 #ifndef CONFIG_IA64
-	int tmp;
+	int i;
 	struct pnp_dev *tdev;
 	resource_size_t *dma = &dev->res.dma_resource[idx].start;
 
@@ -418,15 +418,15 @@
 		return 0;
 
 	/* check if the resource is reserved */
-	for (tmp = 0; tmp < 8; tmp++) {
-		if (pnp_reserve_dma[tmp] == *dma)
+	for (i = 0; i < 8; i++) {
+		if (pnp_reserve_dma[i] == *dma)
 			return 0;
 	}
 
 	/* check for internal conflicts */
-	for (tmp = 0; tmp < PNP_MAX_DMA && tmp != idx; tmp++) {
-		if (dev->res.dma_resource[tmp].flags & IORESOURCE_DMA) {
-			if (dev->res.dma_resource[tmp].start == *dma)
+	for (i = 0; i < PNP_MAX_DMA && i != idx; i++) {
+		if (dev->res.dma_resource[i].flags & IORESOURCE_DMA) {
+			if (dev->res.dma_resource[i].start == *dma)
 				return 0;
 		}
 	}
@@ -443,12 +443,12 @@
 	pnp_for_each_dev(tdev) {
 		if (tdev == dev)
 			continue;
-		for (tmp = 0; tmp < PNP_MAX_DMA; tmp++) {
-			if (tdev->res.dma_resource[tmp].flags & IORESOURCE_DMA) {
+		for (i = 0; i < PNP_MAX_DMA; i++) {
+			if (tdev->res.dma_resource[i].flags & IORESOURCE_DMA) {
 				if (cannot_compare
-				    (tdev->res.dma_resource[tmp].flags))
+				    (tdev->res.dma_resource[i].flags))
 					continue;
-				if ((tdev->res.dma_resource[tmp].start == *dma))
+				if ((tdev->res.dma_resource[i].start == *dma))
 					return 0;
 			}
 		}
#28 lore raw

PATCH 28/37 PNP: add pnp_get_resource() interface

In reply to Bjorn Helgaas

This adds a pnp_get_resource() that works the same way as platform_get_resource(). This will enable us to consolidate many pnp_resource_table references in one place, which will make it easier to make the table dynamic.

---
 include/linux/pnp.h    |    1 +
 drivers/pnp/resource.c |   17 +++++++++++++++++
 2 files changed, 18 insertions(+)
include/linux/pnp.h +1 -0
Index: work7/include/linux/pnp.h
===================================================================
@@ -25,6 +25,7 @@
 /*
  * Resource Management
  */
+struct resource *pnp_get_resource(struct pnp_dev *, unsigned int, unsigned int);
 
 /* Use these instead of directly reading pnp_dev to get resource information */
 #define pnp_port_start(dev,bar)   ((dev)->res.port_resource[(bar)].start)
drivers/pnp/resource.c +26 -0
Index: work7/drivers/pnp/resource.c
===================================================================
@@ -560,6 +560,32 @@
 	return 0;
 }
 
+struct resource *pnp_get_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_resource[num];
+	case IORESOURCE_MEM:
+		if (num >= PNP_MAX_MEM)
+			return NULL;
+		return &res->mem_resource[num];
+	case IORESOURCE_IRQ:
+		if (num >= PNP_MAX_IRQ)
+			return NULL;
+		return &res->irq_resource[num];
+	case IORESOURCE_DMA:
+		if (num >= PNP_MAX_DMA)
+			return NULL;
+		return &res->dma_resource[num];
+	}
+	return NULL;
+}
+
 /* format is: pnp_reserve_irq=irq1[,irq2] .... */
 static int __init pnp_setup_reserve_irq(char *str)
 {
#29 lore raw

Re: [patch 28/37] PNP: add pnp_get_resource() interface

PNP: add pnp_get_resource() interface

This adds a pnp_get_resource() that works the same way as platform_get_resource(). This will enable us to consolidate many pnp_resource_table references in one place, which will make it easier to make the table dynamic.

I used EXPORT_SYMBOL, not EXPORT_SYMBOL_GPL, because it doesn't seem right to break existing non-GPL PNP drivers.

---
 drivers/pnp/resource.c |   27 +++++++++++++++++++++++++++
 include/linux/pnp.h    |    1 +
 2 files changed, 28 insertions(+)
include/linux/pnp.h +1 -0
Index: work7/include/linux/pnp.h
===================================================================
@@ -25,6 +25,7 @@
 /*
  * Resource Management
  */
+struct resource *pnp_get_resource(struct pnp_dev *, unsigned int, unsigned int);
 
 /* Use these instead of directly reading pnp_dev to get resource information */
 #define pnp_port_start(dev,bar)   ((dev)->res.port_resource[(bar)].start)
drivers/pnp/resource.c +27 -0
Index: work7/drivers/pnp/resource.c
===================================================================
@@ -560,6 +560,33 @@
 	return 0;
 }
 
+struct resource *pnp_get_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_resource[num];
+	case IORESOURCE_MEM:
+		if (num >= PNP_MAX_MEM)
+			return NULL;
+		return &res->mem_resource[num];
+	case IORESOURCE_IRQ:
+		if (num >= PNP_MAX_IRQ)
+			return NULL;
+		return &res->irq_resource[num];
+	case IORESOURCE_DMA:
+		if (num >= PNP_MAX_DMA)
+			return NULL;
+		return &res->dma_resource[num];
+	}
+	return NULL;
+}
+EXPORT_SYMBOL(pnp_get_resource);
+
 /* format is: pnp_reserve_irq=irq1[,irq2] .... */
 static int __init pnp_setup_reserve_irq(char *str)
 {
#30 lore raw

PATCH 29/37 PNP: convert encoders to use pnp_get_resource(), not pnp_resource_table

In reply to Bjorn Helgaas

This removes more direct references to pnp_resource_table. This path is used when setting the resources used by a device.

drivers/pnp/isapnp/core.c +28 -33
Index: work7/drivers/pnp/isapnp/core.c
===================================================================
@@ -982,42 +982,37 @@
 
 static int isapnp_set_resources(struct pnp_dev *dev)
 {
-	struct pnp_resource_table *res = &dev->res;
-	int tmp;
+	int i, irq;
+	struct resource *res;
 
 	isapnp_cfg_begin(dev->card->number, dev->number);
 	dev->active = 1;
-	for (tmp = 0;
-	     tmp < ISAPNP_MAX_PORT
-	     && (res->port_resource[tmp].
-		 flags & (IORESOURCE_IO | IORESOURCE_UNSET)) == IORESOURCE_IO;
-	     tmp++)
-		isapnp_write_word(ISAPNP_CFG_PORT + (tmp << 1),
-				  res->port_resource[tmp].start);
-	for (tmp = 0;
-	     tmp < ISAPNP_MAX_IRQ
-	     && (res->irq_resource[tmp].
-		 flags & (IORESOURCE_IRQ | IORESOURCE_UNSET)) == IORESOURCE_IRQ;
-	     tmp++) {
-		int irq = res->irq_resource[tmp].start;
-		if (irq == 2)
-			irq = 9;
-		isapnp_write_byte(ISAPNP_CFG_IRQ + (tmp << 1), irq);
-	}
-	for (tmp = 0;
-	     tmp < ISAPNP_MAX_DMA
-	     && (res->dma_resource[tmp].
-		 flags & (IORESOURCE_DMA | IORESOURCE_UNSET)) == IORESOURCE_DMA;
-	     tmp++)
-		isapnp_write_byte(ISAPNP_CFG_DMA + tmp,
-				  res->dma_resource[tmp].start);
-	for (tmp = 0;
-	     tmp < ISAPNP_MAX_MEM
-	     && (res->mem_resource[tmp].
-		 flags & (IORESOURCE_MEM | IORESOURCE_UNSET)) == IORESOURCE_MEM;
-	     tmp++)
-		isapnp_write_word(ISAPNP_CFG_MEM + (tmp << 3),
-				  (res->mem_resource[tmp].start >> 8) & 0xffff);
+	for (i = 0; i < ISAPNP_MAX_PORT; i++) {
+		res = pnp_get_resource(dev, IORESOURCE_IO, i);
+		if (res && !(res->flags & IORESOURCE_UNSET))
+			isapnp_write_word(ISAPNP_CFG_PORT + (i << 1),
+					  res->start);
+	}
+	for (i = 0; i < ISAPNP_MAX_IRQ; i++) {
+		res = pnp_get_resource(dev, IORESOURCE_IRQ, i);
+		if (res && !(res->flags & IORESOURCE_UNSET)) {
+			irq = res->start;
+			if (irq == 2)
+				irq = 9;
+			isapnp_write_byte(ISAPNP_CFG_IRQ + (i << 1), irq);
+		}
+	}
+	for (i = 0; i < ISAPNP_MAX_DMA; i++) {
+		res = pnp_get_resource(dev, IORESOURCE_DMA, i);
+		if (res && !(res->flags & IORESOURCE_UNSET))
+			isapnp_write_byte(ISAPNP_CFG_DMA + i, res->start);
+	}
+	for (i = 0; i < ISAPNP_MAX_MEM; i++) {
+		res = pnp_get_resource(dev, IORESOURCE_MEM, i);
+		if (res && !(res->flags & IORESOURCE_UNSET))
+			isapnp_write_word(ISAPNP_CFG_MEM + (i << 3),
+					  (res->start >> 8) & 0xffff);
+	}
 	/* FIXME: We aren't handling 32bit mems properly here */
 	isapnp_activate(dev->number);
 	isapnp_cfg_end();
drivers/pnp/pnpacpi/rsparser.c +8 -11
Index: work7/drivers/pnp/pnpacpi/rsparser.c
===================================================================
@@ -852,7 +852,6 @@
 
 int pnpacpi_encode_resources(struct pnp_dev *dev, struct acpi_buffer *buffer)
 {
-	struct pnp_resource_table *res_table = &dev->res;
 	int i = 0;
 	/* pnpacpi_build_resource_template allocates extra mem */
 	int res_cnt = (buffer->length - 1) / sizeof(struct acpi_resource) - 1;
@@ -863,45 +862,43 @@
 		switch (resource->type) {
 		case ACPI_RESOURCE_TYPE_IRQ:
 			pnpacpi_encode_irq(resource,
-					   &res_table->irq_resource[irq]);
+				pnp_get_resource(dev, IORESOURCE_IRQ, irq));
 			irq++;
 			break;
 
 		case ACPI_RESOURCE_TYPE_DMA:
 			pnpacpi_encode_dma(resource,
-					   &res_table->dma_resource[dma]);
+				pnp_get_resource(dev, IORESOURCE_DMA, dma));
 			dma++;
 			break;
 		case ACPI_RESOURCE_TYPE_IO:
 			pnpacpi_encode_io(resource,
-					  &res_table->port_resource[port]);
+				pnp_get_resource(dev, IORESOURCE_IO, port));
 			port++;
 			break;
 		case ACPI_RESOURCE_TYPE_FIXED_IO:
 			pnpacpi_encode_fixed_io(resource,
-						&res_table->
-						port_resource[port]);
+				pnp_get_resource(dev, IORESOURCE_IO, port));
 			port++;
 			break;
 		case ACPI_RESOURCE_TYPE_MEMORY24:
 			pnpacpi_encode_mem24(resource,
-					     &res_table->mem_resource[mem]);
+				pnp_get_resource(dev, IORESOURCE_MEM, mem));
 			mem++;
 			break;
 		case ACPI_RESOURCE_TYPE_MEMORY32:
 			pnpacpi_encode_mem32(resource,
-					     &res_table->mem_resource[mem]);
+				pnp_get_resource(dev, IORESOURCE_MEM, mem));
 			mem++;
 			break;
 		case ACPI_RESOURCE_TYPE_FIXED_MEMORY32:
 			pnpacpi_encode_fixed_mem32(resource,
-						   &res_table->
-						   mem_resource[mem]);
+				pnp_get_resource(dev, IORESOURCE_MEM, mem));
 			mem++;
 			break;
 		case ACPI_RESOURCE_TYPE_EXTENDED_IRQ:
 			pnpacpi_encode_ext_irq(resource,
-					       &res_table->irq_resource[irq]);
+				pnp_get_resource(dev, IORESOURCE_IRQ, irq));
 			irq++;
 			break;
 		case ACPI_RESOURCE_TYPE_START_DEPENDENT:
drivers/pnp/pnpbios/rsparser.c +14 -8
Index: work7/drivers/pnp/pnpbios/rsparser.c
===================================================================
@@ -592,7 +592,6 @@
 							     unsigned char *p,
 							     unsigned char *end)
 {
-	struct pnp_resource_table *res = &dev->res;
 	unsigned int len, tag;
 	int port = 0, irq = 0, dma = 0, mem = 0;
 
@@ -615,42 +614,48 @@
 		case LARGE_TAG_MEM:
 			if (len != 9)
 				goto len_err;
-			pnpbios_encode_mem(p, &res->mem_resource[mem]);
+			pnpbios_encode_mem(p,
+				pnp_get_resource(dev, IORESOURCE_MEM, mem));
 			mem++;
 			break;
 
 		case LARGE_TAG_MEM32:
 			if (len != 17)
 				goto len_err;
-			pnpbios_encode_mem32(p, &res->mem_resource[mem]);
+			pnpbios_encode_mem32(p,
+				pnp_get_resource(dev, IORESOURCE_MEM, mem));
 			mem++;
 			break;
 
 		case LARGE_TAG_FIXEDMEM32:
 			if (len != 9)
 				goto len_err;
-			pnpbios_encode_fixed_mem32(p, &res->mem_resource[mem]);
+			pnpbios_encode_fixed_mem32(p,
+				pnp_get_resource(dev, IORESOURCE_MEM, mem));
 			mem++;
 			break;
 
 		case SMALL_TAG_IRQ:
 			if (len < 2 || len > 3)
 				goto len_err;
-			pnpbios_encode_irq(p, &res->irq_resource[irq]);
+			pnpbios_encode_irq(p,
+				pnp_get_resource(dev, IORESOURCE_IRQ, irq));
 			irq++;
 			break;
 
 		case SMALL_TAG_DMA:
 			if (len != 2)
 				goto len_err;
-			pnpbios_encode_dma(p, &res->dma_resource[dma]);
+			pnpbios_encode_dma(p,
+				pnp_get_resource(dev, IORESOURCE_DMA, dma));
 			dma++;
 			break;
 
 		case SMALL_TAG_PORT:
 			if (len != 7)
 				goto len_err;
-			pnpbios_encode_port(p, &res->port_resource[port]);
+			pnpbios_encode_port(p,
+				pnp_get_resource(dev, IORESOURCE_IO, port));
 			port++;
 			break;
 
@@ -661,7 +666,8 @@
 		case SMALL_TAG_FIXEDPORT:
 			if (len != 3)
 				goto len_err;
-			pnpbios_encode_fixed_port(p, &res->port_resource[port]);
+			pnpbios_encode_fixed_port(p,
+				pnp_get_resource(dev, IORESOURCE_IO, port));
 			port++;
 			break;
 
#31 lore raw

PATCH 30/37 PNP: convert resource accessors to use pnp_get_resource(), not pnp_resource_table

In reply to Bjorn Helgaas

This removes more direct references to pnp_resource_table.

include/linux/pnp.h +64 -23
Index: work7/include/linux/pnp.h
===================================================================
@@ -28,12 +28,24 @@
 struct resource *pnp_get_resource(struct pnp_dev *, unsigned int, unsigned int);
 
 /* Use these instead of directly reading pnp_dev to get resource information */
-#define pnp_port_start(dev,bar)   ((dev)->res.port_resource[(bar)].start)
-#define pnp_port_end(dev,bar)     ((dev)->res.port_resource[(bar)].end)
-#define pnp_port_flags(dev,bar)   ((dev)->res.port_resource[(bar)].flags)
-#define pnp_port_valid(dev,bar) \
-	((pnp_port_flags((dev),(bar)) & (IORESOURCE_IO | IORESOURCE_UNSET)) \
-		== IORESOURCE_IO)
+
+#define pnp_port_start(dev,bar)	\
+	pnp_get_resource(dev, IORESOURCE_IO, bar)->start
+#define pnp_port_end(dev,bar)	\
+	pnp_get_resource(dev, IORESOURCE_IO, bar)->end
+#define pnp_port_flags(dev,bar)	\
+	pnp_get_resource(dev, IORESOURCE_IO, bar)->flags
+
+static inline resource_size_t pnp_port_valid(struct pnp_dev *dev,
+					     unsigned int bar)
+{
+	struct resource *res = pnp_get_resource(dev, IORESOURCE_IO, bar);
+
+	if (res && !(res->flags & IORESOURCE_UNSET))
+		return 1;
+	return 0;
+}
+
 #define pnp_port_len(dev,bar) \
 	((pnp_port_start((dev),(bar)) == 0 &&	\
 	  pnp_port_end((dev),(bar)) ==		\
@@ -42,12 +54,23 @@
 	 (pnp_port_end((dev),(bar)) -		\
 	  pnp_port_start((dev),(bar)) + 1))
 
-#define pnp_mem_start(dev,bar)   ((dev)->res.mem_resource[(bar)].start)
-#define pnp_mem_end(dev,bar)     ((dev)->res.mem_resource[(bar)].end)
-#define pnp_mem_flags(dev,bar)   ((dev)->res.mem_resource[(bar)].flags)
-#define pnp_mem_valid(dev,bar) \
-	((pnp_mem_flags((dev),(bar)) & (IORESOURCE_MEM | IORESOURCE_UNSET)) \
-		== IORESOURCE_MEM)
+#define pnp_mem_start(dev,bar)	\
+	pnp_get_resource(dev, IORESOURCE_MEM, bar)->start
+#define pnp_mem_end(dev,bar)	\
+	pnp_get_resource(dev, IORESOURCE_MEM, bar)->end
+#define pnp_mem_flags(dev,bar)	\
+	pnp_get_resource(dev, IORESOURCE_MEM, bar)->flags
+
+static inline resource_size_t pnp_mem_valid(struct pnp_dev *dev,
+					    unsigned int bar)
+{
+	struct resource *res = pnp_get_resource(dev, IORESOURCE_MEM, bar);
+
+	if (res && !(res->flags & IORESOURCE_UNSET))
+		return 1;
+	return 0;
+}
+
 #define pnp_mem_len(dev,bar) \
 	((pnp_mem_start((dev),(bar)) == 0 &&	\
 	  pnp_mem_end((dev),(bar)) ==		\
@@ -56,17 +79,35 @@
 	 (pnp_mem_end((dev),(bar)) -		\
 	  pnp_mem_start((dev),(bar)) + 1))
 
-#define pnp_irq(dev,bar)	 ((dev)->res.irq_resource[(bar)].start)
-#define pnp_irq_flags(dev,bar)	 ((dev)->res.irq_resource[(bar)].flags)
-#define pnp_irq_valid(dev,bar) \
-	((pnp_irq_flags((dev),(bar)) & (IORESOURCE_IRQ | IORESOURCE_UNSET)) \
-		== IORESOURCE_IRQ)
-
-#define pnp_dma(dev,bar)	 ((dev)->res.dma_resource[(bar)].start)
-#define pnp_dma_flags(dev,bar)	 ((dev)->res.dma_resource[(bar)].flags)
-#define pnp_dma_valid(dev,bar) \
-	((pnp_dma_flags((dev),(bar)) & (IORESOURCE_DMA | IORESOURCE_UNSET)) \
-		== IORESOURCE_DMA)
+#define pnp_irq(dev,bar)	\
+	pnp_get_resource(dev, IORESOURCE_IRQ, bar)->start
+#define pnp_irq_flags(dev,bar)	\
+	pnp_get_resource(dev, IORESOURCE_IRQ, bar)->flags
+
+static inline resource_size_t pnp_irq_valid(struct pnp_dev *dev,
+					    unsigned int bar)
+{
+	struct resource *res = pnp_get_resource(dev, IORESOURCE_IRQ, bar);
+
+	if (res && !(res->flags & IORESOURCE_UNSET))
+		return 1;
+	return 0;
+}
+
+#define pnp_dma(dev,bar)	\
+	pnp_get_resource(dev, IORESOURCE_DMA, bar)->start
+#define pnp_dma_flags(dev,bar)	\
+	pnp_get_resource(dev, IORESOURCE_DMA, bar)->flags
+
+static inline resource_size_t pnp_dma_valid(struct pnp_dev *dev,
+					    unsigned int bar)
+{
+	struct resource *res = pnp_get_resource(dev, IORESOURCE_DMA, bar);
+
+	if (res && !(res->flags & IORESOURCE_UNSET))
+		return 1;
+	return 0;
+}
 
 #define PNP_PORT_FLAG_16BITADDR	(1<<0)
 #define PNP_PORT_FLAG_FIXED	(1<<1)
#32 lore raw

PATCH 31/37 PNP: convert resource checks to use pnp_get_resource(), not pnp_resource_table

In reply to Bjorn Helgaas

This removes more direct references to pnp_resource_table.

drivers/pnp/resource.c +58 -38
Index: work7/drivers/pnp/resource.c
===================================================================
@@ -216,14 +216,17 @@
 int pnp_check_port(struct pnp_dev *dev, int idx)
 {
 	int i;
+	struct resource *res, *tres;
 	struct pnp_dev *tdev;
 	resource_size_t *port, *end, *tport, *tend;
 
-	port = &dev->res.port_resource[idx].start;
-	end = &dev->res.port_resource[idx].end;
+	res = pnp_get_resource(dev, IORESOURCE_IO, idx);
+
+	port = &res->start;
+	end = &res->end;
 
 	/* if the resource doesn't exist, don't complain about it */
-	if (cannot_compare(dev->res.port_resource[idx].flags))
+	if (cannot_compare(res->flags))
 		return 1;
 
 	/* check if the resource is already in use, skip if the
@@ -243,9 +246,10 @@
 
 	/* check for internal conflicts */
 	for (i = 0; i < PNP_MAX_PORT && i != idx; i++) {
-		if (dev->res.port_resource[i].flags & IORESOURCE_IO) {
-			tport = &dev->res.port_resource[i].start;
-			tend = &dev->res.port_resource[i].end;
+		tres = pnp_get_resource(dev, IORESOURCE_IO, i);
+		if (tres->flags & IORESOURCE_IO) {
+			tport = &tres->start;
+			tend = &tres->end;
 			if (ranged_conflict(port, end, tport, tend))
 				return 0;
 		}
@@ -256,12 +260,12 @@
 		if (tdev == dev)
 			continue;
 		for (i = 0; i < PNP_MAX_PORT; i++) {
-			if (tdev->res.port_resource[i].flags & IORESOURCE_IO) {
-				if (cannot_compare
-				    (tdev->res.port_resource[i].flags))
+			tres = pnp_get_resource(dev, IORESOURCE_IO, i);
+			if (tres->flags & IORESOURCE_IO) {
+				if (cannot_compare(tres->flags))
 					continue;
-				tport = &tdev->res.port_resource[i].start;
-				tend = &tdev->res.port_resource[i].end;
+				tport = &tres->start;
+				tend = &tres->end;
 				if (ranged_conflict(port, end, tport, tend))
 					return 0;
 			}
@@ -274,14 +278,17 @@
 int pnp_check_mem(struct pnp_dev *dev, int idx)
 {
 	int i;
+	struct resource *res, *tres;
 	struct pnp_dev *tdev;
 	resource_size_t *addr, *end, *taddr, *tend;
 
-	addr = &dev->res.mem_resource[idx].start;
-	end = &dev->res.mem_resource[idx].end;
+	res = pnp_get_resource(dev, IORESOURCE_MEM, idx);
+
+	addr = &res->start;
+	end = &res->end;
 
 	/* if the resource doesn't exist, don't complain about it */
-	if (cannot_compare(dev->res.mem_resource[idx].flags))
+	if (cannot_compare(res->flags))
 		return 1;
 
 	/* check if the resource is already in use, skip if the
@@ -301,9 +308,10 @@
 
 	/* check for internal conflicts */
 	for (i = 0; i < PNP_MAX_MEM && i != idx; i++) {
-		if (dev->res.mem_resource[i].flags & IORESOURCE_MEM) {
-			taddr = &dev->res.mem_resource[i].start;
-			tend = &dev->res.mem_resource[i].end;
+		tres = pnp_get_resource(dev, IORESOURCE_MEM, i);
+		if (tres->flags & IORESOURCE_MEM) {
+			taddr = &tres->start;
+			tend = &tres->end;
 			if (ranged_conflict(addr, end, taddr, tend))
 				return 0;
 		}
@@ -314,12 +322,12 @@
 		if (tdev == dev)
 			continue;
 		for (i = 0; i < PNP_MAX_MEM; i++) {
-			if (tdev->res.mem_resource[i].flags & IORESOURCE_MEM) {
-				if (cannot_compare
-				    (tdev->res.mem_resource[i].flags))
+			tres = pnp_get_resource(dev, IORESOURCE_MEM, i);
+			if (tres->flags & IORESOURCE_MEM) {
+				if (cannot_compare(tres->flags))
 					continue;
-				taddr = &tdev->res.mem_resource[i].start;
-				tend = &tdev->res.mem_resource[i].end;
+				taddr = &tres->start;
+				tend = &tres->end;
 				if (ranged_conflict(addr, end, taddr, tend))
 					return 0;
 			}
@@ -337,11 +345,16 @@
 int pnp_check_irq(struct pnp_dev *dev, int idx)
 {
 	int i;
+	struct resource *res, *tres;
 	struct pnp_dev *tdev;
-	resource_size_t *irq = &dev->res.irq_resource[idx].start;
+	resource_size_t *irq;
+
+	res = pnp_get_resource(dev, IORESOURCE_IRQ, idx);
+
+	irq = &res->start;
 
 	/* if the resource doesn't exist, don't complain about it */
-	if (cannot_compare(dev->res.irq_resource[idx].flags))
+	if (cannot_compare(res->flags))
 		return 1;
 
 	/* check if the resource is valid */
@@ -356,8 +369,9 @@
 
 	/* check for internal conflicts */
 	for (i = 0; i < PNP_MAX_IRQ && i != idx; i++) {
-		if (dev->res.irq_resource[i].flags & IORESOURCE_IRQ) {
-			if (dev->res.irq_resource[i].start == *irq)
+		tres = pnp_get_resource(dev, IORESOURCE_IRQ, i);
+		if (tres->flags & IORESOURCE_IRQ) {
+			if (tres->start == *irq)
 				return 0;
 		}
 	}
@@ -389,11 +403,11 @@
 		if (tdev == dev)
 			continue;
 		for (i = 0; i < PNP_MAX_IRQ; i++) {
-			if (tdev->res.irq_resource[i].flags & IORESOURCE_IRQ) {
-				if (cannot_compare
-				    (tdev->res.irq_resource[i].flags))
+			tres = pnp_get_resource(dev, IORESOURCE_IRQ, i);
+			if (tres->flags & IORESOURCE_IRQ) {
+				if (cannot_compare(tres->flags))
 					continue;
-				if ((tdev->res.irq_resource[i].start == *irq))
+				if (tres->start == *irq)
 					return 0;
 			}
 		}
@@ -406,11 +420,16 @@
 {
 #ifndef CONFIG_IA64
 	int i;
+	struct resource *res, *tres;
 	struct pnp_dev *tdev;
-	resource_size_t *dma = &dev->res.dma_resource[idx].start;
+	resource_size_t *dma;
+
+	res = pnp_get_resource(dev, IORESOURCE_DMA, idx);
+
+	dma = &res->start;
 
 	/* if the resource doesn't exist, don't complain about it */
-	if (cannot_compare(dev->res.dma_resource[idx].flags))
+	if (cannot_compare(res->flags))
 		return 1;
 
 	/* check if the resource is valid */
@@ -425,8 +444,9 @@
 
 	/* check for internal conflicts */
 	for (i = 0; i < PNP_MAX_DMA && i != idx; i++) {
-		if (dev->res.dma_resource[i].flags & IORESOURCE_DMA) {
-			if (dev->res.dma_resource[i].start == *dma)
+		tres = pnp_get_resource(dev, IORESOURCE_DMA, i);
+		if (tres->flags & IORESOURCE_DMA) {
+			if (tres->start == *dma)
 				return 0;
 		}
 	}
@@ -444,11 +464,11 @@
 		if (tdev == dev)
 			continue;
 		for (i = 0; i < PNP_MAX_DMA; i++) {
-			if (tdev->res.dma_resource[i].flags & IORESOURCE_DMA) {
-				if (cannot_compare
-				    (tdev->res.dma_resource[i].flags))
+			tres = pnp_get_resource(dev, IORESOURCE_DMA, i);
+			if (tres->flags & IORESOURCE_DMA) {
+				if (cannot_compare(tres->flags))
 					continue;
-				if ((tdev->res.dma_resource[i].start == *dma))
+				if (tres->start == *dma)
 					return 0;
 			}
 		}
#33 lore raw

PATCH 32/37 PNP: convert resource assign functions to use pnp_get_resource(), not pnp_resource_table

In reply to Bjorn Helgaas

This removes more direct references to pnp_resource_table.

drivers/pnp/manager.c +28 -16
Index: work7/drivers/pnp/manager.c
===================================================================
@@ -19,6 +19,7 @@
 
 static int pnp_assign_port(struct pnp_dev *dev, struct pnp_port *rule, int idx)
 {
+	struct resource *res;
 	resource_size_t *start, *end;
 	unsigned long *flags;
 
@@ -28,13 +29,15 @@
 		return 1;
 	}
 
+	res = pnp_get_resource(dev, IORESOURCE_IO, idx);
+
 	/* check if this resource has been manually set, if so skip */
-	if (!(dev->res.port_resource[idx].flags & IORESOURCE_AUTO))
+	if (!(res->flags & IORESOURCE_AUTO))
 		return 1;
 
-	start = &dev->res.port_resource[idx].start;
-	end = &dev->res.port_resource[idx].end;
-	flags = &dev->res.port_resource[idx].flags;
+	start = &res->start;
+	end = &res->end;
+	flags = &res->flags;
 
 	/* set the initial values */
 	*flags |= rule->flags | IORESOURCE_IO;
@@ -60,6 +63,7 @@
 
 static int pnp_assign_mem(struct pnp_dev *dev, struct pnp_mem *rule, int idx)
 {
+	struct resource *res;
 	resource_size_t *start, *end;
 	unsigned long *flags;
 
@@ -69,13 +73,15 @@
 		return 1;
 	}
 
+	res = pnp_get_resource(dev, IORESOURCE_MEM, idx);
+
 	/* check if this resource has been manually set, if so skip */
-	if (!(dev->res.mem_resource[idx].flags & IORESOURCE_AUTO))
+	if (!(res->flags & IORESOURCE_AUTO))
 		return 1;
 
-	start = &dev->res.mem_resource[idx].start;
-	end = &dev->res.mem_resource[idx].end;
-	flags = &dev->res.mem_resource[idx].flags;
+	start = &res->start;
+	end = &res->end;
+	flags = &res->flags;
 
 	/* set the initial values */
 	*flags |= rule->flags | IORESOURCE_MEM;
@@ -111,6 +117,7 @@
 
 static int pnp_assign_irq(struct pnp_dev *dev, struct pnp_irq *rule, int idx)
 {
+	struct resource *res;
 	resource_size_t *start, *end;
 	unsigned long *flags;
 	int i;
@@ -126,13 +133,15 @@
 		return 1;
 	}
 
+	res = pnp_get_resource(dev, IORESOURCE_IRQ, idx);
+
 	/* check if this resource has been manually set, if so skip */
-	if (!(dev->res.irq_resource[idx].flags & IORESOURCE_AUTO))
+	if (!(res->flags & IORESOURCE_AUTO))
 		return 1;
 
-	start = &dev->res.irq_resource[idx].start;
-	end = &dev->res.irq_resource[idx].end;
-	flags = &dev->res.irq_resource[idx].flags;
+	start = &res->start;
+	end = &res->end;
+	flags = &res->flags;
 
 	/* set the initial values */
 	*flags |= rule->flags | IORESOURCE_IRQ;
@@ -161,6 +170,7 @@
 
 static void pnp_assign_dma(struct pnp_dev *dev, struct pnp_dma *rule, int idx)
 {
+	struct resource *res;
 	resource_size_t *start, *end;
 	unsigned long *flags;
 	int i;
@@ -175,13 +185,15 @@
 		return;
 	}
 
+	res = pnp_get_resource(dev, IORESOURCE_DMA, idx);
+
 	/* check if this resource has been manually set, if so skip */
-	if (!(dev->res.dma_resource[idx].flags & IORESOURCE_AUTO))
+	if (!(res->flags & IORESOURCE_AUTO))
 		return;
 
-	start = &dev->res.dma_resource[idx].start;
-	end = &dev->res.dma_resource[idx].end;
-	flags = &dev->res.dma_resource[idx].flags;
+	start = &res->start;
+	end = &res->end;
+	flags = &res->flags;
 
 	/* set the initial values */
 	*flags |= rule->flags | IORESOURCE_DMA;
#34 lore raw

PATCH 33/37 PNP: remove PNP_MAX_* uses

In reply to Bjorn Helgaas

Remove some PNP_MAX_* uses. The pnp_resource_table isn't dynamic yet, but with pnp_get_resource(), we can start moving away from the table size constants.

---
 drivers/pnp/interface.c |   43 +++++++++++++++++++++----------------------
 drivers/pnp/quirks.c    |   15 +++++++++------
 drivers/pnp/resource.c  |   40 ++++++++++++++++++++++++----------------
 drivers/pnp/system.c    |   21 ++++++++++-----------
 4 files changed, 64 insertions(+), 55 deletions(-)
drivers/pnp/interface.c +21 -22
Index: work7/drivers/pnp/interface.c
===================================================================
@@ -243,11 +243,14 @@
 
 static DEVICE_ATTR(options, S_IRUGO, pnp_show_options, NULL);
 
+#define set(flags)      ((flags & IORESOURCE_UNSET) == 0)
+
 static ssize_t pnp_show_current_resources(struct device *dmdev,
 					  struct device_attribute *attr,
 					  char *buf)
 {
 	struct pnp_dev *dev = to_pnp_dev(dmdev);
+	struct resource *res;
 	int i, ret;
 	pnp_info_buffer_t *buffer;
 
@@ -267,50 +270,46 @@
 	else
 		pnp_printf(buffer, "disabled\n");
 
-	for (i = 0; i < PNP_MAX_PORT; i++) {
-		if (pnp_port_valid(dev, i)) {
+	for (i = 0; (res = pnp_get_resource(dev, IORESOURCE_IO, i)); i++) {
+		if (set(res->flags)) {
 			pnp_printf(buffer, "io");
-			if (pnp_port_flags(dev, i) & IORESOURCE_DISABLED)
+			if (res->flags & IORESOURCE_DISABLED)
 				pnp_printf(buffer, " disabled\n");
 			else
 				pnp_printf(buffer, " 0x%llx-0x%llx\n",
-					   (unsigned long long)
-					   pnp_port_start(dev, i),
-					   (unsigned long long)pnp_port_end(dev,
-									    i));
+					   (unsigned long long) res->start,
+					   (unsigned long long) res->end);
 		}
 	}
-	for (i = 0; i < PNP_MAX_MEM; i++) {
-		if (pnp_mem_valid(dev, i)) {
+	for (i = 0; (res = pnp_get_resource(dev, IORESOURCE_MEM, i)); i++) {
+		if (set(res->flags)) {
 			pnp_printf(buffer, "mem");
-			if (pnp_mem_flags(dev, i) & IORESOURCE_DISABLED)
+			if (res->flags & IORESOURCE_DISABLED)
 				pnp_printf(buffer, " disabled\n");
 			else
 				pnp_printf(buffer, " 0x%llx-0x%llx\n",
-					   (unsigned long long)
-					   pnp_mem_start(dev, i),
-					   (unsigned long long)pnp_mem_end(dev,
-									   i));
+					   (unsigned long long) res->start,
+					   (unsigned long long) res->end);
 		}
 	}
-	for (i = 0; i < PNP_MAX_IRQ; i++) {
-		if (pnp_irq_valid(dev, i)) {
+	for (i = 0; (res = pnp_get_resource(dev, IORESOURCE_IRQ, i)); i++) {
+		if (set(res->flags)) {
 			pnp_printf(buffer, "irq");
-			if (pnp_irq_flags(dev, i) & IORESOURCE_DISABLED)
+			if (res->flags & IORESOURCE_DISABLED)
 				pnp_printf(buffer, " disabled\n");
 			else
 				pnp_printf(buffer, " %lld\n",
-					   (unsigned long long)pnp_irq(dev, i));
+					   (unsigned long long) res->start);
 		}
 	}
-	for (i = 0; i < PNP_MAX_DMA; i++) {
-		if (pnp_dma_valid(dev, i)) {
+	for (i = 0; (res = pnp_get_resource(dev, IORESOURCE_DMA, i)); i++) {
+		if (set(res->flags)) {
 			pnp_printf(buffer, "dma");
-			if (pnp_dma_flags(dev, i) & IORESOURCE_DISABLED)
+			if (res->flags & IORESOURCE_DISABLED)
 				pnp_printf(buffer, " disabled\n");
 			else
 				pnp_printf(buffer, " %lld\n",
-					   (unsigned long long)pnp_dma(dev, i));
+					   (unsigned long long) res->start);
 		}
 	}
 	ret = (buffer->curr - buf);
drivers/pnp/quirks.c +9 -6
Index: work7/drivers/pnp/quirks.c
===================================================================
@@ -114,6 +114,7 @@
 static void quirk_system_pci_resources(struct pnp_dev *dev)
 {
 	struct pci_dev *pdev = NULL;
+	struct resource *res;
 	resource_size_t pnp_start, pnp_end, pci_start, pci_end;
 	int i, j;
 
@@ -134,13 +135,15 @@
 
 			pci_start = pci_resource_start(pdev, i);
 			pci_end = pci_resource_end(pdev, i);
-			for (j = 0; j < PNP_MAX_MEM; j++) {
-				if (!pnp_mem_valid(dev, j) ||
-				    pnp_mem_len(dev, j) == 0)
+			for (j = 0;
+			     (res = pnp_get_resource(dev, IORESOURCE_MEM, j));
+			     j++) {
+				if (res->flags & IORESOURCE_UNSET ||
+				    (res->start == 0 && res->end == 0))
 					continue;
 
-				pnp_start = pnp_mem_start(dev, j);
-				pnp_end = pnp_mem_end(dev, j);
+				pnp_start = res->start;
+				pnp_end = res->end;
 
 				/*
 				 * If the PNP region doesn't overlap the PCI
@@ -173,7 +176,7 @@
 					pci_name(pdev), i,
 					(unsigned long long) pci_start,
 					(unsigned long long) pci_end);
-				pnp_mem_flags(dev, j) = 0;
+				res->flags = 0;
 			}
 		}
 	}
drivers/pnp/resource.c +24 -16
Index: work7/drivers/pnp/resource.c
===================================================================
@@ -245,8 +245,9 @@
 	}
 
 	/* check for internal conflicts */
-	for (i = 0; i < PNP_MAX_PORT && i != idx; i++) {
-		tres = pnp_get_resource(dev, IORESOURCE_IO, i);
+	for (i = 0;
+	     i != idx && (tres = pnp_get_resource(dev, IORESOURCE_IO, i));
+	     i++) {
 		if (tres->flags & IORESOURCE_IO) {
 			tport = &tres->start;
 			tend = &tres->end;
@@ -259,8 +260,9 @@
 	pnp_for_each_dev(tdev) {
 		if (tdev == dev)
 			continue;
-		for (i = 0; i < PNP_MAX_PORT; i++) {
-			tres = pnp_get_resource(dev, IORESOURCE_IO, i);
+		for (i = 0;
+		     (tres = pnp_get_resource(dev, IORESOURCE_IO, i));
+		     i++) {
 			if (tres->flags & IORESOURCE_IO) {
 				if (cannot_compare(tres->flags))
 					continue;
@@ -307,8 +309,9 @@
 	}
 
 	/* check for internal conflicts */
-	for (i = 0; i < PNP_MAX_MEM && i != idx; i++) {
-		tres = pnp_get_resource(dev, IORESOURCE_MEM, i);
+	for (i = 0;
+	     i != idx && (tres = pnp_get_resource(dev, IORESOURCE_MEM, i));
+	     i++) {
 		if (tres->flags & IORESOURCE_MEM) {
 			taddr = &tres->start;
 			tend = &tres->end;
@@ -321,8 +324,9 @@
 	pnp_for_each_dev(tdev) {
 		if (tdev == dev)
 			continue;
-		for (i = 0; i < PNP_MAX_MEM; i++) {
-			tres = pnp_get_resource(dev, IORESOURCE_MEM, i);
+		for (i = 0;
+		     (tres = pnp_get_resource(dev, IORESOURCE_MEM, i));
+		     i++) {
 			if (tres->flags & IORESOURCE_MEM) {
 				if (cannot_compare(tres->flags))
 					continue;
@@ -368,8 +372,9 @@
 	}
 
 	/* check for internal conflicts */
-	for (i = 0; i < PNP_MAX_IRQ && i != idx; i++) {
-		tres = pnp_get_resource(dev, IORESOURCE_IRQ, i);
+	for (i = 0;
+	     i != idx && (tres = pnp_get_resource(dev, IORESOURCE_IRQ, i));
+	     i++) {
 		if (tres->flags & IORESOURCE_IRQ) {
 			if (tres->start == *irq)
 				return 0;
@@ -402,8 +407,9 @@
 	pnp_for_each_dev(tdev) {
 		if (tdev == dev)
 			continue;
-		for (i = 0; i < PNP_MAX_IRQ; i++) {
-			tres = pnp_get_resource(dev, IORESOURCE_IRQ, i);
+		for (i = 0;
+		     (tres = pnp_get_resource(dev, IORESOURCE_IRQ, i));
+		     i++) {
 			if (tres->flags & IORESOURCE_IRQ) {
 				if (cannot_compare(tres->flags))
 					continue;
@@ -443,8 +449,9 @@
 	}
 
 	/* check for internal conflicts */
-	for (i = 0; i < PNP_MAX_DMA && i != idx; i++) {
-		tres = pnp_get_resource(dev, IORESOURCE_DMA, i);
+	for (i = 0;
+	     i != idx && (tres = pnp_get_resource(dev, IORESOURCE_DMA, i));
+	     i++) {
 		if (tres->flags & IORESOURCE_DMA) {
 			if (tres->start == *dma)
 				return 0;
@@ -463,8 +470,9 @@
 	pnp_for_each_dev(tdev) {
 		if (tdev == dev)
 			continue;
-		for (i = 0; i < PNP_MAX_DMA; i++) {
-			tres = pnp_get_resource(dev, IORESOURCE_DMA, i);
+		for (i = 0;
+		     (tres = pnp_get_resource(dev, IORESOURCE_DMA, i));
+		     i++) {
 			if (tres->flags & IORESOURCE_DMA) {
 				if (cannot_compare(tres->flags))
 					continue;
drivers/pnp/system.c +10 -11
Index: work7/drivers/pnp/system.c
===================================================================
@@ -56,14 +56,15 @@
 
 static void reserve_resources_of_dev(struct pnp_dev *dev)
 {
+	struct resource *res;
 	int i;
 
-	for (i = 0; i < PNP_MAX_PORT; i++) {
-		if (!pnp_port_valid(dev, i))
+	for (i = 0; (res = pnp_get_resource(dev, IORESOURCE_IO, i)); i++) {
+		if (res->flags & IORESOURCE_UNSET)
 			continue;
-		if (pnp_port_start(dev, i) == 0)
+		if (res->start == 0)
 			continue;	/* disabled */
-		if (pnp_port_start(dev, i) < 0x100)
+		if (res->start < 0x100)
 			/*
 			 * Below 0x100 is only standard PC hardware
 			 * (pics, kbd, timer, dma, ...)
@@ -73,19 +74,17 @@
 			 * So, do nothing
 			 */
 			continue;
-		if (pnp_port_end(dev, i) < pnp_port_start(dev, i))
+		if (res->end < res->start)
 			continue;	/* invalid */
 
-		reserve_range(dev, pnp_port_start(dev, i),
-			      pnp_port_end(dev, i), 1);
+		reserve_range(dev, res->start, res->end, 1);
 	}
 
-	for (i = 0; i < PNP_MAX_MEM; i++) {
-		if (!pnp_mem_valid(dev, i))
+	for (i = 0; (res = pnp_get_resource(dev, IORESOURCE_MEM, i)); i++) {
+		if (res->flags & IORESOURCE_UNSET)
 			continue;
 
-		reserve_range(dev, pnp_mem_start(dev, i),
-			      pnp_mem_end(dev, i), 0);
+		reserve_range(dev, res->start, res->end, 0);
 	}
 }
 
#35 lore raw

PATCH 34/37 PNP: remove unused interfaces using pnp_resource_table

In reply to Bjorn Helgaas

Rene Herman <[email protected]> recently removed the only in-tree driver uses of:

pnp_init_resource_table()
pnp_manual_config_dev()
pnp_resource_change()

in this change:

    http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=109c53f840e551d6e99ecfd8b0131a968332c89f

These are no longer used in the PNP core either, so we can just remove them completely.

It's possible that there are out-of-tree drivers that use these interfaces. They should be changed to either (1) use PNP quirks to work around broken hardware or firmware, or (2) use the sysfs interfaces to control resource usage from userspace.

---
 drivers/pnp/manager.c |   78 +-------------------------------------------------
 include/linux/pnp.h   |    9 -----
 2 files changed, 3 insertions(+), 84 deletions(-)
include/linux/pnp.h +1 -8
Index: work7/include/linux/pnp.h
===================================================================
@@ -429,18 +429,14 @@
 int pnp_register_port_resource(struct pnp_option *option,
 			       struct pnp_port *data);
 int pnp_register_mem_resource(struct pnp_option *option, struct pnp_mem *data);
-void pnp_init_resource_table(struct pnp_resource_table *table);
 void pnp_init_resources(struct pnp_dev *dev);
-int pnp_manual_config_dev(struct pnp_dev *dev, struct pnp_resource_table *res,
-			  int mode);
 int pnp_auto_config_dev(struct pnp_dev *dev);
 int pnp_validate_config(struct pnp_dev *dev);
 int pnp_start_dev(struct pnp_dev *dev);
 int pnp_stop_dev(struct pnp_dev *dev);
 int pnp_activate_dev(struct pnp_dev *dev);
 int pnp_disable_dev(struct pnp_dev *dev);
-void pnp_resource_change(struct resource *resource, resource_size_t start,
-			 resource_size_t size);
+
 
 /* protocol helpers */
 int pnp_is_active(struct pnp_dev *dev);
@@ -477,16 +473,13 @@
 static inline int pnp_register_dma_resource(struct pnp_option *option, struct pnp_dma *data) { return -ENODEV; }
 static inline int pnp_register_port_resource(struct pnp_option *option, struct pnp_port *data) { return -ENODEV; }
 static inline int pnp_register_mem_resource(struct pnp_option *option, struct pnp_mem *data) { return -ENODEV; }
-static inline void pnp_init_resource_table(struct pnp_resource_table *table) { }
 static inline void pnp_init_resources(struct pnp_dev *dev) { }
-static inline int pnp_manual_config_dev(struct pnp_dev *dev, struct pnp_resource_table *res, int mode) { return -ENODEV; }
 static inline int pnp_auto_config_dev(struct pnp_dev *dev) { return -ENODEV; }
 static inline int pnp_validate_config(struct pnp_dev *dev) { return -ENODEV; }
 static inline int pnp_start_dev(struct pnp_dev *dev) { return -ENODEV; }
 static inline int pnp_stop_dev(struct pnp_dev *dev) { return -ENODEV; }
 static inline int pnp_activate_dev(struct pnp_dev *dev) { return -ENODEV; }
 static inline int pnp_disable_dev(struct pnp_dev *dev) { return -ENODEV; }
-static inline void pnp_resource_change(struct resource *resource, resource_size_t start, resource_size_t size) { }
 
 /* protocol helpers */
 static inline int pnp_is_active(struct pnp_dev *dev) { return 0; }
drivers/pnp/manager.c +2 -76

104 lines not on this page · raw message

#36 lore raw

PATCH 35/37 rtc: dont reference pnp_resource_table directly

In reply to Bjorn Helgaas

pnp_resource_table is going away soon, so use the more generic public interfaces instead.

drivers/rtc/rtc-cmos.c +4 -3
Index: work7/drivers/rtc/rtc-cmos.c
===================================================================
@@ -855,11 +855,12 @@
 		 * don't define the IRQ. It should always be safe to
 		 * hardcode it in these cases
 		 */
-		return cmos_do_probe(&pnp->dev, &pnp->res.port_resource[0], 8);
+		return cmos_do_probe(&pnp->dev,
+				pnp_get_resource(pnp, IORESOURCE_IO, 0), 8);
 	else
 		return cmos_do_probe(&pnp->dev,
-				     &pnp->res.port_resource[0],
-				     pnp->res.irq_resource[0].start);
+				pnp_get_resource(pnp, IORESOURCE_IO, 0),
+				pnp_irq(pnp, 0));
 }
 
 static void __exit cmos_pnp_remove(struct pnp_dev *pnp)
#37 lore raw

PATCH 36/37 PNP: make pnp_resource_table private to PNP core

In reply to Bjorn Helgaas

There are no remaining references to the PNP_MAX_* constants or the pnp_resource_table structure outside of the PNP core. Make them private to the PNP core.

---
 drivers/pnp/base.h     |   12 ++++++++++++
 drivers/pnp/core.c     |    8 ++++++++
 drivers/pnp/manager.c  |    4 ++--
 drivers/pnp/resource.c |   10 +++++-----
 include/linux/pnp.h    |   14 ++------------
 5 files changed, 29 insertions(+), 19 deletions(-)
include/linux/pnp.h +2 -12
Index: work7/include/linux/pnp.h
===================================================================
@@ -13,14 +13,11 @@
 #include <linux/errno.h>
 #include <linux/mod_devicetable.h>
 
-#define PNP_MAX_PORT		40
-#define PNP_MAX_MEM		24
-#define PNP_MAX_IRQ		2
-#define PNP_MAX_DMA		2
 #define PNP_NAME_LEN		50
 
 struct pnp_protocol;
 struct pnp_dev;
+struct pnp_resource_table;
 
 /*
  * Resource Management
@@ -160,13 +157,6 @@
 	struct pnp_option *next;	/* used to chain dependent resources */
 };
 
-struct pnp_resource_table {
-	struct resource port_resource[PNP_MAX_PORT];
-	struct resource mem_resource[PNP_MAX_MEM];
-	struct resource dma_resource[PNP_MAX_DMA];
-	struct resource irq_resource[PNP_MAX_IRQ];
-};
-
 /*
  * Device Management
  */
@@ -236,7 +226,7 @@
 	int capabilities;
 	struct pnp_option *independent;
 	struct pnp_option *dependent;
-	struct pnp_resource_table res;
+	struct pnp_resource_table *res;
 
 	char name[PNP_NAME_LEN];	/* contains a human-readable name */
 	unsigned short regs;		/* ISAPnP: supported registers */
drivers/pnp/base.h +12 -0
Index: work7/drivers/pnp/base.h
===================================================================
@@ -19,3 +19,15 @@
 int pnp_add_dma_resource(struct pnp_dev *dev, int dma, int flags);
 int pnp_add_io_resource(struct pnp_dev *dev, resource_size_t start, resource_size_t len, int flags);
 int pnp_add_mem_resource(struct pnp_dev *dev, resource_size_t start, resource_size_t len, int flags);
+
+#define PNP_MAX_PORT		40
+#define PNP_MAX_MEM		24
+#define PNP_MAX_IRQ		2
+#define PNP_MAX_DMA		2
+
+struct pnp_resource_table {
+	struct resource port_resource[PNP_MAX_PORT];
+	struct resource mem_resource[PNP_MAX_MEM];
+	struct resource dma_resource[PNP_MAX_DMA];
+	struct resource irq_resource[PNP_MAX_IRQ];
+};
drivers/pnp/manager.c +2 -2
Index: work7/drivers/pnp/manager.c
===================================================================
@@ -218,7 +218,7 @@
  */
 void pnp_init_resources(struct pnp_dev *dev)
 {
-	struct pnp_resource_table *table = &dev->res;
+	struct pnp_resource_table *table = dev->res;
 	int idx;
 
 	for (idx = 0; idx < PNP_MAX_IRQ; idx++) {
@@ -257,7 +257,7 @@
  */
 static void pnp_clean_resource_table(struct pnp_dev *dev)
 {
-	struct pnp_resource_table *res = &dev->res;
+	struct pnp_resource_table *res = dev->res;
 	int idx;
 
 	for (idx = 0; idx < PNP_MAX_IRQ; idx++) {
drivers/pnp/resource.c +5 -5
Index: work7/drivers/pnp/resource.c
===================================================================
@@ -493,7 +493,7 @@
 
 int pnp_add_irq_resource(struct pnp_dev *dev, int irq, int flags)
 {
-	struct pnp_resource_table *res = &dev->res;
+	struct pnp_resource_table *res = dev->res;
 	int i = 0;
 	static unsigned char warned;
 
@@ -517,7 +517,7 @@
 
 int pnp_add_dma_resource(struct pnp_dev *dev, int dma, int flags)
 {
-	struct pnp_resource_table *res = &dev->res;
+	struct pnp_resource_table *res = dev->res;
 	int i = 0;
 	static unsigned char warned;
 
@@ -541,7 +541,7 @@
 
 int pnp_add_io_resource(struct pnp_dev *dev, resource_size_t start, resource_size_t len, int flags)
 {
-	struct pnp_resource_table *res = &dev->res;
+	struct pnp_resource_table *res = dev->res;
 	resource_size_t end = start + len - 1;
 	int i = 0;
 	static unsigned char warned;
@@ -566,7 +566,7 @@
 
 int pnp_add_mem_resource(struct pnp_dev *dev, resource_size_t start, resource_size_t len, int flags)
 {
-	struct pnp_resource_table *res = &dev->res;
+	struct pnp_resource_table *res = dev->res;
 	int i = 0;
 	static unsigned char warned;
 
@@ -591,7 +591,7 @@
 struct resource *pnp_get_resource(struct pnp_dev *dev,
 				  unsigned int type, unsigned int num)
 {
-	struct pnp_resource_table *res = &dev->res;
+	struct pnp_resource_table *res = dev->res;
 
 	switch (type) {
 	case IORESOURCE_IO:
drivers/pnp/core.c +8 -0
Index: work7/drivers/pnp/core.c
===================================================================
@@ -106,6 +106,7 @@
 	pnp_free_option(dev->independent);
 	pnp_free_option(dev->dependent);
 	pnp_free_ids(dev);
+	kfree(dev->res);
 	kfree(dev);
 }
 
@@ -118,6 +119,12 @@
 	if (!dev)
 		return NULL;
 
+	dev->res = kzalloc(sizeof(struct pnp_resource_table), GFP_KERNEL);
+	if (!dev->res) {
+		kfree(dev);
+		return NULL;
+	}
+
 	dev->protocol = protocol;
 	dev->number = id;
 
@@ -127,6 +134,7 @@
 
 	dev_id = pnp_add_id(dev, pnpid);
 	if (!dev_id) {
+		kfree(dev->res);
 		kfree(dev);
 		return NULL;
 	}
#38 lore raw

PATCH 37/37 PNP: make interfaces private to the PNP core

In reply to Bjorn Helgaas

The interfaces for registering protocols, devices, cards, and resource options should only be used inside the PNP core.

---
 drivers/pnp/base.h  |   24 +++++++++++++++++++++++-
 include/linux/pnp.h |   34 ----------------------------------
 2 files changed, 23 insertions(+), 35 deletions(-)
include/linux/pnp.h +0 -34
Index: work7/include/linux/pnp.h
===================================================================
@@ -390,19 +390,12 @@
 #if defined(CONFIG_PNP)
 
 /* device management */
-int pnp_register_protocol(struct pnp_protocol *protocol);
-void pnp_unregister_protocol(struct pnp_protocol *protocol);
-int pnp_add_device(struct pnp_dev *dev);
 int pnp_device_attach(struct pnp_dev *pnp_dev);
 void pnp_device_detach(struct pnp_dev *pnp_dev);
 extern struct list_head pnp_global;
 extern int pnp_platform_devices;
 
 /* multidevice card support */
-int pnp_add_card(struct pnp_card *card);
-void pnp_remove_card(struct pnp_card *card);
-int pnp_add_card_device(struct pnp_card *card, struct pnp_dev *dev);
-void pnp_remove_card_device(struct pnp_dev *dev);
 struct pnp_dev *pnp_request_card_device(struct pnp_card_link *clink,
 					const char *id, struct pnp_dev *from);
 void pnp_release_card_device(struct pnp_dev *dev);
@@ -411,23 +404,12 @@
 extern struct list_head pnp_cards;
 
 /* resource management */
-struct pnp_option *pnp_register_independent_option(struct pnp_dev *dev);
-struct pnp_option *pnp_register_dependent_option(struct pnp_dev *dev,
-						 int priority);
-int pnp_register_irq_resource(struct pnp_option *option, struct pnp_irq *data);
-int pnp_register_dma_resource(struct pnp_option *option, struct pnp_dma *data);
-int pnp_register_port_resource(struct pnp_option *option,
-			       struct pnp_port *data);
-int pnp_register_mem_resource(struct pnp_option *option, struct pnp_mem *data);
-void pnp_init_resources(struct pnp_dev *dev);
 int pnp_auto_config_dev(struct pnp_dev *dev);
-int pnp_validate_config(struct pnp_dev *dev);
 int pnp_start_dev(struct pnp_dev *dev);
 int pnp_stop_dev(struct pnp_dev *dev);
 int pnp_activate_dev(struct pnp_dev *dev);
 int pnp_disable_dev(struct pnp_dev *dev);
 
-
 /* protocol helpers */
 int pnp_is_active(struct pnp_dev *dev);
 int compare_pnp_id(struct pnp_id *pos, const char *id);
@@ -437,35 +419,19 @@
 #else
 
 /* device management */
-static inline int pnp_register_protocol(struct pnp_protocol *protocol) { return -ENODEV; }
-static inline void pnp_unregister_protocol(struct pnp_protocol *protocol) { }
-static inline int pnp_init_device(struct pnp_dev *dev) { return -ENODEV; }
-static inline int pnp_add_device(struct pnp_dev *dev) { return -ENODEV; }
 static inline int pnp_device_attach(struct pnp_dev *pnp_dev) { return -ENODEV; }
 static inline void pnp_device_detach(struct pnp_dev *pnp_dev) { }
 
 #define pnp_platform_devices 0
 
 /* multidevice card support */
-static inline int pnp_add_card(struct pnp_card *card) { return -ENODEV; }
-static inline void pnp_remove_card(struct pnp_card *card) { }
-static inline int pnp_add_card_device(struct pnp_card *card, struct pnp_dev *dev) { return -ENODEV; }
-static inline void pnp_remove_card_device(struct pnp_dev *dev) { }
 static inline struct pnp_dev *pnp_request_card_device(struct pnp_card_link *clink, const char *id, struct pnp_dev *from) { return NULL; }
 static inline void pnp_release_card_device(struct pnp_dev *dev) { }
 static inline int pnp_register_card_driver(struct pnp_card_driver *drv) { return -ENODEV; }
 static inline void pnp_unregister_card_driver(struct pnp_card_driver *drv) { }
 
 /* resource management */
-static inline struct pnp_option *pnp_register_independent_option(struct pnp_dev *dev) { return NULL; }
-static inline struct pnp_option *pnp_register_dependent_option(struct pnp_dev *dev, int priority) { return NULL; }
-static inline int pnp_register_irq_resource(struct pnp_option *option, struct pnp_irq *data) { return -ENODEV; }
-static inline int pnp_register_dma_resource(struct pnp_option *option, struct pnp_dma *data) { return -ENODEV; }
-static inline int pnp_register_port_resource(struct pnp_option *option, struct pnp_port *data) { return -ENODEV; }
-static inline int pnp_register_mem_resource(struct pnp_option *option, struct pnp_mem *data) { return -ENODEV; }
-static inline void pnp_init_resources(struct pnp_dev *dev) { }
 static inline int pnp_auto_config_dev(struct pnp_dev *dev) { return -ENODEV; }
-static inline int pnp_validate_config(struct pnp_dev *dev) { return -ENODEV; }
 static inline int pnp_start_dev(struct pnp_dev *dev) { return -ENODEV; }
 static inline int pnp_stop_dev(struct pnp_dev *dev) { return -ENODEV; }
 static inline int pnp_activate_dev(struct pnp_dev *dev) { return -ENODEV; }
drivers/pnp/base.h +23 -1
Index: work7/drivers/pnp/base.h
===================================================================
@@ -1,10 +1,32 @@
 extern spinlock_t pnp_lock;
 void *pnp_alloc(long size);
+
+int pnp_register_protocol(struct pnp_protocol *protocol);
+void pnp_unregister_protocol(struct pnp_protocol *protocol);
+
 struct pnp_dev *pnp_alloc_dev(struct pnp_protocol *, int id, char *pnpid);
 struct pnp_card *pnp_alloc_card(struct pnp_protocol *, int id, char *pnpid);
+
+int pnp_add_device(struct pnp_dev *dev);
 struct pnp_id *pnp_add_id(struct pnp_dev *dev, char *id);
-struct pnp_id *pnp_add_card_id(struct pnp_card *card, char *id);
 int pnp_interface_attach_device(struct pnp_dev *dev);
+
+int pnp_add_card(struct pnp_card *card);
+struct pnp_id *pnp_add_card_id(struct pnp_card *card, char *id);
+void pnp_remove_card(struct pnp_card *card);
+int pnp_add_card_device(struct pnp_card *card, struct pnp_dev *dev);
+void pnp_remove_card_device(struct pnp_dev *dev);
+
+struct pnp_option *pnp_register_independent_option(struct pnp_dev *dev);
+struct pnp_option *pnp_register_dependent_option(struct pnp_dev *dev,
+						 int priority);
+int pnp_register_irq_resource(struct pnp_option *option, struct pnp_irq *data);
+int pnp_register_dma_resource(struct pnp_option *option, struct pnp_dma *data);
+int pnp_register_port_resource(struct pnp_option *option,
+			       struct pnp_port *data);
+int pnp_register_mem_resource(struct pnp_option *option, struct pnp_mem *data);
+void pnp_init_resources(struct pnp_dev *dev);
+
 void pnp_fixup_device(struct pnp_dev *dev);
 void pnp_free_option(struct pnp_option *option);
 int __pnp_add_device(struct pnp_dev *dev);
reviewer
#39 lore raw
In reply to Bjorn Helgaas

i386:

#1 errs.ACPI.ACPI_DEBUG.ACPI_PROCFS.HIBERNATION.X86_LOCAL_APIC.X86_IO_APIC Cleaning...Building...
ERROR: "pnp_get_resource" [sound/oss/sb.ko] undefined!
ERROR: "pnp_get_resource" [sound/oss/ad1848.ko] undefined!
ERROR: "pnp_get_resource" [sound/isa/wavefront/snd-wavefront.ko] undefined!
ERROR: "pnp_get_resource" [sound/isa/snd-sscape.ko] undefined!
ERROR: "pnp_get_resource" [sound/isa/snd-opl3sa2.ko] undefined!
ERROR: "pnp_get_resource" [sound/isa/snd-es18xx.ko] undefined!
ERROR: "pnp_get_resource" [sound/isa/snd-dt019x.ko] undefined!
ERROR: "pnp_get_resource" [sound/isa/snd-cmi8330.ko] undefined!
ERROR: "pnp_get_resource" [sound/isa/snd-azt2320.ko] undefined!
ERROR: "pnp_get_resource" [sound/isa/snd-als100.ko] undefined!
ERROR: "pnp_get_resource" [sound/isa/sb/snd-sbawe.ko] undefined!
ERROR: "pnp_get_resource" [sound/isa/sb/snd-sb16.ko] undefined!
ERROR: "pnp_get_resource" [sound/isa/sb/snd-es968.ko] undefined!
ERROR: "pnp_get_resource" [sound/isa/opti9xx/snd-opti93x.ko] undefined!
ERROR: "pnp_get_resource" [sound/isa/opti9xx/snd-opti92x-cs4231.ko] undefined!
ERROR: "pnp_get_resource" [sound/isa/opti9xx/snd-opti92x-ad1848.ko] undefined!
ERROR: "pnp_get_resource" [sound/isa/gus/snd-interwave.ko] undefined!
ERROR: "pnp_get_resource" [sound/isa/gus/snd-interwave-stb.ko] undefined!
ERROR: "pnp_get_resource" [sound/isa/cs423x/snd-cs4236.ko] undefined!
ERROR: "pnp_get_resource" [sound/isa/cs423x/snd-cs4232.ko] undefined!
ERROR: "pnp_get_resource" [sound/isa/ad1816a/snd-ad1816a.ko] undefined!
ERROR: "pnp_get_resource" [sound/drivers/mpu401/snd-mpu401.ko] undefined!
ERROR: "pnp_get_resource" [drivers/watchdog/sc1200wdt.ko] undefined!
ERROR: "pnp_get_resource" [drivers/telephony/ixj.ko] undefined!
ERROR: "pnp_get_resource" [drivers/scsi/sym53c416.ko] undefined!
ERROR: "pnp_get_resource" [drivers/scsi/g_NCR5380_mmio.ko] undefined!
ERROR: "pnp_get_resource" [drivers/scsi/g_NCR5380.ko] undefined!
ERROR: "pnp_get_resource" [drivers/scsi/aha1542.ko] undefined!
ERROR: "pnp_get_resource" [drivers/scsi/aha152x.ko] undefined!
ERROR: "pnp_get_resource" [drivers/pcmcia/i82365.ko] undefined!
ERROR: "pnp_get_resource" [drivers/parport/parport_pc.ko] undefined!
ERROR: "pnp_get_resource" [drivers/net/smc-ultra.ko] undefined!
ERROR: "pnp_get_resource" [drivers/net/sb1000.ko] undefined!
ERROR: "pnp_get_resource" [drivers/net/ne.ko] undefined!
ERROR: "pnp_get_resource" [drivers/net/irda/smsc-ircc2.ko] undefined!
ERROR: "pnp_get_resource" [drivers/net/irda/nsc-ircc.ko] undefined!
ERROR: "pnp_get_resource" [drivers/net/3c515.ko] undefined!
ERROR: "pnp_get_resource" [drivers/net/3c509.ko] undefined!
ERROR: "pnp_get_resource" [drivers/mmc/host/wbsd.ko] undefined!
ERROR: "pnp_get_resource" [drivers/media/radio/radio-sf16fmi.ko] undefined!
ERROR: "pnp_get_resource" [drivers/media/radio/radio-cadet.ko] undefined!
ERROR: "pnp_get_resource" [drivers/isdn/hisax/hisax_fcpcipnp.ko] undefined!
ERROR: "pnp_get_resource" [drivers/isdn/hisax/hisax.ko] undefined!
ERROR: "pnp_get_resource" [drivers/input/gameport/ns558.ko] undefined!
ERROR: "pnp_get_resource" [drivers/ide/ide-pnp.ko] undefined!
ERROR: "pnp_get_resource" [drivers/char/tpm/tpm_tis.ko] undefined!
ERROR: "pnp_get_resource" [drivers/char/tpm/tpm_infineon.ko] undefined! Build FAILED

x86_64:

#33 errs.allmodconfig
Cleaning...Building...
ERROR: "pnp_get_resource" [sound/oss/sb.ko] undefined!
ERROR: "pnp_get_resource" [sound/drivers/mpu401/snd-mpu401.ko] undefined!
ERROR: "pnp_get_resource" [drivers/watchdog/sc1200wdt.ko] undefined!
ERROR: "pnp_get_resource" [drivers/serial/8250_pnp.ko] undefined!
ERROR: "pnp_get_resource" [drivers/rtc/rtc-cmos.ko] undefined!
ERROR: "pnp_get_resource" [drivers/parport/parport_pc.ko] undefined!
ERROR: "pnp_get_resource" [drivers/net/sb1000.ko] undefined!
ERROR: "pnp_get_resource" [drivers/net/irda/smsc-ircc2.ko] undefined!
ERROR: "pnp_get_resource" [drivers/net/irda/nsc-ircc.ko] undefined!
ERROR: "pnp_get_resource" [drivers/mmc/host/wbsd.ko] undefined!
ERROR: "pnp_get_resource" [drivers/input/serio/i8042.ko] undefined!
ERROR: "pnp_get_resource" [drivers/input/gameport/ns558.ko] undefined!
ERROR: "pnp_get_resource" [drivers/ide/ide-pnp.ko] undefined!
ERROR: "pnp_get_resource" [drivers/char/tpm/tpm_tis.ko] undefined!
ERROR: "pnp_get_resource" [drivers/char/tpm/tpm_infineon.ko] undefined! Build FAILED

ia64:

#9 errs.allmodconfig
Cleaning...Building...
ERROR: "pnp_get_resource" [sound/drivers/mpu401/snd-mpu401.ko] undefined!
ERROR: "pnp_get_resource" [drivers/serial/8250_pnp.ko] undefined!
ERROR: "pnp_get_resource" [drivers/parport/parport_pc.ko] undefined!
ERROR: "pnp_get_resource" [drivers/net/sb1000.ko] undefined!
ERROR: "pnp_get_resource" [drivers/input/serio/i8042.ko] undefined!
ERROR: "pnp_get_resource" [drivers/input/gameport/ns558.ko] undefined!
ERROR: "pnp_get_resource" [drivers/ide/ide-pnp.ko] undefined!
ERROR: "pnp_get_resource" [drivers/char/tpm/tpm_tis.ko] undefined!
ERROR: "pnp_get_resource" [drivers/char/tpm/tpm_infineon.ko] undefined! Build FAILED

#40 lore raw
Quoted 6 lines · Len Brown

i386:

#1 errs.ACPI.ACPI_DEBUG.ACPI_PROCFS.HIBERNATION.X86_LOCAL_APIC.X86_IO_APIC Cleaning...Building...
ERROR: "pnp_get_resource" [sound/oss/sb.ko] undefined!
ERROR: "pnp_get_resource" [sound/oss/ad1848.ko] undefined!

Oops, sorry, I forgot to export pnp_get_resource(). I'll post an updated patch as a response to the original. Is that easy for you to handle, Len? If not, let me know what you'd prefer.

Bjorn

#41 lore raw
Quoted 3 lines · Bjorn Helgaas

Oops, sorry, I forgot to export pnp_get_resource(). I'll post an updated patch as a response to the original. Is that easy for you to handle, Len? If not, let me know what you'd prefer.

Works for me at least. Only had time for a quick test. After applying these, my snd-cs4236 driver ISAPnP soundcard is no longer found.

Rene.

#42 lore raw
Quoted 8 lines · Rene Herman
Quoted 3 lines · Bjorn Helgaas

Oops, sorry, I forgot to export pnp_get_resource(). I'll post an updated patch as a response to the original. Is that easy for you to handle, Len? If not, let me know what you'd prefer.

Works for me at least. Only had time for a quick test. After applying these, my snd-cs4236 driver ISAPnP soundcard is no longer found.

Are any ISAPNP devices at all found? It's possible I broke ISAPNP completely, because I don't have anything to test it with. If you have a chance, maybe you could turn on CONFIG_PNP_DEBUG and send me a dmesg log.

Bjorn

#43 lore raw
Quoted 1 line · Bjorn Helgaas

Are any ISAPNP devices at all found?

Yes, but at most 100 * 10^4 / 2^16 percent of them. Trouble is in 04/37:

http://lkml.org/lkml/2008/3/26/273

The numeric part of the ID is a 4-digit hexadecimal value, not just decimal, so '0'+ don't work. Unfortunately, that one has a few dependents, so you'll probably want to restructure things yourself. If I just place the attached on top, the card's found again.

Not too much avail unfortunately, as I then get a resource assignment problem with your patches:

pnp: the driver 'cs4236_isapnp' has been registered cs4236_isapnp 01:01.00: driver attached
cs4236_isapnp 01:01.02: driver attached
cs4236_isapnp 01:01.03: driver attached
cs4236_isapnp 01:01.00: unable to assign resources CS4236+ WSS PnP configure failed for WSS (out of resources?) isapnp detection failed and probing for CS4236+ is not supported

(and exact same thing for snd-es18xx) which I'll try to look at next, but this ID thing will certainly need a fix first.

Rene.

drivers/pnp/isapnp/core.c +6 -8
index c4b95b5..0a41e05 100644
@@ -403,14 +403,12 @@ static void __init isapnp_skip_bytes(int count)
 static void isapnp_to_pnpid(unsigned short vendor, unsigned short device,
 			    char *id)
 {
-	id[0] = 'A' + ((vendor >> 2) & 0x3f) - 1;
-	id[1] = 'A' + (((vendor & 3) << 3) | ((vendor >> 13) & 7)) - 1;
-	id[2] = 'A' + ((vendor >> 8) & 0x1f) - 1;
-	id[3] = '0' + ((device >> 4) & 0x0f);
-	id[4] = '0' + (device & 0x0f);
-	id[5] = '0' + ((device >> 12) & 0x0f);
-	id[6] = '0' + ((device >> 8) & 0x0f);
-	id[7] = '\0';
+	sprintf(id, "%c%c%c%x%x%x%x",
+		'A' + ((vendor >> 2) & 0x3f) - 1,
+		'A' + (((vendor & 3) << 3) | ((vendor >> 13) & 7)) - 1,
+		'A' + ((vendor >> 8) & 0x1f) - 1,
+		(device >> 4) & 0x0f, device & 0x0f,
+		(device >> 12) & 0x0f, (device >> 8) & 0x0f);
 }
 
 /*
#44 lore raw
Quoted 5 lines · Rene Herman
Quoted 1 line · Bjorn Helgaas

Are any ISAPNP devices at all found?

Yes, but at most 100 * 10^4 / 2^16 percent of them. Trouble is in 04/37:

Well, that should read "Yes, no less than 100 * 10^4 / 2^16 percent of them even" but you get the point...

Rene.

#45 lore raw
In reply to Rene Herman
Quoted 4 lines · Rene Herman
Quoted 1 line · Bjorn Helgaas

Are any ISAPNP devices at all found?

Quoted 8 lines

Yes, but at most 100 * 10^4 / 2^16 percent of them. Trouble is in 04/37:

http://lkml.org/lkml/2008/3/26/273

The numeric part of the ID is a 4-digit hexadecimal value, not just decimal, so '0'+ don't work. Unfortunately, that one has a few dependents, so you'll probably want to restructure things yourself. If I just place the attached on top, the card's found again.

Ah, right. Thanks for tracking that down. I forgot to factor out isapnp_to_pnpid() and pnpid32_to_pnpid() (and acpi_ex_eisa_id_to_string() for that matter, though that's buried in the ACPI CA)-- they're really doing the same thing and we should only need one copy (plus the CA one).

Quoted 11 lines

Not too much avail unfortunately, as I then get a resource assignment problem with your patches:

pnp: the driver 'cs4236_isapnp' has been registered cs4236_isapnp 01:01.00: driver attached
cs4236_isapnp 01:01.02: driver attached
cs4236_isapnp 01:01.03: driver attached
cs4236_isapnp 01:01.00: unable to assign resources CS4236+ WSS PnP configure failed for WSS (out of resources?) isapnp detection failed and probing for CS4236+ is not supported

Quoted 2 lines

(and exact same thing for snd-es18xx) which I'll try to look at next, but this ID thing will certainly need a fix first.

Hmmm... that sounds harder. I'll read over it again and see if I can figure anything out.

Bjorn

#46 lore raw
Quoted 5 lines · Bjorn Helgaas

Ah, right. Thanks for tracking that down. I forgot to factor out isapnp_to_pnpid() and pnpid32_to_pnpid() (and acpi_ex_eisa_id_to_string() for that matter, though that's buried in the ACPI CA)-- they're really doing the same thing and we should only need one copy (plus the CA one).

I'll wait for another round then, before reviewing this further.

Quoted 2 lines

Hmmm... that sounds harder. I'll read over it again and see if I can figure anything out.

Actually, other than looking right past it a few dozen times, it's fairly evident. If I apply the attached, my soundcard's functional again.

Rene.

drivers/pnp/resource.c +4 -4
index 454290b..9ff76dc 100644
@@ -261,7 +261,7 @@ int pnp_check_port(struct pnp_dev *dev, int idx)
 		if (tdev == dev)
 			continue;
 		for (i = 0;
-		     (tres = pnp_get_resource(dev, IORESOURCE_IO, i));
+		     (tres = pnp_get_resource(tdev, IORESOURCE_IO, i));
 		     i++) {
 			if (tres->flags & IORESOURCE_IO) {
 				if (cannot_compare(tres->flags))
@@ -325,7 +325,7 @@ int pnp_check_mem(struct pnp_dev *dev, int idx)
 		if (tdev == dev)
 			continue;
 		for (i = 0;
-		     (tres = pnp_get_resource(dev, IORESOURCE_MEM, i));
+		     (tres = pnp_get_resource(tdev, IORESOURCE_MEM, i));
 		     i++) {
 			if (tres->flags & IORESOURCE_MEM) {
 				if (cannot_compare(tres->flags))
@@ -408,7 +408,7 @@ int pnp_check_irq(struct pnp_dev *dev, int idx)
 		if (tdev == dev)
 			continue;
 		for (i = 0;
-		     (tres = pnp_get_resource(dev, IORESOURCE_IRQ, i));
+		     (tres = pnp_get_resource(tdev, IORESOURCE_IRQ, i));
 		     i++) {
 			if (tres->flags & IORESOURCE_IRQ) {
 				if (cannot_compare(tres->flags))
@@ -471,7 +471,7 @@ int pnp_check_dma(struct pnp_dev *dev, int idx)
 		if (tdev == dev)
 			continue;
 		for (i = 0;
-		     (tres = pnp_get_resource(dev, IORESOURCE_DMA, i));
+		     (tres = pnp_get_resource(tdev, IORESOURCE_DMA, i));
 		     i++) {
 			if (tres->flags & IORESOURCE_DMA) {
 				if (cannot_compare(tres->flags))
#47 lore raw
Quoted 9 lines · Rene Herman
Quoted 5 lines · Bjorn Helgaas

Ah, right. Thanks for tracking that down. I forgot to factor out isapnp_to_pnpid() and pnpid32_to_pnpid() (and acpi_ex_eisa_id_to_string() for that matter, though that's buried in the ACPI CA)-- they're really doing the same thing and we should only need one copy (plus the CA one).

I'll wait for another round then, before reviewing this further.

There's actually a hex_asc() helper in kernel.h, so before you overlook it as well, this is a somewhat nicer ID fix.

Rene.

commit cfed76bc2a76f2094338bc43dae55aa814017b5f

Author: Rene Herman <[email protected]>
Date:   Tue Apr 1 12:03:15 2008 +0200

[ISAPNP] fix isapnp_to_pnpid

drivers/pnp/isapnp/core.c +5 -5
index c4b95b5..9727050 100644
@@ -406,11 +406,11 @@ static void isapnp_to_pnpid(unsigned short vendor, unsigned short device,
 	id[0] = 'A' + ((vendor >> 2) & 0x3f) - 1;
 	id[1] = 'A' + (((vendor & 3) << 3) | ((vendor >> 13) & 7)) - 1;
 	id[2] = 'A' + ((vendor >> 8) & 0x1f) - 1;
-	id[3] = '0' + ((device >> 4) & 0x0f);
-	id[4] = '0' + (device & 0x0f);
-	id[5] = '0' + ((device >> 12) & 0x0f);
-	id[6] = '0' + ((device >> 8) & 0x0f);
-	id[7] = '\0';
+	id[3] = hex_asc((device >> 4) & 0x0f);
+	id[4] = hex_asc(device & 0x0f);
+	id[5] = hex_asc((device >> 12) & 0x0f);
+	id[6] = hex_asc((device >> 8) & 0x0f);
+	id[7] = 0;
 }
 
 /*
#48 lore raw
Quoted 9 lines · Rene Herman
Quoted 8 lines · Rene Herman
Quoted 5 lines · Bjorn Helgaas

Ah, right. Thanks for tracking that down. I forgot to factor out isapnp_to_pnpid() and pnpid32_to_pnpid() (and acpi_ex_eisa_id_to_string() for that matter, though that's buried in the ACPI CA)-- they're really doing the same thing and we should only need one copy (plus the CA one).

Quoted 4 lines
Quoted 1 line

I'll wait for another round then, before reviewing this further.

There's actually a hex_asc() helper in kernel.h, so before you overlook it as well, this is a somewhat nicer ID fix.

Thanks, I didn't know about hex_asc()! I'll post v2 of the series today. And thanks a lot for finding those typos in resource.c. I never would have found those...

Bjorn