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

[Mitsyanko Igor <i.mitsyanko@samsung.com>]

On 01/11/2012 08:31 PM, Mark Langsdorf wrote:

>          Removed the automatic detection and resetting of ram_size. Image MUST
> be loaded with -m 4089 or it will crash

I don't know what is maintainer's politics on this, but as a user of 
your board I don't really like that I will have to remember this strange 
number when I launch Highbank emulation. Have you considered adding an 
automatic upper limitation to user-defined ram_size?


> +    sysram = g_new(MemoryRegion, 1);
> +    memory_region_init_ram(sysram, "highbank.sysram", 0x8000);
> +    memory_region_add_subregion(sysmem, 0xfff88000, sysram);
> +    if (bios_name != NULL) {
> +        sysboot_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
> +        if (sysboot_filename != NULL) {
> +            uint32_t filesize = get_image_size(sysboot_filename);
> +            if (load_image_targphys("sysram.bin", 0xfff88000, filesize)<  0) {
> +                hw_error("Unable to load %s\n", bios_name);
> +            }

Probably should be
if (load_image_targphys(sysboot_filename, 0xfff88000, 0x8000)<  0) {
and then you don't need "uint32_t filesize" at all.

> +    dev = qdev_create(NULL, "l2x0");
> +    qdev_init_nofail(dev);
> +    busdev = sysbus_from_qdev(dev);
> +    sysbus_mmio_map(busdev, 0, 0xfff12000);

 > +    dev = qdev_create(NULL, "highbank-regs");
 > +    qdev_init_nofail(dev);
 > +    busdev = sysbus_from_qdev(dev);
 > +    sysbus_mmio_map(busdev, 0, 0xfff3c000);
 > +

You can use sysbus_create_simple() here (of course, if you didn't avoid 
it intentionally for some reason).



-- 
Mitsyanko Igor
ASWG, Moscow R&D center, Samsung Electronics
email: i.mitsyanko@samsung.com