Skip to content
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

Jump to bottom

Add I2C last_abort_reason #1370

Draft
wants to merge 2 commits into
base: develop
Choose a base branch
from
Draft

Add I2C last_abort_reason #1370

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
aa786a1
Add last_abort_reason
peterharperuk May 11, 2023
60f5901
Add i2c_result_t
peterharperuk Sep 25, 2024
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
117 changes: 78 additions & 39 deletions src/rp2_common/hardware_i2c/i2c.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ void i2c_set_slave_mode(i2c_inst_t *i2c, bool slave, uint8_t addr) {
i2c->hw->enable = 1;
}

static int i2c_write_blocking_internal(i2c_inst_t *i2c, uint8_t addr, const uint8_t *src, size_t len, bool nostop,
static i2c_result_t i2c_write_blocking_internal(i2c_inst_t *i2c, uint8_t addr, const uint8_t *src, size_t len, bool nostop,
Copy link
Contributor

Choose a reason for hiding this comment

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

For consistency with function-names and return-types, I wonder if i2c_write_blocking_internal ought to be renamed to i2c_write_result_blocking_internal ? 🤔 (It's an internal-only function name, so I think we're free to rename it?)

check_timeout_fn timeout_check, struct timeout_state *ts) {
invalid_params_if(HARDWARE_I2C, addr >= 0x80); // 7-bit addresses
invalid_params_if(HARDWARE_I2C, i2c_reserved_addr(addr));
Expand All @@ -146,7 +146,7 @@ static int i2c_write_blocking_internal(i2c_inst_t *i2c, uint8_t addr, const uint
bool abort = false;
bool timeout = false;

uint32_t abort_reason = 0;
i2c_result_t result = { 0, 0 };
int byte_ctr;

int ilen = (int)len;
Expand Down Expand Up @@ -177,8 +177,8 @@ static int i2c_write_blocking_internal(i2c_inst_t *i2c, uint8_t addr, const uint

// If there was a timeout, don't attempt to do anything else.
if (!timeout) {
abort_reason = i2c->hw->tx_abrt_source;
if (abort_reason) {
result.abort_reason = i2c->hw->tx_abrt_source;
if (result.abort_reason) {
// Note clearing the abort flag also clears the reason, and
// this instance of flag is clear-on-read! Note also the
// IC_CLR_TX_ABRT register always reads as 0.
Expand Down Expand Up @@ -215,57 +215,77 @@ static int i2c_write_blocking_internal(i2c_inst_t *i2c, uint8_t addr, const uint
break;
}

int rval;

// A lot of things could have just happened due to the ingenious and
// creative design of I2C. Try to figure things out.
if (abort) {
if (timeout)
rval = PICO_ERROR_TIMEOUT;
else if (!abort_reason || abort_reason & I2C_IC_TX_ABRT_SOURCE_ABRT_7B_ADDR_NOACK_BITS) {
result.rval = PICO_ERROR_TIMEOUT;
else if (!result.abort_reason || result.abort_reason & I2C_IC_TX_ABRT_SOURCE_ABRT_7B_ADDR_NOACK_BITS) {
// No reported errors - seems to happen if there is nothing connected to the bus.
// Address byte not acknowledged
rval = PICO_ERROR_GENERIC;
} else if (abort_reason & I2C_IC_TX_ABRT_SOURCE_ABRT_TXDATA_NOACK_BITS) {
result.rval = PICO_ERROR_GENERIC;
} else if (result.abort_reason & I2C_IC_TX_ABRT_SOURCE_ABRT_TXDATA_NOACK_BITS) {
// Address acknowledged, some data not acknowledged
rval = byte_ctr;
result.rval = byte_ctr;
} else {
//panic("Unknown abort from I2C instance @%08x: %08x\n", (uint32_t) i2c->hw, abort_reason);
rval = PICO_ERROR_GENERIC;
//panic("Unknown abort from I2C instance @%08x: %08x\n", (uint32_t) i2c->hw, result.abort_reason);
result.rval = PICO_ERROR_GENERIC;
}
} else {
rval = byte_ctr;
result.rval = byte_ctr;
}

// nostop means we are now at the end of a *message* but not the end of a *transfer*
i2c->restart_on_next = nostop;
return rval;
return result;
}

int i2c_write_blocking(i2c_inst_t *i2c, uint8_t addr, const uint8_t *src, size_t len, bool nostop) {
i2c_result_t i2c_write_result_blocking(i2c_inst_t *i2c, uint8_t addr, const uint8_t *src, size_t len, bool nostop) {
return i2c_write_blocking_internal(i2c, addr, src, len, nostop, NULL, NULL);
}

int i2c_write_blocking_until(i2c_inst_t *i2c, uint8_t addr, const uint8_t *src, size_t len, bool nostop,
int i2c_write_blocking(i2c_inst_t *i2c, uint8_t addr, const uint8_t *src, size_t len, bool nostop) {
i2c_result_t r = i2c_write_result_blocking(i2c, addr, src, len, nostop);
return r.rval;
}

i2c_result_t i2c_write_result_blocking_until(i2c_inst_t *i2c, uint8_t addr, const uint8_t *src, size_t len, bool nostop,
absolute_time_t until) {
timeout_state_t ts;
return i2c_write_blocking_internal(i2c, addr, src, len, nostop, init_single_timeout_until(&ts, until), &ts);
}

int i2c_write_timeout_per_char_us(i2c_inst_t *i2c, uint8_t addr, const uint8_t *src, size_t len, bool nostop,
int i2c_write_blocking_until(i2c_inst_t *i2c, uint8_t addr, const uint8_t *src, size_t len, bool nostop,
absolute_time_t until) {
i2c_result_t r = i2c_write_result_blocking_until(i2c, addr, src, len, nostop, until);
return r.rval;
}

i2c_result_t i2c_write_result_timeout_per_char_us(i2c_inst_t *i2c, uint8_t addr, const uint8_t *src, size_t len, bool nostop,
uint timeout_per_char_us) {
timeout_state_t ts;
return i2c_write_blocking_internal(i2c, addr, src, len, nostop,
init_per_iteration_timeout_us(&ts, timeout_per_char_us), &ts);
}

int i2c_write_burst_blocking(i2c_inst_t *i2c, uint8_t addr, const uint8_t *src, size_t len) {
int rc = i2c_write_blocking_internal(i2c, addr, src, len, true, NULL, NULL);
int i2c_write_timeout_per_char_us(i2c_inst_t *i2c, uint8_t addr, const uint8_t *src, size_t len, bool nostop,
uint timeout_per_char_us) {
i2c_result_t r = i2c_write_result_timeout_per_char_us(i2c, addr, src, len, nostop, timeout_per_char_us);
return r.rval;
}

i2c_result_t i2c_write_result_burst_blocking(i2c_inst_t *i2c, uint8_t addr, const uint8_t *src, size_t len) {
i2c_result_t r = i2c_write_blocking_internal(i2c, addr, src, len, true, NULL, NULL);
i2c->restart_on_next = false;
return rc;
return r;
}

int i2c_write_burst_blocking(i2c_inst_t *i2c, uint8_t addr, const uint8_t *src, size_t len) {
i2c_result_t r = i2c_write_result_burst_blocking(i2c, addr, src, len);
return r.rval;
}

static int i2c_read_blocking_internal(i2c_inst_t *i2c, uint8_t addr, uint8_t *dst, size_t len, bool nostop,
static i2c_result_t i2c_read_blocking_internal(i2c_inst_t *i2c, uint8_t addr, uint8_t *dst, size_t len, bool nostop,
check_timeout_fn timeout_check, timeout_state_t *ts) {
invalid_params_if(HARDWARE_I2C, addr >= 0x80); // 7-bit addresses
invalid_params_if(HARDWARE_I2C, i2c_reserved_addr(addr));
Expand All @@ -278,7 +298,7 @@ static int i2c_read_blocking_internal(i2c_inst_t *i2c, uint8_t addr, uint8_t *ds

bool abort = false;
bool timeout = false;
uint32_t abort_reason;
i2c_result_t result = { 0, 0 };
int byte_ctr;
int ilen = (int)len;
for (byte_ctr = 0; byte_ctr < ilen; ++byte_ctr) {
Expand All @@ -297,7 +317,7 @@ static int i2c_read_blocking_internal(i2c_inst_t *i2c, uint8_t addr, uint8_t *ds
I2C_IC_DATA_CMD_CMD_BITS; // -> 1 for read

do {
abort_reason = i2c->hw->tx_abrt_source;
result.abort_reason = i2c->hw->tx_abrt_source;
abort = (bool) i2c->hw->clr_tx_abrt;
if (timeout_check) {
timeout = timeout_check(ts, false);
Expand All @@ -311,45 +331,64 @@ static int i2c_read_blocking_internal(i2c_inst_t *i2c, uint8_t addr, uint8_t *ds
*dst++ = (uint8_t) i2c->hw->data_cmd;
}

int rval;

if (abort) {
if (timeout)
rval = PICO_ERROR_TIMEOUT;
else if (!abort_reason || abort_reason & I2C_IC_TX_ABRT_SOURCE_ABRT_7B_ADDR_NOACK_BITS) {
result.rval = PICO_ERROR_TIMEOUT;
else if (!result.abort_reason || result.abort_reason & I2C_IC_TX_ABRT_SOURCE_ABRT_7B_ADDR_NOACK_BITS) {
// No reported errors - seems to happen if there is nothing connected to the bus.
// Address byte not acknowledged
rval = PICO_ERROR_GENERIC;
result.rval = PICO_ERROR_GENERIC;
} else {
// panic("Unknown abort from I2C instance @%08x: %08x\n", (uint32_t) i2c->hw, abort_reason);
rval = PICO_ERROR_GENERIC;
// panic("Unknown abort from I2C instance @%08x: %08x\n", (uint32_t) i2c->hw, result.abort_reason);
result.rval = PICO_ERROR_GENERIC;
}
} else {
rval = byte_ctr;
result.rval = byte_ctr;
}

i2c->restart_on_next = nostop;
return rval;
return result;
}

int i2c_read_blocking(i2c_inst_t *i2c, uint8_t addr, uint8_t *dst, size_t len, bool nostop) {
i2c_result_t i2c_read_result_blocking(i2c_inst_t *i2c, uint8_t addr, uint8_t *dst, size_t len, bool nostop) {
return i2c_read_blocking_internal(i2c, addr, dst, len, nostop, NULL, NULL);
}

int i2c_read_blocking_until(i2c_inst_t *i2c, uint8_t addr, uint8_t *dst, size_t len, bool nostop, absolute_time_t until) {
int i2c_read_blocking(i2c_inst_t *i2c, uint8_t addr, uint8_t *dst, size_t len, bool nostop) {
i2c_result_t r = i2c_read_result_blocking(i2c, addr, dst, len, nostop);
return r.rval;
}

i2c_result_t i2c_read_result_blocking_until(i2c_inst_t *i2c, uint8_t addr, uint8_t *dst, size_t len, bool nostop, absolute_time_t until) {
timeout_state_t ts;
return i2c_read_blocking_internal(i2c, addr, dst, len, nostop, init_single_timeout_until(&ts, until), &ts);
}

int i2c_read_timeout_per_char_us(i2c_inst_t *i2c, uint8_t addr, uint8_t *dst, size_t len, bool nostop,
int i2c_read_blocking_until(i2c_inst_t *i2c, uint8_t addr, uint8_t *dst, size_t len, bool nostop, absolute_time_t until) {
i2c_result_t r = i2c_read_result_blocking_until(i2c, addr, dst, len, nostop, until);
return r.rval;
}

i2c_result_t i2c_read_result_timeout_per_char_us(i2c_inst_t *i2c, uint8_t addr, uint8_t *dst, size_t len, bool nostop,
uint timeout_per_char_us) {
timeout_state_t ts;
return i2c_read_blocking_internal(i2c, addr, dst, len, nostop,
init_per_iteration_timeout_us(&ts, timeout_per_char_us), &ts);
}

int i2c_read_burst_blocking(i2c_inst_t *i2c, uint8_t addr, uint8_t *dst, size_t len) {
int rc = i2c_read_blocking_internal(i2c, addr, dst, len, true, NULL, NULL);
int i2c_read_timeout_per_char_us(i2c_inst_t *i2c, uint8_t addr, uint8_t *dst, size_t len, bool nostop,
uint timeout_per_char_us) {
i2c_result_t r = i2c_read_result_timeout_per_char_us(i2c, addr, dst, len, nostop, timeout_per_char_us);
return r.rval;
}

i2c_result_t i2c_read_result_burst_blocking(i2c_inst_t *i2c, uint8_t addr, uint8_t *dst, size_t len) {
i2c_result_t r = i2c_read_blocking_internal(i2c, addr, dst, len, true, NULL, NULL);
i2c->restart_on_next = false;
return rc;
return r;
}

int i2c_read_burst_blocking(i2c_inst_t *i2c, uint8_t addr, uint8_t *dst, size_t len) {
i2c_result_t r = i2c_read_result_burst_blocking(i2c, addr, dst, len);
return r.rval;
}
13 changes: 13 additions & 0 deletions src/rp2_common/hardware_i2c/include/hardware/i2c.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,11 @@ struct i2c_inst {
bool restart_on_next;
};

typedef struct i2c_result {
int32_t rval;
uint32_t abort_reason;
} i2c_result_t;

/**
* \def I2C_NUM(i2c)
* \ingroup hardware_i2c
Expand Down Expand Up @@ -246,6 +251,7 @@ static inline i2c_inst_t *i2c_get_instance(uint num) {
*
* \return Number of bytes written, or PICO_ERROR_GENERIC if address not acknowledged, no device present, or PICO_ERROR_TIMEOUT if a timeout occurred.
*/
i2c_result_t i2c_write_result_blocking_until(i2c_inst_t *i2c, uint8_t addr, const uint8_t *src, size_t len, bool nostop, absolute_time_t until);
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this (and the other new _result_ functions) need to be added to the header file after the old functions, in order not to invalidate the documented doxygen \return parameter?

int i2c_write_blocking_until(i2c_inst_t *i2c, uint8_t addr, const uint8_t *src, size_t len, bool nostop, absolute_time_t until);

/*! \brief Attempt to read specified number of bytes from address, blocking until the specified absolute time is reached.
Expand All @@ -260,6 +266,7 @@ int i2c_write_blocking_until(i2c_inst_t *i2c, uint8_t addr, const uint8_t *src,
* \param until The absolute time that the block will wait until the entire transaction is complete.
* \return Number of bytes read, or PICO_ERROR_GENERIC if address not acknowledged, no device present, or PICO_ERROR_TIMEOUT if a timeout occurred.
*/
i2c_result_t i2c_read_result_blocking_until(i2c_inst_t *i2c, uint8_t addr, uint8_t *dst, size_t len, bool nostop, absolute_time_t until);
int i2c_read_blocking_until(i2c_inst_t *i2c, uint8_t addr, uint8_t *dst, size_t len, bool nostop, absolute_time_t until);

/*! \brief Attempt to write specified number of bytes to address, with timeout
Expand All @@ -282,6 +289,7 @@ static inline int i2c_write_timeout_us(i2c_inst_t *i2c, uint8_t addr, const uint
return i2c_write_blocking_until(i2c, addr, src, len, nostop, t);
}

i2c_result_t i2c_write_result_timeout_per_char_us(i2c_inst_t *i2c, uint8_t addr, const uint8_t *src, size_t len, bool nostop, uint timeout_per_char_us);
int i2c_write_timeout_per_char_us(i2c_inst_t *i2c, uint8_t addr, const uint8_t *src, size_t len, bool nostop, uint timeout_per_char_us);

/*! \brief Attempt to read specified number of bytes from address, with timeout
Expand All @@ -301,6 +309,7 @@ static inline int i2c_read_timeout_us(i2c_inst_t *i2c, uint8_t addr, uint8_t *ds
return i2c_read_blocking_until(i2c, addr, dst, len, nostop, t);
}

i2c_result_t i2c_read_result_timeout_per_char_us(i2c_inst_t *i2c, uint8_t addr, uint8_t *dst, size_t len, bool nostop, uint timeout_per_char_us);
int i2c_read_timeout_per_char_us(i2c_inst_t *i2c, uint8_t addr, uint8_t *dst, size_t len, bool nostop, uint timeout_per_char_us);

/*! \brief Attempt to write specified number of bytes to address, blocking
Expand All @@ -314,6 +323,7 @@ int i2c_read_timeout_per_char_us(i2c_inst_t *i2c, uint8_t addr, uint8_t *dst, si
* and the next transfer will begin with a Restart rather than a Start.
* \return Number of bytes written, or PICO_ERROR_GENERIC if address not acknowledged, no device present.
*/
i2c_result_t i2c_write_result_blocking(i2c_inst_t *i2c, uint8_t addr, const uint8_t *src, size_t len, bool nostop);
int i2c_write_blocking(i2c_inst_t *i2c, uint8_t addr, const uint8_t *src, size_t len, bool nostop);

/*! \brief Attempt to write specified number of bytes to address, blocking in burst mode
Expand All @@ -329,6 +339,7 @@ int i2c_write_blocking(i2c_inst_t *i2c, uint8_t addr, const uint8_t *src, size_t
* \param len Length of data in bytes to receive
* \return Number of bytes read, or PICO_ERROR_GENERIC if address not acknowledged or no device present.
*/
i2c_result_t i2c_write_result_burst_blocking(i2c_inst_t *i2c, uint8_t addr, const uint8_t *src, size_t len);
int i2c_write_burst_blocking(i2c_inst_t *i2c, uint8_t addr, const uint8_t *src, size_t len);

/*! \brief Attempt to read specified number of bytes from address, blocking
Expand All @@ -342,6 +353,7 @@ int i2c_write_burst_blocking(i2c_inst_t *i2c, uint8_t addr, const uint8_t *src,
* and the next transfer will begin with a Restart rather than a Start.
* \return Number of bytes read, or PICO_ERROR_GENERIC if address not acknowledged or no device present.
*/
i2c_result_t i2c_read_result_blocking(i2c_inst_t *i2c, uint8_t addr, uint8_t *dst, size_t len, bool nostop);
int i2c_read_blocking(i2c_inst_t *i2c, uint8_t addr, uint8_t *dst, size_t len, bool nostop);

/*! \brief Attempt to read specified number of bytes from address, blocking in burst mode
Expand All @@ -357,6 +369,7 @@ int i2c_read_blocking(i2c_inst_t *i2c, uint8_t addr, uint8_t *dst, size_t len, b
* \param len Length of data in bytes to receive
* \return Number of bytes read, or PICO_ERROR_GENERIC if address not acknowledged or no device present.
*/
i2c_result_t i2c_read_result_burst_blocking(i2c_inst_t *i2c, uint8_t addr, uint8_t *dst, size_t len);
int i2c_read_burst_blocking(i2c_inst_t *i2c, uint8_t addr, uint8_t *dst, size_t len);

/*! \brief Determine non-blocking write space available
Expand Down
Loading