From e7a5c006d9777a4009da934f408961aa2d2e6fb1 Mon Sep 17 00:00:00 2001 From: Joel Challis Date: Tue, 7 Sep 2021 16:34:59 +0100 Subject: [PATCH 1/2] Refactor use of legacy i2c implementation (#14341) --- keyboards/sx60/i2cmaster.h | 178 ------------------------------- keyboards/sx60/matrix.c | 21 ++-- keyboards/sx60/rules.mk | 4 +- keyboards/sx60/sx60.c | 27 +++-- keyboards/sx60/sx60.h | 5 +- keyboards/sx60/twimaster.c | 207 ------------------------------------- 6 files changed, 23 insertions(+), 419 deletions(-) delete mode 100644 keyboards/sx60/i2cmaster.h delete mode 100644 keyboards/sx60/twimaster.c diff --git a/keyboards/sx60/i2cmaster.h b/keyboards/sx60/i2cmaster.h deleted file mode 100644 index 3917b9e6c..000000000 --- a/keyboards/sx60/i2cmaster.h +++ /dev/null @@ -1,178 +0,0 @@ -#ifndef _I2CMASTER_H -#define _I2CMASTER_H 1 -/************************************************************************* -* Title: C include file for the I2C master interface -* (i2cmaster.S or twimaster.c) -* Author: Peter Fleury http://jump.to/fleury -* File: $Id: i2cmaster.h,v 1.10 2005/03/06 22:39:57 Peter Exp $ -* Software: AVR-GCC 3.4.3 / avr-libc 1.2.3 -* Target: any AVR device -* Usage: see Doxygen manual -**************************************************************************/ - -#ifdef DOXYGEN -/** - @defgroup pfleury_ic2master I2C Master library - @code #include @endcode - - @brief I2C (TWI) Master Software Library - - Basic routines for communicating with I2C slave devices. This single master - implementation is limited to one bus master on the I2C bus. - - This I2c library is implemented as a compact assembler software implementation of the I2C protocol - which runs on any AVR (i2cmaster.S) and as a TWI hardware interface for all AVR with built-in TWI hardware (twimaster.c). - Since the API for these two implementations is exactly the same, an application can be linked either against the - software I2C implementation or the hardware I2C implementation. - - Use 4.7k pull-up resistor on the SDA and SCL pin. - - Adapt the SCL and SDA port and pin definitions and eventually the delay routine in the module - i2cmaster.S to your target when using the software I2C implementation ! - - Adjust the CPU clock frequence F_CPU in twimaster.c or in the Makfile when using the TWI hardware implementaion. - - @note - The module i2cmaster.S is based on the Atmel Application Note AVR300, corrected and adapted - to GNU assembler and AVR-GCC C call interface. - Replaced the incorrect quarter period delays found in AVR300 with - half period delays. - - @author Peter Fleury pfleury@gmx.ch http://jump.to/fleury - - @par API Usage Example - The following code shows typical usage of this library, see example test_i2cmaster.c - - @code - - #include - - - #define Dev24C02 0xA2 // device address of EEPROM 24C02, see datasheet - - int main(void) - { - unsigned char ret; - - i2c_init(); // initialize I2C library - - // write 0x75 to EEPROM address 5 (Byte Write) - i2c_start_wait(Dev24C02+I2C_WRITE); // set device address and write mode - i2c_write(0x05); // write address = 5 - i2c_write(0x75); // write value 0x75 to EEPROM - i2c_stop(); // set stop conditon = release bus - - - // read previously written value back from EEPROM address 5 - i2c_start_wait(Dev24C02+I2C_WRITE); // set device address and write mode - - i2c_write(0x05); // write address = 5 - i2c_rep_start(Dev24C02+I2C_READ); // set device address and read mode - - ret = i2c_readNak(); // read one byte from EEPROM - i2c_stop(); - - for(;;); - } - @endcode - -*/ -#endif /* DOXYGEN */ - -/**@{*/ - -#if (__GNUC__ * 100 + __GNUC_MINOR__) < 304 -#error "This library requires AVR-GCC 3.4 or later, update to newer AVR-GCC compiler !" -#endif - -#include - -/** defines the data direction (reading from I2C device) in i2c_start(),i2c_rep_start() */ -#define I2C_READ 1 - -/** defines the data direction (writing to I2C device) in i2c_start(),i2c_rep_start() */ -#define I2C_WRITE 0 - - -/** - @brief initialize the I2C master interace. Need to be called only once - @param void - @return none - */ -extern void i2c_init(void); - - -/** - @brief Terminates the data transfer and releases the I2C bus - @param void - @return none - */ -extern void i2c_stop(void); - - -/** - @brief Issues a start condition and sends address and transfer direction - - @param addr address and transfer direction of I2C device - @retval 0 device accessible - @retval 1 failed to access device - */ -extern unsigned char i2c_start(unsigned char addr); - - -/** - @brief Issues a repeated start condition and sends address and transfer direction - - @param addr address and transfer direction of I2C device - @retval 0 device accessible - @retval 1 failed to access device - */ -extern unsigned char i2c_rep_start(unsigned char addr); - - -/** - @brief Issues a start condition and sends address and transfer direction - - If device is busy, use ack polling to wait until device ready - @param addr address and transfer direction of I2C device - @return none - */ -extern void i2c_start_wait(unsigned char addr); - - -/** - @brief Send one byte to I2C device - @param data byte to be transfered - @retval 0 write successful - @retval 1 write failed - */ -extern unsigned char i2c_write(unsigned char data); - - -/** - @brief read one byte from the I2C device, request more data from device - @return byte read from I2C device - */ -extern unsigned char i2c_readAck(void); - -/** - @brief read one byte from the I2C device, read is followed by a stop condition - @return byte read from I2C device - */ -extern unsigned char i2c_readNak(void); - -/** - @brief read one byte from the I2C device - - Implemented as a macro, which calls either i2c_readAck or i2c_readNak - - @param ack 1 send ack, request more data from device
- 0 send nak, read is followed by a stop condition - @return byte read from I2C device - */ -extern unsigned char i2c_read(unsigned char ack); -#define i2c_read(ack) (ack) ? i2c_readAck() : i2c_readNak(); - - -/**@}*/ -#endif diff --git a/keyboards/sx60/matrix.c b/keyboards/sx60/matrix.c index 6fa0dd145..b7dc25425 100644 --- a/keyboards/sx60/matrix.c +++ b/keyboards/sx60/matrix.c @@ -243,15 +243,11 @@ static bool read_cols_on_row(matrix_row_t current_matrix[], uint8_t current_row) /* if there was an error */ return 0; } else { - uint16_t data = 0; - mcp23018_status = i2c_start(I2C_ADDR_WRITE); if (mcp23018_status) goto out; - mcp23018_status = i2c_write(GPIOA); if (mcp23018_status) goto out; - mcp23018_status = i2c_start(I2C_ADDR_READ); if (mcp23018_status) goto out; - data = i2c_readNak(); - data = ~data; - out: - i2c_stop(); - current_matrix[current_row] |= (data << 8); + uint8_t data = 0; + mcp23018_status = i2c_readReg(I2C_ADDR, GPIOA, &data, 1, I2C_TIMEOUT); + if (!mcp23018_status) { + current_matrix[current_row] |= (~((uint16_t)data) << 8); + } } /* For each col... */ @@ -278,11 +274,8 @@ static void select_row(uint8_t row) /* set active row low : 0 set active row output : 1 set other rows hi-Z : 1 */ - mcp23018_status = i2c_start(I2C_ADDR_WRITE); if (mcp23018_status) goto out; - mcp23018_status = i2c_write(GPIOB); if (mcp23018_status) goto out; - mcp23018_status = i2c_write(0xFF & ~(1< #include -#include "i2cmaster.h" +#include "i2c_master.h" #include /* I2C aliases and register addresses (see "mcp23018.md") */ #define I2C_ADDR 0b0100000 -#define I2C_ADDR_WRITE ( (I2C_ADDR<<1) | I2C_WRITE ) -#define I2C_ADDR_READ ( (I2C_ADDR<<1) | I2C_READ ) +#define I2C_TIMEOUT 100 #define IODIRA 0x00 /* i/o direction register */ #define IODIRB 0x01 #define GPPUA 0x0C /* GPIO pull-up resistor register */ diff --git a/keyboards/sx60/twimaster.c b/keyboards/sx60/twimaster.c deleted file mode 100644 index 30d8c24bf..000000000 --- a/keyboards/sx60/twimaster.c +++ /dev/null @@ -1,207 +0,0 @@ -/************************************************************************* -* Title: I2C master library using hardware TWI interface -* Author: Peter Fleury http://jump.to/fleury -* File: $Id: twimaster.c,v 1.3 2005/07/02 11:14:21 Peter Exp $ -* Software: AVR-GCC 3.4.3 / avr-libc 1.2.3 -* Target: any AVR device with hardware TWI -* Usage: API compatible with I2C Software Library i2cmaster.h -**************************************************************************/ -#include -#include - -#include - -/* define CPU frequency in Hz here if not defined in Makefile */ -#ifndef F_CPU -#define F_CPU 16000000UL -#endif - -/* I2C clock in Hz */ -#define SCL_CLOCK 400000L - - -/************************************************************************* - Initialization of the I2C bus interface. Need to be called only once -*************************************************************************/ -void i2c_init(void) -{ - /* initialize TWI clock - * minimal values in Bit Rate Register (TWBR) and minimal Prescaler - * bits in the TWI Status Register should give us maximal possible - * I2C bus speed - about 444 kHz - * - * for more details, see 20.5.2 in ATmega16/32 secification - */ - - TWSR = 0; /* no prescaler */ - TWBR = 10; /* must be >= 10 for stable operation */ - -}/* i2c_init */ - - -/************************************************************************* - Issues a start condition and sends address and transfer direction. - return 0 = device accessible, 1= failed to access device -*************************************************************************/ -unsigned char i2c_start(unsigned char address) -{ - uint8_t twst; - - /* send START condition */ - TWCR = (1< Date: Tue, 7 Sep 2021 16:35:13 +0100 Subject: [PATCH 2/2] 3w6 - Refactor use of AVR only I2C functions (#14339) * Refactor use of legacy i2c functions * Align rev2 * Review fixes --- keyboards/3w6/rev1/matrix.c | 82 ++++++++++++++----------------------- keyboards/3w6/rev2/matrix.c | 75 +++++++++++++-------------------- 2 files changed, 58 insertions(+), 99 deletions(-) diff --git a/keyboards/3w6/rev1/matrix.c b/keyboards/3w6/rev1/matrix.c index 7262fd22e..af3d21067 100644 --- a/keyboards/3w6/rev1/matrix.c +++ b/keyboards/3w6/rev1/matrix.c @@ -35,8 +35,6 @@ extern i2c_status_t tca9555_status; // | 0 | 1 | 0 | 0 | A2 | A1 | A0 | // | 0 | 1 | 0 | 0 | 0 | 0 | 0 | #define I2C_ADDR 0b0100000 -#define I2C_ADDR_WRITE ((I2C_ADDR << 1) | I2C_WRITE) -#define I2C_ADDR_READ ((I2C_ADDR << 1) | I2C_READ) // Register addresses #define IODIRA 0x06 // i/o direction register @@ -64,19 +62,14 @@ uint8_t init_tca9555(void) { // - unused : input : 1 // - input : input : 1 // - driving : output : 0 - tca9555_status = i2c_start(I2C_ADDR_WRITE, I2C_TIMEOUT); - if (tca9555_status) goto out; - tca9555_status = i2c_write(IODIRA, I2C_TIMEOUT); - if (tca9555_status) goto out; - // This means: write on pin 5 of port 0, read on rest - tca9555_status = i2c_write(0b11011111, I2C_TIMEOUT); - if (tca9555_status) goto out; - // This means: we will write on pins 0 to 2 on port 1. read rest - tca9555_status = i2c_write(0b11111000, I2C_TIMEOUT); - if (tca9555_status) goto out; + uint8_t conf[2] = { + // This means: write on pin 5 of port 0, read on rest + 0b11011111, + // This means: we will write on pins 0 to 2 on port 1. read rest + 0b11111000, + }; + tca9555_status = i2c_writeReg(I2C_ADDR, IODIRA, conf, 2, I2C_TIMEOUT); -out: - i2c_stop(); return tca9555_status; } @@ -192,36 +185,29 @@ static matrix_row_t read_cols(uint8_t row) { if (tca9555_status) { // if there was an error return 0; } else { - uint8_t data = 0; - uint8_t port0 = 0; - uint8_t port1 = 0; - tca9555_status = i2c_start(I2C_ADDR_WRITE, I2C_TIMEOUT); - if (tca9555_status) goto out; - tca9555_status = i2c_write(IREGP0, I2C_TIMEOUT); - if (tca9555_status) goto out; - tca9555_status = i2c_start(I2C_ADDR_READ, I2C_TIMEOUT); - if (tca9555_status) goto out; - tca9555_status = i2c_read_ack(I2C_TIMEOUT); - if (tca9555_status < 0) goto out; - port0 = (uint8_t)tca9555_status; - tca9555_status = i2c_read_nack(I2C_TIMEOUT); - if (tca9555_status < 0) goto out; - port1 = (uint8_t)tca9555_status; + uint8_t data = 0; + uint8_t ports[2] = {0}; + tca9555_status = i2c_readReg(I2C_ADDR, IREGP0, ports, 2, I2C_TIMEOUT); + if (tca9555_status) { // if there was an error + // do nothing + return 0; + } else { + uint8_t port0 = ports[0]; + uint8_t port1 = ports[1]; - // The initial state was all ones and any depressed key at a given column for the currently selected row will have its bit flipped to zero. - // The return value is a row as represented in the generic matrix code were the rightmost bits represent the lower columns and zeroes represent non-depressed keys while ones represent depressed keys. - // Since the pins are not ordered sequentially, we have to build the correct dataset from the two ports. Refer to the schematic to see where every pin is connected. - data |= ( port0 & 0x01 ); - data |= ( port0 & 0x02 ); - data |= ( port1 & 0x10 ) >> 2; - data |= ( port1 & 0x08 ); - data |= ( port0 & 0x40 ) >> 2; - data = ~(data); + // The initial state was all ones and any depressed key at a given column for the currently selected row will have its bit flipped to zero. + // The return value is a row as represented in the generic matrix code were the rightmost bits represent the lower columns and zeroes represent non-depressed keys while ones represent depressed keys. + // Since the pins are not ordered sequentially, we have to build the correct dataset from the two ports. Refer to the schematic to see where every pin is connected. + data |= ( port0 & 0x01 ); + data |= ( port0 & 0x02 ); + data |= ( port1 & 0x10 ) >> 2; + data |= ( port1 & 0x08 ); + data |= ( port0 & 0x40 ) >> 2; + data = ~(data); - tca9555_status = I2C_STATUS_SUCCESS; - out: - i2c_stop(); - return data; + tca9555_status = I2C_STATUS_SUCCESS; + return data; + } } } } @@ -263,18 +249,10 @@ static void select_row(uint8_t row) { default: break; } - tca9555_status = i2c_start(I2C_ADDR_WRITE, I2C_TIMEOUT); - if (tca9555_status) goto out; - tca9555_status = i2c_write(OREGP0, I2C_TIMEOUT); - if (tca9555_status) goto out; - tca9555_status = i2c_write(port0, I2C_TIMEOUT); - if (tca9555_status) goto out; - tca9555_status = i2c_write(port1, I2C_TIMEOUT); - if (tca9555_status) goto out; + uint8_t ports[2] = {port0, port1}; + tca9555_status = i2c_writeReg(I2C_ADDR, OREGP0, ports, 2, I2C_TIMEOUT); // Select the desired row by writing a byte for the entire GPIOB bus where only the bit representing the row we want to select is a zero (write instruction) and every other bit is a one. // Note that the row - MATRIX_ROWS_PER_SIDE reflects the fact that being on the right hand, the columns are numbered from MATRIX_ROWS_PER_SIDE to MATRIX_ROWS, but the pins we want to write to are indexed from zero up on the GPIOB bus. - out: - i2c_stop(); } } } diff --git a/keyboards/3w6/rev2/matrix.c b/keyboards/3w6/rev2/matrix.c index 5bc967bed..4df161b89 100644 --- a/keyboards/3w6/rev2/matrix.c +++ b/keyboards/3w6/rev2/matrix.c @@ -35,8 +35,6 @@ extern i2c_status_t tca9555_status; // | 0 | 1 | 0 | 0 | A2 | A1 | A0 | // | 0 | 1 | 0 | 0 | 0 | 0 | 0 | #define I2C_ADDR 0b0100000 -#define I2C_ADDR_WRITE ((I2C_ADDR << 1) | I2C_WRITE) -#define I2C_ADDR_READ ((I2C_ADDR << 1) | I2C_READ) // Register addresses #define IODIRA 0x06 // i/o direction register @@ -64,19 +62,14 @@ uint8_t init_tca9555(void) { // - unused : input : 1 // - input : input : 1 // - driving : output : 0 - tca9555_status = i2c_start(I2C_ADDR_WRITE, I2C_TIMEOUT); - if (tca9555_status) goto out; - tca9555_status = i2c_write(IODIRA, I2C_TIMEOUT); - if (tca9555_status) goto out; - // This means: read all pins of port 0 - tca9555_status = i2c_write(0b11111111, I2C_TIMEOUT); - if (tca9555_status) goto out; - // This means: we will write on pins 0 to 3 on port 1. read rest - tca9555_status = i2c_write(0b11110000, I2C_TIMEOUT); - if (tca9555_status) goto out; + uint8_t conf[2] = { + // This means: read all pins of port 0 + 0b11111111, + // This means: we will write on pins 0 to 3 on port 1. read rest + 0b11110000, + }; + tca9555_status = i2c_writeReg(I2C_ADDR, IODIRA, conf, 2, I2C_TIMEOUT); -out: - i2c_stop(); return tca9555_status; } @@ -194,32 +187,27 @@ static matrix_row_t read_cols(uint8_t row) { } else { uint8_t data = 0; uint8_t port0 = 0; - tca9555_status = i2c_start(I2C_ADDR_WRITE, I2C_TIMEOUT); - if (tca9555_status) goto out; - tca9555_status = i2c_write(IREGP0, I2C_TIMEOUT); - if (tca9555_status) goto out; - tca9555_status = i2c_start(I2C_ADDR_READ, I2C_TIMEOUT); - if (tca9555_status) goto out; - tca9555_status = i2c_read_nack(I2C_TIMEOUT); - if (tca9555_status < 0) goto out; - - port0 = ~(uint8_t)tca9555_status; + tca9555_status = i2c_readReg(I2C_ADDR, IREGP0, port0, 1, I2C_TIMEOUT); + if (tca9555_status) { // if there was an error + // do nothing + return 0; + } else { + uint8_t port0 = ports[0]; + uint8_t port1 = ports[1]; - // We read all the pins on GPIOA. - // The initial state was all ones and any depressed key at a given column for the currently selected row will have its bit flipped to zero. - // The return value is a row as represented in the generic matrix code were the rightmost bits represent the lower columns and zeroes represent non-depressed keys while ones represent depressed keys. - // the pins connected to eact columns are sequential, but in reverse order, and counting from zero down (col 5 -> GPIO04, col6 -> GPIO03 and so on). - data |= ( port0 & 0x01 ) << 4; - data |= ( port0 & 0x02 ) << 2; - data |= ( port0 & 0x04 ); - data |= ( port0 & 0x08 ) >> 2; - data |= ( port0 & 0x10 ) >> 4; + // We read all the pins on GPIOA. + // The initial state was all ones and any depressed key at a given column for the currently selected row will have its bit flipped to zero. + // The return value is a row as represented in the generic matrix code were the rightmost bits represent the lower columns and zeroes represent non-depressed keys while ones represent depressed keys. + // the pins connected to eact columns are sequential, but in reverse order, and counting from zero down (col 5 -> GPIO04, col6 -> GPIO03 and so on). + data |= ( port0 & 0x01 ) << 4; + data |= ( port0 & 0x02 ) << 2; + data |= ( port0 & 0x04 ); + data |= ( port0 & 0x08 ) >> 2; + data |= ( port0 & 0x10 ) >> 4; - tca9555_status = I2C_STATUS_SUCCESS; - out: - i2c_stop(); - - return data; + tca9555_status = I2C_STATUS_SUCCESS; + return data; + } } } } @@ -256,20 +244,13 @@ static void select_row(uint8_t row) { case 4: port1 &= ~(1 << 0); break; case 5: port1 &= ~(1 << 1); break; case 6: port1 &= ~(1 << 2); break; - case 7: port1 &= ~(1 << 3); break; + case 7: port0 &= ~(1 << 5); break; default: break; } + tca9555_status = i2c_writeReg(I2C_ADDR, OREGP1, port1, 2, I2C_TIMEOUT); // Select the desired row by writing a byte for the entire GPIOB bus where only the bit representing the row we want to select is a zero (write instruction) and every other bit is a one. // Note that the row - MATRIX_ROWS_PER_SIDE reflects the fact that being on the right hand, the columns are numbered from MATRIX_ROWS_PER_SIDE to MATRIX_ROWS, but the pins we want to write to are indexed from zero up on the GPIOB bus. - tca9555_status = i2c_start(I2C_ADDR_WRITE, I2C_TIMEOUT); - if (tca9555_status) goto out; - tca9555_status = i2c_write(OREGP1, I2C_TIMEOUT); - if (tca9555_status) goto out; - tca9555_status = i2c_write(port1, I2C_TIMEOUT); - if (tca9555_status) goto out; - out: - i2c_stop(); } } }