Move mcp23018 driver to core

[zvecr]

Description

• Reuse existing mcp23018 driver from sp111
• Add docs comments

Should really have done this after the sp111 port...

Types of Changes

• Core
• Bugfix
• New feature
• Enhancement/optimization
• Keyboard (addition or update)
• Keymap/layout/userspace (addition or update)
• Documentation

Checklist

• My code follows the code style of this project: C, Python
• I have read the PR Checklist document and have made the appropriate changes.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• I have tested the changes and verified that they work and don't break anything (as well as I can manage).

[Gigahawk]

I'm having trouble with using this on an STM32F405 + MCP23017, I've played around with it for a while and I have a patch that seems to work.
Gigahawk@4220109

This shouldn't really affect anything in cases where the existing code already worked.

From 422010997ae8d671d8c2a3d50726e275f4f4de6b Mon Sep 17 00:00:00 2001
From: Jasper Chan <[email protected]>
Date: Wed, 19 Jan 2022 21:10:17 -0800
Subject: [PATCH] mcp23018: Ensure IOCON is zeroed on init

---
 drivers/gpio/mcp23018.c | 41 ++++++++++++++++++++++++++++++++++-------
 drivers/gpio/mcp23018.h |  2 +-
 2 files changed, 35 insertions(+), 8 deletions(-)

diff --git a/drivers/gpio/mcp23018.c b/drivers/gpio/mcp23018.c
index dc8ab03c50..b6481739b8 100644
--- a/drivers/gpio/mcp23018.c
+++ b/drivers/gpio/mcp23018.c
@@ -10,22 +10,49 @@
 #define TIMEOUT 100
 
 enum {
-    CMD_IODIRA = 0x00,  // i/o direction register
-    CMD_IODIRB = 0x01,
-    CMD_GPPUA  = 0x0C,  // GPIO pull-up resistor register
-    CMD_GPPUB  = 0x0D,
-    CMD_GPIOA  = 0x12,  // general purpose i/o port register (write modifies OLAT)
-    CMD_GPIOB  = 0x13,
+    CMD_IODIRA      = 0x00,  // i/o direction register
+    CMD_IODIRB      = 0x01,
+    CMD_GPPUA       = 0x0C,  // GPIO pull-up resistor register
+    CMD_GPPUB       = 0x0D,
+    CMD_GPIOA       = 0x12,  // general purpose i/o port register (write modifies OLAT)
+    CMD_GPIOB       = 0x13,
+    CMD_IOCON_BANK1 = 0x05,  // IOCON address when IOCON.BANK = 1
+    CMD_IOCON       = 0x0A,  // Regular IOCON address
 };
 
-void mcp23018_init(uint8_t addr) {
+bool mcp23018_init(uint8_t slave_addr) {
     static uint8_t s_init = 0;
+    uint8_t        addr   = SLAVE_TO_ADDR(slave_addr);
+    uint8_t        conf   = 0;
+    uint8_t        cmd;
+    i2c_status_t   ret;
     if (!s_init) {
         i2c_init();
         wait_ms(1000);
 
         s_init = 1;
     }
+
+    // While IOCON.BANK (and the rest of IOCON) is supposed to be 0 on POR,
+    // it appears that this is not guaranteed.
+    // We want to zero the IOCON register to make sure we are in a known state.
+    // Address CMD_IOCON_BANK1 points to GPINTENB when IOCON.BANK = 0;
+    // both registers are safe to zero during initialization.
+    cmd = CMD_IOCON_BANK1;
+    ret = i2c_writeReg(addr, cmd, &conf, sizeof(conf), TIMEOUT);
+    if (ret != I2C_STATUS_SUCCESS) {
+        dprintf("mcp23018_init::IOCONBANK1FAILED::%u\n", ret);
+        return false;
+    }
+
+    wait_ms(1);
+    cmd = CMD_IOCON;
+    ret = i2c_writeReg(addr, cmd, &conf, sizeof(conf), TIMEOUT);
+    if (ret != I2C_STATUS_SUCCESS) {
+        dprintf("mcp23018_init::IOCONBANK0FAILED::%u\n", ret);
+        return false;
+    }
+    return true;
 }
 
 bool mcp23018_set_config(uint8_t slave_addr, mcp23018_port_t port, uint8_t conf) {
diff --git a/drivers/gpio/mcp23018.h b/drivers/gpio/mcp23018.h
index e7c2730dd1..081fdb24e3 100644
--- a/drivers/gpio/mcp23018.h
+++ b/drivers/gpio/mcp23018.h
@@ -33,7 +33,7 @@ enum {
 /**
  * Init expander and any other dependent drivers
  */
-void mcp23018_init(uint8_t slave_addr);
+bool mcp23018_init(uint8_t slave_addr);
 
 /**
  * Configure input/output to a given port
-- 
2.33.0

Note the wait between the register writes seems to be necessary, everything seems to hang if that isn't there.
.Maybe you need to wait for the MCP23018 to apply the settings or something.

I haven't tried lower wait times, but for an init function 1 ms should be fine.

[zvecr]

For what its worth, I didnt hit any issues when testing on f303.

And given the amount of keyboards already in the repo that are running without the suggested fix, I question if there is something else in play.

[Gigahawk]

Fair enough, I'm just suggesting this change because it shouldn't have an effect on boards that don't experience this problem.

It may also help in other edge cases such as an MCU reset without resetting the expander.