drivers: add MCP23x17 I/O expander support #10688

gschorcht · 2019-01-02T02:22:04Z

Contribution description

This PR adds a driver for Microchip MCP23x17 I/O expanders which can be used to extend the number of GPIOs over I2C or SPI. It is a complete rework of the driver that was provided in PR #7652 and rewritten from scratch.

The following expander devices are supported:

Expander	Type	Interface	Pseudomodule
MCP23017	16-bit I/O expander	I2C	`mcp23017` or `mcp23x17_i2c`
MCP23S17	16-bit I/O expander	SPI	`mcp23s17` or `mcp23x17_spi`

The main features of the driver are:

Its interface is compatible with the peripheral GPIO interface. The only difference is that the functions have the prefix mcp23x17_ and require an additional parameter, the pointer to the expander device of type mcp23x17_
It supports the GPIO extension API as defined in PR periph_*: Allow extensions for external periph adapters #9582 and provided in PR periph/gpio: support for extension API #9860 and periph/gpio: support for extension API (part 2) #9958.
It supports the following GPIO modes
- GPIO_IN, GPIO_IN_PU and GPIO_OUT in hardware
- GPIO_OD and GPIO_OD_PU are emulated.
Optionally, the a low-active push-pull INT signal of MCP23x17 expanders can be used to provide external interrupt capabilities for all expander pins. The driver interface is compatible to module periph_gpio_irq
It supports direct GPIO mapping to SAUL which is compatible to the peripheral GPIO SAUL interface. Thus MCP23x17 I/O expander pins can be used with SAUL in the same way as peripheral GPIOS.
Multiple MCP23x17 expander modules and different variants can be used simultaneously.

Benchmarks

MCP23x17 driver run-time performance benchmark over SPI using 10 MHz clock speed:

                 nop loop:       634us  ---   0.063us per call  ---   15772870 calls per sec
                 gpio_set:    540009us  ---  54.000us per call  ---      18518 calls per sec
               gpio_clear:    540000us  ---  54.000us per call  ---      18518 calls per sec
              gpio_toggle:    811005us  ---  81.100us per call  ---      12330 calls per sec
                gpio_read:    271001us  ---  27.100us per call  ---      36900 calls per sec
               gpio_write:    539005us  ---  53.900us per call  ---      18552 calls per sec

MCP23x17 driver run-time performance benchmark over SPI using 1 MHz clock speed:

                 nop loop:       634us  ---   0.063us per call  ---   15772870 calls per sec
                 gpio_set:   1429044us  ---  142.904us per call  ---       6997 calls per sec
               gpio_clear:   1429005us  ---  142.900us per call  ---       6997 calls per sec
              gpio_toggle:   2144009us  ---  214.400us per call  ---       4664 calls per sec
                gpio_read:    715000us  ---  71.500us per call  ---      13986 calls per sec
               gpio_write:   1428004us  ---  142.800us per call  ---       7002 calls per sec

MCP23x17 driver run-time performance benchmark over I2C using 400 kHz clock speed:

                 nop loop:       639us  ---   0.063us per call  ---   15649452 calls per sec
                 gpio_set:   1965040us  ---  196.504us per call  ---       5088 calls per sec
               gpio_clear:   1960008us  ---  196.000us per call  ---       5102 calls per sec
              gpio_toggle:   3088493us  ---  308.849us per call  ---       3237 calls per sec
                gpio_read:   1127503us  ---  112.750us per call  ---       8869 calls per sec
               gpio_write:   1960004us  ---  196.000us per call  ---       5102 calls per sec

Reference board was an esp8266-esp-12x.

Testing procedure

The test application tests/driver_mcp23x17 can be used to test each MCP23x17 I/O expander pin with shell commands.

