EFM32 - Revert IRQ Handling to 5.8.0

[DBS06]

Description

Referring to issue #6783 the pull request/commit #2a3d6d4 must be reverted. This commit causes the problem that the rise callback will never be called.

Pull request type

[x] Fix
[ ] Refactor
[ ] New target
[ ] Feature
[ ] Breaking change

[stevew817]

Reverting is not the solution, because then we'll go back to previous behaviour that was faulty as well.

The issue that was fixed before is checking the pin state after the interrupt has fired. This may cause short pulses to be missed, ergo, the driver needs to start relying on the IRQ flags and just fire those callbacks up the chain.

[DBS06]

According to the EFM32PG12-RM there is no other way to decide between a rise and a fall interrupt, because you have only one IRQ-Flag for rise AND fall. Therefore you need (it is mandatory) to check the pin state.

So yes, reverting is the solution, because the previous behavior was correct. To prevent missing short pulses (whatever "short" means) you need to redesign the µC itself by adding another register, or use a different one.

The registers GPIO->EXTIFALL and GPIO->EXTIRISE just indicates you if an interrupt for rise and/or fall is enabled. Checking only this registers will not work when both (rise and fall) interrupts are enabled.

Perhaps the checking of the pin state could be made faster, but that should be another pull request.

[DBS06]

BTW: What does "make gpio interrupts faster by offloading expected pin state check to user" mean?

Where else should the check happen?
Did anybody thought about it?
Furthermore this breaks with InterruptIn. If the user should/must do it. then InterruptIn should only have an attach() method instead of rise() and fall().
And the most disturbing point for me is, that we are using mbed-OS because of its portability features. I don't want to call vendor specific HAL-functions in the application code and goes, in my meaning, against the concept of mebd-OS (it is for me the most crucial point of the mbed-OS concept).

Just an example to make the code faster:
uint8_t channel_ports[NUM_GPIO_CHANNELS/2] = { 0 }; // Storing 2 ports in each uint8
Ok 8 Bytes are saved but more process time consumed by doing:
(pin & 0x1) ? channel_ports[(pin>>1) & 0x7] >> 4 & 0xF : channel_ports[(pin>>1) & 0x7] & 0xF
and
channel_ports[(obj->pin >> 1) & 0x7] = (obj->pin & 0x1) ? (channel_ports[(obj->pin >> 1) & 0x7] & 0x0F) | (obj->pin & 0xF0) : (channel_ports[(obj->pin >> 1) & 0x7] & 0xF0) | ((obj->pin >> 4) & 0xF);
Does this really legitimate to save 8 Bytes?

[stevew817]

PR #6984 supersedes this. @DBS06 please review my proposed fix.

[stevew817]

And as a closing thought to your remark: yes, saving dynamic memory does matter. This code not only goes into mbed-os, but also into mbed 2, where we have targets with as low as 4kB of RAM. That's where you really need these savings.

[stevew817]

#6984 has been merged for a while now, and fixes this issue. Time to close the ticket?