Re: [Qemu-devel] [PATCH v5 1/2] pl330: Initial version

[Peter Crosthwaite]

All changed made,

Thanks,
Peter

On Mon, Oct 29, 2012 at 7:41 PM, Igor Mitsyanko <address@hidden> wrote:
> Good day, Peter)
>
>
> On 10/29/2012 10:35 AM, Peter Crosthwaite wrote:
>>
>> Device model for Primecell PL330 dma controller.
>>
>> +
>> +static Property pl330_properties[] = {
>> +    /* CR0 */
>> +    DEFINE_PROP_UINT8("num_chnls", PL330, num_chnls, 8),
>> +    DEFINE_PROP_UINT8("num_periph_req", PL330, num_periph_req, 8),
>> +    DEFINE_PROP_UINT8("num_events", PL330, num_events, 8),
>> +    DEFINE_PROP_UINT8("mgr_ns_at_rst", PL330, mgr_ns_at_rst, 0),
>> +    /* CR1 */
>> +    DEFINE_PROP_UINT8("i-cache_len", PL330, i_cache_len, 4),
>> +    DEFINE_PROP_UINT8("num_i-cache_lines", PL330, num_i_cache_lines, 8),
>> +
>> +    DEFINE_PROP_UINT8("mgr_ns_at_rst", PL330, mgr_ns_at_rst, 0),
>
>
> That's a duplicate, you had the same property three rows before.
>
>
>> +
>> +    /* CR2-4 */
>> +    DEFINE_PROP_UINT32("boot_addr", PL330, cfg[CFG_BOOT_ADDR], 0),
>> +    DEFINE_PROP_UINT32("INS", PL330, cfg[CFG_INS], 0),
>> +    DEFINE_PROP_UINT32("PNS", PL330, cfg[CFG_PNS], 0),
>> +    /* CRD */
>> +    DEFINE_PROP_UINT8("data_width", PL330, data_width, 0),
>
>
> You do not decode this value in pl330_init() like you do for, for example,
> i_cache_len property. I think default value
> here should be 32, which corresponds to 0b010 in LSB of CRD register. And
> you should also check for reserved
> values of this property and, perhaps, hw_error() on them.
>
>
>> +    DEFINE_PROP_UINT8("wr_cap", PL330, wr_cap, 0),
>> +    DEFINE_PROP_UINT8("wr_q_dep", PL330, wr_q_dep, 0),
>> +    DEFINE_PROP_UINT8("rd_cap", PL330, rd_cap, 0),
>> +    DEFINE_PROP_UINT8("rd_q_dep", PL330, rd_q_dep, 0),
>> +    DEFINE_PROP_UINT16("data_buffer_dep", PL330, data_buffer_dep, 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;
>> +    dc->vmsd = &vmstate_pl330;
>> +}
>> +
>> +static const TypeInfo pl330_type_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_type_info);
>> +}
>> +
>> +type_init(pl330_register_types)
>
>
> --
> Mitsyanko Igor
> ASWG, Moscow R&D center, Samsung Electronics
> email: address@hidden
>
>