Funny Monkey Coding (FMC) Time by ChongOscar · Pull Request #131 · RIT-EVT/EVT-core

ChongOscar · 2026-02-28T18:42:16Z

FMC support

Added support for configuring and reading SDRAM through a Flexible Memory Controller (FMC)

Change list:

FMC: is an abstract class containing only the base address for future chip support
FMCf4xx: defines a FMC implementation for F4 boards

…gnment

ActuallyTaylor

I have quite a few comments, my main note is that the PR needs to use more EVT-Core abstractions. There is a re-implementation of GPIO handling, which is already covered by our GPIO class. If it is not completely covered, the GPIO class should be subclassed and adapted.

ActuallyTaylor · 2026-02-28T19:23:37Z

include/core/io/platform/f4xx/FMCf4xx.hpp

+typedef struct {
+    uint16_t PIN;
+    GPIO_TypeDef* PORT;
+} GPIO;


EVT Core has a GPIO implementation already... We should always stray away from re-defining hardware abstraction since many edge cases and behaviors are handled in our existing abstractions.

https://github.com/RIT-EVT/EVT-core/blob/main/include/core/io/GPIO.hpp

This is directly taken from the STM32 Cube HUDL project's FMC I am working. It can definitely be removed without causing conflicts

Agreed, it will be removed

ActuallyTaylor · 2026-02-28T19:24:56Z

include/core/io/platform/f4xx/FMCf4xx.hpp

+    struct SdramInitConfig {
+        FMC_SDRAM_TypeDef* sdramDevice = FMC_SDRAM_DEVICE;
+        uint32_t sdBank                = FMC_SDRAM_BANK1;
+        uint32_t columnBitsNumber      = FMC_SDRAM_COLUMN_BITS_NUM_8;
+        uint32_t rowBitsNumber         = FMC_SDRAM_ROW_BITS_NUM_12;
+        uint32_t memoryDataWidth       = FMC_SDRAM_MEM_BUS_WIDTH_16;
+        uint32_t internalBankNumber    = FMC_SDRAM_INTERN_BANKS_NUM_4;
+        uint32_t casLatency            = FMC_SDRAM_CAS_LATENCY_2;
+        uint32_t writeProtection       = FMC_SDRAM_WRITE_PROTECTION_DISABLE;
+        uint32_t sdClockPeriod         = FMC_SDRAM_CLOCK_PERIOD_2;
+        uint32_t readBurst             = FMC_SDRAM_RBURST_ENABLE;
+        uint32_t readPipeDelay         = FMC_SDRAM_RPIPE_DELAY_0;
+    };


These structures are not going to be unique for the F4. They should be a part of the core FMC class. Initialization of the values can happen within the FMCf4xx.hpp but the struct definition should happen in the main FMC.hpp

These also might change based on what RAM on what platform we are targeting. This was generated by CubeMX, so it might be worthwhile to make this a passed in struct

Got it, the sdramDevice is a HAL specific macro but that can be separated from the init config so that the config can be defined in the base FMC.hpp.

ActuallyTaylor · 2026-02-28T19:25:13Z

include/core/io/platform/f4xx/FMCf4xx.hpp

+    /**
+     * Structure to simplify SDRAM timing initialization, pre-filled with default values
+     *
+     * Contains all required SDRAM timing delays in clock cycles.
+     */
+    struct SdramTimingConfig {
+        uint32_t loadToActiveDelay    = LOAD_MODE_REGISTER_TO_ACTIVE;
+        uint32_t exitSelfRefreshDelay = EXIT_SELF_REFRESH_DELAY;
+        uint32_t selfRefreshTime      = SELF_REFRESH_TIME;
+        uint32_t rowCycleDelay        = ROW_CYCLE_DELAY;
+        uint32_t writeRecoveryTime    = RECOVERY_DELAY;
+        uint32_t rpDelay              = ROW_PRECHARGE_DELAY;
+        uint32_t rcdDelay             = ROW_TO_COLUMN_DELAY;
+    };


This should also be located within the FMC base class.

ActuallyTaylor · 2026-02-28T19:28:34Z

src/core/io/platform/f4xx/FMCf4xx.cpp

+void FMCf4xx::write32(uint32_t offset, uint32_t value) const {
+    // Ensure 4-byte alignment
+    if (offset % sizeof(uint32_t) != 0)
+        return;
+
+    // Ensure within SDRAM bounds
+    if (offset >= RAM_SIZE)
+        return;
+
+    auto* const ptr = reinterpret_cast<volatile uint32_t*>(sdramMemoryAddress + offset);
+
+    *ptr = value;
+}


This should return a STATUS enumeration that will tell someone if it succeeded or not.

This method should not exist

ActuallyTaylor · 2026-02-28T19:29:33Z

