lkml.org 
[lkml]   [2010]   [Jan]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 1/9] pci: add pci_bridge_release_unused_res and pci_bus_release_unused_bridge_res

    Sorry, this is getting into bike-shed territory.

    I'm a lot happier with the code now, so may as well fix up a few
    style issues before it goes in.

    > +static void pci_bridge_release_resources(struct pci_bus *bus,
    > + unsigned long type)
    > +{

    [...]

    > + if (changed) {
    > + if (type & IORESOURCE_PREFETCH) {
    > + /* avoiding touch the one without PREF */
    > + type = IORESOURCE_PREFETCH;
    > + }

    Strictly speaking, you don't need those curly braces. If you want
    readability, how about moving the comment up?

    /* Only setup prefetch resources */
    if (type & IORESOURCE_PREFETCH)
    type = IORESOURCE_PREFETCH;


    > + __pci_setup_bridge(bus, type);
    > + }
    > +}
    > +
    > +static void __ref pci_bus_release_bridge_resources(struct pci_bus *bus,
    > + unsigned long type,
    > + enum release_type rel_type)
    > +{

    [...]

    > +
    > + /* The root bus? */

    Useless comment.

    > + if (pci_is_root_bus(bus))
    > + return;
    > +
    > + if ((bus->self->class >> 8) != PCI_CLASS_BRIDGE_PCI)
    > + return;
    > +
    > + if (((rel_type == leaf_only) && is_leaf_bridge) ||
    > + (rel_type == whole_subtree))
    > + pci_bridge_release_resources(bus, type);
    > +}

    Can clean this up a bit too with short-circuit logic.

    if ((rel_type == whole_subtree) || is_leaf_bridge)
    pci_bridge_release_resources(bus, type);

    If you clean those up, you can add my:

    Reviewed-by: Alex Chiang <achiang@hp.com>

    > +
    > static void pci_bus_dump_res(struct pci_bus *bus)
    > {
    > int i;
    > --
    > 1.6.4.2
    >


    \
     
     \ /
      Last update: 2010-01-21 19:21    [W:3.816 / U:0.884 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site