Optimization breaks SPI code on L562

[Harvey White]

L562 processor.  SPI1.  Blocking mode.  Two sequential writes, second write variably returns error.

code:

// uses CS if USE_CS is set to CS_ACTIVE (default)
// CS is not used if USE_CS is set to CS_INACTIVE

uint32_t HAL_SPI::Send(
						uint8_t* cmdptr,
						uint32_t  cmd_count,
						uint8_t* dataptr,
						uint32_t  data_count,
						bool USE_CS,
						enum SPI_TRANSFER_MODE Override)
{

	enum SPI_TRANSFER_MODE		NEW_mode = OUT_mode;
	int							error_code;

//	int							old_divisor;

	// ********************************** check input parameters***********************************
	// need to check overrides
	switch (Override)
	{
		case SPI_DMA:			// TRANSMIT IS DMA FROM BUFFER, RECEIVE IS IRQ QUEUE (MAY CHANGE)
		case SPI_IRQ:			// TRANSMIT IS IRQ DRIVEN
		case SPI_BLOCKING:		// TRANSMIT AND RECEIVE ARE BYTE BLOCKING
		{
			NEW_mode = Override;
			break;
		}
		default:
		{
			NEW_mode = OUT_mode;
			// leave mode as is, cannot override
		}
	}

	switch (NEW_mode)
	{
		case SPI_BLOCKING:		// TRANSMIT AND RECEIVE ARE BYTE BLOCKING
		{
			time_start = _MICROSECOND_TIMER.Instance->CNT;
			// always semaphore protected
			if (!dedicated) xSemaphoreTake(busy, portMAX_DELAY);
			// direct call to STMICRO driver, blocking mode
			hspi->Instance->CR1 &= ~SPI_BAUDRATEPRESCALER_256;// reset to highest baudrate
			hspi->Instance->CR1 |= transmit_divisor;

			if (RESET.port != nullptr) RESET.set();				// make sure reset if high
			if (USE_CS) RESET_CS();								// CS low if needed
			/*
				HAL_OK       = 0x00,
				HAL_ERROR    = 0x01,
				HAL_BUSY     = 0x02,
				HAL_TIMEOUT  = 0x03
 	 	 	 */


			if ((cmdptr != nullptr) && (cmd_count != 0))
			{
				// command pointer says to send data with A0 down
				if (A0.port != nullptr) A0.reset();
				result = HAL_SPI_Transmit(hspi, cmdptr, cmd_count, timeout);
				// place to add delay if available
			}


			if ((dataptr != nullptr) && (data_count != 0))
			{
				// command pointer says to send data with A0 down
				if (A0.port != nullptr) A0.set();
				result = HAL_SPI_Transmit(hspi, dataptr, data_count, timeout);
				// place to add delay if available
//				while (hspi->State == HAL_SPI_STATE_BUSY_TX)
//				{
//					HAL_Delay(1);
//				}
			}
			if (USE_CS) SET_CS();								// CS high if needed
			// error reporting
			ERROR_CODE = hspi->ErrorCode;
			ERROR_REASON = (enum COMM_ERROR_TYPE) ERROR_CODE;
			operations++;
			// record errors if needed
			switch (result)
			{
				case HAL_ERROR:    					//= 0x01U,
				{
					fail++;
					break;
				}
				case HAL_BUSY:     					//= 0x02U,
				{
					fail++;
					break;
				}
				case HAL_TIMEOUT:  					//= 0x03U
				{
					fail++;
					break;
				}
				case HAL_OK:
				{
					// was OK
					succeed++;
					break;
				}
			}

			// return semaphore for next operation
			if (!dedicated) xSemaphoreGive(busy);
			time_stop = _MICROSECOND_TIMER.Instance->CNT;
			delta_time = time_stop - time_start;
			break;
		}
		

Using optimization for debug, this code works perfectly. 

Using G0 (no optimization), the code fails either at line #70 after executing line #61 (write two bytes), or the second call to the routine at line #61 (writing one byte per call).

This is a display driver routine.  Using version 1.19.0

The rest of the code in this routine simply returns HAL_OK.

[MM..1]

---

@Harvey White wrote:

Using G0 (no optimization), the code fails either at line #70

---

i repeat G0 isnt optimization ! But your issue isnt optimization, but code style. C++ and HAL and RTOS etc.

Are you sure you change optimization in C++ section or in C or bpth etc.

[Harvey White]

No optimization (G0) is a level of optimization, just none.  Regardless, it did not work.

I generally change the level of optimization in both C and C++ sections, and have changed one if not the other to try to see what the problem is.  

If you have an idea where code style is the problem, I'd love to hear where and why you think so.  

The simplified driver structure (display driver, i.e. ST7735R with direct interfaces to the low level hal drivers (ST drivers) does work, using -Og optimization.  I'm going to test the concept a bit more, its not complete, needs semaphores, for one, to protect the driver (interface is dedicated, hence the driver semaphore protects it as well).

[Harvey White]

having rewritten the display driver, and only working on the blocking part of the driver, the following happens, depending on optimization:

/*
C optimization is debug
C++ optimization working none, debug, 1
C++ optimization not working 2, 3, size, speed, more for speed

C optimization with C++ = none
C optimization working with none, debug, 1, 2, 3, size, speed



*/

as you see the problem seems not to be in the C code, but in the C++ code.  Where it breaks seems to be a hard fault related to memory allocation.  Right at this point, I'm puzzled.

I've included (and please concern yourself with the code in #ifdef _SIMPLE, I wanted to make the changes reversible).  This is only working in the blocking mode because the _SIMPLE mode code is only blocking mode.

Code attached.

please note that this is a chip level graphics driver with the main focus on the chip initialization.  

The place where things break seems now to be the C++ code, and not the C code in the drivers.  FreeRTOS is written in C, not C++

comments?

PS:  apparently the formatter for this forum doesn't believe in .hpp file extensions and thinks that whatever they are, the content doesn't match some rule.

Here's the hpp file anyway

Now the message can't exceed 30000 characters (who knew?)

ok, renamed as ;h

<sigh>

[Harvey White]

forgot a pices about which optimization did what.  Editing is apparently unimportant.

*
C optimization is debug
C++ optimization working none, debug, 1
C++ optimization not working 2, 3, size, speed, more for speed

C optimization with C++ = none
C optimization working with none, debug, 1, 2, 3, size, speed



*/

On the surface it seems to be the C++ code, but then I'd ask why the C code doesn't work, aren't the C and C++ optimizations more or less independent?

Sill puzzled.........