Thanks to you and everybody else for the detailed feedback. I really appreciate the time you put into this.
I've cleaned up the formatting and licensing, and will address the other comments when I repost my patches in the new few days. Thanks, AG On Thu, Feb 14, 2013 at 7:08 AM, Andreas Färber <afaer...@suse.de> wrote: > 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 <gr...@moxielogic.com> >> --- >> 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
