[patch 0/6] PNP: minor code cleanups, use dev_printk()

[patch 0/6] PNP: minor code cleanups, use dev_printk()

[Bjorn Helgaas]

These patches remove a few more null pointer checks, simplify a
couple code paths (without functional change), and make PNP use
dev_printk() when the device structure is available.

--

[patch 1/6] PNP: remove null pointer checks

[Bjorn Helgaas]

[-- Attachment #1: pnp-null-checks --]
[-- Type: text/plain, Size: 2644 bytes --]

Remove some null pointer checks.  Null pointers in these areas indicate
programming errors, and I think it's better to oops immediately rather
than return an error that is easily ignored.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>

Index: w/drivers/pnp/card.c
===================================================================
--- w.orig/drivers/pnp/card.c	2007-09-27 13:54:22.000000000 -0600
+++ w/drivers/pnp/card.c	2007-09-27 13:54:23.000000000 -0600
@@ -104,10 +104,6 @@
 {
 	struct pnp_id *ptr;
 
-	if (!id)
-		return -EINVAL;
-	if (!card)
-		return -EINVAL;
 	id->next = NULL;
 	ptr = card->id;
 	while (ptr && ptr->next)
@@ -124,8 +120,6 @@
 	struct pnp_id *id;
 	struct pnp_id *next;
 
-	if (!card)
-		return;
 	id = card->id;
 	while (id) {
 		next = id->next;
@@ -197,9 +191,6 @@
 	int error;
 	struct list_head *pos, *temp;
 
-	if (!card || !card->protocol)
-		return -EINVAL;
-
 	sprintf(card->dev.bus_id, "%02x:%02x", card->protocol->number,
 		card->number);
 	card->dev.parent = &card->protocol->dev;
@@ -243,8 +234,6 @@
 {
 	struct list_head *pos, *temp;
 
-	if (!card)
-		return;
 	device_unregister(&card->dev);
 	spin_lock(&pnp_lock);
 	list_del(&card->global_list);
@@ -263,8 +252,6 @@
  */
 int pnp_add_card_device(struct pnp_card *card, struct pnp_dev *dev)
 {
-	if (!card || !dev || !dev->protocol)
-		return -EINVAL;
 	dev->dev.parent = &card->dev;
 	dev->card_link = NULL;
 	snprintf(dev->dev.bus_id, BUS_ID_SIZE, "%02x:%02x.%02x",
@@ -348,8 +335,6 @@
 {
 	struct pnp_card_driver *drv = dev->card_link->driver;
 
-	if (!drv)
-		return;
 	drv->link.remove = &card_remove;
 	device_release_driver(&dev->dev);
 	drv->link.remove = &card_remove_first;
Index: w/include/linux/pnp.h
===================================================================
--- w.orig/include/linux/pnp.h	2007-09-27 13:54:29.000000000 -0600
+++ w/include/linux/pnp.h	2007-09-27 13:56:03.000000000 -0600
@@ -243,11 +243,11 @@
 #define PNP_CONFIGURABLE	0x0008
 #define PNP_REMOVABLE		0x0010
 
-#define pnp_can_read(dev)	(((dev)->protocol) && ((dev)->protocol->get) && \
+#define pnp_can_read(dev)	(((dev)->protocol->get) && \
 				 ((dev)->capabilities & PNP_READ))
-#define pnp_can_write(dev)	(((dev)->protocol) && ((dev)->protocol->set) && \
+#define pnp_can_write(dev)	(((dev)->protocol->set) && \
 				 ((dev)->capabilities & PNP_WRITE))
-#define pnp_can_disable(dev)	(((dev)->protocol) && ((dev)->protocol->disable) && \
+#define pnp_can_disable(dev)	(((dev)->protocol->disable) && \
 				 ((dev)->capabilities & PNP_DISABLE))
 #define pnp_can_configure(dev)	((!(dev)->active) && \
 				 ((dev)->capabilities & PNP_CONFIGURABLE))

--

[patch 2/6] PNP: simplify PNP card error handling

[Bjorn Helgaas]

[-- Attachment #1: pnp-simple --]
[-- Type: text/plain, Size: 2595 bytes --]

No functional change; just return errors early instead of putting the
main part of the function inside an "if" statement.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>

Index: w/drivers/pnp/card.c
===================================================================
--- w.orig/drivers/pnp/card.c	2007-09-13 16:25:52.000000000 -0600
+++ w/drivers/pnp/card.c	2007-09-14 16:10:29.000000000 -0600
@@ -197,33 +197,34 @@
 	card->dev.bus = NULL;
 	card->dev.release = &pnp_release_card;
 	error = device_register(&card->dev);
-
-	if (error == 0) {
-		pnp_interface_attach_card(card);
-		spin_lock(&pnp_lock);
-		list_add_tail(&card->global_list, &pnp_cards);
-		list_add_tail(&card->protocol_list, &card->protocol->cards);
-		spin_unlock(&pnp_lock);
-
-		/* we wait until now to add devices in order to ensure the drivers
-		 * will be able to use all of the related devices on the card
-		 * without waiting any unresonable length of time */
-		list_for_each(pos, &card->devices) {
-			struct pnp_dev *dev = card_to_pnp_dev(pos);
-			__pnp_add_device(dev);
-		}
-
-		/* match with card drivers */
-		list_for_each_safe(pos, temp, &pnp_card_drivers) {
-			struct pnp_card_driver *drv =
-			    list_entry(pos, struct pnp_card_driver,
-				       global_list);
-			card_probe(card, drv);
-		}
-	} else
+	if (error) {
 		pnp_err("sysfs failure, card '%s' will be unavailable",
 			card->dev.bus_id);
-	return error;
+		return error;
+	}
+
+	pnp_interface_attach_card(card);
+	spin_lock(&pnp_lock);
+	list_add_tail(&card->global_list, &pnp_cards);
+	list_add_tail(&card->protocol_list, &card->protocol->cards);
+	spin_unlock(&pnp_lock);
+
+	/* we wait until now to add devices in order to ensure the drivers
+	 * will be able to use all of the related devices on the card
+	 * without waiting an unreasonable length of time */
+	list_for_each(pos, &card->devices) {
+		struct pnp_dev *dev = card_to_pnp_dev(pos);
+		__pnp_add_device(dev);
+	}
+
+	/* match with card drivers */
+	list_for_each_safe(pos, temp, &pnp_card_drivers) {
+		struct pnp_card_driver *drv =
+		    list_entry(pos, struct pnp_card_driver,
+			       global_list);
+		card_probe(card, drv);
+	}
+	return 0;
 }
 
 /**
@@ -291,14 +292,15 @@
 	struct pnp_card *card;
 
 	if (!clink || !id)
-		goto done;
+		return NULL;
+
 	card = clink->card;
 	drv = clink->driver;
 	if (!from) {
 		pos = card->devices.next;
 	} else {
 		if (from->card != card)
-			goto done;
+			return NULL;
 		pos = from->card_list.next;
 	}
 	while (pos != &card->devices) {
@@ -308,7 +310,6 @@
 		pos = pos->next;
 	}
 
-done:
 	return NULL;
 
 found:

--

[patch 3/6] PNP: use dev_info(), dev_err(), etc in core

[Bjorn Helgaas]

[-- Attachment #1: pnp-print --]
[-- Type: text/plain, Size: 6528 bytes --]

If we have the struct pnp_dev available, we can use dev_info(), dev_err(),
etc., to give a little more information and consistency.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>

Index: w/drivers/pnp/card.c
===================================================================
--- w.orig/drivers/pnp/card.c	2007-09-13 16:26:02.000000000 -0600
+++ w/drivers/pnp/card.c	2007-09-13 16:26:04.000000000 -0600
@@ -198,8 +198,7 @@
 	card->dev.release = &pnp_release_card;
 	error = device_register(&card->dev);
 	if (error) {
-		pnp_err("sysfs failure, card '%s' will be unavailable",
-			card->dev.bus_id);
+		dev_err(&card->dev, "could not register (err=%d)\n", error);
 		return error;
 	}
 
Index: w/drivers/pnp/driver.c
===================================================================
--- w.orig/drivers/pnp/driver.c	2007-09-13 16:25:48.000000000 -0600
+++ w/drivers/pnp/driver.c	2007-09-13 16:26:04.000000000 -0600
@@ -86,9 +86,6 @@
 	pnp_dev = to_pnp_dev(dev);
 	pnp_drv = to_pnp_driver(dev->driver);
 
-	pnp_dbg("match found with the PnP device '%s' and the driver '%s'",
-		dev->bus_id, pnp_drv->name);
-
 	error = pnp_device_attach(pnp_dev);
 	if (error < 0)
 		return error;
@@ -116,6 +113,8 @@
 		error = 0;
 	} else
 		goto fail;
+
+	dev_dbg(dev, "driver attached\n");
 	return error;
 
 fail:
Index: w/drivers/pnp/interface.c
===================================================================
--- w.orig/drivers/pnp/interface.c	2007-09-13 16:25:48.000000000 -0600
+++ w/drivers/pnp/interface.c	2007-09-13 16:26:04.000000000 -0600
@@ -327,8 +327,7 @@
 
 	if (dev->status & PNP_ATTACHED) {
 		retval = -EBUSY;
-		pnp_info("Device %s cannot be configured because it is in use.",
-			 dev->dev.bus_id);
+		dev_info(&dev->dev, "in use; can't configure\n");
 		goto done;
 	}
 
Index: w/drivers/pnp/manager.c
===================================================================
--- w.orig/drivers/pnp/manager.c	2007-09-13 16:25:48.000000000 -0600
+++ w/drivers/pnp/manager.c	2007-09-13 16:26:04.000000000 -0600
@@ -22,8 +22,7 @@
 	unsigned long *flags;
 
 	if (idx >= PNP_MAX_PORT) {
-		pnp_err
-		    ("More than 4 ports is incompatible with pnp specifications.");
+		dev_err(&dev->dev, "too many I/O port resources\n");
 		/* pretend we were successful so at least the manager won't try again */
 		return 1;
 	}
@@ -64,8 +63,7 @@
 	unsigned long *flags;
 
 	if (idx >= PNP_MAX_MEM) {
-		pnp_err
-		    ("More than 8 mems is incompatible with pnp specifications.");
+		dev_err(&dev->dev, "too many memory resources\n");
 		/* pretend we were successful so at least the manager won't try again */
 		return 1;
 	}
@@ -122,8 +120,7 @@
 	};
 
 	if (idx >= PNP_MAX_IRQ) {
-		pnp_err
-		    ("More than 2 irqs is incompatible with pnp specifications.");
+		dev_err(&dev->dev, "too many IRQ resources\n");
 		/* pretend we were successful so at least the manager won't try again */
 		return 1;
 	}
@@ -173,8 +170,7 @@
 	};
 
 	if (idx >= PNP_MAX_DMA) {
-		pnp_err
-		    ("More than 2 dmas is incompatible with pnp specifications.");
+		dev_err(&dev->dev, "too many DMA resources\n");
 		/* pretend we were successful so at least the manager won't try again */
 		return 1;
 	}
