Hi Stephen,
On 21 October 2015 at 14:25, Stephen Warren swarren@wwwdotorg.org wrote:
On 10/17/2015 11:50 AM, Simon Glass wrote:
At present we add a new resource entry for every range entry. But some range entries refer to configuration regions. To make this work, avoid adding two regions of the same time. The later ranges will overwrite the earlier (configuration) ones.
s/time/type/ in the last-but-one line.
What's wrong with having two regions of the same type? Equally, if we can "get away" with not storing some of the regions that happen to have a duplicate type, why not recast the function so that it only stores regions of specific (useful/desired) types, and simply dropping all of the other regions. That'd be a lot more consistent than only storing a somewhat arbitrary subset of the regions.
I'm not 100% sure that we want to disallow multiple regions. Although on non-x86 boards I've haven't seen hardware that supports multiple regions.
There does not seem to be a way to distinguish the configuration ranges other than by ordering.
Well, they do have different addresses too. But yes, the DT binding is written so that the entries in ranges must appear in a specific order, so order is the correct way to index the entries.
diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
@@ -720,9 +721,15 @@ static int decode_regions(struct pci_controller *hose, const void *blob, } else { continue; }
debug(" - type=%d\n", type);pci_set_region(hose->regions + hose->region_count++,pci_addr,
addr, size, type);
pos = -1;for (i = 0; i < hose->region_count; i++) {if (hose->regions[i].flags == type)pos = i;and break too?
Could do, might be clearer.
}if (pos == -1)pos = hose->region_count++;debug(" - type=%d, pos=%d\n", type, pos);pci_set_region(hose->regions + pos, pci_addr, addr, size,type); }
Regards, Simon