CAN on ATSAMC21J18A

[Rasmo]

I have managed to make a program that downloads my binary to the micro, however I am having a few issues with verifying the data.

Firstly, I found that the CRC generated by the DSU was the inverse of the standard CRC32 implementation which caught me up for a little bit. I also noticed that the size and address written to the DSU needed to be converted to words rather than bytes (although I think this was actually okay because the ADDR & LENGTH registers were offset by 2 bits anyway). However, the DSU calculated CRC differed from my CRC. I read the flash using my ICE and it was identical to the uploaded binary so there wasn't any corruption. Do you have any ideas on where to start looking? I have attached my new crc_calc code below.

Code: [Select]

static uint32_t crc32_calc(uint32_t addr, uint32_t size)
{
  DSU->ADDR.reg = DSU_ADDR_ADDR(WORDS(addr));
  DSU->LENGTH.reg = DSU_LENGTH_LENGTH(WORDS(size));
  DSU->DATA.reg = 0xffffffff;
  DSU->STATUSA.reg = DSU_STATUSA_MASK;
  DSU->CTRL.reg = DSU_CTRL_CRC;

  while (0 == DSU->STATUSA.bit.DONE);

  if (DSU->STATUSA.bit.BERR)
    return 0xffffffff;

  return DSU->DATA.reg ^ 0xFFFFFFFF;
}

[eutectique]

Firstly, I found that the CRC generated by the DSU was the inverse of the standard CRC32 implementation which caught me up for a little bit.

You should be more specific about "standard" CRC-32. There are several implementations, most of them are standard: https://crccalc.com/?crc=123456789&method=crc32&datatype=ascii&outtype=0

In-depth view: https://github.com/Michaelangel007/crc32

[eutectique]

I also noticed that the size and address written to the DSU needed to be converted to words rather than bytes ...

Code: [Select]

static uint32_t crc32_calc(uint32_t addr, uint32_t size)
{
  DSU->ADDR.reg = DSU_ADDR_ADDR(WORDS(addr));
  DSU->LENGTH.reg = DSU_LENGTH_LENGTH(WORDS(size));
  ...
}

Would you please elaborate on that? Do you mean that length of 0x100 bytes should be passed as 0x40? Then you are wrong. I have no idea how you define the macro WORDS().

Section 13.11.3.1 Starting CRC32 Calculation says:

CRC32 calculation for a memory range is started after writing the start address into the Address register (ADDR) and
the size of the memory range into the Length register (LENGTH). Both must be word-aligned.

It's not the number of words you need to write into the registers. It's the number of bytes, but aligned properly.

[ataradov]

Address and length are designed to accept direct values, but those values must be aligned at 4 bytes. With your modifications you are just doing more math for no real reason. My code is functional and tested. This CRC code was tested 100 times over in other projects.

Yes, the CRC is for the reversed polynomial. There is no real single standard. This CRC implementation matches  IEEE 802.3.

Here is a matching C implementation:

Code: [Select]

// IEEE CRC32 with a polynomial of 0xedb88320 (0x04c11db7 in reverse)

//-----------------------------------------------------------------------------
static uint32_t crc32_tab[256]};

//-----------------------------------------------------------------------------
static void crc32_tab_gen(void)
{
  for (int i = 0; i < 256; i++)
  {
    uint32_t value = i;

    for (int j = 0; j < 8; j++)
    {
      if (value & 1)
        value = (value >> 1) ^ 0xedb88320ul;
      else
        value = value >> 1;
    }

    crc32_tab[i] = value;
  }
}

//-----------------------------------------------------------------------------
static uint32_t crc32(uint8_t *data, int size)
{
  uint32_t crc = 0xffffffff;

  for (int i = 0; i < size; i++)
    crc = crc32_tab[(crc ^ data[i]) & 0xff] ^ (crc >> 8);

  return crc;
}

You can generate the table once and just have it in the code. Or call crc32_tab_gen() in  the beginning and let it initialize the table.

[Rasmo]

Address and length are designed to accept direct values, but those values must be aligned at 4 bytes. With your modifications you are just doing more math for no real reason. My code is functional and tested. This CRC code was tested 100 times over in other projects.

Yes, the CRC is for the reversed polynomial. There is no real single standard. This CRC implementation matches  IEEE 802.3.

Sorry, I am not saying that your code is wrong, I am just trying to learn and understand why / how you have done things.

I was looking at the datasheet and it said that the LENGTH register is the 'length in words needed for memory operations.' although I see that it is left shifted two bits so you can write directly the bytes. I guess that the only time this would be an issue is if you needed to write to the AMOD register? But then I guess you could always just do that after anyway. I will revert those changes.

The CRC implementation that I was using in the uploading application and a hex editor to verify, was the non reversed polynomial 0x04C11DB7 so I guess that is why I needed to do the XOR on the result.

I think I have also found why I was getting the CRC error when uploading. I see that the bootloader is expecting an exact multiple of pages in the data transfer. My binary was 0x6528 bytes long so it was dropping the last 40 bytes. Would you normally pack the last page with 0xFF's to make up the 64 bytes in the sending application, rather than within the bootloader? I guess that saves some processing in the bootloader?

[ataradov]

Even for AMOD you still can do something like this "DSU->ADDR.reg = addr | DSU_ADDR_AMOD_xxx;".

XOR with 0xffffffff is a separate from the reversal. CRC algorithms generally have independent parameters bit reversal and the need to invert the result are completely independent.

Often you inverse to get the final value, and if you are updating the value, intermediate results are not inversed.

Yes, you need to pad the data. The write is triggered when PAGE_SIZE bytes are received, so you were not writing the last page with incomplete data. It is easy enough to fix, but I personally believe that the host must provide the padding. You don't want the bootloader to "guess" the stuff for you.

I never use inversed bersion, since I see no point in doing additional work.