Best way to enable SPIM. Mbed experts needed

Hi,
Over the last week or so I have been working with the SPIM (SPI Master with EasyDMA) code under the NORDIC TARGET_SDK_15_0. I would like to make a contribution to the Mbed-os platform so someone else can benefit from the work I have done.
I have the SPIM code running but in doing so it seems the Mbed-os was not really setup to allow it to be easily configured/enabled. I made the necessary changes but need advice.
I see a number of issues that would need to be discussed/resolved to allow me to contribute my work to Mbed.

  1. The \targets\TARGET_NORDIC\TARGET_NRF5x\TARGET_NRF52\objects.h includes the nfrx_spi.h, and does not care if the SPIM was enabled needed to include nfrx_spim.h.
    I fixed this by adding
#if NRFX_SPIM_ENABLED
#include "nrfx_spim.h"
#elif NRFX_SPI_ENABLED
#include "nrfx_spi.h"
#endif
  1. The SPI and SPIM structs and methods are named almost the same, but the SPIM structs add ‘m’ to them (spim). See nrfx_spim.h and nrfx_spi.h for slight name changes. This means that I have to do things such as:
struct spi_s {
    int instance;
    PinName cs;
#if NRFX_SPIM_ENABLED
    nrfx_spim_config_t config;
#elif NRFX_SPI_ENABLED
    nrfx_spi_config_t config;
#endif
  1. Next is probably the most awkward file to modify, spi_api.c. Again because of all the struct and function name changes between SPI and SPIM, I have done the quickest , but maybe not cleanest way. I duplicated all spi the code in this file and then added
#if NRFX_SPIM_ENABLED
  spim code...
#elif NRFX_SPI_ENABLED
  spi code...
#endif
around the entire blocks. Then I changed all the method and struct definitions under NFRX_SPIM_ENABLED to reference spim.

If you want to see my changes it is out on github here: https://github.com/winneymj/mbed-os/tree/attempt_enable_SPIM3_highspeed_SPI

Anyhow what I really want to do is talk about the best way to implement the changes to bring in the SPIM. My main worry is the spi_api.c and where I duplicated the spi code and modified it to call spim. There seems to be a number of solutions:

  1. Instead of duplicating the code in spi_api.c and renaming structs and method calls to point to spim I could try to add more #if NRFX_SPIM_ENABLED around pieces of code that are effected. Downside to this seems to be cluttered code, readability and trying to figure out the flow of the code with so many #defines already present, and if I need to make a change specific to SPIM even more defines needed.
  2. Create a copy of the spi_api.c and save it as spim_api.c and link this in conditionally. Not ideal as we now have 2 similar pieces of code with minimal differences…bad smell.
  3. Any other solutions?

I am guessing we are going to have to live with some bad smelling code with it being mostly C and not C++, so no easy way to do inheritance etc.

I really would like to contribute to Mbed as I am enjoying it very much but am a little apprehensive about how to go about making the changes needed.
Should I create this post as an issue so that more mbed experts can contribute?
Thanks for any help
Mark.

Hi Mark,

thanks for reaching out in the forum. I would suggest raising a PR against Mbed OS with your changes and add a caveat to the description that this PR contains a messy solution but can form the basis of further discussions on a possible way forward ?
That way you may get more interaction with the core Mbed OS team.

Regards
Anna

That is exactly what I did. I have a PR: https://github.com/ARMmbed/mbed-os/pull/13397