From 53e2fab34c6df3d82725e1939f1e4d1cfd6375c6 Mon Sep 17 00:00:00 2001 From: Theo Arends <11044339+arendst@users.noreply.github.com> Date: Fri, 13 Dec 2024 17:20:36 +0100 Subject: [PATCH] Fix MCP23XXX driver open-drain interrupt --- .../tasmota_xdrv_driver/xdrv_67_mcp23xxx.ino | 77 +++++++++++-------- 1 file changed, 45 insertions(+), 32 deletions(-) diff --git a/tasmota/tasmota_xdrv_driver/xdrv_67_mcp23xxx.ino b/tasmota/tasmota_xdrv_driver/xdrv_67_mcp23xxx.ino index 18f7ad933..3e5968301 100644 --- a/tasmota/tasmota_xdrv_driver/xdrv_67_mcp23xxx.ino +++ b/tasmota/tasmota_xdrv_driver/xdrv_67_mcp23xxx.ino @@ -146,11 +146,25 @@ typedef struct { uint8_t olatb; uint8_t address; uint8_t interface; - uint8_t pins; // 8 (MCP23x08) or 16 (MCP23x17) + uint8_t pins; // 8 (MCP23x08) or 16 (MCP23x17) int8_t pin_cs; int8_t pin_int; } tMcp23xDevice; +typedef union { // Restricted by MISRA-C Rule 18.4 but so useful... + uint8_t reg; // Allow bit manipulation using template IOCON + struct { + uint8_t spare0 : 1; // 0 Unimplemented + uint8_t INTPOL : 1; // (0) INT pin active-low. (1) active-high + uint8_t ODR : 1; // (0) INT pin active driver output. (1) Open-drain output (overrides INTPOL) + uint8_t HAEN : 1; // (1) Hardware Address enabled (MCS23S17 only) + uint8_t DISSLW : 1; // (0) SDA output slew rate disabled + uint8_t SEQOP : 1; // 0 Sequential operation enabled, address pointer increments + uint8_t MIRROR : 1; // (1) INT pins are internally connected + uint8_t BANK : 1; // 0 Registers are in the same bank (addresses are sequential) (MCS23x17 only) + }; +} tIOCON; + struct MCP230 { tMcp23xDevice device[MCP23XXX_MAX_DEVICES]; uint32_t relay_inverted; @@ -162,7 +176,7 @@ struct MCP230 { uint8_t relay_offset; uint8_t button_max; uint8_t switch_max; - uint8_t iocon; + tIOCON iocon; int8_t button_offset; int8_t switch_offset; bool base; @@ -464,6 +478,15 @@ uint32_t MCP23xGetPin(uint32_t lpin) { /*********************************************************************************************/ +bool MCP23xAddItem(uint8_t &item) { + if (item >= MAX_RELAYS_SET) { + AddLog(LOG_LEVEL_INFO, PSTR("MCP: Max reached")); + return false; + } + item++; + return true; +} + String MCP23xTemplateLoadFile(void) { String mcptmplt = ""; #ifdef USE_UFILESYS @@ -514,44 +537,36 @@ bool MCP23xLoadTemplate(void) { if (!val) { break; } uint16_t mpin = val.getUInt(); if (mpin) { // Above GPIO_NONE - if ((mpin >= AGPIO(GPIO_SWT1)) && (mpin < (AGPIO(GPIO_SWT1) + MAX_SWITCHES_SET))) { - Mcp23x.switch_max++; + if ((mpin >= AGPIO(GPIO_SWT1)) && (mpin < (AGPIO(GPIO_SWT1) + MAX_SWITCHES_SET)) && MCP23xAddItem(Mcp23x.switch_max)) { MCP23xSetPinModes(pin, INPUT_PULLUP); } - else if ((mpin >= AGPIO(GPIO_SWT1_NP)) && (mpin < (AGPIO(GPIO_SWT1_NP) + MAX_SWITCHES_SET))) { + else if ((mpin >= AGPIO(GPIO_SWT1_NP)) && (mpin < (AGPIO(GPIO_SWT1_NP) + MAX_SWITCHES_SET)) && MCP23xAddItem(Mcp23x.switch_max)) { mpin -= (AGPIO(GPIO_SWT1_NP) - AGPIO(GPIO_SWT1)); - Mcp23x.switch_max++; MCP23xSetPinModes(pin, INPUT); } - else if ((mpin >= AGPIO(GPIO_KEY1)) && (mpin < (AGPIO(GPIO_KEY1) + MAX_KEYS_SET))) { - Mcp23x.button_max++; + else if ((mpin >= AGPIO(GPIO_KEY1)) && (mpin < (AGPIO(GPIO_KEY1) + MAX_KEYS_SET)) && MCP23xAddItem(Mcp23x.button_max)) { MCP23xSetPinModes(pin, INPUT_PULLUP); } - else if ((mpin >= AGPIO(GPIO_KEY1_NP)) && (mpin < (AGPIO(GPIO_KEY1_NP) + MAX_KEYS_SET))) { + else if ((mpin >= AGPIO(GPIO_KEY1_NP)) && (mpin < (AGPIO(GPIO_KEY1_NP) + MAX_KEYS_SET)) && MCP23xAddItem(Mcp23x.button_max)) { mpin -= (AGPIO(GPIO_KEY1_NP) - AGPIO(GPIO_KEY1)); - Mcp23x.button_max++; MCP23xSetPinModes(pin, INPUT); } - else if ((mpin >= AGPIO(GPIO_KEY1_INV)) && (mpin < (AGPIO(GPIO_KEY1_INV) + MAX_KEYS_SET))) { + else if ((mpin >= AGPIO(GPIO_KEY1_INV)) && (mpin < (AGPIO(GPIO_KEY1_INV) + MAX_KEYS_SET)) && MCP23xAddItem(Mcp23x.button_max)) { bitSet(Mcp23x.button_inverted, mpin - AGPIO(GPIO_KEY1_INV)); mpin -= (AGPIO(GPIO_KEY1_INV) - AGPIO(GPIO_KEY1)); - Mcp23x.button_max++; MCP23xSetPinModes(pin, INPUT_PULLUP); } - else if ((mpin >= AGPIO(GPIO_KEY1_INV_NP)) && (mpin < (AGPIO(GPIO_KEY1_INV_NP) + MAX_KEYS_SET))) { + else if ((mpin >= AGPIO(GPIO_KEY1_INV_NP)) && (mpin < (AGPIO(GPIO_KEY1_INV_NP) + MAX_KEYS_SET)) && MCP23xAddItem(Mcp23x.button_max)) { bitSet(Mcp23x.button_inverted, mpin - AGPIO(GPIO_KEY1_INV_NP)); mpin -= (AGPIO(GPIO_KEY1_INV_NP) - AGPIO(GPIO_KEY1)); - Mcp23x.button_max++; MCP23xSetPinModes(pin, INPUT); } - else if ((mpin >= AGPIO(GPIO_REL1)) && (mpin < (AGPIO(GPIO_REL1) + MAX_RELAYS_SET))) { - Mcp23x.relay_max++; + else if ((mpin >= AGPIO(GPIO_REL1)) && (mpin < (AGPIO(GPIO_REL1) + MAX_RELAYS_SET)) && MCP23xAddItem(Mcp23x.relay_max)) { MCP23xPinMode(pin, OUTPUT); } - else if ((mpin >= AGPIO(GPIO_REL1_INV)) && (mpin < (AGPIO(GPIO_REL1_INV) + MAX_RELAYS_SET))) { + else if ((mpin >= AGPIO(GPIO_REL1_INV)) && (mpin < (AGPIO(GPIO_REL1_INV) + MAX_RELAYS_SET)) && MCP23xAddItem(Mcp23x.relay_max)) { bitSet(Mcp23x.relay_inverted, mpin - AGPIO(GPIO_REL1_INV)); mpin -= (AGPIO(GPIO_REL1_INV) - AGPIO(GPIO_REL1)); - Mcp23x.relay_max++; MCP23xPinMode(pin, OUTPUT); } else if (mpin == AGPIO(GPIO_OUTPUT_HI)) { @@ -565,14 +580,9 @@ bool MCP23xLoadTemplate(void) { else { mpin = 0; } Mcp23x_gpio_pin[pin] = mpin; } - if ((Mcp23x.switch_max >= MAX_SWITCHES_SET) || - (Mcp23x.button_max >= MAX_KEYS_SET) || - (Mcp23x.relay_max >= MAX_RELAYS_SET)) { - AddLog(LOG_LEVEL_INFO, PSTR("MCP: Max reached (S%d/B%d/R%d)"), Mcp23x.switch_max, Mcp23x.button_max, Mcp23x.relay_max); - break; - } } Mcp23x.max_pins = pin; // Max number of configured pins + AddLog(LOG_LEVEL_INFO, PSTR("MCP: Pins used %d (S%d/B%d/R%d)"), Mcp23x.max_pins, Mcp23x.switch_max, Mcp23x.button_max, Mcp23x.relay_max); } // AddLog(LOG_LEVEL_DEBUG, PSTR("MCP: Pins %d, Mcp23x_gpio_pin %*_V"), Mcp23x.max_pins, Mcp23x.max_pins, (uint8_t*)Mcp23x_gpio_pin); @@ -591,7 +601,7 @@ uint32_t MCP23xTemplateGpio(void) { JsonParserToken val = root[PSTR(D_JSON_IOCON)]; if (val) { - Mcp23x.iocon = val.getUInt() & 0x5E; // Only allow 0 MIRROR 0 DISSLW HAEN ODR INTPOL 0 + Mcp23x.iocon.reg = val.getUInt() & 0x5E; // Only allow 0 MIRROR 0 DISSLW HAEN ODR INTPOL 0 } JsonParserArray arr = root[PSTR(D_JSON_GPIO)]; if (arr.isArray()) { @@ -601,7 +611,7 @@ uint32_t MCP23xTemplateGpio(void) { } void MCP23xModuleInit(void) { - Mcp23x.iocon = 0b01011000; // Default 0x58 = Enable INT mirror, Disable Slew rate, HAEN pins for addressing + Mcp23x.iocon.reg = 0b01011000; // Default 0x58 = Enable INT mirror, Disable Slew rate, HAEN pins for addressing int32_t pins_needed = MCP23xTemplateGpio(); if (!pins_needed) { AddLog(LOG_LEVEL_DEBUG_MORE, PSTR("MCP: Invalid template")); @@ -618,7 +628,8 @@ void MCP23xModuleInit(void) { #endif while ((Mcp23x.max_devices < MCP23XXX_MAX_DEVICES) && PinUsed(GPIO_MCP23SXX_CS, Mcp23x.max_devices)) { Mcp23x.chip = Mcp23x.max_devices; - Mcp23x.device[Mcp23x.chip].pin_int = (PinUsed(GPIO_MCP23XXX_INT, Mcp23x.chip)) ? Pin(GPIO_MCP23XXX_INT, Mcp23x.chip) : -1; + uint32_t pin_int = (Mcp23x.iocon.ODR) ? 0 : Mcp23x.chip; // INT pins are open-drain outputs and supposedly connected together to one GPIO + Mcp23x.device[Mcp23x.chip].pin_int = (PinUsed(GPIO_MCP23XXX_INT, pin_int)) ? Pin(GPIO_MCP23XXX_INT, pin_int) : -1; Mcp23x.device[Mcp23x.chip].pin_cs = Pin(GPIO_MCP23SXX_CS, Mcp23x.max_devices); digitalWrite(Mcp23x.device[Mcp23x.chip].pin_cs, 1); pinMode(Mcp23x.device[Mcp23x.chip].pin_cs, OUTPUT); @@ -632,13 +643,13 @@ void MCP23xModuleInit(void) { AddLog(LOG_LEVEL_INFO, PSTR("SPI: MCP23S08 found at CS%d"), Mcp23x.chip +1); Mcp23x.device[Mcp23x.chip].pins = 8; // MCP23xWrite(MCP23X08_IOCON, 0b00011000); // Slew rate disabled, HAEN pins for addressing - MCP23xWrite(MCP23X08_IOCON, Mcp23x.iocon & 0x3E); + MCP23xWrite(MCP23X08_IOCON, Mcp23x.iocon.reg & 0x3E); Mcp23x.device[Mcp23x.chip].olata = MCP23xRead(MCP23X08_OLAT); } else if (0x80 == buffer) { // MCP23S17 AddLog(LOG_LEVEL_INFO, PSTR("SPI: MCP23S17 found at CS%d"), Mcp23x.chip +1); Mcp23x.device[Mcp23x.chip].pins = 16; // MCP23xWrite(MCP23X17_IOCONA, 0b01011000); // Enable INT mirror, Slew rate disabled, HAEN pins for addressing - MCP23xWrite(MCP23X17_IOCONA, Mcp23x.iocon); + MCP23xWrite(MCP23X17_IOCONA, Mcp23x.iocon.reg); Mcp23x.device[Mcp23x.chip].olata = MCP23xRead(MCP23X17_OLATA); Mcp23x.device[Mcp23x.chip].olatb = MCP23xRead(MCP23X17_OLATB); } @@ -655,8 +666,9 @@ void MCP23xModuleInit(void) { uint8_t mcp23xxx_address = MCP23XXX_ADDR_START; while ((Mcp23x.max_devices < MCP23XXX_MAX_DEVICES) && (mcp23xxx_address < MCP23XXX_ADDR_END)) { Mcp23x.chip = Mcp23x.max_devices; + uint32_t pin_int = (Mcp23x.iocon.ODR) ? 0 : Mcp23x.chip; // INT pins are open-drain outputs and supposedly connected together to one GPIO if (I2cSetDevice(mcp23xxx_address)) { - Mcp23x.device[Mcp23x.chip].pin_int = (PinUsed(GPIO_MCP23XXX_INT, Mcp23x.chip)) ? Pin(GPIO_MCP23XXX_INT, Mcp23x.chip) : -1; + Mcp23x.device[Mcp23x.chip].pin_int = (PinUsed(GPIO_MCP23XXX_INT, pin_int)) ? Pin(GPIO_MCP23XXX_INT, pin_int) : -1; Mcp23x.device[Mcp23x.chip].interface = MCP23X_I2C; Mcp23x.device[Mcp23x.chip].address = mcp23xxx_address; @@ -667,7 +679,7 @@ void MCP23xModuleInit(void) { I2cSetActiveFound(mcp23xxx_address, "MCP23008"); Mcp23x.device[Mcp23x.chip].pins = 8; // MCP23xWrite(MCP23X08_IOCON, 0b00011000); // Slew rate disabled, HAEN pins for addressing - MCP23xWrite(MCP23X08_IOCON, Mcp23x.iocon & 0x3E); + MCP23xWrite(MCP23X08_IOCON, Mcp23x.iocon.reg & 0x3E); Mcp23x.device[Mcp23x.chip].olata = MCP23xRead(MCP23X08_OLAT); Mcp23x.max_devices++; } @@ -676,7 +688,7 @@ void MCP23xModuleInit(void) { Mcp23x.device[Mcp23x.chip].pins = 16; MCP23xWrite(MCP23X08_IOCON, 0x00); // Reset bank mode to 0 (MCP23X17_GPINTENB) // MCP23xWrite(MCP23X17_IOCONA, 0b01011000); // Enable INT mirror, Slew rate disabled, HAEN pins for addressing - MCP23xWrite(MCP23X17_IOCONA, Mcp23x.iocon); + MCP23xWrite(MCP23X17_IOCONA, Mcp23x.iocon.reg); Mcp23x.device[Mcp23x.chip].olata = MCP23xRead(MCP23X17_OLATA); Mcp23x.device[Mcp23x.chip].olatb = MCP23xRead(MCP23X17_OLATB); Mcp23x.max_devices++; @@ -761,6 +773,7 @@ void MCP23xInit(void) { } else { gpio = MCP23xRead16(MCP23X17_GPIOA); // Clear MCP23x17 interrupt } + if (Mcp23x.iocon.ODR && Mcp23x.chip) { continue; } pinMode(Mcp23x.device[Mcp23x.chip].pin_int, INPUT_PULLUP); attachInterrupt(Mcp23x.device[Mcp23x.chip].pin_int, MCP23xInputIsr, CHANGE); }