diff mbox series

[6/6] hw/i2c: Implement NPCM7XX SMBus Module FIFO Mode

Message ID 20210126193237.1534208-7-wuhaotsh@google.com (mailing list archive)
State New, archived
Headers show
Series hw/i2c: Add NPCM7XX SMBus Device | expand

Commit Message

Denis V. Lunev" via Jan. 26, 2021, 7:32 p.m. UTC
This patch implements the FIFO mode of the SMBus module. In FIFO, the
user transmits or receives at most 16 bytes at a time. The FIFO mode
allows the module to transmit large amount of data faster than single
byte mode.

Reviewed-by: Doug Evans<dje@google.com>
Reviewed-by: Tyrong Ting<kfting@nuvoton.com>
Signed-off-by: Hao Wu <wuhaotsh@google.com>
---
 hw/i2c/npcm7xx_smbus.c           | 331 +++++++++++++++++++++++++++++--
 hw/i2c/trace-events              |   1 +
 include/hw/i2c/npcm7xx_smbus.h   |  25 +++
 tests/qtest/npcm7xx_smbus-test.c | 149 +++++++++++++-
 4 files changed, 490 insertions(+), 16 deletions(-)

Comments

Corey Minyard Jan. 26, 2021, 11:47 p.m. UTC | #1
On Tue, Jan 26, 2021 at 11:32:37AM -0800, wuhaotsh--- via wrote:
> +
> +static void npcm7xx_smbus_read_byte_fifo(NPCM7xxSMBusState *s)
> +{
> +    uint8_t received_bytes = NPCM7XX_SMBRXF_STS_RX_BYTES(s->rxf_sts);
> +
> +    if (received_bytes == 0) {
> +        npcm7xx_smbus_recv_fifo(s);
> +        return;
> +    }
> +
> +    s->sda = s->rx_fifo[s->rx_cur];
> +    s->rx_cur = (s->rx_cur + 1u) % NPCM7XX_SMBUS_FIFO_SIZE;
> +    --s->rxf_sts;

This open-coded decrement seems a little risky.  Are you sure in every
case that s->rxf_sts > 0?  There's no way what's running in the VM can
game this and cause a buffer overrun?  One caller to this function seems
to protect against this, and another does not.

Other than this, I didn't see any issues with this patch.

-corey
Denis V. Lunev" via Jan. 27, 2021, 8:37 p.m. UTC | #2
On Tue, Jan 26, 2021 at 3:47 PM Corey Minyard <minyard@acm.org> wrote:

> On Tue, Jan 26, 2021 at 11:32:37AM -0800, wuhaotsh--- via wrote:
> > +
> > +static void npcm7xx_smbus_read_byte_fifo(NPCM7xxSMBusState *s)
> > +{
> > +    uint8_t received_bytes = NPCM7XX_SMBRXF_STS_RX_BYTES(s->rxf_sts);
> > +
> > +    if (received_bytes == 0) {
> > +        npcm7xx_smbus_recv_fifo(s);
> > +        return;
> > +    }
> > +
> > +    s->sda = s->rx_fifo[s->rx_cur];
> > +    s->rx_cur = (s->rx_cur + 1u) % NPCM7XX_SMBUS_FIFO_SIZE;
> > +    --s->rxf_sts;
>
> This open-coded decrement seems a little risky.  Are you sure in every
> case that s->rxf_sts > 0?  There's no way what's running in the VM can
> game this and cause a buffer overrun?  One caller to this function seems
> to protect against this, and another does not.
>
s->rxf_sts is uint8_t so it's guaranteed to be >=0.
In the case s->rxf_sts == 0,  NPCM7XX_SMBRXF_STS_RX_BYTES(s->rxf_sts) is
also 0, so it'll take the if-branch and return without running --s->rxf_sts.
I'll probably add "g_assert(s->rxf_sts > 0)" to clarify.

>
> Other than this, I didn't see any issues with this patch.
>
> -corey
>
Corey Minyard Jan. 27, 2021, 9:42 p.m. UTC | #3
On Wed, Jan 27, 2021 at 12:37:46PM -0800, wuhaotsh--- via wrote:
> On Tue, Jan 26, 2021 at 3:47 PM Corey Minyard <minyard@acm.org> wrote:
> 
> > On Tue, Jan 26, 2021 at 11:32:37AM -0800, wuhaotsh--- via wrote:
> > > +
> > > +static void npcm7xx_smbus_read_byte_fifo(NPCM7xxSMBusState *s)
> > > +{
> > > +    uint8_t received_bytes = NPCM7XX_SMBRXF_STS_RX_BYTES(s->rxf_sts);
> > > +
> > > +    if (received_bytes == 0) {
> > > +        npcm7xx_smbus_recv_fifo(s);
> > > +        return;
> > > +    }
> > > +
> > > +    s->sda = s->rx_fifo[s->rx_cur];
> > > +    s->rx_cur = (s->rx_cur + 1u) % NPCM7XX_SMBUS_FIFO_SIZE;
> > > +    --s->rxf_sts;
> >
> > This open-coded decrement seems a little risky.  Are you sure in every
> > case that s->rxf_sts > 0?  There's no way what's running in the VM can
> > game this and cause a buffer overrun?  One caller to this function seems
> > to protect against this, and another does not.
> >
> s->rxf_sts is uint8_t so it's guaranteed to be >=0.
> In the case s->rxf_sts == 0,  NPCM7XX_SMBRXF_STS_RX_BYTES(s->rxf_sts) is
> also 0, so it'll take the if-branch and return without running --s->rxf_sts.

That is true if called from the
NPCM7XX_SMBUS_STATUS_STOPPING_LAST_RECEIVE case.  There is no such check
in the NPCM7XX_SMBUS_STATUS_RECEIVING case.

> I'll probably add "g_assert(s->rxf_sts > 0)" to clarify.

You never want to do an assert if the hosted system can do something to
cause it.  If you add the check to the NPCM7XX_SMBUS_STATUS_RECEIVING
case, it would be ok, but really unnecessary.

If it's fine if s->rxf_sts wraps to 0xff, then this all doesn't matter,
but you want to add a comment to that effect if so.  These sorts of
things look dangerous.

There is also the question about who takes these patches in.  I'm the
I2C maintainer, but there's other code in this series.  Once everything
is ready, I can ack them if we take it through the ARM tree.  Or I can
take it through my tree with the proper acks.

-corey

> 
> >
> > Other than this, I didn't see any issues with this patch.
> >
> > -corey
> >
Denis V. Lunev" via Jan. 27, 2021, 9:59 p.m. UTC | #4
On Wed, Jan 27, 2021 at 1:42 PM Corey Minyard <minyard@acm.org> wrote:

> On Wed, Jan 27, 2021 at 12:37:46PM -0800, wuhaotsh--- via wrote:
> > On Tue, Jan 26, 2021 at 3:47 PM Corey Minyard <minyard@acm.org> wrote:
> >
> > > On Tue, Jan 26, 2021 at 11:32:37AM -0800, wuhaotsh--- via wrote:
> > > > +
> > > > +static void npcm7xx_smbus_read_byte_fifo(NPCM7xxSMBusState *s)
> > > > +{
> > > > +    uint8_t received_bytes =
> NPCM7XX_SMBRXF_STS_RX_BYTES(s->rxf_sts);
> > > > +
> > > > +    if (received_bytes == 0) {
> > > > +        npcm7xx_smbus_recv_fifo(s);
> > > > +        return;
> > > > +    }
> > > > +
> > > > +    s->sda = s->rx_fifo[s->rx_cur];
> > > > +    s->rx_cur = (s->rx_cur + 1u) % NPCM7XX_SMBUS_FIFO_SIZE;
> > > > +    --s->rxf_sts;
> > >
> > > This open-coded decrement seems a little risky.  Are you sure in every
> > > case that s->rxf_sts > 0?  There's no way what's running in the VM can
> > > game this and cause a buffer overrun?  One caller to this function
> seems
> > > to protect against this, and another does not.
> > >
> > s->rxf_sts is uint8_t so it's guaranteed to be >=0.
> > In the case s->rxf_sts == 0,  NPCM7XX_SMBRXF_STS_RX_BYTES(s->rxf_sts) is
> > also 0, so it'll take the if-branch and return without running
> --s->rxf_sts.
>
> That is true if called from the
> NPCM7XX_SMBUS_STATUS_STOPPING_LAST_RECEIVE case.  There is no such check
> in the NPCM7XX_SMBUS_STATUS_RECEIVING case.
>
I don't understand the reasoning here. The caller doesn't matter.
Previous code has:
 #define NPCM7XX_SMBRXF_STS_RX_BYTES(rv)     extract8((rv), 0, 5)
So
 uint8_t received_bytes = NPCM7XX_SMBRXF_STS_RX_BYTES(s->rxf_sts);
is guaranteed to be 0 if s->rxf_sts == 0.
As a result the code will take the following branch and returns:
 if (received_bytes == 0) {
    npcm7xx_smbus_recv_fifo(s);
    return;
 }
And will not execute the --s->rxf_sts sentence.
Please let me know if I missed anything here.

>
> > I'll probably add "g_assert(s->rxf_sts > 0)" to clarify.
>
> You never want to do an assert if the hosted system can do something to
> cause it.  If you add the check to the NPCM7XX_SMBUS_STATUS_RECEIVING
> case, it would be ok, but really unnecessary.
>
> If it's fine if s->rxf_sts wraps to 0xff, then this all doesn't matter,
> but you want to add a comment to that effect if so.  These sorts of
> things look dangerous.
>
> There is also the question about who takes these patches in.  I'm the
> I2C maintainer, but there's other code in this series.  Once everything
> is ready, I can ack them if we take it through the ARM tree.  Or I can
> take it through my tree with the proper acks.
>
I think either  way is fine. Previous NPCM7XX patch series were taken in
the ARM tree. But as i2c code taking into your tree is also fine.

>
> -corey
>
> >
> > >
> > > Other than this, I didn't see any issues with this patch.
> > >
> > > -corey
> > >
>
Corey Minyard Jan. 27, 2021, 11:37 p.m. UTC | #5
On Wed, Jan 27, 2021 at 01:59:07PM -0800, Hao Wu wrote:
> On Wed, Jan 27, 2021 at 1:42 PM Corey Minyard <minyard@acm.org> wrote:
> 
> > On Wed, Jan 27, 2021 at 12:37:46PM -0800, wuhaotsh--- via wrote:
> > > On Tue, Jan 26, 2021 at 3:47 PM Corey Minyard <minyard@acm.org> wrote:
> > >
> > > > On Tue, Jan 26, 2021 at 11:32:37AM -0800, wuhaotsh--- via wrote:
> > > > > +
> > > > > +static void npcm7xx_smbus_read_byte_fifo(NPCM7xxSMBusState *s)
> > > > > +{
> > > > > +    uint8_t received_bytes =
> > NPCM7XX_SMBRXF_STS_RX_BYTES(s->rxf_sts);
> > > > > +
> > > > > +    if (received_bytes == 0) {
> > > > > +        npcm7xx_smbus_recv_fifo(s);
> > > > > +        return;
> > > > > +    }
> > > > > +
> > > > > +    s->sda = s->rx_fifo[s->rx_cur];
> > > > > +    s->rx_cur = (s->rx_cur + 1u) % NPCM7XX_SMBUS_FIFO_SIZE;
> > > > > +    --s->rxf_sts;
> > > >
> > > > This open-coded decrement seems a little risky.  Are you sure in every
> > > > case that s->rxf_sts > 0?  There's no way what's running in the VM can
> > > > game this and cause a buffer overrun?  One caller to this function
> > seems
> > > > to protect against this, and another does not.
> > > >
> > > s->rxf_sts is uint8_t so it's guaranteed to be >=0.
> > > In the case s->rxf_sts == 0,  NPCM7XX_SMBRXF_STS_RX_BYTES(s->rxf_sts) is
> > > also 0, so it'll take the if-branch and return without running
> > --s->rxf_sts.
> >
> > That is true if called from the
> > NPCM7XX_SMBUS_STATUS_STOPPING_LAST_RECEIVE case.  There is no such check
> > in the NPCM7XX_SMBUS_STATUS_RECEIVING case.
> >
> I don't understand the reasoning here. The caller doesn't matter.
> Previous code has:
>  #define NPCM7XX_SMBRXF_STS_RX_BYTES(rv)     extract8((rv), 0, 5)
> So
>  uint8_t received_bytes = NPCM7XX_SMBRXF_STS_RX_BYTES(s->rxf_sts);
> is guaranteed to be 0 if s->rxf_sts == 0.
> As a result the code will take the following branch and returns:
>  if (received_bytes == 0) {
>     npcm7xx_smbus_recv_fifo(s);
>     return;
>  }
> And will not execute the --s->rxf_sts sentence.
> Please let me know if I missed anything here.

Ah, sorry, I missed that.  Yes, this is ok.  So...

Reviewed-by: Corey Minyard <cminyard@mvista.com>

> 
> >
> > > I'll probably add "g_assert(s->rxf_sts > 0)" to clarify.
> >
> > You never want to do an assert if the hosted system can do something to
> > cause it.  If you add the check to the NPCM7XX_SMBUS_STATUS_RECEIVING
> > case, it would be ok, but really unnecessary.
> >
> > If it's fine if s->rxf_sts wraps to 0xff, then this all doesn't matter,
> > but you want to add a comment to that effect if so.  These sorts of
> > things look dangerous.
> >
> > There is also the question about who takes these patches in.  I'm the
> > I2C maintainer, but there's other code in this series.  Once everything
> > is ready, I can ack them if we take it through the ARM tree.  Or I can
> > take it through my tree with the proper acks.
> >
> I think either  way is fine. Previous NPCM7XX patch series were taken in
> the ARM tree. But as i2c code taking into your tree is also fine.
> 
> >
> > -corey
> >
> > >
> > > >
> > > > Other than this, I didn't see any issues with this patch.
> > > >
> > > > -corey
> > > >
> >
Corey Minyard Jan. 28, 2021, 5:36 a.m. UTC | #6
On Wed, Jan 27, 2021 at 05:37:25PM -0600, Corey Minyard wrote:
> On Wed, Jan 27, 2021 at 01:59:07PM -0800, Hao Wu wrote:
> > >
> > > There is also the question about who takes these patches in.  I'm the
> > > I2C maintainer, but there's other code in this series.  Once everything
> > > is ready, I can ack them if we take it through the ARM tree.  Or I can
> > > take it through my tree with the proper acks.
> > >
> > I think either  way is fine. Previous NPCM7XX patch series were taken in
> > the ARM tree. But as i2c code taking into your tree is also fine.
> > 

Let's go through the ARM tree, then.  So you have an:

Acked-by: Corey Minyard <cminyard@mvista.com>

For patches 2 and 6.

Patch 4 still has the issue with the eeprom size.  If you are expecting
a 32K eeprom to work, it's not going to, you are only going to get 256
bytes.

-corey
diff mbox series

Patch

diff --git a/hw/i2c/npcm7xx_smbus.c b/hw/i2c/npcm7xx_smbus.c
index e8a8fdbaff..19a9cdb179 100644
--- a/hw/i2c/npcm7xx_smbus.c
+++ b/hw/i2c/npcm7xx_smbus.c
@@ -27,7 +27,7 @@ 
 #include "trace.h"
 
 #define NPCM7XX_SMBUS_VERSION 1
-#define NPCM7XX_SMBUS_FIFO_EN 0
+#define NPCM7XX_SMBUS_FIFO_EN 1
 
 enum NPCM7xxSMBusCommonRegister {
     NPCM7XX_SMB_SDA     = 0x0,
@@ -132,10 +132,41 @@  enum NPCM7xxSMBusBank1Register {
 #define NPCM7XX_ADDR_EN             BIT(7)
 #define NPCM7XX_ADDR_A(rv)          extract8((rv), 0, 6)
 
+/* FIFO Mode Register Fields */
+/* FIF_CTL fields */
+#define NPCM7XX_SMBFIF_CTL_FIFO_EN          BIT(4)
+#define NPCM7XX_SMBFIF_CTL_FAIR_RDY_IE      BIT(2)
+#define NPCM7XX_SMBFIF_CTL_FAIR_RDY         BIT(1)
+#define NPCM7XX_SMBFIF_CTL_FAIR_BUSY        BIT(0)
+/* FIF_CTS fields */
+#define NPCM7XX_SMBFIF_CTS_STR              BIT(7)
+#define NPCM7XX_SMBFIF_CTS_CLR_FIFO         BIT(6)
+#define NPCM7XX_SMBFIF_CTS_RFTE_IE          BIT(3)
+#define NPCM7XX_SMBFIF_CTS_RXF_TXE          BIT(1)
+/* TXF_CTL fields */
+#define NPCM7XX_SMBTXF_CTL_THR_TXIE         BIT(6)
+#define NPCM7XX_SMBTXF_CTL_TX_THR(rv)       extract8((rv), 0, 5)
+/* T_OUT fields */
+#define NPCM7XX_SMBT_OUT_ST                 BIT(7)
+#define NPCM7XX_SMBT_OUT_IE                 BIT(6)
+#define NPCM7XX_SMBT_OUT_CLKDIV(rv)         extract8((rv), 0, 6)
+/* TXF_STS fields */
+#define NPCM7XX_SMBTXF_STS_TX_THST          BIT(6)
+#define NPCM7XX_SMBTXF_STS_TX_BYTES(rv)     extract8((rv), 0, 5)
+/* RXF_STS fields */
+#define NPCM7XX_SMBRXF_STS_RX_THST          BIT(6)
+#define NPCM7XX_SMBRXF_STS_RX_BYTES(rv)     extract8((rv), 0, 5)
+/* RXF_CTL fields */
+#define NPCM7XX_SMBRXF_CTL_THR_RXIE         BIT(6)
+#define NPCM7XX_SMBRXF_CTL_LAST             BIT(5)
+#define NPCM7XX_SMBRXF_CTL_RX_THR(rv)       extract8((rv), 0, 5)
+
 #define KEEP_OLD_BIT(o, n, b)       (((n) & (~(b))) | ((o) & (b)))
 #define WRITE_ONE_CLEAR(o, n, b)    ((n) & (b) ? (o) & (~(b)) : (o))
 
 #define NPCM7XX_SMBUS_ENABLED(s)    ((s)->ctl2 & NPCM7XX_SMBCTL2_ENABLE)
+#define NPCM7XX_SMBUS_FIFO_ENABLED(s) (NPCM7XX_SMBUS_FIFO_EN && \
+        (s)->fif_ctl & NPCM7XX_SMBFIF_CTL_FIFO_EN)
 
 /* Reset values */
 #define NPCM7XX_SMB_ST_INIT_VAL     0x00
@@ -150,6 +181,14 @@  enum NPCM7xxSMBusBank1Register {
 #define NPCM7XX_SMB_ADDR_INIT_VAL   0x00
 #define NPCM7XX_SMB_SCLLT_INIT_VAL  0x00
 #define NPCM7XX_SMB_SCLHT_INIT_VAL  0x00
+#define NPCM7XX_SMB_FIF_CTL_INIT_VAL 0x00
+#define NPCM7XX_SMB_FIF_CTS_INIT_VAL 0x00
+#define NPCM7XX_SMB_FAIR_PER_INIT_VAL 0x00
+#define NPCM7XX_SMB_TXF_CTL_INIT_VAL 0x00
+#define NPCM7XX_SMB_T_OUT_INIT_VAL 0x3f
+#define NPCM7XX_SMB_TXF_STS_INIT_VAL 0x00
+#define NPCM7XX_SMB_RXF_STS_INIT_VAL 0x00
+#define NPCM7XX_SMB_RXF_CTL_INIT_VAL 0x01
 
 static uint8_t npcm7xx_smbus_get_version(void)
 {
@@ -169,7 +208,13 @@  static void npcm7xx_smbus_update_irq(NPCM7xxSMBusState *s)
                    (s->ctl1 & NPCM7XX_SMBCTL1_STASTRE &&
                     s->st & NPCM7XX_SMBST_SDAST) ||
                    (s->ctl1 & NPCM7XX_SMBCTL1_EOBINTE &&
-                    s->cst3 & NPCM7XX_SMBCST3_EO_BUSY));
+                    s->cst3 & NPCM7XX_SMBCST3_EO_BUSY) ||
+                   (s->rxf_ctl & NPCM7XX_SMBRXF_CTL_THR_RXIE &&
+                    s->rxf_sts & NPCM7XX_SMBRXF_STS_RX_THST) ||
+                   (s->txf_ctl & NPCM7XX_SMBTXF_CTL_THR_TXIE &&
+                    s->txf_sts & NPCM7XX_SMBTXF_STS_TX_THST) ||
+                   (s->fif_cts & NPCM7XX_SMBFIF_CTS_RFTE_IE &&
+                    s->fif_cts & NPCM7XX_SMBFIF_CTS_RXF_TXE));
 
         if (level) {
             s->cst2 |= NPCM7XX_SMBCST2_INTSTS;
@@ -187,6 +232,13 @@  static void npcm7xx_smbus_nack(NPCM7xxSMBusState *s)
     s->status = NPCM7XX_SMBUS_STATUS_NEGACK;
 }
 
+static void npcm7xx_smbus_clear_buffer(NPCM7xxSMBusState *s)
+{
+    s->fif_cts &= ~NPCM7XX_SMBFIF_CTS_RXF_TXE;
+    s->txf_sts = 0;
+    s->rxf_sts = 0;
+}
+
 static void npcm7xx_smbus_send_byte(NPCM7xxSMBusState *s, uint8_t value)
 {
     int rv = i2c_send(s->bus, value);
@@ -195,6 +247,15 @@  static void npcm7xx_smbus_send_byte(NPCM7xxSMBusState *s, uint8_t value)
         npcm7xx_smbus_nack(s);
     } else {
         s->st |= NPCM7XX_SMBST_SDAST;
+        if (NPCM7XX_SMBUS_FIFO_ENABLED(s)) {
+            s->fif_cts |= NPCM7XX_SMBFIF_CTS_RXF_TXE;
+            if (NPCM7XX_SMBTXF_STS_TX_BYTES(s->txf_sts) ==
+                NPCM7XX_SMBTXF_CTL_TX_THR(s->txf_ctl)) {
+                s->txf_sts = NPCM7XX_SMBTXF_STS_TX_THST;
+            } else {
+                s->txf_sts = 0;
+            }
+        }
     }
     trace_npcm7xx_smbus_send_byte((DEVICE(s)->canonical_path), value, !rv);
     npcm7xx_smbus_update_irq(s);
@@ -213,6 +274,67 @@  static void npcm7xx_smbus_recv_byte(NPCM7xxSMBusState *s)
     npcm7xx_smbus_update_irq(s);
 }
 
+static void npcm7xx_smbus_recv_fifo(NPCM7xxSMBusState *s)
+{
+    uint8_t expected_bytes = NPCM7XX_SMBRXF_CTL_RX_THR(s->rxf_ctl);
+    uint8_t received_bytes = NPCM7XX_SMBRXF_STS_RX_BYTES(s->rxf_sts);
+    uint8_t pos;
+
+    if (received_bytes == expected_bytes) {
+        return;
+    }
+
+    while (received_bytes < expected_bytes &&
+           received_bytes < NPCM7XX_SMBUS_FIFO_SIZE) {
+        pos = (s->rx_cur + received_bytes) % NPCM7XX_SMBUS_FIFO_SIZE;
+        s->rx_fifo[pos] = i2c_recv(s->bus);
+        trace_npcm7xx_smbus_recv_byte((DEVICE(s)->canonical_path),
+                                      s->rx_fifo[pos]);
+        ++received_bytes;
+    }
+
+    trace_npcm7xx_smbus_recv_fifo((DEVICE(s)->canonical_path),
+                                  received_bytes, expected_bytes);
+    s->rxf_sts = received_bytes;
+    if (unlikely(received_bytes < expected_bytes)) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: invalid rx_thr value: 0x%02x\n",
+                      DEVICE(s)->canonical_path, expected_bytes);
+        return;
+    }
+
+    s->rxf_sts |= NPCM7XX_SMBRXF_STS_RX_THST;
+    if (s->rxf_ctl & NPCM7XX_SMBRXF_CTL_LAST) {
+        trace_npcm7xx_smbus_nack(DEVICE(s)->canonical_path);
+        i2c_nack(s->bus);
+        s->rxf_ctl &= ~NPCM7XX_SMBRXF_CTL_LAST;
+    }
+    if (received_bytes == NPCM7XX_SMBUS_FIFO_SIZE) {
+        s->st |= NPCM7XX_SMBST_SDAST;
+        s->fif_cts |= NPCM7XX_SMBFIF_CTS_RXF_TXE;
+    } else if (!(s->rxf_ctl & NPCM7XX_SMBRXF_CTL_THR_RXIE)) {
+        s->st |= NPCM7XX_SMBST_SDAST;
+    } else {
+        s->st &= ~NPCM7XX_SMBST_SDAST;
+    }
+    npcm7xx_smbus_update_irq(s);
+}
+
+static void npcm7xx_smbus_read_byte_fifo(NPCM7xxSMBusState *s)
+{
+    uint8_t received_bytes = NPCM7XX_SMBRXF_STS_RX_BYTES(s->rxf_sts);
+
+    if (received_bytes == 0) {
+        npcm7xx_smbus_recv_fifo(s);
+        return;
+    }
+
+    s->sda = s->rx_fifo[s->rx_cur];
+    s->rx_cur = (s->rx_cur + 1u) % NPCM7XX_SMBUS_FIFO_SIZE;
+    --s->rxf_sts;
+    npcm7xx_smbus_update_irq(s);
+}
+
 static void npcm7xx_smbus_start(NPCM7xxSMBusState *s)
 {
     /*
@@ -226,6 +348,9 @@  static void npcm7xx_smbus_start(NPCM7xxSMBusState *s)
     if (available) {
         s->st |= NPCM7XX_SMBST_MODE | NPCM7XX_SMBST_XMIT | NPCM7XX_SMBST_SDAST;
         s->cst |= NPCM7XX_SMBCST_BUSY;
+        if (NPCM7XX_SMBUS_FIFO_ENABLED(s)) {
+            s->fif_cts |= NPCM7XX_SMBFIF_CTS_RXF_TXE;
+        }
     } else {
         s->st &= ~NPCM7XX_SMBST_MODE;
         s->cst &= ~NPCM7XX_SMBCST_BUSY;
@@ -277,7 +402,15 @@  static void npcm7xx_smbus_send_address(NPCM7xxSMBusState *s, uint8_t value)
             s->st |= NPCM7XX_SMBST_SDAST;
         }
     } else if (recv) {
-        npcm7xx_smbus_recv_byte(s);
+        s->st |= NPCM7XX_SMBST_SDAST;
+        if (NPCM7XX_SMBUS_FIFO_ENABLED(s)) {
+            npcm7xx_smbus_recv_fifo(s);
+        } else {
+            npcm7xx_smbus_recv_byte(s);
+        }
+    } else if (NPCM7XX_SMBUS_FIFO_ENABLED(s)) {
+        s->st |= NPCM7XX_SMBST_SDAST;
+        s->fif_cts |= NPCM7XX_SMBFIF_CTS_RXF_TXE;
     }
     npcm7xx_smbus_update_irq(s);
 }
@@ -320,11 +453,31 @@  static uint8_t npcm7xx_smbus_read_sda(NPCM7xxSMBusState *s)
 
     switch (s->status) {
     case NPCM7XX_SMBUS_STATUS_STOPPING_LAST_RECEIVE:
-        npcm7xx_smbus_execute_stop(s);
+        if (NPCM7XX_SMBUS_FIFO_ENABLED(s)) {
+            if (NPCM7XX_SMBRXF_STS_RX_BYTES(s->rxf_sts) <= 1) {
+                npcm7xx_smbus_execute_stop(s);
+            }
+            if (NPCM7XX_SMBRXF_STS_RX_BYTES(s->rxf_sts) == 0) {
+                qemu_log_mask(LOG_GUEST_ERROR,
+                              "%s: read to SDA with an empty rx-fifo buffer, "
+                              "result undefined: %u\n",
+                              DEVICE(s)->canonical_path, s->sda);
+                break;
+            }
+            npcm7xx_smbus_read_byte_fifo(s);
+            value = s->sda;
+        } else {
+            npcm7xx_smbus_execute_stop(s);
+        }
         break;
 
     case NPCM7XX_SMBUS_STATUS_RECEIVING:
-        npcm7xx_smbus_recv_byte(s);
+        if (NPCM7XX_SMBUS_FIFO_ENABLED(s)) {
+            npcm7xx_smbus_read_byte_fifo(s);
+            value = s->sda;
+        } else {
+            npcm7xx_smbus_recv_byte(s);
+        }
         break;
 
     default:
@@ -370,8 +523,12 @@  static void npcm7xx_smbus_write_st(NPCM7xxSMBusState *s, uint8_t value)
     }
 
     if (value & NPCM7XX_SMBST_STASTR &&
-            s->status == NPCM7XX_SMBUS_STATUS_RECEIVING) {
-        npcm7xx_smbus_recv_byte(s);
+        s->status == NPCM7XX_SMBUS_STATUS_RECEIVING) {
+        if (NPCM7XX_SMBUS_FIFO_ENABLED(s)) {
+            npcm7xx_smbus_recv_fifo(s);
+        } else {
+            npcm7xx_smbus_recv_byte(s);
+        }
     }
 
     npcm7xx_smbus_update_irq(s);
@@ -417,6 +574,7 @@  static void npcm7xx_smbus_write_ctl2(NPCM7xxSMBusState *s, uint8_t value)
         s->st = 0;
         s->cst3 = s->cst3 & (~NPCM7XX_SMBCST3_EO_BUSY);
         s->cst = 0;
+        npcm7xx_smbus_clear_buffer(s);
     }
 }
 
@@ -429,6 +587,70 @@  static void npcm7xx_smbus_write_ctl3(NPCM7xxSMBusState *s, uint8_t value)
                             NPCM7XX_SMBCTL3_SCL_LVL | NPCM7XX_SMBCTL3_SDA_LVL);
 }
 
+static void npcm7xx_smbus_write_fif_ctl(NPCM7xxSMBusState *s, uint8_t value)
+{
+    uint8_t new_ctl = value;
+
+    new_ctl = KEEP_OLD_BIT(s->fif_ctl, new_ctl, NPCM7XX_SMBFIF_CTL_FAIR_RDY);
+    new_ctl = WRITE_ONE_CLEAR(new_ctl, value, NPCM7XX_SMBFIF_CTL_FAIR_RDY);
+    new_ctl = KEEP_OLD_BIT(s->fif_ctl, new_ctl, NPCM7XX_SMBFIF_CTL_FAIR_BUSY);
+    s->fif_ctl = new_ctl;
+}
+
+static void npcm7xx_smbus_write_fif_cts(NPCM7xxSMBusState *s, uint8_t value)
+{
+    s->fif_cts = WRITE_ONE_CLEAR(s->fif_cts, value, NPCM7XX_SMBFIF_CTS_STR);
+    s->fif_cts = WRITE_ONE_CLEAR(s->fif_cts, value, NPCM7XX_SMBFIF_CTS_RXF_TXE);
+    s->fif_cts = KEEP_OLD_BIT(value, s->fif_cts, NPCM7XX_SMBFIF_CTS_RFTE_IE);
+
+    if (value & NPCM7XX_SMBFIF_CTS_CLR_FIFO) {
+        npcm7xx_smbus_clear_buffer(s);
+    }
+}
+
+static void npcm7xx_smbus_write_txf_ctl(NPCM7xxSMBusState *s, uint8_t value)
+{
+    s->txf_ctl = value;
+}
+
+static void npcm7xx_smbus_write_t_out(NPCM7xxSMBusState *s, uint8_t value)
+{
+    uint8_t new_t_out = value;
+
+    if ((value & NPCM7XX_SMBT_OUT_ST) || (!(s->t_out & NPCM7XX_SMBT_OUT_ST))) {
+        new_t_out &= ~NPCM7XX_SMBT_OUT_ST;
+    } else {
+        new_t_out |= NPCM7XX_SMBT_OUT_ST;
+    }
+
+    s->t_out = new_t_out;
+}
+
+static void npcm7xx_smbus_write_txf_sts(NPCM7xxSMBusState *s, uint8_t value)
+{
+    s->txf_sts = WRITE_ONE_CLEAR(s->txf_sts, value, NPCM7XX_SMBTXF_STS_TX_THST);
+}
+
+static void npcm7xx_smbus_write_rxf_sts(NPCM7xxSMBusState *s, uint8_t value)
+{
+    if (value & NPCM7XX_SMBRXF_STS_RX_THST) {
+        s->rxf_sts &= ~NPCM7XX_SMBRXF_STS_RX_THST;
+        if (s->status == NPCM7XX_SMBUS_STATUS_RECEIVING) {
+            npcm7xx_smbus_recv_fifo(s);
+        }
+    }
+}
+
+static void npcm7xx_smbus_write_rxf_ctl(NPCM7xxSMBusState *s, uint8_t value)
+{
+    uint8_t new_ctl = value;
+
+    if (!(value & NPCM7XX_SMBRXF_CTL_LAST)) {
+        new_ctl = KEEP_OLD_BIT(s->rxf_ctl, new_ctl, NPCM7XX_SMBRXF_CTL_LAST);
+    }
+    s->rxf_ctl = new_ctl;
+}
+
 static uint64_t npcm7xx_smbus_read(void *opaque, hwaddr offset, unsigned size)
 {
     NPCM7xxSMBusState *s = opaque;
@@ -484,9 +706,41 @@  static uint64_t npcm7xx_smbus_read(void *opaque, hwaddr offset, unsigned size)
     default:
         if (bank) {
             /* Bank 1 */
-            qemu_log_mask(LOG_GUEST_ERROR,
-                    "%s: read from invalid offset 0x%" HWADDR_PRIx "\n",
-                    DEVICE(s)->canonical_path, offset);
+            switch (offset) {
+            case NPCM7XX_SMB_FIF_CTS:
+                value = s->fif_cts;
+                break;
+
+            case NPCM7XX_SMB_FAIR_PER:
+                value = s->fair_per;
+                break;
+
+            case NPCM7XX_SMB_TXF_CTL:
+                value = s->txf_ctl;
+                break;
+
+            case NPCM7XX_SMB_T_OUT:
+                value = s->t_out;
+                break;
+
+            case NPCM7XX_SMB_TXF_STS:
+                value = s->txf_sts;
+                break;
+
+            case NPCM7XX_SMB_RXF_STS:
+                value = s->rxf_sts;
+                break;
+
+            case NPCM7XX_SMB_RXF_CTL:
+                value = s->rxf_ctl;
+                break;
+
+            default:
+                qemu_log_mask(LOG_GUEST_ERROR,
+                        "%s: read from invalid offset 0x%" HWADDR_PRIx "\n",
+                        DEVICE(s)->canonical_path, offset);
+                break;
+            }
         } else {
             /* Bank 0 */
             switch (offset) {
@@ -534,6 +788,10 @@  static uint64_t npcm7xx_smbus_read(void *opaque, hwaddr offset, unsigned size)
                 value = s->scllt;
                 break;
 
+            case NPCM7XX_SMB_FIF_CTL:
+                value = s->fif_ctl;
+                break;
+
             case NPCM7XX_SMB_SCLHT:
                 value = s->sclht;
                 break;
@@ -614,9 +872,41 @@  static void npcm7xx_smbus_write(void *opaque, hwaddr offset, uint64_t value,
     default:
         if (bank) {
             /* Bank 1 */
-            qemu_log_mask(LOG_GUEST_ERROR,
-                    "%s: write to invalid offset 0x%" HWADDR_PRIx "\n",
-                    DEVICE(s)->canonical_path, offset);
+            switch (offset) {
+            case NPCM7XX_SMB_FIF_CTS:
+                npcm7xx_smbus_write_fif_cts(s, value);
+                break;
+
+            case NPCM7XX_SMB_FAIR_PER:
+                s->fair_per = value;
+                break;
+
+            case NPCM7XX_SMB_TXF_CTL:
+                npcm7xx_smbus_write_txf_ctl(s, value);
+                break;
+
+            case NPCM7XX_SMB_T_OUT:
+                npcm7xx_smbus_write_t_out(s, value);
+                break;
+
+            case NPCM7XX_SMB_TXF_STS:
+                npcm7xx_smbus_write_txf_sts(s, value);
+                break;
+
+            case NPCM7XX_SMB_RXF_STS:
+                npcm7xx_smbus_write_rxf_sts(s, value);
+                break;
+
+            case NPCM7XX_SMB_RXF_CTL:
+                npcm7xx_smbus_write_rxf_ctl(s, value);
+                break;
+
+            default:
+                qemu_log_mask(LOG_GUEST_ERROR,
+                        "%s: write to invalid offset 0x%" HWADDR_PRIx "\n",
+                        DEVICE(s)->canonical_path, offset);
+                break;
+            }
         } else {
             /* Bank 0 */
             switch (offset) {
@@ -664,6 +954,10 @@  static void npcm7xx_smbus_write(void *opaque, hwaddr offset, uint64_t value,
                 s->scllt = value;
                 break;
 
+            case NPCM7XX_SMB_FIF_CTL:
+                npcm7xx_smbus_write_fif_ctl(s, value);
+                break;
+
             case NPCM7XX_SMB_SCLHT:
                 s->sclht = value;
                 break;
@@ -710,7 +1004,18 @@  static void npcm7xx_smbus_enter_reset(Object *obj, ResetType type)
     s->scllt = NPCM7XX_SMB_SCLLT_INIT_VAL;
     s->sclht = NPCM7XX_SMB_SCLHT_INIT_VAL;
 
+    s->fif_ctl = NPCM7XX_SMB_FIF_CTL_INIT_VAL;
+    s->fif_cts = NPCM7XX_SMB_FIF_CTS_INIT_VAL;
+    s->fair_per = NPCM7XX_SMB_FAIR_PER_INIT_VAL;
+    s->txf_ctl = NPCM7XX_SMB_TXF_CTL_INIT_VAL;
+    s->t_out = NPCM7XX_SMB_T_OUT_INIT_VAL;
+    s->txf_sts = NPCM7XX_SMB_TXF_STS_INIT_VAL;
+    s->rxf_sts = NPCM7XX_SMB_RXF_STS_INIT_VAL;
+    s->rxf_ctl = NPCM7XX_SMB_RXF_CTL_INIT_VAL;
+
+    npcm7xx_smbus_clear_buffer(s);
     s->status = NPCM7XX_SMBUS_STATUS_IDLE;
+    s->rx_cur = 0;
 }
 
 static void npcm7xx_smbus_hold_reset(Object *obj)
diff --git a/hw/i2c/trace-events b/hw/i2c/trace-events
index c3bb70ad04..82fe6f965f 100644
--- a/hw/i2c/trace-events
+++ b/hw/i2c/trace-events
@@ -25,3 +25,4 @@  npcm7xx_smbus_send_byte(const char *id, uint8_t value, int success) "%s send byt
 npcm7xx_smbus_recv_byte(const char *id, uint8_t value) "%s recv byte: 0x%02x"
 npcm7xx_smbus_stop(const char *id) "%s stopping"
 npcm7xx_smbus_nack(const char *id) "%s nacking"
+npcm7xx_smbus_recv_fifo(const char *id, uint8_t received, uint8_t expected) "%s recv fifo: received %u, expected %u"
diff --git a/include/hw/i2c/npcm7xx_smbus.h b/include/hw/i2c/npcm7xx_smbus.h
index b9761a6993..7d59ee917e 100644
--- a/include/hw/i2c/npcm7xx_smbus.h
+++ b/include/hw/i2c/npcm7xx_smbus.h
@@ -27,6 +27,9 @@ 
  */
 #define NPCM7XX_SMBUS_NR_ADDRS 10
 
+/* Size of the FIFO buffer. */
+#define NPCM7XX_SMBUS_FIFO_SIZE 16
+
 typedef enum NPCM7xxSMBusStatus {
     NPCM7XX_SMBUS_STATUS_IDLE,
     NPCM7XX_SMBUS_STATUS_SENDING,
@@ -53,6 +56,16 @@  typedef enum NPCM7xxSMBusStatus {
  * @addr: The SMBus module's own addresses on the I2C bus.
  * @scllt: The SCL low time register.
  * @sclht: The SCL high time register.
+ * @fif_ctl: The FIFO control register.
+ * @fif_cts: The FIFO control status register.
+ * @fair_per: The fair preriod register.
+ * @txf_ctl: The transmit FIFO control register.
+ * @t_out: The SMBus timeout register.
+ * @txf_sts: The transmit FIFO status register.
+ * @rxf_sts: The receive FIFO status register.
+ * @rxf_ctl: The receive FIFO control 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 NPCM7xxSMBusState {
@@ -78,6 +91,18 @@  typedef struct NPCM7xxSMBusState {
     uint8_t      scllt;
     uint8_t      sclht;
 
+    uint8_t      fif_ctl;
+    uint8_t      fif_cts;
+    uint8_t      fair_per;
+    uint8_t      txf_ctl;
+    uint8_t      t_out;
+    uint8_t      txf_sts;
+    uint8_t      rxf_sts;
+    uint8_t      rxf_ctl;
+
+    uint8_t      rx_fifo[NPCM7XX_SMBUS_FIFO_SIZE];
+    uint8_t      rx_cur;
+
     NPCM7xxSMBusStatus status;
 } NPCM7xxSMBusState;
 
diff --git a/tests/qtest/npcm7xx_smbus-test.c b/tests/qtest/npcm7xx_smbus-test.c
index 4594b107df..4f9f493872 100644
--- a/tests/qtest/npcm7xx_smbus-test.c
+++ b/tests/qtest/npcm7xx_smbus-test.c
@@ -132,6 +132,44 @@  enum NPCM7xxSMBusBank1Register {
 #define ADDR_EN             BIT(7)
 #define ADDR_A(rv)          extract8((rv), 0, 6)
 
+/* FIF_CTL fields */
+#define FIF_CTL_FIFO_EN         BIT(4)
+
+/* FIF_CTS fields */
+#define FIF_CTS_CLR_FIFO        BIT(6)
+#define FIF_CTS_RFTE_IE         BIT(3)
+#define FIF_CTS_RXF_TXE         BIT(1)
+
+/* TXF_CTL fields */
+#define TXF_CTL_THR_TXIE        BIT(6)
+#define TXF_CTL_TX_THR(rv)      extract8((rv), 0, 5)
+
+/* TXF_STS fields */
+#define TXF_STS_TX_THST         BIT(6)
+#define TXF_STS_TX_BYTES(rv)    extract8((rv), 0, 5)
+
+/* RXF_CTL fields */
+#define RXF_CTL_THR_RXIE        BIT(6)
+#define RXF_CTL_LAST            BIT(5)
+#define RXF_CTL_RX_THR(rv)      extract8((rv), 0, 5)
+
+/* RXF_STS fields */
+#define RXF_STS_RX_THST         BIT(6)
+#define RXF_STS_RX_BYTES(rv)    extract8((rv), 0, 5)
+
+
+static void choose_bank(QTestState *qts, uint64_t base_addr, uint8_t bank)
+{
+    uint8_t ctl3 = qtest_readb(qts, base_addr + OFFSET_CTL3);
+
+    if (bank) {
+        ctl3 |= CTL3_BNK_SEL;
+    } else {
+        ctl3 &= ~CTL3_BNK_SEL;
+    }
+
+    qtest_writeb(qts, base_addr + OFFSET_CTL3, ctl3);
+}
 
 static void check_running(QTestState *qts, uint64_t base_addr)
 {
@@ -203,10 +241,33 @@  static void send_byte(QTestState *qts, uint64_t base_addr, uint8_t byte)
     qtest_writeb(qts, base_addr + OFFSET_SDA, byte);
 }
 
+static bool check_recv(QTestState *qts, uint64_t base_addr)
+{
+    uint8_t st, fif_ctl, rxf_ctl, rxf_sts;
+    bool fifo;
+
+    st = qtest_readb(qts, base_addr + OFFSET_ST);
+    choose_bank(qts, base_addr, 0);
+    fif_ctl = qtest_readb(qts, base_addr + OFFSET_FIF_CTL);
+    fifo = fif_ctl & FIF_CTL_FIFO_EN;
+    if (!fifo) {
+        return st == (ST_MODE | ST_SDAST);
+    }
+
+    choose_bank(qts, base_addr, 1);
+    rxf_ctl = qtest_readb(qts, base_addr + OFFSET_RXF_CTL);
+    rxf_sts = qtest_readb(qts, base_addr + OFFSET_RXF_STS);
+
+    if ((rxf_ctl & RXF_CTL_THR_RXIE) && RXF_STS_RX_BYTES(rxf_sts) < 16) {
+        return st == ST_MODE;
+    } else {
+        return st == (ST_MODE | ST_SDAST);
+    }
+}
+
 static uint8_t recv_byte(QTestState *qts, uint64_t base_addr)
 {
-    g_assert_cmphex(qtest_readb(qts, base_addr + OFFSET_ST), ==,
-                    ST_MODE | ST_SDAST);
+    g_assert_true(check_recv(qts, base_addr));
     return qtest_readb(qts, base_addr + OFFSET_SDA);
 }
 
@@ -229,7 +290,7 @@  static void send_address(QTestState *qts, uint64_t base_addr, uint8_t addr,
         qtest_writeb(qts, base_addr + OFFSET_ST, ST_STASTR);
         st = qtest_readb(qts, base_addr + OFFSET_ST);
         if (recv) {
-            g_assert_cmphex(st, ==, ST_MODE | ST_SDAST);
+            g_assert_true(check_recv(qts, base_addr));
         } else {
             g_assert_cmphex(st, ==, ST_MODE | ST_XMIT | ST_SDAST);
         }
@@ -251,6 +312,29 @@  static void send_nack(QTestState *qts, uint64_t base_addr)
     qtest_writeb(qts, base_addr + OFFSET_CTL1, ctl1);
 }
 
+static void start_fifo_mode(QTestState *qts, uint64_t base_addr)
+{
+    choose_bank(qts, base_addr, 0);
+    qtest_writeb(qts, base_addr + OFFSET_FIF_CTL, FIF_CTL_FIFO_EN);
+    g_assert_true(qtest_readb(qts, base_addr + OFFSET_FIF_CTL) &
+                  FIF_CTL_FIFO_EN);
+    choose_bank(qts, base_addr, 1);
+    qtest_writeb(qts, base_addr + OFFSET_FIF_CTS,
+                 FIF_CTS_CLR_FIFO | FIF_CTS_RFTE_IE);
+    g_assert_cmphex(qtest_readb(qts, base_addr + OFFSET_FIF_CTS), ==,
+                    FIF_CTS_RFTE_IE);
+    g_assert_cmphex(qtest_readb(qts, base_addr + OFFSET_TXF_STS), ==, 0);
+    g_assert_cmphex(qtest_readb(qts, base_addr + OFFSET_RXF_STS), ==, 0);
+}
+
+static void start_recv_fifo(QTestState *qts, uint64_t base_addr, uint8_t bytes)
+{
+    choose_bank(qts, base_addr, 1);
+    qtest_writeb(qts, base_addr + OFFSET_TXF_CTL, 0);
+    qtest_writeb(qts, base_addr + OFFSET_RXF_CTL,
+                 RXF_CTL_THR_RXIE | RXF_CTL_LAST | bytes);
+}
+
 /* Check the SMBus's status is set correctly when disabled. */
 static void test_disable_bus(gconstpointer data)
 {
@@ -324,6 +408,64 @@  static void test_single_mode(gconstpointer data)
     qtest_quit(qts);
 }
 
+/* Check the SMBus can send and receive bytes in FIFO mode. */
+static void test_fifo_mode(gconstpointer data)
+{
+    intptr_t index = (intptr_t)data;
+    uint64_t base_addr = SMBUS_ADDR(index);
+    int irq = SMBUS_IRQ(index);
+    uint8_t value = 0x60;
+    QTestState *qts = qtest_init("-machine npcm750-evb");
+
+    qtest_irq_intercept_in(qts, "/machine/soc/a9mpcore/gic");
+    enable_bus(qts, base_addr);
+    start_fifo_mode(qts, base_addr);
+    g_assert_false(qtest_get_irq(qts, irq));
+
+    /* Sending */
+    start_transfer(qts, base_addr);
+    send_address(qts, base_addr, EVB_DEVICE_ADDR, false, true);
+    choose_bank(qts, base_addr, 1);
+    g_assert_true(qtest_readb(qts, base_addr + OFFSET_FIF_CTS) &
+                  FIF_CTS_RXF_TXE);
+    qtest_writeb(qts, base_addr + OFFSET_TXF_CTL, TXF_CTL_THR_TXIE);
+    send_byte(qts, base_addr, TMP105_REG_CONFIG);
+    send_byte(qts, base_addr, value);
+    g_assert_true(qtest_readb(qts, base_addr + OFFSET_FIF_CTS) &
+                  FIF_CTS_RXF_TXE);
+    g_assert_true(qtest_readb(qts, base_addr + OFFSET_TXF_STS) &
+                  TXF_STS_TX_THST);
+    g_assert_cmpuint(TXF_STS_TX_BYTES(
+                        qtest_readb(qts, base_addr + OFFSET_TXF_STS)), ==, 0);
+    g_assert_true(qtest_get_irq(qts, irq));
+    stop_transfer(qts, base_addr);
+    check_stopped(qts, base_addr);
+
+    /* Receiving */
+    start_fifo_mode(qts, base_addr);
+    start_transfer(qts, base_addr);
+    send_address(qts, base_addr, EVB_DEVICE_ADDR, false, true);
+    send_byte(qts, base_addr, TMP105_REG_CONFIG);
+    start_transfer(qts, base_addr);
+    qtest_writeb(qts, base_addr + OFFSET_FIF_CTS, FIF_CTS_RXF_TXE);
+    start_recv_fifo(qts, base_addr, 1);
+    send_address(qts, base_addr, EVB_DEVICE_ADDR, true, true);
+    g_assert_false(qtest_readb(qts, base_addr + OFFSET_FIF_CTS) &
+                   FIF_CTS_RXF_TXE);
+    g_assert_true(qtest_readb(qts, base_addr + OFFSET_RXF_STS) &
+                  RXF_STS_RX_THST);
+    g_assert_cmpuint(RXF_STS_RX_BYTES(
+                        qtest_readb(qts, base_addr + OFFSET_RXF_STS)), ==, 1);
+    send_nack(qts, base_addr);
+    stop_transfer(qts, base_addr);
+    check_running(qts, base_addr);
+    g_assert_cmphex(recv_byte(qts, base_addr), ==, value);
+    g_assert_cmpuint(RXF_STS_RX_BYTES(
+                        qtest_readb(qts, base_addr + OFFSET_RXF_STS)), ==, 0);
+    check_stopped(qts, base_addr);
+    qtest_quit(qts);
+}
+
 static void smbus_add_test(const char *name, int index, GTestDataFunc fn)
 {
     g_autofree char *full_name = g_strdup_printf(
@@ -346,6 +488,7 @@  int main(int argc, char **argv)
 
     for (i = 0; i < ARRAY_SIZE(evb_bus_list); ++i) {
         add_test(single_mode, evb_bus_list[i]);
+        add_test(fifo_mode, evb_bus_list[i]);
     }
 
     return g_test_run();