Re: [Qemu-devel] [PATCH v2 5/6] arm: SoC model for Calxeda Highbank

[Peter Maydell]

On 10 January 2012 16:45, Mark Langsdorf <address@hidden> wrote:
> From: Rob Herring <address@hidden>
>
> Adds support for Calxeda's Highbank SoC.
> +static int highbank_regs_init(SysBusDevice *dev)
> +{
> +    highbank_regs_state *s = FROM_SYSBUS(highbank_regs_state, dev);
> +
> +    s->iomem = g_new(MemoryRegion, 1);
> +    s->regs[0x40] = 0x05F20121;
> +    s->regs[0x41] = 0x2;
> +    s->regs[0x42] = 0x05F30121;
> +    s->regs[0x43] = 0x05F40121;

Do these reset to these values as well? (ie should they be in a reset function?)

> +    memory_region_init_io(s->iomem, &hb_mem_ops, s->regs, "highbank_regs",
> +                          0x1000);

You also need
  sysbus_init_mmio(dev, &s->iomem);
to expose the memory region.

> +    dev = qdev_create(NULL, "highbank-regs");
> +    qdev_init_nofail(dev);
> +    busdev = sysbus_from_qdev(dev);
> +    s = FROM_SYSBUS(highbank_regs_state, busdev);
> +    memory_region_add_subregion(sysmem, 0xfff3c000, s->iomem);

...then you can just use sysbus_mmio_map() rather than fishing
around in the internal state struct.

> +    highbank_binfo.ram_size = ram_size;
> +    highbank_binfo.kernel_filename = kernel_filename;
> +    highbank_binfo.kernel_cmdline = kernel_cmdline;
> +    highbank_binfo.initrd_filename = initrd_filename;
> +    highbank_binfo.board_id = -1; /* provided by deviceTree */
> +    arm_load_kernel(env, &highbank_binfo);

Does your kernel really have the code at the start to cope with
the secondary CPUs so it doesn't need the code in arm_boot.c?
Most kernels don't (and so the board has to set .nb_cpus,
.loader_start (and once the Samsung series lands, the field
for where the secondary boot loader's "release cores now"
flag is).

-- PMM