# Function not working probably like expected, using TM4C123 MCU

I am trying to generate a Clock synchronized with data, where the frequency of the clock should be twice the frequency of the data. I wrote a code for this purpose and it works fine for 32bits of data, anything more than 32 it will generate a bunch of 1's. Here is what I got for 64bit data=(0XAAAAAAAAAAAAAAAA):

Here is the code I wrote:

  void DATA(unsigned long  int number, int iteration){   //number: data to be send serially
// iteration: number of bits to send
unsigned long  int x=0;         //An integer to hold each bit from the data
char even = (char)0x1;
TIMER1->ICR |= (1<<0);
for(int y=0; y<=iteration;) {
while((TIMER1->RIS & 0x00000001) != 1){}   //wait until Timer1 times out
TIMER1->ICR |= (1<<0);     //Reset Timer1 flag
odd ^= (char)0x1;
GPIOC->DATA ^= (1<<7);     //Toggle the clock at PC7
if (odd) continue;        // data change every even cycle
x = number & (0x01UL << y);      //Get each bit individually
if(x==(0x01UL<<y))           //If bit is 1
GPIOC->DATA |= (1<<6); //Make C6 High
else if(x==0X0)            //If bit is 0
GPIOC->DATA &= ~(1<<6);//Make C6 Low
y++;
}
}

int main(){
DATA(0XAAAAAAAAAAAAAAAA,64);//
}


Note: I have tried to make the number in my function to be long long int, and that didn't solve the problem too. Is there any explanation for getting those 1's after the first 32 bits ?

• Have you stepped through the code in the debugger? – kkrambo Sep 4 '16 at 22:23
• @kkrambo Yes, I did. The MCU is reading 1's in the last 32 bit's, I don't know why. – Abdelrahman Elshafiey Sep 4 '16 at 22:31
• Why aren't you using explicitly sized types such as uint64_t? – user_1818839 Sep 5 '16 at 6:16
• Yeah, start by getting rid of int and long in favour for sane types from stdint.h. Then ensure that all integer literals have a large enough type. Then study implicit type promotion: Google "the usual arithmetic conversions". Then adopt a consistent indention and brace style. Then stop writing really obscure nonsense like for(int y=0; y<=iteration;)... y++.Somewhere along the way, the bug will likely disappear. As this code currently stands, it looks quite unsalvagable. – Lundin Sep 5 '16 at 6:58
• an unsigned long int in your compiler seems to be only 32 bits. So when the code tries to reference a 33 bit, it is referencing trash. suggest using a parameter of type: uint64_t Then you could go to 64 for iteration. However, if you want to be able to use data of any length then use: char* number then index through the data 8 bits at a time. Note: this may result in the need of a third parameter, indicating how many characters (or bits) are in the data. – user3629249 Sep 6 '16 at 20:36

Just for reference, here's how I would code your function:

// number: data to be send serially
// iteration: number of bits to send
void DATA (unsigned long int number, int iteration)
{
while (iteration--) {
TIMER1->ICR |= (1<<0);       //Reset Timer1 flag
while (!(TIMER1->RIS & 0x01)){} //wait until Timer1 times out
GPIOC->DATA |= (1<<7);       //Toggle the clock at PC7 high

if (number & 0x01)           //If bit is 1
GPIOC->DATA |= (1<<6);     //Make C6 High
else                         //If bit is 0
GPIOC->DATA &= ~(1<<6);    //Make C6 Low
number >>= 1;

TIMER1->ICR |= (1<<0);       //Reset Timer1 flag
while (!(TIMER1->RIS & 0x01)){} //wait until Timer1 times out
GPIOC->DATA &= ~(1<<7);      //Toggle the clock at PC7 low
}
}


Note that all of your internal variables are completely unnecessary. I've also unrolled the loop to make the polarity of the clock edges explicit.

EDIT: In order to be able to send arbitrary amounts of data, a function that accepts an array of bytes would probably be more useful:

// p: pointer to data to be sent serially
// n: number of bytes to send
void data_str (unsigned char *p, int n)
{
while (n--) {
char c = *p++;
int i;

for (i=0; i<8; ++i) {
TIMER1->ICR |= (1<<0);       //Reset Timer1 flag
while (!(TIMER1->RIS & 0x01)){} //wait until Timer1 times out
GPIOC->DATA |= (1<<7);       //Toggle the clock at PC7 high

if (c & 0x01)                //If bit is 1
GPIOC->DATA |= (1<<6);     //Make C6 High
else                         //If bit is 0
GPIOC->DATA &= ~(1<<6);    //Make C6 Low
c >>= 1;

TIMER1->ICR |= (1<<0);       //Reset Timer1 flag
while (!(TIMER1->RIS & 0x01)){} //wait until Timer1 times out
GPIOC->DATA &= ~(1<<7);      //Toggle the clock at PC7 low
}
}
}

// Send 128 bits (16 bytes) of data.
data_str ("\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA\xAA", 16);


Now we need two temporary variables: c holds the data so that we don't actually modify the caller's array, and i is used as a counter for the inner loop. Note that the bytes are sent left-to-right, but each byte is sent LSB-first.

• Yes, that is definitely better. – Abdelrahman Elshafiey Sep 5 '16 at 23:03
• Sorry, I messed up the data phasing relative to the clock. See the edit above for a corrected version. – Dave Tweed Sep 5 '16 at 23:21
• Is there a data type for 128 bits ? – Abdelrahman Elshafiey Sep 6 '16 at 3:55
• Read your compiler documentation. Some support a long long type. However, a more general solution would be to rewrite the function to accept an array of byes of arbitrary length. – Dave Tweed Sep 6 '16 at 10:36

I believe the problem is in the line

X = number & (0x01 << y);


The problem is that the result of 0x01 << y is an integer because 0x01 fits in one. Change this to read

X = number & (0x01UL << y);


You should also change every long to an unsigned long to avoid problems with 64-bit values.

• Ok, I will try that. – Abdelrahman Elshafiey Sep 4 '16 at 21:42
• I am still getting the same problem. – Abdelrahman Elshafiey Sep 4 '16 at 21:46
• You need to apply the same fix to the if(x==(0X1<<y)) line. Also, it isn't clear how this is working in the first place, becausey is counting iterations (half cycles of the clock), not bits. – Dave Tweed Sep 4 '16 at 22:16
• The y is only incrementing every even cycle, which is the number of bits I need. The y is also shifting a '1' to the left every even cycle, to the next bit I need. Sorry if the code is not clear, I know it's not a readable one. When I changed 'if(x==(0X1<<y))' to 'if(x==(0x01UL<<y))' still not working. – Abdelrahman Elshafiey Sep 4 '16 at 22:41
• @DaveTweed: you are right about changing the type of 0x01, it should be uint64_t. Thanks for helping – Abdelrahman Elshafiey Sep 5 '16 at 19:40