From a98070f21288fae025c6ffa2e08b775fb661bb4b Mon Sep 17 00:00:00 2001 From: Ryan Date: Tue, 21 Jan 2025 09:53:35 +1100 Subject: [PATCH 1/2] [Docs] Correct I2C API reference (#24840) --- docs/drivers/i2c.md | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/docs/drivers/i2c.md b/docs/drivers/i2c.md index c806a090c56..2ef7afccc9d 100644 --- a/docs/drivers/i2c.md +++ b/docs/drivers/i2c.md @@ -147,7 +147,7 @@ void i2c_init(void) { --- -### `i2c_status_t i2c_transmit(uint8_t address, uint8_t *data, uint16_t length, uint16_t timeout)` {#api-i2c-transmit} +### `i2c_status_t i2c_transmit(uint8_t address, const uint8_t* data, uint16_t length, uint16_t timeout)` {#api-i2c-transmit} Send multiple bytes to the selected I2C device. @@ -155,7 +155,7 @@ Send multiple bytes to the selected I2C device. - `uint8_t address` The 7-bit I2C address of the device. - - `uint8_t *data` + - `const uint8_t* data` A pointer to the data to transmit. - `uint16_t length` The number of bytes to write. Take care not to overrun the length of `data`. @@ -176,7 +176,7 @@ Receive multiple bytes from the selected I2C device. - `uint8_t address` The 7-bit I2C address of the device. - - `uint8_t *data` + - `uint8_t* data` A pointer to the buffer to read into. - `uint16_t length` The number of bytes to read. Take care not to overrun the length of `data`. @@ -189,7 +189,7 @@ Receive multiple bytes from the selected I2C device. --- -### `i2c_status_t i2c_write_register(uint8_t devaddr, uint8_t regaddr, uint8_t* data, uint16_t length, uint16_t timeout)` {#api-i2c-write-register} +### `i2c_status_t i2c_write_register(uint8_t devaddr, uint8_t regaddr, const uint8_t* data, uint16_t length, uint16_t timeout)` {#api-i2c-write-register} Writes to a register with an 8-bit address on the I2C device. @@ -199,7 +199,7 @@ Writes to a register with an 8-bit address on the I2C device. The 7-bit I2C address of the device. - `uint8_t regaddr` The register address to write to. - - `uint8_t *data` + - `const uint8_t* data` A pointer to the data to transmit. - `uint16_t length` The number of bytes to write. Take care not to overrun the length of `data`. @@ -212,7 +212,7 @@ Writes to a register with an 8-bit address on the I2C device. --- -### `i2c_status_t i2c_write_register16(uint8_t devaddr, uint16_t regaddr, uint8_t* data, uint16_t length, uint16_t timeout)` {#api-i2c-write-register16} +### `i2c_status_t i2c_write_register16(uint8_t devaddr, uint16_t regaddr, const uint8_t* data, uint16_t length, uint16_t timeout)` {#api-i2c-write-register16} Writes to a register with a 16-bit address (big endian) on the I2C device. @@ -222,10 +222,10 @@ Writes to a register with a 16-bit address (big endian) on the I2C device. The 7-bit I2C address of the device. - `uint16_t regaddr` The register address to write to. - - `uint8_t *data` + - `const uint8_t* data` A pointer to the data to transmit. - `uint16_t length` - The number of bytes to write. Take care not to overrun the length of `data`. + The number of bytes to write. Take care not to overrun the length of `data`. - `uint16_t timeout` The time in milliseconds to wait for a response from the target device. @@ -245,8 +245,10 @@ Reads from a register with an 8-bit address on the I2C device. The 7-bit I2C address of the device. - `uint8_t regaddr` The register address to read from. + - `uint8_t data` + A pointer to a buffer to read into. - `uint16_t length` - The number of bytes to read. Take care not to overrun the length of `data`. + The number of bytes to read. Take care not to overrun the length of `data`. - `uint16_t timeout` The time in milliseconds to wait for a response from the target device. @@ -266,8 +268,10 @@ Reads from a register with a 16-bit address (big endian) on the I2C device. The 7-bit I2C address of the device. - `uint16_t regaddr` The register address to read from. + - `uint8_t* data` + A pointer to a buffer to read into. - `uint16_t length` - The number of bytes to read. Take care not to overrun the length of `data`. + The number of bytes to read. Take care not to overrun the length of `data`. - `uint16_t timeout` The time in milliseconds to wait for a response from the target device. @@ -279,19 +283,19 @@ Reads from a register with a 16-bit address (big endian) on the I2C device. ### `i2c_status_t i2c_ping_address(uint8_t address, uint16_t timeout)` {#api-i2c-ping-address} -Pings the I2C bus for a specific address. +Pings the I2C bus for a specific address. -On ChibiOS a "best effort" attempt is made by reading a single byte from register 0 at the requested address. This should generally work except for I2C devices that do not not respond to a register 0 read request, which will result in a false negative result (unsucessful response to ping attempt). +On ChibiOS a "best effort" attempt is made by reading a single byte from register 0 at the requested address. This should generally work except for I2C devices that do not not respond to a register 0 read request, which will result in a false negative result (unsuccessful response to ping attempt). -This function is weakly defined, meaning it can be overridden if necessary for your particular use case: +This function is weakly defined, meaning it can be overridden if necessary for your particular use case. -#### Arguments +#### Arguments {#api-i2c-ping-address-arguments} - `uint8_t address` The 7-bit I2C address of the device (ie. without the read/write bit - this will be set automatically). - `uint16_t timeout` The time in milliseconds to wait for a response from the target device. -#### Return Value +#### Return Value {#api-i2c-ping-address-return} `I2C_STATUS_TIMEOUT` if the timeout period elapses, `I2C_STATUS_ERROR` if some other error occurs, otherwise `I2C_STATUS_SUCCESS`. From 68130cc8a5557ac997a4aa6c95c7f4c67d07229b Mon Sep 17 00:00:00 2001 From: Ryan Date: Tue, 21 Jan 2025 09:53:46 +1100 Subject: [PATCH 2/2] `ferris/0_1`: update I2C API usage (#24839) --- keyboards/ferris/0_1/matrix.c | 45 +++++++++++++----------------- keyboards/ferris/0_2/matrix.c | 3 +- platforms/avr/drivers/i2c_master.h | 3 -- 3 files changed, 21 insertions(+), 30 deletions(-) diff --git a/keyboards/ferris/0_1/matrix.c b/keyboards/ferris/0_1/matrix.c index a3e2bebba6c..c05b193c1a0 100644 --- a/keyboards/ferris/0_1/matrix.c +++ b/keyboards/ferris/0_1/matrix.c @@ -29,7 +29,7 @@ along with this program. If not, see . #include "i2c_master.h" extern i2c_status_t mcp23017_status; -#define I2C_TIMEOUT 1000 +#define MCP23017_I2C_TIMEOUT 1000 // For a better understanding of the i2c protocol, this is a good read: // https://www.robot-electronics.co.uk/i2c-tutorial @@ -41,9 +41,7 @@ extern i2c_status_t mcp23017_status; // All address pins of the mcp23017 are connected to the ground on the ferris // | 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) +#define I2C_ADDR (0b0100000 << 1) // Register addresses // See https://github.com/adafruit/Adafruit-MCP23017-Arduino-Library/blob/master/Adafruit_MCP23017.h @@ -51,8 +49,8 @@ extern i2c_status_t mcp23017_status; #define IODIRB 0x01 #define GPPUA 0x0C // GPIO pull-up resistor register #define GPPUB 0x0D -#define GPIOA 0x12 // general purpose i/o port register (write modifies OLAT) -#define GPIOB 0x13 +#define MCP23017_GPIOA 0x12 // general purpose i/o port register (write modifies OLAT) +#define MCP23017_GPIOB 0x13 #define OLATA 0x14 // output latch register #define OLATB 0x15 @@ -60,14 +58,14 @@ bool i2c_initialized = 0; i2c_status_t mcp23017_status = I2C_ADDR; uint8_t init_mcp23017(void) { - print("starting init"); + print("init mcp23017\n"); mcp23017_status = I2C_ADDR; // I2C subsystem if (i2c_initialized == 0) { i2c_init(); // on pins D(1,0) i2c_initialized = true; - wait_ms(I2C_TIMEOUT); + wait_ms(MCP23017_I2C_TIMEOUT); } // set pin direction @@ -76,8 +74,10 @@ uint8_t init_mcp23017(void) { // - driving : output : 0 // This means: we will read all the bits on GPIOA // This means: we will write to the pins 0-4 on GPIOB (in select_rows) - uint8_t buf[] = {IODIRA, 0b11111111, 0b11110000}; - mcp23017_status = i2c_transmit(I2C_ADDR_WRITE, buf, sizeof(buf), I2C_TIMEOUT); + uint8_t buf[] = {0b11111111, 0b11110000}; + print("before transmit\n"); + mcp23017_status = i2c_write_register(I2C_ADDR, IODIRA, buf, sizeof(buf), MCP23017_I2C_TIMEOUT); + uprintf("after transmit %i\n", mcp23017_status); if (!mcp23017_status) { // set pull-up // - unused : on : 1 @@ -85,8 +85,8 @@ uint8_t init_mcp23017(void) { // - driving : off : 0 // This means: we will read all the bits on GPIOA // This means: we will write to the pins 0-4 on GPIOB (in select_rows) - uint8_t pullup_buf[] = {GPPUA, 0b11111111, 0b11110000}; - mcp23017_status = i2c_transmit(I2C_ADDR_WRITE, pullup_buf, sizeof(pullup_buf), I2C_TIMEOUT); + mcp23017_status = i2c_write_register(I2C_ADDR, GPPUA, buf, sizeof(buf), MCP23017_I2C_TIMEOUT); + uprintf("after transmit2 %i\n", mcp23017_status); } return mcp23017_status; } @@ -144,12 +144,12 @@ bool matrix_scan_custom(matrix_row_t current_matrix[]) { // if (++mcp23017_reset_loop >= 1300) { // since mcp23017_reset_loop is 8 bit - we'll try to reset once in 255 matrix scans // this will be approx bit more frequent than once per second - dprint("trying to reset mcp23017\n"); + print("trying to reset mcp23017\n"); mcp23017_status = init_mcp23017(); if (mcp23017_status) { - dprint("right side not responding\n"); + print("right side not responding\n"); } else { - dprint("right side attached\n"); + print("right side attached\n"); } } } @@ -204,18 +204,13 @@ static matrix_row_t read_cols(uint8_t row) { if (mcp23017_status) { // if there was an error return 0; } else { - uint8_t buf[] = {GPIOA}; - mcp23017_status = i2c_transmit(I2C_ADDR_WRITE, buf, sizeof(buf), I2C_TIMEOUT); // 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. // Since the pins connected to eact columns are sequential, and counting from zero up (col 5 -> GPIOA0, col 6 -> GPIOA1 and so on), the only transformation needed is a bitwise not to swap all zeroes and ones. uint8_t data[] = {0}; - if (!mcp23017_status) { - mcp23017_status = i2c_receive(I2C_ADDR_READ, data, sizeof(data), I2C_TIMEOUT); - data[0] = ~(data[0]); - } - return data[0]; + mcp23017_status = i2c_read_register(I2C_ADDR, MCP23017_GPIOA, data, sizeof(data), MCP23017_I2C_TIMEOUT); + return ~data[0]; } } } @@ -236,7 +231,7 @@ static void unselect_rows(void) { static void select_row(uint8_t row) { if (row < MATRIX_ROWS_PER_SIDE) { - // select on atmega32u4 + // select on MCU pin_t matrix_row_pins_mcu[MATRIX_ROWS_PER_SIDE] = MATRIX_ROW_PINS_MCU; pin_t pin = matrix_row_pins_mcu[row]; gpio_set_pin_output(pin); @@ -248,8 +243,8 @@ static void select_row(uint8_t row) { } else { // 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. - uint8_t buf[] = {GPIOB, 0xFF & ~(1 << (row - MATRIX_ROWS_PER_SIDE))}; - mcp23017_status = i2c_transmit(I2C_ADDR_WRITE, buf, sizeof(buf), I2C_TIMEOUT); + uint8_t buf[] = {0xFF & ~(1 << (row - MATRIX_ROWS_PER_SIDE))}; + mcp23017_status = i2c_write_register(I2C_ADDR, MCP23017_GPIOB, buf, sizeof(buf), MCP23017_I2C_TIMEOUT); } } } diff --git a/keyboards/ferris/0_2/matrix.c b/keyboards/ferris/0_2/matrix.c index 74fab717a1a..76bbcaf7b5f 100644 --- a/keyboards/ferris/0_2/matrix.c +++ b/keyboards/ferris/0_2/matrix.c @@ -30,8 +30,7 @@ along with this program. If not, see . extern i2c_status_t mcp23017_status; #define MCP23017_I2C_TIMEOUT 1000 -#define I2C_WRITE 0x00 -#define I2C_READ 0x01 + // For a better understanding of the i2c protocol, this is a good read: // https://www.robot-electronics.co.uk/i2c-tutorial diff --git a/platforms/avr/drivers/i2c_master.h b/platforms/avr/drivers/i2c_master.h index 258bb2b9bf2..ad92caa55aa 100644 --- a/platforms/avr/drivers/i2c_master.h +++ b/platforms/avr/drivers/i2c_master.h @@ -21,9 +21,6 @@ #include -#define I2C_READ 0x01 -#define I2C_WRITE 0x00 - typedef int16_t i2c_status_t; #define I2C_STATUS_SUCCESS (0)