Re: [coreboot] [PATCH] add support for 256 byte CMOS

[Carl-Daniel Hailfinger]

On 22.04.2009 10:39, Stefan Reinauer wrote:
> Allow coreboot to use the full 256 bytes of CMOS memory
> Make functions out of the accessor macros in mc146818rtc.c
> don't hide reserved cmos entries from coreboot, only from the user.
>
> Signed-off-by: Stefan Reinauer

This has the potential to break ROMCC targets. If it survives abuild, it
is (with the review addressed)
Acked-by: Carl-Daniel Hailfinger
> Index: util/options/build_opt_tbl.c
>
> --- util/options/build_opt_tbl.c	(revision 4168)
> +++ util/options/build_opt_tbl.c	(working copy)
> @@ -8,7 +8,8 @@
>  #include "../../src/include/pc80/mc146818rtc.h"
>  #include "../../src/include/boot/coreboot_tables.h"
>
> -#define CMOS_IMAGE_BUFFER_SIZE 128
> +//#define CMOS_IMAGE_BUFFER_SIZE 128
> +#define CMOS_IMAGE_BUFFER_SIZE 256

Please kill the commented out value or add an explanatory comment.
> #define INPUT_LINE_MAX 256
>  #define MAX_VALUE_BYTE_LENGTH 64
>
> @@ -563,9 +564,12 @@
>    			continue;
>    		}
>    		ce = (struct cmos_entries *)ptr;
> +#if 0

I'm not happy about that #if 0. If the code is definitely unwanted, kill
it. If we may want later, maybe move the comment below to the line
containing the #if 0.
> 	// Internally, we don't want to ignore reserved values
> if (ce->config == 'r') {
> 	continue;
> }
>
> +#endif
>    		if (!is_ident((char *)ce->name)) {
>    			fprintf(stderr, "Invalid identifier: %s\n",
>    				ce->name);

Regards,
Carl-Daniel