Re: [Qemu-devel] [PATCH v2 3/5] arm: xilinx_zynq: Add linux pre-boot

[Alistair Francis <alistair.francis@xilinx.com>]

On Thu, Oct 29, 2015 at 10:34 PM, Peter Crosthwaite
<crosthwaitepeter@gmail.com> wrote:
> Add a Linux-specific pre-boot routine that matches the device-
> specific bootloaders behaviour. This is needed for modern Linux that
> expects the ARM PLL in SLCR to be a more even value (not 26).
>
> Cc: Alistair Francis <alistair.francis@xilinx.com>
> Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>

This looks fine to me. I haven't looked much at the other patches though.

I think a comment in the zynq_write_board_setup() function saying what
it is doing wouldn't hurt either. Basically just copying the commit
message.

Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>

Thanks,

Alistair

> ---
> Changed since v1:
> Left main loader address unchanged
> Added comments for SLCR_WRITE macro (PMM review)
> Convert SLCR_WRITE impl. to use movw/movt (PMM review)
> Missing "-" for device-specific
> Changed since RFC:
> Use bootloader callback to load blob.
> Change "firmware" to "board-setup" for consistency.
>
>  hw/arm/xilinx_zynq.c | 42 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 42 insertions(+)
>
> diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
> index 9f89483..82a9db8 100644
> --- a/hw/arm/xilinx_zynq.c
> +++ b/hw/arm/xilinx_zynq.c
> @@ -43,6 +43,45 @@ static const int dma_irqs[8] = {
>      46, 47, 48, 49, 72, 73, 74, 75
>  };
>
> +#define BOARD_SETUP_ADDR        0x100
> +
> +#define SLCR_LOCK_OFFSET        0x004
> +#define SLCR_UNLOCK_OFFSET      0x008
> +#define SLCR_ARM_PLL_OFFSET     0x100
> +
> +#define SLCR_XILINX_UNLOCK_KEY  0xdf0d
> +#define SLCR_XILINX_LOCK_KEY    0x767b
> +
> +#define ARMV7_IMM16(x) (extract32((x),  0, 12) | \
> +                        extract32((x), 12,  4) << 16)
> +
> +/* Write immediate val to address r0 + addr. r0 should contain base offset
> + * of the SLCR block. Clobbers r1.
> + */
> +
> +#define SLCR_WRITE(addr, val) \
> +    0xe3001000 + ARMV7_IMM16(extract32((val),  0, 16)), /* movw r1 ... */ \
> +    0xe3401000 + ARMV7_IMM16(extract32((val), 16, 16)), /* movt r1 ... */ \
> +    0xe5801000 + (addr)
> +
> +static void zynq_write_board_setup(ARMCPU *cpu,
> +                                   const struct arm_boot_info *info)
> +{
> +    int n;
> +    uint32_t board_setup_blob[] = {
> +        0xe3a004f8, /* mov r0, #0xf8000000 */
> +        SLCR_WRITE(SLCR_UNLOCK_OFFSET, SLCR_XILINX_UNLOCK_KEY),
> +        SLCR_WRITE(SLCR_ARM_PLL_OFFSET, 0x00014008),
> +        SLCR_WRITE(SLCR_LOCK_OFFSET, SLCR_XILINX_LOCK_KEY),
> +        0xe12fff1e, /* bx lr */
> +    };
> +    for (n = 0; n < ARRAY_SIZE(board_setup_blob); n++) {
> +        board_setup_blob[n] = tswap32(board_setup_blob[n]);
> +    }
> +    rom_add_blob_fixed("board-setup", board_setup_blob,
> +                       sizeof(board_setup_blob), BOARD_SETUP_ADDR);
> +}
> +
>  static struct arm_boot_info zynq_binfo = {};
>
>  static void gem_init(NICInfo *nd, uint32_t base, qemu_irq irq)
> @@ -252,6 +291,9 @@ static void zynq_init(MachineState *machine)
>      zynq_binfo.nb_cpus = 1;
>      zynq_binfo.board_id = 0xd32;
>      zynq_binfo.loader_start = 0;
> +    zynq_binfo.board_setup_addr = BOARD_SETUP_ADDR;
> +    zynq_binfo.write_board_setup = zynq_write_board_setup;
> +
>      arm_load_kernel(ARM_CPU(first_cpu), &zynq_binfo);
>  }
>
> --
> 1.9.1
>
>