Using the BIT macro for multi-bit register fields is confusing

[bennoleslie]

In this code:

#ifdef CONFIG_PLAT_IMX7
    /* eanble the 24MHz source and select the oscillator as CLKSRC */
    gptcr |= (BIT(EN_24M) | (5u << CLKSRC));
#else
    gptcr |= BIT(CLKSRC);
#endif

(from libplatsupport/src/mach/imx/gpt.c) the use of the BIT macro is very confusing.

The value of CLKSRC is 6, but that field is not a single bit, rather it is a 3-bit field (bits 6, 7, 8).

Although it is the same after expansion, I think:

   gpctr |= (1u << CLKSRC)

would make the intent clearer. Or even:

#define NO_CLOCK 0
#define PERIPHERAL_CLOCK 1
#define HIGH_FREQUENCY_REFERENCE_CLOCK 2
#define EXTERNAL_CLOCK 3
....
...

 qpctr |= (PERIPHERAL_CLOCK << CLKSRC)