1

I am new to ARM LPC2148 micro controller and also new to StackOverflow. I just saw one piece of code in one of the evaluation boards. I am pasting as it is below.

Port pins P0.19 to P0.22 are mapped to D4 to D7 of LCD. The function below is used to send commands to LCD operated in 4 bit mode:

void LCD_Command(unsigned int data)     // This function is used to send LCD commands
{
 unsigned int temp=0;
 EN_LOW();                      // Set EN pin of LCD to to Low
 COMMAND_PORT();
 WRITE_DATA();

 temp=data;
 IO0PIN&=0xFF87FFFF;
 IO0PIN|=(temp & 0xF0) << 15;

 EN_HI();   // Give strobe by enabling and disabling En pin of LCD
 EN_LOW();

 temp=data & 0x0F;
 IO0PIN&=0xFF87FFFF;
 IO0PIN|=(temp) << 19;

 EN_HI();
 EN_LOW();
 while(Busy_Wait());
 Delay(10);
} 

My questions are:

  1. The variable "data" is already 32 bit wide. Is it efficient to shift the data in this way? Coder could have passed 32 bit data and then masked (&)/ORed (|). Or are there any other impacts?

  2. Do we save any memory in LPC21xx if we use unsigned char instead of unsigned int? Since registers are 32 bit wide, I am not sure whether internally any segmentation is done to save memory.

  3. Is there any way we can easily map 8 bit data to one of the 8 bit portions of 32 bit data? In the above code, shifting is done by hard coding (<<15 or <<19 etc). Can we avoid this hard coding and use some #defines to map the bits?

Vagish
  • 2,520
  • 19
  • 32
Mahesha Padyana
  • 431
  • 6
  • 22

3 Answers3

1
  1. Do we save any memory in LPC21xx if we use unsigned char instead of unsigned int?

Only when storing them into RAM, which this small function will not do once the optimizer is on. Note that using char types may introduce additional code to be generated to handle overflows correctly.

  1. [...] Can we avoid this hard coding and use some #defines to map the bits?

Easy:

#define LCD_SHIFT_BITS 19

void LCD_Command(unsigned int data)     // This function is used to send LCD commands
{
 unsigned int temp=0;
 EN_LOW();                      // Set EN pin of LCD to to Low
 COMMAND_PORT();
 WRITE_DATA();

 temp=data;
 IO0CLR = 0x0F << LCD_SHIFT_BITS;
 IO0SET = (temp & 0xF0) << (LCD_SHIFT_BITS - 4);

 EN_HI();   // Give strobe by enabling and disabling En pin of LCD
 EN_LOW();

 temp=data & 0x0F;
 IO0CLR = 0x0F << LCD_SHIFT_BITS;
 IO0SET = temp << LCD_SHIFT_BITS;

 EN_HI();
 EN_LOW();
 while(Busy_Wait());
 Delay(10);
} 

I also changed pin set and clear to be atomic.

Turbo J
  • 7,563
  • 1
  • 23
  • 43
0
  1. The variable "data" is already 32 bit wide. Is it efficient to shift the data in this way? Coder could have passed 32 bit data and then masked (&)/ORed (|). Or are there any other impacts?

  2. Do we save any memory in LPC21xx if we use unsigned char instead of unsigned int? Since registers are 32 bit wide, I am not sure whether internally any segmentation is done to save memory.

Since you are using a 32 bit MCU, reducing the variable sizes will not make the code any faster. It could possibly make it slower, even though you might possible also save a few bytes of RAM that way.

However, these are micro-optimizations that you shouldn't concern yourself about. Enable optimization and leave them to the compiler. If you for some reason unknown must micro-optimize your code then you could use uint_fast8_t instead. It is a type which is at least 8 bits and the compiler will pick the fastest possible type.

It is generally a sound idea to use 32 bit integers as much as possible on a 32 bit CPU, to avoid the numerous subtle bugs caused by the various complicated implicit type promotion rules in the C language. In embedded systems in particular, integer promotion and type balancing are notorious for causing many subtle bugs. (A MISRA-C checker can help protecting against that.)

  1. Is there any way we can easily map 8 bit data to one of the 8 bit portions of 32 bit data? In the above code, shifting is done by hard coding (<<15 or <<19 etc). Can we avoid this hard coding and use some #defines to map the bits?

Generally you should avoid "magic numbers" and such. Not for performance reasons, but for readability.

The easiest way to do this is to use the pre-made register map for the processor, if you got one with the compiler. If not, you'll have to #define the register manually:

#define REGISTER (*(volatile uint32_t*)0x12345678)
#define REGISTER_SOMETHING 0x00FF0000 // some part of the register

Then either define all the possible values such as

#define REGISTER_SOMETHING_X 0x00010000
#define REGISTER_SOMETHING_Y 0x00020000
...

REGISTER = REGISTER_SOMETHING & REGISTER_SOMETHING_X;
// or just:
REGISTER |= REGISTER_SOMETHING_X;

REGISTER = REGISTER_SOMETHING_X | REGISTER_SOMETHING_Y;
// and so on

