C switch statement on a pointer

[HwAoRrDk]

In some C code I'm writing I need to do something different depending on the pointer to a particular USART peripheral that is passed to the function.

Switch statements only work off integer values, so in order to use a switch statement I ended up casting the switched-on and case values as uintptr_t.

Code: [Select]

/* From an include file: */
#define PERIPH_BASE                             ((uint32_t)0x40000000)
#define APB2PERIPH_BASE                         (PERIPH_BASE + 0x10000)
#define USART1_BASE                             (APB2PERIPH_BASE + 0x3800)
#define USART1                                  ((USART_TypeDef *)USART1_BASE)
/* ...and similar for USART 2,3,4 */

typedef struct {
USART_TypeDef *uart;
/* etc... */
} uart_context_t;

void my_function(uart_context_t *ctx) {
/* etc... */
switch((uintptr_t)ctx->uart) {
case (uintptr_t)USART1: RCC->APB2PCENR |= RCC_USART1EN; break;
case (uintptr_t)USART2: RCC->APB1PCENR |= RCC_USART2EN; break;
case (uintptr_t)USART3: RCC->APB1PCENR |= RCC_USART3EN; break;
case (uintptr_t)USART4: RCC->APB1PCENR |= RCC_USART4EN; break;
}
/* etc... */
}

But this feels a bit hacky to me, a bit like I shouldn't be doing it.

Is there a better way?

[pqass]

Rather than defines for each USART instance, create a struct and then initialize an array of them:

Code: [Select]

struct usart_t { void *base, ... };
static struct usart_t usarts[] = {
        { .base = APB2PERIPH_BASE + 0x3800, ... },
        { .base = APB2PERIPH_BASE + 0x3810, ... }
};

#define NELEMS(x)  (sizeof(x) / sizeof((x)[0]))

main() {
        for(uint8_t i = 0; i < NELEMS(usarts); i++) {
                ...
        }
}


[SiliconWizard]

There is nothing hacky about it - if you find all those casts "ugly", you can always define a macro to compare pointers that will hide the cast. If it feels less ugly.
Yep unfortunately you can't use pointers directly in switch (), something that is a bit quirky in C as you can compare them with == and != (consistency IMO should allow in switch/case the same as for the == operator, but that's not the case, so.)

Of course you can just use a series of if / else with which you can directly compare pointers.

Using an array as shown above also works and avoids the series of if / else as it'll do that in a loop. If the number of elements is known at compile time (also as in the above code), it should yield about the same compiled code as a switch or manual series of if/else.

[langwadt]

There is nothing hacky about it - if you find all those casts "ugly", you can always define a macro to compare pointers that will hide the cast. If it feels less ugly.
Yep unfortunately you can't use pointers directly in switch (), something that is a bit quirky in C as you can compare them with == and != (consistency IMO should allow in switch/case the same as for the == operator, but that's not the case, so.)

maybe it is a side effect the case statements have to be compile-time constants?

[IanB]

The difference of two pointers yields an integer, so maybe you could do something like the following?

Code: [Select]

switch (usart_ptr - USART_BASE)
{
    case USART1 - USART_BASE : /* do something */
    case USART2 - USART_BASE : /* etc. */
    /* etc. */
}

[SiliconWizard]

There is nothing hacky about it - if you find all those casts "ugly", you can always define a macro to compare pointers that will hide the cast. If it feels less ugly.
Yep unfortunately you can't use pointers directly in switch (), something that is a bit quirky in C as you can compare them with == and != (consistency IMO should allow in switch/case the same as for the == operator, but that's not the case, so.)

maybe it is a side effect the case statements have to be compile-time constants?

Yes - that's pretty much it. Original C probably implemented switch this way to have a more efficient switch/case than a corresponding series of if/else, with the primitive compilers at the time. A modern compiler would have no issue compiling a switch/case potentially more efficiently if the case values are known at compile time and otherwise implement it as a series of run time tests. But the limitation was kept as is.

