| Message ID | 20230301165619.2171090-4-clg@kaod.org |
|---|---|
| State | New |
| Headers | show |
| Series | aspeed queue | expand |
On 01/03/2023 17.56, Cédric Le Goater wrote: > From: Klaus Jensen <k.jensen@samsung.com> > > Add an example I2C device to demonstrate how a slave may master the bus > and send data asynchronously to another slave. > > The device will echo whatever it is sent to the device identified by the > first byte received. > > Signed-off-by: Klaus Jensen <k.jensen@samsung.com> > [ clg: integrated fixes : > https://lore.kernel.org/qemu-devel/Y3yMKAhOkYGtnkOp@cormorant.local/ ] > Message-Id: <20220601210831.67259-7-its@irrelevant.dk> > Signed-off-by: Cédric Le Goater <clg@kaod.org> > --- > hw/misc/i2c-echo.c | 156 ++++++++++++++++++++++++++++++++++++++++++++ > hw/misc/meson.build | 2 + > 2 files changed, 158 insertions(+) > create mode 100644 hw/misc/i2c-echo.c > > diff --git a/hw/misc/i2c-echo.c b/hw/misc/i2c-echo.c > new file mode 100644 > index 0000000000..5705ab5d73 > --- /dev/null > +++ b/hw/misc/i2c-echo.c > @@ -0,0 +1,156 @@ > +#include "qemu/osdep.h" > +#include "qemu/timer.h" > +#include "qemu/main-loop.h" > +#include "block/aio.h" > +#include "hw/i2c/i2c.h" Hi Klaus, I've got two questions with regards to this new devices: 1) The file lacks a license statement (and a short comment at the beginning what it is all about). Could you maybe provide a follow up patch with a proper header comment that includes a license and a short description about the device? 2) Why is it in hw/misc/ and not in hw/i2c/ ? I think we should also have a proper Kconfig switch for this device, so we can disable it with --without-default-devices, what do you think? Thomas
On Aug 22 19:20, Thomas Huth wrote: > On 01/03/2023 17.56, Cédric Le Goater wrote: > > From: Klaus Jensen <k.jensen@samsung.com> > > > > Add an example I2C device to demonstrate how a slave may master the bus > > and send data asynchronously to another slave. > > > > The device will echo whatever it is sent to the device identified by the > > first byte received. > > > > Signed-off-by: Klaus Jensen <k.jensen@samsung.com> > > [ clg: integrated fixes : > > https://lore.kernel.org/qemu-devel/Y3yMKAhOkYGtnkOp@cormorant.local/ ] > > Message-Id: <20220601210831.67259-7-its@irrelevant.dk> > > Signed-off-by: Cédric Le Goater <clg@kaod.org> > > --- > > hw/misc/i2c-echo.c | 156 ++++++++++++++++++++++++++++++++++++++++++++ > > hw/misc/meson.build | 2 + > > 2 files changed, 158 insertions(+) > > create mode 100644 hw/misc/i2c-echo.c > > > > diff --git a/hw/misc/i2c-echo.c b/hw/misc/i2c-echo.c > > new file mode 100644 > > index 0000000000..5705ab5d73 > > --- /dev/null > > +++ b/hw/misc/i2c-echo.c > > @@ -0,0 +1,156 @@ > > +#include "qemu/osdep.h" > > +#include "qemu/timer.h" > > +#include "qemu/main-loop.h" > > +#include "block/aio.h" > > +#include "hw/i2c/i2c.h" > > Hi Klaus, > > I've got two questions with regards to this new devices: > > 1) The file lacks a license statement (and a short comment at the beginning > what it is all about). Could you maybe provide a follow up patch with a > proper header comment that includes a license and a short description about > the device? > > 2) Why is it in hw/misc/ and not in hw/i2c/ ? > > I think we should also have a proper Kconfig switch for this device, so we > can disable it with --without-default-devices, what do you think? > > Thomas > > Hi Thomas, 1) My apologies, I'll add the proper GPL license comment. 2) It's an example of using the asynchronous i2c send API. It's not a "real" device, its more like the edu-device. A proper note in the file should make this clear. I'll send a patch to fix this up! Thanks, Klaus
diff --git a/hw/misc/i2c-echo.c b/hw/misc/i2c-echo.c new file mode 100644 index 0000000000..5705ab5d73 --- /dev/null +++ b/hw/misc/i2c-echo.c @@ -0,0 +1,156 @@ +#include "qemu/osdep.h" +#include "qemu/timer.h" +#include "qemu/main-loop.h" +#include "block/aio.h" +#include "hw/i2c/i2c.h" + +#define TYPE_I2C_ECHO "i2c-echo" +OBJECT_DECLARE_SIMPLE_TYPE(I2CEchoState, I2C_ECHO) + +enum i2c_echo_state { + I2C_ECHO_STATE_IDLE, + I2C_ECHO_STATE_START_SEND, + I2C_ECHO_STATE_ACK, +}; + +typedef struct I2CEchoState { + I2CSlave parent_obj; + + I2CBus *bus; + + enum i2c_echo_state state; + QEMUBH *bh; + + unsigned int pos; + uint8_t data[3]; +} I2CEchoState; + +static void i2c_echo_bh(void *opaque) +{ + I2CEchoState *state = opaque; + + switch (state->state) { + case I2C_ECHO_STATE_IDLE: + return; + + case I2C_ECHO_STATE_START_SEND: + if (i2c_start_send_async(state->bus, state->data[0])) { + goto release_bus; + } + + state->pos++; + state->state = I2C_ECHO_STATE_ACK; + return; + + case I2C_ECHO_STATE_ACK: + if (state->pos > 2) { + break; + } + + if (i2c_send_async(state->bus, state->data[state->pos++])) { + break; + } + + return; + } + + + i2c_end_transfer(state->bus); +release_bus: + i2c_bus_release(state->bus); + + state->state = I2C_ECHO_STATE_IDLE; +} + +static int i2c_echo_event(I2CSlave *s, enum i2c_event event) +{ + I2CEchoState *state = I2C_ECHO(s); + + switch (event) { + case I2C_START_RECV: + state->pos = 0; + + break; + + case I2C_START_SEND: + state->pos = 0; + + break; + + case I2C_FINISH: + state->pos = 0; + state->state = I2C_ECHO_STATE_START_SEND; + i2c_bus_master(state->bus, state->bh); + + break; + + case I2C_NACK: + break; + + default: + return -1; + } + + return 0; +} + +static uint8_t i2c_echo_recv(I2CSlave *s) +{ + I2CEchoState *state = I2C_ECHO(s); + + if (state->pos > 2) { + return 0xff; + } + + return state->data[state->pos++]; +} + +static int i2c_echo_send(I2CSlave *s, uint8_t data) +{ + I2CEchoState *state = I2C_ECHO(s); + + if (state->pos > 2) { + return -1; + } + + state->data[state->pos++] = data; + + return 0; +} + +static void i2c_echo_realize(DeviceState *dev, Error **errp) +{ + I2CEchoState *state = I2C_ECHO(dev); + BusState *bus = qdev_get_parent_bus(dev); + + state->bus = I2C_BUS(bus); + state->bh = qemu_bh_new(i2c_echo_bh, state); + + return; +} + +static void i2c_echo_class_init(ObjectClass *oc, void *data) +{ + I2CSlaveClass *sc = I2C_SLAVE_CLASS(oc); + DeviceClass *dc = DEVICE_CLASS(oc); + + dc->realize = i2c_echo_realize; + + sc->event = i2c_echo_event; + sc->recv = i2c_echo_recv; + sc->send = i2c_echo_send; +} + +static const TypeInfo i2c_echo = { + .name = TYPE_I2C_ECHO, + .parent = TYPE_I2C_SLAVE, + .instance_size = sizeof(I2CEchoState), + .class_init = i2c_echo_class_init, +}; + +static void register_types(void) +{ + type_register_static(&i2c_echo); +} + +type_init(register_types); diff --git a/hw/misc/meson.build b/hw/misc/meson.build index fe869b98ca..a40245ad44 100644 --- a/hw/misc/meson.build +++ b/hw/misc/meson.build @@ -128,6 +128,8 @@ softmmu_ss.add(when: 'CONFIG_NRF51_SOC', if_true: files('nrf51_rng.c')) softmmu_ss.add(when: 'CONFIG_GRLIB', if_true: files('grlib_ahb_apb_pnp.c')) +softmmu_ss.add(when: 'CONFIG_I2C', if_true: files('i2c-echo.c')) + specific_ss.add(when: 'CONFIG_AVR_POWER', if_true: files('avr_power.c')) specific_ss.add(when: 'CONFIG_MAC_VIA', if_true: files('mac_via.c'))