AVR Freaks

The reason for the 1284 is 5V compatibility and also the 16K SRAM that it has.

Good thoughts JC - I'm wanting to keep it simple with code I already have for a bootloader, etc., so that was why I chose the 1284p.  I can't argue with your direction though and may try it out if i can't find something suitable!

No errors detected so far, but I just implemented the detection and retry a couple nights ago.  I suppose I should throw a 6 feet DB25M/DB25F cable in the mix and see if any show up.

On chip memory would be fine, I'm currently doing it as a post operation to receiving the data.

I may try integrating it into the receiving loop, but then I'll have to either check to see if the byte being received or sent is past the first 4 bytes or not which contain the size of the packet (2b) and the crc16 (2b).  Either that or unwind that into two loops instead of just one which I'd rather not do.

I'm going to start with testing the performance of table based crc16 and if that is still slower than I want, maybe look at crc8.

How does one make a lookup table for this:

uint16_t _crc16_update(uint16_t crc, uint8_t a)
{
  uint8_t i;

  crc ^= a;
  for (i = 0; i < 8; ++i)
    {
      if (crc & 1)
        crc = (uint16_t)((crc >> 1) ^ 0xA001);
      else crc = (uint16_t)(crc >> 1);
    }
  return crc;
}

EDIT- never mind I'll see if I can figure it out.

I agree - I like collecting old equipment like the original PC AT/5170 or the Compaq Deskpro/M 486.

I figured out the make table function so I've got that to try and see what type of performance difference it makes!

sparrow2 are you saying the lookup should have made much more difference?  I thought so too.

gcc is lready inlining crc16t_update() - I tried to inline it with no difference.

const uint16_t /*__flash*/ crc16t_lookup[256] =
{
0x0000, 0xc0c1, 0xc181, 0x0140, 0xc301, 0x03c0, 0x0280, 0xc241, 0xc601, 0x06c0, 0x0780, 0xc741, 0x0500, 0xc5c1, 0xc481, 0x0440, 
0xcc01, 0x0cc0, 0x0d80, 0xcd41, 0x0f00, 0xcfc1, 0xce81, 0x0e40, 0x0a00, 0xcac1, 0xcb81, 0x0b40, 0xc901, 0x09c0, 0x0880, 0xc841, 
0xd801, 0x18c0, 0x1980, 0xd941, 0x1b00, 0xdbc1, 0xda81, 0x1a40, 0x1e00, 0xdec1, 0xdf81, 0x1f40, 0xdd01, 0x1dc0, 0x1c80, 0xdc41, 
0x1400, 0xd4c1, 0xd581, 0x1540, 0xd701, 0x17c0, 0x1680, 0xd641, 0xd201, 0x12c0, 0x1380, 0xd341, 0x1100, 0xd1c1, 0xd081, 0x1040, 
0xf001, 0x30c0, 0x3180, 0xf141, 0x3300, 0xf3c1, 0xf281, 0x3240, 0x3600, 0xf6c1, 0xf781, 0x3740, 0xf501, 0x35c0, 0x3480, 0xf441, 
0x3c00, 0xfcc1, 0xfd81, 0x3d40, 0xff01, 0x3fc0, 0x3e80, 0xfe41, 0xfa01, 0x3ac0, 0x3b80, 0xfb41, 0x3900, 0xf9c1, 0xf881, 0x3840, 
0x2800, 0xe8c1, 0xe981, 0x2940, 0xeb01, 0x2bc0, 0x2a80, 0xea41, 0xee01, 0x2ec0, 0x2f80, 0xef41, 0x2d00, 0xedc1, 0xec81, 0x2c40, 
0xe401, 0x24c0, 0x2580, 0xe541, 0x2700, 0xe7c1, 0xe681, 0x2640, 0x2200, 0xe2c1, 0xe381, 0x2340, 0xe101, 0x21c0, 0x2080, 0xe041, 
0xa001, 0x60c0, 0x6180, 0xa141, 0x6300, 0xa3c1, 0xa281, 0x6240, 0x6600, 0xa6c1, 0xa781, 0x6740, 0xa501, 0x65c0, 0x6480, 0xa441, 
0x6c00, 0xacc1, 0xad81, 0x6d40, 0xaf01, 0x6fc0, 0x6e80, 0xae41, 0xaa01, 0x6ac0, 0x6b80, 0xab41, 0x6900, 0xa9c1, 0xa881, 0x6840, 
0x7800, 0xb8c1, 0xb981, 0x7940, 0xbb01, 0x7bc0, 0x7a80, 0xba41, 0xbe01, 0x7ec0, 0x7f80, 0xbf41, 0x7d00, 0xbdc1, 0xbc81, 0x7c40, 
0xb401, 0x74c0, 0x7580, 0xb541, 0x7700, 0xb7c1, 0xb681, 0x7640, 0x7200, 0xb2c1, 0xb381, 0x7340, 0xb101, 0x71c0, 0x7080, 0xb041, 
0x5000, 0x90c1, 0x9181, 0x5140, 0x9301, 0x53c0, 0x5280, 0x9241, 0x9601, 0x56c0, 0x5780, 0x9741, 0x5500, 0x95c1, 0x9481, 0x5440, 
0x9c01, 0x5cc0, 0x5d80, 0x9d41, 0x5f00, 0x9fc1, 0x9e81, 0x5e40, 0x5a00, 0x9ac1, 0x9b81, 0x5b40, 0x9901, 0x59c0, 0x5880, 0x9841, 
0x8801, 0x48c0, 0x4980, 0x8941, 0x4b00, 0x8bc1, 0x8a81, 0x4a40, 0x4e00, 0x8ec1, 0x8f81, 0x4f40, 0x8d01, 0x4dc0, 0x4c80, 0x8c41, 
0x4400, 0x84c1, 0x8581, 0x4540, 0x8701, 0x47c0, 0x4680, 0x8641, 0x8201, 0x42c0, 0x4380, 0x8341, 0x4100, 0x81c1, 0x8081, 0x4040, 
};

