hideakitai / ESP32DMASPI

SPI library for ESP32 which use DMA buffer to send/receive transactions
MIT License
181 stars 37 forks source link

About buffers block alignment checks... #20

Closed jlarnal closed 2 years ago

jlarnal commented 2 years ago

Dear author. I see you use this syntax when checking the size requirement of the buffers used in the Master::transfer(...) methods :

if ( n % 4 != 0) {
  printf("[WARN] DMA buffer size must be multiples of 4 bytes\n");
}

I'd recommend this 🤔 :

if ( n & 3 ) { // same as  modulo 4, since modulus of 4 is a bitmask of 0b011 .
  ESP_LOGW("ESP32DMASPI::Master", "DMA buffer size must be multiples of 4 bytes\n"); // can be silent according to the config scheme.
}

It saves us a division, and depending on the arch ( Xtensa or Risc-V (for ESP32-C3) ) the operation can be optimized to operate only the least significant byte of "n", regardless of the variable's bytesize.

😘

jlarnal commented 2 years ago

Addendum : transfers shorter than 32 bits (4 bytes) aren't forbidden on ESP32 by the way, they're just really inefficient (DMA transfers are costly to initialize and arbiter, so not really worth it for small buffers) .

Cf this : https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-reference/peripherals/spi_master.html#transactions-with-data-not-exceeding-32-bits

You SHOULD however ensure that the given buffer is in DMA-capable space, and that its address IS a multiple of 4 (i.e. on a DWORD boundary), otherwise... Panic on the core ! 😅

hideakitai commented 2 years ago

Great to know! But I think n % 4 != 0 is more intuitive than n & 3. Thank you for the suggestion.