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

Timer driver refactoring #3120

Merged
merged 11 commits into from
May 23, 2018
Merged

Timer driver refactoring #3120

merged 11 commits into from
May 23, 2018

Conversation

digitalentity
Copy link
Member

Code simplification, de-duplication and better timer driver abstraction.
Work in progress. Seems to work well on REVO

@digitalentity
Copy link
Member Author

digitalentity commented Apr 25, 2018

Despite the fact that now all timers present on the CPU are included into the code (previously they were guarded by USED_TIMERS) code didn't became much bigger:

Before:

arm-none-eabi-size ./obj/main/inav_REVO.elf
   text    data     bss     dec     hex filename
 211416    5528   40648  257592   3ee38 ./obj/main/inav_REVO.elf
arm-none-eabi-objcopy -O ihex --set-start 0x8000000 obj/main/inav_REVO.elf obj/inav_2.0.0_REVO.hex

After:

arm-none-eabi-size ./obj/main/inav_REVO.elf
   text    data     bss     dec     hex filename
 211468    5528   40984  257980   3efbc ./obj/main/inav_REVO.elf
arm-none-eabi-objcopy -O ihex --set-start 0x8000000 obj/main/inav_REVO.elf obj/inav_2.0.0_REVO.hex

@digitalentity
Copy link
Member Author

Rebased, tested on REVO

@digitalentity
Copy link
Member Author

Size before:

arm-none-eabi-size ./obj/main/inav_OMNIBUSF7NXT.elf
   text    data     bss     dec     hex filename
 240876    9804   43792  294472   47e48 ./obj/main/inav_OMNIBUSF7NXT.elf

After:

arm-none-eabi-size ./obj/main/inav_OMNIBUSF7NXT.elf
   text    data     bss     dec     hex filename
 240764    9804   44776  295344   481b0 ./obj/main/inav_OMNIBUSF7NXT.elf

Timer tables are bigger - they now store all timers available on the CPU, not the used ones as before. But simpler and cleaner general timer code pays back - flash usage is not bigger.

@fiam
Copy link
Member

fiam commented May 7, 2018

There's +1K in BSS on F7 and +500 on F3, looks like from the globals in timer.c. Is there any easy way to make them readonly so they can use TEXT instead? Maybe we could automatically generate the code for timer metadata, since it has to be done only one per MCU.

@digitalentity
Copy link
Member Author

It's the timer run-time state that eats up bss. Maybe it's time to make this state dynamically allocated... State for each timer is 41 bytes (44 bytes if aligned), but it's only used when timer is configured for input (Softserial RX or PPM/PWM in).

@digitalentity
Copy link
Member Author

Switched to dynamic timer state allocation. Finally dynmemory allocator is used for something 😆

Copy link
Member

@DzikuVx DzikuVx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@digitalentity this is just too long ;) I trust you that if it works then it works ;)

@digitalentity
Copy link
Member Author

@DzikuVx I'd appreciate if you could test if SOFTSERIAL and LEDTRIP features work for you ;)

@DzikuVx
Copy link
Member

DzikuVx commented May 12, 2018

This I can do around Tuesday

@digitalentity
Copy link
Member Author

@DzikuVx if it works for you, please go ahead and merge this. I'll be in Moscow next week and most likely will have very limited connectivity.

@digitalentity digitalentity merged commit 9e6e2b3 into development May 23, 2018
@digitalentity digitalentity deleted the de_timer_cleanups branch May 23, 2018 06:44
@digitalentity
Copy link
Member Author

Ok, this is now merged. Let's see how it will work out

@stronnag
Copy link
Collaborator

flight tested on MATEKF405 and QUARKF4. Nothing bad happened.

@digitalentity
Copy link
Member Author

Good to know, thanks for testing @stronnag

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

Successfully merging this pull request may close these issues.

None yet

4 participants