From 0074c9ecf3fafcceaf8a84d60304153cec7eed68 Mon Sep 17 00:00:00 2001 From: Christian Mauderer Date: Thu, 3 Feb 2022 09:38:40 +0100 Subject: 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 #4592 --- bsps/arm/atsam/i2c/atsam_i2c_bus.c | 94 ++++++++++++++++------------------ bsps/arm/atsam/include/bsp/atsam-i2c.h | 12 ++--- 2 files changed, 47 insertions(+), 59 deletions(-) (limited to 'bsps/arm/atsam') diff --git a/bsps/arm/atsam/i2c/atsam_i2c_bus.c b/bsps/arm/atsam/i2c/atsam_i2c_bus.c index 22c63fca89..3451d15bed 100644 --- a/bsps/arm/atsam/i2c/atsam_i2c_bus.c +++ b/bsps/arm/atsam/i2c/atsam_i2c_bus.c @@ -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); } -} -static bool -atsam_i2c_is_state(atsam_i2c_bus *bus, transfer_state state) -{ - return (bus->trans_state == state); + return done; } -static void +/* + * 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_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; diff --git a/bsps/arm/atsam/include/bsp/atsam-i2c.h b/bsps/arm/atsam/include/bsp/atsam-i2c.h index 532be1dae5..a6a9c27d48 100644 --- a/bsps/arm/atsam/include/bsp/atsam-i2c.h +++ b/bsps/arm/atsam/include/bsp/atsam-i2c.h @@ -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; -- cgit v1.2.3