Re: [RFC PATCH v2 2/6] hw/i2c/aspeed: add DEV_ADDR in old register mode

[Cédric Le Goater]

On 6/1/22 23:08, Klaus Jensen wrote:
> From: Klaus Jensen <k.jensen@samsung.com>
>
> Add support for writing and reading the device address register in old
> register mode.
>
> Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> ---
>   hw/i2c/aspeed_i2c.c         | 5 +++--
>   include/hw/i2c/aspeed_i2c.h | 8 ++++++++
>   2 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c
> index 576425898b09..5a7eb5579b01 100644
> --- a/hw/i2c/aspeed_i2c.c
> +++ b/hw/i2c/aspeed_i2c.c
> @@ -104,6 +104,7 @@ static uint64_t aspeed_i2c_bus_old_read(AspeedI2CBus *bus, 
> hwaddr offset,
>       case A_I2CD_AC_TIMING2:
>       case A_I2CD_INTR_CTRL:
>       case A_I2CD_INTR_STS:
> +    case A_I2CD_DEV_ADDR:
>       case A_I2CD_POOL_CTRL:
>       case A_I2CD_BYTE_BUF:
>           /* Value is already set, don't do anything. */
> @@ -741,8 +742,7 @@ static void aspeed_i2c_bus_old_write(AspeedI2CBus *bus, 
> hwaddr offset,
>           }
>           break;
>       case A_I2CD_DEV_ADDR:
> -        qemu_log_mask(LOG_UNIMP, "%s: slave mode not implemented\n",
> -                      __func__);
> +        bus->regs[R_I2CD_DEV_ADDR] = value;
>           break;
>       case A_I2CD_POOL_CTRL:
>           bus->regs[R_I2CD_POOL_CTRL] &= ~0xffffff;
> @@ -1060,6 +1060,7 @@ static void aspeed_i2c_bus_reset(DeviceState *dev)
>   
>       s->regs[R_I2CD_INTR_CTRL] = 0;
>       s->regs[R_I2CD_INTR_STS] = 0;
> +    s->regs[R_I2CD_DEV_ADDR] = 0;
>       s->regs[R_I2CD_CMD] = 0;
>       s->regs[R_I2CD_BYTE_BUF] = 0;
>       s->regs[R_I2CD_DMA_ADDR] = 0;
> diff --git a/include/hw/i2c/aspeed_i2c.h b/include/hw/i2c/aspeed_i2c.h
> index 79c6779c6c1e..03fe829a3a57 100644
> --- a/include/hw/i2c/aspeed_i2c.h
> +++ b/include/hw/i2c/aspeed_i2c.h
> @@ -297,6 +297,14 @@ static inline uint32_t 
> aspeed_i2c_bus_cmd_offset(AspeedI2CBus *bus)
>       return R_I2CD_CMD;
>   }
>   
> +static inline uint32_t aspeed_i2c_bus_dev_addr_offset(AspeedI2CBus *bus)

This routine seems unused.

Thanks,

C.

> +{
> +    if (aspeed_i2c_is_new_mode(bus->controller)) {
> +        return R_I2CS_DEV_ADDR;
> +    }
> +    return R_I2CD_DEV_ADDR;
> +}
> +
>   static inline uint32_t aspeed_i2c_bus_intr_ctrl_offset(AspeedI2CBus *bus)
>   {
>       if (aspeed_i2c_is_new_mode(bus->controller)) {