free-pdk / free-pdk-examples

Code Examples for Padauk MCUs using the free-pdk/SDCC toolchain
15 stars 10 forks source link

SDCC function _sdcc_external_startup will be changing to __sdcc_external_startup #9

Open basilhussain opened 1 year ago

basilhussain commented 1 year ago

Some advance warning: as of current SDCC development version 4.2.10, and for the forthcoming release version 4.3.0, the _sdcc_external_startup function has been renamed to __sdcc_external_startup (note the additional underscore at the beginning). See SDCC feature request #859.

This means all the examples in this repository that define a function with the former name will no longer work properly, because the function will not be called. Also, the failure may be entirely transparent, because SDCC may not warn about an unused function.

Perhaps old and new versions of SDCC can be accommodated with some conditional defines?

serisman commented 1 year ago

Yes, I noticed that a few weeks ago. It caused some really annoying bugs that were hard to track down as well. Because we usually setup and calibrate the clock in this function, it means that pretty much every program written for these Padauk MCUs will probably have to be updated, or they will (silently) fail to operate properly.

For now, I've been using this in some of my projects:

#if __SDCC_REVISION >= 13762
unsigned char __sdcc_external_startup(void) {
#else
unsigned char _sdcc_external_startup(void) {
#endif
...
}

But that is pretty ugly.

Maybe @spth has some ideas for a better way to handle this (ideally some kind of backward compatibility inside SDCC itself?

I suppose worst case we could to a conditional define in one of the common header files to implement backwards compatibility with a compiler warning maybe.

Something like this (in one of the include files) would probably work:

#if __SDCC_REVISION >= 13762
  #define _sdcc_external_startup __sdcc_external_startup
  #warning "Update to the newer __sdcc_external_startup syntax!"
#endif

But, that won't help someone that just updates SDCC without pulling in new include files.

basilhussain commented 1 year ago

ideally some kind of backward compatibility inside SDCC itself?

I would predict that it's unlikely.

Unfortunately for the PDK platform, the init code is generated by the compiler, rather than like on some other platforms where it is a separate, override-able, crt0 library that's linked in, so defines are probably going to be the only option.

Your solution involving the include file warning sounds good to me. Maybe reverse it so that it warns to update the version of SDCC.

BTW, I was motivated to file this issue because I have been referring to the FreePDK website documentation while putting together a patch to unify __sdcc_external_startup behaviour across platforms. Did you know that no platform other than MCS51 actually acted on the return value from __sdcc_external_startup? If my patch is accepted, the PDK platform will also respect the return value and skip GSINIT code if required.

serisman commented 1 year ago

Looks like a 'fix' for SDCC feature request #868 has been implemented with commit r13850.

JShabtai commented 5 months ago

This issue was causing the following error for me. Posting here so that others who run into this while getting started might find it, and then can fix it pretty easily

* IHRC SYSCLK=1000000Hz @ 4.00V ... calibration result: 0Hz (0x00)  out of range.

I've also submitted a PR that should fix this by adapting the macro that Serisman shared above (I've only tested it with sdcc version 4.3.0 but it's pretty straight forward)