[nctnico]

In some C code I'm writing I need to do something different depending on the pointer to a particular USART peripheral that is passed to the function.

Switch statements only work off integer values, so in order to use a switch statement I ended up casting the switched-on and case values as uintptr_t.

Code: [Select]

/* From an include file: */
#define PERIPH_BASE                             ((uint32_t)0x40000000)
#define APB2PERIPH_BASE                         (PERIPH_BASE + 0x10000)
#define USART1_BASE                             (APB2PERIPH_BASE + 0x3800)
#define USART1                                  ((USART_TypeDef *)USART1_BASE)
/* ...and similar for USART 2,3,4 */

typedef struct {
USART_TypeDef *uart;
/* etc... */
} uart_context_t;

void my_function(uart_context_t *ctx) {
/* etc... */
switch((uintptr_t)ctx->uart) {
case (uintptr_t)USART1: RCC->APB2PCENR |= RCC_USART1EN; break;
case (uintptr_t)USART2: RCC->APB1PCENR |= RCC_USART2EN; break;
case (uintptr_t)USART3: RCC->APB1PCENR |= RCC_USART3EN; break;
case (uintptr_t)USART4: RCC->APB1PCENR |= RCC_USART4EN; break;
}
/* etc... */
}

But this feels a bit hacky to me, a bit like I shouldn't be doing it.

Is there a better way?

Yes. I implement these kind of things using an index into an array of pointers (or better: an array with UART contexts to keep the internals of the UART driver constrained within the UART driver) WITH a check whether the index is valid.

[SiliconWizard]

Using an index instead of the pointers themselves indeed avoids having to do a series of tests.
That has an additional benefit of abstracting the reference to peripherals a bit, making porting potentially easier.

[TheCalligrapher]

The difference of two pointers yields an integer, so maybe you could do something like the following?

Code: [Select]

switch (usart_ptr - USART_BASE)
{
    case USART1 - USART_BASE : /* do something */
    case USART2 - USART_BASE : /* etc. */
    /* etc. */
}

Case labels are required to be integer constant expressions. Standard requirements do not allow pointer subtraction to be used in an integer constant expression, even if the pointers themselves qualify as address constants.

Specific compilers can allow more latitude in integer constant expressions, and I would expect this to be accepted by GCC or Clang as an extension, provided the pointers are defined the way they are defined in the OP's post (i.e. an integer constant expression converted to pointer type)

[golden_labels]

In C switch defines program structure, with case setting labels. And in C program structure can’t dynamically change at runtime. Consequently, cases are constant. Most importantly switch is not and never been a shorthand for if-else. It has different semantics. This is separate from switch in PHP or match in Python. There, regardless of any limitations the language may put on types and syntax, the semantics are more closely related to if-else.

In switch cases’ values are unordered and mutually exclusive. The branching is distinct and unambiguous not thanks to soft logic, as possible to construct using a sequence of if-else, but a hard guarantee. That’s reminiscent of a piecewise function in maths. That’s why a dynamic value can’t be used as a case label. It’s the same reasoning as with why we can’t dynamically set labels goto targets or rename functions at runtime.

So despite it’s tempting and may seem like making code clearer, this is not how switch works. In C it’s like a goto with the target label selected at runtime.

[IanB]

The limitation on case statement labels having to be, not just constants, but constant expressions, is slightly inconvenient.

But nevertheless, hardware addresses of peripherals do tend to be compile time constants. So if I do something like the following I should be within the compiler rules:

Code: [Select]

#define USART1     0x40013804

switch ((uint32_t)usart_ptr)
{
    case USART1: /* do something */
...
}


[langwadt]

The limitation on case statement labels having to be, not just constants, but constant expressions, is slightly inconvenient.

But nevertheless, hardware addresses of peripherals do tend to be compile time constants. So if I do something like the following I should be within the compiler rules:

Code: [Select]

#define USART1     0x40013804

