[coreboot] Patch for Nokia-IP530, now with working PIRQ table, patch on 5591

Myles Watson mylesgw at gmail.com
Wed May 26 23:14:33 CEST 2010


On Wed, May 26, 2010 at 2:15 PM, mbertens <mbertens at xs4all.nl> wrote:
> On Wed, 2010-05-26 at 11:00 -0600, Myles Watson wrote:
>> +                     // fix made by Marc Bertens <mbertens at xs4all.nl>
>> +                     if (link > 0x5f) {
>> +                             // This is basically for the 440BX
>> +                             link -= 0x5f;
>> +                     }
>>
>> I'd prefer this to be guarded by
>> #if CONFIG_NORTHBRIDGE_INTEL_440BX (or whatever the correct one is)
> I was thinking of it to put it that way, but i'd. But i will make the
> changes to the code.
>>
>> It would also be nice to have an explanation.
> And give more explaination why the change was made.
>>
>> The rest of your patch touches a lot of code with little explanation.  It
>> takes a lot more time to review patches like that.  For a faster review you
>> should split it up into pieces that add functionality.  For example, the
> I will give the patches in seperate diffs, with more explainations
Great.

>> heap size (which seems really large) part of the patch should have an
>> explanation of what problem you see with a normal heap size.
> I was running in to problems with the heap size, therefor i increased it
> to such a value that it would not bother me again :-). I will decrease
> the value for it to see on which value it needs to be.

The reason why we care is that usually your device tree is wrong if
you run out of heap size.  We'd rather fix the root problem.

> This is my first attempt to develop in an open source environment. And
> i'm still learning things ie "the coding standard", i hope that i'm not
> to much trouble, i will get it right one day :-)

It's not too much trouble.  Thanks for contributing.

Thanks,
Myles




More information about the coreboot mailing list