|
From: | Igor Mitsyanko |
Subject: | Re: [Qemu-devel] [PATCH v4 1/2] pl330: initial version |
Date: | Mon, 18 Jun 2012 18:33:00 +0400 |
User-agent: | Mozilla/5.0 (X11; Linux i686; rv:12.0) Gecko/20120430 Thunderbird/12.0.1 |
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:
According to description its more likely to cause UNDEF_INSTR here, not OPERAND_INVALIDDevice model for Primecell PL330 dma controller. Signed-off-by: Peter A. G. Crosthwaite<address@hidden> Signed-off-by: Kirill Batuzov<address@hidden> --- [..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);
I think Andreas requires all static TypeInfos to have const qualifier and their names to comply with "<device>_type_info" naming convention. I'm not sure about this though.+ return; + } + ns = !!(opcode& 2); [..snip..] + +static Property pl330_properties[] = { + 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 = DEVICE_CLASS(klass); + SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass); + + k->init = pl330_init; + dc->reset = pl330_reset; + dc->props = pl330_properties; +} + +static TypeInfo pl330_info = { + .name = "pl330", + .parent = TYPE_SYS_BUS_DEVICE, + .instance_size = sizeof(PL330), + .class_init = pl330_class_init, +}; +
+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 that every device at least needs to mark itself as non-migratable, if it doesn't implement a proper vmstate.
We used this PL330 implementation to transfer sound data in our emulated exynos-based system. It works, but very slow, because the way real hardware performs data transfers is not optimal for emulation.
Tested by: Igor Mitsyanko <address@hidden>
[Prev in Thread] | Current Thread | [Next in Thread] |