Re: [Qemu-devel] [PATCH 1/3] sh4: mmio based CF support on r2d board.

["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. So, this code is to supports generally mmio based IDEs
>   which are supported by "pata_platform" driver in linux kernel.

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

>  +    extern void mmio_ide_init (int*, BlockDriverState*, BlockDriverState*,
>  +        qemu_irq, int);

This should be in a .h file (maybe pc.h), without "extern" and the
parameter names should be included.

Otherwise, the whole patch looks OK.