From 5dcd72897edddb8bd49ce0e46f05e5face452a86 Mon Sep 17 00:00:00 2001 From: Kinza Qamar Date: Thu, 7 May 2026 21:21:42 +0100 Subject: [PATCH 1/5] [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 --- 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 Date: Fri, 8 May 2026 13:49:34 +0100 Subject: [PATCH 2/5] [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 --- 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 Date: Mon, 11 May 2026 21:28:30 +0100 Subject: [PATCH 3/5] [I2C, sw] Added a HAL function to return I2C RData Signed-off-by: Kinza Qamar --- 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 Date: Fri, 8 May 2026 09:56:22 +0100 Subject: [PATCH 4/5] [I2C, SW, hal] Extended read / write functions to R/W N bytes Signed-off-by: Kinza Qamar --- 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 Date: Tue, 19 May 2026 12:29:21 +0100 Subject: [PATCH 5/5] [i2c, sw] Added an overflow check when the byte is equal to FMT FIFO depth Signed-off-by: Kinza Qamar --- 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;