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

Newlib local patch: Add weak symbols for locking functions #13

Open
wants to merge 1 commit into
base: lx106-g++
Choose a base branch
from

Conversation

projectgus
Copy link

Hi Max,

I don't know how you'll feel about this patch, but I thought I'd ask. It lets me produce a thread safe newlib that I can link with FreeRTOS' locking primitives for esp-open-rtos.

There might be a better way to do this that I don't know of. Most of the people using FreeRTOS with newlib seem to just modify it directly and compile a FreeRTOS-specific libc, but I figured that was a last resort.

Cheers,

Angus


This allows a lock implementation to be optionally added at link time.

Small performance hit if using single threaded, in that locks are now a
call to a no-op dummy function rather than a compiled in no-op.

This allows a lock implementation to be added at link time.

Small performance hit if using single threaded, in that locks are now a
call to a no-op dummy function rather than a compiled in no-op.
@jcmvbkbc
Copy link
Owner

Hi Angus,
so, for the FreeRTOS you'll configure it with --enable-newlib-multithread and provide locking functions.
People that don't need multithreading are not affected at all by this change.
And this is to separate locking implementation from the libc, right?

This looks a bit awkward, because then you've no control over the lock structure, it's fixed at the compile time. Maybe instead of implementing such locking for xtensa it'd be better to implement locking for FreeRTOS in a way similar to linux implementation?

@projectgus
Copy link
Author

Yes, I agree it's a bit awkward.

To explain my current understanding, I think crosstool builds newlib with --enable-newlib-multithread=yes at the moment (it's the default), but for all non-Linux platforms these are dummied out to no-ops in /include/sys/lock.h.

Just to check I understand the "similar to Linux" suggestion - the Linux implementation ends up weak linking against pthread functions. So a FreeRTOS-like implementation of this would mean weak linking against FreeRTOS mutex functions?

I was hoping to avoid having to compile a FreeRTOS-specific libc. Maybe this is a mistaken priority, though.

If you'd be happy merging a weak-linked FreeRTOS locking implementation into the "mainline" esp crosstool as default then that'd be great from my point of view! ie libc will link FreeRTOS symbols if they're present at link time, or no locking if they're not. I didn't think that'd be a good idea from your/crosstool's point of view though, it seems very implementation-specific for a generic toolchain project.

If it came down to requiring a specifically FreeRTOS-compiled libc, I think it's probably easier for everyone if I I just fork/build a FreeRTOS-specific newlib and put it into esp-open-rtos. There are already two "special cases" that esp-open-rtos needs - the other one is putting .text in .irom, which currently happens via a special target in esp-open-sdk. So it probably makes sense to isolate those in a custom libc (hopefully tracking upstream closely for everything else), and then people can use any toolchain.

I was initially reluctant to do that, but maybe that's actually the best outcome here.

Does that reasoning make sense? I guess I'm happy to do any of these 3 approaches mentioned (current implementation-agnostic approach, compile a weak linked FreeRTOS libc by default in crosstool, or split off a FreeRTOS libc just for esp-open-rtos). I'd be interested to know what you think is the best approach.

Although after writing it all out I'm leaning a lot towards just forking a FreeRTOS-specific libc now - seems the most usable approach, and it could always merge later.

Cheers!

@projectgus
Copy link
Author

This looks a bit awkward, because then you've no control over the lock structure, it's fixed at the
compile time. Maybe instead of implementing such locking for xtensa it'd be better to implement
locking for FreeRTOS in a way similar to linux implementation?

I went to write a FreeRTOS-specific locking implementation but it ended up looking generic anyway, just because the FreeRTOS locking primitives don't match the newlib API (you can't statically initialise a FreeRTOS mutex, but newlib's lock.h API assumes you can.)

So I stuck with this generic approach. For now I'm just bundling a newlib fork with esp-open-rtos. I also forward-ported your xtensa patches to newlib 2.2.0. Seems OK so far.
SuperHouse/esp-open-rtos@689cf87
&
http://github.com/projectgus/newlib-xtensa

Please close this PR if you like, it seems reasonable to use a forked newlib for this.

nitrotm pushed a commit to nitrotm/crosstool-NG that referenced this pull request May 17, 2016
libc/newlib: Canadian baremetal builds require core pass-1
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

Successfully merging this pull request may close these issues.

2 participants