Am 19.06.2012 08:40, schrieb Peter Crosthwaite:
> On Tue, Jun 19, 2012 at 12:33 AM, Igor Mitsyanko
> <i.mitsya...@samsung.com> 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<peter.crosthwa...@petalogix.com>
>>> Signed-off-by: Kirill Batuzov<batuz...@ispras.ru>
>>> ---
>>> [..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, not
>> OPERAND_INVALID
>
> Ok
>
>>>
>>> + 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,
>>> +};
>>> +
>>
>> 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.
>>
>
> 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 that
>> every device at least needs to mark itself as non-migratable, if it doesn'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 emulated
>> exynos-based system. It works, but very slow, because the way real hardware
>> performs data transfers is not optimal for emulation.
>>
>
> Thats another battle for another day,
>
>> Tested by: Igor Mitsyanko <i.mitsya...@samsung.com>
>>
>
> Sweet,
>
> Ill roll a V5 soon, but im guessing PMM will do a review cycle here as
> well, so ill give it a few days.
>
> Regards,
> Peter
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
