MSP430F5xxx CRC16 module

Thanks for the help me, but they not match :(

Test source code:

t[3] = {0x01, 0x02, 0x03};

unsigned short crc_ccitt_update (unsigned short crc, unsigned char data)
{
        int i;

        crc = crc ^ ((unsigned short)data << 8);
        for (i=0; i<8; i++)
        {
            if (crc & 0x8000)
                crc = (crc << 1) ^ 0x1021;
            else
                crc <<= 1;
        }

        return crc;
}

unsigned int SyGetCRC(unsigned char *buf, unsigned int sz, unsigned int init)
{
    unsigned int i;
    unsigned char ch;
   
    CRCINIRES = init;
    for (i = 0; i < sz; i++)
    {
        ch = *(buf + i);
        CRCDI = ch;
    }
   
    return CRCINIRES;
}

Main code:

CRC1 = SyGetCRC(t, 3, 0x00);
CRC2 = crc_ccitt_update (0x00, t[0]);
CRC2 = crc_ccitt_update (CRC2, t[1]);
CRC2 = crc_ccitt_update (CRC2, t[2]);

------------------------------------------------------------

Steps calculation:

CRC1 (in function SyGetCRC()): 0x1B98, 0xD0F4, 0xACE8

CRC2: 0x1021, 0x1373, 0x6131

I think, that CRC2 is right and hardware calculation is wrong.  May be I do something erroneously in SyGetCRC() hardware computation...

Hi,

Also from your CRC results it seems the compiler is converting your code in to 16-bit write in to CRCDI. So if you perform the following on your PC code you should get the results you got from MSP:

crc_res = crc_ccitt_update(0x0000,0x80);  // Bit reversed 0x01...

crc_res = crc_ccitt_update(crc_res,0x00); // Your 0x01 is being interpreted as 0x0001....

crc_res = crc_ccitt_update(crc_res,0x40);  // Bit reversed 0x02...

crc_res = crc_ccitt_update(crc_res,0x00); // Your 0x02 is being interpreted as 0x0002....

crc_res = crc_ccitt_update(crc_res,0xC0);  // Bit reversed 0x03..

crc_res = crc_ccitt_update(crc_res,0x00); // Your 0x03 is being interpreted as 0x0003....

Therefore you need to tell the compiler explicitly that you want byte access and not word access. In IAR you can use CRCDI_L instead of CRCDI and confirm in dis-assembly that it is being accessed with mov.b instruction.

Regards,

Praval

Yes, below code working correctly (CRC1 == CRC2)!

        CRCINIRES = 0x0000;
        CRCDI_L = 0x01;
        CRCDI_L = 0x02;
        CRCDI_L = 0x03;
        CRC1 = CRCINIRES;

        CRC2 = crc_ccitt_update(0x0000,0x80);  // Bit reversed 0x01...
        CRC2 = crc_ccitt_update(CRC2,0x40);  // Bit reversed 0x02...
        CRC2 = crc_ccitt_update(CRC2,0xC0);  // Bit reversed 0x03..

Thank you very much, Praval!

I think that it would be quite good if TI will created aplication note about CRC16 hardware features.

Hello

I have the same problem. I've read this topic carefully and check the solution on myself and unfortunately it isn’t work L! I don’t know how this is possible.

For example if I send on hardware CRC module 0x01, 0x02, 0x03 I have such as crc’s: 1B98, D0F4 , ACE8

If I use crc_ccitt_update routine, they I have copy from this topic,  with data 0x80 , 0x40 , 0xC0 I have results 9188 , 535C, EFDA . They are  ofcourse different. Can somebody tell my, where I make mistake or check results. Running at x5438.  This CRC its killning me…

I too had some problems with the hardware CRC module.

Previously, we used (and on most devices, we still use) the following algorithm:

unsigned short crc16(volatile unsigned char *sbuf,unsigned char len){
 unsigned short crc=0xFFFF;
 while(len){
  crc=(unsigned char)(crc >> 8) | (crc << 8);
  crc^=(unsigned char) *sbuf;
  crc^=(unsigned char)(crc & 0xff) >> 4;
  crc^=(crc << 8) << 4;
  crc^=((crc & 0xff) << 4) << 1;
  len--;
  sbuf++;
 }
 return crc;
}//crc16()

This gives a CCITT CRC16 compliant result.

When using the CRC16 module first, I also got erraneous results. Then I discovered, that the CRC16 registers are actually bit-reversed to the CCITT algorithm, while the bit-reversed registers which shoudl do the trick are not available on the non-A version of the 5438.

So I tested several variants to do the job.

The function which uses the bit-reversed registers (if there were any) would bring 410% of the software solution. Quite an achievement - if it were working. And only 26 bytes code.

The function that uses unoptimized C code for bit inversion was much, Much , MUCH slower. I dumped it immediately.

Then I did go for hand-optimized bit-inverting in assembly language, using two registers, shifting a bit out and in. After all, I got 146% speed compared to the software CCITT function (100%). Only 50% gain by using a dedicated hardware. Really not such a big step forward. And with 62 bytes size it wasn't that much smaller than the original 86 bytes for the software solution.

Last try was using a table for the bit inversion. I got 273% Speed (well, at least some noticeable speedup) but the size jumped to whopping 292 bytes, including the 256 bytes for the inversion table.

After all, the CRC16 module was quite a disappointment. If you're already plan to use the A version, then it is a good additional thing. But for us it would be the only reason to no stay with the 5438 and it doesn't make up for the significantly higher price of the A. I guess I have to find some additional uses for a CRC16 in our projects, so the low 46% speed gain will make up for the 3 days I wasted on the whole topic instead of keeping the plain software solution (which perhaps might be optimizeable on assembly level too)

To summarize: yes, it works, and it also delivers the same results as the CCITT algorithm, if you 1) initialize with 0xffff and 2) bit-reverse the bytes before putting them in to the CRC16 module.

