Skip to content

Commit

Permalink
Run formatter, improve CacheAlignedBuffer docs
Browse files Browse the repository at this point in the history
  • Loading branch information
multiplemonomials committed Dec 4, 2023
1 parent b45357e commit 86dbd36
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 43 deletions.
10 changes: 5 additions & 5 deletions drivers/include/drivers/SPI.h
Original file line number Diff line number Diff line change
Expand Up @@ -478,15 +478,15 @@ class SPI : private NonCopyable<SPI> {
*/
template<typename WordT, size_t RxBufferLen>
typename std::enable_if<std::is_integral<WordT>::value, int>::type
transfer(const WordT *tx_buffer, int tx_length, CacheAlignedBuffer<WordT, RxBufferLen> & rx_buffer, int rx_length, const event_callback_t &callback, int event = SPI_EVENT_COMPLETE)
transfer(const WordT *tx_buffer, int tx_length, CacheAlignedBuffer<WordT, RxBufferLen> &rx_buffer, int rx_length, const event_callback_t &callback, int event = SPI_EVENT_COMPLETE)
{
return transfer_internal(tx_buffer, tx_length, rx_buffer.data(), rx_length, callback, event);
}

// Overloads of the above to support passing nullptr
template<typename WordT, size_t RxBufferLen>
typename std::enable_if<std::is_integral<WordT>::value, int>::type
transfer(const std::nullptr_t *tx_buffer, int tx_length, CacheAlignedBuffer<WordT, RxBufferLen> & rx_buffer, int rx_length, const event_callback_t &callback, int event = SPI_EVENT_COMPLETE)
transfer(const std::nullptr_t *tx_buffer, int tx_length, CacheAlignedBuffer<WordT, RxBufferLen> &rx_buffer, int rx_length, const event_callback_t &callback, int event = SPI_EVENT_COMPLETE)
{
return transfer_internal(tx_buffer, tx_length, rx_buffer, rx_length, callback, event);
}
Expand Down Expand Up @@ -524,15 +524,15 @@ class SPI : private NonCopyable<SPI> {
*/
template<typename WordT, size_t RxBufferLen>
typename std::enable_if<std::is_integral<WordT>::value, int>::type
transfer_and_wait(const WordT *tx_buffer, int tx_length, CacheAlignedBuffer<WordT, RxBufferLen> & rx_buffer, int rx_length, rtos::Kernel::Clock::duration_u32 timeout = rtos::Kernel::wait_for_u32_forever)
transfer_and_wait(const WordT *tx_buffer, int tx_length, CacheAlignedBuffer<WordT, RxBufferLen> &rx_buffer, int rx_length, rtos::Kernel::Clock::duration_u32 timeout = rtos::Kernel::wait_for_u32_forever)
{
return transfer_and_wait_internal(tx_buffer, tx_length, rx_buffer.data(), rx_length, timeout);
}

// Overloads of the above to support passing nullptr
template<typename WordT, size_t RxBufferLen>
typename std::enable_if<std::is_integral<WordT>::value, int>::type
transfer_and_wait(const std::nullptr_t *tx_buffer, int tx_length, CacheAlignedBuffer<WordT, RxBufferLen> & rx_buffer, int rx_length, rtos::Kernel::Clock::duration_u32 timeout = rtos::Kernel::wait_for_u32_forever)
transfer_and_wait(const std::nullptr_t *tx_buffer, int tx_length, CacheAlignedBuffer<WordT, RxBufferLen> &rx_buffer, int rx_length, rtos::Kernel::Clock::duration_u32 timeout = rtos::Kernel::wait_for_u32_forever)
{
return transfer_and_wait_internal(tx_buffer, tx_length, rx_buffer.data(), rx_length, timeout);
}
Expand Down Expand Up @@ -741,7 +741,7 @@ class SPI : private NonCopyable<SPI> {
#if __DCACHE_PRESENT
// These variables store the location and length in bytes of the Rx buffer if an async transfer
// is in progress. They are used for invalidating the cache after the transfer completes.
void * _transfer_in_progress_rx_buffer;
void *_transfer_in_progress_rx_buffer;
size_t _transfer_in_progress_rx_len;
#endif

Expand Down
17 changes: 6 additions & 11 deletions drivers/source/SPI.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -397,8 +397,7 @@ void SPI::abort_transfer()
}

#if __DCACHE_PRESENT
if(_transfer_in_progress_uses_dma && _transfer_in_progress_rx_len > 0)
{
if (_transfer_in_progress_uses_dma && _transfer_in_progress_rx_len > 0) {
// If the cache is present, invalidate the Rx data so it's loaded from main RAM.
// We only want to do this if DMA actually got used for the transfer because, if interrupts
// were used instead, the cache might have the correct data and NOT the main memory.
Expand Down Expand Up @@ -484,17 +483,15 @@ void SPI::start_transfer(const void *tx_buffer, int tx_length, void *rx_buffer,
// not interoperate with the CPU cache. So, manual flushing/invalidation will be required.
// This page is very useful for how to do this correctly:
// https://community.st.com/t5/stm32-mcus-products/maintaining-cpu-data-cache-coherence-for-dma-buffers/td-p/95746
if(tx_length > 0)
{
if (tx_length > 0) {
// For chips with a cache, we need to evict the Tx data from cache to main memory.
// This ensures that the DMA controller can see the most up-to-date copy of the data.
SCB_CleanDCache_by_Addr(const_cast<void*>(tx_buffer), tx_length);
SCB_CleanDCache_by_Addr(const_cast<void *>(tx_buffer), tx_length);
}

// Additionally, we have to make sure that there aren't any pending changes which could be written back
// to the Rx buffer memory by the cache at a later date, corrupting the DMA results.
if(rx_length > 0)
{
if (rx_length > 0) {
SCB_InvalidateDCache_by_Addr(rx_buffer, rx_length);
}
_transfer_in_progress_rx_buffer = rx_buffer;
Expand Down Expand Up @@ -547,8 +544,7 @@ void SPI::irq_handler_asynch(void)
if ((event & SPI_EVENT_ALL)) {

#if __DCACHE_PRESENT
if(_transfer_in_progress_uses_dma && _transfer_in_progress_rx_len > 0)
{
if (_transfer_in_progress_uses_dma && _transfer_in_progress_rx_len > 0) {
// If the cache is present, invalidate the Rx data so it's loaded from main RAM.
// We only want to do this if DMA actually got used for the transfer because, if interrupts
// were used instead, the cache might have the correct data and NOT the main memory.
Expand All @@ -566,8 +562,7 @@ void SPI::irq_handler_asynch(void)

unlock_deep_sleep();

if(_callback)
{
if (_callback) {
_callback.call(event & SPI_EVENT_ALL);
}
}
Expand Down
74 changes: 52 additions & 22 deletions platform/include/platform/CacheAlignedBuffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,11 @@ namespace mbed {
/**
* @brief CacheAlignedBuffer is used by Mbed in locations where we need a cache-aligned buffer.
*
* Cache alignment is desirable in several different situations in embedded programming; often
* when working with DMA or other peripherals which write their results back to main memory.
* Cache alignment is desirable in several different situations in embedded programming -- one common
* use is when working with DMA or other peripherals which write their results back to main memory.
* After these peripherals do their work, the data will be correct in main memory, but the CPU cache
* might also contain a value cached from that memory which is now incorrect.
*
* In order to read those results from memory without risk of getting old data from the
* CPU cache, one needs to align the buffer so it takes up an integer number of cache lines,
* then invalidate the cache lines so that the data gets reread from RAM.
Expand All @@ -43,7 +46,7 @@ namespace mbed {
*/
template<typename DataT, size_t BufferSize>
struct
CacheAlignedBuffer {
CacheAlignedBuffer {
private:
#if __DCACHE_PRESENT
// Allocate enough extra space that we can shift the start of the buffer forward to be on a cache line.
Expand All @@ -54,14 +57,14 @@ CacheAlignedBuffer {
// So, we need to round up the backing buffer size to the nearest multiple of the cache line size.
// The math for rounding up can be found here:
// https://community.st.com/t5/stm32-mcus-products/maintaining-cpu-data-cache-coherence-for-dma-buffers/td-p/95746
constexpr static size_t requiredSizeRoundedUp = (BufferSize * sizeof(DataT) + __SCB_DCACHE_LINE_SIZE - 1) & ~((__SCB_DCACHE_LINE_SIZE) - 1);
constexpr static size_t requiredSizeRoundedUp = (BufferSize *sizeof(DataT) + __SCB_DCACHE_LINE_SIZE - 1) & ~((__SCB_DCACHE_LINE_SIZE) - 1);
constexpr static size_t backingBufferSizeBytes = requiredSizeRoundedUp + __SCB_DCACHE_LINE_SIZE - 1;
#else
constexpr static size_t backingBufferSizeBytes = BufferSize * sizeof(DataT);
#endif

uint8_t _backingBuffer[backingBufferSizeBytes];
DataT * _alignedArrayPtr;
DataT *_alignedArrayPtr;

/**
* Find and return the first location in the given buffer that starts on a cache line.
Expand All @@ -71,7 +74,7 @@ CacheAlignedBuffer {
*
* @return Pointer to first data item, aligned at the start of a cache line.
*/
inline DataT * findCacheLineStart(uint8_t * buffer)
inline DataT *findCacheLineStart(uint8_t *buffer)
{
#if __DCACHE_PRESENT
// Use integer division to divide the address down to the cache line size, which
Expand All @@ -90,78 +93,105 @@ CacheAlignedBuffer {
public:

// Iterator types
typedef DataT * iterator;
typedef DataT const * const_iterator;
typedef DataT *iterator;
typedef DataT const *const_iterator;

/**
* @brief Construct new cache-aligned buffer. Buffer will be zero-initialized.
*/
CacheAlignedBuffer():
_backingBuffer{},
_alignedArrayPtr(findCacheLineStart(_backingBuffer))
_backingBuffer{},
_alignedArrayPtr(findCacheLineStart(_backingBuffer))
{}

/**
* @brief Copy from other cache-aligned buffer. Buffer memory will be copied.
*/
CacheAlignedBuffer(CacheAlignedBuffer const & other):
_alignedArrayPtr(findCacheLineStart(_backingBuffer))
CacheAlignedBuffer(CacheAlignedBuffer const &other):
_alignedArrayPtr(findCacheLineStart(_backingBuffer))
{
memcpy(this->_alignedArrayPtr, other._alignedArrayPtr, BufferSize * sizeof(DataT));
}

/**
* @brief Assign from other cache-aligned buffer. Buffer memory will be assigned.
*/
CacheAlignedBuffer & operator=(CacheAlignedBuffer const & other)
CacheAlignedBuffer &operator=(CacheAlignedBuffer const &other)
{
memcpy(this->_alignedArrayPtr, other._alignedArrayPtr, BufferSize * sizeof(DataT));
}

/**
* @brief Get a pointer to the aligned data array inside the buffer
*/
DataT * data() { return _alignedArrayPtr; }
DataT *data()
{
return _alignedArrayPtr;
}

/**
* @brief Get a pointer to the aligned data array inside the buffer (const version)
*/
DataT const * data() const { return _alignedArrayPtr; }
DataT const *data() const
{
return _alignedArrayPtr;
}

/**
* @brief Element access
*/
DataT & operator[](size_t index) { return _alignedArrayPtr[index]; }
DataT &operator[](size_t index)
{
return _alignedArrayPtr[index];
}

/**
* @brief Element access (const)
*/
DataT operator[](size_t index) const { return _alignedArrayPtr[index]; }
DataT operator[](size_t index) const
{
return _alignedArrayPtr[index];
}

/**
* @brief Get iterator for start of buffer
*/
iterator begin() { return _alignedArrayPtr; }
iterator begin()
{
return _alignedArrayPtr;
}

/**
* @brief Get iterator for start of buffer
*/
const_iterator begin() const { return _alignedArrayPtr; }
const_iterator begin() const
{
return _alignedArrayPtr;
}

/**
* @brief Get iterator for end of buffer
*/
iterator end() { return _alignedArrayPtr + BufferSize; }
iterator end()
{
return _alignedArrayPtr + BufferSize;
}

/**
* @brief Get iterator for end of buffer
*/
const_iterator end() const { return _alignedArrayPtr + BufferSize; }
const_iterator end() const
{
return _alignedArrayPtr + BufferSize;
}

/**
* @return The maximum amount of DataT elements that this buffer can hold
*/
constexpr size_t capacity() { return BufferSize; }
constexpr size_t capacity()
{
return BufferSize;
}

/**
* @brief If this MCU has a data cache, this function _invalidates_ the buffer in the data cache.
Expand Down
8 changes: 3 additions & 5 deletions storage/blockdevice/COMPONENT_SD/source/SDBlockDevice.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -921,8 +921,7 @@ int SDBlockDevice::_read_bytes(uint8_t *buffer, uint32_t length)
// read data
#if DEVICE_SPI_ASYNCH
if (_async_spi_enabled) {
if(length > _async_data_buffer.capacity())
{
if (length > _async_data_buffer.capacity()) {
return SD_BLOCK_DEVICE_ERROR_PARAMETER;
}

Expand Down Expand Up @@ -973,8 +972,7 @@ int SDBlockDevice::_read(uint8_t *buffer, uint32_t length)
// read data
#if DEVICE_SPI_ASYNCH
if (_async_spi_enabled) {
if(length > _async_data_buffer.capacity())
{
if (length > _async_data_buffer.capacity()) {
return SD_BLOCK_DEVICE_ERROR_PARAMETER;
}

Expand Down Expand Up @@ -1081,7 +1079,7 @@ bd_size_t SDBlockDevice::_sd_sectors()
}
#ifdef DEVICE_SPI_ASYNCH
CacheAlignedBuffer<uint8_t, 16> csd_buffer;
uint8_t * csd = csd_buffer.data();
uint8_t *csd = csd_buffer.data();
#else
uint8_t csd[16];
#endif
Expand Down

0 comments on commit 86dbd36

Please sign in to comment.