Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CH552, sdcc and delay.c #4

Open
dfxhub opened this issue Nov 18, 2023 · 3 comments
Open

CH552, sdcc and delay.c #4

dfxhub opened this issue Nov 18, 2023 · 3 comments

Comments

@dfxhub
Copy link

dfxhub commented Nov 18, 2023

Hi! Delays are not correct.

sdcc 4.2.0 compiles your implemetation to this:

;	src/delay.c:25: SAFE_MOD++;                     // 2 Fsys cycles, for higher Fsys, add operation here
	mov	a,_SAFE_MOD
	inc	a
	mov	_SAFE_MOD,a

and it is not 2 Fsys cycles, so f.e. DLY_ms(1000) at 24MHz becomes ~2+ secs. While this is more correct (about 1.200 secs at 24MHz) (as of Blinkinlabs or official SDK):

;	src/delay.c:25: ++SAFE_MOD;                     // 2 Fsys cycles, for higher Fsys, add operation here
	inc	_SAFE_MOD

Thanks.

@wagiminator
Copy link
Owner

Yes, I know, this delay routine is very inaccurate. It also varies because with the 8051 the number of clock cycles for jumps can only be predicted exactly under certain conditions. Depending on the version, the compiler then does the rest with its questionable results. For precise delays you have to use a timer or the touchkey interrupt flag.
In this case, however, the delay routine should not and does not have to be precise.

@dfxhub
Copy link
Author

dfxhub commented Nov 18, 2023

I agree in that for applications with strong timings requirements delays based on timers or any other hardware are preferred. But why not to use right syntax for particular compiler that leads to more reasonable results? Of cause, IMO, more right way for software delays with predictable results is using assembler and not to trust to compiler, or at least to check for it.

Thanks again for your helpful projects.

wagiminator added a commit that referenced this issue Nov 19, 2023
@wagiminator
Copy link
Owner

I've made a few changes so that the delays are hopefully more precise now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants