driver/sdram.c: Potential breakage due to naive code

[a3f]

The file has instances like

at91bootstrap/driver/sdramc.c

Lines 109 to 110 in 1d9e673

	/* Pause cycles */
	for (i = 0; i < 1000; i++) ;

which a reasonable compiler would just optimize away. Proper waiting with e.g. PIT-based timer would resolve this.

I found this while looking into at91bootstrap boot-time breakage on older ARM9-based SoC customer experienced with updated toolchain. It was decided to move to barebox as first stage bootloader as it's already used for second stage. Just dropping an issue here as a heads up.

Cheers,

[ehristev]

Hi,

Thanks for the heads-up. This is pretty old code, which has not been used on any of the new platforms. It definitely needs a rework, but this would also mean a lot of testing. So it will not be on a priority list at the moment.
If the sdramc works like this, then changes could bring improvement but could also add bugs.
Does your platform work fine with this code or you found some issues ?

Eugen

[a3f]

Customer observed breakage on a custom at91sam9263-based board, which no longer booted after updating to newer Yocto-built toolchain and a newer at91bootstrap didn't help. I'll ask a colleague about the software versions involved in case you want to reproduce.

[ehristev]

If it's a breakage with newer toolchain then it's a bug and we would have to fix it. We have to prove it by testing with and without the changes into the delays, such that we are sure that this is the root cause.
Another way is to attempt to build the bootstrap with the new toolchain with less optimizations, like -O0 and see how it goes.
I don't have an sam9263 board at my disposal, but this driver is used by a SiP which we have with sam9x60 which has the SDRAMC embedded and this SiP has 8 MB of SDR-SDRAM connected with this controller.
When I initially tested this SiP was about two years ago and the driver worked.
If the customer fixes this code on his custom board and can provide a patch, then I can merge it.

[a3f]

If it's a breakage with newer toolchain then it's a bug and we would have to fix it. We have to prove it by testing with and without the changes into the delays, such that we are sure that this is the root cause.

Building at91bootstrap-3.8.7 with GCC-8.2 worked, but building same with GCC-9.3 yielded an image that was no longer bootable.

Another way is to attempt to build the bootstrap with the new toolchain with less optimizations, like -O0 and see how it goes.

That would be one possibility to narrow it down, yes.

I don't have an sam9263 board at my disposal, but this driver is used by a SiP which we have with sam9x60 which has the SDRAMC embedded and this SiP has 8 MB of SDR-SDRAM connected with this controller.
When I initially tested this SiP was about two years ago and the driver worked.

Ah, I didn't know that one had the same SDRAMC.

If the customer fixes this code on his custom board and can provide a patch, then I can merge it.

As mentioned, customer decided to migrate away from at91bootstrap, so we won't look into this further. I hope the mentioned versions are useful for you in case you want to reproduce though.

Cheers,
Ahmad