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

[Andreas Färber]

Am 12.01.2012 13:47, schrieb Mitsyanko Igor:
> On 01/11/2012 08:31 PM, Mark Langsdorf wrote:
>> +    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.

You need it either way; if you use 0x8000 there, you need to check if
filesize is actually 0x8000. Doing it this way allows to load smaller
files; a check for larger files should be added though. Thanks for
making me aware.

>> +    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).

Depends on how you read this:

/* Legacy helper function for creating devices.  */
DeviceState *sysbus_create_varargs(const char *name,
                                 target_phys_addr_t addr, ...);
DeviceState *sysbus_try_create_varargs(const char *name,
                                       target_phys_addr_t addr, ...);
static inline DeviceState *sysbus_create_simple(const char *name,
                                              target_phys_addr_t addr,
                                              qemu_irq irq)
{
    return sysbus_create_varargs(name, addr, irq, NULL);
}

I interpret it as sysbus_create_simple() using deprecated
sysbus_create_varargs() and therefore being deprecated, too.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg