Jump to content
43oh

Passing ports as parameters to functions


Recommended Posts

Hi, is it possible to pass ports around as parameters in functions? Reason I'm asking is because in TI's USB Keyboard application note source code (SLAA0514_SW), it has P2OUT defined as 

#define KSO0_PORT_2     ((uint16_t) &P2OUT)    /*! KSO0 Port */

which is stored in an array to be used later.

const USBKBD_Port_t USBKBD_DKS_DSO[KSO_PINS] = {
    { KSO0_PORT_2,  KSO0_2_0}, //...

I tried to use 

void goHigh(uint16_t *dir, uint16_t *out, unsigned int bit)

and passed in goHigh(P1DIR, P1OUT, BIT0) but CCS corrected me to use unsigned char instead. It compiled and downloaded fine but the code doesn't work. So can ports be passed around?

#include <msp430.h> 

void goHigh(unsigned char dir, unsigned char out, unsigned int pin) {
	dir |= pin;
	out |= pin;
}

int main(void) {
    WDTCTL = WDTPW | WDTHOLD;	// Stop watchdog timer
	
    //green LED
    P1DIR |= BIT6;
    P1OUT |= BIT6;
    
    for(; {
    	goHigh(P1DIR, P1OUT, BIT0); //red LED doesn't light up
    }
	return 0;
}

Link to post
Share on other sites

 

Try this:

void goHigh(volatile unsigned char * dir, volatile unsigned char * out, unsigned int pin) {
	*dir |= pin;
	*out |= pin;
}


goHigh(&P1DIR, &P1OUT, BIT1);

Thanks for the code it works beautifully.

 

I dont see why not, a port is just a pointer to a 16 bit number/ address in nemory just like any other "thing", or "setting" in the processor

Assuming my C understanding is up to par, (unsigned char *) is a 8-bit pointer. I tried to change "volatile unsigned char *" to "volatile uint16_t" and "volatile unsigned int *", which are 16-bit pointer but CCS complains in both cases that the parameter have type unsigned char *. Do you know why that is the case?

Link to post
Share on other sites

Thanks for the code it works beautifully.

 

Assuming my C understanding is up to par, (unsigned char *) is a 8-bit pointer. I tried to change "volatile unsigned char *" to "volatile uint16_t" and "volatile unsigned int *", which are 16-bit pointer but CCS complains in both cases that the parameter have type unsigned char *. Do you know why that is the case?

Pointers are 16 bits no matter what because the MSP430 uses a 16-bit address bus.

 

But the pointer "type" is instructive to the compiler about what size of data underlies that pointer. In the case of these SFRs (special function registers, like P1DIR or P2OUT) they are only 8-bit wide and any operations you perform on them need to be limited to 8 bit data (else you might inadvertently clobber or set a different SFR adjacent to that one).

 

Sent from my Galaxy Note II with Tapatalk

 

 

Link to post
Share on other sites

I've done so with C++ objects, but it keeps feeling a little clumsy, since each port is always a little different. For example, P2SEL defaults to 0xC0, while all other ports default to 0x00.

Since numbered ports (P1, P2, etc.) are 8 bits wide (lettered ports are 16 bits wide, like PA, PB, etc.), you could assume P1IN to be a volatile unsigned char. This means that to access the content by reference (required to actually edit it, instead of editing a copy of the value in it) you must take a pointer reference to it.

void toggleBit(unsigned char* pout, const char pin)
{
  *pout ^= pin;
}

unsigned char* my_port;
myport = &P1OUT;
toggleBit(my_port, BIT6); // toggle 1.6
toggleBit(&P2OUT, BIT0); // toggle 2.0
Link to post
Share on other sites

Thanks for pointing that out. No biggie then. Type-casting should cast 8-bit pointers to 16-bit pointers well enough.

Typecasting is in many cases a signal that your design is wrong. Is you can see in my examples, I don't do typecasting. Typecasting is useful when you need to "tunnel" a memory pointer from one library to another, with a predefined structure (eg: read a bitmap from flash as a series of bytes, and then interpret them as an array of R,G,B structs). But mostly (and firstly) you should consider why your compiler is complaining about type conflicts.

 

In C a pointer is just a pointer, the type it points to is just a convenience to help you to not accidentally pass the wrong pointer type to a function. The tricky part is void pointers, any pointer can be assigned a void pointer and vice versa. This means that whenever a void pointer comes into play, any type checking is bypassed.

int a[20];
int* p;
void* q;
char* r;
p = a; // GOOD: let int pointer point to array of int
q = p; // TRICKY: discard the pointer type by assigning to void pointer
r = q; // BAD: assign a void pointer (that used to point to integers) to a char pointer
r = p; // WRONG: type conflict; int* and char* are not compatible types

In C++ there are several type cast options, which are intentionally "ugly".

int a;
char b = static_cast<char>(a); // take the value of a and try to fit it in b
char c = dynamic_cast<char>(a); // same as above, but do run-time verification
char d = reinterpret_cast<char>(a); // don't convert anything, just dump the memory over

Too bad in the later additions to C++, the same form was used for templates, thus making the ugliness of type casts less visible.

int a = square_root<int>(64); // perform the square_root specialisation to int on the value 64

To come back to your comment: since numbered ports are 8 bit, make sure to use unsigned char or uint8_t and their pointer type everywhere. You might get away with using 16 bit types, but you'd better not try it!

void set_all_to_act_on_rising_edge(unsigned short* p_ies)
{
  *p_ies = 0;
}

P1IE |= BIT3; // P1.3 is an interrupt line
unsigned short* my_ies = (unsigned short*)&P1IES; // BAD: assign P1IES as if it were a 16 bit word pointer
set_all_to_act_on_rising_edge(my_ies);
//P1.3 never fires 

Why does this go bad in the last line? The cause is that P1IES is at memory location 0x24, while P1IE is at location 0x25. So when I write an all zero 16-bit integer to 0x24, it will overwrite 0x25 as well, thus clearing P1IE!

These kinds of errors are best avoided by not using casts, since casts almost always are root of bad design.

Link to post
Share on other sites

These kinds of errors are best avoided by not using casts, since casts almost always are root of bad design.

 

Thank you. That is comprehensive explanation. So if I want to get a function to handle both lettered ports and numeric ports, how can I do so without type-casting?

Link to post
Share on other sites

You can't. That is, you can't in C, you can in C++ (Energia is C++). This is done using function overloading. This is a fancy term to denote that you have two functions with an identical name, where the executed one is dependent on the parameters.

void pinToHigh(uint8_t* port, uint8_t pin)
{
  port &= pin;
}

void pinToHigh(uint16_t* port, uint16_t pin)
{
  port &= pin;
}

uint8_t* my_small_port_out = &P1OUT;
uint16_t* my_big_port_out = &PAOUT;
pinToHigh(my_small_port_out, BIT1);
pinToHigh(my_big_port_out, BIT1);

Because you pass either a uint8_t* or a uint16_t* as the first parameter, the compiler can deduce which of the two you want to execute. You do have to describe the both explicitly, though through templates you could describe only one. The drawback of templates is that it's harder to control which template parameter was passed, so it can break more easy.

Link to post
Share on other sites

Looking again at the USB Keyboard example from TI, I found that they cast 16-bit address to 8-bit to make it work as a general function:

1. Define a port address as a 16-bit integer x

2. Cast that 16-bit integer x as an 8-bit integer pointer

3. Use pointer to change port values

 

For example:

#define PORTJ_OUT ((uint16_t) &PJOUT)

void portJPinToHigh(uint8_t pin) {
    uint8_t *port_out = (uint8_t *) PORT_OUT;
    *port_out |= pin;
}

I've tried this with a P4OUT and PJOUT, and they both works fine. Though I'm not sure yet how it works.

Link to post
Share on other sites

I think it is tricky (to be polite):

  • Assumed is that a pointer is a 16-bit integer
  • Assumed is that writing to the base address (8-bit reference) of PORTJ does indeed do what it's supposed to do

Since PJOUT does have a type (either uint8_t or uint16_t probably) the type of the constant reference should be a pointer to that type, not an integer; saying that it is an integer promises that you can write values to it, while it is actually a pointer!

//PORTJ_OUT is defined as being of the type uint16_t, so we can treat it as a uint16_t
uint16_t my_var;
myvar = 20;
PORTJ_OUT = 20; // the compiler won't complain; you may write to a uint16_t. In reality you're trying to write to a constant pointer to PJOUT!

Then it is assumed that this pointer does indeed fit in a 16-bit integer, which is not good practice. When using an MSP430 with a CPU peripheral, it does hold. When your MSP430 does have a CPUX it may not work as expected, since pointers are actually 20-bit integers.

 

Then this 16-bit integer is casted (totally unnecessarily) to a uint8_t*; a pointer to a uint8_t. This cast is a signal that there is a design error; in this case that the type of PORTJ_OUT was defined incorrectly.

 

Better designs that should work identical are:

void portJPinToHigh(uint8_t pin) {
    PJOUT |= pin; // why use pointery magic when this function can only write to a single location?
}

or

uint8_t* const PortJ_Out = &PJOUT; // using a constant instead of a define will enable type checking, plus we use the right type

void portJPinToHigh(uint8_t pin) {
    *PortJ_Out |= pin;
}
Link to post
Share on other sites

 

Then this 16-bit integer is casted (totally unnecessarily) to a uint8_t*; a pointer to a uint8_t. This cast is a signal that there is a design error; in this case that the type of PORTJ_OUT was defined incorrectly.

uint8_t* const PortJ_Out = &PJOUT; // using a constant instead of a define will enable type checking, plus we use the right type

void portJPinToHigh(uint8_t pin) {
    *PortJ_Out |= pin;
}

 

Thanks for pointing that out!

Link to post
Share on other sites

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Paste as plain text instead

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

×
×
  • Create New...