[PATCH v2 06/14] board_f: m68k: Factor out m68k-specific bdinfo setup

[Ovidiu Panait]

Hi Simon,

On 15.07.2020 04:05, Simon Glass wrote:
> On Fri, 10 Jul 2020 at 04:25, Ovidiu Panait <ovidiu.panait at windriver.com> wrote:
>> Factor out m68k-specific bdinfo setup to arch_setup_bdinfo in
>> arch/m68k/lib/bdinfo.c. Also, use if(IS_ENABLED()) instead of #ifdef where
>> possible.
>>
>> Signed-off-by: Ovidiu Panait <ovidiu.panait at windriver.com>
>> ---
>> v2 updates:
>> - use "if (IS_ENABLED(CONFIG_SYS_HAS_SRAM))" instead of
>>    "#ifdef CONFIG_SYS_SRAM_BASE"
>>
>>   arch/m68k/lib/bdinfo.c | 32 ++++++++++++++++++++++++++++++++
>>   common/board_f.c       | 14 ++++----------
>>   2 files changed, 36 insertions(+), 10 deletions(-)
> Reviewed-by: Simon Glass <sjg at chromium.org>
>
> See below
>
>> diff --git a/arch/m68k/lib/bdinfo.c b/arch/m68k/lib/bdinfo.c
>> index 971c47c306..3257195add 100644
>> --- a/arch/m68k/lib/bdinfo.c
>> +++ b/arch/m68k/lib/bdinfo.c
>> @@ -11,6 +11,38 @@
>>
>>   DECLARE_GLOBAL_DATA_PTR;
>>
>> +int arch_setup_bdinfo(void)
>> +{
>> +       bd_t *bd = gd->bd;
>> +
>> +       /*
>> +        * Save local variables to board info struct
>> +        */
>> +       bd->bi_memstart = CONFIG_SYS_SDRAM_BASE;        /* start of memory */
>> +       bd->bi_memsize = gd->ram_size;                  /* size in bytes */
>> +
>> +       if (IS_ENABLED(CONFIG_SYS_HAS_SRAM)) {
>> +               bd->bi_sramstart = CONFIG_SYS_SRAM_BASE; /* start of SRAM */
>> +               bd->bi_sramsize = CONFIG_SYS_SRAM_SIZE;  /* size  of SRAM */
>> +       }
> I wonder if we can do better - can't we move the SDRAM code back to
> the generic bdinfo.c ?

I am not sure that I understand, could you please explain it further?

When talking about the generic bdinfo.c, you are referring to 
cmd/bdinfo.c?  I see that cmd/bdinfo.c currently only takes care of 
printing various bd->bi_* fields, not assigning them.

Where exactly should I move the "if (IS_ENABLED(CONFIG_SYS_HAS_SRAM)) 
{...}" block?


Do you mean to create a generic setup_bdinfo that populates generic 
bd->bi_* fields and at the end calls the arch-specific variant 
arch_setup_bdinfo?


Ovidiu

>> +
>> +       bd->bi_mbar_base = CONFIG_SYS_MBAR; /* base of internal registers */
>> +
>> +       bd->bi_intfreq = gd->cpu_clk;   /* Internal Freq, in Hz */
>> +       bd->bi_busfreq = gd->bus_clk;   /* Bus Freq,      in Hz */
>> +
>> +       if (IS_ENABLED(CONFIG_PCI))
>> +               bd->bi_pcifreq = gd->pci_clk;
>> +
>> +#if defined(CONFIG_EXTRA_CLOCK)
>> +       bd->bi_inpfreq = gd->arch.inp_clk;      /* input Freq in Hz */
>> +       bd->bi_vcofreq = gd->arch.vco_clk;      /* vco Freq in Hz */
>> +       bd->bi_flbfreq = gd->arch.flb_clk;      /* flexbus Freq in Hz */
>> +#endif
>> +
>> +       return 0;
>> +}
>> +
>>   void arch_print_bdinfo(void)
>>   {
>>          bd_t *bd = gd->bd;
>> diff --git a/common/board_f.c b/common/board_f.c
>> index e597749d2f..7d65879b93 100644
>> --- a/common/board_f.c
>> +++ b/common/board_f.c
>> @@ -602,7 +602,7 @@ __weak int arch_setup_bdinfo(void)
>>          return 0;
>>   }
>>
>> -#if defined(CONFIG_M68K) || defined(CONFIG_MIPS) || defined(CONFIG_PPC) || \
>> +#if defined(CONFIG_MIPS) || defined(CONFIG_PPC) || \
>>          defined(CONFIG_SH)
>>   static int setup_board_part1(void)
>>   {
>> @@ -622,9 +622,6 @@ static int setup_board_part1(void)
>>   #if defined(CONFIG_E500) || defined(CONFIG_MPC86xx)
>>          bd->bi_immr_base = CONFIG_SYS_IMMR;     /* base  of IMMR register     */
>>   #endif
>> -#if defined(CONFIG_M68K)
>> -       bd->bi_mbar_base = CONFIG_SYS_MBAR;     /* base of internal registers */
>> -#endif
>>   #if defined(CONFIG_MPC83xx)
>>          bd->bi_immrbar = CONFIG_SYS_IMMR;
>>   #endif
>> @@ -633,7 +630,7 @@ static int setup_board_part1(void)
>>   }
>>   #endif
>>
>> -#if defined(CONFIG_PPC) || defined(CONFIG_M68K)
>> +#if defined(CONFIG_PPC)
>>   static int setup_board_part2(void)
>>   {
>>          bd_t *bd = gd->bd;
>> @@ -646,9 +643,6 @@ static int setup_board_part2(void)
>>          bd->bi_sccfreq = gd->arch.scc_clk;
>>          bd->bi_vco = gd->arch.vco_out;
>>   #endif /* CONFIG_CPM2 */
>> -#if defined(CONFIG_M68K) && defined(CONFIG_PCI)
>> -       bd->bi_pcifreq = gd->pci_clk;
>> -#endif
>>   #if defined(CONFIG_EXTRA_CLOCK)
>>          bd->bi_inpfreq = gd->arch.inp_clk;      /* input Freq in Hz */
>>          bd->bi_vcofreq = gd->arch.vco_clk;      /* vco Freq in Hz */
>> @@ -980,11 +974,11 @@ static const init_fnc_t init_sequence_f[] = {
>>          dram_init_banksize,
>>          show_dram_config,
>>          arch_setup_bdinfo,
>> -#if defined(CONFIG_M68K) || defined(CONFIG_MIPS) || defined(CONFIG_PPC) || \
>> +#if defined(CONFIG_MIPS) || defined(CONFIG_PPC) || \
>>          defined(CONFIG_SH)
>>          setup_board_part1,
>>   #endif
>> -#if defined(CONFIG_PPC) || defined(CONFIG_M68K)
>> +#if defined(CONFIG_PPC)
>>          INIT_FUNC_WATCHDOG_RESET
>>          setup_board_part2,
>>   #endif
>> --
>> 2.17.1
>>