@@ -447,8 +443,7 @@
 	int i = 1;
 
 	if (!pnp_can_configure(dev)) {
-		pnp_dbg("Device %s does not support resource configuration.",
-			dev->dev.bus_id);
+		dev_dbg(&dev->dev, "configuration not supported\n");
 		return -ENODEV;
 	}
 
@@ -465,7 +460,7 @@
 		} while (dep);
 	}
 
-	pnp_err("Unable to assign resources to device %s.", dev->dev.bus_id);
+	dev_err(&dev->dev, "unable to assign resources\n");
 	return -EBUSY;
 }
 
@@ -478,17 +473,16 @@
 int pnp_start_dev(struct pnp_dev *dev)
 {
 	if (!pnp_can_write(dev)) {
-		pnp_dbg("Device %s does not support activation.",
-			dev->dev.bus_id);
+		dev_dbg(&dev->dev, "activation not supported\n");
 		return -EINVAL;
 	}
 
 	if (dev->protocol->set(dev, &dev->res) < 0) {
-		pnp_err("Failed to activate device %s.", dev->dev.bus_id);
+		dev_err(&dev->dev, "activation failed\n");
 		return -EIO;
 	}
 
-	pnp_info("Device %s activated.", dev->dev.bus_id);
+	dev_info(&dev->dev, "activated\n");
 	return 0;
 }
 
