[coreboot] [patch][v2] AMD Fam10 memory controller updates

Marc Jones Marc.Jones at amd.com
Mon Apr 7 23:56:33 CEST 2008


Peter Stuge wrote:
> On Mon, Apr 07, 2008 at 03:33:20PM -0600, Marc Jones wrote:
>   
>>>> -	DQSTiming_D(pMCTstat, pDCTstatA);	/* Get Receiver Enable and DQS signal timing*/
>>>> +	DQSTiming_D(pMCTstat, pDCTstatA);	/* Get Receiver Enable and DQS signal timing */
>>>> +
>>>> +	print_t("mctAutoInitMCT_D: UMAMemTyping_D\n");
>>>> +	UMAMemTyping_D(pMCTstat, pDCTstatA);	/* Fix up for UMA sizing */
>>>>         
>>> Please keep code changes and whitespace changes apart. In particular
>>> when changing things like these (that are not very well known) I
>>> think that is important.
>>>       
>> Serious?
>>     
>
> Yes, afraid so.
>
>
>   
>> You are naking over a couple whitespace changes that get fixed
>> as the code is fixed?
>>     
>
> Right.
>
>
>   
>> Do you have concerns that the patch is confused by the changes
>>     
>
> Yes, I think it is.
>
>
>   
>> or are you just naking on principal?
>>     
>
> No, not really.
>
>
> This is how I reason:
>
> Ideally whitespace changes should not be needed, all commits should
> always be formatted perfectly.
>
> In practice this isn't true, and whitespace changes are a good thing.
> I don't care all that much about whitespace formatting myself, but
> others do and I agree it does make the code as a whole look more
> tidy.
>
> But, since the basic assumption is that commits change code and
> nothing else, I think whitespace changes should be isolated commits,
> since they clutter code diffs and make human diff processing more
> difficult.
>
> I think this argument is even more important when code is changed
> that is not very well-known and well understood.
>
>
> //Peter
>
>   
So, yes, you are naking on principal.

-- 
Marc Jones
Senior Firmware Engineer
(970) 226-9684 Office
mailto:Marc.Jones at amd.com
http://www.amd.com/embeddedprocessors






More information about the coreboot mailing list