ataradov / dgw

USB HID Data Gateway
51 stars 11 forks source link

Potential issue in embedded/spi_master.c #1

Open omzlo opened 7 years ago

omzlo commented 7 years ago

Thanks for this great source of info. I've been reading your code to learn about programming the SAMD21.

I found a potential issue in the SPI code: when calling spi_ss(1) at the end of a transmission, you need to make sure that the transmit buffer is empty before unselecting the slave to avoid losing the last character with some slaves.

I would rewrite spi_ss as follows:

void spi_ss(int state)
{
  if (state)
  {
     while (SPI_SERCOM.INTFLAG.bit.TXC==0);        // Wait for transmit to finish
     SPI_SERCOM->SPI.INTFLAG.bit.TXC = 1;          // clear Int flag
     HAL_GPIO_SS_write(1);
  }
  else
  {
     HAL_GPIO_SS_write(0);
  }
}

In my case, I broke down this call in two functions: spi_begin() and spi_end(), following the same idea.

ataradov commented 7 years ago

You are correct, there is an issue. Just doing

void spi_ss(int state)
{
  while (!SPI_SERCOM->SPI.INTFLAG.bit.TXC);
  HAL_GPIO_SS_write(state);
}

should be sufficient. I'll test and commit this later.

ataradov commented 7 years ago

I pushed the change. Just checking DRE should be sufficient here.

Although I could not find a way to reproduce this, since minimum clock frequency this implementation allows is 93 kHz, and sending deselect command this fast is not really possible.