lkml.org 
[lkml]   [2010]   [Jan]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 03/14] pci: add pci_bridge_release_unused_res and pci_bus_release_unused_bridge_res
    On 01/15/2010 10:53 AM, Jesse Barnes wrote:
    > On Tue, 22 Dec 2009 15:02:23 -0800
    > Yinghai Lu <yinghai@kernel.org> wrote:
    >> +static void pci_bridge_release_unused_res(struct pci_bus *bus,
    >> + unsigned long type)
    >> +{
    >> + int idx;
    >> + bool changed = false;
    >> + struct pci_dev *dev;
    >> + struct resource *r;
    >> + unsigned long type_mask = IORESOURCE_IO | IORESOURCE_MEM |
    >> + IORESOURCE_PREFETCH;
    >> +
    >> + dev = bus->self;
    >> + for (idx = PCI_BRIDGE_RESOURCES; idx <=
    >> PCI_BRIDGE_RESOURCE_END;
    >> + idx++) {
    >> + r = &dev->resource[idx];
    >> + if ((r->flags & type_mask) != type)
    >> + continue;
    >> + if (!r->parent)
    >> + continue;
    >> + /*
    >> + * if there are children under that, we should
    >> release them
    >> + * all
    >> + */
    >> + release_child_resources(r);
    >> + if (!release_resource(r)) {
    >> + dev_printk(KERN_DEBUG, &dev->dev,
    >> + "resource %d %pR released\n", idx,
    >> r);
    >> + /* keep the old size */
    >> + r->end = resource_size(r) - 1;
    >> + r->start = 0;
    >> + r->flags = 0;
    >> + changed = true;
    >> + }
    >> + }
    >> +
    >> + if (changed) {
    >> + if (type & IORESOURCE_PREFETCH) {
    >> + /* avoiding touch the one without PREF */
    >> + type = IORESOURCE_PREFETCH;
    >> + }
    >> + __pci_setup_bridge(bus, type);
    >> + }
    >> +}
    >
    > Isn't this freeing resources regardless of whether there are children?
    > If so, shouldn't it just be called pci_bridge_release_resources?
    >
    ok
    >> +
    >> +/*
    >> + * try to release pci bridge resources that is from leaf bridge,
    >> + * so we can allocate big new one later
    >> + * check:
    >> + * 0: only release the bridge and only the bridge is leaf
    >> + * 1: release all down side bridge for third shoot
    >> + */
    >> +static void __ref pci_bus_release_unused_bridge_res(struct pci_bus
    >> *bus,
    >> + unsigned long
    >> type,
    >> + int check_leaf)
    >> +{
    >> + struct pci_dev *dev;
    >> + bool is_leaf_bridge = true;
    >> +
    >> + list_for_each_entry(dev, &bus->devices, bus_list) {
    >> + struct pci_bus *b = dev->subordinate;
    >> + if (!b)
    >> + continue;
    >> +
    >> + switch (dev->class >> 8) {
    >> + case PCI_CLASS_BRIDGE_CARDBUS:
    >> + is_leaf_bridge = false;
    >> + break;
    >> +
    >> + case PCI_CLASS_BRIDGE_PCI:
    >> + default:
    >> + is_leaf_bridge = false;
    >> + if (!check_leaf)
    >> + pci_bus_release_unused_bridge_res(b,
    >> type,
    >> + check_leaf);
    >> + break;
    >> + }
    >> + }
    >> +
    >> + /* The root bus? */
    >> + if (!bus->self)
    >> + return;
    >> +
    >> + switch (bus->self->class >> 8) {
    >> + case PCI_CLASS_BRIDGE_CARDBUS:
    >> + break;
    >> +
    >> + case PCI_CLASS_BRIDGE_PCI:
    >> + default:
    >> + if ((check_leaf && is_leaf_bridge) || !check_leaf)
    >> + pci_bridge_release_unused_res(bus, type);
    >> + break;
    >> + }
    >> +}
    >
    > Naming comment applies here too. I'd also rather see the "check_leaf"
    > flag be an enum, that makes the callers more self documenting. The
    > enums should probably be called "leaf_only" and "whole_subtree" or
    > similar , since the function will only release the resources of a leaf
    > bridge when the former is passed, while the whole bridge and its
    > subtree will be released in the latter case.
    ok

    YH


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