From a02c7de4d69c08ba30ece64783c5f83fbf9b8c8c Mon Sep 17 00:00:00 2001 From: Stuart Baker Date: Tue, 28 Mar 2023 14:58:17 +0200 Subject: [PATCH 01/27] Fix uninitialized variable warning. --- src/os/os.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/os/os.h b/src/os/os.h index fe608e145..c50be6263 100644 --- a/src/os/os.h +++ b/src/os/os.h @@ -667,7 +667,7 @@ OS_INLINE int os_sem_post(os_sem_t *sem) */ OS_INLINE int os_sem_post_from_isr(os_sem_t *sem, int *woken) { - portBASE_TYPE local_woken; + portBASE_TYPE local_woken = 0; xSemaphoreGiveFromISR(*sem, &local_woken); *woken |= local_woken; return 0; From 87da3df9432b43b28a83ab9ed10b01a3cb3a8482 Mon Sep 17 00:00:00 2001 From: Stuart Baker Date: Tue, 28 Mar 2023 16:03:32 +0200 Subject: [PATCH 02/27] Bit-banged I2C driver ready for testing. --- src/freertos_drivers/common/BitBangI2C.cxx | 341 +++++++++++++++++++++ src/freertos_drivers/common/BitBangI2C.hxx | 287 +++++++++++++++++ src/freertos_drivers/sources | 1 + 3 files changed, 629 insertions(+) create mode 100644 src/freertos_drivers/common/BitBangI2C.cxx create mode 100644 src/freertos_drivers/common/BitBangI2C.hxx diff --git a/src/freertos_drivers/common/BitBangI2C.cxx b/src/freertos_drivers/common/BitBangI2C.cxx new file mode 100644 index 000000000..dd0ec1a8e --- /dev/null +++ b/src/freertos_drivers/common/BitBangI2C.cxx @@ -0,0 +1,341 @@ +/** @copyright + * Copyright (c) 2023, Stuart W Baker + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * + * - Redistributions of source code must retain the above copyright notice, + * this list of conditions and the following disclaimer. + * + * - Redistributions in binary form must reproduce the above copyright notice, + * this list of conditions and the following disclaimer in the documentation + * and/or other materials provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE + * POSSIBILITY OF SUCH DAMAGE. + * + * @file BitBangI2C.hxx + * This file implements a bit bang implementation of I2C. + * + * @author Stuart W. Baker + * @date 28 March 2023 + */ + +#include "BitBangI2C.hxx" + +// +// BitBangI2C::tick_interrupt() +// +void BitBangI2C::tick_interrupt() +{ + bool exit = false; + switch (state_) + { + // start sequence + case State::START: + if (state_start()) + { + // start done, send the address + state_ = State::ADDRESS; + stateTx_ = StateTx::FIRST; + } + break; + case State::ADDRESS: + { + /// @todo only supporting 7-bit I2C addresses at the moment. + uint8_t address = msg_->addr << 1; + if (msg_->flags | I2C_M_RD) + { + address |= 0x1; + } + if (state_tx(address)) + { + if (count_ < 0) + { + // Some error occured, likely an unexpected NACK + exit = true; + } + else + { + if (msg_->flags | I2C_M_RD) + { + state_ = State::DATA_RX; + stateRx_ = StateRx::FIRST; + } + else + { + state_ = State::DATA_TX; + stateTx_ = StateTx::FIRST; + } + } + } + break; + } + case State::DATA_TX: + if (state_tx(msg_->buf[count_])) + { + if (count_ < 0) + { + // Some error occured, likely an unexpected NACK + exit = true; + } + else if (++count_ == msg_->len) + { + // All of the data has been transmitted. + if (stop_) + { + state_ = State::STOP; + stateStop_ = StateStop::FIRST; + } + else + { + exit = true; + } + } + } + break; + case State::DATA_RX: + if (state_rx(msg_->buf + count_, (count_ + 1 == msg_->len))) + { + if (count_ < 0) + { + // Some error occured, likely an unexpected NACK + exit = true; + } + else if (++count_ == msg_->len) + { + // All of the data has been received. + if (stop_) + { + state_ = State::STOP; + stateStop_ = StateStop::FIRST; + } + else + { + exit = true; + } + } + } + break; + case State::STOP: + exit = state_stop(); + break; + } + + if (exit) + { + disableTick_(); + int woken = 0; + sem_.post_from_isr(&woken); + os_isr_exit_yield_test(woken); + } +} + +// +// BitBangI2C::state_start() +// +bool BitBangI2C::state_start() +{ + switch (stateStart_) + { + // start sequence + case StateStart::SDA_SET: + gpio_set(sda_); + ++stateStart_; + break; + case StateStart::SCL_SET: + gpio_set(scl_); + ++stateStart_; + break; + case StateStart::SDA_CLR: + gpio_clr(sda_); + ++stateStart_; + break; + case StateStart::SCL_CLR: + gpio_clr(scl_); + return true; + } + return false; +} + +// +// BitBangI2C::state_stop() +// +bool BitBangI2C::state_stop() +{ + switch (stateStop_) + { + case StateStop::SDA_CLR: + gpio_clr(sda_); + ++stateStop_; + break; + case StateStop::SCL_SET: + gpio_set(scl_); + ++stateStop_; + break; + case StateStop::SDA_SET: + gpio_set(sda_); + stop_ = false; + return true; // exit + } + return false; +} + +// +// BitBangI2C::state_tx() +// +bool BitBangI2C::state_tx(uint8_t data) +{ + switch(stateTx_) + { + case StateTx::DATA_7_SCL_SET: + case StateTx::DATA_6_SCL_SET: + case StateTx::DATA_5_SCL_SET: + case StateTx::DATA_4_SCL_SET: + case StateTx::DATA_3_SCL_SET: + case StateTx::DATA_2_SCL_SET: + case StateTx::DATA_1_SCL_SET: + case StateTx::DATA_0_SCL_SET: + { + uint8_t mask = 0x80 >> + ((static_cast(stateTx_) - + static_cast(StateTx::DATA_7_SCL_SET)) >> 1); + if (data & mask) + { + gpio_set(sda_); + } + else + { + gpio_clr(sda_); + } + gpio_set(scl_); + ++stateTx_; + break; + } + case StateTx::DATA_7_SCL_CLR: + case StateTx::DATA_6_SCL_CLR: + case StateTx::DATA_5_SCL_CLR: + case StateTx::DATA_4_SCL_CLR: + case StateTx::DATA_3_SCL_CLR: + case StateTx::DATA_2_SCL_CLR: + case StateTx::DATA_1_SCL_CLR: + case StateTx::DATA_0_SCL_CLR: + gpio_clr(scl_); + ++stateTx_; + break; + case StateTx::ACK_SDA_SCL_SET: + gpio_set(sda_); + gpio_set(scl_); + ++stateTx_; + break; + case StateTx::ACK_SCL_CLR: + bool ack = (sda_->read() == Gpio::Value::CLR); + gpio_clr(scl_); + if (!ack) + { + count_ = -EIO; + } + return true; // done + } + return false; +} + +// +// BitBangI2C::state_rx() +bool BitBangI2C::state_rx(uint8_t *data, bool nack) +{ + switch(stateRx_) + { + case StateRx::DATA_7_SCL_SET: + gpio_set(sda_); // Always start with SDA high. + // fall through + case StateRx::DATA_6_SCL_SET: + case StateRx::DATA_5_SCL_SET: + case StateRx::DATA_4_SCL_SET: + case StateRx::DATA_3_SCL_SET: + case StateRx::DATA_2_SCL_SET: + case StateRx::DATA_1_SCL_SET: + case StateRx::DATA_0_SCL_SET: + { + gpio_set(scl_); + if (scl_->read() == Gpio::Value::SET) + { + // not clock stretching, move on. + ++stateRx_; + } + // else clock stretching, stay in this state + break; + } + case StateRx::DATA_7_SCL_CLR: + case StateRx::DATA_6_SCL_CLR: + case StateRx::DATA_5_SCL_CLR: + case StateRx::DATA_4_SCL_CLR: + case StateRx::DATA_3_SCL_CLR: + case StateRx::DATA_2_SCL_CLR: + case StateRx::DATA_1_SCL_CLR: + case StateRx::DATA_0_SCL_CLR: + *data <<= 1; + if (sda_->read() == Gpio::Value::SET) + { + *data |= 0x01; + } + gpio_clr(scl_); + ++stateRx_; + break; + case StateRx::ACK_SDA_SCL_SET: + if (nack) + { + // Send NACK. + gpio_set(sda_); + } + else + { + // Send ACK. + gpio_clr(sda_); + } + gpio_set(scl_); + ++stateRx_; + break; + case StateRx::ACK_SCL_CLR: + gpio_clr(scl_); + gpio_set(sda_); + return true; + } + return false; +} + +// +// BitBangI2C::transfer() +// +int BitBangI2C::transfer(struct i2c_msg *msg, bool stop) +{ + while (stop_) + { + // waiting for the initial "stop" condition on reset + sem_.wait(); + } + if (msg_->len == 0) + { + // Message must have length of at least 1. + return -EINVAL; + } + msg_ = msg; + count_ = 0; + stop_ = stop; + state_ = State::START; + stateStart_ = StateStart::FIRST; + enableTick_(); + sem_.wait(); + return count_; +} + diff --git a/src/freertos_drivers/common/BitBangI2C.hxx b/src/freertos_drivers/common/BitBangI2C.hxx new file mode 100644 index 000000000..edf1bd09d --- /dev/null +++ b/src/freertos_drivers/common/BitBangI2C.hxx @@ -0,0 +1,287 @@ +/** @copyright + * Copyright (c) 2023, Stuart W Baker + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * + * - Redistributions of source code must retain the above copyright notice, + * this list of conditions and the following disclaimer. + * + * - Redistributions in binary form must reproduce the above copyright notice, + * this list of conditions and the following disclaimer in the documentation + * and/or other materials provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE + * POSSIBILITY OF SUCH DAMAGE. + * + * @file BitBangI2C.hxx + * This file implements a bit bang implementation of I2C. + * + * @author Stuart W. Baker + * @date 28 March 2023 + */ + +#ifndef _FREERTOS_DRIVERS_COMMON_BITBANG_I2C_HXX_ +#define _FREERTOS_DRIVERS_COMMON_BITBANG_I2C_HXX_ + +#include "I2C.hxx" +#include "os/Gpio.hxx" +#include "os/OS.hxx" + +/// Implement a bit-banged I2C master. A periodic timer [interrupt] should call +/// the BitBangI2C::tick_interrupt() method at a rate that is one half the +/// desired clock rate. For example, for a 100kHz bus, call once every 5 +/// microseconds. The tick should be enabled to start. The driver will +/// enable/disable the tick as needed. +class BitBangI2C : public I2C +{ +public: + /// Constructor. + /// @param name name of this device instance in the file system + /// @param sda_gpio Gpio instance of the SDA line + /// @param scl_gpio Gpio instance of the SCL line + /// @param enable_tick callback to enable ticks + /// @param disable_tick callback to disable ticks + BitBangI2C(const char *name, const Gpio *sda_gpio, const Gpio *scl_gpio, + void (*enable_tick)(void), void (*disable_tick)(void)) + : I2C(name) + , sda_(sda_gpio) + , scl_(scl_gpio) + , enableTick_(enable_tick) + , disableTick_(disable_tick) + , msg_(nullptr) + , sem_() + , state_(State::STOP) // start-up with a stop sequence + , stateStop_(StateStop::SDA_CLR) + , count_(0) + , stop_(true) + { + gpio_set(sda_); + gpio_clr(scl_); + } + + /// Destructor. + ~BitBangI2C() + { + } + + /// Called at a periodic tick, when enabled. + void tick_interrupt(); + +private: + /// High level I2C States + enum class State + { + START, ///< start state + ADDRESS, ///< address state + DATA_TX, ///< data TX state + DATA_RX, ///< data TX state + STOP, ///< stop state + }; + + /// Low level I2C start states + enum class StateStart + { + SDA_SET, ///< start sequence + SCL_SET, ///< start sequence + SDA_CLR, ///< start sequence + SCL_CLR, ///< start sequence + FIRST = SDA_SET, ///< first start sequence state + LAST = SCL_CLR, /// last start sequence state + }; + + /// Low level I2C stop states + enum class StateStop + { + SDA_CLR, ///< stop sequence + SCL_SET, ///< stop sequence + SDA_SET, ///< stop sequence + FIRST = SDA_CLR, ///< first stop sequence state + LAST = SDA_SET, ///< last stop sequence state + }; + + /// Low level I2C data TX states + enum class StateTx + { + DATA_7_SCL_SET, ///< data TX sequence + DATA_7_SCL_CLR, ///< data TX sequence + DATA_6_SCL_SET, ///< data TX sequence + DATA_6_SCL_CLR, ///< data TX sequence + DATA_5_SCL_SET, ///< data TX sequence + DATA_5_SCL_CLR, ///< data TX sequence + DATA_4_SCL_SET, ///< data TX sequence + DATA_4_SCL_CLR, ///< data TX sequence + DATA_3_SCL_SET, ///< data TX sequence + DATA_3_SCL_CLR, ///< data TX sequence + DATA_2_SCL_SET, ///< data TX sequence + DATA_2_SCL_CLR, ///< data TX sequence + DATA_1_SCL_SET, ///< data TX sequence + DATA_1_SCL_CLR, ///< data TX sequence + DATA_0_SCL_SET, ///< data TX sequence + DATA_0_SCL_CLR, ///< data TX sequence + ACK_SDA_SCL_SET, ///< data TX sequence + ACK_SCL_CLR, ///< data TX sequence + FIRST = DATA_7_SCL_SET, ///< first data TX sequence state + LAST = ACK_SCL_CLR, ///< last data TX sequence state + }; + + /// Low level I2C data TX states + enum class StateRx + { + DATA_7_SCL_SET, ///< data RX sequence + DATA_7_SCL_CLR, ///< data RX sequence + DATA_6_SCL_SET, ///< data RX sequence + DATA_6_SCL_CLR, ///< data RX sequence + DATA_5_SCL_SET, ///< data RX sequence + DATA_5_SCL_CLR, ///< data RX sequence + DATA_4_SCL_SET, ///< data RX sequence + DATA_4_SCL_CLR, ///< data RX sequence + DATA_3_SCL_SET, ///< data RX sequence + DATA_3_SCL_CLR, ///< data RX sequence + DATA_2_SCL_SET, ///< data RX sequence + DATA_2_SCL_CLR, ///< data RX sequence + DATA_1_SCL_SET, ///< data RX sequence + DATA_1_SCL_CLR, ///< data RX sequence + DATA_0_SCL_SET, ///< data RX sequence + DATA_0_SCL_CLR, ///< data RX sequence + ACK_SDA_SCL_SET, ///< data RX sequence + ACK_SCL_CLR, ///< data RX sequence + FIRST = DATA_7_SCL_SET, ///< first data RX sequence state + LAST = ACK_SCL_CLR, ///< last data RX sequence state + }; + + /// Allow pre-increment definition + friend StateStart &operator++(StateStart &); + + /// Allow pre-increment definition + friend StateStop &operator++(StateStop &); + + /// Allow pre-increment definition + friend StateTx &operator++(StateTx &); + + /// Allow pre-increment definition + friend StateRx &operator++(StateRx &); + + /// Execute start state machine + /// @return true to exit the high level state machine + bool state_start(); + + /// Execute stop state machine + /// @return true to exit the high level state machine + bool state_stop(); + + /// Execute data TX state machine + /// @return true to exit the high level state machine + bool state_tx(); + + /// Execute data RX state machine + /// @return true to exit the high level state machine + bool state_rx(); + + /// Execute data TX state machine + /// @param data value to send + /// @return true if the state machine is finished, count_ may be negative + /// to indicate an error. + bool state_tx(uint8_t data); + + /// Execute data RX state machine + /// @param location to shift data into + /// @param nack send a NACK in the (N)ACK slot + /// @return true if the state machine is finished, count_ may be negative + /// to indicate an error. + bool state_rx(uint8_t *data, bool nack); + + /// Method to transmit/receive the data. + /// @param msg message to transact. + /// @param stop produce a stop condition at the end of the transfer + /// @return bytes transfered upon success or -1 with errno set + int transfer(struct i2c_msg *msg, bool stop) override; + + /// Set the GPIO state. + void gpio_set(const Gpio *gpio) + { + gpio->set_direction(Gpio::Direction::DINPUT); + } + + /// Clear the GPIO state. + void gpio_clr(const Gpio *gpio) + { + gpio->clr(); + gpio->set_direction(Gpio::Direction::DOUTPUT); + gpio->clr(); + } + + const Gpio *sda_; ///< GPIO for the SDA line + const Gpio *scl_; ///< GPIO for the SCL line + void (*enableTick_)(void); ///< Enable the timer tick + void (*disableTick_)(void); ///< Disable the timer tick + struct i2c_msg *msg_; ///< I2C message to presently act upon + OSSem sem_; ///< semaphore to wakeup task level from ISR + State state_; ///< state machine state + union + { + StateStart stateStart_; ///< I2C start state machine state + StateStop stateStop_; ///< I2C stop state machine state + StateTx stateTx_; ///< I2C data TX state machine state + StateRx stateRx_; ///< I2C data RX state machine state + }; + int count_; ///< the count of data bytes transferred + bool stop_; ///< if true, issue stop condition at the end of the message +}; + +/// Pre-increment operator overload +/// @param x starting value +inline BitBangI2C::StateStart &operator++(BitBangI2C::StateStart &x) +{ + if (x >= BitBangI2C::StateStart::FIRST && x <= BitBangI2C::StateStart::LAST) + { + x = static_cast(static_cast(x) + 1); + } + return x; +} + +/// Pre-increment operator overload +/// @param x starting value +inline BitBangI2C::StateStop &operator++(BitBangI2C::StateStop &x) +{ + if (x >= BitBangI2C::StateStop::FIRST && x <= BitBangI2C::StateStop::LAST) + { + x = static_cast(static_cast(x) + 1); + } + return x; +} + +/// Pre-increment operator overload +/// @param x starting value +inline BitBangI2C::StateTx &operator++(BitBangI2C::StateTx &x) +{ + if (x >= BitBangI2C::StateTx::FIRST && x <= BitBangI2C::StateTx::LAST) + { + x = static_cast(static_cast(x) + 1); + } + return x; +} + +/// Pre-increment operator overload +/// @param x starting value +inline BitBangI2C::StateRx &operator++(BitBangI2C::StateRx &x) +{ + if (x >= BitBangI2C::StateRx::FIRST && x <= BitBangI2C::StateRx::LAST) + { + x = static_cast(static_cast(x) + 1); + } + return x; +} + + +#endif // _FREERTOS_DRIVERS_COMMON_BITBANG_I2C_HXX_ diff --git a/src/freertos_drivers/sources b/src/freertos_drivers/sources index a570b39e6..cd6675107 100644 --- a/src/freertos_drivers/sources +++ b/src/freertos_drivers/sources @@ -28,6 +28,7 @@ CXXSRCS += Fileio.cxx \ SN74HC595GPO.cxx \ TCAN4550Can.cxx \ SPIFlash.cxx \ + BitBangI2C.cxx From 2de91833bef11d0917bd4f2002b950798897ac27 Mon Sep 17 00:00:00 2001 From: Stuart Baker Date: Tue, 28 Mar 2023 16:33:05 +0200 Subject: [PATCH 03/27] Fix compile/link errors. --- src/freertos_drivers/common/BitBangI2C.hxx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/freertos_drivers/common/BitBangI2C.hxx b/src/freertos_drivers/common/BitBangI2C.hxx index edf1bd09d..2f71a8752 100644 --- a/src/freertos_drivers/common/BitBangI2C.hxx +++ b/src/freertos_drivers/common/BitBangI2C.hxx @@ -53,7 +53,7 @@ public: /// @param enable_tick callback to enable ticks /// @param disable_tick callback to disable ticks BitBangI2C(const char *name, const Gpio *sda_gpio, const Gpio *scl_gpio, - void (*enable_tick)(void), void (*disable_tick)(void)) + void (*enable_tick)(), void (*disable_tick)()) : I2C(name) , sda_(sda_gpio) , scl_(scl_gpio) @@ -201,6 +201,9 @@ private: /// to indicate an error. bool state_rx(uint8_t *data, bool nack); + void enable() override {} /**< function to enable device */ + void disable() override {} /**< function to disable device */ + /// Method to transmit/receive the data. /// @param msg message to transact. /// @param stop produce a stop condition at the end of the transfer From 7939d9d195ec656ea6d176e2182bd1b38372e278 Mon Sep 17 00:00:00 2001 From: Stuart Baker Date: Tue, 28 Mar 2023 20:54:46 +0200 Subject: [PATCH 04/27] Fix I2C bit-bang state machine bugs. --- src/freertos_drivers/common/BitBangI2C.cxx | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/freertos_drivers/common/BitBangI2C.cxx b/src/freertos_drivers/common/BitBangI2C.cxx index dd0ec1a8e..eb0613b68 100644 --- a/src/freertos_drivers/common/BitBangI2C.cxx +++ b/src/freertos_drivers/common/BitBangI2C.cxx @@ -54,7 +54,7 @@ void BitBangI2C::tick_interrupt() { /// @todo only supporting 7-bit I2C addresses at the moment. uint8_t address = msg_->addr << 1; - if (msg_->flags | I2C_M_RD) + if (msg_->flags & I2C_M_RD) { address |= 0x1; } @@ -62,12 +62,14 @@ void BitBangI2C::tick_interrupt() { if (count_ < 0) { - // Some error occured, likely an unexpected NACK - exit = true; + // Some error occured, likely an unexpected NACK. Send a + // stop in order to shutdown gracefully. + state_ = State::STOP; + stateStop_ = StateStop::FIRST; } else { - if (msg_->flags | I2C_M_RD) + if (msg_->flags & I2C_M_RD) { state_ = State::DATA_RX; stateRx_ = StateRx::FIRST; @@ -86,8 +88,10 @@ void BitBangI2C::tick_interrupt() { if (count_ < 0) { - // Some error occured, likely an unexpected NACK - exit = true; + // Some error occured, likely an unexpected NACK. Send a + // stop in order to shutdown gracefully. + state_ = State::STOP; + stateStop_ = StateStop::FIRST; } else if (++count_ == msg_->len) { @@ -245,6 +249,7 @@ bool BitBangI2C::state_tx(uint8_t data) { count_ = -EIO; } + stateTx_ = StateTx::DATA_7_SCL_SET; return true; // done } return false; @@ -309,6 +314,7 @@ bool BitBangI2C::state_rx(uint8_t *data, bool nack) case StateRx::ACK_SCL_CLR: gpio_clr(scl_); gpio_set(sda_); + stateRx_ = StateRx::DATA_7_SCL_SET; return true; } return false; From 3a85474f834843ee480e01c2c2f08511180fe689 Mon Sep 17 00:00:00 2001 From: Stuart Baker Date: Tue, 28 Mar 2023 21:03:31 +0200 Subject: [PATCH 05/27] Final pre-review commentary tweaks. --- src/freertos_drivers/common/BitBangI2C.cxx | 3 +- src/freertos_drivers/common/BitBangI2C.hxx | 41 ++++++++++------------ 2 files changed, 21 insertions(+), 23 deletions(-) diff --git a/src/freertos_drivers/common/BitBangI2C.cxx b/src/freertos_drivers/common/BitBangI2C.cxx index eb0613b68..4a0aa026c 100644 --- a/src/freertos_drivers/common/BitBangI2C.cxx +++ b/src/freertos_drivers/common/BitBangI2C.cxx @@ -24,7 +24,7 @@ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE * POSSIBILITY OF SUCH DAMAGE. * - * @file BitBangI2C.hxx + * @file BitBangI2C.cxx * This file implements a bit bang implementation of I2C. * * @author Stuart W. Baker @@ -257,6 +257,7 @@ bool BitBangI2C::state_tx(uint8_t data) // // BitBangI2C::state_rx() +// bool BitBangI2C::state_rx(uint8_t *data, bool nack) { switch(stateRx_) diff --git a/src/freertos_drivers/common/BitBangI2C.hxx b/src/freertos_drivers/common/BitBangI2C.hxx index 2f71a8752..4b9046470 100644 --- a/src/freertos_drivers/common/BitBangI2C.hxx +++ b/src/freertos_drivers/common/BitBangI2C.hxx @@ -42,7 +42,7 @@ /// the BitBangI2C::tick_interrupt() method at a rate that is one half the /// desired clock rate. For example, for a 100kHz bus, call once every 5 /// microseconds. The tick should be enabled to start. The driver will -/// enable/disable the tick as needed. +/// enable/disable the tick as needed to save on spurious interrupts. class BitBangI2C : public I2C { public: @@ -85,7 +85,7 @@ private: START, ///< start state ADDRESS, ///< address state DATA_TX, ///< data TX state - DATA_RX, ///< data TX state + DATA_RX, ///< data RX state STOP, ///< stop state }; @@ -135,7 +135,7 @@ private: LAST = ACK_SCL_CLR, ///< last data TX sequence state }; - /// Low level I2C data TX states + /// Low level I2C data RX states enum class StateRx { DATA_7_SCL_SET, ///< data RX sequence @@ -172,33 +172,24 @@ private: /// Allow pre-increment definition friend StateRx &operator++(StateRx &); - /// Execute start state machine - /// @return true to exit the high level state machine + /// Execute start state machine. + /// @return true if the sub-state machine is finished. bool state_start(); - /// Execute stop state machine - /// @return true to exit the high level state machine + /// Execute stop state machine. + /// @return true if the sub-state machine is finished. bool state_stop(); - /// Execute data TX state machine - /// @return true to exit the high level state machine - bool state_tx(); - - /// Execute data RX state machine - /// @return true to exit the high level state machine - bool state_rx(); - - /// Execute data TX state machine + /// Execute data TX state machine. /// @param data value to send - /// @return true if the state machine is finished, count_ may be negative - /// to indicate an error. + /// @return true if the sub-state machine is finished, count_ may be + /// negative to indicate an error. bool state_tx(uint8_t data); - /// Execute data RX state machine + /// Execute data RX state machine. /// @param location to shift data into /// @param nack send a NACK in the (N)ACK slot - /// @return true if the state machine is finished, count_ may be negative - /// to indicate an error. + /// @return true if the sub-state machine is finished. bool state_rx(uint8_t *data, bool nack); void enable() override {} /**< function to enable device */ @@ -211,12 +202,14 @@ private: int transfer(struct i2c_msg *msg, bool stop) override; /// Set the GPIO state. + /// @param gpio GPIO to set void gpio_set(const Gpio *gpio) { gpio->set_direction(Gpio::Direction::DINPUT); } /// Clear the GPIO state. + /// @param gpio GPIO to clear void gpio_clr(const Gpio *gpio) { gpio->clr(); @@ -238,12 +231,13 @@ private: StateTx stateTx_; ///< I2C data TX state machine state StateRx stateRx_; ///< I2C data RX state machine state }; - int count_; ///< the count of data bytes transferred + int count_; ///< the count of data bytes transferred, error if < 0 bool stop_; ///< if true, issue stop condition at the end of the message }; /// Pre-increment operator overload /// @param x starting value +/// @return incremented value inline BitBangI2C::StateStart &operator++(BitBangI2C::StateStart &x) { if (x >= BitBangI2C::StateStart::FIRST && x <= BitBangI2C::StateStart::LAST) @@ -255,6 +249,7 @@ inline BitBangI2C::StateStart &operator++(BitBangI2C::StateStart &x) /// Pre-increment operator overload /// @param x starting value +/// @return incremented value inline BitBangI2C::StateStop &operator++(BitBangI2C::StateStop &x) { if (x >= BitBangI2C::StateStop::FIRST && x <= BitBangI2C::StateStop::LAST) @@ -266,6 +261,7 @@ inline BitBangI2C::StateStop &operator++(BitBangI2C::StateStop &x) /// Pre-increment operator overload /// @param x starting value +/// @return incremented value inline BitBangI2C::StateTx &operator++(BitBangI2C::StateTx &x) { if (x >= BitBangI2C::StateTx::FIRST && x <= BitBangI2C::StateTx::LAST) @@ -277,6 +273,7 @@ inline BitBangI2C::StateTx &operator++(BitBangI2C::StateTx &x) /// Pre-increment operator overload /// @param x starting value +/// @return incremented value inline BitBangI2C::StateRx &operator++(BitBangI2C::StateRx &x) { if (x >= BitBangI2C::StateRx::FIRST && x <= BitBangI2C::StateRx::LAST) From ae1f0e83a42cbad222dcd248332315314f7a9c27 Mon Sep 17 00:00:00 2001 From: Stuart Baker Date: Tue, 28 Mar 2023 21:54:55 +0200 Subject: [PATCH 06/27] Force optimize -03 for I2C bit-bang interrupt handler. --- src/freertos_drivers/common/BitBangI2C.cxx | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/freertos_drivers/common/BitBangI2C.cxx b/src/freertos_drivers/common/BitBangI2C.cxx index 4a0aa026c..c7fd0f3ca 100644 --- a/src/freertos_drivers/common/BitBangI2C.cxx +++ b/src/freertos_drivers/common/BitBangI2C.cxx @@ -36,6 +36,7 @@ // // BitBangI2C::tick_interrupt() // +__attribute__((optimize("-O3"))) void BitBangI2C::tick_interrupt() { bool exit = false; @@ -148,6 +149,7 @@ void BitBangI2C::tick_interrupt() // // BitBangI2C::state_start() // +__attribute__((optimize("-O3"))) bool BitBangI2C::state_start() { switch (stateStart_) @@ -175,6 +177,7 @@ bool BitBangI2C::state_start() // // BitBangI2C::state_stop() // +__attribute__((optimize("-O3"))) bool BitBangI2C::state_stop() { switch (stateStop_) @@ -198,6 +201,7 @@ bool BitBangI2C::state_stop() // // BitBangI2C::state_tx() // +__attribute__((optimize("-O3"))) bool BitBangI2C::state_tx(uint8_t data) { switch(stateTx_) @@ -258,6 +262,7 @@ bool BitBangI2C::state_tx(uint8_t data) // // BitBangI2C::state_rx() // +__attribute__((optimize("-O3"))) bool BitBangI2C::state_rx(uint8_t *data, bool nack) { switch(stateRx_) From 9043718fa57c0b2d0d03ac252669dd46c14f6790 Mon Sep 17 00:00:00 2001 From: Stuart Baker Date: Sun, 30 Apr 2023 17:29:41 -0500 Subject: [PATCH 07/27] Refactor state_tx() based on review feedback. --- src/freertos_drivers/common/BitBangI2C.cxx | 79 ++++++++++++++++------ src/freertos_drivers/common/BitBangI2C.hxx | 23 ++++--- 2 files changed, 70 insertions(+), 32 deletions(-) diff --git a/src/freertos_drivers/common/BitBangI2C.cxx b/src/freertos_drivers/common/BitBangI2C.cxx index c7fd0f3ca..1c62576b0 100644 --- a/src/freertos_drivers/common/BitBangI2C.cxx +++ b/src/freertos_drivers/common/BitBangI2C.cxx @@ -94,7 +94,7 @@ void BitBangI2C::tick_interrupt() state_ = State::STOP; stateStop_ = StateStop::FIRST; } - else if (++count_ == msg_->len) + else if (++count_ >= msg_->len) { // All of the data has been transmitted. if (stop_) @@ -110,14 +110,14 @@ void BitBangI2C::tick_interrupt() } break; case State::DATA_RX: - if (state_rx(msg_->buf + count_, (count_ + 1 == msg_->len))) + if (state_rx(msg_->buf + count_, (count_ + 1 >= msg_->len))) { if (count_ < 0) { // Some error occured, likely an unexpected NACK exit = true; } - else if (++count_ == msg_->len) + else if (++count_ >= msg_->len) { // All of the data has been received. if (stop_) @@ -204,17 +204,33 @@ bool BitBangI2C::state_stop() __attribute__((optimize("-O3"))) bool BitBangI2C::state_tx(uint8_t data) { + // I2C is specified such that the data on the SDA line must be stable + // during the high period of the clock, and the data line can only change + // when SCL is Low. + // + // This means that the sequence always has to be + // 1a. SCL := low + // 1b. set/clear SDA + // 2. wait a cycle for lines to settle + // 3a. SCL := high + // 3b. this edge is when the receiver samples + // 4. wait a cycle, lines are stable switch(stateTx_) { - case StateTx::DATA_7_SCL_SET: - case StateTx::DATA_6_SCL_SET: - case StateTx::DATA_5_SCL_SET: - case StateTx::DATA_4_SCL_SET: - case StateTx::DATA_3_SCL_SET: - case StateTx::DATA_2_SCL_SET: - case StateTx::DATA_1_SCL_SET: - case StateTx::DATA_0_SCL_SET: + case StateTx::DATA_7_SCL_CLR: + case StateTx::DATA_6_SCL_CLR: + case StateTx::DATA_5_SCL_CLR: + case StateTx::DATA_4_SCL_CLR: + case StateTx::DATA_3_SCL_CLR: + case StateTx::DATA_2_SCL_CLR: + case StateTx::DATA_1_SCL_CLR: + case StateTx::DATA_0_SCL_CLR: { + // We can only change the data when SCL is low. + gpio_clr(scl_); + // The divide by 2 factor (shift right by 1) is because the enum + // states alternate between *_SCL_SET and *_SCL_CLR states in + // increasing value. uint8_t mask = 0x80 >> ((static_cast(stateTx_) - static_cast(StateTx::DATA_7_SCL_SET)) >> 1); @@ -226,27 +242,46 @@ bool BitBangI2C::state_tx(uint8_t data) { gpio_clr(sda_); } - gpio_set(scl_); ++stateTx_; break; } - case StateTx::DATA_7_SCL_CLR: - case StateTx::DATA_6_SCL_CLR: - case StateTx::DATA_5_SCL_CLR: - case StateTx::DATA_4_SCL_CLR: - case StateTx::DATA_3_SCL_CLR: - case StateTx::DATA_2_SCL_CLR: - case StateTx::DATA_1_SCL_CLR: - case StateTx::DATA_0_SCL_CLR: - gpio_clr(scl_); + case StateTx::DATA_7_SCL_SET: + case StateTx::DATA_6_SCL_SET: + case StateTx::DATA_5_SCL_SET: + case StateTx::DATA_4_SCL_SET: + case StateTx::DATA_3_SCL_SET: + case StateTx::DATA_2_SCL_SET: + case StateTx::DATA_1_SCL_SET: + case StateTx::DATA_0_SCL_SET: + // Data is sampled by the slave following this state transition. + gpio_set(scl_); ++stateTx_; break; - case StateTx::ACK_SDA_SCL_SET: + case StateTx::ACK_SDA_SET_SCL_CLR: + gpio_clr(scl_); gpio_set(sda_); + ++stateTx_; + break; + case StateTx::ACK_SCL_SET: gpio_set(scl_); ++stateTx_; break; case StateTx::ACK_SCL_CLR: + if (scl_->read() == Gpio::Value::CLR) + { + // Clock stretching, do the same state again. + clockStretchActive_ = true; + break; + } + if (clockStretchActive_) + { + // I2C spec requires minimum 4 microseconds after the SCL line + // goes high following a clock stretch for the SCL line to be + // pulled low again by the master. Do the same state one more + // time to ensure this. + clockStretchActive_ = false; + break; + } bool ack = (sda_->read() == Gpio::Value::CLR); gpio_clr(scl_); if (!ack) diff --git a/src/freertos_drivers/common/BitBangI2C.hxx b/src/freertos_drivers/common/BitBangI2C.hxx index 4b9046470..04f267d97 100644 --- a/src/freertos_drivers/common/BitBangI2C.hxx +++ b/src/freertos_drivers/common/BitBangI2C.hxx @@ -65,6 +65,7 @@ public: , stateStop_(StateStop::SDA_CLR) , count_(0) , stop_(true) + , clockStretchActive_(false) { gpio_set(sda_); gpio_clr(scl_); @@ -113,25 +114,26 @@ private: /// Low level I2C data TX states enum class StateTx { - DATA_7_SCL_SET, ///< data TX sequence DATA_7_SCL_CLR, ///< data TX sequence - DATA_6_SCL_SET, ///< data TX sequence + DATA_7_SCL_SET, ///< data TX sequence DATA_6_SCL_CLR, ///< data TX sequence - DATA_5_SCL_SET, ///< data TX sequence + DATA_6_SCL_SET, ///< data TX sequence DATA_5_SCL_CLR, ///< data TX sequence - DATA_4_SCL_SET, ///< data TX sequence + DATA_5_SCL_SET, ///< data TX sequence DATA_4_SCL_CLR, ///< data TX sequence - DATA_3_SCL_SET, ///< data TX sequence + DATA_4_SCL_SET, ///< data TX sequence DATA_3_SCL_CLR, ///< data TX sequence - DATA_2_SCL_SET, ///< data TX sequence + DATA_3_SCL_SET, ///< data TX sequence DATA_2_SCL_CLR, ///< data TX sequence - DATA_1_SCL_SET, ///< data TX sequence + DATA_2_SCL_SET, ///< data TX sequence DATA_1_SCL_CLR, ///< data TX sequence - DATA_0_SCL_SET, ///< data TX sequence + DATA_1_SCL_SET, ///< data TX sequence DATA_0_SCL_CLR, ///< data TX sequence - ACK_SDA_SCL_SET, ///< data TX sequence + DATA_0_SCL_SET, ///< data TX sequence + ACK_SDA_SET_SCL_CLR, ///< data TX sequence + ACK_SCL_SET, ///< data TX sequence ACK_SCL_CLR, ///< data TX sequence - FIRST = DATA_7_SCL_SET, ///< first data TX sequence state + FIRST = DATA_7_SCL_CLR, ///< first data TX sequence state LAST = ACK_SCL_CLR, ///< last data TX sequence state }; @@ -233,6 +235,7 @@ private: }; int count_; ///< the count of data bytes transferred, error if < 0 bool stop_; ///< if true, issue stop condition at the end of the message + bool clockStretchActive_; ///< true if the slave is clock stretching. }; /// Pre-increment operator overload From a112a78faed9de100002f07a5432d6fd7aac80bf Mon Sep 17 00:00:00 2001 From: Stuart Baker Date: Sun, 30 Apr 2023 17:50:42 -0500 Subject: [PATCH 08/27] state_start() and state_tx() tweaks. --- src/freertos_drivers/common/BitBangI2C.cxx | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/freertos_drivers/common/BitBangI2C.cxx b/src/freertos_drivers/common/BitBangI2C.cxx index 1c62576b0..ba5e890da 100644 --- a/src/freertos_drivers/common/BitBangI2C.cxx +++ b/src/freertos_drivers/common/BitBangI2C.cxx @@ -166,9 +166,6 @@ bool BitBangI2C::state_start() case StateStart::SDA_CLR: gpio_clr(sda_); ++stateStart_; - break; - case StateStart::SCL_CLR: - gpio_clr(scl_); return true; } return false; @@ -233,7 +230,7 @@ bool BitBangI2C::state_tx(uint8_t data) // increasing value. uint8_t mask = 0x80 >> ((static_cast(stateTx_) - - static_cast(StateTx::DATA_7_SCL_SET)) >> 1); + static_cast(StateTx::DATA_7_SCL_CLR)) >> 1); if (data & mask) { gpio_set(sda_); @@ -288,7 +285,7 @@ bool BitBangI2C::state_tx(uint8_t data) { count_ = -EIO; } - stateTx_ = StateTx::DATA_7_SCL_SET; + stateTx_ = StateTx::DATA_7_SCL_CLR; return true; // done } return false; From acd931a885215ddf0541f79bf2b1b706540a3b58 Mon Sep 17 00:00:00 2001 From: Stuart Baker Date: Sun, 30 Apr 2023 17:53:05 -0500 Subject: [PATCH 09/27] Fix build error. --- src/freertos_drivers/common/BitBangI2C.hxx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/freertos_drivers/common/BitBangI2C.hxx b/src/freertos_drivers/common/BitBangI2C.hxx index 04f267d97..649d091aa 100644 --- a/src/freertos_drivers/common/BitBangI2C.hxx +++ b/src/freertos_drivers/common/BitBangI2C.hxx @@ -96,9 +96,8 @@ private: SDA_SET, ///< start sequence SCL_SET, ///< start sequence SDA_CLR, ///< start sequence - SCL_CLR, ///< start sequence FIRST = SDA_SET, ///< first start sequence state - LAST = SCL_CLR, /// last start sequence state + LAST = SDA_CLR, /// last start sequence state }; /// Low level I2C stop states From f58307d23cb01df009f203a67a94ca6c543eda04 Mon Sep 17 00:00:00 2001 From: Stuart Baker Date: Sun, 30 Apr 2023 18:38:11 -0500 Subject: [PATCH 10/27] Move state_tx() reset to the caller. --- src/freertos_drivers/common/BitBangI2C.cxx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/freertos_drivers/common/BitBangI2C.cxx b/src/freertos_drivers/common/BitBangI2C.cxx index ba5e890da..60a88075a 100644 --- a/src/freertos_drivers/common/BitBangI2C.cxx +++ b/src/freertos_drivers/common/BitBangI2C.cxx @@ -107,6 +107,11 @@ void BitBangI2C::tick_interrupt() exit = true; } } + else + { + // Setup the next byte TX + stateTx_ = StateTx::FIRST; + } } break; case State::DATA_RX: @@ -285,7 +290,6 @@ bool BitBangI2C::state_tx(uint8_t data) { count_ = -EIO; } - stateTx_ = StateTx::DATA_7_SCL_CLR; return true; // done } return false; From 4c1a51e4b5ae971e5447d1854e279cf8e4bf83f9 Mon Sep 17 00:00:00 2001 From: Stuart Baker Date: Sun, 30 Apr 2023 18:40:10 -0500 Subject: [PATCH 11/27] Type the enum class objects. --- src/freertos_drivers/common/BitBangI2C.hxx | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/freertos_drivers/common/BitBangI2C.hxx b/src/freertos_drivers/common/BitBangI2C.hxx index 649d091aa..be908b107 100644 --- a/src/freertos_drivers/common/BitBangI2C.hxx +++ b/src/freertos_drivers/common/BitBangI2C.hxx @@ -81,7 +81,7 @@ public: private: /// High level I2C States - enum class State + enum class State : uint8_t { START, ///< start state ADDRESS, ///< address state @@ -91,7 +91,7 @@ private: }; /// Low level I2C start states - enum class StateStart + enum class StateStart : uint8_t { SDA_SET, ///< start sequence SCL_SET, ///< start sequence @@ -101,7 +101,7 @@ private: }; /// Low level I2C stop states - enum class StateStop + enum class StateStop : uint8_t { SDA_CLR, ///< stop sequence SCL_SET, ///< stop sequence @@ -111,7 +111,7 @@ private: }; /// Low level I2C data TX states - enum class StateTx + enum class StateTx : uint8_t { DATA_7_SCL_CLR, ///< data TX sequence DATA_7_SCL_SET, ///< data TX sequence @@ -137,7 +137,7 @@ private: }; /// Low level I2C data RX states - enum class StateRx + enum class StateRx : uint8_t { DATA_7_SCL_SET, ///< data RX sequence DATA_7_SCL_CLR, ///< data RX sequence From 65c63ce7f82e797d832a96da0d8233884e31550d Mon Sep 17 00:00:00 2001 From: Stuart Baker Date: Sun, 30 Apr 2023 18:46:42 -0500 Subject: [PATCH 12/27] Additional review fixes. --- src/freertos_drivers/common/BitBangI2C.hxx | 30 ++++++++++++---------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/src/freertos_drivers/common/BitBangI2C.hxx b/src/freertos_drivers/common/BitBangI2C.hxx index be908b107..a5f11d306 100644 --- a/src/freertos_drivers/common/BitBangI2C.hxx +++ b/src/freertos_drivers/common/BitBangI2C.hxx @@ -61,9 +61,9 @@ public: , disableTick_(disable_tick) , msg_(nullptr) , sem_() + , count_(0) , state_(State::STOP) // start-up with a stop sequence , stateStop_(StateStop::SDA_CLR) - , count_(0) , stop_(true) , clockStretchActive_(false) { @@ -173,24 +173,28 @@ private: /// Allow pre-increment definition friend StateRx &operator++(StateRx &); - /// Execute start state machine. - /// @return true if the sub-state machine is finished. + /// Execute state machine for sending start condition. + /// @return true if the sub-state machine is finished, after doing the work + /// required by the current tick. bool state_start(); - /// Execute stop state machine. - /// @return true if the sub-state machine is finished. + /// Execute state machine for sending the stop condition. + /// @return true if the sub-state machine is finished, after doing the work + /// required by the current tick. bool state_stop(); /// Execute data TX state machine. /// @param data value to send - /// @return true if the sub-state machine is finished, count_ may be - /// negative to indicate an error. + /// @return true if the sub-state machine is finished, after doing the work + /// required by the current tick. count_ may be negative to + /// indicate an error. bool state_tx(uint8_t data); /// Execute data RX state machine. /// @param location to shift data into /// @param nack send a NACK in the (N)ACK slot - /// @return true if the sub-state machine is finished. + /// @return true if the sub-state machine is finished, after doing the work + /// required by the current tick. bool state_rx(uint8_t *data, bool nack); void enable() override {} /**< function to enable device */ @@ -224,6 +228,7 @@ private: void (*disableTick_)(void); ///< Disable the timer tick struct i2c_msg *msg_; ///< I2C message to presently act upon OSSem sem_; ///< semaphore to wakeup task level from ISR + int count_; ///< the count of data bytes transferred, error if < 0 State state_; ///< state machine state union { @@ -232,7 +237,6 @@ private: StateTx stateTx_; ///< I2C data TX state machine state StateRx stateRx_; ///< I2C data RX state machine state }; - int count_; ///< the count of data bytes transferred, error if < 0 bool stop_; ///< if true, issue stop condition at the end of the message bool clockStretchActive_; ///< true if the slave is clock stretching. }; @@ -242,7 +246,7 @@ private: /// @return incremented value inline BitBangI2C::StateStart &operator++(BitBangI2C::StateStart &x) { - if (x >= BitBangI2C::StateStart::FIRST && x <= BitBangI2C::StateStart::LAST) + if (x >= BitBangI2C::StateStart::FIRST && x < BitBangI2C::StateStart::LAST) { x = static_cast(static_cast(x) + 1); } @@ -254,7 +258,7 @@ inline BitBangI2C::StateStart &operator++(BitBangI2C::StateStart &x) /// @return incremented value inline BitBangI2C::StateStop &operator++(BitBangI2C::StateStop &x) { - if (x >= BitBangI2C::StateStop::FIRST && x <= BitBangI2C::StateStop::LAST) + if (x >= BitBangI2C::StateStop::FIRST && x < BitBangI2C::StateStop::LAST) { x = static_cast(static_cast(x) + 1); } @@ -266,7 +270,7 @@ inline BitBangI2C::StateStop &operator++(BitBangI2C::StateStop &x) /// @return incremented value inline BitBangI2C::StateTx &operator++(BitBangI2C::StateTx &x) { - if (x >= BitBangI2C::StateTx::FIRST && x <= BitBangI2C::StateTx::LAST) + if (x >= BitBangI2C::StateTx::FIRST && x < BitBangI2C::StateTx::LAST) { x = static_cast(static_cast(x) + 1); } @@ -278,7 +282,7 @@ inline BitBangI2C::StateTx &operator++(BitBangI2C::StateTx &x) /// @return incremented value inline BitBangI2C::StateRx &operator++(BitBangI2C::StateRx &x) { - if (x >= BitBangI2C::StateRx::FIRST && x <= BitBangI2C::StateRx::LAST) + if (x >= BitBangI2C::StateRx::FIRST && x < BitBangI2C::StateRx::LAST) { x = static_cast(static_cast(x) + 1); } From bf71f5d8ac7b4bc5769706f7e01bb679e3f6567b Mon Sep 17 00:00:00 2001 From: Stuart Baker Date: Sun, 30 Apr 2023 19:08:43 -0500 Subject: [PATCH 13/27] Refactor the state_rx() based on review feedback. --- src/freertos_drivers/common/BitBangI2C.cxx | 49 +++++++++++++++------- 1 file changed, 34 insertions(+), 15 deletions(-) diff --git a/src/freertos_drivers/common/BitBangI2C.cxx b/src/freertos_drivers/common/BitBangI2C.cxx index 60a88075a..1850067e3 100644 --- a/src/freertos_drivers/common/BitBangI2C.cxx +++ b/src/freertos_drivers/common/BitBangI2C.cxx @@ -315,12 +315,6 @@ bool BitBangI2C::state_rx(uint8_t *data, bool nack) case StateRx::DATA_0_SCL_SET: { gpio_set(scl_); - if (scl_->read() == Gpio::Value::SET) - { - // not clock stretching, move on. - ++stateRx_; - } - // else clock stretching, stay in this state break; } case StateRx::DATA_7_SCL_CLR: @@ -331,6 +325,21 @@ bool BitBangI2C::state_rx(uint8_t *data, bool nack) case StateRx::DATA_2_SCL_CLR: case StateRx::DATA_1_SCL_CLR: case StateRx::DATA_0_SCL_CLR: + if (scl_->read() == Gpio::Value::CLR) + { + // Clock stretching, do the same state again. + clockStretchActive_ = true; + break; + } + if (clockStretchActive_) + { + // I2C spec requires minimum 4 microseconds after the SCL line + // goes high following a clock stretch for the SCL line to be + // pulled low again by the master or for the master to sample + // the SDA data. Do the same state one more time to ensure this. + clockStretchActive_ = false; + break; + } *data <<= 1; if (sda_->read() == Gpio::Value::SET) { @@ -338,22 +347,32 @@ bool BitBangI2C::state_rx(uint8_t *data, bool nack) } gpio_clr(scl_); ++stateRx_; - break; - case StateRx::ACK_SDA_SCL_SET: - if (nack) - { - // Send NACK. - gpio_set(sda_); - } - else + if (stateRx_ == StateRx::DATA_0_SCL_CLR && !nack) { - // Send ACK. + // Send the ACK. If a NACK, SDA is already set. gpio_clr(sda_); } + break; + case StateRx::ACK_SDA_SCL_SET: gpio_set(scl_); ++stateRx_; break; case StateRx::ACK_SCL_CLR: + if (scl_->read() == Gpio::Value::CLR) + { + // Clock stretching, do the same state again. + clockStretchActive_ = true; + break; + } + if (clockStretchActive_) + { + // I2C spec requires minimum 4 microseconds after the SCL line + // goes high following a clock stretch for the SCL line to be + // pulled low again by the master. Do the same state one more + // time to ensure this. + clockStretchActive_ = false; + break; + } gpio_clr(scl_); gpio_set(sda_); stateRx_ = StateRx::DATA_7_SCL_SET; From 1a25cf1dc92d4e332608996f71d0b95408c0d434 Mon Sep 17 00:00:00 2001 From: Stuart Baker Date: Sun, 30 Apr 2023 19:21:45 -0500 Subject: [PATCH 14/27] Fix state_rx() stateRx_ math bug. --- src/freertos_drivers/common/BitBangI2C.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/freertos_drivers/common/BitBangI2C.cxx b/src/freertos_drivers/common/BitBangI2C.cxx index 1850067e3..e261c6fc5 100644 --- a/src/freertos_drivers/common/BitBangI2C.cxx +++ b/src/freertos_drivers/common/BitBangI2C.cxx @@ -346,12 +346,12 @@ bool BitBangI2C::state_rx(uint8_t *data, bool nack) *data |= 0x01; } gpio_clr(scl_); - ++stateRx_; if (stateRx_ == StateRx::DATA_0_SCL_CLR && !nack) { // Send the ACK. If a NACK, SDA is already set. gpio_clr(sda_); } + ++stateRx_; break; case StateRx::ACK_SDA_SCL_SET: gpio_set(scl_); From 2f0177d25c656b29a58c56ae86ca6b2ba34aa57d Mon Sep 17 00:00:00 2001 From: Stuart Baker Date: Sun, 30 Apr 2023 19:23:54 -0500 Subject: [PATCH 15/27] Move the state_rx reseet to the caller. --- src/freertos_drivers/common/BitBangI2C.cxx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/freertos_drivers/common/BitBangI2C.cxx b/src/freertos_drivers/common/BitBangI2C.cxx index e261c6fc5..e72c94d51 100644 --- a/src/freertos_drivers/common/BitBangI2C.cxx +++ b/src/freertos_drivers/common/BitBangI2C.cxx @@ -135,6 +135,11 @@ void BitBangI2C::tick_interrupt() exit = true; } } + else + { + // Setup the next byte RX + stateRx_ = StateRx::FIRST; + } } break; case State::STOP: @@ -375,7 +380,6 @@ bool BitBangI2C::state_rx(uint8_t *data, bool nack) } gpio_clr(scl_); gpio_set(sda_); - stateRx_ = StateRx::DATA_7_SCL_SET; return true; } return false; From 2f7e11811800497bf77467bed05d16d8ea0d5675 Mon Sep 17 00:00:00 2001 From: Stuart Baker Date: Sun, 30 Apr 2023 20:10:16 -0500 Subject: [PATCH 16/27] Fix state_rx stateRx_ advancement bug. --- src/freertos_drivers/common/BitBangI2C.cxx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/freertos_drivers/common/BitBangI2C.cxx b/src/freertos_drivers/common/BitBangI2C.cxx index e72c94d51..134be43ce 100644 --- a/src/freertos_drivers/common/BitBangI2C.cxx +++ b/src/freertos_drivers/common/BitBangI2C.cxx @@ -318,10 +318,9 @@ bool BitBangI2C::state_rx(uint8_t *data, bool nack) case StateRx::DATA_2_SCL_SET: case StateRx::DATA_1_SCL_SET: case StateRx::DATA_0_SCL_SET: - { gpio_set(scl_); + ++stateRx_; break; - } case StateRx::DATA_7_SCL_CLR: case StateRx::DATA_6_SCL_CLR: case StateRx::DATA_5_SCL_CLR: From a1c1ca2553e135efeaefafa0ca5c30ee7bb80d36 Mon Sep 17 00:00:00 2001 From: Stuart Baker Date: Sun, 30 Apr 2023 20:49:22 -0500 Subject: [PATCH 17/27] Move function implementations over to the header (inline). --- src/freertos_drivers/common/BitBangI2C.cxx | 375 -------------------- src/freertos_drivers/common/BitBangI2C.hxx | 388 ++++++++++++++++++++- 2 files changed, 382 insertions(+), 381 deletions(-) diff --git a/src/freertos_drivers/common/BitBangI2C.cxx b/src/freertos_drivers/common/BitBangI2C.cxx index 134be43ce..27d5f6e37 100644 --- a/src/freertos_drivers/common/BitBangI2C.cxx +++ b/src/freertos_drivers/common/BitBangI2C.cxx @@ -33,379 +33,4 @@ #include "BitBangI2C.hxx" -// -// BitBangI2C::tick_interrupt() -// -__attribute__((optimize("-O3"))) -void BitBangI2C::tick_interrupt() -{ - bool exit = false; - switch (state_) - { - // start sequence - case State::START: - if (state_start()) - { - // start done, send the address - state_ = State::ADDRESS; - stateTx_ = StateTx::FIRST; - } - break; - case State::ADDRESS: - { - /// @todo only supporting 7-bit I2C addresses at the moment. - uint8_t address = msg_->addr << 1; - if (msg_->flags & I2C_M_RD) - { - address |= 0x1; - } - if (state_tx(address)) - { - if (count_ < 0) - { - // Some error occured, likely an unexpected NACK. Send a - // stop in order to shutdown gracefully. - state_ = State::STOP; - stateStop_ = StateStop::FIRST; - } - else - { - if (msg_->flags & I2C_M_RD) - { - state_ = State::DATA_RX; - stateRx_ = StateRx::FIRST; - } - else - { - state_ = State::DATA_TX; - stateTx_ = StateTx::FIRST; - } - } - } - break; - } - case State::DATA_TX: - if (state_tx(msg_->buf[count_])) - { - if (count_ < 0) - { - // Some error occured, likely an unexpected NACK. Send a - // stop in order to shutdown gracefully. - state_ = State::STOP; - stateStop_ = StateStop::FIRST; - } - else if (++count_ >= msg_->len) - { - // All of the data has been transmitted. - if (stop_) - { - state_ = State::STOP; - stateStop_ = StateStop::FIRST; - } - else - { - exit = true; - } - } - else - { - // Setup the next byte TX - stateTx_ = StateTx::FIRST; - } - } - break; - case State::DATA_RX: - if (state_rx(msg_->buf + count_, (count_ + 1 >= msg_->len))) - { - if (count_ < 0) - { - // Some error occured, likely an unexpected NACK - exit = true; - } - else if (++count_ >= msg_->len) - { - // All of the data has been received. - if (stop_) - { - state_ = State::STOP; - stateStop_ = StateStop::FIRST; - } - else - { - exit = true; - } - } - else - { - // Setup the next byte RX - stateRx_ = StateRx::FIRST; - } - } - break; - case State::STOP: - exit = state_stop(); - break; - } - - if (exit) - { - disableTick_(); - int woken = 0; - sem_.post_from_isr(&woken); - os_isr_exit_yield_test(woken); - } -} - -// -// BitBangI2C::state_start() -// -__attribute__((optimize("-O3"))) -bool BitBangI2C::state_start() -{ - switch (stateStart_) - { - // start sequence - case StateStart::SDA_SET: - gpio_set(sda_); - ++stateStart_; - break; - case StateStart::SCL_SET: - gpio_set(scl_); - ++stateStart_; - break; - case StateStart::SDA_CLR: - gpio_clr(sda_); - ++stateStart_; - return true; - } - return false; -} - -// -// BitBangI2C::state_stop() -// -__attribute__((optimize("-O3"))) -bool BitBangI2C::state_stop() -{ - switch (stateStop_) - { - case StateStop::SDA_CLR: - gpio_clr(sda_); - ++stateStop_; - break; - case StateStop::SCL_SET: - gpio_set(scl_); - ++stateStop_; - break; - case StateStop::SDA_SET: - gpio_set(sda_); - stop_ = false; - return true; // exit - } - return false; -} - -// -// BitBangI2C::state_tx() -// -__attribute__((optimize("-O3"))) -bool BitBangI2C::state_tx(uint8_t data) -{ - // I2C is specified such that the data on the SDA line must be stable - // during the high period of the clock, and the data line can only change - // when SCL is Low. - // - // This means that the sequence always has to be - // 1a. SCL := low - // 1b. set/clear SDA - // 2. wait a cycle for lines to settle - // 3a. SCL := high - // 3b. this edge is when the receiver samples - // 4. wait a cycle, lines are stable - switch(stateTx_) - { - case StateTx::DATA_7_SCL_CLR: - case StateTx::DATA_6_SCL_CLR: - case StateTx::DATA_5_SCL_CLR: - case StateTx::DATA_4_SCL_CLR: - case StateTx::DATA_3_SCL_CLR: - case StateTx::DATA_2_SCL_CLR: - case StateTx::DATA_1_SCL_CLR: - case StateTx::DATA_0_SCL_CLR: - { - // We can only change the data when SCL is low. - gpio_clr(scl_); - // The divide by 2 factor (shift right by 1) is because the enum - // states alternate between *_SCL_SET and *_SCL_CLR states in - // increasing value. - uint8_t mask = 0x80 >> - ((static_cast(stateTx_) - - static_cast(StateTx::DATA_7_SCL_CLR)) >> 1); - if (data & mask) - { - gpio_set(sda_); - } - else - { - gpio_clr(sda_); - } - ++stateTx_; - break; - } - case StateTx::DATA_7_SCL_SET: - case StateTx::DATA_6_SCL_SET: - case StateTx::DATA_5_SCL_SET: - case StateTx::DATA_4_SCL_SET: - case StateTx::DATA_3_SCL_SET: - case StateTx::DATA_2_SCL_SET: - case StateTx::DATA_1_SCL_SET: - case StateTx::DATA_0_SCL_SET: - // Data is sampled by the slave following this state transition. - gpio_set(scl_); - ++stateTx_; - break; - case StateTx::ACK_SDA_SET_SCL_CLR: - gpio_clr(scl_); - gpio_set(sda_); - ++stateTx_; - break; - case StateTx::ACK_SCL_SET: - gpio_set(scl_); - ++stateTx_; - break; - case StateTx::ACK_SCL_CLR: - if (scl_->read() == Gpio::Value::CLR) - { - // Clock stretching, do the same state again. - clockStretchActive_ = true; - break; - } - if (clockStretchActive_) - { - // I2C spec requires minimum 4 microseconds after the SCL line - // goes high following a clock stretch for the SCL line to be - // pulled low again by the master. Do the same state one more - // time to ensure this. - clockStretchActive_ = false; - break; - } - bool ack = (sda_->read() == Gpio::Value::CLR); - gpio_clr(scl_); - if (!ack) - { - count_ = -EIO; - } - return true; // done - } - return false; -} - -// -// BitBangI2C::state_rx() -// -__attribute__((optimize("-O3"))) -bool BitBangI2C::state_rx(uint8_t *data, bool nack) -{ - switch(stateRx_) - { - case StateRx::DATA_7_SCL_SET: - gpio_set(sda_); // Always start with SDA high. - // fall through - case StateRx::DATA_6_SCL_SET: - case StateRx::DATA_5_SCL_SET: - case StateRx::DATA_4_SCL_SET: - case StateRx::DATA_3_SCL_SET: - case StateRx::DATA_2_SCL_SET: - case StateRx::DATA_1_SCL_SET: - case StateRx::DATA_0_SCL_SET: - gpio_set(scl_); - ++stateRx_; - break; - case StateRx::DATA_7_SCL_CLR: - case StateRx::DATA_6_SCL_CLR: - case StateRx::DATA_5_SCL_CLR: - case StateRx::DATA_4_SCL_CLR: - case StateRx::DATA_3_SCL_CLR: - case StateRx::DATA_2_SCL_CLR: - case StateRx::DATA_1_SCL_CLR: - case StateRx::DATA_0_SCL_CLR: - if (scl_->read() == Gpio::Value::CLR) - { - // Clock stretching, do the same state again. - clockStretchActive_ = true; - break; - } - if (clockStretchActive_) - { - // I2C spec requires minimum 4 microseconds after the SCL line - // goes high following a clock stretch for the SCL line to be - // pulled low again by the master or for the master to sample - // the SDA data. Do the same state one more time to ensure this. - clockStretchActive_ = false; - break; - } - *data <<= 1; - if (sda_->read() == Gpio::Value::SET) - { - *data |= 0x01; - } - gpio_clr(scl_); - if (stateRx_ == StateRx::DATA_0_SCL_CLR && !nack) - { - // Send the ACK. If a NACK, SDA is already set. - gpio_clr(sda_); - } - ++stateRx_; - break; - case StateRx::ACK_SDA_SCL_SET: - gpio_set(scl_); - ++stateRx_; - break; - case StateRx::ACK_SCL_CLR: - if (scl_->read() == Gpio::Value::CLR) - { - // Clock stretching, do the same state again. - clockStretchActive_ = true; - break; - } - if (clockStretchActive_) - { - // I2C spec requires minimum 4 microseconds after the SCL line - // goes high following a clock stretch for the SCL line to be - // pulled low again by the master. Do the same state one more - // time to ensure this. - clockStretchActive_ = false; - break; - } - gpio_clr(scl_); - gpio_set(sda_); - return true; - } - return false; -} - -// -// BitBangI2C::transfer() -// -int BitBangI2C::transfer(struct i2c_msg *msg, bool stop) -{ - while (stop_) - { - // waiting for the initial "stop" condition on reset - sem_.wait(); - } - if (msg_->len == 0) - { - // Message must have length of at least 1. - return -EINVAL; - } - msg_ = msg; - count_ = 0; - stop_ = stop; - state_ = State::START; - stateStart_ = StateStart::FIRST; - enableTick_(); - sem_.wait(); - return count_; -} diff --git a/src/freertos_drivers/common/BitBangI2C.hxx b/src/freertos_drivers/common/BitBangI2C.hxx index a5f11d306..a7305a3b7 100644 --- a/src/freertos_drivers/common/BitBangI2C.hxx +++ b/src/freertos_drivers/common/BitBangI2C.hxx @@ -77,7 +77,7 @@ public: } /// Called at a periodic tick, when enabled. - void tick_interrupt(); + inline void tick_interrupt(); private: /// High level I2C States @@ -176,26 +176,26 @@ private: /// Execute state machine for sending start condition. /// @return true if the sub-state machine is finished, after doing the work /// required by the current tick. - bool state_start(); + inline bool state_start(); /// Execute state machine for sending the stop condition. /// @return true if the sub-state machine is finished, after doing the work /// required by the current tick. - bool state_stop(); + inline bool state_stop(); /// Execute data TX state machine. /// @param data value to send /// @return true if the sub-state machine is finished, after doing the work /// required by the current tick. count_ may be negative to /// indicate an error. - bool state_tx(uint8_t data); + inline bool state_tx(uint8_t data); /// Execute data RX state machine. /// @param location to shift data into /// @param nack send a NACK in the (N)ACK slot /// @return true if the sub-state machine is finished, after doing the work /// required by the current tick. - bool state_rx(uint8_t *data, bool nack); + inline bool state_rx(uint8_t *data, bool nack); void enable() override {} /**< function to enable device */ void disable() override {} /**< function to disable device */ @@ -204,7 +204,7 @@ private: /// @param msg message to transact. /// @param stop produce a stop condition at the end of the transfer /// @return bytes transfered upon success or -1 with errno set - int transfer(struct i2c_msg *msg, bool stop) override; + inline int transfer(struct i2c_msg *msg, bool stop) override; /// Set the GPIO state. /// @param gpio GPIO to set @@ -290,4 +290,380 @@ inline BitBangI2C::StateRx &operator++(BitBangI2C::StateRx &x) } +// +// BitBangI2C::tick_interrupt() +// +__attribute__((optimize("-O3"))) +inline void BitBangI2C::tick_interrupt() +{ + bool exit = false; + switch (state_) + { + // start sequence + case State::START: + if (state_start()) + { + // start done, send the address + state_ = State::ADDRESS; + stateTx_ = StateTx::FIRST; + } + break; + case State::ADDRESS: + { + /// @todo only supporting 7-bit I2C addresses at the moment. + uint8_t address = msg_->addr << 1; + if (msg_->flags & I2C_M_RD) + { + address |= 0x1; + } + if (state_tx(address)) + { + if (count_ < 0) + { + // Some error occured, likely an unexpected NACK. Send a + // stop in order to shutdown gracefully. + state_ = State::STOP; + stateStop_ = StateStop::FIRST; + } + else + { + if (msg_->flags & I2C_M_RD) + { + state_ = State::DATA_RX; + stateRx_ = StateRx::FIRST; + } + else + { + state_ = State::DATA_TX; + stateTx_ = StateTx::FIRST; + } + } + } + break; + } + case State::DATA_TX: + if (state_tx(msg_->buf[count_])) + { + if (count_ < 0) + { + // Some error occured, likely an unexpected NACK. Send a + // stop in order to shutdown gracefully. + state_ = State::STOP; + stateStop_ = StateStop::FIRST; + } + else if (++count_ >= msg_->len) + { + // All of the data has been transmitted. + if (stop_) + { + state_ = State::STOP; + stateStop_ = StateStop::FIRST; + } + else + { + exit = true; + } + } + else + { + // Setup the next byte TX + stateTx_ = StateTx::FIRST; + } + } + break; + case State::DATA_RX: + if (state_rx(msg_->buf + count_, (count_ + 1 >= msg_->len))) + { + if (count_ < 0) + { + // Some error occured, likely an unexpected NACK + exit = true; + } + else if (++count_ >= msg_->len) + { + // All of the data has been received. + if (stop_) + { + state_ = State::STOP; + stateStop_ = StateStop::FIRST; + } + else + { + exit = true; + } + } + else + { + // Setup the next byte RX + stateRx_ = StateRx::FIRST; + } + } + break; + case State::STOP: + exit = state_stop(); + break; + } + + if (exit) + { + disableTick_(); + int woken = 0; + sem_.post_from_isr(&woken); + os_isr_exit_yield_test(woken); + } +} + +// +// BitBangI2C::state_start() +// +__attribute__((optimize("-O3"))) +inline bool BitBangI2C::state_start() +{ + switch (stateStart_) + { + // start sequence + case StateStart::SDA_SET: + gpio_set(sda_); + ++stateStart_; + break; + case StateStart::SCL_SET: + gpio_set(scl_); + ++stateStart_; + break; + case StateStart::SDA_CLR: + gpio_clr(sda_); + ++stateStart_; + return true; + } + return false; +} + +// +// BitBangI2C::state_stop() +// +__attribute__((optimize("-O3"))) +inline bool BitBangI2C::state_stop() +{ + switch (stateStop_) + { + case StateStop::SDA_CLR: + gpio_clr(sda_); + ++stateStop_; + break; + case StateStop::SCL_SET: + gpio_set(scl_); + ++stateStop_; + break; + case StateStop::SDA_SET: + gpio_set(sda_); + stop_ = false; + return true; // exit + } + return false; +} + +// +// BitBangI2C::state_tx() +// +__attribute__((optimize("-O3"))) +inline bool BitBangI2C::state_tx(uint8_t data) +{ + // I2C is specified such that the data on the SDA line must be stable + // during the high period of the clock, and the data line can only change + // when SCL is Low. + // + // This means that the sequence always has to be + // 1a. SCL := low + // 1b. set/clear SDA + // 2. wait a cycle for lines to settle + // 3a. SCL := high + // 3b. this edge is when the receiver samples + // 4. wait a cycle, lines are stable + switch(stateTx_) + { + case StateTx::DATA_7_SCL_CLR: + case StateTx::DATA_6_SCL_CLR: + case StateTx::DATA_5_SCL_CLR: + case StateTx::DATA_4_SCL_CLR: + case StateTx::DATA_3_SCL_CLR: + case StateTx::DATA_2_SCL_CLR: + case StateTx::DATA_1_SCL_CLR: + case StateTx::DATA_0_SCL_CLR: + { + // We can only change the data when SCL is low. + gpio_clr(scl_); + // The divide by 2 factor (shift right by 1) is because the enum + // states alternate between *_SCL_SET and *_SCL_CLR states in + // increasing value. + uint8_t mask = 0x80 >> + ((static_cast(stateTx_) - + static_cast(StateTx::DATA_7_SCL_CLR)) >> 1); + if (data & mask) + { + gpio_set(sda_); + } + else + { + gpio_clr(sda_); + } + ++stateTx_; + break; + } + case StateTx::DATA_7_SCL_SET: + case StateTx::DATA_6_SCL_SET: + case StateTx::DATA_5_SCL_SET: + case StateTx::DATA_4_SCL_SET: + case StateTx::DATA_3_SCL_SET: + case StateTx::DATA_2_SCL_SET: + case StateTx::DATA_1_SCL_SET: + case StateTx::DATA_0_SCL_SET: + // Data is sampled by the slave following this state transition. + gpio_set(scl_); + ++stateTx_; + break; + case StateTx::ACK_SDA_SET_SCL_CLR: + gpio_clr(scl_); + gpio_set(sda_); + ++stateTx_; + break; + case StateTx::ACK_SCL_SET: + gpio_set(scl_); + ++stateTx_; + break; + case StateTx::ACK_SCL_CLR: + if (scl_->read() == Gpio::Value::CLR) + { + // Clock stretching, do the same state again. + clockStretchActive_ = true; + break; + } + if (clockStretchActive_) + { + // I2C spec requires minimum 4 microseconds after the SCL line + // goes high following a clock stretch for the SCL line to be + // pulled low again by the master. Do the same state one more + // time to ensure this. + clockStretchActive_ = false; + break; + } + bool ack = (sda_->read() == Gpio::Value::CLR); + gpio_clr(scl_); + if (!ack) + { + count_ = -EIO; + } + return true; // done + } + return false; +} + +// +// BitBangI2C::state_rx() +// +__attribute__((optimize("-O3"))) +inline bool BitBangI2C::state_rx(uint8_t *data, bool nack) +{ + switch(stateRx_) + { + case StateRx::DATA_7_SCL_SET: + gpio_set(sda_); // Always start with SDA high. + // fall through + case StateRx::DATA_6_SCL_SET: + case StateRx::DATA_5_SCL_SET: + case StateRx::DATA_4_SCL_SET: + case StateRx::DATA_3_SCL_SET: + case StateRx::DATA_2_SCL_SET: + case StateRx::DATA_1_SCL_SET: + case StateRx::DATA_0_SCL_SET: + gpio_set(scl_); + ++stateRx_; + break; + case StateRx::DATA_7_SCL_CLR: + case StateRx::DATA_6_SCL_CLR: + case StateRx::DATA_5_SCL_CLR: + case StateRx::DATA_4_SCL_CLR: + case StateRx::DATA_3_SCL_CLR: + case StateRx::DATA_2_SCL_CLR: + case StateRx::DATA_1_SCL_CLR: + case StateRx::DATA_0_SCL_CLR: + if (scl_->read() == Gpio::Value::CLR) + { + // Clock stretching, do the same state again. + clockStretchActive_ = true; + break; + } + if (clockStretchActive_) + { + // I2C spec requires minimum 4 microseconds after the SCL line + // goes high following a clock stretch for the SCL line to be + // pulled low again by the master or for the master to sample + // the SDA data. Do the same state one more time to ensure this. + clockStretchActive_ = false; + break; + } + *data <<= 1; + if (sda_->read() == Gpio::Value::SET) + { + *data |= 0x01; + } + gpio_clr(scl_); + if (stateRx_ == StateRx::DATA_0_SCL_CLR && !nack) + { + // Send the ACK. If a NACK, SDA is already set. + gpio_clr(sda_); + } + ++stateRx_; + break; + case StateRx::ACK_SDA_SCL_SET: + gpio_set(scl_); + ++stateRx_; + break; + case StateRx::ACK_SCL_CLR: + if (scl_->read() == Gpio::Value::CLR) + { + // Clock stretching, do the same state again. + clockStretchActive_ = true; + break; + } + if (clockStretchActive_) + { + // I2C spec requires minimum 4 microseconds after the SCL line + // goes high following a clock stretch for the SCL line to be + // pulled low again by the master. Do the same state one more + // time to ensure this. + clockStretchActive_ = false; + break; + } + gpio_clr(scl_); + gpio_set(sda_); + return true; + } + return false; +} + +// +// BitBangI2C::transfer() +// +inline int BitBangI2C::transfer(struct i2c_msg *msg, bool stop) +{ + while (stop_) + { + // waiting for the initial "stop" condition on reset + sem_.wait(); + } + if (msg_->len == 0) + { + // Message must have length of at least 1. + return -EINVAL; + } + msg_ = msg; + count_ = 0; + stop_ = stop; + state_ = State::START; + stateStart_ = StateStart::FIRST; + enableTick_(); + sem_.wait(); + return count_; +} + #endif // _FREERTOS_DRIVERS_COMMON_BITBANG_I2C_HXX_ From aa0f4fb328f90bdec788c030867b1203c91f0322 Mon Sep 17 00:00:00 2001 From: Stuart Baker Date: Sun, 30 Apr 2023 20:52:03 -0500 Subject: [PATCH 18/27] Turn the BitBangI2C object into a template with an HW parameter. --- src/freertos_drivers/common/BitBangI2C.hxx | 136 ++++++++++++--------- 1 file changed, 76 insertions(+), 60 deletions(-) diff --git a/src/freertos_drivers/common/BitBangI2C.hxx b/src/freertos_drivers/common/BitBangI2C.hxx index a7305a3b7..ccb37ff2f 100644 --- a/src/freertos_drivers/common/BitBangI2C.hxx +++ b/src/freertos_drivers/common/BitBangI2C.hxx @@ -38,48 +38,16 @@ #include "os/Gpio.hxx" #include "os/OS.hxx" -/// Implement a bit-banged I2C master. A periodic timer [interrupt] should call -/// the BitBangI2C::tick_interrupt() method at a rate that is one half the -/// desired clock rate. For example, for a 100kHz bus, call once every 5 -/// microseconds. The tick should be enabled to start. The driver will -/// enable/disable the tick as needed to save on spurious interrupts. -class BitBangI2C : public I2C +/// Consolidate all the state machine enumerations for easy operator +/// overloading. +class BitBangI2CStates { -public: - /// Constructor. - /// @param name name of this device instance in the file system - /// @param sda_gpio Gpio instance of the SDA line - /// @param scl_gpio Gpio instance of the SCL line - /// @param enable_tick callback to enable ticks - /// @param disable_tick callback to disable ticks - BitBangI2C(const char *name, const Gpio *sda_gpio, const Gpio *scl_gpio, - void (*enable_tick)(), void (*disable_tick)()) - : I2C(name) - , sda_(sda_gpio) - , scl_(scl_gpio) - , enableTick_(enable_tick) - , disableTick_(disable_tick) - , msg_(nullptr) - , sem_() - , count_(0) - , state_(State::STOP) // start-up with a stop sequence - , stateStop_(StateStop::SDA_CLR) - , stop_(true) - , clockStretchActive_(false) - { - gpio_set(sda_); - gpio_clr(scl_); - } - - /// Destructor. - ~BitBangI2C() +protected: + /// Default constructor. + BitBangI2CStates() { } - /// Called at a periodic tick, when enabled. - inline void tick_interrupt(); - -private: /// High level I2C States enum class State : uint8_t { @@ -172,7 +140,50 @@ private: /// Allow pre-increment definition friend StateRx &operator++(StateRx &); +}; +/// Implement a bit-banged I2C master. A periodic timer [interrupt] should call +/// the BitBangI2C::tick_interrupt() method at a rate that is one half the +/// desired clock rate. For example, for a 100kHz bus, call once every 5 +/// microseconds. The tick should be enabled to start. The driver will +/// enable/disable the tick as needed to save on spurious interrupts. +template class BitBangI2C : protected BitBangI2CStates, public I2C +{ +public: + /// Constructor. + /// @param name name of this device instance in the file system + /// @param sda_gpio Gpio instance of the SDA line + /// @param scl_gpio Gpio instance of the SCL line + /// @param enable_tick callback to enable ticks + /// @param disable_tick callback to disable ticks + BitBangI2C(const char *name, const Gpio *sda_gpio, const Gpio *scl_gpio, + void (*enable_tick)(), void (*disable_tick)()) + : I2C(name) + , sda_(sda_gpio) + , scl_(scl_gpio) + , enableTick_(enable_tick) + , disableTick_(disable_tick) + , msg_(nullptr) + , sem_() + , count_(0) + , state_(State::STOP) // start-up with a stop sequence + , stateStop_(StateStop::SDA_CLR) + , stop_(true) + , clockStretchActive_(false) + { + gpio_set(sda_); + gpio_clr(scl_); + } + + /// Destructor. + ~BitBangI2C() + { + } + + /// Called at a periodic tick, when enabled. + inline void tick_interrupt(); + +private: /// Execute state machine for sending start condition. /// @return true if the sub-state machine is finished, after doing the work /// required by the current tick. @@ -244,11 +255,12 @@ private: /// Pre-increment operator overload /// @param x starting value /// @return incremented value -inline BitBangI2C::StateStart &operator++(BitBangI2C::StateStart &x) +inline BitBangI2CStates::StateStart &operator++(BitBangI2CStates::StateStart &x) { - if (x >= BitBangI2C::StateStart::FIRST && x < BitBangI2C::StateStart::LAST) + if (x >= BitBangI2CStates::StateStart::FIRST && + x < BitBangI2CStates::StateStart::LAST) { - x = static_cast(static_cast(x) + 1); + x = static_cast(static_cast(x) + 1); } return x; } @@ -256,11 +268,12 @@ inline BitBangI2C::StateStart &operator++(BitBangI2C::StateStart &x) /// Pre-increment operator overload /// @param x starting value /// @return incremented value -inline BitBangI2C::StateStop &operator++(BitBangI2C::StateStop &x) +inline BitBangI2CStates::StateStop &operator++(BitBangI2CStates::StateStop &x) { - if (x >= BitBangI2C::StateStop::FIRST && x < BitBangI2C::StateStop::LAST) + if (x >= BitBangI2CStates::StateStop::FIRST && + x < BitBangI2CStates::StateStop::LAST) { - x = static_cast(static_cast(x) + 1); + x = static_cast(static_cast(x) + 1); } return x; } @@ -268,11 +281,12 @@ inline BitBangI2C::StateStop &operator++(BitBangI2C::StateStop &x) /// Pre-increment operator overload /// @param x starting value /// @return incremented value -inline BitBangI2C::StateTx &operator++(BitBangI2C::StateTx &x) +inline BitBangI2CStates::StateTx &operator++(BitBangI2CStates::StateTx &x) { - if (x >= BitBangI2C::StateTx::FIRST && x < BitBangI2C::StateTx::LAST) + if (x >= BitBangI2CStates::StateTx::FIRST && + x < BitBangI2CStates::StateTx::LAST) { - x = static_cast(static_cast(x) + 1); + x = static_cast(static_cast(x) + 1); } return x; } @@ -280,11 +294,12 @@ inline BitBangI2C::StateTx &operator++(BitBangI2C::StateTx &x) /// Pre-increment operator overload /// @param x starting value /// @return incremented value -inline BitBangI2C::StateRx &operator++(BitBangI2C::StateRx &x) +inline BitBangI2CStates::StateRx &operator++(BitBangI2CStates::StateRx &x) { - if (x >= BitBangI2C::StateRx::FIRST && x < BitBangI2C::StateRx::LAST) + if (x >= BitBangI2CStates::StateRx::FIRST && + x < BitBangI2CStates::StateRx::LAST) { - x = static_cast(static_cast(x) + 1); + x = static_cast(static_cast(x) + 1); } return x; } @@ -293,7 +308,7 @@ inline BitBangI2C::StateRx &operator++(BitBangI2C::StateRx &x) // // BitBangI2C::tick_interrupt() // -__attribute__((optimize("-O3"))) +template __attribute__((optimize("-O3"))) inline void BitBangI2C::tick_interrupt() { bool exit = false; @@ -416,8 +431,8 @@ inline void BitBangI2C::tick_interrupt() // // BitBangI2C::state_start() // -__attribute__((optimize("-O3"))) -inline bool BitBangI2C::state_start() +template __attribute__((optimize("-O3"))) +inline bool BitBangI2C::state_start() { switch (stateStart_) { @@ -441,8 +456,8 @@ inline bool BitBangI2C::state_start() // // BitBangI2C::state_stop() // -__attribute__((optimize("-O3"))) -inline bool BitBangI2C::state_stop() +template __attribute__((optimize("-O3"))) +inline bool BitBangI2C::state_stop() { switch (stateStop_) { @@ -465,8 +480,8 @@ inline bool BitBangI2C::state_stop() // // BitBangI2C::state_tx() // -__attribute__((optimize("-O3"))) -inline bool BitBangI2C::state_tx(uint8_t data) +template __attribute__((optimize("-O3"))) +inline bool BitBangI2C::state_tx(uint8_t data) { // I2C is specified such that the data on the SDA line must be stable // during the high period of the clock, and the data line can only change @@ -560,8 +575,8 @@ inline bool BitBangI2C::state_tx(uint8_t data) // // BitBangI2C::state_rx() // -__attribute__((optimize("-O3"))) -inline bool BitBangI2C::state_rx(uint8_t *data, bool nack) +template __attribute__((optimize("-O3"))) +inline bool BitBangI2C::state_rx(uint8_t *data, bool nack) { switch(stateRx_) { @@ -644,7 +659,8 @@ inline bool BitBangI2C::state_rx(uint8_t *data, bool nack) // // BitBangI2C::transfer() // -inline int BitBangI2C::transfer(struct i2c_msg *msg, bool stop) +template +inline int BitBangI2C::transfer(struct i2c_msg *msg, bool stop) { while (stop_) { From f6aa2cd6d1b8ec94815dab395f8d8955688f2147 Mon Sep 17 00:00:00 2001 From: Stuart Baker Date: Sun, 30 Apr 2023 21:26:51 -0500 Subject: [PATCH 19/27] Implement SCL and SDA I/O as template HW parameters. --- src/freertos_drivers/common/BitBangI2C.hxx | 133 +++++++++++++-------- 1 file changed, 81 insertions(+), 52 deletions(-) diff --git a/src/freertos_drivers/common/BitBangI2C.hxx b/src/freertos_drivers/common/BitBangI2C.hxx index ccb37ff2f..68cb68c3c 100644 --- a/src/freertos_drivers/common/BitBangI2C.hxx +++ b/src/freertos_drivers/common/BitBangI2C.hxx @@ -147,20 +147,67 @@ protected: /// desired clock rate. For example, for a 100kHz bus, call once every 5 /// microseconds. The tick should be enabled to start. The driver will /// enable/disable the tick as needed to save on spurious interrupts. +/// +/// Example: +/// @code +/// struct I2CHwDefs +/// { +/// class SDA_Pin +/// { +/// public: +/// static void set(bool on) +/// { +/// ... +/// } +/// static bool get() +/// { +/// ... +/// } +/// static void hw_init() +/// { +/// ... +/// } +/// }; +/// +/// class SCL_Pin +/// { +/// public: +/// static void set(bool on) +/// { +/// ... +/// } +/// static bool get() +/// { +/// ... +/// } +/// static void hw_init() +/// { +/// ... +/// } +/// }; +/// }; +/// +/// // or +/// +/// struct I2CHwDefs +/// { +/// using SCL_Pin = ::MY_SCL_Pin; +/// using SDA_Pin = ::MY_SDA_Pin; +/// }; +/// +/// BitBangI2C bitBangI2C("/dev/i2c0", disable_tick, enable_tick); +/// @endcode +/// +/// @tparam HW hardware interface to the access the SCL and SDA I/O lines template class BitBangI2C : protected BitBangI2CStates, public I2C { public: /// Constructor. /// @param name name of this device instance in the file system - /// @param sda_gpio Gpio instance of the SDA line - /// @param scl_gpio Gpio instance of the SCL line /// @param enable_tick callback to enable ticks /// @param disable_tick callback to disable ticks - BitBangI2C(const char *name, const Gpio *sda_gpio, const Gpio *scl_gpio, - void (*enable_tick)(), void (*disable_tick)()) + BitBangI2C(const char *name, void (*enable_tick)(), void (*disable_tick)()) : I2C(name) - , sda_(sda_gpio) - , scl_(scl_gpio) , enableTick_(enable_tick) , disableTick_(disable_tick) , msg_(nullptr) @@ -171,8 +218,8 @@ public: , stop_(true) , clockStretchActive_(false) { - gpio_set(sda_); - gpio_clr(scl_); + HW::SDA_Pin::set(1); + HW::SCL_Pin::set(0); } /// Destructor. @@ -217,24 +264,6 @@ private: /// @return bytes transfered upon success or -1 with errno set inline int transfer(struct i2c_msg *msg, bool stop) override; - /// Set the GPIO state. - /// @param gpio GPIO to set - void gpio_set(const Gpio *gpio) - { - gpio->set_direction(Gpio::Direction::DINPUT); - } - - /// Clear the GPIO state. - /// @param gpio GPIO to clear - void gpio_clr(const Gpio *gpio) - { - gpio->clr(); - gpio->set_direction(Gpio::Direction::DOUTPUT); - gpio->clr(); - } - - const Gpio *sda_; ///< GPIO for the SDA line - const Gpio *scl_; ///< GPIO for the SCL line void (*enableTick_)(void); ///< Enable the timer tick void (*disableTick_)(void); ///< Disable the timer tick struct i2c_msg *msg_; ///< I2C message to presently act upon @@ -438,15 +467,15 @@ inline bool BitBangI2C::state_start() { // start sequence case StateStart::SDA_SET: - gpio_set(sda_); + HW::SDA_Pin::set(1); ++stateStart_; break; case StateStart::SCL_SET: - gpio_set(scl_); + HW::SCL_Pin::set(1); ++stateStart_; break; case StateStart::SDA_CLR: - gpio_clr(sda_); + HW::SDA_Pin::set(0); ++stateStart_; return true; } @@ -462,15 +491,15 @@ inline bool BitBangI2C::state_stop() switch (stateStop_) { case StateStop::SDA_CLR: - gpio_clr(sda_); + HW::SDA_Pin::set(0); ++stateStop_; break; case StateStop::SCL_SET: - gpio_set(scl_); + HW::SCL_Pin::set(1); ++stateStop_; break; case StateStop::SDA_SET: - gpio_set(sda_); + HW::SDA_Pin::set(1); stop_ = false; return true; // exit } @@ -506,7 +535,7 @@ inline bool BitBangI2C::state_tx(uint8_t data) case StateTx::DATA_0_SCL_CLR: { // We can only change the data when SCL is low. - gpio_clr(scl_); + HW::SCL_Pin::set(0); // The divide by 2 factor (shift right by 1) is because the enum // states alternate between *_SCL_SET and *_SCL_CLR states in // increasing value. @@ -515,11 +544,11 @@ inline bool BitBangI2C::state_tx(uint8_t data) static_cast(StateTx::DATA_7_SCL_CLR)) >> 1); if (data & mask) { - gpio_set(sda_); + HW::SDA_Pin::set(1); } else { - gpio_clr(sda_); + HW::SDA_Pin::set(0); } ++stateTx_; break; @@ -533,20 +562,20 @@ inline bool BitBangI2C::state_tx(uint8_t data) case StateTx::DATA_1_SCL_SET: case StateTx::DATA_0_SCL_SET: // Data is sampled by the slave following this state transition. - gpio_set(scl_); + HW::SCL_Pin::set(1); ++stateTx_; break; case StateTx::ACK_SDA_SET_SCL_CLR: - gpio_clr(scl_); - gpio_set(sda_); + HW::SCL_Pin::set(0); + HW::SDA_Pin::set(1); ++stateTx_; break; case StateTx::ACK_SCL_SET: - gpio_set(scl_); + HW::SCL_Pin::set(1); ++stateTx_; break; case StateTx::ACK_SCL_CLR: - if (scl_->read() == Gpio::Value::CLR) + if (HW::SCL_Pin::get() == false) { // Clock stretching, do the same state again. clockStretchActive_ = true; @@ -561,8 +590,8 @@ inline bool BitBangI2C::state_tx(uint8_t data) clockStretchActive_ = false; break; } - bool ack = (sda_->read() == Gpio::Value::CLR); - gpio_clr(scl_); + bool ack = (HW::SDA_Pin::get() == false); + HW::SCL_Pin::set(0); if (!ack) { count_ = -EIO; @@ -581,7 +610,7 @@ inline bool BitBangI2C::state_rx(uint8_t *data, bool nack) switch(stateRx_) { case StateRx::DATA_7_SCL_SET: - gpio_set(sda_); // Always start with SDA high. + HW::SDA_Pin::set(1); // Always start with SDA high. // fall through case StateRx::DATA_6_SCL_SET: case StateRx::DATA_5_SCL_SET: @@ -590,7 +619,7 @@ inline bool BitBangI2C::state_rx(uint8_t *data, bool nack) case StateRx::DATA_2_SCL_SET: case StateRx::DATA_1_SCL_SET: case StateRx::DATA_0_SCL_SET: - gpio_set(scl_); + HW::SCL_Pin::set(1); ++stateRx_; break; case StateRx::DATA_7_SCL_CLR: @@ -601,7 +630,7 @@ inline bool BitBangI2C::state_rx(uint8_t *data, bool nack) case StateRx::DATA_2_SCL_CLR: case StateRx::DATA_1_SCL_CLR: case StateRx::DATA_0_SCL_CLR: - if (scl_->read() == Gpio::Value::CLR) + if (HW::SCL_Pin::get() == false) { // Clock stretching, do the same state again. clockStretchActive_ = true; @@ -617,24 +646,24 @@ inline bool BitBangI2C::state_rx(uint8_t *data, bool nack) break; } *data <<= 1; - if (sda_->read() == Gpio::Value::SET) + if (HW::SDA_Pin::get() == true) { *data |= 0x01; } - gpio_clr(scl_); + HW::SCL_Pin::set(0); if (stateRx_ == StateRx::DATA_0_SCL_CLR && !nack) { // Send the ACK. If a NACK, SDA is already set. - gpio_clr(sda_); + HW::SDA_Pin::set(0); } ++stateRx_; break; case StateRx::ACK_SDA_SCL_SET: - gpio_set(scl_); + HW::SCL_Pin::set(1); ++stateRx_; break; case StateRx::ACK_SCL_CLR: - if (scl_->read() == Gpio::Value::CLR) + if (HW::SCL_Pin::get() == false) { // Clock stretching, do the same state again. clockStretchActive_ = true; @@ -649,8 +678,8 @@ inline bool BitBangI2C::state_rx(uint8_t *data, bool nack) clockStretchActive_ = false; break; } - gpio_clr(scl_); - gpio_set(sda_); + HW::SCL_Pin::set(0); + HW::SDA_Pin::set(1); return true; } return false; From 12d4505d29aa86690a1662e119e85d19211e74c9 Mon Sep 17 00:00:00 2001 From: Stuart Baker Date: Sun, 30 Apr 2023 21:59:16 -0500 Subject: [PATCH 20/27] Correct documentation. --- src/freertos_drivers/common/BitBangI2C.hxx | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/src/freertos_drivers/common/BitBangI2C.hxx b/src/freertos_drivers/common/BitBangI2C.hxx index 68cb68c3c..3867a1506 100644 --- a/src/freertos_drivers/common/BitBangI2C.hxx +++ b/src/freertos_drivers/common/BitBangI2C.hxx @@ -152,7 +152,7 @@ protected: /// @code /// struct I2CHwDefs /// { -/// class SDA_Pin +/// class SCL_Pin /// { /// public: /// static void set(bool on) @@ -169,7 +169,7 @@ protected: /// } /// }; /// -/// class SCL_Pin +/// class SDA_Pin /// { /// public: /// static void set(bool on) @@ -187,14 +187,6 @@ protected: /// }; /// }; /// -/// // or -/// -/// struct I2CHwDefs -/// { -/// using SCL_Pin = ::MY_SCL_Pin; -/// using SDA_Pin = ::MY_SDA_Pin; -/// }; -/// /// BitBangI2C bitBangI2C("/dev/i2c0", disable_tick, enable_tick); /// @endcode /// From f2293b94be1062685c276b47442efc090e8abae0 Mon Sep 17 00:00:00 2001 From: Stuart Baker Date: Sun, 30 Apr 2023 21:59:38 -0500 Subject: [PATCH 21/27] Add GpioInputOutputPin definition. --- src/freertos_drivers/ti/CC3200GPIO.hxx | 63 ++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/src/freertos_drivers/ti/CC3200GPIO.hxx b/src/freertos_drivers/ti/CC3200GPIO.hxx index bd416c64e..bcf0bd359 100644 --- a/src/freertos_drivers/ti/CC3200GPIO.hxx +++ b/src/freertos_drivers/ti/CC3200GPIO.hxx @@ -296,4 +296,67 @@ public: } }; +/// Common class for GPIO input pins. +template struct GpioInputOutputPin : public Defs +{ +public: + using Defs::GPIO_PERIPH; + using Defs::GPIO_BASE; + using Defs::GPIO_PIN; + /// Initializes the hardware pin. + static void hw_init() + { + MAP_PRCMPeripheralClkEnable(GPIO_PERIPH, PRCM_RUN_MODE_CLK); + MAP_GPIODirModeSet(GPIO_BASE, GPIO_PIN, GPIO_DIR_MODE_IN); + } + /// Sets the output pin to a safe value. + static void hw_set_to_safe() + { + hw_init(); + } + /// Sets the output pin to a defined value. @param value if true, output + /// will be set to HIGH, otherwise to LOW. + static void __attribute__((always_inline)) set(bool value) + { + volatile uint8_t *ptr = reinterpret_cast( + GPIO_BASE + (((unsigned)GPIO_PIN) << 2)); + *ptr = value ? 0xff : 0; + /// See note at the top of the file about barriers. + __asm__ volatile("dsb" : : : "memory"); + } + /// @return current value of the input pin: if true HIGH. + static bool __attribute__((always_inline)) get() + { + const volatile uint8_t *ptr = reinterpret_cast( + GPIO_BASE + (((unsigned)GPIO_PIN) << 2)); + return *ptr; + } + /// Changes the value of an output pin. + static void __attribute__((always_inline)) toggle() + { + set(!get()); + } + + /// @return static Gpio ovject instance that controls this output pin. + static constexpr const Gpio *instance() + { + return &CC3200Gpio::instance_; + } + + /// Sets the direction of the I/O pin. + /// @param direction direction to set pin to + static void set_direction(Gpio::Direction direction) + { + MAP_GPIODirModeSet(GPIO_BASE, GPIO_PIN, + direction == Gpio::Direction::DINPUT ? + GPIO_DIR_MODE_IN : GPIO_DIR_MODE_OUT); + } + + /// @return true if this pin is an output pin. + static bool is_output() + { + return (MAP_GPIODirModeGet(GPIO_BASE, GPIO_PIN) == GPIO_DIR_MODE_OUT); + } +}; + #endif //_FREERTOS_DRIVERS_TI_CC3200GPIO_HXX_ From 5d6b27a088b151a3cf9330515a302725a27bf2cc Mon Sep 17 00:00:00 2001 From: Stuart Baker Date: Sun, 30 Apr 2023 22:20:19 -0500 Subject: [PATCH 22/27] Add tick enable/disable to the HW template argument. --- src/freertos_drivers/common/BitBangI2C.hxx | 25 ++++++++++++++-------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/src/freertos_drivers/common/BitBangI2C.hxx b/src/freertos_drivers/common/BitBangI2C.hxx index 3867a1506..99b027ca0 100644 --- a/src/freertos_drivers/common/BitBangI2C.hxx +++ b/src/freertos_drivers/common/BitBangI2C.hxx @@ -145,8 +145,9 @@ protected: /// Implement a bit-banged I2C master. A periodic timer [interrupt] should call /// the BitBangI2C::tick_interrupt() method at a rate that is one half the /// desired clock rate. For example, for a 100kHz bus, call once every 5 -/// microseconds. The tick should be enabled to start. The driver will -/// enable/disable the tick as needed to save on spurious interrupts. +/// microseconds. The tick should be enabled to start. The driver will call +/// HW::tick_enable()/HW::tick_disable() to enable/disable the tick as needed +/// to save on spurious interrupts. /// /// Example: /// @code @@ -185,6 +186,16 @@ protected: /// ... /// } /// }; +/// +/// static void tick_enable() +/// { +/// ... +/// } +/// +/// static void tick_disable() +/// { +/// ... +/// } /// }; /// /// BitBangI2C bitBangI2C("/dev/i2c0", disable_tick, enable_tick); @@ -198,10 +209,8 @@ public: /// @param name name of this device instance in the file system /// @param enable_tick callback to enable ticks /// @param disable_tick callback to disable ticks - BitBangI2C(const char *name, void (*enable_tick)(), void (*disable_tick)()) + BitBangI2C(const char *name) : I2C(name) - , enableTick_(enable_tick) - , disableTick_(disable_tick) , msg_(nullptr) , sem_() , count_(0) @@ -256,8 +265,6 @@ private: /// @return bytes transfered upon success or -1 with errno set inline int transfer(struct i2c_msg *msg, bool stop) override; - void (*enableTick_)(void); ///< Enable the timer tick - void (*disableTick_)(void); ///< Disable the timer tick struct i2c_msg *msg_; ///< I2C message to presently act upon OSSem sem_; ///< semaphore to wakeup task level from ISR int count_; ///< the count of data bytes transferred, error if < 0 @@ -442,7 +449,7 @@ inline void BitBangI2C::tick_interrupt() if (exit) { - disableTick_(); + HW::tick_disable(); int woken = 0; sem_.post_from_isr(&woken); os_isr_exit_yield_test(woken); @@ -698,7 +705,7 @@ inline int BitBangI2C::transfer(struct i2c_msg *msg, bool stop) stop_ = stop; state_ = State::START; stateStart_ = StateStart::FIRST; - enableTick_(); + HW::tick_enable(); sem_.wait(); return count_; } From 7a6bd287838fd3df13c451b7c7eb868e8d9d3d2e Mon Sep 17 00:00:00 2001 From: Stuart Baker Date: Sun, 30 Apr 2023 22:30:47 -0500 Subject: [PATCH 23/27] Remove obsolete file. --- src/freertos_drivers/common/BitBangI2C.cxx | 36 ---------------------- 1 file changed, 36 deletions(-) delete mode 100644 src/freertos_drivers/common/BitBangI2C.cxx diff --git a/src/freertos_drivers/common/BitBangI2C.cxx b/src/freertos_drivers/common/BitBangI2C.cxx deleted file mode 100644 index 27d5f6e37..000000000 --- a/src/freertos_drivers/common/BitBangI2C.cxx +++ /dev/null @@ -1,36 +0,0 @@ -/** @copyright - * Copyright (c) 2023, Stuart W Baker - * All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions are met: - * - * - Redistributions of source code must retain the above copyright notice, - * this list of conditions and the following disclaimer. - * - * - Redistributions in binary form must reproduce the above copyright notice, - * this list of conditions and the following disclaimer in the documentation - * and/or other materials provided with the distribution. - * - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" - * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE - * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE - * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE - * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR - * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF - * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS - * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN - * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) - * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE - * POSSIBILITY OF SUCH DAMAGE. - * - * @file BitBangI2C.cxx - * This file implements a bit bang implementation of I2C. - * - * @author Stuart W. Baker - * @date 28 March 2023 - */ - -#include "BitBangI2C.hxx" - - From 66ed5d80542cd041328204115c15abfae131e315 Mon Sep 17 00:00:00 2001 From: Stuart Baker Date: Sun, 14 May 2023 10:32:00 -0500 Subject: [PATCH 24/27] Fix bug on input condition (length) check. --- src/freertos_drivers/common/BitBangI2C.hxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/freertos_drivers/common/BitBangI2C.hxx b/src/freertos_drivers/common/BitBangI2C.hxx index 99b027ca0..983701b4f 100644 --- a/src/freertos_drivers/common/BitBangI2C.hxx +++ b/src/freertos_drivers/common/BitBangI2C.hxx @@ -695,7 +695,7 @@ inline int BitBangI2C::transfer(struct i2c_msg *msg, bool stop) // waiting for the initial "stop" condition on reset sem_.wait(); } - if (msg_->len == 0) + if (msg->len == 0) { // Message must have length of at least 1. return -EINVAL; From 3037f3e5aed16c3a0b21f01e32d6fd4876a11e67 Mon Sep 17 00:00:00 2001 From: Stuart Baker Date: Sun, 14 May 2023 10:34:49 -0500 Subject: [PATCH 25/27] Add timeout capability. --- src/freertos_drivers/common/BitBangI2C.hxx | 30 +++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/src/freertos_drivers/common/BitBangI2C.hxx b/src/freertos_drivers/common/BitBangI2C.hxx index 983701b4f..185fcbbcf 100644 --- a/src/freertos_drivers/common/BitBangI2C.hxx +++ b/src/freertos_drivers/common/BitBangI2C.hxx @@ -692,7 +692,8 @@ inline int BitBangI2C::transfer(struct i2c_msg *msg, bool stop) { while (stop_) { - // waiting for the initial "stop" condition on reset + // No need for a timed wait here because the stop state machine is not + // gated by what happens on the bus (e.g. zero stretching). sem_.wait(); } if (msg->len == 0) @@ -700,14 +701,37 @@ inline int BitBangI2C::transfer(struct i2c_msg *msg, bool stop) // Message must have length of at least 1. return -EINVAL; } + + // Reset state for a start/restart. msg_ = msg; count_ = 0; stop_ = stop; state_ = State::START; stateStart_ = StateStart::FIRST; + + // Flush/reset semaphore. + while (sem_.timedwait(0) == 0); + // Enable tick timer. HW::tick_enable(); - sem_.wait(); - return count_; + + // We wait a minimum of 10 msec to account for any rounding in the "tick" + // rate conversion. msg_->len is at least 1. We assume that worst ~50kHz + // is the slowest that anyone will try to run the I2C bus, which will + // result in just under 200 microseconds per byte. This leaves some room + // for zero stretching. + long long wait = std::max(MSEC_TO_NSEC(msg_->len), MSEC_TO_NSEC(10)); + if (sem_.timedwait(wait) == 0) + { + return count_; + } + else + { + // stop_ must be false for the next call of this method. It should only + // be true on first entry at start to "reset" the bus to a known state. + // On a timeout, it may not have been reset back to false. + stop_ = false; + return -ETIMEDOUT; + } } #endif // _FREERTOS_DRIVERS_COMMON_BITBANG_I2C_HXX_ From e91e321a6dcf2c51b1f41772b08f3c0037f496fe Mon Sep 17 00:00:00 2001 From: Stuart Baker Date: Sun, 14 May 2023 10:46:36 -0500 Subject: [PATCH 26/27] Remove BitBangI2C.cxx from the build. --- src/freertos_drivers/sources | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/freertos_drivers/sources b/src/freertos_drivers/sources index cd6675107..9d90fa32c 100644 --- a/src/freertos_drivers/sources +++ b/src/freertos_drivers/sources @@ -27,8 +27,7 @@ CXXSRCS += Fileio.cxx \ PCA9685PWM.cxx \ SN74HC595GPO.cxx \ TCAN4550Can.cxx \ - SPIFlash.cxx \ - BitBangI2C.cxx + SPIFlash.cxx From df58be8f3ee6b41f98f67d516c3d18fc6aae371c Mon Sep 17 00:00:00 2001 From: Stuart Baker Date: Sun, 13 Aug 2023 12:29:36 -0500 Subject: [PATCH 27/27] Address review comments. --- src/freertos_drivers/common/BitBangI2C.hxx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/freertos_drivers/common/BitBangI2C.hxx b/src/freertos_drivers/common/BitBangI2C.hxx index 185fcbbcf..fc7c12ce3 100644 --- a/src/freertos_drivers/common/BitBangI2C.hxx +++ b/src/freertos_drivers/common/BitBangI2C.hxx @@ -207,8 +207,6 @@ template class BitBangI2C : protected BitBangI2CStates, public I2C public: /// Constructor. /// @param name name of this device instance in the file system - /// @param enable_tick callback to enable ticks - /// @param disable_tick callback to disable ticks BitBangI2C(const char *name) : I2C(name) , msg_(nullptr) @@ -419,8 +417,10 @@ inline void BitBangI2C::tick_interrupt() { if (count_ < 0) { - // Some error occured, likely an unexpected NACK - exit = true; + // Some error occured, likely an unexpected NACK. Send a + // stop in order to shutdown gracefully. + state_ = State::STOP; + stateStop_ = StateStop::FIRST; } else if (++count_ >= msg_->len) {