switch ((uint32_t)usart_ptr)
{
    case USART1: /* do something */
...
}


but then you have the problem when you want to use USART1 as a pointer

OP could just use USART1_BASE in the case instead of USART1

[HwAoRrDk]

Hmm, not really sure I like the concept of an index into an array, due to it being zero-based. There is no USART0, so index 0 == USART1, etc. is a mental taxation I'd prefer to avoid. Of course I could create some defines against the indices, but the obvious names (i.e. "USART1") are already taken by the vendor peripheral header.

I guess I'm fortunate that the USART peripheral addresses are constant defines so that I can use them in a switch case statement.

It seems that outside of indexing into an array, you're always going to be doing some kind of pointer comparison, whether that's by absolute value or offset from a base. So I guess I might as well stick with casting to uintptr_t.

[gamalot]

I'm not familiar with the CH32V microcontrollers, but if you store the offset of peripheral clock enable register (APBxPCENR) and the bitmask (RCC_USARTxEN) for the UART in your context structure, you won't need a switch statement anymore.

[golden_labels]

The trouble comes from insisting on using switch. From attempts to forcefully squeeze an idea into an incompatible container. Just let go.

C’s switch could easily support pointers. It’s trivial: express it in terms of if-else and there is no problem for the compiler to follow this route. But in the process of changing semantics we’d lose what C’s switch currently offers.


I find storing a register pointer and the bitmask in a descriptor the most elegant and clean way. Yet, this may be a poor choice for a microcontroller. I don’t know this microcontroller or your constraints. However it’s often the case that controllers have dedicated instructions for setting/clearning bits in registers. If that’s the case, after the right execution path is selected, the entire operation is stored and executed in a single instruction, and all its data loaded behind the scenes. With the descriptor solution this isn’t going to happen: data has to be stored separately, also constructed if kept in SRAM, the code needs to actively load it and process, with construction, loading, and setting taking many additional instructions. In a heavily constrained environment this may be too costly in therms of both space and time. But I agree this is the cleanest option.

If you wish to have matching working as you conceived in your original example, just replace it with series of if-else. It would have to result in the same code anyway: comparing the pointer against each single option, in order, one by one. There is no loss.⁽¹⁾

If you follow the port identifiers idea, consider this kind of a code:

Code: [Select]

enum UartPort {uart1, uart2, uart3, uart4};

static PortType* portById(enum UartPort port) {
    switch (port) {
        case uart1: return pointer1;
        case uart2: return pointer2;
        case uart3: return pointer3;
        case uart4: return pointer4;
        default: abort();
    }
}
The advangage of this code is that, if port is known at compile time, code generated by any decent compiler will behave as if pointerN was used directly in code.⁽²⁾ That means: zero cost of invoking portById.

⁽¹⁾ Hypothetically a compiler might make a more optimal implementation, if e.g. addresses representing pointers form a sequence. But not only I don’t know of a compiler doing this now, this kind of hypothetical may as well work the other way (for if-else).
⁽²⁾ If the one you use doesn’t, then regarding performance we have a much bigger problem than that.

[SiliconWizard]

Yes agree to the above, which was already suggested by nctnico.

Apparently, the OP rejected the idea due to identifier collision ("can't use USARTx for the constants because they are already defined"). Just abstract the names for peripherals. And as we all know, especially in C where there is only a single namespace for all enum constants (or ditto of course if you use macros instead), avoiding short and relatively generic global identifiers is a good idea. I personally use a generous amount of prefixing (xxx_yyy) to avoid that. That reduces the probability of identifier clash to very low. Yes that's more characters to type, but that saves a lot of time along the way (having to refactor code later on to fix identifier clash is not time I consider well invested), and decent editors have autocompletion.

Many MCU vendor SDKs do not do that and themselves use very short and very generic identifiers, which is pretty nasty, especially when these are macros.

[bson]

Why not just add the RCC clock enable register and the enable bit as fields to the usart struct typedef?

[Siwastaja]

