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

[Anthony Green]

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 <address@hidden> 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 <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