Reputation: 2428
Let's suppose I have two addresses mapped in memory:
#define MY_REGISTER_1 (*(volatile uint8_t *)(0x1B))
#define MY_REGISTER_2 (*(volatile uint8_t *)(0x18))
I wanna create a function that set a bit in one register, like this:
set_bit_low(MY_REGISTER_1, 3);
Is it correct to declare my function as below?
void set_bit_low(uint8_t, uint8_t);
Upvotes: 0
Views: 186
Reputation: 47563
Many answers already mentioned that passing MY_REGISTER_1
the way you defined it to a function would result in a copy of it to be sent. However, the actual solution often used in microcontrollers is different than what the other answers suggest.
Usually, the registers are defined the way you defined them, i.e.
#define MY_REGISTER_1 (*(volatile uint8_t *)(0x1B))
#define MY_REGISTER_2 (*(volatile uint8_t *)(0x18))
is ok. However, you should take care not to pass them to a function if you expect them to be changed by that function. The cleanest way to perform bit manipulation of such registers is through macros. For example:
#define SET_BIT_LOW(reg, n) (reg) &= (uint8_t)~(1 << (n))
#define SET_BIT_HIGH(reg, n) (reg) |= (uint8_t)(1 << (n))
This not only correctly manipulates the actual register, but also has the added benefit that you avoid a function call. If these were functions, n
wouldn't be known and therefore the shift and not operations would have to be done either way as well as a (relatively) costly function call. With macros, the compiler could already calculate the right-hand-side expressions if n
is a constant and replace the line with 1 or 2 instructions.
Upvotes: 0
Reputation: 532
No. You want to alter the content of the memory location. So, you must pass it as pointer.
The function header should like
void set_bit_low(uint8_t *, uint8_t);
The function shoul like
void set_bit_low(uint8_t *my_reg, uint8_t bit_location)
{
// do your bit manupulation here by assigning to *my_reg.
}
Upvotes: 1
Reputation: 40859
Let's start with your definitions:
#define MY_REGISTER_1 (*(volatile uint8_t *)(0x1B))
dereferences the octet at address 0x1b
, so it represents the content of your memory mapped register, rather than its location. It would be preferable to have a macro with the location of your register(s):
#define REGISTER_1 ((volatile uint8_t *) (0x1B)) /* Substitute _1 for some meaningful name */
which you can then dereference at will, and you can declare your function as follows:
void set_bit_low(volatile uint8_t *register, uint8_t bitpos);
/* Use like this: */
set_bit_low(REGISTER_1, 3);
Upvotes: 2
Reputation: 2824
The following should be the correct way to declare your function:
void set_bit_low(uint8_t *, uint8_t);
Upvotes: 1
Reputation: 11566
This:
#define MY_REGISTER_1 (*(volatile uint8_t *)(0x1B))
Yields a copy of the value in the (mem mapped) register. So this:
set_bit_low(MY_REGISTER_1, 3);
Would pass that copy to set_bit_low()
. You can then set the bits in the copy (which would be local to the function). If you want to set bits of the value in the register, then you have to pass the address itself:
#define MY_REGISTER_1_ADDR 0x1b
Make sense?
Upvotes: 0