From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:58668) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SgvVG-0001OK-QK for qemu-devel@nongnu.org; Tue, 19 Jun 2012 06:17:44 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SgvVB-0001aL-53 for qemu-devel@nongnu.org; Tue, 19 Jun 2012 06:17:42 -0400 Received: from cantor2.suse.de ([195.135.220.15]:42179 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SgvVA-0001a3-S3 for qemu-devel@nongnu.org; Tue, 19 Jun 2012 06:17:37 -0400 Message-ID: <4FE051B5.6020904@suse.de> Date: Tue, 19 Jun 2012 12:17:25 +0200 From: =?ISO-8859-1?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <15126b43916ee3b1db4eafe3b0fabfcbfc8c0584.1339979922.git.peter.crosthwaite@petalogix.com> <4FDF3C1C.1020008@samsung.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v4 1/2] pl330: initial version List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Crosthwaite Cc: Peter Maydell , Igor Mitsyanko , qemu-devel@nongnu.org, zhur@ispras.ru, batuzovk@ispras.ru, kyungmin.park@samsung.com, paul@codesourcery.com, duyl@xilinx.com, linnj@xilinx.com, edgar.iglesias@gmail.com, john.williams@petalogix.com Am 19.06.2012 08:40, schrieb Peter Crosthwaite: > On Tue, Jun 19, 2012 at 12:33 AM, Igor Mitsyanko > wrote: >> >> Hi Peter, sorry for not properly reviewing your patch for such a long = time, >> I'll try to do this as soon as possible. Right now I have a few small >> coments >> >> >> >> On 06/18/2012 04:42 AM, Peter A. G. Crosthwaite wrote: >>> >>> Device model for Primecell PL330 dma controller. >>> >>> Signed-off-by: Peter A. G. Crosthwaite >>> Signed-off-by: Kirill Batuzov >>> --- >>> [..snip..] >>> >>> +static void pl330_dmago(PL330Chan *ch, uint8_t opcode, uint8_t *args= , int >>> len) >>> +{ >>> + uint8_t chan_id; >>> + uint8_t ns; >>> + uint32_t pc; >>> + PL330Chan *s; >>> + >>> + DB_PRINT("\n"); >>> + >>> + if (!ch->is_manager) { >>> + pl330_fault(ch, PL330_FAULT_OPERAND_INVALID); >> >> According to description its more likely to cause UNDEF_INSTR here, no= t >> OPERAND_INVALID >=20 > Ok >=20 >>> >>> + return; >>> + } >>> + ns =3D !!(opcode& 2); >>> [..snip..] >>> >>> + >>> +static Property pl330_properties[] =3D { >>> + DEFINE_PROP_UINT32("cfg0", PL330, cfg[0], 0), >>> + DEFINE_PROP_UINT32("cfg1", PL330, cfg[1], 0), >>> + DEFINE_PROP_UINT32("cfg2", PL330, cfg[2], 0), >>> + DEFINE_PROP_UINT32("cfg3", PL330, cfg[3], 0), >>> + DEFINE_PROP_UINT32("cfg4", PL330, cfg[4], 0), >>> + DEFINE_PROP_UINT32("cfg5", PL330, cfg[5], 0), >>> + DEFINE_PROP_END_OF_LIST(), >>> +}; >>> + >>> +static void pl330_class_init(ObjectClass *klass, void *data) >>> +{ >>> + DeviceClass *dc =3D DEVICE_CLASS(klass); >>> + SysBusDeviceClass *k =3D SYS_BUS_DEVICE_CLASS(klass); >>> + >>> + k->init =3D pl330_init; >>> + dc->reset =3D pl330_reset; >>> + dc->props =3D pl330_properties; >>> +} >>> + >>> +static TypeInfo pl330_info =3D { >>> + .name =3D "pl330", >>> + .parent =3D TYPE_SYS_BUS_DEVICE, >>> + .instance_size =3D sizeof(PL330), >>> + .class_init =3D pl330_class_init, >>> +}; >>> + >> >> I think Andreas requires all static TypeInfos to have const qualifier = and >> their names to comply with "_type_info" naming convention. I'm= not >> sure about this though. >> >=20 > Ok Yes, the lack of const in uses such as these has historic reasons and keeps propagating. If you touch it anyway, ..._type_info would be more self-describing but not a hard requirement. Thanks for keeping eyes open, Igor. :) >>> +static void pl330_register_types(void) >>> +{ >>> + type_register_static(&pl330_info); >>> +} >>> + >>> +type_init(pl330_register_types) >> >> >> And it still has no save/load support, it is really mandatory for all = new >> devices. I can recall that one of the maintainers wrote a while ago th= at >> every device at least needs to mark itself as non-migratable, if it do= esn't >> implement a proper vmstate. >> > Ok, ccing Andreas Not my requirement but Peter's (cc'ing). Usually it's really trivial adding a handful of fields to the VMSD, so I can understand though. Regards, Andreas >> We used this PL330 implementation to transfer sound data in our emulat= ed >> exynos-based system. It works, but very slow, because the way real har= dware >> performs data transfers is not optimal for emulation. >> >=20 > Thats another battle for another day, >=20 >> Tested by: Igor Mitsyanko >> >=20 > Sweet, >=20 > Ill roll a V5 soon, but im guessing PMM will do a review cycle here as > well, so ill give it a few days. >=20 > Regards, > Peter --=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