We are running a survey to help us improve the experience for all of our members. If you see the survey appear, please take the time to tell us about your experience if you can.
Hi All,
TARGET I am using C51 tools to compile a simple app for an ST uPSD33xx.
OUTLINE The problem I am experiencing maifests itself as an incorrect value in a pointer passed as an argument to a reentrant function.
DETAILS OK, I am writing a serial driver. Each channel of the serial driver has a struct to hold pointer to a data buffer, number of bytes, etc, etc. Each struct is static in XDATA as is the data buffer for each channel:
typedef struct serial_data_buffer_t { int8 xdata *pBuffer; uint16 read; uint16 write; uint16 length; uint16 bytesInBuffer; #ifdef SERIAL_HW_FLOW_CONTROL uint16 lowWater; uint16 highWater; #endif } SERIAL_DATA_BUFFER_T; typedef struct serial_channel_t { uint8 channelNumber; int32 baudRate; bool flowControlActive; bool txRunning; SERIAL_DATA_BUFFER_T txBuffer; SERIAL_DATA_BUFFER_T rxBuffer; } SERIAL_CHANNEL_T; #ifdef USE_SERIAL_PORT1 static int8 xdata serial1TxDataBuffer[SERIAL_PORT1_TX_BUFFER_SIZE]; static int8 xdata serial1RxDataBuffer[SERIAL_PORT1_RX_BUFFER_SIZE]; static SERIAL_CHANNEL_T xdata serialChannel1; #endif /* #ifdef USE_SERIAL_PORT1 */ #if (defined USE_SERIAL_PORT0) || (defined USE_SERIAL_PORT1) static void recieveByte (SERIAL_CHANNEL_T *pChannel) reentrant; static void sendByte (SERIAL_CHANNEL_T *pChannel) reentrant; static uint16 _writeSerial (char* buffer, uint16 numBytes, SERIAL_CHANNEL_T *pChannel); #endif /* #if (defined USE_SERIAL_PORT0) || defined (USE_SERIAL_PORT1) */
The pBuffer pointer is initialised at run time using straight forward assignment:
serialChannel1.txBuffer.pBuffer = (int8 xdata*) &serial1TxDataBuffer[0];
When a client wishes to transmit some data the following call chain takes place:
writeSerial1 ("Hello World!\n", strlen ("Hello World!\n"));
calls
bytesSent = _writeSerial (buffer, numBytes, &serialChannel1);
sendByte (pChannel);
The routine sendByte is declared reentrant as it is called from background (as just described) but also from the ISR (to contiune the transmission of the contents of the buffer).
The problem (first) occurs when sendByte is called from _writeSerial. The pChannel argument passed into sendByte is OK prior to the call (as viewed with the debugger), but once inside sendByte the pChannel argument contains a crazy value.
What could be causing this problem? Is there some fundamental problem or rule I am breaking here to cause this problem?
Any help appreciated
Cheers
Andy
Andy,
Let me just get this one bit of advice out of the way before someone else does in a less gentle manner: Making function calls from ISRs is generally a bad idea (ie. calling sendByte from within the ISR as well as main code).
Also, what do you mean by pChannel having a "crazy value?" What is the value before the call and then inside the function? Remember that when you use a function declaration like:
static uint16 _writeSerial (char* buffer, uint16 numBytes, SERIAL_CHANNEL_T *pChannel);
both buffer and pChannel will be 3-byte values. This is because you've made them "generic pointers" by not explicitly specifying which memory area they reside in. The first (highest-order) byte of the pointer will contain the memory area, while the lower two bytes should be what you expect in a "normal" pointer.
Hi Jay,
Regarding the functional calling from background and ISR - what, specifically, is the problem(s) with this? I have seen nothing in the Keil documentation to suggest that doing so in a 'bad idea'. In fact the tool chain supports directives to do just this (NOAREGS/ARGES, reentrant, etc - section 8.4.2.5 of the C51 Primer). Are you referring to the run-time overhead of making such a call?
So if I have 2 serial ports, so 2 ISRs, and the serial driver is based on a ringbuffer, surely you either have to make a function call to the common ringbuffer code or duplicate the code in each ISR.
Do you use a different strategy for tackling such a situatuon? Does this come down to a 'code size vs execution speed' trade-off or are you concious of more fundamental issues with this type of basic design problem?
Regarding the 'crazy value' is it either 0x0000 or 0xFFFF and the data being pointed to is also not correct - all zeros - so something is definitley not 100%. What common problems are there regarding passing generic/memory-specific pointers to functions? I would have thought that this is a bread-n-butter (basic!) activity. Incidentally, making the pointers memory-specific made no difference, but I take on-board the benefits of using this type of pointer regarding code size and execution speed. Thanks.
I have moved the code from sendByte to 'inline' and all is well, but it does not explain what was going wrong (and result is some duplication of code!).
As you say, it's an efficiency concern.
The 8051 architecture is poorly suited to maintaining a stack. It just doesn't have many pointer registers or good indexed addressing modes to take advantage of them if it did. So, the Keil compiler does some fairly clever compile-time analysis to assign parameters and locals to fixed memory locations. The code generator can then generate direct access to the data. The compiler and linker analyze the call tree to know which functions cannot be active at the same time, and thus which memory can be reused in different functions.
For this purpose, every context is a different call tree. main() has one call tree, and each ISR has a different one. A function shared between main and an ISR, or two ISRs, can't participate in the overlay scheme, because you might be in both copies of the function at once.
To avoid this problem, you need to make the functions reentrant. As you note, there are special Keil keywords just for this. However, this means that the code generator starts putting all those parametes and locals on a "software stack", maintaining a stack pointer and dragging the data back and forth through the DPTR. The tools make the C pretty painless, but you execute many (many) more instructions and generate larger code for the convenience.
Some people thus prefer to duplicate functions for ISRs. Usually the idea is to make the ISR faster, but sometimes it might even be smaller code once you eliminate all the stack simulation overhead.
It's not always a bad idea to make functions reentrant and share them, but true reentrancy is expensive enough on an 8051 that it's worth thinking about.
As Drew has already pointed out, my basic concern is with the cost in terms of execution speed. My own tendencies lead toward duplicating the code in the ISRs in order to avoid reentrant function calls (and often to just avoid function calls at all). If the bit of code is large enough that duplicating it in the ISRs is difficult in terms of code size, then that's probably an indication that it doesn't belong in the ISR itself, but in main code. Those things notwithstanding, it's not a hard-and-fast rule, but just a bit of hard-earned advice I thought I'd pass along.
Regarding the 'crazy value' is it either 0x0000 or 0xFFFF and the data being pointed to is also not correct - all zeros - so something is definitley not 100%. What common problems are there regarding passing generic/memory-specific pointers to functions?
This situation is indeed strange. Unless you're missing something very obvious in the code (which is unlikely in what you've posted), you may be running up against the wall I call the "real-world." The fact that the call to a reentrant function isn't going well leads me to wonder about the state of the stack. When a function is tagged as reentrant, the compiler does not generate code to pass parameters in registers, but instead passes them on the stack. What is the value of the stack pointer when you're in the bad function call? One way to quickly get a feel for your stack usage is to initialize it all to some known value (like ascii 'X') at startup, then look at it when your program is running. If there are no 'X's left, you know you're at least perilously close to a stack overflow. Give that a try and let us know the results.
-Jay Daniel
Just one more clarification: The stack used for reentrant functions is distinct from the "actual" 8051 stack. Make sure you're checking the right stack status based on the memory model you're using. For details, take a look at reentrant functions in the manual.
Thanks for the advice.
Regards