drivers: add PCF857X I2C I/O expander driver

[gschorcht]

Contribution description

This PR adds a driver for the widespread PCF857x serial I/O expander modules which extend the number of GPIOs over I2C. The following expander modules are supported by the driver:

Expander	Type	Pseudomodule
PCF8574	8-bit I2C I/O expander	pcf8574
PCF8574A	8-bit I2C I/O expander	pcf8574a
PCF8575	16-bit I2C I/O expander	pcf8575

The driver defines the PCF857x expander variants by a pseudomodule for each of supported expander module

The driver interface is kept as compatible as possible with the peripheral GPIO interface. The only difference is that the functions have the prefix pcf857x_ and require an additional parameter, the pointer to the expansion device of type pcf857x_t

Since PCF857x expander pins are only quasi-bidirectional without having a separate direction control signal, they only support active LOW driven outputs and inputs that are pulled up to HIGH. HIGH outputs are nothing else than pulled-up inputs. The driver maps this behavior to the GPIO_IN_PU and GPIO_OD and additionally to the GPIO_IN, GPIO_OUT and GPIO_OD_PU modes.

Optionally, the driver can use the low-active open-drain INT signal of PCF857x expander modules

• to provide external interrupt capabilities for all expander pins which are compatible to module periph_gpio_irq
• to keep internal up-to-state of expander inputs to avoid reading expander input pins via I2C each time a single input value read with pcf857x_gpio_read, see the benchmarks.

The driver supports direct GPIO mapping to SAUL which is compatible to the peripheral GPIO SAUL interface. Thus PCF857X I/O expander pins can be used with SAUL in the same way as peripheral GPIOS.

Update The driver also supports GPIO extension API as proposed in PR #9582 and implemented in PR #9860 and #9958 .

Benchmarks

The performance of the driver strongly depends on using interrupt capabilities or not:

• PCF857x driver run-time performance benchmark without interrupt capabilities @ 400 kHz I2C bus speed:

                 nop loop:       639us  ---   0.063us per call  ---   15649452 calls per sec
                 gpio_set:    842995us  ---  84.299us per call  ---      11862 calls per sec
               gpio_clear:    840009us  ---  84.000us per call  ---      11904 calls per sec
              gpio_toggle:    842000us  ---  84.200us per call  ---      11876 calls per sec
                gpio_read:    877514us  ---  87.751us per call  ---      11395 calls per sec
               gpio_write:    841430us  ---  84.143us per call  ---      11884 calls per sec

• PCF857x driver run-time performance benchmark with interrupt capabilities (module periph_gpio_irq) @ 400 kHz I2C bus speed:

                 nop loop:       626us  ---   0.062us per call  ---   15974440 calls per sec
                 gpio_set:   1751632us  ---  175.163us per call  ---       5708 calls per sec
               gpio_clear:   1755016us  ---  175.501us per call  ---       5697 calls per sec
              gpio_toggle:   1755082us  ---  175.508us per call  ---       5697 calls per sec
                gpio_read:      4751us  ---   0.475us per call  ---    2104820 calls per sec
               gpio_write:   1759999us  ---  175.999us per call  ---       5681 calls per sec

Reference board was an esp8266-esp-12x in I2C fast mode.

Testing procedure

The test application tests/driver_pcf857x can be used to test each PCF857x expander I/O pin with shell commands. Dependent on used PCF857x variant, it has to be compiled with at least one of the pseudomodules pcf8574, pcf8574a or pcf8575, e.g.:

 USEMODULE=pcf8575 make -C tests/driver_pcf857x BOARD=... flash 

