Port bit masks rather than bit numbers?

Go To Last Post

22 posts / 0 new

Author

Message

blargg

Level: Hangaround

Joined: Fri. Aug 30, 2013

Posts: 104 View posts

Posted by blargg: Sun. Nov 3, 2013 - 09:43 PM

The port bit symbolic names are defined to the bit number rathers than the masks (bit values). This means that code is littered with bit shifts or various macros

TIMSK |= 1<<TOIE1;
TIMSK &= ~(1<<TOIE1);
TIMSK |= _BV(TOIE1);
TIMSK &= ~_BV(TOIE1);
if ( UCSRA & (1<<RXC) ) ...
if ( bit_is_set( UCSRA, RXC ) ) ...

rather than the trivial

TIMSK |= TOIE1;
TIMSK &= ~TOIE1;
if ( UCSRA & RXC ) ...

In cases where you need to know the bit number, a helper macro in the libraries can extract that from the mask:

#define BITN(m) (m&1?0 : m&2?1 : m&4?2 : m&8?3 : m&0x10?4 : m&0x20?5 : m&0x40?6 : m&0x80?7 : -1)

int bits_per_char = UCSRC >> BITN(UCSZ0) & 3;

This puts things how they should be, where the mask, which is almost always what you want, is immediately available and concise to specify, and the bit number, which is rarely needed, requires using a macro.

