[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v4 1/2] pl330: initial version

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:
Device model for Primecell PL330 dma controller.

Signed-off-by: Peter A. G. Crosthwaite<address@hidden>
Signed-off-by: Kirill Batuzov<address@hidden>
+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, not OPERAND_INVALID
+        return;
+    }
+    ns = !!(opcode&  2);
+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),
+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,
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.
+static void pl330_register_types(void)
+    type_register_static(&pl330_info);

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>

reply via email to

[Prev in Thread] Current Thread [Next in Thread]