From dda3c5a4ce2dd2f1084be22e31200c24c3122562 Mon Sep 17 00:00:00 2001 From: Ali Shah Date: Fri, 16 Feb 2024 01:27:56 -0500 Subject: [PATCH 1/3] Fix for broken I2C functionality on x41 chips - CLK/SDA Pins are swapped and comments are misleading - Pin swap Discussion can be viewed here: https://github.com/SpenceKonde/ATTinyCore/discussions/833 - Fix recursive call: Wire.cpp in soft master / hw slave --- avr/libraries/Wire/src/SoftI2CMaster.h | 3 +-- avr/libraries/Wire/src/SoftWire.h | 4 ++-- avr/libraries/Wire/src/Wire.cpp | 9 ++++----- avr/variants/tinyx41_cw/pins_arduino.h | 10 +++++----- avr/variants/tinyx41_legacy/pins_arduino.h | 10 +++++----- 5 files changed, 17 insertions(+), 19 deletions(-) diff --git a/avr/libraries/Wire/src/SoftI2CMaster.h b/avr/libraries/Wire/src/SoftI2CMaster.h index ccd7875e..6903badc 100644 --- a/avr/libraries/Wire/src/SoftI2CMaster.h +++ b/avr/libraries/Wire/src/SoftI2CMaster.h @@ -199,7 +199,6 @@ void i2c_delay_half(void) __asm__ __volatile__ (" ret"); // 7 cycles for call and return #else - uint8_t temp = 0; __asm__ __volatile__ ( " ldi r25, %[DELAY] ; load delay constant ; 4C \n\t" @@ -207,7 +206,7 @@ void i2c_delay_half(void) " dec r25 ; decrement counter ; 4C + xC \n\t" " brne _Lidelay ; 5C+(x-1)2C + xC\n\t" " ret ; 9C+(x-1)2C + xC = 7C + xC" - :"+d" ((uint8_t) temp) : [DELAY] "M" I2C_DELAY_COUNTER); + : : [DELAY] "M" I2C_DELAY_COUNTER : "r25"); // 7 cycles + 3 times x cycles #endif } diff --git a/avr/libraries/Wire/src/SoftWire.h b/avr/libraries/Wire/src/SoftWire.h index 91e5f8e7..b1341e1c 100644 --- a/avr/libraries/Wire/src/SoftWire.h +++ b/avr/libraries/Wire/src/SoftWire.h @@ -113,11 +113,11 @@ class SoftWire : public Stream beginTransmission(address); // the maximum size of internal address is 3 bytes if (isize > 3) { - isize = 3; + isize = 3; } // write internal register address - most significant byte first while (isize-- > 0) - write((uint8_t)(iaddress >> (isize*8))); + write((uint8_t)(iaddress >> (isize*8))); endTransmission(false); } // clamp to buffer length diff --git a/avr/libraries/Wire/src/Wire.cpp b/avr/libraries/Wire/src/Wire.cpp index 80f443fb..b9c5aeec 100644 --- a/avr/libraries/Wire/src/Wire.cpp +++ b/avr/libraries/Wire/src/Wire.cpp @@ -732,10 +732,9 @@ return value; } #else // Implementations for slave only mode - void TwoWire::begin(int address) { - begin((uint8_t)address); +void TwoWire::begin(uint8_t address) { + TinyWireS.begin((uint8_t)address, 0); } - // must be called in slave onRequest event callback size_t TwoWire::write(uint8_t data) { size_t numBytes = 0; @@ -814,7 +813,7 @@ /* END MASTER ONLY METHODS */ /* BEGIN SLAVE ONLY METHODS */ - #if defined(SLAVE_MASTER_ONLY) || defined(WIRE_BOTH) + #if defined(WIRE_SLAVE_ONLY) || defined(WIRE_BOTH) void TwoWire::onReceive( void (*function)(size_t)) { TinyWireS.onReceive(function); @@ -822,7 +821,7 @@ void TwoWire::onReceive( void (*function)(int)) { // arduino api compatibility fixer: // really hope size parameter will not exceed 2^31 :) - static_assert(sizeof(int) == sizeof(size_t), "something is wrong in Arduino kingdom"); + static_assert(sizeof(int) == sizeof(size_t), "something is wrong in the Arduino kingdom"); TinyWireS.onReceive(reinterpret_cast(function)); } // sets function called on slave read diff --git a/avr/variants/tinyx41_cw/pins_arduino.h b/avr/variants/tinyx41_cw/pins_arduino.h index 2609dee1..db2c2725 100644 --- a/avr/variants/tinyx41_cw/pins_arduino.h +++ b/avr/variants/tinyx41_cw/pins_arduino.h @@ -292,13 +292,13 @@ anyway) and instead just use TOCPMCOE bits to control whether PWM is output */ * a markedly inferior software TWI master implementation if that is requested. *---------------------------------------------------------------------------*/ -/* Hardware I2C slave */ -#define SCL PIN_PA6 +/* Used by hardware I2C slave and software I2C master */ +#define SCL PIN_PA4 #define SCL_PORT (PORTA) -#define SCL_PIN (6) -#define SDA PIN_PA4 +#define SCL_PIN (4) +#define SDA PIN_PA6 #define SDA_PORT (PORTA) -#define SDA_PIN (4) +#define SDA_PIN (6) /* Hardware SPI */ #if defined(SET_REMAP) && SET_REMAP > 1 diff --git a/avr/variants/tinyx41_legacy/pins_arduino.h b/avr/variants/tinyx41_legacy/pins_arduino.h index beec9fce..b0d84d6a 100644 --- a/avr/variants/tinyx41_legacy/pins_arduino.h +++ b/avr/variants/tinyx41_legacy/pins_arduino.h @@ -293,13 +293,13 @@ anyway) and instead just use TOCPMCOE bits to control whether PWM is output */ * a markedly inferior software TWI master implementation if that is requested. *---------------------------------------------------------------------------*/ -/* Hardware I2C slave */ -#define SCL PIN_PA6 +/* Used by hardware I2C slave and software I2C master */ +#define SCL PIN_PA4 #define SCL_PORT (PORTA) -#define SCL_PIN (6) -#define SDA PIN_PA4 +#define SCL_PIN (4) +#define SDA PIN_PA6 #define SDA_PORT (PORTA) -#define SDA_PIN (4) +#define SDA_PIN (6) /* Hardware SPI */ #if defined(SET_REMAP) && SET_REMAP > 1 From 0ae2cadff832553841a3fa2fb659495645937d83 Mon Sep 17 00:00:00 2001 From: Ali Shah Date: Fri, 16 Feb 2024 10:30:12 -0500 Subject: [PATCH 2/3] Fix serial corruption on x7 chips - We never clear LTXOK when we finish transmission of the ring buffer. This means that on the next pass, when new data needs transmission, enabling the Transmit Performed Interrupt fires the interrupt handler immediately, which then proceeds to load data from pointers that are stale as they haven't been set to the correct indexing. - Don't enable Transmit Performed Interrupt prior to setting the correct tail index and prior to loading data in LINDAT reg --- avr/cores/tiny/HardwareSerial.cpp | 5 +++-- avr/cores/tiny/Serial0.cpp | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/avr/cores/tiny/HardwareSerial.cpp b/avr/cores/tiny/HardwareSerial.cpp index 633102b9..47bb98ba 100644 --- a/avr/cores/tiny/HardwareSerial.cpp +++ b/avr/cores/tiny/HardwareSerial.cpp @@ -179,11 +179,12 @@ *_ucsrb |= _udrie; #else if(!(LINENIR & _BV(LENTXOK))){ - //The buffer was previously empty, so enable TX Complete interrupt and load first byte. - LINENIR = _BV(LENTXOK) | _BV(LENRXOK); + // The buffer was previously empty, so load the first byte, then enable + // the TX Complete interrupt unsigned char c = _tx_buffer->buffer[_tx_buffer->tail]; _tx_buffer->tail = (_tx_buffer->tail + 1) & (SERIAL_BUFFER_SIZE - 1); LINDAT = c; + LINENIR = _BV(LENTXOK) | _BV(LENRXOK); } #endif diff --git a/avr/cores/tiny/Serial0.cpp b/avr/cores/tiny/Serial0.cpp index 4dcdc4bf..636e9b50 100644 --- a/avr/cores/tiny/Serial0.cpp +++ b/avr/cores/tiny/Serial0.cpp @@ -37,7 +37,7 @@ if(LINSIR & _BV(LTXOK)) { //PINA |= _BV(PINA5); //debug if (tx_buffer.head == tx_buffer.tail) { - // Buffer empty, so disable interrupts + // Buffer empty, so disable the Transmit Performed Interrupt LINENIR = LENRXOK; //unset LENTXOK } else { // There is more data in the output buffer. Send the next byte From b2856b78f5591e18521de3526a98f7c407b70309 Mon Sep 17 00:00:00 2001 From: Hans Date: Mon, 8 Apr 2024 14:52:22 +0200 Subject: [PATCH 3/3] Update pins_arduino.h --- avr/variants/tinyx5/pins_arduino.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/avr/variants/tinyx5/pins_arduino.h b/avr/variants/tinyx5/pins_arduino.h index e0b97fe8..92138f97 100644 --- a/avr/variants/tinyx5/pins_arduino.h +++ b/avr/variants/tinyx5/pins_arduino.h @@ -155,8 +155,8 @@ static const uint8_t A3 = ADC_CH(3); #define INTERNAL INTERNAL1V1 /* deprecated */ /* Special Analog Channels */ #define ADC_TEMPERATURE ADC_CH(0x0F) -#define ADC_INTERNAL1V1 ADC_CH(0x0D) -#define ADC_GROUND ADC_CH(0x0C) +#define ADC_INTERNAL1V1 ADC_CH(0x0C) +#define ADC_GROUND ADC_CH(0x0D) /* Differential Analog Channels */ #define DIFF_A2_A2_1X ADC_CH(0x04)