Compile the application. Dependent on the MCP23x17 expander module, compile it with at least one of the pseudomodules mcp23x17_i2c or mcp23x17_spi, e.g., for the I2C version MCP23017:
```
USEMODULE=mcp23x17_i2c make flash -C tests/driver_mcp23x17 BOARD=...
```
Add the interrupt functionality by enabling module mcp23x17_irq and override the default GPIO pin for the interrupt signal, e.g. for the I2C version MCP23017:
```
CFLAGS="-DMCP23X17_PARAM_I2C_INT=\(GPIO\(0,6\)\)" \
USEMODULE="mcp23x17_i2c mcp23x17_irq"  flash -C tests/driver_mcp23x17 BOARD=...
```
Connect the MCP23x17 I/O expander module using the interface and the GPIO for the interrupt signal as defined during compilation.
Connect a GPIO pin of the MCU, e.g. GPIO_PIN(0,4), and a MCP23x17 I/O expander pin, e.g. Port A (port 16 in test application) and Pin 6.

Execute some or all of the following commands (results in comment):

 > init_in 0 4
 > init_od_pu 16 6

 > set 0 4
 > read 0 4          # GPIO pin (port 0, pin 4)) is HIGH

 > clear 16 6
 > read 0 4          # GPIO pin (port 0, pin 4)) is LOW

 > init_od 16 6
 > init_in_pu 0 4

 > set 16 6
 > read 0 4          # GPIO pin (port 0, pin 4)) is HIGH

 > clear 16 6
 > read 0 4          # GPIO pin (port 0, pin 4)) is LOW

 > init_in 16 6
 > read 16 6         # MCP23x17 pin (dev 16, pin 6)) is HIGH
 > init_out 0 4 
 > read 16 6         # MCP23x17 pin (dev 16, pin 6)) is LOW

 > set 0 4 
 > read 16 6         # MCP23x17 pin (dev 16, pin 6)) is HIGH

 > init_int 16 6 0
 > clear 0 4         # INT: external interrupt from MCP23x17 pin 6
 > set 0 4

 > init_int 16 6 1
 > clear 0 4
 > set 0 4           # INT: external interrupt from MCP23x17 pin 6

 > init_int 16 6 2
 > clear 0 4         # INT: external interrupt from MCP23x17 pin 6
 > set 0 4           # INT: external interrupt from MCP23x17 pin 6 

 > disable_int 16 6
 > clear 0 4
 > enable_int 16 6
 > set 0 4           INT: external interrupt from MCP23x17 pin 6
 > clear 0 4         INT: external interrupt from MCP23x17 pin 6

 > init_int 16 5 2
 > disable_int 16 5
 > set 0 4           INT: external interrupt from MCP23x17 pin 6

Issues/PRs references

~~The driver is compatible with the GPIO extension API, PRs #9582, #9860 and #9958, but it does not depend on it.~~

gschorcht · 2019-01-02T02:23:11Z

@ZetaR60 Again a GPIO expander driver that is working perfect with your last version of the extension API.

stale · 2019-08-10T03:08:02Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

stale · 2020-03-10T12:38:11Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

jeandudey

This might be a little bit old but left some comments for a first pass, I hope it gets a rebase in the near future as the driver seems very complete.

jeandudey · 2021-02-27T12:57:17Z

drivers/mcp23x17/mcp23x17.c

