From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:46158) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SXtKh-00066z-Ke for qemu-devel@nongnu.org; Fri, 25 May 2012 08:09:33 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SXtKc-0001FW-OY for qemu-devel@nongnu.org; Fri, 25 May 2012 08:09:27 -0400 Received: from cantor2.suse.de ([195.135.220.15]:44498 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SXtKc-0001FH-Ij for qemu-devel@nongnu.org; Fri, 25 May 2012 08:09:22 -0400 Message-ID: <4FBF7669.1050002@suse.de> Date: Fri, 25 May 2012 14:09:13 +0200 From: =?ISO-8859-15?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: In-Reply-To: Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 0/9] *** SUBJECT HERE *** List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Guan Xuetao Cc: blauwirbel@gmail.com, qemu-devel@nongnu.org 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 --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe= rg