Why not just add the RCC clock enable register and the enable bit as fields to the usart struct typedef?

That you need to ask from ST. It's a significant silicon change 

But your question is not stupid. Many simple 8-bit microcontrollers only use enable bits at peripheral registers. And 32-bittiness/complexity is no excuse. nRF52 microcontrollers also have just single enable bits in peripheral registers - despite being 32-bit ARM and despite being low-power: they are fully capable of doing everything STM32 does, with half the number of enable bits, and with them distributed where they belong - in the peripheral config.

In the end, they are just flip-flops in memory-mapped address space. There is absolutely no physical reason why power control / clock control registers have to live in some weird separate system, except the desire for complexity and difficulty of programming, which is the core expertise of ST.

[bson]

Code: [Select]

typedef struct {
USART_TypeDef* const uart;
       volatile uint32_t* const rcc_ena;
       const uint32_t rcc_ena_mask;
/* etc... */
} uart_context_t;

void my_function(uart_context_t *ctx) {
/* etc... */
       *ctx->rcc_ena |= ctx->rcc_ena_mask;
/* etc... */
}
And if everything else in uart_context_t is const, relocate it to a pure section (like .rodata).

[langwadt]

Why not just add the RCC clock enable register and the enable bit as fields to the usart struct typedef?

That you need to ask from ST. It's a significant silicon change 

But your question is not stupid. Many simple 8-bit microcontrollers only use enable bits at peripheral registers. And 32-bittiness/complexity is no excuse. nRF52 microcontrollers also have just single enable bits in peripheral registers - despite being 32-bit ARM and despite being low-power: they are fully capable of doing everything STM32 does, with half the number of enable bits, and with them distributed where they belong - in the peripheral config.

In the end, they are just flip-flops in memory-mapped address space. There is absolutely no physical reason why power control / clock control registers have to live in some weird separate system, except the desire for complexity and difficulty of programming, which is the core expertise of ST.

Then you end up with registers in peripherals that has mix of bits that are always powered/clocked, and some that are sometimes powered/clocked
controlling separate modules: pwr/clk controlland peripheral and having to deal with being and able to write to a register that isn't powered not by ignoring it because some of the bit are need to power it up

[Siwastaja]

Then you end up with registers in peripherals that has mix of bits that are always powered/clocked, and some that are sometimes powered/clocked
controlling separate modules: pwr/clk controlland peripheral and having to deal with being and able to write to a register that isn't powered not by ignoring it because some of the bit are need to power it up

Being "in the peripheral" is a human abstraction, doesn't happen on real HW - logic synthesizer optimizes it from HDL, totally mixing up any code module boundaries (just like whole program optimization would do in the software world). For memory-mapped anything, something has to decode the address and then access the correct flip-flop, and that part has to be powered up, but it really does not matter what the address actually is, or where the flipflop is physically located - it can be a physical power control module if it needs to be that way for physical reasons, but the bit could still reside in the peripheral's address space, and documented in the manual at the peripheral, as "enable" bit.

And obviously the bits that control clocking/power need to be always powered - otherwise it's chicken-and-egg problem.

Proof is that the lowest-power designs, like nRF52, do not need separate control registers in a separate "block", so clearly it works. STM32 adds extra block, which is harder to use (having to write in two registers, described 1000 pages apart in the manual), and consumes more power.

This is totally a human complication. It's putting more stuff on top of existing designs. It is sometimes easier to add layers, components, complexity. This is what ST is very good at.

[SiliconWizard]

Most Cortex-M7 and more complex SoCs I've used had this separation between a clock controller and individual peripherals. It's actually obvious why. Peripherals are each completely separate IPs, sometimes (or often) coming from different IP vendors. All peripheral IPs have an 'enable', that's kind of standard and pretty obivous why. Due to how complex clock trees can be in those MCUs/SoCs, having a dedicated clock controller is also perfectly obvious. The two are effectively separate.

If you find that annoying, you should probably not try baremetal on any more complex SoC.