/*void crc16t_make()
{
  uint16_t i1, result;
  uint8_t c1;

  for (i1=0; i1<256;i1++)
    {
      result=i1;
      for (c1=0; c1<8; c1++)
        if (result & 1)
          result = (uint16_t)((result >> 1) ^ 0xA001);
        else result = (uint16_t)(result >> 1);
      crc16t_lookup[i1]=result;
    }
}*/

uint16_t crc16t_update(uint16_t crc, uint8_t a)
{
  return (uint16_t)((crc>>8)^crc16t_lookup[(uint8_t)(crc^a)]);
}

uint16_t crc_memory(uint8_t *AData, uint16_t ASize)
  {
    uint16_t ui1, crc;

    crc=0xffff;

    for (ui1=0; ui1<ASize; ui1++)
      //crc=_crc16_update(crc,AData[ui1]);
      crc=crc16t_update(crc,AData[ui1]);

    return ~crc;
  }

...

    //does the crc match
    //debug
    PORTA&=~_BV(PA_S3_PIN);
    if (crc_memory(buffer+4, ui1)!=*(uint16_t*)&buffer[2])
      {
        if (debuglevel>=1)
          uart_puts("crc16 does not match\r\n");
        pc_send_error=1;
        receive_available=0;
        goto avr_ready;
      }
    //debug
    PORTA|=_BV(PA_S3_PIN);

I'll post the LSS for the traditional non table version in a little bit if that helps to see why.

That is a good idea clawson - I'll give that a try.

Here is the LSS for the built in non-table function:

uint16_t crc_memory(uint8_t *AData, uint16_t ASize)
  {
 228:	68 0f       	add	r22, r24
 22a:	79 1f       	adc	r23, r25
    uint16_t ui1, crc;

    crc=0xffff;
 22c:	2f ef       	ldi	r18, 0xFF	; 255
 22e:	3f ef       	ldi	r19, 0xFF	; 255

    for (ui1=0; ui1<ASize; ui1++)
 230:	86 17       	cp	r24, r22
 232:	97 07       	cpc	r25, r23
 234:	d9 f0       	breq	.+54     	; 0x26c <crc_memory+0x44>
      crc=_crc16_update(crc,AData[ui1]);
 236:	fc 01       	movw	r30, r24
 238:	41 91       	ld	r20, Z+
 23a:	cf 01       	movw	r24, r30
_crc16_update(uint16_t __crc, uint8_t __data)
{
	uint8_t __tmp;
	uint16_t __ret;

	__asm__ __volatile__ (
 23c:	24 27       	eor	r18, r20
 23e:	42 2f       	mov	r20, r18
 240:	42 95       	swap	r20
 242:	42 27       	eor	r20, r18
 244:	04 2e       	mov	r0, r20
 246:	46 95       	lsr	r20
 248:	46 95       	lsr	r20
 24a:	40 25       	eor	r20, r0
 24c:	04 2e       	mov	r0, r20
 24e:	46 95       	lsr	r20
 250:	40 25       	eor	r20, r0
 252:	47 70       	andi	r20, 0x07	; 7
 254:	02 2e       	mov	r0, r18
 256:	23 2f       	mov	r18, r19
 258:	46 95       	lsr	r20
 25a:	07 94       	ror	r0
 25c:	47 95       	ror	r20
 25e:	30 2d       	mov	r19, r0
 260:	24 27       	eor	r18, r20
 262:	06 94       	lsr	r0
 264:	47 95       	ror	r20
 266:	30 25       	eor	r19, r0
 268:	24 27       	eor	r18, r20
 26a:	e2 cf       	rjmp	.-60     	; 0x230 <crc_memory+0x8>
      //crc=crc16t_update(crc,AData[ui1]);

    return ~crc;
  }
 26c:	c9 01       	movw	r24, r18
 26e:	80 95       	com	r24
 270:	90 95       	com	r25
 272:	08 95       	ret

Could

_crc16_update(uint16_t __crc, uint8_t __data)

already be optimized to not have a loop of 8 - I don't even see any branching in it...

There are no interrupts with this project - part of the parallel is supporting EPP which is has assisted handshaking done in hardware - the AVR has to respond quickly without the risk of being in an interrupt somewhere.  Essentially the AVR listens for a packet, validates it, processes it, produces a response (also with crc16), and then the PC reads the response back.

I had some good ideas about this project last night and got it about half way implemented.  Testing with the crc16 being calculated in realtime with the transfer gave a speed up of about 14% over doing the two as separate processes.

Instead of just a read or write mode, I've implemented 5 modes which allow me to do all the crc calculations during the transfer now.  For the PC sending to the AVR, first it will use the WRITE_BUFFER command followed by an EXECUTE that supplies the crc16.  Both the sending and receiving side can calculate the crc in realtime during the transfer.  On the PC receiving side, a READ_SIZE gives the size of what can be received, then READ_BUFFER, and finally READ_CRC.  Now each mode is distinct and can have the fastest loop time without having to test when it should or shouldn't calculate a crc and so on.  I'm going to work on the PC side code tonight and then see what type of performance it has.  From there I can revisit if I need to change the crc16 to a different type of checksum to speed it up further.

The first block is 1K being sent from the PC.