Re: [coreboot] [PATCH] libpayload: Fix upper 128 NVRAM bytes handling

[Paul Menzel]

Dear Uwe,

comment inline.

Am Samstag, den 29.03.2008, 02:52 +0100 schrieb Uwe Hermann:
> Fix the CMOS/NVRAM access functions to work correctly for the
> upper 128 bytes of NVRAM (if enabled).
>
> For most chipsets this means using I/O ports 0x72/0x73, but at least
> on some VIA chipsets (I tested the VIA VT8237R on actual hardware)
> these ports won't work and you have to use 0x74/0x75. Thus, make this
> a Kconfig option for now.
>
> Signed-off-by: Uwe Hermann
> Index: Config.in
>
> --- Config.in   (Revision 3193)
> +++ Config.in   (Arbeitskopie)
> @@ -72,6 +72,22 @@
>         bool "Support for reading/writing CMOS bytes"
>         default y
>
> +config RTC_PORT_EXTENDED_VIA
>   bool "Extended RTC ports are 0x74/0x75"
>   default n
>   help
>     For recent chipsets with 256 CMOS bytes, you have to access the
>     upper 128 bytes (128 - 255) using two different I/O ports,
>     usually 0x72/0x73.

Just nitpicking. I do not know what the guidelines say.

According to [1] there are no spaces used before and after a dash.
Secondly if the file supports UTF-8 we could use a dash (–) instead of a
hyphen (-). (Good to have the NEO-layout [2]. ;) )
>     On some chipsets this can be a different set of ports, though.
>     The VIA VT8237R for example only recognizes the ports 0x74/0x75
>     for accessing the high 128 CMOS bytes (as seems to be the case for
>     multiple VIA chipsets).
>     If you want to read or write CMOS bytes on computers with one of
>     these chipsets, say 'y' here.
>
> endmenu
>
> menu "Build Options"
> Index: drivers/cmos.c
> ===================================================================
> --- drivers/cmos.c      (Revision 3193)
> +++ drivers/cmos.c      (Arbeitskopie)
> @@ -42,7 +42,28 @@
>
> #include <libpayload.h>
>
> -#define RTC_PORT 0x70
> +/**
> PCs can have either 64 (very old ones), 128, or 256 bytes of CMOS RAM.
> Usually you access the lower 128 CMOS bytes via I/O port 0x70/0x71.
> For more recent chipsets with 256 bytes, you have to access the upper
> 128 bytes (128 - 255) using two different registers, usually 0x72/0x73.

Same as above.

Thanks,

Paul

[1] http://en.wikipedia.org/wiki/Wikipedia:Manual_of_Style#En_dashes
[2] http://de.wikipedia.org/wiki/NEO-Tastaturbelegung