+        return -MCP23X17_ERROR_NO_DEV;
+    }
+#endif
+    if (params->reset_pin != GPIO_UNDEF) {


Suggested change

if (params->reset_pin != GPIO_UNDEF) {

if (gpio_is_valid(params->reset_pin)) {

jeandudey · 2021-02-27T12:58:59Z

drivers/mcp23x17/mcp23x17.c

+     * If GPIO pin for interrupt signals INTA and INTB is defined in parameters,
+     * we use interrupts.
+     */
+    if (params->int_pin != GPIO_UNDEF) {


jeandudey · 2021-02-27T13:02:30Z

drivers/mcp23x17/mcp23x17.c

+    return MCP23X17_OK;
+}
+
+int mcp23x17_gpio_init(mcp23x17_t *dev, gpio_t pin, gpio_mode_t mode)


I'm bit unsure about using gpio_t in here as some CPUs could define it as an structure, I think it may be more suitable to add a proper mc23x17_gpio_t type, same could apply for mode as CPUs could define their own and probably the IO expander has it's modes defined independently of the host.

gpio_t and gpio_mode_t are used here intentionally. As a result of the review of the first driver provided in PR #7652, the goal of this PR was to implement an API that is compatible with the GPIO API in order to integrate the GPIO expanders transparently.

At the moment, gpio_t must be a scalar for all GPIO implementations because there are hundreds of comparisons in RIOT that use the == operator.

In PR #12877 and later in PR #14602 I tried to introduce a GPIO API using a structured gpio_t type more than a year ago, but I didn't succeed until now. So there is no reason not to use gpio_t at the moment. Once, the structured GPIO is introduced, the driver could easily be changed, as the example of the low-level GPIO API implementation of an expander driver in PR #14602 shows.

crasbe · 2025-03-11T15:54:25Z

But the Kconfig stuff is obsolete now anyways, so it could be deleted entirely.

I think my assessment back then was incorrect, but I can create a separate PR to add the support back in and debug the issues that were present at the time.

IMO this is good for merge. I went through it very thoroughly a couple of months ago and the test worked out too.
Since this is a driver, the impact on anything else would be minimal if we overlooked anything.

crasbe · 2025-03-11T16:39:58Z

drivers/mcp23x17/mcp23x17.c

+#define  MCP23X17_EVENT_PRIO    EVENT_PRIO_HIGHEST
+#endif
+
+/* interrutp service routine for IRQs */


Suggested change

/* interrutp service routine for IRQs */

/* interrupt service routine for IRQs */

This is the typo the static tests were complaining about. Odd that the workflow didn't catch it before?

Edit: a local run of make static-test doesn't complain about it either.

This typo is still here 👀

crasbe · 2025-03-26T18:00:17Z

So basically there are three issues that let the CI fail:

The cc2538 CPU not supporting GPIO_OD and GPIO_OD_PU:

RIOT/cpu/cc2538/include/periph_cpu.h

Lines 149 to 160 in 04a1698

    
           #define HAVE_GPIO_MODE_T 
        
           typedef enum { 
        
               GPIO_IN         = ((uint8_t)OVERRIDE_DISABLE),      /**< input, no pull */ 
        
               GPIO_IN_ANALOG  = ((uint8_t)OVERRIDE_ANALOG),       /**< input, analog */ 
        
               GPIO_IN_PD      = ((uint8_t)OVERRIDE_PULLDOWN),     /**< input, pull-down */ 
        
               GPIO_IN_PU      = ((uint8_t)OVERRIDE_PULLUP),       /**< input, pull-up */ 
        
               GPIO_OUT        = ((uint8_t)OVERRIDE_ENABLE),       /**< output */ 
        
               GPIO_OD         = (0xff),                           /**< not supported */ 
        
               GPIO_OD_PU      = (0xff)                            /**< not supported */ 
        
           } gpio_mode_t; 
        
           /** @} */ 
        
           #endif /* ndef DOXYGEN */

In this case, GPIO_OD and GPIO_OD_PU are both 0xff, which leads to the issue in the switch case.
Some other CPUs don't support GPIO_OD_PU, but that is not an issue because the mode is never 0xff.

I'm not sure yet what's the best way to solve this. The gpio_init function checks for 0xff. It would probably be possible to set GPIO_OD to a second value like 0xfe and change the condition to if (mode >= MODE_NOTSUP) with #define MODE_NOTSUP (0xf0)

RIOT/cpu/cc2538/periph/gpio.c

Lines 100 to 105 in 04a1698

    
           int gpio_init(gpio_t pin, gpio_mode_t mode) 
        
           { 
        
               /* check if mode is valid */ 
        
               if (mode == MODE_NOTSUP) { 
        
                   return -1; 
        
               }

0xf0 is still very far away from the valid configuration values:

RIOT/cpu/cc2538/include/cc2538_gpio.h

Lines 304 to 313 in 04a1698

    
            /** 
        
             * @brief Values to override pin configuration 
        
             */ 
        
            typedef enum { 
        
                OVERRIDE_DISABLE    = 0x0, 
        
                OVERRIDE_ANALOG     = 0x1, 
        
                OVERRIDE_PULLDOWN   = 0x2, 
        
                OVERRIDE_PULLUP     = 0x4, 
        
                OVERRIDE_ENABLE     = 0x8, 
        
            } cc2538_ioc_over_t;

The atmega8 doesn't have enough memory. This is easily solved by adding a tests/drivers/mcp23x17/Makefile.ci with the following content:

BOARD_INSUFFICIENT_MEMORY := \
    atmega8 \
    #

The kintis CPU used by the mulle board and others include the unistd.h file, which is taken from newlib and implements the function read. This gives a conflict with the built-in read function.

The easiest solution would probably be to rename the internal read function to _read in line 340 and 485.

I tested the changes locally and they work.
@gschorcht if you want to, I can create a PR for the first point.

Enoch247 · 2025-03-27T01:19:15Z

FYI, you can use make -C tests/drivers/mcp23x17 generate-Makefile.ci to auto generate the Makefile.ci.

gschorcht · 2025-05-13T15:45:12Z

@crasbe Let's come back to this 6 years old PR which I had to revise completly a couple of times due to bigger changes in the master.

First of all, many thanks for testing the PR so comprehensive 👍
If it would be easy for you, I would prefer when you create a PR with your changes.

tests/drivers/mcp23x17/main.c

drivers/mcp23x17/mcp23x17_saul.c

tests/drivers/mcp23x17/main.c

drivers/include/mcp23x17.h

drivers/mcp23x17/mcp23x17.c

tests/drivers/mcp23x17/Makefile.ci

crasbe

Please address the one "interrutp" typo. You can squash the changes directly.

If CI is happy, this can be merged :)

gschorcht · 2025-05-20T05:34:20Z

@crasbe Thanks for reviewing and merging.

gschorcht added Type: new feature The issue requests / The PR implemements a new feature for RIOT Area: drivers Area: Device drivers Area: SAUL Area: Sensor/Actuator Uber Layer labels Jan 2, 2019

gschorcht mentioned this pull request Jan 10, 2019

sys: single interrupt handler thread for interrupts in modules with blocking functions #10555

Merged

stale bot added the State: stale State: The issue / PR has no activity for >185 days label Aug 10, 2019

gschorcht mentioned this pull request Aug 13, 2019

periph/gpio: enable API usage for expanders #9690

Open

5 tasks

benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Sep 3, 2019

stale bot removed the State: stale State: The issue / PR has no activity for >185 days label Sep 3, 2019

This was referenced Nov 7, 2019

periph/gpio: support for extension API #12333

Closed

[WIP, RFC] drivers/eeprom: Common API to access EEPROM memory #12091

Closed

gschorcht mentioned this pull request Dec 5, 2019

[POC/RFC/WIP] periph/gpio: Proposal for the change of the GPIO API #12877

Closed

stale bot added the State: stale State: The issue / PR has no activity for >185 days label Mar 10, 2020

gschorcht added State: don't stale State: Tell state-bot to ignore this issue State: waiting for other PR State: The PR requires another PR to be merged first labels Mar 10, 2020

stale bot removed State: stale State: The issue / PR has no activity for >185 days labels Mar 10, 2020

jeandudey reviewed Feb 27, 2021

View reviewed changes

MrKevinWeiss added this to the Release 2021.07 milestone Jun 22, 2021

MrKevinWeiss removed this from the Release 2021.07 milestone Jul 15, 2021

gschorcht force-pushed the drivers_mcp23x17 branch from b7f0347 to ddaa3bf Compare October 27, 2021 10:52

gschorcht requested review from aabadie, fjmolinas, leandrolanzieri, MichelRottleuthner and smlng as code owners October 27, 2021 10:52

github-actions bot added Area: doc Area: Documentation Area: tests Area: tests and testing framework labels Oct 27, 2021

gschorcht force-pushed the drivers_mcp23x17 branch from 2afd59e to f9a040e Compare November 22, 2024 13:12

crasbe approved these changes Mar 11, 2025

View reviewed changes

benpicco added this pull request to the merge queue Mar 11, 2025

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 11, 2025

crasbe reviewed Mar 11, 2025

View reviewed changes

crasbe added State: waiting for other PR State: The PR requires another PR to be merged first and removed State: don't stale State: Tell state-bot to ignore this issue State: waiting for maintainer State: Action by a maintainer is required labels May 13, 2025

crasbe mentioned this pull request May 13, 2025

cpu/cc2538: Distinguish between GPIO_OD and GPIO_OD_PU #21488

Merged

crasbe reviewed May 14, 2025

View reviewed changes

crasbe removed the State: waiting for other PR State: The PR requires another PR to be merged first label May 14, 2025

crasbe approved these changes May 19, 2025

View reviewed changes

gschorcht added 3 commits May 19, 2025 16:42

drivers/mcp23x17: add support for MCP23x17 I/O expanders

f8585d8

tests/drivers/mcp23x17: test app for MCP23x17 driver

a5da148

drivers/mcp23x17: SAUL integration

a23a849

gschorcht force-pushed the drivers_mcp23x17 branch from e838c06 to a23a849 Compare May 19, 2025 14:42

crasbe added this pull request to the merge queue May 19, 2025

Merged via the queue into RIOT-OS:master with commit 876792e May 20, 2025
25 checks passed

gschorcht deleted the drivers_mcp23x17 branch May 20, 2025 05:34

Teufelchen1 added this to the Release 2025.07 milestone Jul 14, 2025

	if (params->reset_pin != GPIO_UNDEF) {
	if (gpio_is_valid(params->reset_pin)) {

	/* interrutp service routine for IRQs */
	/* interrupt service routine for IRQs */

drivers: add MCP23x17 I/O expander support #10688

drivers: add MCP23x17 I/O expander support #10688

Uh oh!

Conversation

gschorcht commented Jan 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution description

Benchmarks

Testing procedure

Issues/PRs references

Uh oh!

gschorcht commented Jan 2, 2019

Uh oh!

stale bot commented Aug 10, 2019

Uh oh!

stale bot commented Mar 10, 2020

Uh oh!

jeandudey left a comment

Choose a reason for hiding this comment

Uh oh!

jeandudey Feb 27, 2021

Choose a reason for hiding this comment

Uh oh!

jeandudey Feb 27, 2021

Choose a reason for hiding this comment

Uh oh!

jeandudey Feb 27, 2021

Choose a reason for hiding this comment

Uh oh!

gschorcht Feb 27, 2021

Choose a reason for hiding this comment

Uh oh!

crasbe commented Mar 11, 2025

Uh oh!

Uh oh!

crasbe Mar 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

crasbe May 19, 2025

Choose a reason for hiding this comment

Uh oh!

gschorcht May 19, 2025

Choose a reason for hiding this comment

Uh oh!

crasbe commented Mar 26, 2025

Uh oh!

Enoch247 commented Mar 27, 2025

Uh oh!

gschorcht commented May 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

crasbe left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gschorcht commented May 20, 2025

Uh oh!

Uh oh!

gschorcht commented Jan 2, 2019 •

edited

Loading

crasbe Mar 11, 2025 •

edited

Loading

gschorcht commented May 13, 2025 •

edited

Loading