Re: [PATCH v8 12/14] hw/arm/npcm7xx: add board setup stub for CPU and UART clocks

[Philippe Mathieu-Daudé]

On 8/25/20 2:17 AM, Havard Skinnemoen via wrote:
> When booting directly into a kernel, bypassing the boot loader, the CPU and
> UART clocks are not set up correctly. This makes the system appear very
> slow, and causes the initrd boot test to fail when optimization is off.
> 
> The UART clock must run at 24 MHz. The default 25 MHz reference clock
> cannot achieve this, so switch to PLL2/2 @ 480 MHz, which works
> perfectly with the default /20 divider.
> 
> The CPU clock should run at 800 MHz, so switch it to PLL1/2. PLL1 runs
> at 800 MHz by default, so we need to double the feedback divider as well
> to make it run at 1600 MHz (so PLL1/2 runs at 800 MHz).
> 
> We don't bother checking for PLL lock because we know our emulated PLLs
> lock instantly.
> 
> Signed-off-by: Havard Skinnemoen <hskinnemoen@google.com>
> ---
>  include/hw/arm/npcm7xx.h |  1 +
>  hw/arm/npcm7xx.c         | 32 ++++++++++++++++++++++++++++++++
>  2 files changed, 33 insertions(+)
> 
> diff --git a/include/hw/arm/npcm7xx.h b/include/hw/arm/npcm7xx.h
> index 78d0d78c52..13106af215 100644
> --- a/include/hw/arm/npcm7xx.h
> +++ b/include/hw/arm/npcm7xx.h
> @@ -37,6 +37,7 @@
>  #define NPCM7XX_SMP_LOADER_START        (0xffff0000)  /* Boot ROM */
>  #define NPCM7XX_SMP_BOOTREG_ADDR        (0xf080013c)  /* GCR.SCRPAD */
>  #define NPCM7XX_GIC_CPU_IF_ADDR         (0xf03fe100)  /* GIC within A9 */
> +#define NPCM7XX_BOARD_SETUP_ADDR        (0xffff1000)  /* Boot ROM */
>  
>  typedef struct NPCM7xxMachine {
>      MachineState        parent;
> diff --git a/hw/arm/npcm7xx.c b/hw/arm/npcm7xx.c
> index 7884b2b03d..037f3a26f2 100644
> --- a/hw/arm/npcm7xx.c
> +++ b/hw/arm/npcm7xx.c
> @@ -55,6 +55,13 @@
>  #define NPCM7XX_ROM_BA          (0xffff0000)
>  #define NPCM7XX_ROM_SZ          (64 * KiB)
>  
> +/* Clock configuration values to be fixed up when bypassing bootloader */
> +
> +/* Run PLL1 at 1600 MHz */
> +#define NPCM7XX_PLLCON1_FIXUP_VAL   (0x00402101)
> +/* Run the CPU from PLL1 and UART from PLL2 */
> +#define NPCM7XX_CLKSEL_FIXUP_VAL    (0x004aaba9)
> +
>  /*
>   * Interrupt lines going into the GIC. This does not include internal 
> Cortex-A9
>   * interrupts.
> @@ -132,6 +139,29 @@ static const struct {
>      },
>  };
>  
> +static void npcm7xx_write_board_setup(ARMCPU *cpu,
> +                                      const struct arm_boot_info *info)
> +{
> +    uint32_t board_setup[] = {
> +        0xe59f0010,     /* ldr r0, clk_base_addr */
> +        0xe59f1010,     /* ldr r1, pllcon1_value */
> +        0xe5801010,     /* str r1, [r0, #16] */
> +        0xe59f100c,     /* ldr r1, clksel_value */
> +        0xe5801004,     /* str r1, [r0, #4] */
> +        0xe12fff1e,     /* bx lr */
> +        NPCM7XX_CLK_BA,
> +        NPCM7XX_PLLCON1_FIXUP_VAL,
> +        NPCM7XX_CLKSEL_FIXUP_VAL,
> +    };
> +    int i;
> +
> +    for (i = 0; i < ARRAY_SIZE(board_setup); i++) {
> +        board_setup[i] = tswap32(board_setup[i]);
> +    }
> +    rom_add_blob_fixed("board-setup", board_setup, sizeof(board_setup),
> +                       info->board_setup_addr);
> +}
> +
>  static void npcm7xx_write_secondary_boot(ARMCPU *cpu,
>                                           const struct arm_boot_info *info)
>  {
> @@ -170,6 +200,8 @@ static struct arm_boot_info npcm7xx_binfo = {
>      .gic_cpu_if_addr        = NPCM7XX_GIC_CPU_IF_ADDR,
>      .write_secondary_boot   = npcm7xx_write_secondary_boot,
>      .board_id               = -1,
> +    .board_setup_addr       = NPCM7XX_BOARD_SETUP_ADDR,
> +    .write_board_setup      = npcm7xx_write_board_setup,
>  };
>  
>  void npcm7xx_load_kernel(MachineState *machine, NPCM7xxState *soc)
> 

Very very clean :) :)

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>