On 01/15/2010 10:53 AM, Jesse Barnes wrote:
> 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?
>
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
--
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
