Non PPS PIC32s: compile error with any sketch that uses DSPI

[EmbeddedMan]

Reported by Digilent while testing on the chipKIT Pro MX4.

When

#if defined(PIC32MX1XX) || defined(PIC32MX2XX) || defined(PIC32MX3XX) || defined(PIC32MX4XX) || defined(PIC32MZXX) || defined(PIC32MX47X)
#define _SPI2_IPL_ISR IPL3SOFT //interrupt priority for the ISR
#define _SPI2_IPL_IPC 3 //interrupt priority for the IPC register
#define _SPI2_SPL_IPC 0 //interrupt subpriority for the IPC register
#endif

got changed to

#if defined (PIC32_PPS)

in System_Defs.h (line 375) it broke DSPI for non-PPS PIC32s.

The compile error for the pModJstkDspiInt sketch on the chipKIT Pro MX4 board with chipKIT-core v1.4.0 is as follows:

Compiling library "DSPI"
"C:\Users\bschmalz\AppData\Local\Arduino15\packages\chipKIT\tools\pic32-tools\1.42-pic32gcc/bin/pic32-g++"  -c -g -O2 -Wall -Wextra  -DARDUINO_ARCH_PIC32 -mno-smart-io -ffunction-sections -fdata-sections  -mdebugger -Wcast-align -fno-short-double -ftoplevel-reorder -MMD -fno-exceptions -mprocessor=32MX460F512L -DF_CPU=80000000L  -DARDUINO=10801 -D_BOARD_CEREBOT_MX4CK_ -DMPIDEVER=16777998 -DMPIDE=150 -DIDE=Arduino "-G1024"  -IC:\Users\bschmalz\AppData\Local\Temp\arduino_build_82358/sketch "-IC:\Users\bschmalz\AppData\Local\Arduino15\packages\chipKIT\hardware\pic32\1.4.0\cores\pic32" "-IC:\Users\bschmalz\AppData\Local\Arduino15\packages\chipKIT\hardware\pic32\1.4.0\variants\Cerebot_MX4cK" "-IC:\Users\bschmalz\AppData\Local\Arduino15\packages\chipKIT\hardware\pic32\1.4.0\libraries\DSPI" "C:\Users\bschmalz\AppData\Local\Arduino15\packages\chipKIT\hardware\pic32\1.4.0\libraries\DSPI\DSPI.cpp" -o "C:\Users\bschmalz\AppData\Local\Temp\arduino_build_82358\libraries\DSPI\DSPI.cpp.o"
In file included from C:\Users\bschmalz\AppData\Local\Arduino15\packages\chipKIT\hardware\pic32\1.4.0\cores\pic32/pins_arduino.h:263:0,

                 from C:\Users\bschmalz\AppData\Local\Arduino15\packages\chipKIT\hardware\pic32\1.4.0\cores\pic32/WProgram.h:9,

                 from C:\Users\bschmalz\AppData\Local\Arduino15\packages\chipKIT\hardware\pic32\1.4.0\libraries\DSPI/DSPI.h:41,

                 from C:\Users\bschmalz\AppData\Local\Arduino15\packages\chipKIT\hardware\pic32\1.4.0\libraries\DSPI\DSPI.cpp:46:

C:\Users\bschmalz\AppData\Local\Arduino15\packages\chipKIT\hardware\pic32\1.4.0\libraries\DSPI\DSPI.cpp: In constructor 'DSPI0::DSPI0()':

C:\Users\bschmalz\AppData\Local\Arduino15\packages\chipKIT\hardware\pic32\1.4.0\variants\Cerebot_MX4cK/Board_Defs.h:334:22: error: '_SPI2_IPL_IPC' was not declared in this scope

 #define _DSPI0_IPL   _SPI2_IPL_IPC

                      ^

C:\Users\bschmalz\AppData\Local\Arduino15\packages\chipKIT\hardware\pic32\1.4.0\libraries\DSPI\DSPI.cpp:1072:10: note: in expansion of macro '_DSPI0_IPL'

  ipl = ((_DSPI0_IPL & 0x07) << 2) + (_DSPI0_SPL & 0x03);

          ^

