Re: [PATCH 04/21] aspeed: i2c: Use reg array instead of individual vars

[Joel Stanley]

On Mon, 6 Jun 2022 at 15:08, Cédric Le Goater <clg@kaod.org> wrote:
>
> From: Joe Komlodi <komlodi@google.com>
>
> Using a register array will allow us to represent old-mode and new-mode
> I2C registers by using the same underlying register array, instead of
> adding an entire new set of variables to represent new mode.

The downside of this approach is you lose the safety of having
discrete types. A write to s->regs[R_FOO] can overwrite R_BAR.


>
> As part of this, we also do additional cleanup to use ARRAY_FIELD_
> macros instead of FIELD_ macros on registers.
>
> Signed-off-by: Joe Komlodi <komlodi@google.com>
> Change-Id: Ib94996b17c361b8490c042b43c99d8abc69332e3
> Message-Id: <20220331043248.2237838-5-komlodi@google.com>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  include/hw/i2c/aspeed_i2c.h |  11 +-
>  hw/i2c/aspeed_i2c.c         | 286 +++++++++++++++++-------------------
>  2 files changed, 133 insertions(+), 164 deletions(-)

> @@ -858,12 +834,12 @@ static void aspeed_i2c_bus_reset(DeviceState *dev)
>  {
>      AspeedI2CBus *s = ASPEED_I2C_BUS(dev);
>
> -    s->intr_ctrl = 0;
> -    s->intr_status = 0;
> -    s->cmd = 0;
> -    s->buf = 0;
> -    s->dma_addr = 0;
> -    s->dma_len = 0;
> +    s->regs[R_I2CD_INTR_CTRL] = 0;
> +    s->regs[R_I2CD_INTR_STS] = 0;
> +    s->regs[R_I2CD_CMD] = 0;
> +    s->regs[R_I2CD_BYTE_BUF] = 0;
> +    s->regs[R_I2CD_DMA_ADDR] = 0;
> +    s->regs[R_I2CD_DMA_LEN] = 0;

Could this become a memset of s->regs?

>      i2c_end_transfer(s->bus);
>  }
>
> @@ -921,10 +897,10 @@ static qemu_irq 
> aspeed_2400_i2c_bus_get_irq(AspeedI2CBus *bus)
>  static uint8_t *aspeed_2400_i2c_bus_pool_base(AspeedI2CBus *bus)
>  {
>      uint8_t *pool_page =
> -        &bus->controller->pool[FIELD_EX32(bus->ctrl, I2CD_FUN_CTRL,
> -                                          POOL_PAGE_SEL) * 0x100];
> +        &bus->controller->pool[ARRAY_FIELD_EX32(bus->regs, I2CD_FUN_CTRL,
> +                                                POOL_PAGE_SEL) * 0x100];
>
> -    return &pool_page[FIELD_EX32(bus->pool_ctrl, I2CD_POOL_CTRL, OFFSET)];
> +    return &pool_page[ARRAY_FIELD_EX32(bus->regs, I2CD_POOL_CTRL, OFFSET)];
>  }
>
>  static void aspeed_2400_i2c_class_init(ObjectClass *klass, void *data)
> --
> 2.35.3
>