From: Nicholas Piggin <npiggin@gmail.com>
To: Corey Minyard <cminyard@mvista.com>
Cc: "Nicholas Piggin" <npiggin@gmail.com>,
"Alistair Francis" <alistair.francis@wdc.com>,
"Daniel Henrique Barboza" <daniel.barboza@oss.qualcomm.com>,
"Chao Liu" <chao.liu.zevorn@gmail.com>,
"Chris Rauer" <crauer@google.com>,
"Michael Ellerman" <mpe@kernel.org>,
"Joel Stanley" <jms@oss.tenstorrent.com>,
"Anirudh Srinivasan" <asrinivasan@oss.tenstorrent.com>,
"Portia Stephens" <portias@oss.tenstorrent.com>,
qemu-riscv@nongnu.org, qemu-devel@nongnu.org,
"Hao Wu" <wuhaotsh@google.com>,
"Philippe Mathieu-Daudé" <philmd@linaro.org>
Subject: [PATCH 2/4] [RFC] hw/i2c/designware_i2c: Switch to Fifo8
Date: Thu, 7 May 2026 22:05:20 +1000 [thread overview]
Message-ID: <20260507120524.111056-3-npiggin@gmail.com> (raw)
In-Reply-To: <20260507120524.111056-1-npiggin@gmail.com>
Alistair suggested moving to Fifo8, which I think is
a good improvement.
Broken out for individual review, but IMO we should
squash before merge since it changes VMState format.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
hw/i2c/designware_i2c.c | 37 ++++++++++++++++++++-------------
include/hw/i2c/designware_i2c.h | 7 +++----
2 files changed, 25 insertions(+), 19 deletions(-)
diff --git a/hw/i2c/designware_i2c.c b/hw/i2c/designware_i2c.c
index 1e5505f571..83d0968580 100644
--- a/hw/i2c/designware_i2c.c
+++ b/hw/i2c/designware_i2c.c
@@ -208,10 +208,8 @@ static void dw_i2c_update_irq(DesignWareI2CState *s)
qemu_set_irq(s->irq, level);
}
-static uint32_t dw_i2c_read_ic_data_cmd(DesignWareI2CState *s)
+static uint8_t dw_i2c_read_ic_data_cmd(DesignWareI2CState *s)
{
- uint32_t value = s->rx_fifo[s->rx_cur];
-
if (s->status != DW_I2C_STATUS_RECEIVING) {
qemu_log_mask(LOG_GUEST_ERROR,
"%s: Attempted to read from RX fifo when not in receive "
@@ -223,22 +221,21 @@ static uint32_t dw_i2c_read_ic_data_cmd(DesignWareI2CState *s)
return 0;
}
- s->rx_cur = (s->rx_cur + 1) % DESIGNWARE_I2C_RX_FIFO_SIZE;
+ g_assert(s->ic_rxflr == fifo8_num_used(&s->rx_fifo));
- if (s->ic_rxflr > 0) {
- s->ic_rxflr--;
- } else {
+ if (fifo8_is_empty(&s->rx_fifo)) {
s->ic_raw_intr_stat |= DW_IC_INTR_RX_UNDER;
dw_i2c_update_irq(s);
return 0;
}
+ s->ic_rxflr--;
if (s->ic_rxflr <= s->ic_rx_tl) {
s->ic_raw_intr_stat &= ~DW_IC_INTR_RX_FULL;
dw_i2c_update_irq(s);
}
- return value;
+ return fifo8_pop(&s->rx_fifo);
}
static uint64_t dw_i2c_read(void *opaque, hwaddr offset, unsigned size)
@@ -445,6 +442,7 @@ static void dw_i2c_reset_to_idle(DesignWareI2CState *s)
s->ic_raw_intr_stat &= ~DW_IC_INTR_RX_UNDER;
s->ic_raw_intr_stat &= ~DW_IC_INTR_RX_OVER;
s->ic_rxflr = 0;
+ fifo8_reset(&s->rx_fifo);
s->ic_status &= ~DW_IC_STATUS_ACTIVITY;
s->status = DW_I2C_STATUS_IDLE;
dw_i2c_update_irq(s);
@@ -509,10 +507,10 @@ static void dw_i2c_write_ic_data_cmd(DesignWareI2CState *s, uint32_t value)
/* Receive data */
if (recv) {
- uint8_t pos = (s->rx_cur + s->ic_rxflr) % DESIGNWARE_I2C_RX_FIFO_SIZE;
+ g_assert(s->ic_rxflr == fifo8_num_used(&s->rx_fifo));
- if (s->ic_rxflr < DESIGNWARE_I2C_RX_FIFO_SIZE) {
- s->rx_fifo[pos] = i2c_recv(s->bus);
+ if (!fifo8_is_full(&s->rx_fifo)) {
+ fifo8_push(&s->rx_fifo, i2c_recv(s->bus));
s->ic_rxflr++;
} else {
s->ic_raw_intr_stat |= DW_IC_INTR_RX_OVER;
@@ -738,7 +736,8 @@ static void designware_i2c_enter_reset(Object *obj, ResetType type)
s->ic_comp_version = DW_IC_COMP_VERSION_INIT_VAL;
s->ic_comp_type = DW_IC_COMP_TYPE_INIT_VAL;
- s->rx_cur = 0;
+ fifo8_reset(&s->rx_fifo);
+
s->status = DW_I2C_STATUS_IDLE;
}
@@ -777,10 +776,8 @@ static const VMStateDescription vmstate_designware_i2c = {
VMSTATE_UINT32(ic_comp_param_1, DesignWareI2CState),
VMSTATE_UINT32(ic_comp_version, DesignWareI2CState),
VMSTATE_UINT32(ic_comp_type, DesignWareI2CState),
+ VMSTATE_FIFO8(rx_fifo, DesignWareI2CState),
VMSTATE_UINT32(status, DesignWareI2CState),
- VMSTATE_UINT8_ARRAY(rx_fifo, DesignWareI2CState,
- DESIGNWARE_I2C_RX_FIFO_SIZE),
- VMSTATE_UINT8(rx_cur, DesignWareI2CState),
VMSTATE_END_OF_LIST(),
},
};
@@ -790,6 +787,8 @@ static void designware_i2c_smbus_init(Object *obj)
DesignWareI2CState *s = DESIGNWARE_I2C(obj);
SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
+ fifo8_create(&s->rx_fifo, DESIGNWARE_I2C_RX_FIFO_SIZE);
+
sysbus_init_irq(sbd, &s->irq);
memory_region_init_io(&s->iomem, obj, &designware_i2c_ops, s,
@@ -799,6 +798,13 @@ static void designware_i2c_smbus_init(Object *obj)
s->bus = i2c_init_bus(DEVICE(s), "i2c-bus");
}
+static void designware_i2c_finalize(Object *obj)
+{
+ DesignWareI2CState *s = DESIGNWARE_I2C(obj);
+
+ fifo8_destroy(&s->rx_fifo);
+}
+
static void designware_i2c_class_init(ObjectClass *klass, const void *data)
{
ResettableClass *rc = RESETTABLE_CLASS(klass);
@@ -819,6 +825,7 @@ static const TypeInfo designware_i2c_types[] = {
.instance_size = sizeof(DesignWareI2CState),
.class_init = designware_i2c_class_init,
.instance_init = designware_i2c_smbus_init,
+ .instance_finalize = designware_i2c_finalize,
},
};
DEFINE_TYPES(designware_i2c_types);
diff --git a/include/hw/i2c/designware_i2c.h b/include/hw/i2c/designware_i2c.h
index 71d74c9141..affaf983a2 100644
--- a/include/hw/i2c/designware_i2c.h
+++ b/include/hw/i2c/designware_i2c.h
@@ -8,6 +8,7 @@
#ifndef DESIGNWARE_I2C_H
#define DESIGNWARE_I2C_H
+#include "qemu/fifo8.h"
#include "hw/i2c/i2c.h"
#include "hw/core/irq.h"
#include "hw/core/sysbus.h"
@@ -53,8 +54,6 @@ typedef enum DesignWareI2CStatus {
* @ic_comp_version: I2C component version register
* @ic_comp_type: I2C component type register
* @rx_fifo: The FIFO buffer for receiving in FIFO mode.
- * @rx_cur: The current position of rx_fifo.
- * @status: The current status of the SMBus.
*/
typedef struct DesignWareI2CState {
SysBusDevice parent_obj;
@@ -88,8 +87,8 @@ typedef struct DesignWareI2CState {
uint32_t ic_comp_version;
uint32_t ic_comp_type;
- uint8_t rx_fifo[DESIGNWARE_I2C_RX_FIFO_SIZE];
- uint8_t rx_cur;
+ /* fifo8_num_used(rx_fifo) should always equal ic_rxflr */
+ Fifo8 rx_fifo;
DesignWareI2CStatus status;
} DesignWareI2CState;
--
2.53.0
next prev parent reply other threads:[~2026-05-07 12:07 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-07 12:05 [PATCH 0/4] hw/i2c: Add designware i2c controller Nicholas Piggin
2026-05-07 12:05 ` [PATCH 1/4] " Nicholas Piggin
2026-05-11 10:20 ` Philippe Mathieu-Daudé
2026-05-07 12:05 ` Nicholas Piggin [this message]
2026-05-11 10:18 ` [PATCH 2/4] [RFC] hw/i2c/designware_i2c: Switch to Fifo8 Philippe Mathieu-Daudé
2026-05-15 18:23 ` Nicholas Piggin
2026-05-07 12:05 ` [PATCH 3/4] [RFC] hw/i2c/designware_i2c: Switch to QEMU register API Nicholas Piggin
2026-05-13 2:03 ` Alistair Francis
2026-05-15 18:34 ` Nicholas Piggin
2026-05-07 12:05 ` [PATCH 4/4] [RFC] hw/i2c/designware_i2c: add SMBUS_INTR_MASK Nicholas Piggin
2026-05-11 10:19 ` Philippe Mathieu-Daudé
2026-05-10 13:03 ` [PATCH 0/4] hw/i2c: Add designware i2c controller Corey Minyard
2026-05-15 23:00 ` Nicholas Piggin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260507120524.111056-3-npiggin@gmail.com \
--to=npiggin@gmail.com \
--cc=alistair.francis@wdc.com \
--cc=asrinivasan@oss.tenstorrent.com \
--cc=chao.liu.zevorn@gmail.com \
--cc=cminyard@mvista.com \
--cc=crauer@google.com \
--cc=daniel.barboza@oss.qualcomm.com \
--cc=jms@oss.tenstorrent.com \
--cc=mpe@kernel.org \
--cc=philmd@linaro.org \
--cc=portias@oss.tenstorrent.com \
--cc=qemu-devel@nongnu.org \
--cc=qemu-riscv@nongnu.org \
--cc=wuhaotsh@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.