From 5dcd72897edddb8bd49ce0e46f05e5face452a86 Mon Sep 17 00:00:00 2001
From: Kinza Qamar <kqzaman@lowrisc.org>
Date: Thu, 7 May 2026 21:21:42 +0100
Subject: [PATCH 01/11] [I2C, hal, SW] Restructured and refactored the
 functions

Taking the transfer checking part out from the i2c_read_byte()
and i2c_write_byte() functions. It is needed when you want to
controller wants to read / write multiple bytes.

Signed-off-by: Kinza Qamar <kqzaman@lowrisc.org>
---
 sw/device/examples/i2c.c        |  9 ++++--
 sw/device/lib/hal/i2c.c         | 52 ++++++++++++++++++---------------
 sw/device/lib/hal/i2c.h         | 13 +++++++--
 sw/device/tests/i2c/smoketest.c | 17 +++++++++--
 4 files changed, 59 insertions(+), 32 deletions(-)

diff --git a/sw/device/examples/i2c.c b/sw/device/examples/i2c.c
index 26c15ee1c..3fc824984 100644
--- a/sw/device/examples/i2c.c
+++ b/sw/device/examples/i2c.c
@@ -29,9 +29,12 @@ int main(void)
         timer_busy_sleep_us(timer, 1000u);
 
         // Read current temperature from an AS6212 I^2C-bus sensor and print the value
-        if (i2c_write_byte(i2c, 0x48u, 0u)) { // select TVAL reg; also a presence check
-            uint16_t sensor_reading = i2c_read_byte(i2c, 0x48u); // read TVAl reg
-            if (sensor_reading != 0xFF) { // only print if we get a non-error value
+        i2c_write_byte(i2c, 0x48u, 0u);
+        if (i2c_wait_write_finish(i2c)) { // select TVAL reg; also a presence check
+            i2c_read_byte(i2c, 0x48u);
+            if (i2c_wait_read_finish(i2c)) {
+                i2c_rdata rdata_reg = VOLATILE_READ(i2c->rdata);
+                uint16_t sensor_reading = rdata_reg.rdata; // read TVAl reg
                 uprintf(uart, "Temperature: 0x%x degC\n",
                         (sensor_reading << 1)); // no decimal printf
             }
diff --git a/sw/device/lib/hal/i2c.c b/sw/device/lib/hal/i2c.c
index 4069fb6aa..37df65373 100644
--- a/sw/device/lib/hal/i2c.c
+++ b/sw/device/lib/hal/i2c.c
@@ -60,7 +60,7 @@ void i2c_init(i2c_t i2c)
     VOLATILE_WRITE(i2c->timing4, t4_reg);
 }
 
-bool i2c_write_byte(i2c_t i2c, uint8_t addr, uint8_t data)
+void i2c_write_byte(i2c_t i2c, uint8_t addr, uint8_t data)
 {
     // Reset FMT FIFO (because we currently don't clean-up after errors)
     i2c_fifo_ctrl fifo_ctrl_reg = { .fmtrst = 1u };
@@ -79,24 +79,9 @@ bool i2c_write_byte(i2c_t i2c, uint8_t addr, uint8_t data)
     fdata_reg.start = 0;
     fdata_reg.stop = 1u;
     VOLATILE_WRITE(i2c->fdata, fdata_reg);
-
-    // Wait for transaction to complete and report simple succeed/fail
-    for (uint32_t ii = 0; ii < 10000000ul /*arbitrary number*/; ii++) {
-        i2c_intr i2c_intr_state_reg = VOLATILE_READ(i2c->intr_state);
-        if (i2c_intr_state_reg & i2c_intr_controller_halt) {
-            return false; // transaction failed
-        }
-        if (i2c_intr_state_reg & i2c_intr_cmd_complete) {
-            i2c_status i2c_status_reg = VOLATILE_READ(i2c->status);
-            if (i2c_status_reg & i2c_status_fmtempty) {
-                return true; // transaction succeeded
-            }
-        }
-    }
-    return false; // timeout
 }
 
-uint8_t i2c_read_byte(i2c_t i2c, uint8_t addr)
+void i2c_read_byte(i2c_t i2c, uint8_t addr)
 {
     // Reset FMT FIFO (because we currently don't clean-up after errors)
     i2c_fifo_ctrl fifo_ctrl_reg = { .fmtrst = 1u };
@@ -116,21 +101,40 @@ uint8_t i2c_read_byte(i2c_t i2c, uint8_t addr)
     fdata_reg.start = 0;
     fdata_reg.stop = 1u;
     VOLATILE_WRITE(i2c->fdata, fdata_reg);
+}
+
+bool i2c_wait_write_finish(i2c_t i2c)
+{
+    // Wait for transaction to complete and report simple succeed / fail
+    while (true) {
+        i2c_intr i2c_intr_state_reg = VOLATILE_READ(i2c->intr_state);
+        if (i2c_intr_state_reg & i2c_intr_controller_halt) {
+            return false; // Transaction failed
+        }
+        if (i2c_intr_state_reg & i2c_intr_cmd_complete) {
+            i2c_status i2c_status_reg = VOLATILE_READ(i2c->status);
+            if (i2c_status_reg & i2c_status_fmtempty) {
+                return true; // Transaction succeeded
+            }
+        }
+    }
+    return false; // Timeout
+}
 
-    // Wait for transaction to complete and return either read data or 0xFF
-    for (uint32_t ii = 0; ii < 10000000ul /*arbitrary number*/; ii++) {
+bool i2c_wait_read_finish(i2c_t i2c)
+{
+    // Wait for transaction to complete and report simple succeed / fail
+    while (true) {
         i2c_intr i2c_intr_state_reg = VOLATILE_READ(i2c->intr_state);
         if (i2c_intr_state_reg & i2c_intr_controller_halt) {
-            return 0xFF; // transaction failed
+            return false; // Transaction failed
         }
         i2c_status i2c_status_reg = VOLATILE_READ(i2c->status);
         if (i2c_status_reg & i2c_status_fmtempty) {
-            // transaction succeeded, return read data
-            i2c_rdata rdata_reg = VOLATILE_READ(i2c->rdata);
-            return rdata_reg.rdata;
+            return true;
         }
     }
-    return 0xFF; // timeout
+    return false; // Timeout
 }
 
 void enable_controller_mode(i2c_t i2c)
diff --git a/sw/device/lib/hal/i2c.h b/sw/device/lib/hal/i2c.h
index 73857c40d..1af51b06f 100644
--- a/sw/device/lib/hal/i2c.h
+++ b/sw/device/lib/hal/i2c.h
@@ -90,8 +90,17 @@
 
 void i2c_init(i2c_t i2c);
 
-bool i2c_write_byte(i2c_t i2c, uint8_t addr, uint8_t data);
-uint8_t i2c_read_byte(i2c_t i2c, uint8_t addr);
+// Transmits a single byte to the target
+void i2c_write_byte(i2c_t i2c, uint8_t addr, uint8_t data);
+
+// Receive a single byte from the target
+void i2c_read_byte(i2c_t i2c, uint8_t addr);
+
+// Wait for the read transfer to complete by checking interrupt state and status register fields
+bool i2c_wait_read_finish(i2c_t i2c);
+
+// Wait for the write transfer to complete by checking interrupt state and status register fields
+bool i2c_wait_write_finish(i2c_t i2c);
 
 // Enable I2C in controller mode
 void enable_controller_mode(i2c_t i2c);
diff --git a/sw/device/tests/i2c/smoketest.c b/sw/device/tests/i2c/smoketest.c
index 8d82e0a34..0fbac83dc 100644
--- a/sw/device/tests/i2c/smoketest.c
+++ b/sw/device/tests/i2c/smoketest.c
@@ -12,14 +12,25 @@
 static bool as6212_test(i2c_t i2c)
 {
     // Write the desired register index
-    if (!i2c_write_byte(i2c, 0x48u, 0u)) {
+    i2c_write_byte(i2c, 0x48u, 0u);
+
+    // Check if the write was successful
+    if (!i2c_wait_write_finish(i2c)) {
         return false;
     }
+
     // Read current temperature
-    uint8_t byte = i2c_read_byte(i2c, 0x48u);
-    if (byte == 0xFFu /* error value */) {
+    i2c_read_byte(i2c, 0x48u);
+
+    // Check if the read was successful
+    if (!i2c_wait_read_finish(i2c)) {
         return false;
     }
+
+    // If the read was successful, then retrieve the data from the fifo.
+    i2c_rdata rdata_reg = VOLATILE_READ(i2c->rdata);
+    uint8_t byte = rdata_reg.rdata;
+
     int16_t tval = byte; // signed, as temperature can be negative
     tval <<= 8; // first byte is the most-significant byte of two
     tval >>= 7; // convert from units of 1/128 degC to 1 degC

From 33033525a874cad5e81a1fd2f170ff245a6eb5fc Mon Sep 17 00:00:00 2001
From: Kinza Qamar <kqzaman@lowrisc.org>
Date: Fri, 8 May 2026 13:49:34 +0100
Subject: [PATCH 02/11] [I2C, SW] Place the resetting of FMT fifo step when
 halt occurs

This is suggested in programmer's guide.

As a precautionary step, reset the FMT fifo before sending any transfer

Signed-off-by: Kinza Qamar <kqzaman@lowrisc.org>
---
 sw/device/lib/hal/i2c.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/sw/device/lib/hal/i2c.c b/sw/device/lib/hal/i2c.c
index 37df65373..b4a6300f0 100644
--- a/sw/device/lib/hal/i2c.c
+++ b/sw/device/lib/hal/i2c.c
@@ -62,7 +62,8 @@ void i2c_init(i2c_t i2c)
 
 void i2c_write_byte(i2c_t i2c, uint8_t addr, uint8_t data)
 {
-    // Reset FMT FIFO (because we currently don't clean-up after errors)
+    // Reset the FMT FIFO as a precautionary step in case something goes wrong when controller's FSM
+    // is halted and the SW didn't manage to clear the FIFO during that scenario.
     i2c_fifo_ctrl fifo_ctrl_reg = { .fmtrst = 1u };
     VOLATILE_WRITE(i2c->fifo_ctrl, fifo_ctrl_reg);
 
@@ -83,7 +84,8 @@ void i2c_write_byte(i2c_t i2c, uint8_t addr, uint8_t data)
 
 void i2c_read_byte(i2c_t i2c, uint8_t addr)
 {
-    // Reset FMT FIFO (because we currently don't clean-up after errors)
+    // Reset the FMT FIFO as a precautionary step in case something goes wrong when controller's FSM
+    // is halted and the SW didn't manage to clear the FIFO during that scenario.
     i2c_fifo_ctrl fifo_ctrl_reg = { .fmtrst = 1u };
     VOLATILE_WRITE(i2c->fifo_ctrl, fifo_ctrl_reg);
 
@@ -109,6 +111,13 @@ bool i2c_wait_write_finish(i2c_t i2c)
     while (true) {
         i2c_intr i2c_intr_state_reg = VOLATILE_READ(i2c->intr_state);
         if (i2c_intr_state_reg & i2c_intr_controller_halt) {
+            // Reset FMT FIFO as controller's FSM is in halt
+            i2c_fifo_ctrl fifo_ctrl_reg = { .fmtrst = 1u };
+            VOLATILE_WRITE(i2c->fifo_ctrl, fifo_ctrl_reg);
+
+            // According to programmer's guide, the CONTROLLER_EVENTS register would be cleared
+            // here to acknowledge the controller halt interrupt. However, since we want to
+            // treat a halt event as a failure, we intentionally skip clearing it.
             return false; // Transaction failed
         }
         if (i2c_intr_state_reg & i2c_intr_cmd_complete) {
@@ -127,6 +136,13 @@ bool i2c_wait_read_finish(i2c_t i2c)
     while (true) {
         i2c_intr i2c_intr_state_reg = VOLATILE_READ(i2c->intr_state);
         if (i2c_intr_state_reg & i2c_intr_controller_halt) {
+            // Reset FMT FIFO as controller's FSM is in halt
+            i2c_fifo_ctrl fifo_ctrl_reg = { .fmtrst = 1u };
+            VOLATILE_WRITE(i2c->fifo_ctrl, fifo_ctrl_reg);
+
+            // According to programmer's guide, the CONTROLLER_EVENTS register would be cleared
+            // here to acknowledge the controller halt interrupt. However, since we want to
+            // treat a halt event as a failure, we intentionally skip clearing it.
             return false; // Transaction failed
         }
         i2c_status i2c_status_reg = VOLATILE_READ(i2c->status);

From ab608e2a7089381388593fbd9b4ecc1bd0f2c5b0 Mon Sep 17 00:00:00 2001
From: Kinza Qamar <kqzaman@lowrisc.org>
Date: Mon, 11 May 2026 21:28:30 +0100
Subject: [PATCH 03/11] [I2C, sw] Added a HAL function to return I2C RData

Signed-off-by: Kinza Qamar <kqzaman@lowrisc.org>
---
 sw/device/examples/i2c.c        | 3 +--
 sw/device/lib/hal/i2c.c         | 6 ++++++
 sw/device/lib/hal/i2c.h         | 3 +++
 sw/device/tests/i2c/smoketest.c | 3 +--
 4 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/sw/device/examples/i2c.c b/sw/device/examples/i2c.c
index 3fc824984..977edfa84 100644
--- a/sw/device/examples/i2c.c
+++ b/sw/device/examples/i2c.c
@@ -33,8 +33,7 @@ int main(void)
         if (i2c_wait_write_finish(i2c)) { // select TVAL reg; also a presence check
             i2c_read_byte(i2c, 0x48u);
             if (i2c_wait_read_finish(i2c)) {
-                i2c_rdata rdata_reg = VOLATILE_READ(i2c->rdata);
-                uint16_t sensor_reading = rdata_reg.rdata; // read TVAl reg
+                uint16_t sensor_reading = i2c_rdata_byte(i2c); // read TVAl reg
                 uprintf(uart, "Temperature: 0x%x degC\n",
                         (sensor_reading << 1)); // no decimal printf
             }
diff --git a/sw/device/lib/hal/i2c.c b/sw/device/lib/hal/i2c.c
index b4a6300f0..5a20f53c6 100644
--- a/sw/device/lib/hal/i2c.c
+++ b/sw/device/lib/hal/i2c.c
@@ -157,3 +157,9 @@ void enable_controller_mode(i2c_t i2c)
 {
     VOLATILE_WRITE(i2c->ctrl, i2c_ctrl_enablehost);
 }
+
+uint8_t i2c_rdata_byte(i2c_t i2c)
+{
+    i2c_rdata rdata_reg = VOLATILE_READ(i2c->rdata);
+    return rdata_reg.rdata;
+}
diff --git a/sw/device/lib/hal/i2c.h b/sw/device/lib/hal/i2c.h
index 1af51b06f..bf8d74162 100644
--- a/sw/device/lib/hal/i2c.h
+++ b/sw/device/lib/hal/i2c.h
@@ -104,3 +104,6 @@ bool i2c_wait_write_finish(i2c_t i2c);
 
 // Enable I2C in controller mode
 void enable_controller_mode(i2c_t i2c);
+
+// Return the data in the target's tx fifo
+uint8_t i2c_rdata_byte(i2c_t i2c);
diff --git a/sw/device/tests/i2c/smoketest.c b/sw/device/tests/i2c/smoketest.c
index 0fbac83dc..88290a90b 100644
--- a/sw/device/tests/i2c/smoketest.c
+++ b/sw/device/tests/i2c/smoketest.c
@@ -28,8 +28,7 @@ static bool as6212_test(i2c_t i2c)
     }
 
     // If the read was successful, then retrieve the data from the fifo.
-    i2c_rdata rdata_reg = VOLATILE_READ(i2c->rdata);
-    uint8_t byte = rdata_reg.rdata;
+    uint8_t byte = i2c_rdata_byte(i2c);
 
     int16_t tval = byte; // signed, as temperature can be negative
     tval <<= 8; // first byte is the most-significant byte of two

From dbf6a8d183f2bca31036fda5a27d0c70c3c04d11 Mon Sep 17 00:00:00 2001
From: Kinza Qamar <kqzaman@lowrisc.org>
Date: Fri, 8 May 2026 09:56:22 +0100
Subject: [PATCH 04/11] [I2C, SW, hal] Extended read / write functions to R/W N
 bytes

Signed-off-by: Kinza Qamar <kqzaman@lowrisc.org>
---
 sw/device/examples/i2c.c        |  5 +++--
 sw/device/lib/hal/i2c.c         | 19 ++++++++++++-------
 sw/device/lib/hal/i2c.h         |  8 ++++----
 sw/device/tests/i2c/smoketest.c |  6 ++++--
 4 files changed, 23 insertions(+), 15 deletions(-)

diff --git a/sw/device/examples/i2c.c b/sw/device/examples/i2c.c
index 977edfa84..2129db949 100644
--- a/sw/device/examples/i2c.c
+++ b/sw/device/examples/i2c.c
@@ -28,10 +28,11 @@ int main(void)
     while (true) {
         timer_busy_sleep_us(timer, 1000u);
 
+        uint8_t w_data = 0;
         // Read current temperature from an AS6212 I^2C-bus sensor and print the value
-        i2c_write_byte(i2c, 0x48u, 0u);
+        i2c_write_bytes(i2c, 0x48u, &w_data, 1);
         if (i2c_wait_write_finish(i2c)) { // select TVAL reg; also a presence check
-            i2c_read_byte(i2c, 0x48u);
+            i2c_read_bytes(i2c, 0x48u, 1);
             if (i2c_wait_read_finish(i2c)) {
                 uint16_t sensor_reading = i2c_rdata_byte(i2c); // read TVAl reg
                 uprintf(uart, "Temperature: 0x%x degC\n",
diff --git a/sw/device/lib/hal/i2c.c b/sw/device/lib/hal/i2c.c
index 5a20f53c6..cf33b2c20 100644
--- a/sw/device/lib/hal/i2c.c
+++ b/sw/device/lib/hal/i2c.c
@@ -60,7 +60,7 @@ void i2c_init(i2c_t i2c)
     VOLATILE_WRITE(i2c->timing4, t4_reg);
 }
 
-void i2c_write_byte(i2c_t i2c, uint8_t addr, uint8_t data)
+void i2c_write_bytes(i2c_t i2c, uint8_t addr, const uint8_t *data, uint8_t len_bytes)
 {
     // Reset the FMT FIFO as a precautionary step in case something goes wrong when controller's FSM
     // is halted and the SW didn't manage to clear the FIFO during that scenario.
@@ -75,14 +75,19 @@ void i2c_write_byte(i2c_t i2c, uint8_t addr, uint8_t data)
     fdata_reg.start = 1u;
     VOLATILE_WRITE(i2c->fdata, fdata_reg);
 
-    // Send stop bit and data
-    fdata_reg.fbyte = data;
     fdata_reg.start = 0;
-    fdata_reg.stop = 1u;
-    VOLATILE_WRITE(i2c->fdata, fdata_reg);
+
+    // Send all data bytes; assert STOP only on the last byte
+    for (uint8_t i = 0; i < len_bytes; i++) {
+        fdata_reg.fbyte = data[i];
+        if (i == (len_bytes - 1u)) {
+            fdata_reg.stop = 1u;
+        }
+        VOLATILE_WRITE(i2c->fdata, fdata_reg);
+    }
 }
 
-void i2c_read_byte(i2c_t i2c, uint8_t addr)
+void i2c_read_bytes(i2c_t i2c, uint8_t addr, uint8_t len_bytes)
 {
     // Reset the FMT FIFO as a precautionary step in case something goes wrong when controller's FSM
     // is halted and the SW didn't manage to clear the FIFO during that scenario.
@@ -99,7 +104,7 @@ void i2c_read_byte(i2c_t i2c, uint8_t addr)
 
     // Send stop bit, read bit and number of bytes to read
     fdata_reg.readb = 1u;
-    fdata_reg.fbyte = 1u; // If readb = 1 then fbyte contains the number of bytes to read
+    fdata_reg.fbyte = len_bytes; // If readb = 1 then fbyte contains the number of bytes to read
     fdata_reg.start = 0;
     fdata_reg.stop = 1u;
     VOLATILE_WRITE(i2c->fdata, fdata_reg);
diff --git a/sw/device/lib/hal/i2c.h b/sw/device/lib/hal/i2c.h
index bf8d74162..c46b1628c 100644
--- a/sw/device/lib/hal/i2c.h
+++ b/sw/device/lib/hal/i2c.h
@@ -90,11 +90,11 @@
 
 void i2c_init(i2c_t i2c);
 
-// Transmits a single byte to the target
-void i2c_write_byte(i2c_t i2c, uint8_t addr, uint8_t data);
+// Transmits multiple bytes to the target
+void i2c_write_bytes(i2c_t i2c, uint8_t addr, const uint8_t *data, uint8_t len_bytes);
 
-// Receive a single byte from the target
-void i2c_read_byte(i2c_t i2c, uint8_t addr);
+// Receive multiple bytes from the target
+void i2c_read_bytes(i2c_t i2c, uint8_t addr, uint8_t len_bytes);
 
 // Wait for the read transfer to complete by checking interrupt state and status register fields
 bool i2c_wait_read_finish(i2c_t i2c);
diff --git a/sw/device/tests/i2c/smoketest.c b/sw/device/tests/i2c/smoketest.c
index 88290a90b..254cb949c 100644
--- a/sw/device/tests/i2c/smoketest.c
+++ b/sw/device/tests/i2c/smoketest.c
@@ -11,8 +11,10 @@
 // Read temperature from an AS6212 sensor (default address 0x48)
 static bool as6212_test(i2c_t i2c)
 {
+    uint8_t w_data = 0;
+
     // Write the desired register index
-    i2c_write_byte(i2c, 0x48u, 0u);
+    i2c_write_bytes(i2c, 0x48u, &w_data, 1);
 
     // Check if the write was successful
     if (!i2c_wait_write_finish(i2c)) {
@@ -20,7 +22,7 @@ static bool as6212_test(i2c_t i2c)
     }
 
     // Read current temperature
-    i2c_read_byte(i2c, 0x48u);
+    i2c_read_bytes(i2c, 0x48u, 1);
 
     // Check if the read was successful
     if (!i2c_wait_read_finish(i2c)) {

From ed795b3740a022b74f9bfe79c7af7591b6cc3914 Mon Sep 17 00:00:00 2001
From: Kinza Qamar <kqzaman@lowrisc.org>
Date: Tue, 19 May 2026 12:29:21 +0100
Subject: [PATCH 05/11] [i2c, sw] Added an overflow check when the byte is
 equal to FMT FIFO depth

Signed-off-by: Kinza Qamar <kqzaman@lowrisc.org>
---
 sw/device/lib/hal/i2c.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/sw/device/lib/hal/i2c.c b/sw/device/lib/hal/i2c.c
index cf33b2c20..cf9f43563 100644
--- a/sw/device/lib/hal/i2c.c
+++ b/sw/device/lib/hal/i2c.c
@@ -77,8 +77,16 @@ void i2c_write_bytes(i2c_t i2c, uint8_t addr, const uint8_t *data, uint8_t len_b
 
     fdata_reg.start = 0;
 
-    // Send all data bytes; assert STOP only on the last byte
     for (uint8_t i = 0; i < len_bytes; i++) {
+        // Check the overflow condition before writing to the FMT FIFO.
+        i2c_status i2c_status_reg = VOLATILE_READ(i2c->status);
+
+        // Wait until FMT FIFO has some space
+        while (i2c_status_reg & i2c_status_fmtfull) {
+            i2c_status_reg = VOLATILE_READ(i2c->status);
+        }
+
+        // Send all data bytes; assert STOP only on the last byte
         fdata_reg.fbyte = data[i];
         if (i == (len_bytes - 1u)) {
             fdata_reg.stop = 1u;

From 6527cb4ab88dc611f178e30459f580ff338e966d Mon Sep 17 00:00:00 2001
From: Kinza Qamar <kqzaman@lowrisc.org>
Date: Sun, 17 May 2026 12:59:55 +0100
Subject: [PATCH 06/11] [i2c, sw, hal] Added a function to calculate SCL high
 cycles

It takes care that the resultant SCL high cycles must satisfy below
equations:
** scl_high_cycles >= 4 (to support correct function in clock
streching)
** scl_high_cycles = scl_period_cycles - (rise_cycles + fall_cycles
                     + scl_low_cycles)

Signed-off-by: Kinza Qamar <kqzaman@lowrisc.org>
---
 sw/device/lib/hal/i2c.c | 50 ++++++++++++++++++++++++++++++++---------
 1 file changed, 39 insertions(+), 11 deletions(-)

diff --git a/sw/device/lib/hal/i2c.c b/sw/device/lib/hal/i2c.c
index cf9f43563..3f6bf4dd7 100644
--- a/sw/device/lib/hal/i2c.c
+++ b/sw/device/lib/hal/i2c.c
@@ -22,6 +22,33 @@ static uint16_t rnd_up_div(uint32_t a, uint32_t b)
     return (uint16_t)result;
 }
 
+uint16_t calc_scl_high_cycles(uint16_t rise_cycles, uint16_t fall_cycles,
+                              uint16_t scl_period_cycles, uint16_t scl_low_cycles)
+{
+    // Calculate the minimum allowable value for SCL high time
+    uint16_t scl_high_cycles_min = rnd_up_div(4000, SYSCLK_NS);
+
+    // scl_high_time should be atleast 4 cycles to aid correct clock streching
+    scl_high_cycles_min = (scl_high_cycles_min < 4u) ? 4u : scl_high_cycles_min;
+
+    // An SCL period duration is divided into 4 segments:
+    // 1) Rise time
+    // 2) Fall time
+    // 3) High time
+    // 4) Low time
+    // Hence an SCL period must satisfy the equation below:
+    // scl_period = rise_time + fall_time + high_time + low_time
+    //
+    // Even though SCL_low_cycles and SCL_high_cycles have minimum allowable values, increase in
+    // rise time and fall time influences the SCL_period.
+    uint16_t scl_high_cycles = scl_period_cycles - (scl_low_cycles + rise_cycles + fall_cycles);
+
+    scl_high_cycles =
+        (scl_high_cycles > scl_high_cycles_min) ? scl_high_cycles : scl_high_cycles_min;
+
+    return scl_high_cycles;
+}
+
 void i2c_init(i2c_t i2c)
 {
     // -- Set timing parameters --
@@ -34,17 +61,18 @@ void i2c_init(i2c_t i2c)
     // SCL high cycles calculation adapted from OpenTitan sw/device/lib/dif/dif_i2c.c
 
     // Calculate the timing paramters
-    uint32_t rise_cycles = rnd_up_div(I2C_RISE_NS, SYSCLK_NS);
-    uint32_t fall_cycles = rnd_up_div(I2C_FALL_NS, SYSCLK_NS);
-    uint32_t scl_period_cycles = rnd_up_div((10000 /* 10000 ns -> 100 kHz */), SYSCLK_NS);
-    uint32_t scl_low_cycles = rnd_up_div(4700, SYSCLK_NS);
-    uint32_t scl_high_cycles = scl_period_cycles - scl_low_cycles - rise_cycles - fall_cycles;
-    uint32_t setup_start_cycles = rnd_up_div(4700, SYSCLK_NS);
-    uint32_t hold_start_cycles = rnd_up_div(4000, SYSCLK_NS);
-    uint32_t setup_data_cycles = rnd_up_div(250, SYSCLK_NS);
-    uint32_t hold_data_cycles = 1u;
-    uint32_t setup_stop_cycles = rnd_up_div(4000, SYSCLK_NS);
-    uint32_t bus_free_time_cycles = rnd_up_div(4700, SYSCLK_NS);
+    uint16_t rise_cycles = rnd_up_div(I2C_RISE_NS, SYSCLK_NS);
+    uint16_t fall_cycles = rnd_up_div(I2C_FALL_NS, SYSCLK_NS);
+    uint16_t scl_period_cycles = rnd_up_div((10000 /* 10000 ns -> 100 kHz */), SYSCLK_NS);
+    uint16_t scl_low_cycles = rnd_up_div(4700, SYSCLK_NS);
+    uint16_t scl_high_cycles =
+        calc_scl_high_cycles(rise_cycles, fall_cycles, scl_period_cycles, scl_low_cycles);
+    uint16_t setup_start_cycles = rnd_up_div(4700, SYSCLK_NS);
+    uint16_t hold_start_cycles = rnd_up_div(4000, SYSCLK_NS);
+    uint16_t setup_data_cycles = rnd_up_div(250, SYSCLK_NS);
+    uint16_t hold_data_cycles = 1u;
+    uint16_t setup_stop_cycles = rnd_up_div(4000, SYSCLK_NS);
+    uint16_t bus_free_time_cycles = rnd_up_div(4700, SYSCLK_NS);
 
     // Declare timing registers
     i2c_timing0 t0_reg = { .tlow = scl_low_cycles, .thigh = scl_high_cycles };

From 8539f1dc598216da42091d2a35804bea7bf694cf Mon Sep 17 00:00:00 2001
From: Kinza Qamar <kqzaman@lowrisc.org>
Date: Tue, 19 May 2026 15:23:01 +0100
Subject: [PATCH 07/11] [i2c, sw] Function added to calculate timing params
 based on speed

OT's I2C block supports Standard, Fast and Fast Plus modes.

Signed-off-by: Kinza Qamar <kqzaman@lowrisc.org>
---
 sw/device/examples/i2c.c        |   2 +-
 sw/device/lib/hal/i2c.c         | 109 ++++++++++++++++++++++----------
 sw/device/lib/hal/i2c.h         |  24 ++++++-
 sw/device/tests/i2c/smoketest.c |   2 +-
 4 files changed, 101 insertions(+), 36 deletions(-)

diff --git a/sw/device/examples/i2c.c b/sw/device/examples/i2c.c
index 2129db949..b112d9a77 100644
--- a/sw/device/examples/i2c.c
+++ b/sw/device/examples/i2c.c
@@ -15,7 +15,7 @@ int main(void)
     i2c_t i2c = mocha_system_i2c();
     uart_t uart = mocha_system_uart();
     timer_t timer = mocha_system_timer();
-    i2c_init(i2c);
+    i2c_init(i2c, i2c_speed_mode_standard);
     uart_init(uart);
     timer_init(timer);
     timer_enable_write(timer, true);
diff --git a/sw/device/lib/hal/i2c.c b/sw/device/lib/hal/i2c.c
index 3f6bf4dd7..cf0a76ae8 100644
--- a/sw/device/lib/hal/i2c.c
+++ b/sw/device/lib/hal/i2c.c
@@ -23,11 +23,9 @@ static uint16_t rnd_up_div(uint32_t a, uint32_t b)
 }
 
 uint16_t calc_scl_high_cycles(uint16_t rise_cycles, uint16_t fall_cycles,
-                              uint16_t scl_period_cycles, uint16_t scl_low_cycles)
+                              uint16_t scl_period_cycles, uint16_t scl_low_cycles,
+                              uint16_t scl_high_cycles_min)
 {
-    // Calculate the minimum allowable value for SCL high time
-    uint16_t scl_high_cycles_min = rnd_up_div(4000, SYSCLK_NS);
-
     // scl_high_time should be atleast 4 cycles to aid correct clock streching
     scl_high_cycles_min = (scl_high_cycles_min < 4u) ? 4u : scl_high_cycles_min;
 
@@ -40,7 +38,7 @@ uint16_t calc_scl_high_cycles(uint16_t rise_cycles, uint16_t fall_cycles,
     // scl_period = rise_time + fall_time + high_time + low_time
     //
     // Even though SCL_low_cycles and SCL_high_cycles have minimum allowable values, increase in
-    // rise time and fall time influences the SCL_period.
+    // rise time and fall times influence the SCL_period.
     uint16_t scl_high_cycles = scl_period_cycles - (scl_low_cycles + rise_cycles + fall_cycles);
 
     scl_high_cycles =
@@ -49,37 +47,82 @@ uint16_t calc_scl_high_cycles(uint16_t rise_cycles, uint16_t fall_cycles,
     return scl_high_cycles;
 }
 
-void i2c_init(i2c_t i2c)
+// Calculate the minimum allowable value for each timing parameter taken from the NXP I^2C
+// specification "UM10204" Table 10 (rev. 6) / Table 11 (rev. 7).
+//
+// The values for Rise and Fall times for Fast mode are taken as spec minimum. For Fast plus mode,
+// the values are taken from OT's i2c_host_tx_rx_test.c test.
+i2c_timing_params_cycles_t compute_minimum_timing_paramaters(i2c_speed_mode_t speed)
 {
-    // -- Set timing parameters --
-    //
-    // Using Standard-mode (100 kbits/s) constants, taken from the NXP I^2C specification
-    // "UM10204" Table 10 (rev. 6) / Table 11 (rev. 7).
-    // Faster modes will require different/adjustable constants and checking that SCL high/low
-    // cycles calculated are sufficient to still allow the clock stretching logic to function.
-    //
-    // SCL high cycles calculation adapted from OpenTitan sw/device/lib/dif/dif_i2c.c
-
-    // Calculate the timing paramters
-    uint16_t rise_cycles = rnd_up_div(I2C_RISE_NS, SYSCLK_NS);
-    uint16_t fall_cycles = rnd_up_div(I2C_FALL_NS, SYSCLK_NS);
-    uint16_t scl_period_cycles = rnd_up_div((10000 /* 10000 ns -> 100 kHz */), SYSCLK_NS);
-    uint16_t scl_low_cycles = rnd_up_div(4700, SYSCLK_NS);
-    uint16_t scl_high_cycles =
-        calc_scl_high_cycles(rise_cycles, fall_cycles, scl_period_cycles, scl_low_cycles);
-    uint16_t setup_start_cycles = rnd_up_div(4700, SYSCLK_NS);
-    uint16_t hold_start_cycles = rnd_up_div(4000, SYSCLK_NS);
-    uint16_t setup_data_cycles = rnd_up_div(250, SYSCLK_NS);
-    uint16_t hold_data_cycles = 1u;
-    uint16_t setup_stop_cycles = rnd_up_div(4000, SYSCLK_NS);
-    uint16_t bus_free_time_cycles = rnd_up_div(4700, SYSCLK_NS);
+    switch (speed) {
+    case i2c_speed_mode_standard:
+        return (i2c_timing_params_cycles_t){
+            .rise_cycles = rnd_up_div(I2C_RISE_NS, SYSCLK_NS),
+            .fall_cycles = rnd_up_div(I2C_FALL_NS, SYSCLK_NS),
+            .scl_low_cycles = rnd_up_div(4700, SYSCLK_NS),
+            .scl_high_cycles = rnd_up_div(4000, SYSCLK_NS),
+            .scl_period_cycles = rnd_up_div(10000u, SYSCLK_NS),
+            .setup_start_cycles = rnd_up_div(4700u, SYSCLK_NS),
+            .hold_start_cycles = rnd_up_div(4000u, SYSCLK_NS),
+            .setup_data_cycles = rnd_up_div(250u, SYSCLK_NS),
+            .hold_data_cycles = 1u,
+            .setup_stop_cycles = rnd_up_div(4000u, SYSCLK_NS),
+            .bus_free_time_cycles = rnd_up_div(4700u, SYSCLK_NS)
+        };
+    case i2c_speed_mode_fast:
+        return (i2c_timing_params_cycles_t){
+            .rise_cycles = rnd_up_div(20u, SYSCLK_NS),
+            .fall_cycles = rnd_up_div(20u, SYSCLK_NS),
+            .scl_low_cycles = rnd_up_div(1300u, SYSCLK_NS),
+            .scl_high_cycles = rnd_up_div(600, SYSCLK_NS),
+            .scl_period_cycles = rnd_up_div(2500u, SYSCLK_NS),
+            .setup_start_cycles = rnd_up_div(600u, SYSCLK_NS),
+            .hold_start_cycles = rnd_up_div(600u, SYSCLK_NS),
+            .setup_data_cycles = rnd_up_div(100u, SYSCLK_NS),
+            .hold_data_cycles = 1u,
+            .setup_stop_cycles = rnd_up_div(600u, SYSCLK_NS),
+            .bus_free_time_cycles = rnd_up_div(1300u, SYSCLK_NS)
+        };
+    case i2c_speed_mode_fast_plus:
+        return (i2c_timing_params_cycles_t){
+            .rise_cycles = rnd_up_div(10u, SYSCLK_NS),
+            .fall_cycles = rnd_up_div(10u, SYSCLK_NS),
+            .scl_low_cycles = rnd_up_div(500u, SYSCLK_NS),
+            .scl_high_cycles = rnd_up_div(260, SYSCLK_NS),
+            .scl_period_cycles = rnd_up_div(1000u, SYSCLK_NS),
+            .setup_start_cycles = rnd_up_div(260u, SYSCLK_NS),
+            .hold_start_cycles = rnd_up_div(260u, SYSCLK_NS),
+            .setup_data_cycles = rnd_up_div(50, SYSCLK_NS),
+            .hold_data_cycles = 1u,
+            .setup_stop_cycles = rnd_up_div(260u, SYSCLK_NS),
+            .bus_free_time_cycles = rnd_up_div(500u, SYSCLK_NS)
+        };
+    default:
+        return (i2c_timing_params_cycles_t){ 0 };
+    }
+}
+
+void i2c_init(i2c_t i2c, i2c_speed_mode_t speed_mode)
+{
+    i2c_timing_params_cycles_t timing_params_cycles = compute_minimum_timing_paramaters(speed_mode);
+
+    timing_params_cycles.scl_high_cycles =
+        calc_scl_high_cycles(timing_params_cycles.rise_cycles, timing_params_cycles.fall_cycles,
+                             timing_params_cycles.scl_period_cycles,
+                             timing_params_cycles.scl_low_cycles,
+                             timing_params_cycles.scl_high_cycles);
 
     // Declare timing registers
-    i2c_timing0 t0_reg = { .tlow = scl_low_cycles, .thigh = scl_high_cycles };
-    i2c_timing1 t1_reg = { .t_r = rise_cycles, .t_f = fall_cycles };
-    i2c_timing2 t2_reg = { .tsu_sta = setup_start_cycles, .thd_sta = hold_start_cycles };
-    i2c_timing3 t3_reg = { .tsu_dat = setup_data_cycles, .thd_dat = hold_data_cycles };
-    i2c_timing4 t4_reg = { .tsu_sto = setup_stop_cycles, .t_buf = bus_free_time_cycles };
+    i2c_timing0 t0_reg = { .tlow = timing_params_cycles.scl_low_cycles,
+                           .thigh = timing_params_cycles.scl_high_cycles };
+    i2c_timing1 t1_reg = { .t_r = timing_params_cycles.rise_cycles,
+                           .t_f = timing_params_cycles.fall_cycles };
+    i2c_timing2 t2_reg = { .tsu_sta = timing_params_cycles.setup_start_cycles,
+                           .thd_sta = timing_params_cycles.hold_start_cycles };
+    i2c_timing3 t3_reg = { .tsu_dat = timing_params_cycles.setup_data_cycles,
+                           .thd_dat = timing_params_cycles.hold_data_cycles };
+    i2c_timing4 t4_reg = { .tsu_sto = timing_params_cycles.setup_stop_cycles,
+                           .t_buf = timing_params_cycles.bus_free_time_cycles };
 
     VOLATILE_WRITE(i2c->timing0, t0_reg);
     VOLATILE_WRITE(i2c->timing1, t1_reg);
diff --git a/sw/device/lib/hal/i2c.h b/sw/device/lib/hal/i2c.h
index c46b1628c..8d7283b2a 100644
--- a/sw/device/lib/hal/i2c.h
+++ b/sw/device/lib/hal/i2c.h
@@ -88,7 +88,29 @@
 #define I2C_RISE_NS (450)
 #define I2C_FALL_NS (120)
 
-void i2c_init(i2c_t i2c);
+// All the speed modes supported by OT's I2C block
+typedef enum {
+    i2c_speed_mode_standard,
+    i2c_speed_mode_fast,
+    i2c_speed_mode_fast_plus
+} i2c_speed_mode_t;
+
+// All the timing parameters used by I2C block converted into cycles
+typedef struct {
+    uint16_t rise_cycles;
+    uint16_t fall_cycles;
+    uint16_t scl_low_cycles;
+    uint16_t scl_high_cycles;
+    uint16_t scl_period_cycles;
+    uint16_t setup_start_cycles;
+    uint16_t hold_start_cycles;
+    uint16_t setup_data_cycles;
+    uint16_t hold_data_cycles;
+    uint16_t setup_stop_cycles;
+    uint16_t bus_free_time_cycles;
+} i2c_timing_params_cycles_t;
+
+void i2c_init(i2c_t i2c, i2c_speed_mode_t speed_mode);
 
 // Transmits multiple bytes to the target
 void i2c_write_bytes(i2c_t i2c, uint8_t addr, const uint8_t *data, uint8_t len_bytes);
diff --git a/sw/device/tests/i2c/smoketest.c b/sw/device/tests/i2c/smoketest.c
index 254cb949c..bc2719a56 100644
--- a/sw/device/tests/i2c/smoketest.c
+++ b/sw/device/tests/i2c/smoketest.c
@@ -41,7 +41,7 @@ static bool as6212_test(i2c_t i2c)
 bool test_main()
 {
     i2c_t i2c = mocha_system_i2c();
-    i2c_init(i2c);
+    i2c_init(i2c, i2c_speed_mode_standard);
 
     // -- Configure IP for Controller mode --
     enable_controller_mode(i2c);

From c58a9e9ef66e0fab5276eddf8ded295e3ac23597 Mon Sep 17 00:00:00 2001
From: Kinza Qamar <kqzaman@lowrisc.org>
Date: Tue, 28 Apr 2026 14:40:13 +0100
Subject: [PATCH 08/11] [chip, I2C, DV] Top level wiring of I2C ports with the
 interface

Signed-off-by: Kinza Qamar <kqzaman@lowrisc.org>
---
 hw/top_chip/dv/env/top_chip_dv_env.core |  1 +
 hw/top_chip/dv/tb/tb.sv                 | 23 +++++++++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/hw/top_chip/dv/env/top_chip_dv_env.core b/hw/top_chip/dv/env/top_chip_dv_env.core
index 7083a2a41..463ab0ccd 100644
--- a/hw/top_chip/dv/env/top_chip_dv_env.core
+++ b/hw/top_chip/dv/env/top_chip_dv_env.core
@@ -13,6 +13,7 @@ filesets:
       - lowrisc:dv:uart_agent
       - lowrisc:dv:common_ifs
       - lowrisc:mocha_dv:gpio_env
+      - lowrisc:dv:i2c_env
     files:
       - top_chip_dv_env_pkg.sv
       - mem_clear_util.sv: {is_include_file: true}
diff --git a/hw/top_chip/dv/tb/tb.sv b/hw/top_chip/dv/tb/tb.sv
index 07b2da65f..a1b5a7e6e 100644
--- a/hw/top_chip/dv/tb/tb.sv
+++ b/hw/top_chip/dv/tb/tb.sv
@@ -38,10 +38,22 @@ module tb;
   logic                                      rng_valid;
   logic [top_pkg::EntropySrcRngBusWidth-1:0] rng_bits;
 
+  // I2C connections
+  wire  scl;
+  wire  sda;
+  logic scl_en_o;
+  logic sda_en_o;
+  logic scl_o;
+  logic sda_o;
+
   // ------ Interfaces ------
   clk_rst_if sys_clk_if(.clk(clk), .rst_n(rst_n));
   uart_if uart_if();
   pins_if #(NUM_GPIOS) gpio_pins_if (.pins(gpio_pads));
+  i2c_if i2c_if (.clk_i(clk),
+                 .rst_ni(rst_n),
+                 .scl_io(scl),
+                 .sda_io(sda));
 
   // ------ Mock DRAM ------
   top_pkg::axi_dram_req_t  dram_req;
@@ -82,6 +94,13 @@ module tb;
     // UART receive and transmit.
     .uart_rx_i            (uart_if.uart_rx  ),
     .uart_tx_o            (uart_if.uart_tx  ),
+    // I2C controller/target bidirectional interface.
+    .i2c_scl_i            (scl              ),
+    .i2c_scl_o            (scl_o            ),
+    .i2c_scl_en_o         (scl_en_o         ),
+    .i2c_sda_i            (sda              ),
+    .i2c_sda_o            (sda_o            ),
+    .i2c_sda_en_o         (sda_en_o         ),
     // External Mailbox port
     .axi_mailbox_req_i    ('0               ),
     .axi_mailbox_resp_o   (                 ),
@@ -126,6 +145,10 @@ module tb;
     assign gpio_pads[i] = dut_gpio_en_o[i] ? dut_gpio_o[i] : 1'bz;
   end
 
+  // Modelling the open-drain circuit
+  assign (strong0, weak1) scl = (scl_en_o) ? scl_o : 1'b1;
+  assign (strong0, weak1) sda = (sda_en_o) ? sda_o : 1'b1;
+
   // Signals to connect the sink
   logic               sim_sram_clk;
   logic               sim_sram_rst;

From 7f62c886d3e9686782696e27172874905509e868 Mon Sep 17 00:00:00 2001
From: Kinza Qamar <kqzaman@lowrisc.org>
Date: Tue, 28 Apr 2026 17:34:39 +0100
Subject: [PATCH 09/11] [chip, I2C, DV] Wired up I2C agent in the chip
 environment

Signed-off-by: Kinza Qamar <kqzaman@lowrisc.org>
---
 hw/top_chip/dv/env/top_chip_dv_env.sv     | 7 +++++++
 hw/top_chip/dv/env/top_chip_dv_env_cfg.sv | 4 ++++
 hw/top_chip/dv/tb/tb.sv                   | 1 +
 3 files changed, 12 insertions(+)

diff --git a/hw/top_chip/dv/env/top_chip_dv_env.sv b/hw/top_chip/dv/env/top_chip_dv_env.sv
index b8896f4d1..2ef085242 100644
--- a/hw/top_chip/dv/env/top_chip_dv_env.sv
+++ b/hw/top_chip/dv/env/top_chip_dv_env.sv
@@ -13,6 +13,7 @@ class top_chip_dv_env extends uvm_env;
 
   // Agents
   uart_agent m_uart_agent;
+  i2c_agent  m_i2c_agent;
 
   // Standard SV/UVM methods
   extern function new(string name = "", uvm_component parent = null);
@@ -67,6 +68,12 @@ function void top_chip_dv_env::build_phase(uvm_phase phase);
     `uvm_fatal(`gfn, "Cannot get sys_clk_vif")
   end
 
+  // Set I2C agent config object for I2C agent
+  uvm_config_db#(i2c_agent_cfg)::set(this, "m_i2c_agent", "cfg", cfg.m_i2c_agent_cfg);
+
+  // Create I2C agent
+  m_i2c_agent = i2c_agent::type_id::create("m_i2c_agent", this);
+
   // Instantiate UART agent
   m_uart_agent = uart_agent::type_id::create("m_uart_agent", this);
   uvm_config_db#(uart_agent_cfg)::set(this, "m_uart_agent*", "cfg", cfg.m_uart_agent_cfg);
diff --git a/hw/top_chip/dv/env/top_chip_dv_env_cfg.sv b/hw/top_chip/dv/env/top_chip_dv_env_cfg.sv
index c3aef365d..f76046165 100644
--- a/hw/top_chip/dv/env/top_chip_dv_env_cfg.sv
+++ b/hw/top_chip/dv/env/top_chip_dv_env_cfg.sv
@@ -18,6 +18,7 @@ class top_chip_dv_env_cfg extends uvm_object;
 
   // External interface agent configs
   rand uart_agent_cfg    m_uart_agent_cfg;
+  i2c_agent_cfg          m_i2c_agent_cfg;
 
   `uvm_object_utils_begin(top_chip_dv_env_cfg)
   `uvm_object_utils_end
@@ -45,6 +46,9 @@ function void top_chip_dv_env_cfg::initialize();
   // Configuration is required to perform meaningful monitoring
   m_uart_agent_cfg.en_tx_monitor = 0;
   m_uart_agent_cfg.en_rx_monitor = 0;
+
+  // Create I2C agent config object
+  m_i2c_agent_cfg = i2c_agent_cfg::type_id::create("m_i2c_agent_cfg");
 endfunction : initialize
 
 function void top_chip_dv_env_cfg::get_mem_image_files_from_plusargs();
diff --git a/hw/top_chip/dv/tb/tb.sv b/hw/top_chip/dv/tb/tb.sv
index a1b5a7e6e..ac77de8bd 100644
--- a/hw/top_chip/dv/tb/tb.sv
+++ b/hw/top_chip/dv/tb/tb.sv
@@ -283,6 +283,7 @@ module tb;
     uvm_config_db#(virtual clk_rst_if)::set(null, "*", "sys_clk_if", sys_clk_if);
     uvm_config_db#(virtual uart_if)::set(null, "*.env.m_uart_agent*", "vif", uart_if);
     uvm_config_db#(virtual pins_if #(NUM_GPIOS))::set(null, "*.env", "gpio_vif", gpio_pins_if);
+    uvm_config_db#(virtual i2c_if)::set(null, "*.env.m_i2c_agent", "vif", i2c_if);
 
     // SW logger and test status interfaces.
     uvm_config_db#(virtual sw_test_status_if)::set(

From cae654ed7070452189863de62ebb75bfddeaf919 Mon Sep 17 00:00:00 2001
From: Kinza Qamar <kqzaman@lowrisc.org>
Date: Thu, 30 Apr 2026 11:53:29 +0100
Subject: [PATCH 10/11] [chip, I2C, SW] Added I2C host TX_RX test

Controller writes multiple bytes to the target's receiver and
then reads multiple bytes from the same address. At the end
of both read and write transfer, the test checks if the
transfer was succesful. If yes, then the test compares all
the read bytes with the data bytes that was sent as part of
write transfer.

Signed-off-by: Kinza Qamar <kqzaman@lowrisc.org>
---
 sw/device/tests/CMakeLists.txt  |  1 +
 sw/device/tests/i2c/host_test.c | 68 +++++++++++++++++++++++++++++++++
 2 files changed, 69 insertions(+)
 create mode 100644 sw/device/tests/i2c/host_test.c

diff --git a/sw/device/tests/CMakeLists.txt b/sw/device/tests/CMakeLists.txt
index e582c768f..22afc3e9b 100644
--- a/sw/device/tests/CMakeLists.txt
+++ b/sw/device/tests/CMakeLists.txt
@@ -17,6 +17,7 @@ mocha_add_test(NAME gpio_reg_access_test SOURCES gpio/reg_access_test.c LIBRARIE
 # driven externally
 mocha_add_test(NAME gpio_smoketest SOURCES gpio/smoketest.c LIBRARIES ${LIBS} SKIP_VERILATOR SKIP_FPGA)
 mocha_add_test(NAME i2c_smoketest SOURCES i2c/smoketest.c LIBRARIES ${LIBS})
+mocha_add_test(NAME i2c_host_tx_rx_test SOURCES i2c/host_test.c LIBRARIES ${LIBS})
 mocha_add_test(NAME mailbox_smoketest SOURCES mailbox/smoketest.c LIBRARIES ${LIBS})
 mocha_add_test(NAME plic_smoketest SOURCES plic/smoketest.c LIBRARIES ${LIBS})
 mocha_add_test(NAME pwrmgr_smoketest SOURCES pwrmgr/smoketest.c LIBRARIES ${LIBS})
diff --git a/sw/device/tests/i2c/host_test.c b/sw/device/tests/i2c/host_test.c
new file mode 100644
index 000000000..abdc1d7f7
--- /dev/null
+++ b/sw/device/tests/i2c/host_test.c
@@ -0,0 +1,68 @@
+// Copyright lowRISC contributors (COSMIC project).
+// Licensed under the Apache License, Version 2.0, see LICENSE for details.
+// SPDX-License-Identifier: Apache-2.0
+
+#include "hal/i2c.h"
+#include "hal/mmio.h"
+#include "hal/mocha.h"
+#include <stdbool.h>
+#include <stdint.h>
+
+bool i2c_host_wr_xfer(i2c_t i2c, uint8_t addr, const uint8_t *data, uint8_t num_wr_bytes)
+{
+    // Start a write transfer
+    i2c_write_bytes(i2c, addr, data, num_wr_bytes);
+    return i2c_wait_write_finish(i2c);
+}
+
+bool i2c_host_rd_xfer(i2c_t i2c, uint8_t addr, uint8_t num_rd_bytes)
+{
+    // Start a read transfer
+    i2c_read_bytes(i2c, addr, num_rd_bytes);
+    return i2c_wait_read_finish(i2c);
+}
+
+bool host_rx_tx_test(i2c_t i2c, uint8_t addr, const uint8_t *data, uint8_t num_bytes)
+{
+    bool rd_xfer_status, wr_xfer_status;
+
+    wr_xfer_status = i2c_host_wr_xfer(i2c, addr, data, num_bytes);
+    rd_xfer_status = i2c_host_rd_xfer(i2c, addr, num_bytes);
+
+    return (wr_xfer_status && rd_xfer_status);
+}
+
+bool test_main()
+{
+    // Data bytes to send to the target's receiver. Note that if data bytes are greater than 64 then
+    // the loop back test won't work as the FIFO's depth is 64 so the check (wr_data_bytes[i] !=
+    // rd_data_bytes[i]) at the end of the function will fail.
+    uint8_t wr_data_bytes[8];
+
+    // Return status of the test
+    bool success = false;
+
+    i2c_t i2c = mocha_system_i2c();
+    i2c_init(i2c, i2c_speed_mode_standard);
+
+    // -- Configure IP for Controller mode --
+    enable_controller_mode(i2c);
+
+    // Write walking 1's pattern
+    for (uint8_t i = 0; i < (uint8_t)(sizeof(wr_data_bytes)); i++) {
+        wr_data_bytes[i] = 1u << i;
+    }
+
+    if (host_rx_tx_test(i2c, 0x48u, wr_data_bytes, sizeof(wr_data_bytes))) {
+        uint8_t rd_data_bytes[sizeof(wr_data_bytes)];
+        for (uint8_t i = 0; i < (uint8_t)(sizeof(rd_data_bytes)); i++) {
+            rd_data_bytes[i] = i2c_rdata_byte(i2c);
+            if (wr_data_bytes[i] != rd_data_bytes[i]) {
+                return false;
+            }
+        }
+        success = true;
+    }
+
+    return success;
+}

From f5bc214e244941d30d7ce7ec6b4d05577eb773d9 Mon Sep 17 00:00:00 2001
From: Kinza Qamar <kqzaman@lowrisc.org>
Date: Thu, 30 Apr 2026 12:48:17 +0100
Subject: [PATCH 11/11] [chip, I2C, DV] Added Host TX-RX vseq

This vseq:
** reads the SW symbols to get the timing
   values
** calculates relevant timing parameter use by
   the agent to send Ack, Nack and Rdata.
** start the reactive sequence

Signed-off-by: Kinza Qamar <kqzaman@lowrisc.org>
---
 .../dv/env/seq_lib/top_chip_dv_base_vseq.sv   |  2 +-
 .../top_chip_dv_i2c_host_tx_rx_vseq.sv        | 84 +++++++++++++++++++
 .../env/seq_lib/top_chip_dv_i2c_tx_rx_vseq.sv | 66 +++++++++++++++
 .../dv/env/seq_lib/top_chip_dv_vseq_list.sv   |  2 +
 hw/top_chip/dv/env/top_chip_dv_env.core       |  2 +
 hw/top_chip/dv/env/top_chip_dv_env.sv         |  1 +
 .../dv/env/top_chip_dv_virtual_sequencer.sv   |  1 +
 hw/top_chip/dv/top_chip_sim_cfg.hjson         |  9 +-
 sw/device/lib/hal/i2c.c                       |  9 ++
 9 files changed, 174 insertions(+), 2 deletions(-)
 create mode 100644 hw/top_chip/dv/env/seq_lib/top_chip_dv_i2c_host_tx_rx_vseq.sv
 create mode 100644 hw/top_chip/dv/env/seq_lib/top_chip_dv_i2c_tx_rx_vseq.sv

diff --git a/hw/top_chip/dv/env/seq_lib/top_chip_dv_base_vseq.sv b/hw/top_chip/dv/env/seq_lib/top_chip_dv_base_vseq.sv
index 2ba9607a4..74c1bda3b 100644
--- a/hw/top_chip/dv/env/seq_lib/top_chip_dv_base_vseq.sv
+++ b/hw/top_chip/dv/env/seq_lib/top_chip_dv_base_vseq.sv
@@ -22,7 +22,7 @@ class top_chip_dv_base_vseq extends uvm_sequence;
 
   // Class specific methods
   extern function void set_handles();
-  extern task dut_init(string reset_kind = "HARD");
+  extern virtual task dut_init(string reset_kind = "HARD");
   extern task apply_reset(string kind = "HARD");
   extern task wait_for_sw_test_done();
   // Backdoor-read or override a const symbol in SW to modify the behavior of the test.
diff --git a/hw/top_chip/dv/env/seq_lib/top_chip_dv_i2c_host_tx_rx_vseq.sv b/hw/top_chip/dv/env/seq_lib/top_chip_dv_i2c_host_tx_rx_vseq.sv
new file mode 100644
index 000000000..5411d2b92
--- /dev/null
+++ b/hw/top_chip/dv/env/seq_lib/top_chip_dv_i2c_host_tx_rx_vseq.sv
@@ -0,0 +1,84 @@
+// Copyright lowRISC contributors (COSMIC project).
+// Licensed under the Apache License, Version 2.0, see LICENSE for details.
+// SPDX-License-Identifier: Apache-2.0
+
+// This vseq is going to be starting a reactive sequence.
+//
+// i2c_monitor shares an analysis port with i2c_sequencer. It sends an i2c_item which contains
+// a member "state". i2c_monitor watches the i2c_if and as soon as it sees the communication started
+// on the bus, it change the state accordingly. Based on the state received on the
+// analysis port of sequencer, i2c_base_seq initializes the start_item method to send i2c_item to
+// i2c_driver so that it can drive ack, nack or rdata to the controller.
+class top_chip_dv_i2c_host_tx_rx_vseq extends top_chip_dv_i2c_tx_rx_vseq;
+  `uvm_object_utils(top_chip_dv_i2c_host_tx_rx_vseq)
+
+  // The timing parameter in cycles used by the agent to add relevant delays before driving Ack,
+  // Nack and Rdata
+  local bit [15:0] scl_period_cycles;
+  local bit [15:0] scl_low_cycles;
+  local bit [15:0] sda_hold_cycles;
+
+  // Standard SV/UVM methods
+  extern function new(string name="");
+  extern task body();
+  extern virtual task dut_init(string reset_kind = "HARD");
+  extern local function void configure_agent_timing();
+endclass : top_chip_dv_i2c_host_tx_rx_vseq
+
+function top_chip_dv_i2c_host_tx_rx_vseq::new(string name = "");
+  super.new(name);
+endfunction : new
+
+task top_chip_dv_i2c_host_tx_rx_vseq::dut_init(string reset_kind = "HARD");
+  // Read the timing parameters through SW backdoor load
+  sw_symbol_backdoor_read("SysClkPeriodNS", sw_sys_clk_period_ns);
+  sw_symbol_backdoor_read("SCLLowTimeNs", sw_scl_low_time_ns);
+  sw_symbol_backdoor_read("HoldDataTimeNs", sw_data_hold_time_ns);
+
+  scl_period_cycles = round_up_divide({sw_scl_clk_period_ns[1], sw_scl_clk_period_ns[0]},
+                                      sw_sys_clk_period_ns[0]);
+  scl_low_cycles    = round_up_divide({sw_scl_low_time_ns[1], sw_scl_low_time_ns[0]},
+                                      sw_sys_clk_period_ns[0]);
+  sda_hold_cycles   = round_up_divide({sw_data_hold_time_ns[1], sw_data_hold_time_ns[0]},
+                                      sw_sys_clk_period_ns[0]);
+
+  super.dut_init(reset_kind);
+endtask
+
+function void top_chip_dv_i2c_host_tx_rx_vseq::configure_agent_timing();
+  // tSetupBit are going to be the cycles before SCL goes high to drive SDA. Agent should drive
+  // atleast two cycles before SCL goes high.
+  int unsigned tSetupBit = 2;
+  cfg.m_i2c_agent_cfg.timing_cfg.tSetupBit = tSetupBit;
+
+  // tHoldBit are the cycles to hold SDA after SCL goes low.
+  cfg.m_i2c_agent_cfg.timing_cfg.tHoldBit = sda_hold_cycles;
+
+  // Used by i2c_if to stretch SCL by tClockpulse before driving SDA. If tClockPulse is greater
+  // than scl_low_cycles then i2c_monitor acknowledge the Ack later and then drives Rdata when SCL
+  // is high.
+  cfg.m_i2c_agent_cfg.timing_cfg.tClockPulse = scl_low_cycles;
+
+  // tClockLow are the SCL low cycles that the i2c_driver use before driving SDA after stretching
+  // SCL by tClockPulse cycles. Drive SDA atleast tSetBit cycles earlier to avoid the chances of
+  // SDA interference.
+  cfg.m_i2c_agent_cfg.timing_cfg.tClockLow = scl_low_cycles - tSetupBit;
+endfunction
+
+task top_chip_dv_i2c_host_tx_rx_vseq::body();
+  i2c_device_response_seq seq = i2c_device_response_seq::type_id::create("seq");
+
+  configure_agent_timing();
+  print_i2c_timing_cfg();
+
+  // Configure the agent to be reactive
+  cfg.m_i2c_agent_cfg.if_mode = Device;
+
+  `DV_WAIT(cfg.sw_test_status_vif.sw_test_status == SwTestStatusInTest);
+
+  `uvm_info(`gfn, "Starting I2C Host TX-RX test", UVM_LOW)
+
+  fork
+    seq.start(p_sequencer.i2c_sqr);
+  join_none
+endtask : body
diff --git a/hw/top_chip/dv/env/seq_lib/top_chip_dv_i2c_tx_rx_vseq.sv b/hw/top_chip/dv/env/seq_lib/top_chip_dv_i2c_tx_rx_vseq.sv
new file mode 100644
index 000000000..63441562d
--- /dev/null
+++ b/hw/top_chip/dv/env/seq_lib/top_chip_dv_i2c_tx_rx_vseq.sv
@@ -0,0 +1,66 @@
+// Copyright lowRISC contributors (OpenTitan project).
+// Licensed under the Apache License, Version 2.0, see LICENSE for details.
+// SPDX-License-Identifier: Apache-2.0
+
+class top_chip_dv_i2c_tx_rx_vseq extends top_chip_dv_base_vseq;
+  `uvm_object_utils(top_chip_dv_i2c_tx_rx_vseq)
+
+  // Below variables will get assign through SW backdoor load. They are defined as byte size array
+  // as "sw_symbol_backdoor_read/overwrite" takes an array as an argument to write or read the SW
+  // symbol.
+  protected bit [7:0] sw_sys_clk_period_ns[1];
+  protected bit [7:0] sw_scl_clk_period_ns[2];
+  protected bit [7:0] sw_scl_low_time_ns[2];
+  protected bit [7:0] sw_data_hold_time_ns[2];
+
+  extern function new(string name="");
+
+  // Obtain an integer minimum of timing parameter "a" and round up to the next highest integer.
+  extern protected function int unsigned round_up_divide(int unsigned a, int unsigned b);
+  extern protected function void print_i2c_timing_cfg();
+endclass : top_chip_dv_i2c_tx_rx_vseq
+
+function top_chip_dv_i2c_tx_rx_vseq::new(string name = "");
+  super.new(name);
+endfunction
+
+function int unsigned top_chip_dv_i2c_tx_rx_vseq::round_up_divide(int unsigned a, int unsigned b);
+  return (((a - 1) / b) + 1);
+endfunction
+
+function void top_chip_dv_i2c_tx_rx_vseq::print_i2c_timing_cfg();
+  string str;
+  str = {str, $sformatf("\n    timing_cfg.tSetupStart       : %d",
+            cfg.m_i2c_agent_cfg.timing_cfg.tSetupStart)};
+  str = {str, $sformatf("\n    timing_cfg.tHoldStart        : %d",
+            cfg.m_i2c_agent_cfg.timing_cfg.tHoldStart)};
+  str = {str, $sformatf("\n    timing_cfg.tClockStart       : %d",
+            cfg.m_i2c_agent_cfg.timing_cfg.tClockStart)};
+  str = {str, $sformatf("\n    timing_cfg.tClockLow         : %d",
+            cfg.m_i2c_agent_cfg.timing_cfg.tClockLow)};
+  str = {str, $sformatf("\n    timing_cfg.tSetupBit         : %d",
+            cfg.m_i2c_agent_cfg.timing_cfg.tSetupBit)};
+  str = {str, $sformatf("\n    timing_cfg.tClockPulse       : %d",
+            cfg.m_i2c_agent_cfg.timing_cfg.tClockPulse)};
+  str = {str, $sformatf("\n    timing_cfg.tHoldBit          : %d",
+            cfg.m_i2c_agent_cfg.timing_cfg.tHoldBit)};
+  str = {str, $sformatf("\n    timing_cfg.tClockStop        : %d",
+            cfg.m_i2c_agent_cfg.timing_cfg.tClockStop)};
+  str = {str, $sformatf("\n    timing_cfg.tSetupStop        : %d",
+            cfg.m_i2c_agent_cfg.timing_cfg.tSetupStop)};
+  str = {str, $sformatf("\n    timing_cfg.tHoldStop         : %d",
+            cfg.m_i2c_agent_cfg.timing_cfg.tHoldStop)};
+  str = {str, $sformatf("\n    timing_cfg.tTimeOut          : %d",
+            cfg.m_i2c_agent_cfg.timing_cfg.tTimeOut)};
+  str = {str, $sformatf("\n    timing_cfg.enbTimeOut        : %d",
+            cfg.m_i2c_agent_cfg.timing_cfg.enbTimeOut)};
+  str = {str, $sformatf("\n    timing_cfg.tStretchHostClock : %d",
+            cfg.m_i2c_agent_cfg.timing_cfg.tStretchHostClock)};
+  str = {str, $sformatf("\n    timing_cfg.tSdaUnstable      : %d",
+            cfg.m_i2c_agent_cfg.timing_cfg.tSdaUnstable)};
+  str = {str, $sformatf("\n    timing_cfg.tSdaInterference  : %d",
+            cfg.m_i2c_agent_cfg.timing_cfg.tSdaInterference)};
+  str = {str, $sformatf("\n    timing_cfg.tSclInterference  : %d",
+            cfg.m_i2c_agent_cfg.timing_cfg.tSclInterference)};
+  `uvm_info(`gfn, $sformatf("%s", str), UVM_MEDIUM);
+endfunction
diff --git a/hw/top_chip/dv/env/seq_lib/top_chip_dv_vseq_list.sv b/hw/top_chip/dv/env/seq_lib/top_chip_dv_vseq_list.sv
index 151f6eb4b..14d38ea17 100644
--- a/hw/top_chip/dv/env/seq_lib/top_chip_dv_vseq_list.sv
+++ b/hw/top_chip/dv/env/seq_lib/top_chip_dv_vseq_list.sv
@@ -5,3 +5,5 @@
 `include "top_chip_dv_base_vseq.sv"
 `include "top_chip_dv_uart_base_vseq.sv"
 `include "top_chip_dv_gpio_smoke_vseq.sv"
+`include "top_chip_dv_i2c_tx_rx_vseq.sv"
+`include "top_chip_dv_i2c_host_tx_rx_vseq.sv"
diff --git a/hw/top_chip/dv/env/top_chip_dv_env.core b/hw/top_chip/dv/env/top_chip_dv_env.core
index 463ab0ccd..2ed5b43ac 100644
--- a/hw/top_chip/dv/env/top_chip_dv_env.core
+++ b/hw/top_chip/dv/env/top_chip_dv_env.core
@@ -25,6 +25,8 @@ filesets:
       - seq_lib/top_chip_dv_base_vseq.sv: {is_include_file: true}
       - seq_lib/top_chip_dv_uart_base_vseq.sv: {is_include_file: true}
       - seq_lib/top_chip_dv_gpio_smoke_vseq.sv: {is_include_file: true}
+      - seq_lib/top_chip_dv_i2c_tx_rx_vseq.sv: {is_include_file: true}
+      - seq_lib/top_chip_dv_i2c_host_tx_rx_vseq.sv: {is_include_file: true}
     file_type: systemVerilogSource
 
 targets:
diff --git a/hw/top_chip/dv/env/top_chip_dv_env.sv b/hw/top_chip/dv/env/top_chip_dv_env.sv
index 2ef085242..c4fc5acd8 100644
--- a/hw/top_chip/dv/env/top_chip_dv_env.sv
+++ b/hw/top_chip/dv/env/top_chip_dv_env.sv
@@ -90,6 +90,7 @@ function void top_chip_dv_env::connect_phase(uvm_phase phase);
   // Track specific agent sequencers in the virtual sequencer.
   // Allows virtual sequences to use the agents to drive RX items.
   top_vsqr.uart_sqr = m_uart_agent.sequencer;
+  top_vsqr.i2c_sqr  = m_i2c_agent.sequencer;
 
   // Connect monitor output to matching FIFO in the virtual sequencer.
   // Allows virtual sequences to check TX items.
diff --git a/hw/top_chip/dv/env/top_chip_dv_virtual_sequencer.sv b/hw/top_chip/dv/env/top_chip_dv_virtual_sequencer.sv
index f12a799fc..d6c9a12dc 100644
--- a/hw/top_chip/dv/env/top_chip_dv_virtual_sequencer.sv
+++ b/hw/top_chip/dv/env/top_chip_dv_virtual_sequencer.sv
@@ -14,6 +14,7 @@ class top_chip_dv_virtual_sequencer extends uvm_sequencer;
   // Handles to specific interface agent sequencers. Used by some virtual
   // sequences to drive RX (to-chip) items.
   uart_sequencer uart_sqr;
+  i2c_sequencer  i2c_sqr;
 
   // FIFOs for monitor output. Used by some virtual sequences to check
   // TX (from-chip) items.
diff --git a/hw/top_chip/dv/top_chip_sim_cfg.hjson b/hw/top_chip/dv/top_chip_sim_cfg.hjson
index 115ab003a..dbd9ee96f 100644
--- a/hw/top_chip/dv/top_chip_sim_cfg.hjson
+++ b/hw/top_chip/dv/top_chip_sim_cfg.hjson
@@ -151,7 +151,14 @@
       sw_images: ["i2c_host_tx_rx_test_vanilla_sram:5" "bootrom:5"]
       run_opts: ["+ChipMemSRAM_image_file={run_dir}/i2c_host_tx_rx_test_vanilla_sram.vmem",
                  "+ChipMemROM_image_file={run_dir}/bootrom.vmem"]
-      }
+    }
+    {
+      name: i2c_host_tx_rx_cheri
+      uvm_test_seq: top_chip_dv_i2c_host_tx_rx_vseq
+      sw_images: ["i2c_host_tx_rx_test_cheri_sram:5" "bootrom:5"]
+      run_opts: ["+ChipMemSRAM_image_file={run_dir}/i2c_host_tx_rx_test_cheri_sram.vmem",
+                 "+ChipMemROM_image_file={run_dir}/bootrom.vmem"]
+    }
     {
       name: i2c_device_tx_rx
       uvm_test_seq: top_chip_dv_i2c_device_tx_rx_vseq
diff --git a/sw/device/lib/hal/i2c.c b/sw/device/lib/hal/i2c.c
index cf0a76ae8..f1f58767a 100644
--- a/sw/device/lib/hal/i2c.c
+++ b/sw/device/lib/hal/i2c.c
@@ -8,6 +8,15 @@
 #include <stdbool.h>
 #include <stdint.h>
 
+// Below symbols are going to be read by top_chip_dv_i2c_host_tx_rx_vseq in order to calculate agent
+// timing parameters.
+//
+// The values below are standard mode minimums
+volatile uint8_t SysClkPeriodNS = SYSCLK_NS;
+volatile uint16_t SCLClkPeriodNS = 10000;
+volatile uint16_t SCLLowTimeNs = 4700;
+volatile uint16_t HoldDataTimeNs = 1;
+
 /**
  * Performs a 32-bit integer unsigned division, rounding up. The bottom
  * 16 bits of the result are then returned.