Add module periph_gpio_irq and override the MCU interrupt pin by parameter PCF857X_PARAM_INT_PIN` to use the PCF857x interrupt signal, e.g.,

 CFLAGS='-DPCF857X_PARAM_INT_PIN=\(GPIO_PIN\(0,6\)\)' \
 USEMODULE='pcf8575 pcf857x_irq' make -C tests/driver_pcf857x BOARD=... flash

The number of the first PCF857X expander port used by the test application is defined by the macro PCF857X_P0, which is 16 by default.

For testing the driver follow the steps:

1. Compile the test application the for a board of your choice using module pcf857x_irq and suitable interrupt pin as shown above.
2. Connect any MCU GPIO pin, e.g. GPIO_PIN(0,0) with any pin of the PCF857X expander, e.g. pin 6.
3. Use the following commands to test that the driver including interrupts works correctly

   > init_out 0 0
   > clear 0 0
   > init_int 16 6 2
   > read 16 6
   PCF857X pin (dev 16, pin 06) is LOW
   > toggle 0 0
   INT: external interrupt from pin 6
   > read 16 6
   PCF857X pin (dev 16, pin 06) is HIGH
   > toggle 0 0
   INT: external interrupt from pin 6
   

Issues/PRs references

UPDATE The driver is compatible with the GPIO extension API, PR #9582, #9860 and #9958, but it does not depend on it. Compatibility with the GPIO extension API, PR #9582, #9860 and #9958 has been removed for the moment due to the new proposal of an extensible GPIO API in PR #12877 #14602. The driver will be adapted later for this GPIO API.

[kYc0o]

@ZetaR60 has already made some PRs and opened issues to address this feature. It's not straightforward but I think the expanders should use the underlying GPIO API. Could you take a look into #9860 #9690 #9190 and #9958 ?

[gschorcht]

@kYc0o Thanks for the hint, I will take a look.

[ZetaR60]

@gschorcht See also #9054 which needs updating but may have some overlap. I will take a more in-depth look at your PR soon.

Regarding compatibility with the extension interface:
If you use function definitions in your driver that are compatible with the callback typedefs in drivers/include/extend/gpio.h from #9958 then you should be able to populate the gpio_ext_driver struct by casting your driver's functions inside the definition for that struct. That way pcf857x_gpio_extend.c only contains a const gpio_ext_driver_t definition rather than a bunch of function wrappers. It might be okay to leave the type of the device descriptor in your driver as the normal descriptor pointer rather than a void pointer and have the function cast take care of the conversion, but this may cause the compiler to complain.

[gschorcht]

@ZetaR60 Good news, the driver is working with your GPIO extension API without any changes 😄 It is as fast as direct access:

                 nop loop:       625us  ---   0.062us per call  ---   16000000 calls per sec
                 gpio_set:   1701875us  ---  170.187us per call  ---       5875 calls per sec
               gpio_clear:   1699375us  ---  169.937us per call  ---       5884 calls per sec
              gpio_toggle:   1701251us  ---  170.125us per call  ---       5878 calls per sec
                gpio_read:      9250us  ---   0.925us per call  ---    1081081 calls per sec
               gpio_write:   1701001us  ---  170.100us per call  ---       5878 calls per sec

For some suggestions concerning the implementation, I will use your PRs.

I will update my PR. As long as module extend_gpio is not enabled it behaves exactly as befor and is compilable without the API. So I could already provide the driver before your changes are merged without any conflict.

[ZetaR60]

@gschorcht Great! I am glad it is working out well on real hardware.

I have been looking at the documentation for the ICs supported by this PR and those supported by #9054 and it looks like at least some of the chips are pin-compatible and maybe even register compatible (some of the docs were a bit spotty with the register maps). It seems there is a wide variety of ICs from multiple manufacturers that:

• Use I2C with a pull-down interrupt pin
• Have similar use of address input pins
• Are usually 8 or 16 bits/pins wide
• Have identical pinouts for the same width
• Have similar or identical internal registers

I think that, if possible, we should make a single driver for all of them, with just a few parameters that are changeable depending on the device. I tried to do this with #9054 (see the already long list of devices in drivers/include/pca95xx.h), but it looks like you found even more similar devices.

[gschorcht]

@ZetaR60 I have already seen your PR #9054, but I didn't realize that all of these I2C I/O expanders are quite similar because their naming is very different.

I think that, if possible, we should make a single driver for all of them, with just a few parameters that are changeable depending on the device.

Although the differences seem to be small, the handling of the devices may be too different to support them with only one driver. The biggest difference between TCA99xx and PCF857x are

• TCA95xx has port configuration registers while PCF857x dos not
• TCA95xx has bidirectional ports while PCF857x has only quasi-bidirectional ports

Thus, a completely different port handling is required. Since you don't have registers in PCF857x there are no i2c_reg_write and i2c_reg_read operations but simply i2c_write_bytes and i2c_read_bytes operations. Also, ports in PCF857x do not really need to be configured. You just write a 0 to a port to switch it to a active low-driven output and you just write a 1 to the port to switch it into high-impedance input with week pull-ups.

Other differences of the drivers (yours and mine) that I've seen:

• in PCF857x driver, interrupt handling is only included when module periph_gpio_irq is used
• if interrupts handling is included, PCF857x also uses the interrupts to update an internal input port image which avoids to read again all ports over I2C when a single input put is read via gpio_read(pin).

So I guess that using one driver for all these different I/O expanders would either require a lot of conditional instructions which produces larger code size and causes less run-time efficiency or a lot of #if MODULE_XX ... #endif wrappers that make the code unreadable. Even though I'm usually a friend of more general code instead of having special code variants for everything, I would suggest to keep them separate in this case.

I can offer, if I have time, to change your code to work with PCF857x, just to see if it would be a good idea to merge it at all.

[gschorcht]

@ZetaR60 How deep did you test the interrupt handling? I spent a lot of time to test a lot of different cases with single and multiple expanders and had to realize the following problems:

• If an output port is connected to an input port at the same expander, changing the output does not trigger an interrupt at the input port. The workaround for me was to call the ISR when outputs have been written. The ISR reads the inputs and calls users callback function when they have changed. That is, an interrupt is simulated.
• When PCF857x expander ports of different devices are interconnected, an I2C write access to an output port of one device may cause an interrupt by an input port of another device before the I2C write access to the first device has been finished. The handling of the interrupt of the second device requires I2C read access. If the devices are connected to the same I2C bus, the current I2C write access to the first device avoids the required I2C read access from interrupt handler to the second device. The I2C read access fails with an arbitration lost and the interrupt is lost. Because the interrupt handling occurs in the same thread context as the I2C write access, the mutex-based i2c_aquire does not help to prevent this conflict. Therefore, the interrupt of the second device must be delayed until the I2C write access of the first device is completed. The workaround for the PCF857x driver was to implement an ISR event wait list for each device.

I don't know whether these problems also occur for PCA95xx device, but I would guess yes.

[gschorcht]

@ZetaR60 The MCP23x17 expanders PR #7652 are completly different and require their own driver.
@PeterKietzmann BTW, if it is OK for you I would take the driver from #7652 to adopt it to be conformant with the @ZetaR60's GPIO extension API PR #9582 and to support the SPI variant of the driver.

[ZetaR60]

Although the differences seem to be small, the handling of the devices may be too different to support them with only one driver. The biggest difference between TCA99xx and PCF857x are

• TCA95xx has port configuration registers while PCF857x dos not
• TCA95xx has bidirectional ports while PCF857x has only quasi-bidirectional ports

I missed the register differences when skimming the PCF857x, since it looked like the documentation was just missing. It's weird that they ICs are almost exactly the same in every other way. I agree that they should be a different driver.

How deep did you test the interrupt handling?

I did not test for the sort of problems you mentioned. I do not have a proper break-out for the PCA95xx, and it is a bit awkward to wire TSSOP pins together when probing them. I am glad you caught this! I agree with you expectation that the exact same problems probably exist with PCA95xx.

The workaround for the PCF857x driver was to implement an ISR event wait list for each device.

This seems a bit complex for a workaround. The best solution would be to fix i2c_acquire so that it is aware that it is in an interrupt. I will take a look at what might be required.

[ZetaR60]

@gschorcht Looks like your i2c_acquire issue is actually a bug in the behavior of mutex_lock on your system (and probably many others). See #10488

EDIT: This was a misunderstanding on my part. Still a problem, but not a bug exactly.

[stale]

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.

[gschorcht]

Rebased onto current master. Compiled and tested again, still works as expected.

[benpicco]

Looks good! Just some small nits

[benpicco]

Why special-case CC2538 here?

[gschorcht]

To fix compilation errors due to this enumeration type 😟

RIOT/cpu/cc2538/include/periph_cpu.h

Lines 150 to 158 in fea525f

	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;

where GPIO_OD and GPIO_OD_PU are declared as undefined. I didn't take a look at the implementation to see if these values are used directly as register values. But it wouldn't make any difference, because the special case would still have to be considered.

[benpicco]

But is GPIO_OD the same as GPIO_IN? Otherwise you can just let this case be handled by default and not support it.

[gschorcht]

A bit of reordering to handle everything that is not GPIO_OUT as default. The question is whether not supported GPIO_IN_OD should lead to an assert or simply a failure.

[benpicco]

As long as gpio_init returns an error code, you can do so too

[benpicco]

Now this also catches GPIO_IN_PU and GPIO_OD/GPIO_OD_PU, is this correct?

[gschorcht]

Ups, simultaneous communication.

This I/O expander is a very simple device that doesn't have a direction register. In fact it physically supports only the GPIO modes GPIO_IN_PU and GPIO_OD_PU:

• Writing a 1 to an expander pin configures the pin as an input, which is pulled up to HIGH by a weak pull-up. When reading the pin, its value then depends on the actual voltage.
• Writing a 0 to an expander pin configures the pin as an output and actively drives the pin to LOW.

All other modes are only considered logically identical and behave only like GPIO_IN, GPIO_OD and GPIO_OUT.

[benpicco]

It's probably worth adding a comment about that - feel free to squash directly

[gschorcht]

Now this also catches GPIO_IN_PU and GPIO_OD/GPIO_OD_PU, is this correct?

This affects only the initial level of the pin. If GPIO_OUT is used a 0 is written to the pin which configures the pin as physical output an drives it active LOW. In all other cases a 1 is written to the pin which configures it as physical input an the pin is pulled-up to HIGH by the weak pull-up. It would also be possible to use HIGH as initial level for all modes.

[gschorcht]

It's probably worth adding a comment about that - feel free to squash directly

It is documented here

RIOT/drivers/include/pcf857x.h

Line 67 in 0b9b8c2

* - INPUT:

But I can add a comment in source code too.

[gschorcht]

Comment added to source code in commit d02a87f.

[gschorcht]

Some small changes / clarifications / enhancements in documentation 2ed6c9a.

[benpicco]

Please squash!

[gschorcht]

@benpicco Many thanks for reviewing and merging.