C:\Users\bschmalz\AppData\Local\Arduino15\packages\chipKIT\hardware\pic32\1.4.0\variants\Cerebot_MX4cK/Board_Defs.h:335:22: error: '_SPI2_SPL_IPC' was not declared in this scope

 #define _DSPI0_SPL   _SPI2_SPL_IPC

                      ^

C:\Users\bschmalz\AppData\Local\Arduino15\packages\chipKIT\hardware\pic32\1.4.0\libraries\DSPI\DSPI.cpp:1072:38: note: in expansion of macro '_DSPI0_SPL'

  ipl = ((_DSPI0_IPL & 0x07) << 2) + (_DSPI0_SPL & 0x03);

                                      ^

This seems like a great thing to get fixed for v1.4.1. It the right thing to do simply to revert that #ifdef line?

@majenkotech

[lstandage]

Look at #336. This is kind of a duplicate issue.

I think the root cause is actually because on 5xx/6xx/7xx parts, the SPI interrupt is shared with the UART, but on all other parts, it's normal. I think we change the following:

#if defined(__PIC32_PPS__)
#define	_SPI2_IPL_ISR	IPL3SOFT    //interrupt priority for the ISR
#define	_SPI2_IPL_IPC	3       //interrupt priority for the IPC register
#define	_SPI2_SPL_IPC	0       //interrupt subpriority for the IPC register
#endif

#if defined(__PIC32MX5XX__) || defined(__PIC32MX6XX__) || defined(__PIC32MX7XX__)
#define	_SPI2_IPL_ISR	_UART3_IPL_ISR	//shared with UART3
#define	_SPI2_IPL_IPC	_UART3_IPL_IPC
#define	_SPI2_SPL_IPC	_UART3_SPL_IPC

to something like this

#if defined(__PIC32MX5XX__) || defined(__PIC32MX6XX__) || defined(__PIC32MX7XX__)
#define	_SPI2_IPL_ISR	_UART3_IPL_ISR	//shared with UART3
#define	_SPI2_IPL_IPC	_UART3_IPL_IPC
#define	_SPI2_SPL_IPC	_UART3_SPL_IPC
#else
#define	_SPI2_IPL_ISR	IPL3SOFT    //interrupt priority for the ISR
#define	_SPI2_IPL_IPC	3       //interrupt priority for the IPC register
#define	_SPI2_SPL_IPC	0       //interrupt subpriority for the IPC register
#endif

But we need to handle the MZ case differently, since we can have SRS. Right now, the way this code is defined, I think the SPI interrupt would be a SOFT interrupt on MZ.

[majenkotech]

Surely this is just the fault of the board definition specifying a macro that doesn't exist? Why are we even using these cryptic IPL macros anyway, when they're just a number? Just adjust the board definition to replace _SPI2_IPL etc with just the numbers for the interrupt levels we want on that board.

I have never understood why these macros even exist - what the point of them is. If you want to run something at IPL3 then specify it at IPL3, not some cryptic IPL macro that you have no clue what it equates to.

#define _DSPI0_IPL 3
#define _DSPI0_SPL 0

Now it's obvious that the SPI interrupt runs at priority 3.0 with this board.

Or even better don't have it in the board at all. If DSPI0 wants to be at IPL3 then code it to be at IPL3, not at some unknown IPL that is taken from a macro that points, eventually, to something defined depending on the SPI device it is allocated to. DSPI0's priority should be DSPI0's priority, not SPI1's priority on one board, SPI2's priority on another, etc.

That said, the interrupt system in DSPI has been written wrong (or is outdated) and needs ripping out and re-writing. For a start it has hard coded ISR functions using vectors for MZ parts. Instead it need to use the __USER_ISR macro. The _DSPIx_IPL_ISR macro is then deleted, since it is not needed - in fact it only has any use on the MZ parts, so why it's even being specified for MX parts is a mystery.

[EmbeddedMan]

Pull request #340 resolves this issue and has been merged.