On Tue, 22 Dec 2009 15:02:23 -0800
Yinghai Lu <yinghai@xxxxxxxxxx> 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?
> +
> +/*
> + * 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.
This is starting to look a bit easier to follow though, thanks for your
patience so far.
--
Jesse Barnes, Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