(moderators, please move to the appropriate section as it wasn't clear where this would go)

Tags:

AVR Microcontrollers, megaAVR and tinyAVR

ka7ehk

Level: 10k+ Postman

Joined: Fri. Nov 22, 2002

Posts: 21486 View posts

Location: Tangent, OR, USA

Posted by ka7ehk: Sun. Nov 3, 2013 - 10:31 PM

I use masks all the time.

In the end, there is no difference if you use defines. Its "just" text substitution. If you

#define TOIE1msk (1<<TOIE1)

and apply it as

TIMSK |= TOIE1msk;

it comes out exactly the same as

TIMSK |= (1<<TOIE1);

So, the only benefit is "readability".

Jim

Until Black Lives Matter, we do not have "All Lives Matter"!

gregd99

Level: Resident

Joined: Mon. Oct 10, 2011

Posts: 569 View posts

Location: Sydney, Australia

Posted by gregd99: Sun. Nov 3, 2013 - 10:39 PM

@blargg the following link might also be of interest. https://www.avrfreaks.net/index.php?name=PNphpBB2&file=viewtopic&p=40338

regards
Greg

sternst

Level: Raving Lunatic

Joined: Mon. Jul 23, 2001

Posts: 3499 View posts

Location: Rheine, Germany

Posted by sternst: Sun. Nov 3, 2013 - 11:10 PM

blargg wrote:

This puts things how they should be, where the mask, which is almost always what you want, is immediately available and concise to specify, and the bit number, which is rarely needed, requires using a macro.

Bit numbers are mostly used in assembler files. I doubt that the assembler supports the ?-operator.

BTW:
What is the purpose of your post at all? Do you vote for changing the semantics of the existing headers and breaking all the existing code? Surely that won't happen. Do you vote for changing it only for future headers? Also not a good idea.

Stefan Ernst

clawson

Level: Moderator

Joined: Mon. Jul 18, 2005

Posts: 100115 View posts

Location: (using avr-gcc in) Finchingfield, Essex, England

Posted by clawson: Sun. Nov 3, 2013 - 11:12 PM

You can always convert bit numbers to masks (<< operator) but going the other way is "tricky". Atmel chose for the SBI (etc) opcodes to use bit numbers.

kk6gm

Level: Raving Lunatic

Joined: Fri. Sep 11, 2009

Posts: 3762 View posts

Location: Sacramento, CA

Posted by kk6gm: Mon. Nov 4, 2013 - 12:05 AM

You lost me when you spoke of how things "should be." According to what standard? I consider this a non-issue. The (1<<) notation is a bit more typing, but it conveys more information (I am a bit mask). The alternative would require something like a "_msk" suffix, so it's a tossup.

One thing useful about the << notation is being able to write things like (7<<SOME_FIELD0) rather than SOME_FIELD2 | SOME_FIELD1 | SOME_FIELD0.

blargg

Level: Hangaround

Joined: Fri. Aug 30, 2013

Posts: 104 View posts

Posted by blargg: Mon. Nov 4, 2013 - 01:35 AM

kk6gm wrote:

One thing useful about the << notation is being able to write things like (7<<SOME_FIELD0) rather than SOME_FIELD2 | SOME_FIELD1 | SOME_FIELD0.

7*SOME_FIELD0_MSK

EDIT: added context

Last Edited: Mon. Nov 4, 2013 - 01:57 AM

ka7ehk

Level: 10k+ Postman

Joined: Fri. Nov 22, 2002

Posts: 21486 View posts

Location: Tangent, OR, USA

Posted by ka7ehk: Mon. Nov 4, 2013 - 01:53 AM

I don't understand your most recent post.

Jim

Until Black Lives Matter, we do not have "All Lives Matter"!

kk6gm

Level: Raving Lunatic

Joined: Fri. Sep 11, 2009

Posts: 3762 View posts

Location: Sacramento, CA

Posted by kk6gm: Mon. Nov 4, 2013 - 04:31 AM

blargg wrote:

kk6gm wrote:
One thing useful about the << notation is being able to write things like (7<<SOME_FIELD0) rather than SOME_FIELD2 | SOME_FIELD1 | SOME_FIELD0.

7*SOME_FIELD0_MSK

EDIT: added context

I propose that your approach (multiplying a bit mask) is not nearly as understandable. In fact, until this evening I have never seen it done, and I've been doing this since the 80s. And since you have added the _MSK suffix, you're not even saving characters. So again, why? By what standard do you claim it "should be" done your way? If there's an argument to be made, just lay it out. I'm willing to be convinced.

kk6gm

Level: Raving Lunatic

Joined: Fri. Sep 11, 2009

Posts: 3762 View posts

Location: Sacramento, CA

#10

Posted by kk6gm: Mon. Nov 4, 2013 - 04:43 AM

BTW, the ARM compiler I use, Rowley Crossworks, supplies both bit number and bit mask, e.g. SOME_BIT_num and SOME_BIT_msk. No possibility of mistaking one for the other (as we see a lot here with AVR newbies).

westfw

Level: Raving Lunatic

Joined: Wed. Jun 19, 2002

Posts: 4549 View posts

Location: SF Bay area

#11

Posted by westfw: Mon. Nov 4, 2013 - 04:44 AM

Quote:

7*SOME_FIELD0_MSK

In which case SOME_FIELD0_MSK had better be a single bit...

I agree with Clawson: It's all about the ease of conversion...

ka7ehk

Level: 10k+ Postman

Joined: Fri. Nov 22, 2002

Posts: 21486 View posts

Location: Tangent, OR, USA

#12

Posted by ka7ehk: Mon. Nov 4, 2013 - 05:14 AM

What do you expect the "7" to represent?

Jim

Until Black Lives Matter, we do not have "All Lives Matter"!

clawson

Level: Moderator

Joined: Mon. Jul 18, 2005

Posts: 100115 View posts

Location: (using avr-gcc in) Finchingfield, Essex, England

#13

Posted by clawson: Mon. Nov 4, 2013 - 10:22 AM

Quote:

BTW, the ARM compiler I use, Rowley Crossworks, supplies both bit number and bit mask,

So does avr-gcc for Xmega. (_bm's and _bp's).

Personally I've always thought this might be a bit of fun if you throw away avr/io.h:

#include 

typedef struct {
	uint8_t SPR0:1;
	uint8_t SPR1:1;
	uint8_t CPHA:1;
	uint8_t CPOL:1;
	uint8_t MSTR:1;
	uint8_t DORD:1;
	uint8_t SPE:1;
	uint8_t SPIE:1;
} SPCR_t;

typedef struct {
	uint8_t SPI2X:1;
	uint8_t unused:5;
	uint8_t WCOL:1;
	uint8_t SPIF:1;
} SPSR_t;

typedef struct {
	SPCR_t SPCR;
	SPSR_t SPSR;
	uint8_t SPDR;
} SPI_t;

#define SPI (*(SPI_t *)0x4C)

int main(void)
{
	SPI.SPCR.SPE = 1;
	SPI.SPCR.MSTR = 1;
	SPI.SPCR.SPR1 = 1; //F_CPU / 64
	SPI.SPCR.CPOL = 1; // inverse polarity
	
	while (!SPI.SPSR.SPIF);
	SPI.SPDR = 0x5A;
}

However there are (several!) arguments against this. While the final source code may be the most readable of any potential solution it may not be very efficient though I have to say that 4.7.2 impressed me with:

//==> 	SPI.SPCR.SPE = 1;
	in r24,0x2c	 ;  tmp52,
//==> 	SPI.SPCR.CPOL = 1; // inverse polarity
	ori r24,lo8(82)	 ;  tmp52,
	ori r24,lo8(1<<3)	 ;  tmp52,
	out 0x2c,r24	 ; , tmp52
//==> 	while (!SPI.SPSR.SPIF);
	in __tmp_reg__,0x2d	 ; 
	sbrs __tmp_reg__,7	 ; 
.L2:
	rjmp .L2	 ; 
.L6:
//==> 	SPI.SPDR = 0x5A;
	ldi r24,lo8(90)	 ;  tmp57,
	out 0x2e,r24	 ;  MEM[(struct SPI_t *)76B].SPDR, tmp57

all though that contains run-time OR's that would not have been present in a single SPCR assignment.

Also this way of defining registers and bits, unlike the normal avr/io.h approach could not be used in the avr-as assembler which knows nothing about typedef's or struct's.

For comparison here is the equivalent code using the current avr/io.h (and the _BV() macro just to be perverse):

int main(void) {
	SPCR = _BV(SPE) | _BV(MSTR) | _BV(CPOL) | _BV(SPR1);
	while (!(SPSR & _BV(SPIF)));
	SPDR = 0x5A;
}

It generates:

//==> 	SPCR = _BV(SPE) | _BV(MSTR) | _BV(CPOL) | _BV(SPR1);
	ldi r24,lo8(90)	 ;  tmp46,
	out 0x2c,r24	 ;  MEM[(volatile uint8_t *)76B], tmp46
.L2:
//==> 	while (!(SPSR & _BV(SPIF)));
	in __tmp_reg__,0x2d	 ; 
	sbrs __tmp_reg__,7	 ; 
	rjmp .L2	 ; 
//==> 	SPDR = 0x5A;
	ldi r24,lo8(90)	 ;  tmp49,
	out 0x2e,r24	 ;  MEM[(volatile uint8_t *)78B], tmp49

I guess my point is that nothing says you HAVE to use avr/io.h, it's provided as a luxury but if you have a preferred mechanism for defining register and bit names then feel quite free to do it yourself. Though be warned that defining the names for just a single AVR could be a LOT of work. Just doing one register group for one AVR (48/88/168/328) above wasn't exactly trivial. It also opens itself for a lot of typographical errors. At least the compiler headers are widely peer reviewed by everyone who uses them and the subtle typing errors (like the double underscores in some of Atmel's work) are quickly found.

sparrow2

Level: Raving Lunatic

Joined: Mon. Oct 7, 2002

Posts: 6979 View posts

Location: Denmark

#14

Posted by sparrow2: Mon. Nov 4, 2013 - 10:35 AM

Quote:

top say that 4.7.2 impressed me with:

Not me :)
take a look at the generated code. It will loop forever!

clawson

Level: Moderator

Joined: Mon. Jul 18, 2005

Posts: 100115 View posts

Location: (using avr-gcc in) Finchingfield, Essex, England

#15

Posted by clawson: Mon. Nov 4, 2013 - 10:45 AM

Oh dear. I thought this might have been an artefact of my .s mangling program but that self-referential RJMP really exists in what the compiler generated. I guess I just found a compiler bug as I cannot immediately see anything wrong in what I wrote - can anyone else? If not I'll report this to Bugzilla.

EDIT: OK I know I didn't add the necessary "volatile"s as I was just messing. But even so surely the generated code does not match the programmers intention anyway however the compiler might have chosen to re-order things.

joeymorin

Level: 10k+ Postman

Joined: Tue. Jul 17, 2012

Posts: 11938 View posts

Location: Location, Location

#16

Posted by joeymorin: Mon. Nov 4, 2013 - 05:01 PM

clawson wrote:

OK I know I didn't add the necessary "volatile"s as I was just messing.

That seems to do the trick:

typedef struct {
	volatile SPCR_t SPCR;
	volatile SPSR_t SPSR;
	volatile uint8_t SPDR;
} SPI_t;

	while (!SPI.SPSR.SPIF);
  18:	0d b4       	in	r0, 0x2d	; 45
  1a:	07 fe       	sbrs	r0, 7
  1c:	fd cf       	rjmp	.-6      	; 0x18 <__zero_reg__+0x17>

Alternately, the whole struct could be declared volatile instead of each of its members. It's fair to say that all I/O registers should be treated as volatile, however it seems more appropriate to be explicit about each register.

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience. Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast. Cheap. Good. Pick two."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

clawson

Level: Moderator

Joined: Mon. Jul 18, 2005

Posts: 100115 View posts

Location: (using avr-gcc in) Finchingfield, Essex, England

#17

Posted by clawson: Mon. Nov 4, 2013 - 05:10 PM

Joey,

Do you think the compiler is allowed to interpret my (non volatile) version as requesting an infinite loop? It just seems "wrong" to me.

Cliff

david.prentice

Level: 10k+ Postman

Joined: Sat. Feb 12, 2005

Posts: 30063 View posts

Location: Wormshill, England

#18

Posted by david.prentice: Mon. Nov 4, 2013 - 06:04 PM

Yes. The compiler is quite within its rights to optimise your SPSR.SPIF.

You or someone else could write a SED or PERL script to create the headers automatically from the ASM or XML headers.

I seem to remember a discussion about using structures in headers a few years ago. The Hitech compiler for M*crochip P*C18 takes this route.

Now that PCs are so powerful, there is no great worry about compiler speed. Using structs allows far stricter control of type checking. I bet that a correct (volatile) struct header will generate exactly the same code as the BIT_no version.

David.

clawson

Level: Moderator

Joined: Mon. Jul 18, 2005

Posts: 100115 View posts

Location: (using avr-gcc in) Finchingfield, Essex, England

#19

Posted by clawson: Mon. Nov 4, 2013 - 06:13 PM

Quote:

I bet that a correct (volatile) struct header will generate exactly the same code as the BIT_no version.

It won't as the bit setting is an RMW. I'm not sure how you could get a simple assignment? .... if you assigned values to all 8 fields in the struct I wonder if the compiler would be smart enough to avoid the RMW and just do an LDI/OUT?

joeymorin

Level: 10k+ Postman

Joined: Tue. Jul 17, 2012

Posts: 11938 View posts

Location: Location, Location

#20

Posted by joeymorin: Mon. Nov 4, 2013 - 06:16 PM

clawson wrote:

Do you think the compiler is allowed to interpret my (non volatile) version as requesting an infinite loop? It just seems "wrong" to me.

"Wrong", yes. "Allowed", yes:

int main(void) {
  char foo;
  while(!foo);	
}

The compiler can issue a warning:

foo.c:3:8: warning: 'foo' is used uninitialized in this function [-Wuninitialized]

... but without -Werror, compilation continues with foo assumed to be zero:

00000000 :

int main(void)
{
   0:	ff cf       	rjmp	.-2      	; 0x0

The curious thing is that this warning is absent when explicitly accessing an address:

#define foo *(char *)0x100
int main(void) {
  while(!foo);	
}

... but the assumed zero is the same:

00000000 :
#define foo *(char *)0x100
int main(void) {
   0:	80 91 00 01 	lds	r24, 0x0100
   4:	81 11       	cpse	r24, r1
   6:	01 c0       	rjmp	.+2      	; 0xa <__zero_reg__+0x9>
   8:	ff cf       	rjmp	.-2      	; 0x8 <__zero_reg__+0x7>
  while(!foo);	
}
   a:	08 95       	ret

... however the test is still performed.

The lack of a warning (and the test) is perhaps expected, as a memory location has no relationship to any identifier in program, so the language can't track its use.

A struct is also handled correctly with a test:

typedef struct {
	char bat;
} bar;

#define foo (*(bar *)0x100)

int main(void) {
	while (!foo.bat);
}

00000000 :

int main(void) {
   0:	80 91 00 01 	lds	r24, 0x0100
   4:	81 11       	cpse	r24, r1
   6:	01 c0       	rjmp	.+2      	; 0xa <__zero_reg__+0x9>
   8:	ff cf       	rjmp	.-2      	; 0x8 <__zero_reg__+0x7>
	while (!foo.bat);
}
   a:	08 95       	ret

Bit fields are also handled correctly:

typedef struct {
	char bat:1;
} bar;

#define foo (*(bar *)0x100)

int main(void) {
	while (!foo.bat);
}

00000000 :

int main(void) {
   0:	80 91 00 01 	lds	r24, 0x0100
   4:	80 ff       	sbrs	r24, 0
   6:	ff cf       	rjmp	.-2      	; 0x6 <__zero_reg__+0x5>
	while (!foo.bat);
}

Note that in all cases, there is an infinite loop. This is not a bug. The compiler has expressed its belief that if foo (or foo.bat) is zero that it will forever remain so. Since volatile was not used, and foo (or foo.bat) is not modified anywhere else in the program, this is a valid belief.

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience. Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast. Cheap. Good. Pick two."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

david.prentice

Level: 10k+ Postman

Joined: Sat. Feb 12, 2005

Posts: 30063 View posts

Location: Wormshill, England

#21

Posted by david.prentice: Mon. Nov 4, 2013 - 06:59 PM

Quote:

   while (!SPI.SPSR.SPIF);
  18:   0d b4          in   r0, 0x2d   ; 45
  1a:   07 fe          sbrs   r0, 7
  1c:   fd cf          rjmp   .-6         ; 0x18 <__zero_reg__+0x17>

This seems fine to me. I don't think you can use SBIS for 0x2d.

I still reckon it is worth machine generating a system header. However, you would probably have to go over all the other headers to ensure that you stick to one syntax.

From memory, the M*crochip arrangement has two styles of PIC18 compiler. Creating source code that is compatible with both is 'awkward' sometimes.

So regardless of the supreme advantages of using struct, you are never going to change the world of Tiny and Mega.

David.

joeymorin

Level: 10k+ Postman

Joined: Tue. Jul 17, 2012

Posts: 11938 View posts

Location: Location, Location

#22

Posted by joeymorin: Mon. Nov 4, 2013 - 07:30 PM

david.prentice wrote:

I don't think you can use SBIS for 0x2d.

That is the issue:

#include 

typedef struct {
  volatile uint8_t P0:1;
  volatile uint8_t P1:1;
  volatile uint8_t P2:1;
  volatile uint8_t P3:1;
  volatile uint8_t P4:1;
  volatile uint8_t P5:1;
  volatile uint8_t P6:1;
  volatile uint8_t P7:1;
} IOPORT8BITFIELDS_t;

typedef struct {
  IOPORT8BITFIELDS_t PIN;
  IOPORT8BITFIELDS_t DDR;
  IOPORT8BITFIELDS_t DAT;
} IOPORT8_t;

#define PORTB (*(IOPORT8_t *)0x23)
#define PORTC (*(IOPORT8_t *)0x26)
#define PORTD (*(IOPORT8_t *)0x29)

int main(void) {
  PORTB.DDR.P0 = 1;
  PORTC.DAT.P3 = 0;
  while (!PORTD.PIN.P5);
  PORTB.PIN.P0 = 1;
}

00000000 :

int main(void) {
  PORTB.DDR.P0 = 1;
   0:	20 9a       	sbi	0x04, 0	; 4
  PORTC.DAT.P3 = 0;
   2:	43 98       	cbi	0x08, 3	; 8
  while (!PORTD.PIN.P5);
   4:	4d 9b       	sbis	0x09, 5	; 9
   6:	fe cf       	rjmp	.-4      	; 0x4 <__zero_reg__+0x3>
  PORTB.PIN.P0 = 1;
   8:	18 9a       	sbi	0x03, 0	; 3
}
   a:	08 95       	ret

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience. Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast. Cheap. Good. Pick two."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

Jump To

Search form

Port bit masks rather than bit numbers?

Tags: