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
---
 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(-)

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;
-- 
2.31.1

_______________________________________________
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Reply via email to