I would prefer this change to xtimer to be introduced in a separate PR.

Might make sense, it's in a separate commit in any case :-)

Yes, but from a review perspective it makes even more sense. The know-how about semaphores and xtimer can be assumed to be split among the reviewers, so this PR would essentially need to ACKs: one from a person knowing about xtimer and one about semaphores ;-).

Can this be a inline function instead.

done 👍

Do you really need thread flags and mutex?

Absolutely right, I removed the flags dependency 👍

mh.... actually I meant it the other way around. sema used to be based on flags (or rather posix_semaphore was before we took the base implementation out of the POSIX wrapper and made it message-based) and it didn't work out as we thought it might (see also size of the current sema_t struct vs how it was before).

Before it was unsigned int + priority_queue_node_t, assuming a 32-bit processor this is 4 + 12 = 16 byte. Now it is unsigned int + enum + mutex_t which is 4+4+4 = 12 bytes. This is 4 byte less. One could remove the enum and use signed int for the value (negative meaning destroy) and could save 4 bytes more (the posix implementation uses a signed int).

I liked the point to remove the flags dependency because adding the flags adds 2 bytes to every thread stack even if the semaphore is only used by some of the threads.

Okay, I somehow thought priority_queue_node_t was just 4 byte...

If you remove that: Doesn't lwIP needs fixing, too?

No, lwIP was calling sema_wait_timed which was calling sema_wait_timed_msg. However, your next point about the return values makes that lwIP requires fixing, I'll rollback the return values to the original ones.

Here and in other places: Changing these return values changes the behavior of the POSIX wrapper (it uses the result to set errno, as specified by the POSIX specifications). Either we drop this behavior (and don't care about POSIX compatibility and also fix the POSIX wrapper accordingly) or these values needs to stay the same.

I restored the "old" return codes 👍

Why is this prefered over just taking sema->value (either remove function or rationalize in Doxygen).

Actually it doesn't make a difference, so I removed it 👍 Then there was no need to touch the semaphore.h file :)

Keeping us in the Copyright but removing our authorship? ;-)

(though given the amount of code left I understand the latter step)

Yes, I'm not very clear with that, sorry. Maybe we should write a guideline on when to add/remove an author from a file (I think there's currently any, right?). The criteria I followed is:
sema.h I did some significant additions but most of the code remained, so I added myself. In sema.c the code changed completely, so there was no code left from the previous authors, neither the authors.

I didn't want to remove the copyright because this is the legal thingy... but maybe I should remove it as well. Or do you prefer that I keep all authors and all copyrights?

I don't really care... Just thought it looked quaint that the copyright was amended and the authors stripped ;-)

Minor legacy error that might be fixable in this PR: s/SEM_H_/SEMA_H_/g

I see benefit in initializing a semaphore with a non-zero value e.g. to let more than one thread pass initially (furthermore, you also kept it for dynamic-equivalent sema_create()). So why change it? Or why must it be 0 (as per documentation).

Good point, actually it can only be initialized with a value different from 0. In order to initialize with 0 I need to initialize the mutex as locked, and this requires a change in the mutex.h file to make it clean (see #6210).

If that get's merged fast then I'll update this PR, if this PR gets merged first, I'll create a new one :-) The only addition would be:

/**
 * @brief   Creates 0 initialized semaphore statically.
 * @return  Statically initialized semaphore.
 */
#define SEMA_CREATE_EMPTY()        { (value), SEMA_OK, MUTEX_INIT_LOCKED }

Here and below: please document the assertions you make in the implementation using the @pre doxygen command.

Done

1. SEMA_CREATE_EMPTY does not exist
2. I don't understand the reasoning behind this restriction.

See #6155 (comment) the restriction is that when the semaphore value is 0 the mutex needs to be locked.

Ah sorry, github hid that from me.

Might be problematic, since it is an API change, but since we only have one state after return it might make sense to set the return type to void.

I left it to keep the old return values meaningful. This implementation can't fail here so the return value makes no difference, but the same applies to the creation of the semaphore.

I'm happy to change it to void... I've no strong opinion about this one... your call.

void would minimize the code which in our use case is priority no. 1 ;-). So please go ahead.

Might be problematic, since it is an API change, but since we only have one state after return it might make sense to set the return type to void.

About this you already complained earlier (here and here). The old return codes were restored (except if they made no sense anymore) and we aded a @pre tag. In any case this code is only used inside the posix semaphore and lwip and both have been adapted and tested respectively.

About this you already complained earlier (here [1] and here [2]).

Mh, I would argue that [1] was about the change of the value itself and [2] was about the undocumented precondition (which is a different scope than return values because an unmet precondition leads to a crash, while an error reported by a return-value can be handled gracefully). However the comment in OP of this conversation was about arguing to remove, because according to this documentation the function only has one possible return value (0) and will always succeed, so having a return value seems redundant.

Weird sentence structure

Me English no native. Will fix ;-)