bsp/atsam/i2c: Add error return and fix edge cases

The driver didn't return with an error on (for example) a NACK on the
bus. This adds the expected error return. Due to the new case that a
transfer can be interrupted on an error, there were some new edge cases.
This patch therefore also fixes these edge cases by removing the
transfer_state that more or less duplicated the interrupt states.

Fixes #4591
This commit is contained in:
Christian Mauderer
2022-02-03 09:38:40 +01:00
parent f71c620f7e
commit f7a204e3d7
2 changed files with 47 additions and 59 deletions

View File

@@ -33,18 +33,18 @@
static void
atsam_i2c_disable_interrupts(Twihs *regs)
{
regs->TWIHS_IDR = 0xFFFFFFFF;
TWI_DisableIt(regs, 0xFFFFFFFF);
}
static void
atsam_i2c_set_transfer_status(atsam_i2c_bus *bus, transfer_state state)
{
bus->trans_state = state;
}
static void
/*
* Return true if the message is done right after this. That is the case if all
* bytes are received but no stop is requested.
*/
static bool
atsam_i2c_continue_read(Twihs *regs, atsam_i2c_bus *bus)
{
bool done = false;
*bus->current_msg_byte = TWI_ReadByte(regs);
++bus->current_msg_byte;
--bus->current_msg_todo;
@@ -54,35 +54,35 @@ atsam_i2c_continue_read(Twihs *regs, atsam_i2c_bus *bus)
if (bus->stop_request){
TWI_DisableIt(regs, TWIHS_IDR_RXRDY);
TWI_EnableIt(regs, TWIHS_IER_TXCOMP);
atsam_i2c_set_transfer_status(bus, TX_RX_STOP_SENT);
} else {
atsam_i2c_set_transfer_status(bus, RX_CONT_MESSAGE_NEEDED);
done = true;
}
}
/* Last byte? */
else if (bus->current_msg_todo == 1 && bus->stop_request) {
TWI_Stop(regs);
}
return done;
}
/*
* Return true if the message is done right after this. That is the case if all
* bytes are sent but no stop is requested.
*/
static bool
atsam_i2c_is_state(atsam_i2c_bus *bus, transfer_state state)
{
return (bus->trans_state == state);
}
static void
atsam_i2c_continue_write(Twihs *regs, atsam_i2c_bus *bus)
{
bool done = false;
/* Transfer finished ? */
if (bus->current_msg_todo == 0) {
TWI_DisableIt(regs, TWIHS_IDR_TXRDY);
if (bus->stop_request){
TWI_EnableIt(regs, TWIHS_IER_TXCOMP);
TWI_SendSTOPCondition(regs);
atsam_i2c_set_transfer_status(bus, TX_RX_STOP_SENT);
} else {
atsam_i2c_set_transfer_status(bus, TX_CONT_MESSAGE_NEEDED);
done = true;
}
}
/* Bytes remaining */
@@ -91,13 +91,7 @@ atsam_i2c_continue_write(Twihs *regs, atsam_i2c_bus *bus)
++bus->current_msg_byte;
--bus->current_msg_todo;
}
}
static void
atsam_i2c_finish_write_transfer(Twihs *regs)
{
TWI_ReadByte(regs);
TWI_DisableIt(regs, TWIHS_IDR_TXCOMP);
return done;
}
static void
@@ -202,11 +196,9 @@ atsam_i2c_setup_transfer(atsam_i2c_bus *bus, Twihs *regs)
if (bus->current_msg_todo == 1){
send_stop = true;
}
atsam_i2c_set_transfer_status(bus, RX_SEND_DATA);
atsam_i2c_setup_read_transfer(regs, ten_bit_addr,
slave_addr, send_stop);
} else {
atsam_i2c_set_transfer_status(bus, TX_SEND_DATA);
atsam_i2c_setup_write_transfer(bus, regs, ten_bit_addr,
slave_addr);
}
@@ -222,33 +214,24 @@ atsam_i2c_interrupt(void *arg)
Twihs *regs = bus->regs;
/* read interrupts */
irqstatus = regs->TWIHS_SR;
irqstatus = TWI_GetMaskedStatus(regs);
if((irqstatus & (TWIHS_SR_ARBLST | TWIHS_SR_NACK)) != 0) {
if((irqstatus & ATSAMV_I2C_IRQ_ERROR) != 0) {
done = true;
}
if (((irqstatus & TWIHS_SR_RXRDY) != 0) &&
(atsam_i2c_is_state(bus, RX_SEND_DATA))){
/* carry on read transfer */
atsam_i2c_continue_read(regs, bus);
} else if (((irqstatus & TWIHS_SR_TXCOMP) != 0) &&
(atsam_i2c_is_state(bus, TX_RX_STOP_SENT))){
atsam_i2c_finish_write_transfer(regs);
} else if ((irqstatus & TWIHS_SR_RXRDY) != 0) {
done = atsam_i2c_continue_read(regs, bus);
} else if ((irqstatus & TWIHS_SR_TXCOMP) != 0) {
TWI_DisableIt(regs, TWIHS_IDR_TXCOMP);
done = true;
} else if (((irqstatus & TWIHS_SR_TXRDY) != 0) &&
(atsam_i2c_is_state(bus, TX_SEND_DATA))){
atsam_i2c_continue_write(regs, bus);
if (atsam_i2c_is_state(bus, TX_CONT_MESSAGE_NEEDED)){
done = true;
}
} else if ((irqstatus & TWIHS_SR_TXRDY) != 0) {
done = atsam_i2c_continue_write(regs, bus);
}
if(done){
uint32_t err = irqstatus & ATSAMV_I2C_IRQ_ERROR;
bus->err = irqstatus & ATSAMV_I2C_IRQ_ERROR;
atsam_i2c_next_packet(bus);
if (bus->msg_todo == 0 || err != 0) {
if (bus->msg_todo == 0 || bus->err != 0) {
atsam_i2c_disable_interrupts(regs);
rtems_binary_semaphore_post(&bus->sem);
} else {
@@ -273,27 +256,38 @@ atsam_i2c_transfer(i2c_bus *base, i2c_msg *msgs, uint32_t msg_count)
if ((msgs[i].flags & I2C_M_RECV_LEN) != 0) {
return -EINVAL;
}
if (msgs[i].len == 0) {
/* Hardware doesn't support zero length messages */
return -EINVAL;
}
}
bus->msgs = &msgs[0];
bus->msg_todo = msg_count;
bus->current_msg_todo = msgs[0].len;
bus->current_msg_byte = msgs[0].buf;
bus->err = 0;
regs = bus->regs;
atsam_i2c_setup_transfer(bus, regs);
/* Start with a clean start. Enable error interrupts. */
TWI_ConfigureMaster(bus->regs, bus->output_clock, BOARD_MCK);
TWI_EnableIt(regs, ATSAMV_I2C_IRQ_ERROR);
regs->TWIHS_IER = ATSAMV_I2C_IRQ_ERROR;
atsam_i2c_setup_transfer(bus, regs);
eno = rtems_binary_semaphore_wait_timed_ticks(
&bus->sem,
bus->base.timeout
);
if (eno != 0) {
if (eno != 0 || bus->err != 0) {
TWI_ConfigureMaster(bus->regs, bus->output_clock, BOARD_MCK);
rtems_binary_semaphore_try_wait(&bus->sem);
return -ETIMEDOUT;
if (bus->err != 0) {
return -EIO;
} else {
return -ETIMEDOUT;
}
}
return 0;

View File

@@ -28,14 +28,6 @@ extern "C" {
#define TWI_AMOUNT_PINS 2
typedef enum {
TX_SEND_DATA,
TX_CONT_MESSAGE_NEEDED,
RX_SEND_DATA,
RX_CONT_MESSAGE_NEEDED,
TX_RX_STOP_SENT
}transfer_state;
typedef struct {
i2c_bus base;
Twihs *regs;
@@ -46,10 +38,12 @@ typedef struct {
/* Information about the current transfer. */
bool stop_request;
transfer_state trans_state;
uint32_t current_msg_todo;
uint8_t *current_msg_byte;
/* Error information that can be returned to the task */
uint32_t err;
uint32_t output_clock;
rtems_binary_semaphore sem;
rtems_vector_number irq;