|
| From: | Erik Walthinsen |
| Subject: | [avr-libc-dev] Re: What Happened to the sbi() and cbi() Macros???? |
| Date: | Mon, 31 Jan 2005 00:40:51 -0800 |
| User-agent: | Debian Thunderbird 1.0 (X11/20050116) |
E. Weddington wrote:
Well, the problem is that it's not the avr-gcc list is where this stuff is usually discussed, it's the *avr-libc-dev* list.Perhaps it would be good to create a single list, independent of gcc, libc, avrdude, etc. where developers can ask general questions (such as the issues re: atmega48/88/168), and leave the avr-libc-dev, avr-gcc-list, and avrdude-dev lists for people developing and debugging specific issues related to those packages? This allows a single point of contact for all *users* of all three projects, where issues like this can be discussed with *all* interested parties. Announcing and discussing proposed deprecation on the avr-libc-dev list obviously did not elicit the discussion that needed to be elicited, as very few of the affected *users* were subscribed to the list.
It was discussed on avr-libc-dev a while ago. These macros were deprecated and have been that way for a while. Yes, it was even in the documentation that they were deprecated.I noticed the intent to deprecate in the documentation when I first started using this stuff, but I guess I figured the concept of removing sbi/cbi seemed so ludicrious that someone must have been joking.
It had more to do with me not having enough time to explain it; I assumed that by looking at how they were implemented, the reason for removal would become self-evident.That really seems to be the root of the problem, none of us can figure it out.
I'm certainly not against having a non-confrontational discussion about this, so let's revisit the subject. To this end, I've included Joerg Wunsch in the CC list as he's also now co-maintaining avr-libc and I've also cross-posted this to the avr-libc-dev list, where it really belongs.We can move this discussion there wholly, if everyone involved is on the list. Would rather avoid CC: explosion if possible <g>
On that topic I will again mention that I haven't seen a single one of my emails come through to the avr-gcc list, with error messages from my mail server indicating that it simply cannot talk to the avr1.org SMTP server. Where it gets really screwy is that I just successfully telneted into port 25 on avr1.org from the same IP... I've sent additional emails since through a different server (mail.pdxcolo.net) but have seen neither my messages on the list nor any error messages from that mail server...
This was adopted as what should be used, so outb(), inb(), outp(), and inp() were deprecated. In fact, the later versions of those macros were implemented as the "direct access" show above.As I said in another email that like all my others has not come through on the avr-gcc list, this makes perfect sense. These macros are *obsolete*, should never be used because they are arcane, and their function has been taking over completely by direct memory-mapped register access.
cbi() and sbi() were similarly implemented, and it was thought instead of having a somewhat "redundant" macros was rather pointless, and they too were deprecated.This is where I get completely lost. These macros do *not* represent an arcane and obsolete means of performing an action that is *exceedingly* common on all microcontroller architectures, namely bit-twiddling.
Now before anybody gets up in arms about this, I just want to point out, that these macros were labeled as deprecated in the avr-libc user manual for some time. There are many open source projects where items that are deprecated in one or two releases, are removed in later releases, for example: gcc, gdb, binutils, etc.Yes, but I have never seen an example of such a commonly used feature being removed. The best analogy I can come up with on the spot is gcc deciding to deprecate the ability for one to assign values to variables at the same time they are declared, e.g.:
int num = 0;Ignoring the C standard for the duration, the argument would seem to be that this ability is really not all that interesting, it's not so explicit in *when* it presets the value, and there's a perfectly good alternate (yet more long-winded) means of doing the same thing, namely:
int num; num = 0;I'm sure someone can come up with a better analogy, esp with some thinking time, but I hope my point is made.
We are certainly not unwilling to listen, but honestly, we never got any feedback from people who wanted to keep it. What were we supposed to do?Not sure, this being not unusual, and I have much personal experience as the original author of GStreamer[.net]. However, if I were in the position of wishing to remove something that I knew to be commonly used, I would have made a lot more noise about it, in the form of repeated RFQ's, in every conceivable list that might be affected.
This is entirely possible, but is a royal pain and fundamentally seems to be nothing more than making work for almost every author of code out there.Now, on moving forward...Bruce, for "legacy" code that must require cbi() and sbi(), would it be reasonable to patch your own version of avr-libc to provide these macros? The implemenations are not that hard.
As to the idea that these were very useful macros, then I have a counter-proposal:Yes, and I would very much welcome such operations, especially bit_toggle(). Here are my comments though:I would like to create an API that can be used to operate on bits. I propose that this would be a new header, <avr/bit.h>. In this header, would be macros that would set, clear, toggle, read, and test bits in a variable. These macros would also be defined to work on variables of size 8 bit, 16 bit and 32 bit. For example:---------------------------------------------------------------- #include <inttypes.h> #define bit_set_8(var, mask) ((var) |= (uint8_t)(mask)) #define bit_clear_8(var, mask) ((var) &= (uint8_t)~(mask)) #define bit_toggle_8(var, mask) ((var) ^= (uint8_t)(mask)) #define bit_read_8(var, mask) ((var) & (uint8_t)(mask)) #define bit_set_16(var, mask) ((var) |= (uint16_t)(mask)) #define bit_clear_16(var, mask) ((var) &= (uint16_t)~(mask)) #define bit_toggle_16(var, mask) ((var) ^= (uint16_t)(mask)) #define bit_read_16(var, mask) ((var) & (uint16_t)(mask)) // 32 bit versions here // Shorter named versions for the common operation. #define bit_set(var, mask) bit_set_8(var, mask) #define bit_clear(var, mask) bit_clear_8(var, mask) #define bit_toggle(var, mask) bit_toggle_8(var, mask) #define bit_read(var, mask) bit_read_8(var, mask) ---------------------------------------------------------------- There are several reasons why I think this would be useful:- Doing bit operations with the C language bit operators is confusing to most newbies. This usually has to be explained again and again. These macros make it easier for newbies. - It's makes it easier to read what operation is happening rather than trying to remember the bit operations themselves. - The typecasts are in there to workaround the C language Standard of promoting the operands of bit operaters to ints, which are 16 bits for the AVR (see the avr-libc FAQ for more discussion of this). If these macros are used, then users don't have to remember to do the typecasts, which should, in theory, help with the size of their code.I would also like to propose two other macros for this file: #define BIT(x) (1 << (x)) #define LONGBIT(x) ((uint32_t)1 << (x))Yes, the BIT() macro is the same as _BV(). The _BV() macro is confusing in two ways: one has to remember that "BV" stands for Bit Value. The name BIT is slightly more descriptive. And _BV() has a leading underscore, which technically is supposed to be reserved for the "implementation" according to the C Standard (in this case the library). Having to type out the underscore is annoying. I feel that it's easier to just write out "BIT".The LONGBIT macro is needed to provide a conversion from a bit number to a 32-bit mask.Personally, I don't care whether the macro names are upper or lower case. I'm fine with bit() and longbit() too.I feel that it is important that the bit macros supply a bit "mask" and not just a bit number because a bit mask is more flexible in defining non-contiguous bits to operate on. This means that one can do this:bit_set(PORTD, BIT(0) | BIT(2) | BIT(4) | BIT(7)); bit_clear(PORTG, BIT(1) | BIT(6)); bit_toggle(PORTE, BIT(5) | BIT(3)); bit_read(PORTF, BIT(7)); Would this proposal be useful?
1) I'm not sure there is any need for distinct 8/16/32-bit versions though, is the compiler not smart enough to simply understand:
#define bit_set(var, mask) ((var) |= (mask))C is a rather strongly-typed language, thus AFAIK the compiler is smart enough to promote (mask) to the same type as (var) without any further ado.
2) Is bit_read() any different from bit_is_set()? Currently we have: #define bit_is_set(sfr, bit) (_SFR_BYTE(sfr) & _BV(bit))which is functionally identical (though brings up again one of the root questions of whether _SFR_BYTE is relevant any longer). I can certainly see the use the alias bit_read(), as it isn't obvious by name that bit_is_set() operates the way it does. In fact, I wonder if it would make sense to move to the following:
#define bit_is_set(reg, bit) (((reg & BIT(bit)) ? 1 : 0) #define bit_read(reg, bit) ((reg) & BIT(bit))This altered bit_is_set() routine provides a strict boolean result, which is useful in some cases, but has the problem that I don't see any way for the compiler to code it without a branch. This may be a killer.
3) BIT() is definitely a more sane name for the _BV() function, but since you mention that the leading underscore is typically reserved for internal implementation, it begs the question of why the BV() macro was deprecated either.
4) Being able to operate on multiple bits at the same time, in a separate namespace such as these bit_* functions, would make some things easier, but I worry that it could create new confusion, specifically caused by the *singular* bit prefix. What I would do in this case is to have *two* sets of macros, bit_* and bits_*, which operate accordingly. Kinda hoses the English, as bits_set is kinda odd, but them are the breaks when it comes to sane prefixing.
Further discussion of the above additions would be good, and I'd love to see something along those lines added to avr-libc. However, it leaves the original issue open: should sbi/cbi/BV been removed?
What I fail to understand is exactly what we *lost* by having these functions in place. It is patently clear what we lost by *removing* them, namely a set of exceedingly convenient macros and backwards compilability for an insanely large set of people's programs.
But it seems from your above comments that the only reason for removing sbi/cbi was in order to remove potential confusion among users as to what the routines did, whether they used the assembly instructions of the same name or not, etc.
Why then can't the documentation simply be updated to explain this? Explain the origin of the macros names, what they used to mean and why, and what they mean now. Explain what the "pure" C equivalent is (though Doxygen takes care of this nicely be including the macro definition verbatim) and what the potential advantages and disadvantages are in using these macros.
The people reading this documentation aren't stupid, they're developing firmware for a microcontroller. GNOME removes fundamental features left and right because they seem to think that they need to shave all their uses down to a lowest common denominator. *This* is by far the most obviously elitist and arrogant behavior I've seen to date. But developers using avr-libc should be smart enough be able to read the documentation and make their own decision about what they want to write in the future.
So the question I'd like to discuss is: why do these macros have to be removed? What do we *gain* by no longer having them in the library? I don't see any advantages *at all* to having removed them, only the massive problems the change causes for all of my projects.
| [Prev in Thread] | Current Thread | [Next in Thread] |