src/core/io/platform/f4xx/FMCf4xx.cpp

+uint32_t FMCf4xx::read32(uint32_t offset) const {
+    // Ensure 4-byte alignment
+    if (offset % sizeof(uint32_t) != 0)
+        return 0;
+
+    // Ensure within SDRAM bounds
+    if (offset >= RAM_SIZE)
+        return 0;
+
+    auto* const ptr = reinterpret_cast<volatile uint32_t*>(sdramMemoryAddress + offset);
+
+    return *ptr;
+}


This should return a STATUS enumeration that will tell a user if it was able to read or not. The value should be passed back through a pointer in the function parameters. See how we do this in the BMS: https://github.com/RIT-EVT/BMS/blob/00bc33d695f8865c8ae2ff0de5e493eb70ec08ca/src/dev/BQ76952.cpp#L241

This method should not exist either

ActuallyTaylor · 2026-02-28T19:34:45Z

src/core/io/FMC.cpp

+FMC::FMC(uint32_t sdramMemoryAddress) {
+    this->sdramMemoryAddress = sdramMemoryAddress;
+}


This should be using constructor initialization. An example of this in EVT-Core: https://github.com/RIT-EVT/EVT-core/blob/main/src/core/io/ADC.cpp

ActuallyTaylor · 2026-02-28T19:41:05Z

include/core/io/platform/f4xx/FMCf4xx.hpp

I have a couple file wide notes.

Existing EVT-Core abstractions should be used. The code is using a custom GPIO implementation which is more work than necessary as well as duplicating existing code.

All pins in this file should use the EVT-Core pin class

ActuallyTaylor · 2026-02-28T19:48:04Z

src/core/io/FMC.cpp

@@ -0,0 +1,9 @@
+#include "core/io/FMC.hpp"


This should use angled bracket inclusion <>

ActuallyTaylor · 2026-02-28T19:48:39Z

include/core/io/platform/f4xx/FMCf4xx.hpp

+#include "HALf4/stm32f4xx_hal.h"
+#include "HALf4/stm32f4xx_hal_sdram.h"
+#include "HALf4/stm32f4xx_ll_fmc.h"
+
+#include "core/io/FMC.hpp"


This should use angled bracket inclusion <>

ActuallyTaylor · 2026-02-28T19:49:09Z

include/core/io/platform/f4xx/FMCf4xx.hpp

+ * - SDRAM timing configuration
+ * - SDRAM read/write operations
+ *
+ * @note Requires STM32 HAL libraries.


All of our platforms require STM32 HAL libraries, so this comment is not really applicable.

DannyCato · 2026-03-01T02:42:47Z

include/core/io/platform/f4xx/FMCf4xx.hpp

+#define FMC_SDNWE  ((FMC_CMD){GPIO_PIN_5, GPIOH})
+#define FMC_SDNRAS ((FMC_CMD){GPIO_PIN_11, GPIOF})
+
+// #define FMC_ 		((GPIO) {GPIO_PIN_x, GPIOx})


All the above defines can definitely be removed for the future. They did not prove helpful in the HUDL project, and only add to confusion

DannyCato · 2026-03-01T02:45:38Z

include/core/io/platform/f4xx/FMCf4xx.hpp

+#define LOAD_MODE_REGISTER_TO_ACTIVE (NS_TO_SDRAM_CLK_CYCLES(tMRD))
+
+#define RAM_SIZE      (0x4000000) // 64 megabits
+#define STARTING_ADDR ((uint32_t*) 0xC000000)


There are some additional defines in the HUDL project that could prove useful, but I am wondering if we can simply turn these into functions that get executed on construction of the object.

In any case, a method to simply find the clock speed of the FMC could be helpful, and at that point just make everything happen at runtime. No preprocessing magic

DannyCato · 2026-03-01T02:48:32Z

include/core/io/platform/f4xx/FMCf4xx.hpp

+        FMC_BANK* pins;
+        uint8_t count;
+    };
+


Change these to be wrappers of the evt core GPIO pins instead of the above macros

DannyCato · 2026-03-01T02:50:17Z

include/core/io/platform/f4xx/FMCf4xx.hpp

+
+    void write32(uint32_t offset, uint32_t value) const;
+
+    uint32_t read32(uint32_t offset) const;


These should not be explicitly be defined. If you try to write or read from a value at the address of FMC Bank (the STARTING_ADDRESS macro), then it should simply work with some hardware magic from the peripheral

DannyCato · 2026-03-01T02:51:36Z

include/core/io/FMC.hpp

+     * @param[in] offset Byte offset from the SDRAM base address
+     */
+    uint32_t read32(uint32_t offset) const;
+


Reading or writing data of any size to the address of the FMC will cause reads and writes to the underlying RAM

DannyCato · 2026-03-01T02:52:43Z

include/core/io/FMC.hpp

+    uint32_t read32(uint32_t offset) const;
+
+protected:
+    uint32_t sdramMemoryAddress;


should be a void pointer to signify that it is a memory address. That's what you would do in C, but maybe there is some other convention for C++

Do I still need this if im removing the read and write functions?

DannyCato · 2026-03-01T03:00:58Z

include/core/io/FMC.hpp

+
+protected:
+    uint32_t sdramMemoryAddress;
+};


The public section of this should also include wrappers for the methods that can be seen in stm32f4xx_ll_fmc.h. Namely, FMC_SDRAM_WriteProtection_Enable and FMC_SDRAM_WriteProtection_Disable. I have not checked if those are available on the F3, but I would be shocked if they weren't

The F3 FMC doesn't seem to support any SDRAM operations at all

…to use EVT-core's gpio abstraction

… the base class and finally figured out how GPIO works

…into feature/ChongOscar/fmc-support # Conflicts: # include/core/io/FMC.hpp # include/core/io/platform/f4xx/FMCf4xx.hpp # src/core/io/platform/f4xx/FMCf4xx.cpp

…into feature/ChongOscar/fmc-support # Conflicts: # include/core/io/FMC.hpp # include/core/io/platform/f4xx/FMCf4xx.hpp # src/core/io/FMC.cpp # src/core/io/platform/f4xx/FMCf4xx.cpp

…into feature/ChongOscar/fmc-support # Conflicts: # src/core/io/platform/f4xx/FMCf4xx.cpp

oc6723 and others added 11 commits February 14, 2026 13:26

added FMC.hpp and FMC.cpp, have not tested

e97f649

replaced private vars with structs stored in class

712e8b7

changed structs to pass by value instead of reference

3dd7cad

fixed CMakeLists.txt i think

0160aa4

renamed to FMCf4xx

27cb4f1

improved read and write functions to check bounds and enforce bit ali…

7258706

…gnment

created generic FMC interface

8496802

fixed compile bug

2f47125

used getSdramBaseAddress() in FMC constructor

2de2205

Merge branch 'main' into feature/ChongOscar/fmc-support

d63112a

Applied Formatting Changes During GitHub Build

565b9c4

ActuallyTaylor requested changes Feb 28, 2026

View reviewed changes

DannyCato reviewed Mar 1, 2026

View reviewed changes

oc6723 and others added 11 commits March 5, 2026 00:11

removed unnecessary #defines in FMCf4xx.hpp and swapped pin typedefs …

cb91f45

…to use EVT-core's gpio abstraction

made default init config functions so the definitions can be moved to…

db7d9a4

… the base class and finally figured out how GPIO works

Merge remote-tracking branch 'origin/feature/ChongOscar/fmc-support' …

1646c19

…into feature/ChongOscar/fmc-support # Conflicts: # include/core/io/FMC.hpp # include/core/io/platform/f4xx/FMCf4xx.hpp # src/core/io/platform/f4xx/FMCf4xx.cpp

removed duplicate namespace declaration

ab4bfc3

Applied Formatting Changes During GitHub Build

f9d0ced

added back sdramMemoryAddress as public void* but init looks ugly

4f2798f

Merge remote-tracking branch 'origin/feature/ChongOscar/fmc-support' …

01d8b29

…into feature/ChongOscar/fmc-support # Conflicts: # include/core/io/FMC.hpp # include/core/io/platform/f4xx/FMCf4xx.hpp # src/core/io/FMC.cpp # src/core/io/platform/f4xx/FMCf4xx.cpp

why cant i push

7ce4e85

Applied Formatting Changes During GitHub Build

3498621

formatting changes

99cb57e

merged with github

a5c77d7

GitHub Build and others added 4 commits March 16, 2026 23:42

Applied Formatting Changes During GitHub Build

4f87078

moved everyting back to f4xx because sdram is not supported on te f3's

6a996e6

Merge remote-tracking branch 'origin/feature/ChongOscar/fmc-support' …

7b558ad

…into feature/ChongOscar/fmc-support # Conflicts: # src/core/io/platform/f4xx/FMCf4xx.cpp

chaned clock frequency #defines to functions

6823191

ChongOscar requested a review from DannyCato March 28, 2026 18:03


		void write32(uint32_t offset, uint32_t value) const;

		uint32_t read32(uint32_t offset) const;

Conversation

ChongOscar commented Feb 28, 2026

FMC support

Change list:

Uh oh!

ActuallyTaylor left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DannyCato Mar 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DannyCato Mar 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

DannyCato Mar 1, 2026 •

edited

Loading

DannyCato Mar 1, 2026 •

edited

Loading