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

[Philippe Mathieu-Daudé]

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.