On Wed, Sep 18, 2024 at 1:06 PM Corey Minyard <co...@minyard.net> wrote:
>
> On Wed, Sep 18, 2024 at 12:22:47PM -0700, Octavian Purdila wrote:
> > Add a simple i2c peripheral to be used for testing I2C device
> > models. The peripheral has a fixed number of registers that can be
> > read and written.
>
> Why is this better than just using the eeprom device?
>
The main reason for adding it is that, AFAICT, there is no i2c slave
device that responds with I2C_NACK during a transaction.
Also, having a dedicated device for testing purposes makes it easier
to add new features than adding it to a real device where it might not
always be possible. I tried to add the NACK functionality but I did
not find one where the datasheet would confirm the behaviour I was
looking for.
> This has some uncommon attributes compared to most i2c devices. For
> instance, most i2c devices take their address as the first bytes of a
> write operation, then auto-increment after that for reads and writes.
> This seems to take one address on a write after a system reset, then use
> that forever.
>
> Anyway, unless there is a compelling reason to use this over the eeprom
> device, I'm going to recommend against it.
>
> -corey
>
> >
> > Signed-off-by: Octavian Purdila <ta...@google.com>
> > ---
> > include/hw/misc/i2c_tester.h | 30 ++++++++++
> > hw/misc/i2c_tester.c | 109 +++++++++++++++++++++++++++++++++++
> > hw/misc/Kconfig | 5 ++
> > hw/misc/meson.build | 2 +
> > 4 files changed, 146 insertions(+)
> > create mode 100644 include/hw/misc/i2c_tester.h
> > create mode 100644 hw/misc/i2c_tester.c
> >
> > diff --git a/include/hw/misc/i2c_tester.h b/include/hw/misc/i2c_tester.h
> > new file mode 100644
> > index 0000000000..f6b6491008
> > --- /dev/null
> > +++ b/include/hw/misc/i2c_tester.h
> > @@ -0,0 +1,30 @@
> > +/*
> > + *
> > + * Copyright (c) 2024 Google LLC
> > + *
> > + * SPDX-License-Identifier: GPL-2.0-or-later
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2 or
> > later.
> > + * See the COPYING file in the top-level directory.
> > + */
> > +
> > +#ifndef HW_I2C_TESTER_H
> > +#define HW_I2C_TESTER_H
> > +
> > +#include "qemu/osdep.h"
> > +#include "hw/i2c/i2c.h"
> > +#include "hw/irq.h"
> > +
> > +#define I2C_TESTER_NUM_REGS 0x31
> > +
> > +#define TYPE_I2C_TESTER "i2c-tester"
> > +#define I2C_TESTER(obj) OBJECT_CHECK(I2cTesterState, (obj),
> > TYPE_I2C_TESTER)
> > +
> > +typedef struct {
> > + I2CSlave i2c;
> > + bool set_reg_idx;
> > + uint8_t reg_idx;
> > + uint8_t regs[I2C_TESTER_NUM_REGS];
> > +} I2cTesterState;
> > +
> > +#endif /* HW_I2C_TESTER_H */
> > diff --git a/hw/misc/i2c_tester.c b/hw/misc/i2c_tester.c
> > new file mode 100644
> > index 0000000000..77ce8bf91a
> > --- /dev/null
> > +++ b/hw/misc/i2c_tester.c
> > @@ -0,0 +1,109 @@
> > +/*
> > + * Simple I2C peripheral for testing I2C device models.
> > + *
> > + * Copyright (c) 2024 Google LLC
> > + *
> > + * SPDX-License-Identifier: GPL-2.0-or-later
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2 or
> > later.
> > + * See the COPYING file in the top-level directory.
> > + */
> > +
> > +#include "hw/misc/i2c_tester.h"
> > +
> > +#include "qemu/log.h"
> > +#include "qemu/module.h"
> > +#include "migration/vmstate.h"
> > +
> > +static void i2c_tester_reset_enter(Object *o, ResetType type)
> > +{
> > + I2cTesterState *s = I2C_TESTER(o);
> > +
> > + s->set_reg_idx = false;
> > + s->reg_idx = 0;
> > + memset(s->regs, 0, I2C_TESTER_NUM_REGS);
> > +}
> > +
> > +static int i2c_tester_event(I2CSlave *i2c, enum i2c_event event)
> > +{
> > + I2cTesterState *s = I2C_TESTER(i2c);
> > +
> > + if (event == I2C_START_SEND) {
> > + s->set_reg_idx = true;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static uint8_t i2c_tester_rx(I2CSlave *i2c)
> > +{
> > + I2cTesterState *s = I2C_TESTER(i2c);
> > +
> > + if (s->reg_idx >= I2C_TESTER_NUM_REGS) {
> > + qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid reg 0x%02x\n",
> > __func__,
> > + s->reg_idx);
> > + return I2C_NACK;
> > + }
> > +
> > + return s->regs[s->reg_idx];
> > +}
> > +
> > +static int i2c_tester_tx(I2CSlave *i2c, uint8_t data)
> > +{
> > + I2cTesterState *s = I2C_TESTER(i2c);
> > +
> > + if (s->set_reg_idx) {
> > + /* Setting the register in which the operation will be done. */
> > + s->reg_idx = data;
> > + s->set_reg_idx = false;
> > + return 0;
> > + }
> > +
> > + if (s->reg_idx >= I2C_TESTER_NUM_REGS) {
> > + qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid reg 0x%02x\n",
> > __func__,
> > + s->reg_idx);
> > + return I2C_NACK;
> > + }
> > +
> > + /* Write reg data. */
> > + s->regs[s->reg_idx] = data;
> > +
> > + return 0;
> > +}
> > +
> > +static const VMStateDescription vmstate_i2c_tester = {
> > + .name = "i2c-tester",
> > + .version_id = 1,
> > + .minimum_version_id = 1,
> > + .fields = (const VMStateField[]) {
> > + VMSTATE_I2C_SLAVE(i2c, I2cTesterState),
> > + VMSTATE_BOOL(set_reg_idx, I2cTesterState),
> > + VMSTATE_UINT8(reg_idx, I2cTesterState),
> > + VMSTATE_UINT8_ARRAY(regs, I2cTesterState, I2C_TESTER_NUM_REGS),
> > + VMSTATE_END_OF_LIST()
> > + }
> > +};
> > +
> > +static void i2c_tester_class_init(ObjectClass *oc, void *data)
> > +{
> > + DeviceClass *dc = DEVICE_CLASS(oc);
> > + ResettableClass *rc = RESETTABLE_CLASS(oc);
> > + I2CSlaveClass *isc = I2C_SLAVE_CLASS(oc);
> > +
> > + rc->phases.enter = i2c_tester_reset_enter;
> > + dc->vmsd = &vmstate_i2c_tester;
> > + isc->event = i2c_tester_event;
> > + isc->recv = i2c_tester_rx;
> > + isc->send = i2c_tester_tx;
> > +}
> > +
> > +static const TypeInfo i2c_tester_types[] = {
> > + {
> > + .name = TYPE_I2C_TESTER,
> > + .parent = TYPE_I2C_SLAVE,
> > + .instance_size = sizeof(I2cTesterState),
> > + .class_init = i2c_tester_class_init
> > + },
> > +};
> > +
> > +DEFINE_TYPES(i2c_tester_types);
> > diff --git a/hw/misc/Kconfig b/hw/misc/Kconfig
> > index 4b688aead2..3e93c12c8e 100644
> > --- a/hw/misc/Kconfig
> > +++ b/hw/misc/Kconfig
> > @@ -213,6 +213,11 @@ config IOSB
> > config XLNX_VERSAL_TRNG
> > bool
> >
> > +config I2C_TESTER
> > + bool
> > + default y if TEST_DEVICES
> > + depends on I2C
> > +
> > config FLEXCOMM
> > bool
> > select I2C
> > diff --git a/hw/misc/meson.build b/hw/misc/meson.build
> > index faaf2671ba..4f22231fa3 100644
> > --- a/hw/misc/meson.build
> > +++ b/hw/misc/meson.build
> > @@ -158,6 +158,8 @@ system_ss.add(when: 'CONFIG_SBSA_REF', if_true:
> > files('sbsa_ec.c'))
> > # HPPA devices
> > system_ss.add(when: 'CONFIG_LASI', if_true: files('lasi.c'))
> >
> > +system_ss.add(when: 'CONFIG_I2C_TESTER', if_true: files('i2c_tester.c'))
> > +
> > system_ss.add(when: 'CONFIG_FLEXCOMM', if_true: files('flexcomm.c'))
> > system_ss.add(when: 'CONFIG_RT500_CLKCTL', if_true:
> > files('rt500_clkctl0.c', 'rt500_clkctl1.c'))
> > system_ss.add(when: 'CONFIG_RT500_RSTCTL', if_true:
> > files('rt500_rstctl.c'))
> > --
> > 2.46.0.662.g92d0881bb0-goog
> >
> >
