qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Avi Kivity
Subject: Re: [Qemu-devel] [PATCH v2 14/22] pflash_cfi01/pflash_cfi02: convert to memory API
Date: Thu, 25 Aug 2011 14:48:33 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:6.0) Gecko/20110816 Thunderbird/6.0

On 08/25/2011 02:37 PM, Jan Kiszka wrote:
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.

Sure, but I try to avoid refactoring in these conversions. We already have too many regressions.

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?


Right, thanks.

--
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.




reply via email to

[Prev in Thread] Current Thread [Next in Thread]