lkml.org 
[lkml]   [2009]   [Apr]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 13/21] amd64_edac: add f10-and-later methods-p3

    * Andrew Morton <akpm@linux-foundation.org> wrote:

    > On Wed, 29 Apr 2009 20:22:55 +0200
    > Ingo Molnar <mingo@elte.hu> wrote:
    >
    > > > + InputAddr = ChannelAddrLong >> 8;
    > > > +
    > > > + debugf1(" (ChannelAddrLong=0x%llx) >> 8 becomes "
    > > > + "InputAddr=0x%x\n", ChannelAddrLong, InputAddr);
    > > > +
    > > > + /* Iterate over the DRAM DCTs looking for a
    > > > + * match for InputAddr on the selected NodeID
    > > > + */
    > > > + CSFound = f10_lookup_addr_in_dct(InputAddr,
    > > > + NodeID, ChannelSelect);
    > > > +
    > > > + if (CSFound >= 0) {
    > > > + *node_id = NodeID;
    > > > + *channel_select = ChannelSelect;
    > > > + }
    > > > + }
    > > > +
    > > > + return CSFound;
    > > > +}
    > >
    > > this function is probably too large, and also it uses some weird
    > > hungarian notation coding style. Please dont do that! It's
    > > completely unacceptable.
    >
    > These identifers (or at least, DctSelBaseOffsetLong, which is the
    > only one I googled for) come straight out of the AMD "BIOS and
    > Kernel Developer's Guide".
    >
    > Sucky though they are, there's value in making the kernel code
    > match up with the documentation.

    I'm generally resisting patches that hungarinize arch/x86/ (and heck
    there's been many attempts ...) but there's some conflicting advice
    here. I've Cc:-ed Linus, maybe he has an opinion about this.

    My gut reaction would be 'hell no'. There's other, structural
    problems with this code too, and doing some saner naming would
    mostly be a sed job and would take minimal amount of time. The
    naming can still be intuitive. The symbols from the documentation
    can perhaps be mentioned in a couple of comments to establish a
    mapping.

    Ingo


    \
     
     \ /
      Last update: 2009-04-29 21:25    [W:0.044 / U:36.132 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site