@@ -501,16 +495,15 @@
 int pnp_stop_dev(struct pnp_dev *dev)
 {
 	if (!pnp_can_disable(dev)) {
-		pnp_dbg("Device %s does not support disabling.",
-			dev->dev.bus_id);
+		dev_dbg(&dev->dev, "disabling not supported\n");
 		return -EINVAL;
 	}
 	if (dev->protocol->disable(dev) < 0) {
-		pnp_err("Failed to disable device %s.", dev->dev.bus_id);
+		dev_err(&dev->dev, "disable failed\n");
 		return -EIO;
 	}
 
-	pnp_info("Device %s disabled.", dev->dev.bus_id);
+	dev_info(&dev->dev, "disabled\n");
 	return 0;
 }
 
Index: w/drivers/pnp/quirks.c
===================================================================
--- w.orig/drivers/pnp/quirks.c	2007-09-13 16:25:48.000000000 -0600
+++ w/drivers/pnp/quirks.c	2007-09-13 16:26:04.000000000 -0600
@@ -17,6 +17,7 @@
 #include <linux/slab.h>
 #include <linux/pnp.h>
 #include <linux/io.h>
+#include <linux/kallsyms.h>
 #include "base.h"
 
 static void quirk_awe32_resources(struct pnp_dev *dev)
@@ -133,11 +134,18 @@
 void pnp_fixup_device(struct pnp_dev *dev)
 {
 	int i = 0;
+	void (*quirk)(struct pnp_dev *);
 
 	while (*pnp_fixups[i].id) {
 		if (compare_pnp_id(dev->id, pnp_fixups[i].id)) {
-			pnp_dbg("Calling quirk for %s", dev->dev.bus_id);
-			pnp_fixups[i].quirk_function(dev);
+			quirk = pnp_fixups[i].quirk_function;
+
+#ifdef DEBUG
+			dev_dbg(&dev->dev, "calling quirk 0x%p", quirk);
+			print_fn_descriptor_symbol(": %s()\n",
+				(unsigned long) *quirk);
+#endif
+			(*quirk)(dev);
 		}
 		i++;
 	}
Index: w/drivers/pnp/resource.c
===================================================================
--- w.orig/drivers/pnp/resource.c	2007-09-13 16:25:48.000000000 -0600
+++ w/drivers/pnp/resource.c	2007-09-13 16:26:04.000000000 -0600
@@ -51,7 +51,7 @@
 
 	/* this should never happen but if it does we'll try to continue */
 	if (dev->independent)
-		pnp_err("independent resource already registered");
+		dev_err(&dev->dev, "independent resource already registered\n");
 	dev->independent = option;
 	return option;
 }
Index: w/include/linux/pnp.h
===================================================================
--- w.orig/include/linux/pnp.h	2007-09-13 16:25:48.000000000 -0600
+++ w/include/linux/pnp.h	2007-09-13 16:26:04.000000000 -0600
@@ -8,6 +8,10 @@
 
 #ifdef __KERNEL__
 
+#ifdef CONFIG_PNP_DEBUG
+#define DEBUG
+#endif
+
 #include <linux/device.h>
 #include <linux/list.h>
 #include <linux/errno.h>

--

[patch 4/6] PNP: use dev_info() in system driver

[Bjorn Helgaas]

