UARTSerial() and mutexes in interrupts

[RobMeades]

Description

• Type: Bug
• Priority: Blocker

With the introduction of #4389, UARTSerial() hits the Mutex not allowed in ISR context trap.

Expected behavior
Transmit serial bytes.

Actual behavior
The call stack is as below, basically UARTSerial()’s tx_irq() is calling SerialBase:writeable() which has a mutex in it.

Steps to reproduce
Use the new CellularInterface, which uses UARTSerial() to talk to the cellular modem.

[RobMeades]

Note: already raised with @kjbracey-arm and @hasnainvirk.

[0xc0170]

Note: already raised with @kjbracey-arm and @hasnainvirk.

UARTSerial inherits from other classes. They provide locking that derived class should implement. Is this implemented in the UbloxCellularBase or where in the class hierarchy ?
If mutex is used there, it won't be allowed to invoke from ISR.

Thus the code in the IRQ below would not work if locking is implemented via mutexes.

    /* Write to the peripheral if there is something to write
     * and if the peripheral is available to write. */
    while (!_txbuf.empty() && SerialBase::writeable()) {
        char data;
        _txbuf.pop(data);
        SerialBase::_base_putc(data);
    }

What's your stand for this issue? Is this documented anywhere (not usable if locking is done via mutexes) ?

[kjbracey]

It’s a confusion between two locks – we need a private mutex for our FileHandle API, and we also need a lock implementation for the pure virtual in SerialBase (which we inherit from privately).

Proposed fix:

Rename the existing lock() and unlock() methods to user_lock() or api_lock() or something, and rename all users inside UARTSerial to point to those. And make them protected.

Then add empty private lock() and unlock() overrides for SerialBase – we don’t want any synchronisation for the underlying serial we’re accessing from interrupt. Maybe those could be enter/exit critical? But leave empty for now – we shouldn’t expect to cope with other people messing with our serial port.

Does that make sense?

Downside is that our own API locks no longer have standard names, but I don't think anyone should be overriding them anyway. Maybe they should be private not protected.

It's possible we shouldn't be private inheriting from SerialBase, but should be just having it or RawSerial as a member instead.

[SeppoTakalo]

@0xc0170 @sg-
I added this into a known issue list of mbed OS 5.5.0