[PATCH] Hide CMOS_VSTART_ and CMOS_VLEN_ prefixes for read_option() use

[Patrick Georgi]

[let's try again]
Hi,

as a first step to the CMOS infrastructure project (as detailed in the
wiki), this patch changes read_option() by wrapping it in a macro,
simplifying its use from
read_option(CMOS_VSTART_foo, CMOS_VLEN_foo, somedefault)
to
read_option(foo, somedefault)

Signed-off-by: Patrick Georgi

[Stefan Reinauer]

* Patrick Georgi  [110510 21:23]:
> [let's try again]
> Hi,
>
> as a first step to the CMOS infrastructure project (as detailed in the
> wiki), this patch changes read_option() by wrapping it in a macro,
> simplifying its use from
> read_option(CMOS_VSTART_foo, CMOS_VLEN_foo, somedefault)
> to
> read_option(foo, somedefault)
>
> Signed-off-by: Patrick Georgi
> [PATCH] Hide CMOS_VSTART_ and CMOS_VLEN_ prefixes for read_option() use
>
> Signed-off-by: Patrick Georgi

Acked-by: Stefan Reinauer
> src/cpu/amd/dualcore/dualcore.c           |    2 +-
>  src/cpu/amd/model_10xxx/init_cpus.c       |    2 +-
>  src/cpu/amd/model_fxx/init_cpus.c         |    2 +-
>  src/cpu/amd/quadcore/quadcore.c           |    2 +-
>  src/include/pc80/mc146818rtc.h            |    5 +++--
>  src/lib/uart8250.c                        |    2 +-
>  src/mainboard/getac/p470/romstage.c       |    2 +-
>  src/mainboard/kontron/986lcd-m/romstage.c |    8 ++++----
>  src/mainboard/roda/rk886ex/romstage.c     |    2 +-
>  src/northbridge/amd/amdk8/coherent_ht.c   |    2 +-
>  src/northbridge/amd/amdk8/raminit.c       |    6 +++---
>  src/northbridge/amd/amdk8/raminit_f.c     |    6 +++---
>  src/northbridge/intel/e7520/raminit.c     |    2 +-
>  src/northbridge/intel/e7525/raminit.c     |    2 +-
>  src/pc80/mc146818rtc_early.c              |    2 +-
>  15 files changed, 24 insertions(+), 23 deletions(-)

Wow are those the only occurences of readoption in romstage code?

Should we document that somewhere?
> diff --git a/src/cpu/amd/dualcore/dualcore.c b/src/cpu/amd/dualcore/dualcore.c
> index 5206b07..9c2583f 100644
> --- a/src/cpu/amd/dualcore/dualcore.c
> +++ b/src/cpu/amd/dualcore/dualcore.c
> @@ -49,7 +49,7 @@ static inline void start_other_cores(void)
>    unsigned nodes;
>    unsigned nodeid;
> if (read_option(CMOS_VSTART_multi_core, CMOS_VLEN_multi_core, 0))  {
> if (read_option(multi_core, 0))  {
> return; // disable multi_core
> }
>
> diff --git a/src/cpu/amd/model_10xxx/init_cpus.c b/src/cpu/amd/model_10xxx/init_cpus.c
> index a256f9c..e0538af 100644
> --- a/src/cpu/amd/model_10xxx/init_cpus.c
> +++ b/src/cpu/amd/model_10xxx/init_cpus.c
> @@ -72,7 +72,7 @@ static void for_each_ap(u32 bsp_apicid, u32 core_range, process_ap_t process_ap,
>    nodes = get_nodes();
>
> if (!CONFIG_LOGICAL_CPUS ||
>    read_option(CMOS_VSTART_multi_core, CMOS_VLEN_multi_core, 0) != 0) {	// 0 means multi core
>    read_option(multi_core, 0) != 0) {	// 0 means multi core
> disable_siblings = 1;
>  } else {
>   disable_siblings = 0;
>
> diff --git a/src/cpu/amd/model_fxx/init_cpus.c b/src/cpu/amd/model_fxx/init_cpus.c
> index 570cb4e..0036277 100644
> --- a/src/cpu/amd/model_fxx/init_cpus.c
> +++ b/src/cpu/amd/model_fxx/init_cpus.c
> @@ -25,7 +25,7 @@ static void for_each_ap(u32 bsp_apicid, u32 core_range, process_ap_t process_ap,
>    nodes = get_nodes();
>
> if (!CONFIG_LOGICAL_CPUS ||
>    read_option(CMOS_VSTART_multi_core, CMOS_VLEN_multi_core, 0) != 0) {	// 0 means multi core
>    read_option(multi_core, 0) != 0) {	// 0 means multi core
> disable_siblings = 1;
>  } else {
>   disable_siblings = 0;
>
> diff --git a/src/cpu/amd/quadcore/quadcore.c b/src/cpu/amd/quadcore/quadcore.c
> index e7d9c7b..3bc97a3 100644
> --- a/src/cpu/amd/quadcore/quadcore.c
> +++ b/src/cpu/amd/quadcore/quadcore.c
> @@ -82,7 +82,7 @@ static void start_other_cores(void)
>    u32 nodeid;
>
> // disable multi_core
> if (read_option(CMOS_VSTART_multi_core, CMOS_VLEN_multi_core, 0) != 0)  {
> if (read_option(multi_core, 0) != 0)  {
> printk(BIOS_DEBUG, "Skip additional core init\n");
> return;
> }
>
> diff --git a/src/include/pc80/mc146818rtc.h b/src/include/pc80/mc146818rtc.h
> index 032e385..7be552d 100644
> --- a/src/include/pc80/mc146818rtc.h
> +++ b/src/include/pc80/mc146818rtc.h
> @@ -110,15 +110,16 @@ static inline void cmos_write(unsigned char val, unsigned char addr)
>  void rtc_init(int invalid);
>  #if CONFIG_USE_OPTION_TABLE
>  int get_option(void *dest, const char *name);
> -unsigned read_option(unsigned start, unsigned size, unsigned def);
> +unsigned read_option_lowlevel(unsigned start, unsigned size, unsigned def);
>  #else
>  static inline int get_option(void *dest __attribute__((unused)),
>    const char *name __attribute__((unused))) { return -2; }
> -static inline unsigned read_option(unsigned start, unsigned size, unsigned def)
> +static inline unsigned read_option_lowlevel(unsigned start, unsigned size, unsigned def)
>    { return def; }
>  #endif
>  #else
>  #include <pc80/mc146818rtc_early.c>
>  #endif
> +#define read_option(name, default) read_option_lowlevel(CMOS_VSTART_ ##name, CMOS_VLEN_ ##name, (default))
>
> #endif /*  PC80_MC146818RTC_H */
> diff --git a/src/lib/uart8250.c b/src/lib/uart8250.c
> index 49dc591..64e8854 100644
> --- a/src/lib/uart8250.c
> +++ b/src/lib/uart8250.c
> @@ -99,7 +99,7 @@ void uart_init(void)
>    static const unsigned char divisor[8] = { 1, 2, 3, 6, 12, 24, 48, 96 };
>    unsigned b_index = 0;
>  #if defined(__PRE_RAM__)
> b_index = read_option(CMOS_VSTART_baud_rate, CMOS_VLEN_baud_rate, 0);
> b_index = read_option(baud_rate, 0);
> b_index &= 7;
> div = divisor[b_index];
>
> #else
> diff --git a/src/mainboard/getac/p470/romstage.c b/src/mainboard/getac/p470/romstage.c
> index 582a7bd..0298404 100644
> --- a/src/mainboard/getac/p470/romstage.c
> +++ b/src/mainboard/getac/p470/romstage.c
> @@ -83,7 +83,7 @@ void setup_ich7_gpios(void)
>  static void ich7_enable_lpc(void)
>  {
>    int lpt_en = 0;
> if (read_option(CMOS_VSTART_lpt, CMOS_VLEN_lpt, 0) != 0) {
> if (read_option(lpt, 0) != 0) {
>      lpt_en = 1<<2; // enable LPT
> }
> // Enable Serial IRQ
>
> diff --git a/src/mainboard/kontron/986lcd-m/romstage.c b/src/mainboard/kontron/986lcd-m/romstage.c
> index 357adec..0d6816c 100644
> --- a/src/mainboard/kontron/986lcd-m/romstage.c
> +++ b/src/mainboard/kontron/986lcd-m/romstage.c
> @@ -59,7 +59,7 @@ void setup_ich7_gpios(void)
>  static void ich7_enable_lpc(void)
>  {
>    int lpt_en = 0;
> if (read_option(CMOS_VSTART_lpt, CMOS_VLEN_lpt, 0) != 0) {
> if (read_option(lpt, 0) != 0) {
> lpt_en = 1<<2; // enable LPT
> }
> // Enable Serial IRQ
>
> @@ -228,18 +228,18 @@ static void rcba_config(void)
>    reg32 = FD_ACMOD|FD_ACAUD|FD_PATA;
>    reg32 |= FD_PCIE6|FD_PCIE5|FD_PCIE4;
> if (read_option(CMOS_VSTART_ethernet1, CMOS_VLEN_ethernet1, 0) != 0) {
> if (read_option(ethernet1, 0) != 0) {
> printk(BIOS_DEBUG, "Disabling ethernet adapter 1.\n");
> reg32 |= FD_PCIE1;
> }
> if (read_option(CMOS_VSTART_ethernet2, CMOS_VLEN_ethernet2, 0) != 0) {
> if (read_option(ethernet2, 0) != 0) {
> printk(BIOS_DEBUG, "Disabling ethernet adapter 2.\n");
> reg32 |= FD_PCIE2;
> } else {
> if (reg32 & FD_PCIE1)
> 	port_shuffle = 1;
> }
> if (read_option(CMOS_VSTART_ethernet3, CMOS_VLEN_ethernet3, 0) != 0) {
> if (read_option(ethernet3, 0) != 0) {
> printk(BIOS_DEBUG, "Disabling ethernet adapter 3.\n");
> reg32 |= FD_PCIE3;
> } else {
>
> diff --git a/src/mainboard/roda/rk886ex/romstage.c b/src/mainboard/roda/rk886ex/romstage.c
> index 158e47c..9b9de25 100644
> --- a/src/mainboard/roda/rk886ex/romstage.c
> +++ b/src/mainboard/roda/rk886ex/romstage.c
> @@ -70,7 +70,7 @@ void setup_ich7_gpios(void)
>  static void ich7_enable_lpc(void)
>  {
>    int lpt_en = 0;
> if (read_option(CMOS_VSTART_lpt, CMOS_VLEN_lpt, 0) != 0) {
> if (read_option(lpt, 0) != 0) {
>      lpt_en = 1<<2; // enable LPT
> }
> // Enable Serial IRQ
>
> diff --git a/src/northbridge/amd/amdk8/coherent_ht.c b/src/northbridge/amd/amdk8/coherent_ht.c
> index f66e2cf..8b50eed 100644
> --- a/src/northbridge/amd/amdk8/coherent_ht.c
> +++ b/src/northbridge/amd/amdk8/coherent_ht.c
> @@ -1599,7 +1599,7 @@ static void coherent_ht_finalize(unsigned nodes)
>  #if CONFIG_LOGICAL_CPUS==1
>    unsigned total_cpus;
> if (read_option(CMOS_VSTART_multi_core, CMOS_VLEN_multi_core, 0) == 0) { /* multi_core */
> if (read_option(multi_core, 0) == 0) { /* multi_core */
> total_cpus = verify_dualcore(nodes);
> }
> else {
>
> diff --git a/src/northbridge/amd/amdk8/raminit.c b/src/northbridge/amd/amdk8/raminit.c
> index 2b76cff..3583dc3 100644
> --- a/src/northbridge/amd/amdk8/raminit.c
> +++ b/src/northbridge/amd/amdk8/raminit.c
> @@ -548,7 +548,7 @@ static void hw_enable_ecc(const struct mem_controller *ctrl)
>    if (nbcap & NBCAP_ECC) {
>    	dcl |= DCL_DimmEccEn;
>    }
> if (read_option(CMOS_VSTART_ECC_memory, CMOS_VLEN_ECC_memory, 1) == 0) {
> if (read_option(ECC_memory, 1) == 0) {
> dcl &= ~DCL_DimmEccEn;
> }
> pci_write_config32(ctrl->f2, DRAM_CONFIG_LOW, dcl);
>
> @@ -1100,7 +1100,7 @@ static void order_dimms(const struct mem_controller *ctrl)
>  {
>    unsigned long tom_k, base_k;
> if (read_option(CMOS_VSTART_interleave_chip_selects, CMOS_VLEN_interleave_chip_selects, 1) != 0) {
> if (read_option(interleave_chip_selects, 1) != 0) {
> tom_k = interleave_chip_selects(ctrl);
> } else {
> printk(BIOS_DEBUG, "Interleaving disabled\n");
>
> @@ -1530,7 +1530,7 @@ static struct spd_set_memclk_result spd_set_memclk(const struct mem_controller *
>
> min_cycle_time = min_cycle_times[(value >> NBCAP_MEMCLK_SHIFT) & NBCAP_MEMCLK_MASK];
>    bios_cycle_time = min_cycle_times[
> read_option(CMOS_VSTART_max_mem_clock, CMOS_VLEN_max_mem_clock, 0)];
> read_option(max_mem_clock, 0)];
> if (bios_cycle_time > min_cycle_time) {
> min_cycle_time = bios_cycle_time;
> }
>
> diff --git a/src/northbridge/amd/amdk8/raminit_f.c b/src/northbridge/amd/amdk8/raminit_f.c
> index 9ccc56b..3e6118d 100644
> --- a/src/northbridge/amd/amdk8/raminit_f.c
> +++ b/src/northbridge/amd/amdk8/raminit_f.c
> @@ -1108,7 +1108,7 @@ static unsigned long interleave_chip_selects(const struct mem_controller *ctrl,
>     * and if so count them.
>     */
>  #if defined(CMOS_VSTART_interleave_chip_selects)
> if (read_option(CMOS_VSTART_interleave_chip_selects, CMOS_VLEN_interleave_chip_selects, 1) == 0)
> if (read_option(interleave_chip_selects, 1) == 0)
> return 0;
>
> #else
>  #if !CONFIG_INTERLEAVE_CHIP_SELECTS
> @@ -1806,7 +1806,7 @@ static struct spd_set_memclk_result spd_set_memclk(const struct mem_controller *
>    min_cycle_time = min_cycle_times[(value >> NBCAP_MEMCLK_SHIFT) & NBCAP_MEMCLK_MASK];
>    bios_cycle_time = min_cycle_times[
>  #ifdef CMOS_VSTART_max_mem_clock
> read_option(CMOS_VSTART_max_mem_clock, CMOS_VLEN_max_mem_clock, 0)
> read_option(max_mem_clock, 0)
>
> #else
>  #if defined(CONFIG_MAX_MEM_CLOCK)
>    	CONFIG_MAX_MEM_CLOCK
> @@ -2375,7 +2375,7 @@ static void set_ecc(const struct mem_controller *ctrl,
>    	dcl |= DCL_DimmEccEn;
>    }
>  #ifdef CMOS_VSTART_ECC_memory
> if (read_option(CMOS_VSTART_ECC_memory, CMOS_VLEN_ECC_memory, 1) == 0) {
> if (read_option(ECC_memory, 1) == 0) {
> dcl &= ~DCL_DimmEccEn;
> }
>
> #else // CMOS_VSTART_ECC_memory not defined
> diff --git a/src/northbridge/intel/e7520/raminit.c b/src/northbridge/intel/e7520/raminit.c
> index 6eed196..7a0661c 100644
> --- a/src/northbridge/intel/e7520/raminit.c
> +++ b/src/northbridge/intel/e7520/raminit.c
> @@ -618,7 +618,7 @@ static int spd_set_dram_controller_mode(const struct mem_controller *ctrl,
>    }
>    ecc = 2;
>  #if CONFIG_HAVE_OPTION_TABLE
> if (read_option(CMOS_VSTART_ECC_memory, CMOS_VLEN_ECC_memory, 1) == 0) {
> if (read_option(ECC_memory, 1) == 0) {
> ecc = 0;  /* ECC off in CMOS so disable it */
> print_debug("ECC off\n");
> } else
>
> diff --git a/src/northbridge/intel/e7525/raminit.c b/src/northbridge/intel/e7525/raminit.c
> index aabe8cf..495a142 100644
> --- a/src/northbridge/intel/e7525/raminit.c
> +++ b/src/northbridge/intel/e7525/raminit.c
> @@ -624,7 +624,7 @@ static int spd_set_dram_controller_mode(const struct mem_controller *ctrl,
>    }
>    ecc = 2;
>  #if CONFIG_HAVE_OPTION_TABLE
> if (read_option(CMOS_VSTART_ECC_memory,CMOS_VLEN_ECC_memory,1) == 0) {
> if (read_option(ECC_memory, 1) == 0) {
> ecc = 0;  /* ECC off in CMOS so disable it */
> print_debug("ECC off\n");
> } else
>
> diff --git a/src/pc80/mc146818rtc_early.c b/src/pc80/mc146818rtc_early.c
> index d09d6b9..abddf87 100644
> --- a/src/pc80/mc146818rtc_early.c
> +++ b/src/pc80/mc146818rtc_early.c
> @@ -92,7 +92,7 @@ static inline int do_normal_boot(void)
>    return (byte & (1<<1));
>  }
>
> -unsigned read_option(unsigned start, unsigned size, unsigned def)
> +unsigned read_option_lowlevel(unsigned start, unsigned size, unsigned def)
>  {
>  #if CONFIG_USE_OPTION_TABLE
>    unsigned byte;
> -- 
> 1.7.4.4
>

[Patrick Georgi]

Am 10.05.2011 22:07, schrieb Stefan Reinauer:
> > Signed-off-by: Patrick Georgi
>
> Acked-by: Stefan Reinauer

r6565, thanks.
> > 15 files changed, 24 insertions(+), 23 deletions(-)
>
> Wow are those the only occurences of readoption in romstage code?

read_option is used in romstage, get_option in ramstage. The latter
doesn't provide an option for default values.
I intend to change that once we declare defaults in cmos.layout (or its
successor), so the romstage gets the defaults from the same source.

But yes, there aren't any more read_option uses. Probably a sign that
the nvram subsystem is underused?
> Should we document that somewhere?

Document what?

Patrick

[Stefan Reinauer]

* Patrick Georgi  [110510 23:55]:
> Am 10.05.2011 22:07, schrieb Stefan Reinauer:
> > > Signed-off-by: Patrick Georgi
> >
> > Acked-by: Stefan Reinauer
>
> r6565, thanks.

thanks.
> But yes, there aren't any more read_option uses. Probably a sign that
> the nvram subsystem is underused?

Yes, given how complex most cmos.layouts look, I was surprised.

Is there an easy way to clean those cmos.layout files up?
> > Should we document that somewhere?
>
> Document what?

Which nvram variables are used in romstage (and by whom). e.g. that
multi_core does only make sense on AMD CPUs. Does it matter?

[Patrick Georgi]

Am 11.05.2011 00:04, schrieb Stefan Reinauer:
> Yes, given how complex most cmos.layouts look, I was surprised.
> Is there an easy way to clean those cmos.layout files up?

Obsolete build_opt_tbl by moving its functionality to nvramtool (that's
WIP here), define a new format (eg. the one on Infrastructure_Projects)
and implement that in nvramtool with both reader and writer. Then read
all cmos.layout files and write in their new form.

Not exactly "easy", but automated.
> Which nvram variables are used in romstage (and by whom). e.g. that
> multi_core does only make sense on AMD CPUs. Does it matter?

Infrastructure_Projects :-)
I'd like to move nvram variables to where they are used (eg. multi_core
to AMD CPUs).

Patrick