Project

General

Profile

Review Request #12843

Factor out common code

Added by Richard Neswold over 4 years ago. Updated about 4 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Start date:
06/02/2016
Due date:
% Done:

0%

Estimated time:
Duration:

Description

In this function and several following functions, there is duplicated code used in each case of a switch statement. I'd recommend creating a function that returns the correct GPIO pointer. The functions then collapse into one copy of the code. For instance, with this function:

static LPC_GPIO_TypeDef* gpioGetPtr(uint32_t port)
{
    static LPC_GPIO_TypeDef junk;

    switch (port) {
     case MCU_P0:
        return LPC_GPIO0;

     case MCU_P1:
        return LPC_GPIO1;

     case MCU_P2:
        return LPC_GPIO2;

     case MCU_P3:
        return LPC_GPIO3;

     default:
        return &junk;
    }
}

most of the following functions turn into one-liners:

void gpioIntEnable( uint32_t portNum, uint32_t bitPosi )
{
    gpioGetPtr(portNum)->IE |= 0x1 << bitPosi;
}

The function to which this comment is attached is a little more complicated. But it's still an algorithm copied four times. It can be reduced to1:

void gpioSetInterrupt(uint32_t portNum, uint32_t bitPosi, uint32_t sense,
                      uint32_t single, uint32_t event)
{
    LPC_GPIO_TypeDef* const ptr = gpioGetPtr(portNum);
    uint8_t const mask = 1 << bitPosi;

    if (sense == 0)
    {
        ptr->IS &= ~mask;
        if (single == 0)
            ptr->IBE &= ~mask;
        else
            ptr->IBE |= mask;
    }
    else
        ptr->IS |= mask;

    if (event == 0)
        ptr->IEV &= ~mask;
    else
        ptr->IEV |= mask;
}

1 I calculated the mask once so it wasn't computed it over and over. Since I can't find the definition of LPC_GPIO_TypeDef anywhere, I made mask a uint8_t.

History

#1 Updated by Richard Neswold over 4 years ago

The function returning the pointer to the hardware should probably be defined as

static volatile LPC_GPIO_TypeDef* gpioGetPtr(uint32_t port)
{

This says that the returned pointer points to volatile data (of the form of LPC_GPIO_TypeDef.) The pointer itself isn't volatile, but the data it points to is.

#2 Updated by Gregory Giese about 4 years ago

  • Status changed from Assigned to Closed

Rich is correct. I will factor out the common code before creating the Release code.



Also available in: Atom PDF