Re: [Qemu-devel] [PATCH 0/9] *** SUBJECT HERE ***

[Andreas Färber]

Hello Xuetao,

Am 25.05.2012 13:28, schrieb Guan Xuetao:
>  target-unicore32/op_helper.c          |  187 +++++++++++++++++++++++-

Your patches add new code to op_helper.c, whereas for other
architectures there's work going on to drop old-style op_helper.c
(examples committed are sparc and alpha; ppc and x86 are on the list).
Maybe you want to consider that at least for the new helpers you're
introducing?

In master I noticed that reset was not implemented for unicore32.
Haven't seen whether that is being implemented somewhere in this series?

You don't indicate what this series is based on so I assume master and
would ask you to rebase onto qom-next branch, which is going to be the
basis for 1.2 (in particular no cpu_state_reset() any more).

Please update cpu_uc32_init() to return UniCore32CPU so that you can use
it in puv3_init() in place of cpu_init(). See cpu_arm_init()/cpu_init()
on master or many more examples on qom-next.
Your static helpers should also prefer to pass around UniCore32CPU *cpu
rather than CPUUniCore32State *env, as my recent patch series starts
moving code out of env and into the CPUState base class.

Some general notes:
* machine_init() declares a function, so does not need a semicolon.
* TypeInfo when immutable (the regular case) should be static const.
* Struct type names should be CamelCase, not _t.

Any chance to split up patch 7 further for review?

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