[coreboot] [patch][v2] AMD Fam10 memory controller updates
Peter Stuge
peter at stuge.se
Mon Apr 7 23:53:38 CEST 2008
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
More information about the coreboot
mailing list