Alternatively, if part of the register is variable:

#define REGISTER_SOMETHING_VAL(val) \
  ( REGISTER_SOMETHING & ((uint32_t)val << 16) )
...
REGISTER = REGISTER_SOMETHING_VAL(5);

There are many ways you could write such macros and the code using them. Focus on turning the calling code readable and without "magic numbers". For more complex stuff, consider using inline functions instead of function-like macros.

Also for embedded systems, consider whether it makes any difference if all register parts are written with one single access or not. In some cases, you might get critical bugs if you don't, depending on the nature of the specific register. You need to be particularly careful when clearing interrupt masks etc. It is good practice to always disassemble such code and see what machine code you ended up with.


General advise:

Always consider endianess, alignment and portability. You might not think that your code will never get ported, but portability might mean re-using your own code in other projects.

If you use structs/unions for any form of hardware or data transmission protocol mapping, you must use static_assert to ensure that there is no padding or other alignment tricks. Do not use struct bit-fields under any circumstances! They are bad for numerous reasons and cannot be used reliably in any form of program, least of all in an embedded microcontroller application.

Community
  • 1
  • 1
Lundin
  • 195,001
  • 40
  • 254
  • 396
-1

Three questions, many many programming styles.

This code is defιnιtely bad code. No atomic access... Do your self a favor and don't use it as a reference.

  1. The variable "data" is already 32 bit wide. Is it efficient ...

There is no other impact. The programmer just used an extra 4byte local variable inside the function.

  1. Do we save any memory in LPC21xx if we use unsigned char instead of unsigned int?

In general you can save memory only in RAM. Most of the linked scripts align data in 4 or 8 bytes. Of course you can use structs to bypass this both for RAM and Flash. For ex consider:

// ...
struct lala {
   unsigned int  a  :12;
   unsigned int  b  :20;
   long          c;
   unsigned char d;
};
const struct lala l1;   // l1 is const so it lives in Flash.
// Also l1.d is 8byte long ;)
// ...

This last one is bringing us to question 3.

  1. Is there any way we can easily map 8 bit data to one of the 8 bit portions of 32 bit data? ...

The NXP's LPC2000 is a little-endian CPU see here for details. Thats mean that you can create structures in a way that the members will fit the memory location you want to access. To accomplish that you have to place Low memory address first. For ex:

// file.h
// ...
#include <stdint.h>
typedef volatile union {
   struct {
      uint8_t  p0  :1;
      uint8_t  p1  :1;
      uint8_t  p2  :1;
      uint8_t  p3  :1;
      ...
      uint8_t  p30 :1;
      uint8_t  p31 :1;
   }pin;
   uint32_t port;
}port_io0clr_t;
// You have to check it make sure

// Now we can "put" it in memory.
#define REG_IO0CLR    ((port_io0clr_t *) 0xE002800C)
   //!< This is the memory address of IO0CLR in address space of LPC21xx

Now we can use the REG_IO0CLR pointer. For ex:

// file.c
// ...
int main (void) {
   // ...
   REG_IO0CLR->port = 0x0080;   // Clear pin P0.7
   // or even better
   REG_IO0CLR->pin.p4 = 1;      // Clear pin p0.4
   // ...

return 0;
}
hoo2
  • 515
  • 1
  • 3
  • 11
  • Your bitfield solution ` REG_IO0CLR->pin.p4 = 1;` would create a useless read-modify-write cycle instead of a single write. – Turbo J Mar 28 '15 at 18:43
  • In structure, are we guaranteed of specific members in specific bit position? Ex: I want the LED1 (Say for ex: P0.19) to be only in 19th position. Will compiler guarantee this order whatever I specify or if there a chance that compiler may change it sometimes!! Is there any way we can also specify the bit number to a particular member of the structure in 32 bit register? – Mahesha Padyana Mar 29 '15 at 05:46
  • @TurboJ Yes In most of the cases REG_IO0CLR->pin.lala will generate some extra code. It was just an example of how can simplify things so we don't have to use #defines. – hoo2 Mar 29 '15 at 12:38
  • @ShankariAmma We can. Actually for most of the cores, companies provide header files using the same technique with the difference they don't use bit-fields. For ex the struct below is from STM32F10x library // stm32f10x.h // ... typedef struct { __IO uint32_t CRL; __IO uint32_t CRH; __IO uint32_t IDR; __IO uint32_t ODR; __IO uint32_t BSRR; __IO uint32_t BRR; __IO uint32_t LCKR; } GPIO_TypeDef; #define GPIOA ((GPIO_TypeDef *) GPIOA_BASE) – hoo2 Mar 29 '15 at 12:56
  • Using bit fields is a _very bad_ idea. This code is completely "out there", relying on I don't know how many forms of undefined/unspecified/implementation-defined behavior. [Read this](http://stackoverflow.com/questions/6043483/why-bit-endianness-is-an-issue-in-bitfields). Using `#define` for example, is far superior to bit-fields, since it would turn the code reliable, bug-free and portable. – Lundin Mar 30 '15 at 11:08