A bitwise operation that im confused with

Question

Hi im working on stm32f4. I need to read 8 pin and sum the values after convert like binary to decimal. For example if the values of 8 bits are 000000110, my CardID must be 6. i used this, but there can be better way:

CardID = (GPIOD->IDR & GPIO_Pin_8) + 
         (GPIOD->IDR & GPIO_Pin_9)*2 + 
         (GPIOD->IDR & GPIO_Pin_10)*4 + 
         (GPIOD->IDR & GPIO_Pin_11)*8 + 
         (GPIOD->IDR & GPIO_Pin_12)*16 + 
         (GPIOD->IDR & GPIO_Pin_13)*32 + 
         (GPIOD->IDR & GPIO_Pin_14)*64 + 
         (GPIOD->IDR & GPIO_Pin_15)*128;

Hint: i know the value of GPIO_Pin_8,GPIO_Pin_9....GPIO_Pin_15.

Can we do the AND operation with a number of combinations?

#define GPIO_Pin_8                 ((uint16_t)0x0100)  /* Pin 8 selected */
#define GPIO_Pin_9                 ((uint16_t)0x0200)  /* Pin 9 selected */
#define GPIO_Pin_10                ((uint16_t)0x0400)  /* Pin 10 selected */
#define GPIO_Pin_11                ((uint16_t)0x0800)  /* Pin 11 selected */
#define GPIO_Pin_12                ((uint16_t)0x1000)  /* Pin 12 selected */
#define GPIO_Pin_13                ((uint16_t)0x2000)  /* Pin 13 selected */
#define GPIO_Pin_14                ((uint16_t)0x4000)  /* Pin 14 selected */
#define GPIO_Pin_15                ((uint16_t)0x8000)  /* Pin 15 selected */

Answer 1

This is likely wrong:

• You should not read the input data register multiple times to get corresponding values. The register is defined volatile, so the compiler can not optimize away accesses.
• masking with & does not return 0 or 1 (boolean result), but leaves the bit in the same position it has. The multiplications will move them to positions 15..22, while presumably they should be moved to bits 0..7.

As these are consecutive bits, the correct way would be:

// must be consecutive bits:
#define GPIO_CARD_ID_MASK  0xFF00U
#define GPIO_CARD_ID_SHIFT 8

CardID = (GPIOD->IDR & GPIO_CARD_ID_MASK) >> GPIO_CARD_ID_SHIFT;

This only reads the register once, thus avoiding race-conditions and - possibly more important here - saves many clock cycles. It also is (almost) self-explaining by using properly named macros, thus better maintainable.

Note: For the given architecture/MCU, the mask is actually not necessary (these are the upper 8 bits of an uint16_t), but good style. It documents which bits you are being used. If the field does not happen to be the upper bits, you will need it anyway, so the mask makes it more verswatile (it still requires consecutive bits, discontinous numbering will make it much less straight-forward).