Skip to content

[I2C, hal, SW] Restructured and refactored the functions #535

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
KinzaQamar wants to merge 4 commits into
base: main
Choose a base branch
from
Open

[I2C, hal, SW] Restructured and refactored the functions #535

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
66582aa
[I2C, hal, SW] Restructured and refactored the functions
KinzaQamar May 7, 2026
ced86a4
[I2C, SW] Place the resetting of FMT fifo step when halt occurs
KinzaQamar May 8, 2026
28dd2d3
[I2C, sw] Added a HAL function to return I2C RData
KinzaQamar May 11, 2026
f3b66ea
[I2C, SW, hal] Extended read / write functions to R/W N bytes
KinzaQamar May 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions sw/device/examples/i2c.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,13 @@ 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
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_n_bytes(i2c, 0x48u, &w_data, 1);
if (wait_wr_xfer_status(i2c)) { // select TVAL reg; also a presence check
i2c_read_n_bytes(i2c, 0x48u, 1);
if (wait_rd_xfer_status(i2c)) {
uint16_t sensor_reading = i2c_rdata_byte(i2c); // read TVAl reg
uprintf(uart, "Temperature: 0x%x degC\n",
(sensor_reading << 1)); // no decimal printf
}
Expand Down
87 changes: 59 additions & 28 deletions sw/device/lib/hal/i2c.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,10 @@ 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_n_bytes(i2c_t i2c, uint8_t addr, const uint8_t *data, uint8_t num_wr_bytes)
{
// 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);

Expand All @@ -74,31 +75,22 @@ bool 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);

// 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
}
// Send all data bytes; assert STOP only on the last byte
for (uint8_t i = 0; i < num_wr_bytes; i++) {
fdata_reg.fbyte = data[i];
if (i == (num_wr_bytes - 1u)) {
fdata_reg.stop = 1u;
}
VOLATILE_WRITE(i2c->fdata, fdata_reg);
Copy link
Copy Markdown
Collaborator

@engdoreis engdoreis May 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you should check that the fifo is not full on every iteration, otherwise it would overflow when num_wr_bytes is large.

Copy link
Copy Markdown
Contributor Author

@KinzaQamar KinzaQamar May 13, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is going to be UVM agent receiving these bytes. It is capable of receiving any number of bytes.

I was thinking about checking FMTFULL before sending a transfer but since we always write 1 to fmtrst (which is going to reset the FMT FIFO), this is not needed. About checking RXFULL, this should be a test specific thing.

Copy link
Copy Markdown
Contributor

@elliotb-lowrisc elliotb-lowrisc May 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The UVM agent may be fast, but the i2c block itself might not be. If the processor writes data into the FMT FIFO faster than the I^2C hardware can transmit it, then all it takes is a sufficiently large data transfer for data to be lost when the processor eventually writes to an already-full FMT FIFO.

Copy link
Copy Markdown
Contributor Author

@KinzaQamar KinzaQamar May 15, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you should check that the fifo is not full on every iteration, otherwise it would overflow when num_wr_bytes is large.

Oh, sorry. I misunderstood your comment thinking as if you are talking about target's RX FIFO overflow.

Thanks for clarification @elliotb-lowrisc

}
return false; // timeout
}

uint8_t i2c_read_byte(i2c_t i2c, uint8_t addr)
void i2c_read_n_bytes(i2c_t i2c, uint8_t addr, uint8_t num_rd_bytes)
{
// 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);

Expand All @@ -112,28 +104,67 @@ uint8_t 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 = num_rd_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);
}

bool wait_wr_xfer_status(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) {
// 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);

// Wait for transaction to complete and return either read data or 0xFF
for (uint32_t ii = 0; ii < 10000000ul /*arbitrary number*/; ii++) {
// 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) {
i2c_status i2c_status_reg = VOLATILE_READ(i2c->status);
if (i2c_status_reg & i2c_status_fmtempty) {
return true; // Transaction succeeded
}
}
}
return false; // Timeout
}

bool wait_rd_xfer_status(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
// 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);
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)
{
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;
}
16 changes: 14 additions & 2 deletions sw/device/lib/hal/i2c.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,20 @@

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 multiple bytes to the target
void i2c_write_n_bytes(i2c_t i2c, uint8_t addr, const uint8_t *data, uint8_t num_wr_bytes);

// Receive n bytes from the target
void i2c_read_n_bytes(i2c_t i2c, uint8_t addr, uint8_t num_rd_bytes);

// Check if the write was successful
bool wait_rd_xfer_status(i2c_t i2c);

// checks if the read was successful
bool wait_wr_xfer_status(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);
18 changes: 15 additions & 3 deletions sw/device/tests/i2c/smoketest.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,27 @@
// 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
if (!i2c_write_byte(i2c, 0x48u, 0u)) {
i2c_write_n_bytes(i2c, 0x48u, &w_data, 1);

// Check if the write was successful
if (!wait_wr_xfer_status(i2c)) {
return false;
}

// Read current temperature
uint8_t byte = i2c_read_byte(i2c, 0x48u);
if (byte == 0xFFu /* error value */) {
i2c_read_n_bytes(i2c, 0x48u, 1);

// Check if the read was successful
if (!wait_rd_xfer_status(i2c)) {
return false;
}

// If the read was successful, then retrieve the data from the fifo.
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
tval >>= 7; // convert from units of 1/128 degC to 1 degC
Expand Down