Re: [Qemu-devel] [PATCH moxie 4/5] Add sample moxie system

[Andreas Färber]

Am 13.02.2013 23:27, schrieb Anthony Green:
> Add a simple moxie target, similar to what we have in the gdb simulator today.
> 
> 
> Signed-off-by: Anthony Green <address@hidden>
> ---
>  hw/moxie/Makefile.objs     |   5 ++
>  hw/moxiesim.c              | 200 
> +++++++++++++++++++++++++++++++++++++++++++++
>  include/sysemu/arch_init.h |   1 +
>  3 files changed, 206 insertions(+)
>  create mode 100644 hw/moxie/Makefile.objs
>  create mode 100644 hw/moxiesim.c

Patch has Coding Style issues and is line-wrapped, i.e. broken.

Please place your moxie-specific board in hw/moxie/. In Makefile.objs
just place obj-y = moxiesim.o below the $(addprefix ...) line.

Also respect C99 by not using underscore at the beginning of identifiers
(e.g., struct _loaderparams -> struct LoaderParams).

Are you aware that your loaderparams truncate ram_size from ram_addr_t
to int? If you don't want to use ram_addr_t there you might want to use
[u]int64_t.

Again some overuses of CPUMoxieState, please fix.

Please make your QEMUMachine static, it is not needed elsewhere.
Please drop semicolon after machine_init() macro, it's not a statement.

Please add a comma after QEMU_ARCH_MOXIE enum entry so that further
architectures can more easily be added.

Are pic_info and irq_info really needed? If so, they would be candidates
to place in stubs/ directory instead.

There's #if 0'ed code. If there's a question about that, please label as
RFC rather than PATCH. Otherwise please drop.

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg