Re: [PATCH v8 05/10] hw/fsi: Introduce IBM's FSI master

[Ninad Palsule <ninad@linux.ibm.com>]

Hello Cedric,


>> +static uint64_t fsi_master_read(void *opaque, hwaddr addr, unsigned 
>> size)
>> +{
>> +    FSIMasterState *s = FSI_MASTER(opaque);
>> +
>> +    trace_fsi_master_read(addr, size);
>> +
>> +    if (addr + size > sizeof(s->regs)) {
>
> See comment on patch 3
I fixed it.
>
>> +        qemu_log_mask(LOG_GUEST_ERROR,
>> +                      "%s: Out of bounds read: 0x%"HWADDR_PRIx" for 
>> %u\n",
>> +                      __func__, addr, size);
>> +        return 0;
>> +    }
>> +
>> +    return s->regs[TO_REG(addr)];
>> +}
>> +
>> +static void fsi_master_write(void *opaque, hwaddr addr, uint64_t data,
>> +                             unsigned size)
>> +{
>> +    FSIMasterState *s = FSI_MASTER(opaque);
>> +
>> +    trace_fsi_master_write(addr, size, data);
>> +
>> +    if (addr + size > sizeof(s->regs)) {
I fixed it.
>>
>> +
>> +    /* address ? */
>> +    memory_region_add_subregion(&s->opb2fsi, 0, &s->cfam.mr);
>> +}
>> +
>> +static void fsi_master_reset(DeviceState *dev)
>> +{
>> +    FSIMasterState *s = FSI_MASTER(dev);
>
> Don't we want to set all values to some default ?
Initialize all other registers to 0 as FSI spec expect them to be zero 
except MVER and MLEVP0. I don't have reset value for MLEVP0 for ast2600. 
This is related to HOT plug detection so setting it to 0 for now.

Thanks for the review.

Regards,

Ninad