On 04/04/12 15:31, Kevin O'Connor wrote:
Agreed - the only thing it does is force a minimum size for memory bars as you pointed out in a previous email. As above, I did play with this a little more on Sunday. I also added in two patches from Gerd's series and made alignment handling more explicit. I'm including the series here if you're interested. Again, I think this should wait until after the 1.7.0 release. -Kevin
Hi Kevin,
Sorry for late response, was quite busy. I've reviewed and tried your patches.
[Patches 1-4] are almost the same as I proposed in this series. The noticeable differences are: 1) instead of sorting entries at mapping stage your patches sort entries at allocation stage. (no difference in behavior or readability so any approach is fine for me) 2) instead of storing pointer to bus resource which the bridge device represents (this_bus), it is obtained from pci structure and array of "busses". - entry->this_bus->r[entry->type].base = entry->base; + struct pci_bus *child_bus = &busses[entry->dev->secondary_bus]; + child_bus->r[entry->type].base = addr; Since "entry->this_bus" member is removed the information about whether the resource is bridge region or PCI BAR is encoded inside "entry->bar". Value "-1" - means this is a bridge region, the positive values mean it is a BAR. IMHO 2) makes code less readable, at least a comment explaining the meaning of negative value of PCI BAR is required. I've found just cosmetic difference to my patches so please don't forget to add my sign-off-by to them.
[Patch 5] Track region alignment explicitly. Looks very good and safe. Should reduce address range wastes.
[Patch 6] Small patch to account resources of PCI bridges. Looks fine. May be instead of + num_regions = 2; I would add #define PCI_BRIDGE_NUM_REGIONS 2 ..... + num_regions = PCI_BRIDGE_NUM_REGIONS;
[Patch 7] 64bit aware code. Patch is incomplete. It is not working.
After 1.7.0 is fine.