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.
- 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
- 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
- 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... #endifaround 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:
- 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.
- 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.
- 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.