Align existing pca9555 driver to better match mcp23018 API (#16277)

6 files changed, 114 insertions, 63 deletions

diff --git a/drivers/gpio/pca9555.c b/drivers/gpio/pca9555.c
index 02b5abbdde..adcd040083 100644
--- a/drivers/gpio/pca9555.c
+++ b/drivers/gpio/pca9555.c
@@ -1,18 +1,6 @@
-/* Copyright 2019
- *
- * This program is free software: you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation, either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program.  If not, see <http://www.gnu.org/licenses/>.
- */
+// Copyright 2020 zvecr<git@zvecr.com>
+// SPDX-License-Identifier: GPL-2.0-or-later
+
 #include "i2c_master.h"
 #include "pca9555.h"
 
@@ -45,39 +33,59 @@ void pca9555_init(uint8_t slave_addr) {
     // i2c_stop();
 }
 
-void pca9555_set_config(uint8_t slave_addr, uint8_t port, uint8_t conf) {
+bool pca9555_set_config(uint8_t slave_addr, pca9555_port_t port, uint8_t conf) {
     uint8_t addr = SLAVE_TO_ADDR(slave_addr);
     uint8_t cmd  = port ? CMD_CONFIG_1 : CMD_CONFIG_0;
 
     i2c_status_t ret = i2c_writeReg(addr, cmd, &conf, sizeof(conf), TIMEOUT);
     if (ret != I2C_STATUS_SUCCESS) {
         print("pca9555_set_config::FAILED\n");
+        return false;
     }
+
+    return true;
 }
 
-void pca9555_set_output(uint8_t slave_addr, uint8_t port, uint8_t conf) {
+bool pca9555_set_output(uint8_t slave_addr, pca9555_port_t port, uint8_t conf) {
     uint8_t addr = SLAVE_TO_ADDR(slave_addr);
     uint8_t cmd  = port ? CMD_OUTPUT_1 : CMD_OUTPUT_0;
 
     i2c_status_t ret = i2c_writeReg(addr, cmd, &conf, sizeof(conf), TIMEOUT);
     if (ret != I2C_STATUS_SUCCESS) {
         print("pca9555_set_output::FAILED\n");
+        return false;
     }
+
+    return true;
 }
 
-uint8_t pca9555_readPins(uint8_t slave_addr, uint8_t port) {
+bool pca9555_set_output_all(uint8_t slave_addr, uint8_t confA, uint8_t confB) {
+    uint8_t addr    = SLAVE_TO_ADDR(slave_addr);
+    uint8_t conf[2] = {confA, confB};
+
+    i2c_status_t ret = i2c_writeReg(addr, CMD_OUTPUT_0, &conf[0], sizeof(conf), TIMEOUT);
+    if (ret != I2C_STATUS_SUCCESS) {
+        dprintf("pca9555_set_output::FAILED::%u\n", ret);
+        return false;
+    }
+
+    return true;
+}
+
+bool pca9555_readPins(uint8_t slave_addr, pca9555_port_t port, uint8_t* out) {
     uint8_t addr = SLAVE_TO_ADDR(slave_addr);
     uint8_t cmd  = port ? CMD_INPUT_1 : CMD_INPUT_0;
 
-    uint8_t      data = 0;
-    i2c_status_t ret  = i2c_readReg(addr, cmd, &data, sizeof(data), TIMEOUT);
+    i2c_status_t ret = i2c_readReg(addr, cmd, out, sizeof(uint8_t), TIMEOUT);
     if (ret != I2C_STATUS_SUCCESS) {
         print("pca9555_readPins::FAILED\n");
+        return false;
     }
-    return data;
+
+    return true;
 }
 
-uint16_t pca9555_readAllPins(uint8_t slave_addr) {
+bool pca9555_readPins_all(uint8_t slave_addr, uint16_t* out) {
     uint8_t addr = SLAVE_TO_ADDR(slave_addr);
 
     typedef union {
@@ -85,11 +93,14 @@ uint16_t pca9555_readAllPins(uint8_t slave_addr) {
         uint16_t u16;
     } data16;
 
-    data16 data;
+    data16 data = {.u16 = 0};
 
     i2c_status_t ret = i2c_readReg(addr, CMD_INPUT_0, &data.u8[0], sizeof(data), TIMEOUT);
     if (ret != I2C_STATUS_SUCCESS) {
-        print("pca9555_readAllPins::FAILED\n");
+        print("pca9555_readPins_all::FAILED\n");
+        return false;
     }
-    return data.u16;
+
+    *out = data.u16;
+    return true;
 }
diff --git a/drivers/gpio/pca9555.h b/drivers/gpio/pca9555.h
index 3341ec3eb5..6362ab68ae 100644
--- a/drivers/gpio/pca9555.h
+++ b/drivers/gpio/pca9555.h
@@ -1,20 +1,11 @@
-/* Copyright 2019
- *
- * This program is free software: you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation, either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program.  If not, see <http://www.gnu.org/licenses/>.
- */
+// Copyright 2020 zvecr<git@zvecr.com>
+// SPDX-License-Identifier: GPL-2.0-or-later
+
 #pragma once
 
+#include <stdint.h>
+#include <stdbool.h>
+
 /*
             PCA9555
          ,----------.
@@ -38,20 +29,60 @@
          `----------'
 */
 
-#define PCA9555_PORT0 0
-#define PCA9555_PORT1 1
+/**
+ * Port ID
+ */
+typedef enum {
+    PCA9555_PORT0,
+    PCA9555_PORT1,
+} pca9555_port_t;
 
-#define ALL_OUTPUT 0
-#define ALL_INPUT 0xFF
-#define ALL_LOW 0
-#define ALL_HIGH 0xFF
+/**
+ * Helpers for set_config
+ */
+enum {
+    ALL_OUTPUT = 0,
+    ALL_INPUT  = 0xFF,
+};
+
+/**
+ * Helpers for set_output
+ */
+enum {
+    ALL_LOW  = 0,
+    ALL_HIGH = 0xFF,
+};
 
+/**
+ * Init expander and any other dependent drivers
+ */
 void pca9555_init(uint8_t slave_addr);
 
-void pca9555_set_config(uint8_t slave_addr, uint8_t port, uint8_t conf);
+/**
+ * Configure input/output to a given port
+ */
+bool pca9555_set_config(uint8_t slave_addr, pca9555_port_t port, uint8_t conf);
 
-void pca9555_set_output(uint8_t slave_addr, uint8_t port, uint8_t conf);
+/**
+ * Write high/low to a given port
+ */
+bool pca9555_set_output(uint8_t slave_addr, pca9555_port_t port, uint8_t conf);
 
-uint8_t pca9555_readPins(uint8_t slave_addr, uint8_t port);
+/**
+ * Write high/low to both ports sequentially
+ *
+ *  - slightly faster than multiple set_output
+ */
+bool pca9555_set_output_all(uint8_t slave_addr, uint8_t confA, uint8_t confB);
 
-uint16_t pca9555_readAllPins(uint8_t slave_addr);
+/**
+ * Read state of a given port
+ */
+bool pca9555_readPins(uint8_t slave_addr, pca9555_port_t port, uint8_t* ret);
+
+/**
+ * Read state of both ports sequentially
+ *
+ *  - slightly faster than multiple readPins
+ */
+bool pca9555_readPins_all(uint8_t slave_addr, uint16_t* ret);
diff --git a/keyboards/moon/matrix.c b/keyboards/moon/matrix.c
index 24b4d49560..0615b60ad3 100644
--- a/keyboards/moon/matrix.c
+++ b/keyboards/moon/matrix.c
@@ -158,10 +158,12 @@ static void select_row(uint8_t row) {
 }
 
 static uint16_t read_cols(void) {
-  uint16_t state_1 = pca9555_readPins(IC2, PCA9555_PORT0);
-  uint16_t state_2 = pca9555_readPins(IC2, PCA9555_PORT1);
+  uint8_t state_1 = 0;
+  uint8_t state_2 = 0;
+  pca9555_readPins(IC2, PCA9555_PORT0, &state_1);
+  pca9555_readPins(IC2, PCA9555_PORT1, &state_2);
 
-  uint16_t state = ((state_1 & PORT0_COLS_MASK) << 3) | ((state_2 & PORT1_COLS_MASK));
+  uint16_t state = (((uint16_t)state_1 & PORT0_COLS_MASK) << 3) | (((uint16_t)state_2 & PORT1_COLS_MASK));
 
   // A low pin indicates an active column
   return (~state) & COLS_MASK;
diff --git a/keyboards/switchplate/southpaw_65/matrix.c b/keyboards/switchplate/southpaw_65/matrix.c
index e701d274f0..5895750f89 100644
--- a/keyboards/switchplate/southpaw_65/matrix.c
+++ b/keyboards/switchplate/southpaw_65/matrix.c
@@ -51,11 +51,14 @@ static void select_row(uint8_t row) {
 
 static uint32_t read_cols(void) {
     //Read column inputs. Pins 13-31 are used. Split across both ICs but they are sequential
-    uint32_t state_1 = pca9555_readPins(IC1, PCA9555_PORT1);
-    uint32_t state_2 = pca9555_readPins(IC2, PCA9555_PORT0);
-    uint32_t state_3 = pca9555_readPins(IC2, PCA9555_PORT1);
-
-    uint32_t state = (((state_3 & 0b01111111) << 12) | (state_2 << 4) | ((state_1 & 0b11110000) >> 4));
+    uint8_t state_1 = 0;
+    uint8_t state_2 = 0;
+    uint8_t state_3 = 0;
+    pca9555_readPins(IC2, PCA9555_PORT0, &state_1);
+    pca9555_readPins(IC2, PCA9555_PORT1, &state_2);
+    pca9555_readPins(IC1, PCA9555_PORT1, &state_3);
+
+    uint32_t state = ((((uint32_t)state_3 & 0b01111111) << 12) | ((uint32_t)state_2 << 4) | (((uint32_t)state_1 & 0b11110000) >> 4));
     return ~state;
 }
 
diff --git a/keyboards/xiudi/xd84/matrix.c b/keyboards/xiudi/xd84/matrix.c
index 92b8ff8546..04128561ee 100644
--- a/keyboards/xiudi/xd84/matrix.c
+++ b/keyboards/xiudi/xd84/matrix.c
@@ -53,7 +53,8 @@ static void select_row(uint8_t row) {
 static uint16_t read_cols(void) {
     // uint16_t state_1 = pca9555_readPins(IC2, PCA9555_PORT0);
     // uint16_t state_2 = pca9555_readPins(IC2, PCA9555_PORT1);
-    uint16_t state = pca9555_readAllPins(IC2);
+    uint16_t state = 0;
+    pca9555_readPins_all(IC2, &state);
 
     // For the XD84 all cols are on the same IC and mapped sequentially
     // while this technically gives 16 column reads,
diff --git a/keyboards/xiudi/xd96/matrix.c b/keyboards/xiudi/xd96/matrix.c
index 8cecc79c26..beef7fae12 100644
--- a/keyboards/xiudi/xd96/matrix.c
+++ b/keyboards/xiudi/xd96/matrix.c
@@ -50,13 +50,16 @@ static void select_row(uint8_t row) {
 }
 
 static uint32_t read_cols(void) {
-  uint32_t state_1 = pca9555_readPins(IC2, PCA9555_PORT0);
-  uint32_t state_2 = pca9555_readPins(IC2, PCA9555_PORT1);
-  uint32_t state_3 = pca9555_readPins(IC1, PCA9555_PORT1);
+  uint8_t state_1 = 0;
+  uint8_t state_2 = 0;
+  uint8_t state_3 = 0;
+  pca9555_readPins(IC2, PCA9555_PORT0, &state_1);
+  pca9555_readPins(IC2, PCA9555_PORT1, &state_2);
+  pca9555_readPins(IC1, PCA9555_PORT1, &state_3);
 
   // For the XD96 the pins are mapped to port expanders as follows:
   //   all 8 pins port 0 IC2, first 6 pins port 1 IC2, first 4 pins port 1 IC1
-  uint32_t state = (((state_3 & 0b00001111) << 14) | ((state_2 & 0b00111111) << 8) | state_1);
+  uint32_t state = ((((uint32_t)state_3 & 0b00001111) << 14) | (((uint32_t)state_2 & 0b00111111) << 8) | (uint32_t)state_1);
   return (~state) & 0b111111111111111111;
 }