search

Microchip-MPLAB-Harmony / core

Harmony 3 Core
https://onlinedocs.microchip.com/v2/keyword-lookup?keyword=MH3_core&redirect=true
Other
17 stars 12 forks source link

Spi driver corrupts memory #21

Closed Psy-Kai closed 3 years ago

Psy-Kai commented 3 years ago

If Spi data bits are > 8 bits the driver will corrupt memory. This is due to 328b3cdb5418aec0ce1ed04425561ea39fed8063 where txSize and rxSize is doubled. The problem is that rxSize and txSize is not the word count in the buffer but the buffers size in bytes. Which means that doubling will lead to writing data which is places after the txbuffer and data is written into the memory after rxBuffer.

vishalnxt commented 3 years ago

@Psy-Kai Thank you for reporting the issue. When using SPI driver, if the data width is 8-bit, then the tx and rx size in the DRV_SPI_WriteReadTransferAdd API must be specified in terms of bytes. If the data width is 16-bits, then the tx and rx size should be specified in units of 16-bits. For example, if 5 16-bit words are being transmitted, then application should pass 5 as the tx size, and the transmit buffer should contain 5 16-bit words (or 10 bytes). Internally, the SPI driver multiplies the received tx and rx size by 2 when the data width is other than 8 bit. It does this because the SPI PLIB and the DMA PLIB, both always expects the size in terms of bytes irrespective of the SPI data width or the cell transfer size in DMA. Note: If the application is using the SPI PLIB APIs, then the tx/rx sizes are specified in units of 8-bit irrespective of the configured data-width. The SPI PLIB internally, divides the received tx/rx size by 2 if the data-width is 16-bits.

Having said this, the SPI driver documentation needs to be updated to indicate that the size should be specified in units of the data-width. Currently the documentation says that the size must always be specified in units of bytes which is wrong. I am creating an internal ticket to fix the documentation and will be fixed in the next release of core - v3.10.

Psy-Kai commented 3 years ago

Before that change the size had to be passed in terms of bytes. Why changing that? This will lead to problems with every codebase using the SPI driver. The change was not mentioned anywhere! In therms of API-stability this is not very good...

vishalnxt commented 3 years ago

@Psy-Kai , the documentation is updated in the core v3.10 release. I am closing this ticket. Feel free to re-open it in case you find any issues.