From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from [140.186.70.92] (port=40132 helo=eggs.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1Q47lL-0004Ce-4X
	for qemu-devel@nongnu.org; Mon, 28 Mar 2011 04:25:24 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <Jes.Sorensen@redhat.com>) id 1Q47lJ-0000Wm-LO
	for qemu-devel@nongnu.org; Mon, 28 Mar 2011 04:25:22 -0400
Received: from mx1.redhat.com ([209.132.183.28]:24882)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <Jes.Sorensen@redhat.com>) id 1Q47lJ-0000WW-8S
	for qemu-devel@nongnu.org; Mon, 28 Mar 2011 04:25:21 -0400
Message-ID: <4D9045D9.2090005@redhat.com>
Date: Mon, 28 Mar 2011 10:24:57 +0200
From: Jes Sorensen <Jes.Sorensen@redhat.com>
MIME-Version: 1.0
Subject: Re: [Qemu-devel] [PATCH v2] hw/pc: Support system flash memory with
	-pflash parameter
References: <1300932459-6709-1-git-send-email-jordan.l.justen@intel.com>
In-Reply-To: <1300932459-6709-1-git-send-email-jordan.l.justen@intel.com>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: jordan.l.justen@intel.com
Cc: qemu-devel@nongnu.org

On 03/24/11 03:07, jordan.l.justen@intel.com wrote:
> +static void pc_isa_bios_init(ram_addr_t ram_offset, int ram_size)
> +{
> +    int isa_bios_size;
> +
> +    /* map the last 128KB of the BIOS in ISA space */
> +    isa_bios_size = ram_size;
> +    if (isa_bios_size > (128 * 1024))
> +        isa_bios_size = 128 * 1024;

You're missing braces here to comply with coding style. I realize some
of the cosmetics comes from old code you are moving, but please fix it
up in the move.

> +    cpu_register_physical_memory(0x100000 - isa_bios_size,
> +                                 isa_bios_size,
> +                                 (ram_offset + ram_size - isa_bios_size) | IO_MEM_ROM);

A line is 80 characters, please wrap it or calculate the number before
the function call.

> +    bdrv = pflash_drv->bdrv;
> +    size = bdrv_getlength(pflash_drv->bdrv);
> +    sector_bits = 12;
> +    sector_size = 1 << sector_bits;
> +
> +    if ((size % sector_size) != 0) {
> +        fprintf(stderr,
> +                "qemu: -pflash size must be a multiple of 0x%x\n",
> +                sector_size);
> +        exit(1);
> +    }
> +
> +    phys_addr = 0x100000000ULL - rom_size - size;
> +    addr = qemu_ram_alloc(NULL, "system.flash", size);
> +    DPRINTF("flash addr: 0x%lx\n", (int64_t)phys_addr);
> +#if 1
> +    pflash_cfi01_register(phys_addr, addr, bdrv,
> +                          sector_size, size >> sector_bits,
> +                          4, 0x0000, 0x0000, 0x0000, 0x0000, 0);
> +#else
> +    pflash_cfi02_register(phys_addr, addr, bdrv,
> +                          sector_size, size >> sector_bits,
> +                          1, 4, 0x0000, 0x0000, 0x0000, 0x0000, 0x55, 0xaa, 0);
> +#endif

Please get rid of the #ifdef here.

Cheers,
Jes