From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43)
	id 1KuGvF-0007xg-T3
	for qemu-devel@nongnu.org; Sun, 26 Oct 2008 21:29:33 -0400
Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43)
	id 1KuGvD-0007xS-HR
	for qemu-devel@nongnu.org; Sun, 26 Oct 2008 21:29:32 -0400
Received: from [199.232.76.173] (port=48086 helo=monty-python.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1KuGvD-0007xP-Bx
	for qemu-devel@nongnu.org; Sun, 26 Oct 2008 21:29:31 -0400
Received: from rv-out-0708.google.com ([209.85.198.242]:36332)
	by monty-python.gnu.org with esmtp (Exim 4.60)
	(envelope-from <balrogg@gmail.com>) id 1KuGvC-0001fd-Qz
	for qemu-devel@nongnu.org; Sun, 26 Oct 2008 21:29:31 -0400
Received: by rv-out-0708.google.com with SMTP id f25so1689210rvb.22
	for <qemu-devel@nongnu.org>; Sun, 26 Oct 2008 18:29:26 -0700 (PDT)
Message-ID: <fb249edb0810261829g7b5aac5fw80ddbaf2ddb72bcf@mail.gmail.com>
Date: Mon, 27 Oct 2008 03:29:26 +0200
From: "andrzej zaborowski" <balrogg@gmail.com>
Subject: Re: [Qemu-devel] [PATCH 1/3] sh4: mmio based CF support on r2d board.
In-Reply-To: <f43fc5580810260854q41264525xdaf9199686d9dcb8@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
References: <20080923013136.490117fd.yoshii.takashi@gmail.com>
	<200810261517.m9QFHUR4026096@smtp12.dti.ne.jp>
	<f43fc5580810260854q41264525xdaf9199686d9dcb8@mail.gmail.com>
Reply-To: qemu-devel@nongnu.org
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.gnu.org/pipermail/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: qemu-devel@nongnu.org

2008/10/26 Blue Swirl <blauwirbel@gmail.com>:
> On 10/26/08, takasi-y@ops.dti.ne.jp <takasi-y@ops.dti.ne.jp> wrote:
>> This patch adds emulation for a CompactFlash on sh4/r2d board.
>>  The device is CF, but wired to be worked as True-IDE mode, and connected
>>   directly to SH bus.

The CF-ATA code (IBM microdrive) in ide.c should expose a qemu_irq
that toggles between CF and True-IDE mode so that it can be used as
both... but it can be done another time.  I hadn't done it originally
because the pin is wired to CF mode in zaurus.

>
> +    if(addr & 7)
>
> I'd add a space between if and (.
>
> +static uint32_t mmio_ide_status_read (void *opaque,target_phys_addr_t addr)
>
> Please add a space between the comma after opaque and target_phys_addr_t.
>
>>  +void mmio_ide_init (int *mmio, BlockDriverState *hd0, BlockDriverState *hd1,
>>  +       qemu_irq irq, int shift)
>>  +{
>>  +    MMIOState *s = qemu_mallocz(sizeof(MMIOState));
>>  +    IDEState *ide = qemu_mallocz(sizeof(IDEState) * 2);
>>  +    int *io;
>>  +
>>  +    ide_init2(ide, hd0, hd1, irq);
>>  +
>>  +    s->dev = ide;
>>  +    s->shift = shift;
>>  +
>>  +    mmio[0] = cpu_register_io_memory(0, mmio_ide_reads, mmio_ide_writes, s);
>>  +    mmio[1] = cpu_register_io_memory(0, mmio_ide_status, mmio_ide_cmd, s);
>>  +}
>
> It would be better to make the init function take instead of mmio
> pointer, two target_phys_addr_t parameters and do the physical memory
> registration there.

Why would it be better?  This implementation seems more flexible.

Cheers