On 25.03.2009 23:17, Stefan Reinauer wrote:
>> Index: src/northbridge/amd/amdk8/northbridge.c
>> ===================================================================
>> --- src/northbridge/amd/amdk8/northbridge.c (revision 4026)
>> +++ src/northbridge/amd/amdk8/northbridge.c (working copy)
>> @@ -291,11 +291,12 @@
>> unsigned nodeid, link;
>> int result;
>> res = 0;
>> - for(nodeid = 0; !res && (nodeid < 8); nodeid++) {
>> + for(nodeid = 0; !res && (nodeid < FX_DEVS); nodeid++) {
>> device_t dev;
>> dev = __f0_dev[nodeid];
>> for(link = 0; !res && (link < 3); link++) {
>> - res = probe_resource(dev, 0x100 + (reg | link));
>> + if (dev)
>> + res = probe_resource(dev, 0x100 + (reg | link));
>>
>>
>
> Going through the loop and checking dev every time is a bit nasty. What
> about
>
I was about writing the same comment, but then I stumbled upon a
condition which should be watched. See below.
> for(nodeid = 0; !res && (nodeid < FX_DEVS); nodeid++) {
> device_t dev = __f0_dev[nodeid];
> if (!dev) continue;
> for(link = 0; !res && (link < 3); link++) {
> res = probe_resource(dev, 0x100 + (reg | link));
>
> ...
>
Is link accessed in read mode after the loop? If yes, this changes the
semantics.
I don't have access to the full source code right now, so I can't check.
>> }
>> }
>> result = 2;
>> @@ -760,19 +761,20 @@
>> mem_hole.hole_startk = HW_MEM_HOLE_SIZEK;
>> mem_hole.node_id = -1;
>>
>> - for (i = 0; i < 8; i++) {
>> + for (i = 0; i < FX_DEVS; i++) {
>> uint32_t base;
>> uint32_t hole;
>> base = f1_read_config32(0x40 + (i << 3));
>> if ((base & ((1<<1)|(1<<0))) != ((1<<1)|(1<<0))) {
>> continue;
>> }
>> -
>> - hole = pci_read_config32(__f1_dev[i], 0xf0);
>> - if(hole & 1) { // we find the hole
>> - mem_hole.hole_startk = (hole & (0xff<<24)) >>
>> 10;
>> - mem_hole.node_id = i; // record the node No
>> with hole
>> - break; // only one hole
>> + if (__f1_dev[i]) {
>>
>>
> Same as above. Just add another continue under the base check above.
>
Same here.
>> + hole = pci_read_config32(__f1_dev[i], 0xf0);
>> + if(hole & 1) { // we find the hole
>> + mem_hole.hole_startk = (hole &
>> (0xff<<24)) >> 10;
>> + mem_hole.node_id = i; // record the
>> node No with hole
>> + break; // only one hole
>> + }
>> }
>> }
>>
>> @@ -834,15 +836,15 @@
>> limit = f1_read_config32(0x44 + (i << 3));
>> f1_write_config32(0x44 + (i << 3),limit - (hole_sizek << 2));
>> dev = __f1_dev[i];
>> - hoist = pci_read_config32(dev, 0xf0);
>> - if(hoist & 1) {
>> - pci_write_config32(dev, 0xf0, 0);
>> + if (dev) {
>>
>>
> And the same as above.
>
Same here.
>> + hoist = pci_read_config32(dev, 0xf0);
>> + if(hoist & 1) {
>> + pci_write_config32(dev, 0xf0, 0);
>> + } else {
>> + base = pci_read_config32(dev, 0x40 + (i << 3));
>> + f1_write_config32(0x40 + (i << 3),base - (hole_sizek <<
>> 2));
>> + }
>> }
>> - else {
>> - base = pci_read_config32(dev, 0x40 + (i << 3));
>> - f1_write_config32(0x40 + (i << 3),base - (hole_sizek << 2));
>> - }
>> -
>> }
>>
>>
>>
> ...
>
>
>> Index: src/northbridge/amd/amdfam10/northbridge.c
>> ===================================================================
>> --- src/northbridge/amd/amdfam10/northbridge.c (revision 4026)
>> +++ src/northbridge/amd/amdfam10/northbridge.c (working copy)
>> @@ -339,7 +339,8 @@
>> device_t dev;
>> dev = __f0_dev[nodeid];
>> for(link = 0; !res && (link < 8); link++) {
>> - res = probe_resource(dev, 0x1000 + reg + (link<<16));
>> // 8 links, 0x1000 man f1,
>> + if (dev)
>> + res = probe_resource(dev, 0x1000 + reg +
>> (link<<16)); // 8 links, 0x1000 man f1,
>>
>>
>
> And the same.
>
Same here.
>> }
>> }
>> result = 2;
>>
Regards,
Carl-Daniel
--
http://www.hailfinger.org/
--
coreboot mailing list: [email protected]
http://www.coreboot.org/mailman/listinfo/coreboot
