Re: [Qemu-devel] [PATCH v2 14/22] pflash_cfi01/pflash_cfi02: convert to memory API

[Jan Kiszka]

On 2011-08-24 15:40, Avi Kivity wrote:
> cfi02 is annoying in that is ignores some address bits; we probably
> want explicit support in the memory API for that.

...

> diff --git a/hw/musicpal.c b/hw/musicpal.c
> index 63dd391..5e74ee7 100644
> --- a/hw/musicpal.c
> +++ b/hw/musicpal.c
> @@ -1502,6 +1502,7 @@ static void musicpal_init(ram_addr_t ram_size,
>      unsigned long flash_size;
>      DriveInfo *dinfo;
>      ram_addr_t sram_off;
> +    MemoryRegion *flash = g_new(MemoryRegion, 1);
>  
>      if (!cpu_model) {
>          cpu_model = "arm926";
> @@ -1565,21 +1566,23 @@ static void musicpal_init(ram_addr_t ram_size,
>           * image is smaller than 32 MB.
>           */
>  #ifdef TARGET_WORDS_BIGENDIAN
> -        pflash_cfi02_register(0-MP_FLASH_SIZE_MAX, qemu_ram_alloc(NULL,
> -                              "musicpal.flash", flash_size),
> +        memory_region_init_rom_device(flash, &pflash_cfi02_ops_be,
> +                                      NULL, "musicpal.flash", flash_size);
> +        pflash_cfi02_register(0-MP_FLASH_SIZE_MAX, flash,
>                                dinfo->bdrv, 0x10000,
>                                (flash_size + 0xffff) >> 16,
>                                MP_FLASH_SIZE_MAX / flash_size,
>                                2, 0x00BF, 0x236D, 0x0000, 0x0000,
> -                              0x5555, 0x2AAA, 1);
> +                              0x5555, 0x2AAA);
>  #else
> -        pflash_cfi02_register(0-MP_FLASH_SIZE_MAX, qemu_ram_alloc(NULL,
> -                              "musicpal.flash", flash_size),
> +        memory_region_init_rom_device(flash, &pflash_cfi02_ops_le,
> +                                      NULL, "musicpal.flash", flash_size);
> +        pflash_cfi02_register(0-MP_FLASH_SIZE_MAX, flash,
>                                dinfo->bdrv, 0x10000,
>                                (flash_size + 0xffff) >> 16,
>                                MP_FLASH_SIZE_MAX / flash_size,
>                                2, 0x00BF, 0x236D, 0x0000, 0x0000,
> -                              0x5555, 0x2AAA, 0);
> +                              0x5555, 0x2AAA);
>  #endif

This would deserve some reordering to reduce the code under #ifdef.

Anyway, it also breaks the musicpal (and presumably all other flash
users) because memory_region_init_rom_device is broken. It assumes
mr->ops is set, but I guess it rather should set that field itself, no?

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux