lkml.org 
[lkml]   [2008]   [May]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    SubjectRe: [patch 4/4] PNP: dont sort by type in /sys/.../resources
    Date
    On Monday 19 May 2008 04:42:28 pm Adam M Belay wrote:
    > Does this fix a bug or is it just a more convenient representation?

    Mostly just simpler code.

    It is important to maintain the order of resources within each
    type, of course, so we can encode them in the correct order when
    we set device configuration. But we could still sort them by
    type if you think that's important.

    There is some risk with changing the order, but there's also
    some benefit in reducing the fiddling we do with the data from
    the firmware. It's been conceptually liberating for me to see
    the data correlate more exactly with _CRS and friends.

    > Perhaps, if
    > we're going to change the interface we could switch to PCI style sysfs
    > resources.

    You mean like resource_show(), which looks like this?

    $ cat /sys/devices/pci0000:00/0000:00:00.0/resource
    0x0000000000000000 0x0000000000000000 0x0000000000000000
    0x0000000000000000 0x0000000000000000 0x0000000000000000
    0x0000000000000000 0x0000000000000000 0x0000000000000000
    0x0000000000000000 0x0000000000000000 0x0000000000000000
    0x0000000000000000 0x0000000000000000 0x0000000000000000
    0x0000000000000000 0x0000000000000000 0x0000000000000000
    0x0000000000000000 0x0000000000000000 0x0000000000000000

    I'm all in favor of converging with the PCI style of doing things,
    but I'm not sure I understand your idea in this case. We still do
    quite a bit of manual poking in the PNP resource file, and the
    labels make it nicer for human consumption.

    Bjorn

    > Quoting Bjorn Helgaas <bjorn.helgaas@hp.com>:
    >
    > > Rather than stepping through all IO resources, then stepping through
    > > all MMIO resources, etc., we can just iterate over the resource list
    > > once directly.
    > >
    > > This can change the order in /sys, e.g.,
    > >
    > > # cat /sys/devices/pnp0/00:07/resources # OLD
    > > state = active
    > > io 0x3f8-0x3ff
    > > irq 4
    > >
    > > # cat /sys/devices/pnp0/00:07/resources # NEW
    > > state = active
    > > irq 4
    > > io 0x3f8-0x3ff
    > >
    > > The old code artificially sorted resources by type; the new code
    > > just lists them in the order we read them from the ISAPNP hardware
    > > or the BIOS.
    > >
    > > Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
    > >
    > > Index: work10/drivers/pnp/interface.c
    > > ===================================================================
    > > --- work10.orig/drivers/pnp/interface.c 2008-05-05 11:54:26.000000000 -0600
    > > +++ work10/drivers/pnp/interface.c 2008-05-05 11:59:53.000000000 -0600
    > > @@ -248,8 +248,9 @@ static ssize_t pnp_show_current_resource
    > > char *buf)
    > > {
    > > struct pnp_dev *dev = to_pnp_dev(dmdev);
    > > + struct pnp_resource *pnp_res;
    > > struct resource *res;
    > > - int i, ret;
    > > + int ret;
    > > pnp_info_buffer_t *buffer;
    > >
    > > if (!dev)
    > > @@ -262,46 +263,33 @@ static ssize_t pnp_show_current_resource
    > > buffer->buffer = buf;
    > > buffer->curr = buffer->buffer;
    > >
    > > - pnp_printf(buffer, "state = ");
    > > - if (dev->active)
    > > - pnp_printf(buffer, "active\n");
    > > - else
    > > - pnp_printf(buffer, "disabled\n");
    > > -
    > > - for (i = 0; (res = pnp_get_resource(dev, IORESOURCE_IO, i)); i++) {
    > > - pnp_printf(buffer, "io");
    > > - if (res->flags & IORESOURCE_DISABLED)
    > > - pnp_printf(buffer, " disabled\n");
    > > - else
    > > - pnp_printf(buffer, " 0x%llx-0x%llx\n",
    > > - (unsigned long long) res->start,
    > > - (unsigned long long) res->end);
    > > - }
    > > - for (i = 0; (res = pnp_get_resource(dev, IORESOURCE_MEM, i)); i++) {
    > > - pnp_printf(buffer, "mem");
    > > - if (res->flags & IORESOURCE_DISABLED)
    > > + pnp_printf(buffer, "state = %s\n", dev->active ? "active" : "disabled");
    > > +
    > > + list_for_each_entry(pnp_res, &dev->resources, list) {
    > > + res = &pnp_res->res;
    > > +
    > > + pnp_printf(buffer, pnp_resource_type_name(res));
    > > +
    > > + if (res->flags & IORESOURCE_DISABLED) {
    > > pnp_printf(buffer, " disabled\n");
    > > - else
    > > - pnp_printf(buffer, " 0x%llx-0x%llx\n",
    > > + continue;
    > > + }
    > > +
    > > + switch (pnp_resource_type(res)) {
    > > + case IORESOURCE_IO:
    > > + case IORESOURCE_MEM:
    > > + pnp_printf(buffer, " %#llx-%#llx\n",
    > > (unsigned long long) res->start,
    > > (unsigned long long) res->end);
    > > - }
    > > - for (i = 0; (res = pnp_get_resource(dev, IORESOURCE_IRQ, i)); i++) {
    > > - pnp_printf(buffer, "irq");
    > > - if (res->flags & IORESOURCE_DISABLED)
    > > - pnp_printf(buffer, " disabled\n");
    > > - else
    > > - pnp_printf(buffer, " %lld\n",
    > > - (unsigned long long) res->start);
    > > - }
    > > - for (i = 0; (res = pnp_get_resource(dev, IORESOURCE_DMA, i)); i++) {
    > > - pnp_printf(buffer, "dma");
    > > - if (res->flags & IORESOURCE_DISABLED)
    > > - pnp_printf(buffer, " disabled\n");
    > > - else
    > > + break;
    > > + case IORESOURCE_IRQ:
    > > + case IORESOURCE_DMA:
    > > pnp_printf(buffer, " %lld\n",
    > > (unsigned long long) res->start);
    > > + break;
    > > + }
    > > }
    > > +
    > > ret = (buffer->curr - buf);
    > > kfree(buffer);
    > > return ret;
    > >
    > > --
    > >
    >
    >
    >




    \
     
     \ /
      Last update: 2008-05-20 01:45    [W:0.028 / U:0.708 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site