Hello

Ok, yesterday I found solution. I was my mistake, like always. I compute CRC for 8-bit data but I used CRC module like for 16-bit. Now I thing solution can by made in to two ways :

First, using on MSP430 only CRCDI_L register, than reversing data and using crc_ccitt_update() function.

Secoud way, then I use, reversing data and calculating crc 2 times, with data than with zero :

unsigned short

CRC(unsigned short crc,unsigned char *data,unsigned char len) 

{

unsigned char i,mask,mask2,data2;

while(len--)

{

            data2=0;

            mask=0x01;

            mask2=0x80;

//reversing data

            for (i=0;i<8;i++) {

                        if (*data&mask) data2|=mask2;

                        mask<<=1;

                        mask2>>=1;

                        }

     crc=crc_ccitt_update(crc,data2);   

     crc=crc_ccitt_update(crc,0x00);   

  data++;

}

return crc;       

}

It works wery well and input crc doesn’t matter (works for 0x0000 or 0xFFFF)

 What I found interesting, CRCDI_L register work well on my “old” 5438 (from end ’08 , without rev A) and my old datasheet says nothing about that.

I wonder why you wonder about the CRCDI_L register. It is listed in the 'b' version of the family users guide (I don't have the 'a') from January 2009. And it was always in the 5438. The problem is that thew CRCDI register is actually a CRCDIRB and the so-called CRCDIRB register from the users guide which is actually the 'real' one, isn't there in the non-'A' processors.

Anyway, if you just need a reliable CRC16 and do not need to be compatible with other systems, the CRC module is just fine. YoOu'll get 'a' CRC, not 'the' CRC, but it has the same quality as the CCITT one (just with mirrored data bytes).

Some other thing: The bit-reversion algorithm you use is so slow, you can skip the reversion and do a software CRC calculation and be MUCH faster. You should really use it only for testing purposes. :)

Something most people don't know: if you calculate the CRC over a dataset with two appended zero bytes, then put the CRC into those zero bytes, the CRC calculated over the resulting data is always zero. This way you have your CRC effectively included into the CRC. It is, however, slightly slower than keeping the CRC extra and comparing it (after all, it adds two bytes to the calculation).

Hi all,

 Here is example code that works!!!. Matched software and hardware generated CRCs!. 

Thank you all!.

#include <msp430.h> 

unsigned int crc16_hw(unsigned char *buf, unsigned int sz)
{
    unsigned int i;
    unsigned char ch;

    CRCINIRES = 0xffff; // init. CRCINIRES
    for (i = 0; i < sz; i++)
    {
        ch = *(buf + i);
        CRCDI_L  = ch;
    }

    return CRCINIRES;
}

unsigned short crc16_sw(volatile unsigned char *sbuf,unsigned char len)
{
	unsigned short crc=0xFFFF;
	while(len)
	{
		crc=(unsigned char)(crc >> 8) | (crc << 8);
		crc^=(unsigned char) *sbuf;
		crc^=(unsigned char)(crc & 0xff) >> 4;
		crc^=(crc << 8) << 4;
		crc^=((crc & 0xff) << 4) << 1;
		len--;
		sbuf++;
	}

 return crc;
}

int main(void)
{
    WDTCTL = WDTPW | WDTHOLD;	// Stop watchdog timer

    unsigned char data[5] = {0x31, 0x32, 0x33, 0x34, 0x35};

    volatile unsigned short CRC_hw;
    volatile unsigned short CRC_sw;

    CRC_hw = crc16_hw(data, 5);

// Software crc needs bit reversed values!

    data[0] = 0x8C; // bit reversed 0x31
    data[1] = 0x4C; // bit reversed 0x32
    data[2] = 0xCC; // bit reversed 0x33
    data[3] = 0x2C; // bit reversed 0x34
    data[4] = 0xAC; // bit reversed 0x35

    CRC_sw = crc16_sw(data, 5);

	return 0;
}

Please let me know your comments if any!.

-Rahul