[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/3] sh4: mmio based CF support on r2d board.
From: |
takasi-y |
Subject: |
Re: [Qemu-devel] [PATCH 1/3] sh4: mmio based CF support on r2d board. |
Date: |
Tue, 28 Oct 2008 03:57:32 +0900 (JST) |
Thank you for your review.
> I'd add a space between if and (.
...
> Please add a space between the comma after opaque and target_phys_addr_t.
OK. I fix the styles.
> 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.
I wrote it because I thought it was simple and flexible.
But, after browsing other device init functions, I understand "callee register"
is basic procedure in this program.
Looking at isa_ide_init()
# void isa_ide_init(int iobase, int iobase2, qemu_irq irq,
# BlockDriverState *hd0, BlockDriverState *hd1);
as a reference, I changed the function args like following.
void mmio_ide_init(target_phys_addr_t membase, target_phys_addr_t membase2,
qemu_irq irq, int shift,
BlockDriverState *hd0, BlockDriverState *hd1)
This one is not bad at all.
> This should be in a .h file (maybe pc.h), without "extern" and the
> parameter names should be included.
Sure. But, I wonder which file is suitable.
The system I accustomed to is like
- ide.c define function
- ide.h declare prototype
- pc.c include ide.h and refer the function.
But in QEMU, it looks like
- ide.c define function
- pc.h declare prototype
- pc.c include pc.h and refer the function.
Is it mean the prototype declaration should go into new file "r2d.h" ?
Cheers,
/yoshii