lkml.org 
[lkml]   [2008]   [Jun]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [patch 15/15] PNP: convert resource options to single linked list
    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.


    \
     
     \ /
      Last update: 2008-06-03 23:17    [W:4.497 / U:0.404 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site