Re: [patch 15/15] PNP: convert resource options to single linked list

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 31-05-08 00:49, Bjorn Helgaas wrote:

Forgot this comment:

--- work10.orig/drivers/pnp/quirks.c	2008-05-30 15:58:14.000000000 -0600
+++ work10/drivers/pnp/quirks.c	2008-05-30 15:58:15.000000000 -0600

+static struct pnp_option *pnp_clone_dependent_set(struct pnp_dev *dev,
+						  unsigned int set)
 {
-	struct pnp_option *head = NULL;
-	struct pnp_option *prev = NULL;
-	struct pnp_option *res;
-
-	/*
-	 * Build a functional IRQ-optional variant of each MPU option.
-	 */
-
-	for (res = dev->dependent; res; res = res->next) {
-		struct pnp_option *curr;
-		struct pnp_port *port;
-		struct pnp_port *copy_port;
-		struct pnp_irq *irq;
-		struct pnp_irq *copy_irq;
-
-		port = res->port;
-		irq = res->irq;
-		if (!port || !irq)
-			continue;
+	struct pnp_option *tail = NULL, *first_new_option = NULL;
+	struct pnp_option *option, *new_option;
+	unsigned int flags;
+
+	list_for_each_entry(option, &dev->options, list) {
+		if (pnp_option_is_dependent(option))
+			tail = option;
+	}
+	if (!tail) {
+		dev_err(&dev->dev, "no dependent option sets\n");
+		return NULL;
+	}
- copy_port = pnp_alloc(sizeof *copy_port);
-		if (!copy_port)
-			break;
-
-		copy_irq = pnp_alloc(sizeof *copy_irq);
-		if (!copy_irq) {
-			kfree(copy_port);
-			break;
-		}
+	flags = pnp_dependent_option(dev, PNP_RES_PRIORITY_FUNCTIONAL);
+	list_for_each_entry(option, &dev->options, list) {
+		if (pnp_option_is_dependent(option) &&
+		    pnp_option_set(option) == set) {
+			new_option = kmalloc(sizeof(struct pnp_option),
+					     GFP_KERNEL);
+			if (!new_option) {
+				dev_err(&dev->dev, "couldn't clone dependent "
+					"set %d\n", set);
+				return NULL;
+			}
- *copy_port = *port;
-		copy_port->next = NULL;
+			*new_option = *option;
+			new_option->flags = flags;
+			if (!first_new_option)
+				first_new_option = new_option;
- *copy_irq = *irq;
-		copy_irq->flags |= IORESOURCE_IRQ_OPTIONAL;
-		copy_irq->next = NULL;
-
-		curr = pnp_build_option(PNP_RES_PRIORITY_FUNCTIONAL);
-		if (!curr) {
-			kfree(copy_port);
-			kfree(copy_irq);
-			break;
+			list_add(&new_option->list, &tail->list);
+			tail = new_option;
 		}
-		curr->port = copy_port;
-		curr->irq = copy_irq;
-
-		if (prev)
-			prev->next = curr;
-		else
-			head = curr;
-		prev = curr;
 	}
-	if (head)
-		dev_info(&dev->dev, "adding IRQ-optional MPU options\n");
- return head;
+	return first_new_option;
 }

This works, but I did the "disconnected chain build" that I did due to finding it particularly clumsy to add to the dependent chain while walking it.

This avoids actual problems due to num_sets = dev->num_dependents_sets in caller and the pnp_option_set(option) == set in the loop (not unlike the first version of the original checked for a present irq) but it's again checking the options it just added itself.

If you feel that's perfectly fine then <shrug> but thought I'd still remark on it in case it wasn't intentional.

Rene.
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux