Mbed-os: SPI regression on nrf52

[NeilMacMullen]

Note - I've posted this 'incomplete' as a placeholder. More details to follow as I isolate them but may take a few days.

Somewhere between commit 18634009c5f and the current head of master, a regression in SPI behaviour seems to have been introduced.

The symptoms appear to be that reading/writing is unreliable on a SPI bus using pins

#define WISOL_WIFI_MOSI p6
#define WISOL_WIFI_MISO p8
#define WISOL_WIFI_SCLK p7
#define WISOL_WIFI_CS p5

Usually reading fails to return any bytes at all but other times, a read completes but appears to 'wrap' the inbound buffer. I.e. when expection to receive "AT+CWMODE=1\r\n", instead "AT+CWMODE=1AT" is received instead

Unfortunately I'm struggling to reproduce this with a minimal test application; however the same application code that ran on 18634009c5f fails immediately on head of master.

[NeilMacMullen]

cc'ing @marcuschangarm and @MarceloSalazar

[NeilMacMullen]

More detail on this....

There appear to be two separate problems. The first is that calling ClaimStdOut in this code causes spi reads to always return 0....

// simple class to grab stdio
class StreamRedirector : public Stream
{
  private:
    char *ourName;
  public:
    StreamRedirector(char *name) : Stream(name + 1)
    {
        ourName = name;
    }
    void claim(const char *mode, FILE *file)
    {
        freopen(ourName, mode, file);
    } //!< claim a stream
  protected:
    virtual int _getc() { return EOF;} 
    virtual int _putc(int c)
    {
        return SEGGER_RTT_Write(0, &c, 1);
    }
};

void ClaimStdout()
{
    StreamRedirector *redirector = new StreamRedirector("/mux");
    redirector->claim("w", stdout);
    // turn off buffering for stdout
    setbuf(stdout, NULL);
}

This occurs even though printf is never used. It is _not_ a regression and does appear on older versions. I'm really at a loss to see why this could affect SPI operations but it may be relevant that for the target we are using, the SPI bus is on pins p5-p8 which are also defined like this in pinnames.h

RX_PIN_NUMBER = p5,
TX_PIN_NUMBER = p6,
CTS_PIN_NUMBER = p7,

Note that we never create a serial port on these pins but maybe something is being done with them when the stream is redirected? I'll try removing the definitions in pinnames.h later to see if that makes a difference.

The more serious issue (I can workaround the previous one) is still that later builds are still showing corruption on the SPI tx. Here's an example....

0> Sending 'AT'
0> AT
0>
0> OK
0> Sending 'AT+CWMODE=1'
0> AT+CWMODE=1AT
0>
0> ERROR

The line after 'sending' is an echo from the target device. Since the device also emits 'ERROR' it's probably fair to assume that the corruption is occurring on the send. It may be relevant that the when sending 'AT+CWMODE=1' we then following it with a _separate_ transmission of "\r\n" and it looks like this is what is incorrectly being sent as "AT" (possibly "AT\r\n")

(Just for the avoidance of doubt since the traffic above looks like uart traffic, these are AT commands being sent via SPI to an esp8266)

[NeilMacMullen]

I'll try removing the definitions in pinnames.h later to see if that makes a difference.

Setting

RX_PIN_NUMBER = NC,
TX_PIN_NUMBER = NC,
CTS_PIN_NUMBER = NC,
RTS_PIN_NUMBER = NC,

in pinnames.h causes ClaimStdOut to hang. That doesn't seem like desirable behaviour :(

[ciarmcom]

ARM Internal Ref: MBOTRIAGE-1171

[AGlass0fMilk]

It is worth noting that an _implicit_ behavior of mbed is to lazily create a debug UART using those pins whenever a printf-type function is called. I'm not sure what configuration there is to prevent this (perhaps remove DEVICE_HAS_STDIO_MESSAGES?).

You can look in serial_api.c, you will see what I'm talking about by looking for stdio_uart.

If you're developing on a custom target I would create your own PinNames.h file based on one of the existing ones. Are you on a custom target?

What are you trying to accomplish by using this StreamRedirector class? Maybe what you are looking for is here in mbed_retarget.h:

https://github.com/ARMmbed/mbed-os/blob/master/platform/mbed_retarget.h#L95-L133

You can implement that function (FileHandle* mbed::mbed_override_console(int fd);) and whenever the default stdio_uart is lazily created it will call that function to obtain it instead of using the default (using the pins defined in PinNames.h)

EDIT:
If that's not what you're trying to do, it's probably a bad idea to set the default UART pins to NC (this will likely cause hard faults or MBED_ASSERT to fail, possibly what you're seeing). You could redirect the stdio output to a Serial class that just ignores the data. That way mbed will never try to instantiate a stdio UART on pins you're trying to use.

[marcuschangarm]

https://github.com/ARMmbed/mbed-os/commit/18634009c5f841a4df53c56ec053668c344ce24f is quite some time ago.

Can you help us narrow it down a bit please? These are the NRF52 commits since then: https://github.com/ARMmbed/mbed-os/commits/master/targets/TARGET_NORDIC/TARGET_NRF5x/TARGET_NRF52

[NeilMacMullen]

Yes, I'm on custom target that was once based on the pin-out for the UBLOX NINA B1 - hence the rather historical use of that as a target. You're correct that I should probably create a new target but I'd still be trying to set those pins to NC - we do not have a debug uart on this board and the last thing I want is bits of mbed deciding it should hook up pins to try to create one without being asked ! ;-)

So what am I trying to accomplish by doing this? Basically the bog-standard thing of redirecting stdout to an interface of my choice. I avoid printf in my code but many of the standard mbed examples and library use it and getting out anything out of the trace subsystem seems to require intercepting stdout.

The stream hooking method was borrowed from some mbed library code (C027interface I think) but it looks like the retargeting method is better - thanks for that.

[AGlass0fMilk]

Another tip: If you're not using the SWO pin (p0.18) and want debug output you could redirect the default serial to that:

// Redirect debug output to the SWO port
mbed::FileHandle *mbed::mbed_override_console(int fd) {
    static mbed::SerialWireOutput swo_serial;
    return &swo_serial;
}

[NeilMacMullen]

@AGlass0fMilk - thanks, I'm using RTT but that's a really useful reference implementation!
@marcuschangarm - the problem is difficult to reproduce reliably so this could take some time but if I can narrow it down I will do.

[marcuschangarm]

Thank you! I'll keep an eye out for anything that might be related to what you are seeing.

[NeilMacMullen]

Closing this because I can't reproduce it any more.