SPI does not work without a call to delay(0)

[ghent360]

I found a very weird issue with the SPI code (may affect others as well).
The code bellow works fine, but produces no SPI signal if you comment out the call to delay(0) in the setup().

I'm testing this on STM32F407VE MCU.

-----Bug repro code start -----

#include <SPI.h>

void setup() {
  delay(0);  // You comment this line the SPI does generate produce any signals
                  // the delay code can be placed anywhere in this file the location does not matter.
  // The pins are specific to my setup, they don't matter for the bug
  SPI.setMOSI(PB5);
  SPI.setMISO(PB4);
  SPI.setSCLK(PB3);
  SPI.begin();
}

void loop() {
  SPI.transfer(0x55);
}

-----Bug repro code end -----

The only significant difference I can spot is when I call delay(0), some extra code is linked to the binary. I suspect the SPI code depends on the SysTick handler and does not quite work without it.
I would guess some week linking is causing this weirdness.

Extra code in the working binary:

<HAL_IncTick>:
	4a03      	ldr	r2, [pc, #12]	; (8000528 <HAL_IncTick+0x10>)
	6811      	ldr	r1, [r2, #0]
	4b03      	ldr	r3, [pc, #12]	; (800052c <HAL_IncTick+0x14>)
	781b      	ldrb	r3, [r3, #0]
	440b      	add	r3, r1
	6013      	str	r3, [r2, #0]
	4770      	bx	lr
	bf00      	nop
	200001c8 	.word	0x200001c8
	20000004 	.word	0x20000004

<SysTick_Handler>:
	b508      	push	{r3, lr}
	f7fe fb56 	bl	8000518 <HAL_IncTick>
	f7fe fbd1 	bl	8000612 <HAL_SYSTICK_IRQHandler>
	f7ff fff8 	bl	8001e64 <noOsSystickHandler>
	bd08      	pop	{r3, pc}

[fpistm]

Hi @ghent360
really sound strange as the delay well manages the '0'.

Arduino_Core_STM32/cores/arduino/wiring_time.c

Lines 37 to 45 in 2cc7dc7

	void delay(uint32_t ms)
	{
	if (ms != 0) {
	uint32_t start = getCurrentMillis();
	do {
	yield();
	} while (getCurrentMillis() - start < ms);
	}
	}

I would expect the optimization simply ignore it.

[fpistm]

You use the core 1.7.0 or the git repo (if yes on which commits?) ?

[ghent360]

The latest git code.

…

On Wed, Nov 27, 2019 at 2:51 AM Frederic Pillon ***@***.***> wrote: You use the core 1.7.0 or the git repo (if yes on which commits?) ? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#797?email_source=notifications&email_token=AB6FRVQPU24STF3MN5COKKLQVZGMNA5CNFSM4JSE2AY2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEFJDPUY#issuecomment-559036371>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB6FRVTIRKPIXFW5BGUXGMDQVZGMNANCNFSM4JSE2AYQ> .

[ghent360]

Tested at commit 2cc7dc7
When I call delay, it references getCurrentMillis, from clock.c, this is what brings the SysTick_Handler code. I tried replacing the call to delay(0) with getCurrentMillis() and the SPI starts to work.

There is probably some default "week" SysTick_Handler in the HAL, which gets used otherwise and it messes things up.

[ghent360]

Found it, it is defined in the startup_stm32xxxx.s file:
.weak SysTick_Handler
.thumb_set SysTick_Handler,Default_Handler

So by default it is a noop and the value from HAL_GetTick dies not change. Not sure why that would affect the SPI HAL code though.

[fpistm]

This was introduced by #677.

[fpistm]

@ghent360
In fact we are in this case:
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.faqs/ka15833.html

the first found is used when delay() is not in the sketch because the first found is used and in our case this is the one from the startup_stm32xxx.S.
When delay() is in the sketch then as the sketch is not in the core.a archive it is well resolved.
Moving the clock.c to the SrcWrapper/src directory solve the issue, anyway I will not do that.
I have to find a better way to ensure all defined weaked symbol is used instead of the default... 😭

[ghent360]

The issue is that both symbols are in core.a and the linker picks the first one it sees. The order of objects in the library archive comes into play. I tested that if you make the core.a archive with the startup_stm32yyxx.S.o at the end, then it picks the handles from clock.o

Proposals, not sure how feasible these are with the arduino IDE:
a) place the startup file in a separate library and list that one after core.a in the linking file order
b) re-order the core.a objects so that the startup object is last.

[fpistm]

@ghent360
I already try to do something like this without success. Arduino build process is a nightmare when you want made some specific stuff, it is hard to add customization.
I'm thinking about that and I think I will simply moved all stm32 part in a builtin library, like that all object will not be in the core.a only the Arduino API object.

[fpistm]

@ghent360
Let me know if you are agreed with #811 fixed.

[ghent360]

Yes looks fixed.