Re: [PATCH] hw/i2c/mpc_i2c.c: Fix mmio region size

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 22/7/24 12:16, BALATON Zoltan wrote:
> On Mon, 22 Jul 2024, Philippe Mathieu-Daudé wrote:
>> +Amit & Andrew
>>
>> On 22/7/24 00:55, BALATON Zoltan wrote:
>>> The last register of this device is at offset 0x14 occupying 8 bits so
>>> to cover it the mmio region needs to be 0x15 bytes long. Also correct
>>> the name of the field storing this register value to match the
>>> register name.
>>>
>>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>>> ---
>>>   hw/i2c/mpc_i2c.c | 8 ++++----
>>>   1 file changed, 4 insertions(+), 4 deletions(-)
>>
>>
>>> @@ -329,7 +329,7 @@ static void mpc_i2c_realize(DeviceState *dev, 
>>> Error **errp)
>>>       MPCI2CState  *i2c = MPC_I2C(dev);
>>>       sysbus_init_irq(SYS_BUS_DEVICE(dev), &i2c->irq);
>>>       memory_region_init_io(&i2c->iomem, OBJECT(i2c), &i2c_ops, i2c,
>>> -                          "mpc-i2c", 0x14);
>>> +                          "mpc-i2c", 0x15);
>>
>> Personally I'm not a big fan of non-pow2 regions, so I'd have picked
>> 0x20 or 0x100 where the 2nd i2c controller starts. Amit, what do you
> 
> Coverung unused areas isn't a good idea because that would omit invalid 
> read/write warnings when those are enabled so it's harder to catch 
> unimplemented registers that way. So 0x100 is definitely overkill, 0x20 
> could be acceptable as other regs are also covering some unused area 
> after them but is also unnecessary when we can cover exactly the area 
> where the register is.

Yeah. I'm queuing this patch via hw-misc, thanks.