[-- Attachment #1: pnp-system-messages --]
[-- Type: text/plain, Size: 2621 bytes --]

Use dev_info() for a little consistency.  Changes this:

    pnp: 00:01: ioport range 0xf50-0xf58 has been reserved
    pnp: 00:01: ioport range 0x408-0x40f has been reserved
    pnp: 00:01: ioport range 0x900-0x903 has been reserved

to this:

    system 00:01: ioport range 0xf50-0xf58 has been reserved
    system 00:01: ioport range 0x408-0x40f has been reserved
    system 00:01: ioport range 0x900-0x903 has been reserved

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>

Index: w/drivers/pnp/system.c
===================================================================
--- w.orig/drivers/pnp/system.c	2007-09-13 16:27:50.000000000 -0600
+++ w/drivers/pnp/system.c	2007-09-13 16:49:24.000000000 -0600
@@ -22,36 +22,39 @@
 	{"", 0}
 };
 
-static void reserve_range(const char *pnpid, resource_size_t start,
+static void reserve_range(struct pnp_dev *dev, resource_size_t start,
 			  resource_size_t end, int port)
 {
-	struct resource *res;
 	char *regionid;
+	const char *pnpid = dev->dev.bus_id;
+	struct resource *res;
 
 	regionid = kmalloc(16, GFP_KERNEL);
-	if (regionid == NULL)
+	if (!regionid)
 		return;
+
 	snprintf(regionid, 16, "pnp %s", pnpid);
 	if (port)
 		res = request_region(start, end - start + 1, regionid);
 	else
 		res = request_mem_region(start, end - start + 1, regionid);
-	if (res == NULL)
-		kfree(regionid);
-	else
+	if (res)
 		res->flags &= ~IORESOURCE_BUSY;
+	else
+		kfree(regionid);
+
 	/*
 	 * Failures at this point are usually harmless. pci quirks for
 	 * example do reserve stuff they know about too, so we may well
 	 * have double reservations.
 	 */
-	printk(KERN_INFO "pnp: %s: %s range 0x%llx-0x%llx %s reserved\n",
-	       pnpid, port ? "ioport" : "iomem",
-	       (unsigned long long)start, (unsigned long long)end,
-	       NULL != res ? "has been" : "could not be");
+	dev_info(&dev->dev, "%s range 0x%llx-0x%llx %s reserved\n",
+		port ? "ioport" : "iomem",
+		(unsigned long long) start, (unsigned long long) end,
+		res ? "has been" : "could not be");
 }
 
-static void reserve_resources_of_dev(const struct pnp_dev *dev)
+static void reserve_resources_of_dev(struct pnp_dev *dev)
 {
 	int i;
 
@@ -73,7 +76,7 @@
 		if (pnp_port_end(dev, i) < pnp_port_start(dev, i))
 			continue;	/* invalid */
 
-		reserve_range(dev->dev.bus_id, pnp_port_start(dev, i),
+		reserve_range(dev, pnp_port_start(dev, i),
 			      pnp_port_end(dev, i), 1);
 	}
 
@@ -81,7 +84,7 @@
 		if (!pnp_mem_valid(dev, i))
 			continue;
 
-		reserve_range(dev->dev.bus_id, pnp_mem_start(dev, i),
+		reserve_range(dev, pnp_mem_start(dev, i),
 			      pnp_mem_end(dev, i), 0);
 	}
 }

--

[patch 5/6] PNP: simplify PNPBIOS insert_device

[Bjorn Helgaas]

[-- Attachment #1: pnpbios-simplify-insert --]
[-- Type: text/plain, Size: 1896 bytes --]

Hoist the struct pnp_dev alloc up into the function where it's used.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>

Index: w/drivers/pnp/pnpbios/core.c
===================================================================
--- w.orig/drivers/pnp/pnpbios/core.c	2007-09-18 10:09:04.000000000 -0600
+++ w/drivers/pnp/pnpbios/core.c	2007-09-18 10:15:51.000000000 -0600
@@ -315,25 +315,31 @@
 	.disable = pnpbios_disable_resources,
 };
 
-static int insert_device(struct pnp_dev *dev, struct pnp_bios_node *node)
+static int insert_device(struct pnp_bios_node *node)
 {
 	struct list_head *pos;
-	struct pnp_dev *pnp_dev;
+	struct pnp_dev *dev;
 	struct pnp_id *dev_id;
 	char id[8];
 
 	/* check if the device is already added */
-	dev->number = node->handle;
 	list_for_each(pos, &pnpbios_protocol.devices) {
-		pnp_dev = list_entry(pos, struct pnp_dev, protocol_list);
-		if (dev->number == pnp_dev->number)
+		dev = list_entry(pos, struct pnp_dev, protocol_list);
+		if (dev->number == node->handle)
 			return -1;
 	}
 
-	/* set the initial values for the PnP device */
+	dev = kzalloc(sizeof(struct pnp_dev), GFP_KERNEL);
+	if (!dev)
+		return -1;
+
 	dev_id = kzalloc(sizeof(struct pnp_id), GFP_KERNEL);
-	if (!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);
@@ -367,7 +373,6 @@
 	unsigned int nodes_got = 0;
 	unsigned int devs = 0;
 	struct pnp_bios_node *node;
-	struct pnp_dev *dev;
 
 	node = kzalloc(node_info.max_node_size, GFP_KERNEL);
 	if (!node)
@@ -388,12 +393,7 @@
 				break;
 		}
 		nodes_got++;
-		dev = kzalloc(sizeof(struct pnp_dev), GFP_KERNEL);
-		if (!dev)
-			break;
-		if (insert_device(dev, node) < 0)
-			kfree(dev);
-		else
+		if (insert_device(node) == 0)
 			devs++;
 		if (nodenum <= thisnodenum) {
 			printk(KERN_ERR

--

[patch 6/6] PNP: add debug message for adding new device

[Bjorn Helgaas]

[-- Attachment #1: pnp-add-print --]
[-- Type: text/plain, Size: 1744 bytes --]

Add PNP debug message when adding a device, remove similar PNPACPI message
with less information.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>

Index: w/drivers/pnp/core.c
===================================================================
--- w.orig/drivers/pnp/core.c	2007-09-25 13:44:34.000000000 -0600
+++ w/drivers/pnp/core.c	2007-09-25 13:59:35.000000000 -0600
@@ -125,9 +125,11 @@
 	spin_unlock(&pnp_lock);
 
 	ret = device_register(&dev->dev);
-	if (ret == 0)
-		pnp_interface_attach_device(dev);
-	return ret;
+	if (ret)
+		return ret;
+
+	pnp_interface_attach_device(dev);
+	return 0;
 }
 
 /*
@@ -138,12 +140,27 @@
  */
 int pnp_add_device(struct pnp_dev *dev)
 {
+	int ret;
+	struct pnp_id *id;
+
 	if (dev->card)
 		return -EINVAL;
+
 	dev->dev.parent = &dev->protocol->dev;
 	sprintf(dev->dev.bus_id, "%02x:%02x", dev->protocol->number,
 		dev->number);
-	return __pnp_add_device(dev);
+	ret = __pnp_add_device(dev);
+	if (ret)
+		return ret;
+
+#ifdef CONFIG_PNP_DEBUG
+	dev_printk(KERN_DEBUG, &dev->dev, "%s device, IDs",
+		dev->protocol->name);
+	for (id = dev->id; id; id = id->next)
+		printk(" %s", id->id);
+	printk(" (%s)\n", dev->active ? "active" : "disabled");
+#endif
+	return 0;
 }
 
 void __pnp_remove_device(struct pnp_dev *dev)
Index: w/drivers/pnp/pnpacpi/core.c
===================================================================
--- w.orig/drivers/pnp/pnpacpi/core.c	2007-09-25 13:44:59.000000000 -0600
+++ w/drivers/pnp/pnpacpi/core.c	2007-09-25 13:58:10.000000000 -0600
@@ -166,7 +166,6 @@
 	    is_exclusive_device(device))
 		return 0;
 
-	pnp_dbg("ACPI device : hid %s", acpi_device_hid(device));
 	dev = kzalloc(sizeof(struct pnp_dev), GFP_KERNEL);
 	if (!dev) {
 		pnp_err("Out of memory");

--

Re: [patch 3/6] PNP: use dev_info(), dev_err(), etc in core

[Andrew Morton]

On Fri, 28 Sep 2007 10:39:14 -0600 Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:

> --- w.orig/include/linux/pnp.h	2007-09-13 16:25:48.000000000 -0600
> +++ w/include/linux/pnp.h	2007-09-13 16:26:04.000000000 -0600
> @@ -8,6 +8,10 @@
>  
>  #ifdef __KERNEL__
>  
> +#ifdef CONFIG_PNP_DEBUG
> +#define DEBUG
> +#endif

In file included from include/linux/isapnp.h:26,
                 from drivers/pcmcia/i82365.c:58:
include/linux/pnp.h:12:1: warning: "DEBUG" redefined
<command line>:1:1: warning: this is the location of the previous definition

testing problems?

Re: [patch 3/6] PNP: use dev_info(), dev_err(), etc in core

[Bjorn Helgaas]

On Wednesday 10 October 2007 11:47:39 pm Andrew Morton wrote:
> On Fri, 28 Sep 2007 10:39:14 -0600 Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
> 
> > --- w.orig/include/linux/pnp.h	2007-09-13 16:25:48.000000000 -0600
> > +++ w/include/linux/pnp.h	2007-09-13 16:26:04.000000000 -0600
> > @@ -8,6 +8,10 @@
> >  
> >  #ifdef __KERNEL__
> >  
> > +#ifdef CONFIG_PNP_DEBUG
> > +#define DEBUG
> > +#endif
> 
> In file included from include/linux/isapnp.h:26,
>                  from drivers/pcmcia/i82365.c:58:
> include/linux/pnp.h:12:1: warning: "DEBUG" redefined
> <command line>:1:1: warning: this is the location of the previous definition
> 
> testing problems?

I didn't use the correct idiom for turning on DEBUG, and I didn't
built with allyesconfig (sorry), so didn't notice.  Can you add the
patch below, please?



PNP: turn on -DDEBUG when CONFIG_PNP_DEBUG is enabled

We now use dev_dbg(), which is enabled when DEBUG is defined.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>

Index: w/drivers/pnp/Makefile
===================================================================
--- w.orig/drivers/pnp/Makefile	2007-10-11 08:59:13.000000000 -0600
+++ w/drivers/pnp/Makefile	2007-10-11 09:04:46.000000000 -0600
@@ -7,3 +7,7 @@
 obj-$(CONFIG_PNPACPI)		+= pnpacpi/
 obj-$(CONFIG_PNPBIOS)		+= pnpbios/
 obj-$(CONFIG_ISAPNP)		+= isapnp/
+
+ifeq ($(CONFIG_PNP_DEBUG),y)
+EXTRA_CFLAGS += -DDEBUG
+endif

Re: [patch 6/6] PNP: add debug message for adding new device

[Bjorn Helgaas]

Sigh, this isn't my finest hour.  My patch adds an unused variable
warning when !CONFIG_PNP_DEBUG.  Can you please add this additional
patch?


PNP: add debug message for adding new device (fix unused var)

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>

Index: w/drivers/pnp/core.c
===================================================================
--- w.orig/drivers/pnp/core.c	2007-10-11 14:33:31.000000000 -0600
+++ w/drivers/pnp/core.c	2007-10-11 14:33:40.000000000 -0600
@@ -141,7 +141,9 @@
 int pnp_add_device(struct pnp_dev *dev)
 {
 	int ret;
+#ifdef CONFIG_PNP_DEBUG
 	struct pnp_id *id;
+#endif
 
 	if (dev->card)
 		return -EINVAL;