On Thu, Jun 14, 2007 at 07:45:36PM +0200, [EMAIL PROTECTED] wrote:
> New Revision: 351
> 
> Added:
>    LinuxBIOSv3/arch/x86/geodelx.c

In general very nice!


But.. :)


> Per new standards, naming conventions have changed.

Not new by any means I think, they were just ignored for the rather
big initial LX patches to get them in before they bitrotted.


> +void pll_reset(int manualconf, u32 pll_hi, u32 pll_lo)
> +{
> +     msr_t msrGlcpSysRstpll;

Variable name casing?


> +void set_delay_control(u8 dimm0, u8 dimm1)

I like the simpler comments in here, but ..

> +/* The current thinking. Subject to change...
> +
> +;                                                               "FUTURE 
> ROBUSTNESS" PROPOSAL
> +;                                                               
> ----------------------------
> +;            DIMM     Max MBUS                                          MC 
> 0x2000001A bits 26:24
> +;DIMMs       devices  Frequency       MCP 0x4C00000F Setting          vvv
> +;-----       -------  ---------       ----------------------  ----------
> +;1            4               400MHz          0x82*100FF 0x56960004          
>   4
> +;1            8               400MHz          0x82*100AA 0x56960004          
>   4
> +;1            16              400MHz          0x82*10055 0x56960004          
>   4
> +;
> +;2            4,4     400MHz          0x82710000 0x56960004            4
> +;2            8,8     400MHz          0xC27100A5 0x56960004            4     
> *** OUT OF PUBLISHED ENVELOPE ***
> +;
> +;2           16,4     >333            0xB27100A5 0x56960004            4     
> *** OUT OF PUBLISHED ENVELOPE ***
> +;2           16,8     >333            0xB27100A5 0x56960004            4     
> *** OUT OF PUBLISHED ENVELOPE ***
> +;2           16,16    >333            0xB2710000 0x56960004            4     
> *** OUT OF PUBLISHED ENVELOPE ***
> +;
> +;1            4               <=333MHz        0x83*100FF 0x56960004          
>   3
> +;1            8               <=333MHz        0x83*100AA 0x56960004          
>   3
> +;1            16              <=333MHz        0x83*100AA 0x56960004          
>   3
> +;
> +;2            4,4     <=333MHz        0x837100A5 0x56960004            3
> +;2            8,8     <=333MHz        0x937100A5 0x56960004            3
> +;
> +;2           16,4     <=333MHz        0xB37100A5 0x56960004            3     
> *** OUT OF PUBLISHED ENVELOPE ***
> +;2           16,8     <=333MHz        0xB37100A5 0x56960004            3     
> *** OUT OF PUBLISHED ENVELOPE ***
> +;2           16,16    <=333MHz        0xB37100A5 0x56960004            3     
> *** OUT OF PUBLISHED ENVELOPE ***
> +;=========================================================================
> +;* - Bit 55 (disable SDCLK 1,3,5) should be set if there is a single DIMM in 
> slot 0,
> +;     but it should be clear for all 2 DIMM settings and if a single DIMM is 
> in slot 1.
> +;     Bits 54:52 should always be set to '111'.
> +
> +;No VTT termination
> +;-------------------------------------
> +;ADDR/CTL have 22 ohm series R
> +;DQ/DQM/DQS have 33 ohm series R
> +;
> +;            DIMM     Max MBUS
> +;DIMMs       devices  Frequency       MCP 0x4C00000F Setting
> +;-----       -------  ---------       ----------------------
> +;1            4               400MHz          0xF2F100FF 0x56960004          
>   4                     The MC changes improve Salsa.
> +;1            8               400MHz          0xF2F100FF 0x56960004          
>   4                     Delay controls no real change,
> +;1            4               <=333MHz        0xF2F100FF 0x56960004          
>   3                     just fixing typo in left side.
> +;1            8               <=333MHz        0xF2F100FF 0x56960004          
>   3
> +;1            16              <=333MHz        0xF2F100FF 0x56960004          
>   3
> +*/

This thing is pretty misplaced. It's lovely documentation that
deserves a better home than smack dab in the midst of code.

At the very least doxygen?


> + * cpu_reg_init. All cpu register settings, here in one place, and
> + * done in the proper order.
> + *
> + * @param debug_clock_disable Disable the debug clock to save power. 
> Currently ignored, but we need to 
> + * pick this up from a CMOS setting in future. 

Long line.

> +     /*      GLIU port active enable, limit south pole masters (AES and PCI) 
> to one outstanding transaction. */

..again.. and maybe a few more places too.


Enthousiastic thumbs up from me anyway. :)


//Peter

-- 
linuxbios mailing list
[email protected]
http://www.linuxbios.org/mailman/listinfo/linuxbios

Reply via email to