greiman / SdFat

Arduino FAT16/FAT32 exFAT Library
MIT License
1.07k stars 502 forks source link

STM32F1: SdFat creates 2 or 3 SPI class objects even if not used #100

Open victorpv opened 6 years ago

victorpv commented 6 years ago

When using SdFat with STM32F1, the library creates as many SPI static objects as SPI ports exist, even if they are not used. That causes extra ram usage, around 112 bytes per object. It can be a considerable amount for the versions with 20KB of RAM. I don't know who this could be best solved, perhaps there is some other way to do it? https://github.com/greiman/SdFat/blob/master/src/SpiDriver/SdSpiSTM32.cpp#L35:L40 The SPI library already creates one for port 1, called just "SPI", perhaps the first one could be a reference or pointer to that one, so at we save the RAM from one of them?

I have also ported the libmaple F4 SDIO driver to the F1 and works great, but when using it, I still get those 3 SPI objects. Any idea or advice on how to skip the SPI part if using SDIO? And should I send a PR when the SDIO driver is final to include it in your repo?

greiman commented 6 years ago

It will be some time before I update SdFat for STM32. My first priority is to improve STM32 support for SdFs then perhaps port it back to SdFat.

Teensy and SAMD now support multiple SPI ports with a type SPIClass so I hope STM32 will do the same.

I will use a new structures to define SPI ports and options and SDIO options.

  //----------------------------------------------------------------------------
  /** Initialize SD card and file system for SPI mode.
   *
   * \param[in] spiConfig SPI configuration.
   * \return true for success else false.
   */
  bool begin(SdSpiConfig spiConfig) {
    return cardBegin(spiConfig) && Vol::begin(m_card);
  }
  //---------------------------------------------------------------------------
  /** Initialize SD card and file system for SDIO mode.
   *
   * \param[in] sdioConfig SDIO configuration.
   * \return true for success else false.
   */
  bool begin(SdioConfig sdioConfig) {
    return cardBegin(sdioConfig) && Vol::begin(m_card);
  }

Currently I am only doing bug fixes to SdFat.

I am thinking about using the SdFs code as the next version of SdFat.

victorpv commented 6 years ago

Ok I'll have a look at SdFs and see if I can get the F1 SDIO driver working with it. I don'r consider the ram usage a bug, just a little surprise, so I guess we can close this issue.

greiman commented 6 years ago

Unfortunately I plan major changes to how this part of SdFs works. Currently I have a SPI port number in SdSpiConfig .

The EX option is now in the opt parameter:

// opt bits
/** The SD is the only device on the SPI bus. */
#define DEDICATED_SPI 0X80
/** SPI bus is share with other devices. */
#define SHARED_SPI 0

  /** SdSpiConfig constructor.
   *
   * \param[in] cs Chip select pin.
   * \param[in] opt Options.
   * \param[in] settings SPISettings.
   * \param[in] port The SPI port to use.
   */
  SdSpiConfig(uint8_t cs, uint8_t opt, SPISettings settings, uint8_t port);

I plan to change port to a pointer to an SPI object since Teensy and SAMD have these objects defined in SPI.cpp.

// From Teensy SPI.cpp
SPIClass SPI((uintptr_t)&KINETISK_SPI0, (uintptr_t)&SPIClass::spi0_hardware);
SPIClass SPI1((uintptr_t)&KINETISK_SPI1, (uintptr_t)&SPIClass::spi1_hardware);
SPIClass SPI2((uintptr_t)&KINETISK_SPI2, (uintptr_t)&SPIClass::spi2_hardware);  
// SAMD
  SPIClass SPI (&PERIPH_SPI,  PIN_SPI_MISO,  PIN_SPI_SCK,  PIN_SPI_MOSI,  PAD_SPI_TX,  PAD_SPI_RX);
#endif
#if SPI_INTERFACES_COUNT > 1
  SPIClass SPI1(&PERIPH_SPI1, PIN_SPI1_MISO, PIN_SPI1_SCK, PIN_SPI1_MOSI, PAD_SPI1_TX, PAD_SPI1_RX);
#endif
#if SPI_INTERFACES_COUNT > 2
  SPIClass SPI2(&PERIPH_SPI2, PIN_SPI2_MISO, PIN_SPI2_SCK, PIN_SPI2_MOSI, PAD_SPI2_TX, PAD_SPI2_RX);
#endif
#if SPI_INTERFACES_COUNT > 3
  SPIClass SPI3(&PERIPH_SPI3, PIN_SPI3_MISO, PIN_SPI3_SCK, PIN_SPI3_MOSI, PAD_SPI3_TX, PAD_SPI3_RX);
#endif
#if SPI_INTERFACES_COUNT > 4
  SPIClass SPI4(&PERIPH_SPI4, PIN_SPI4_MISO, PIN_SPI4_SCK, PIN_SPI4_MOSI, PAD_SPI4_TX, PAD_SPI4_RX);
#endif
#if SPI_INTERFACES_COUNT > 5
  SPIClass SPI5(&PERIPH_SPI5, PIN_SPI5_MISO, PIN_SPI5_SCK, PIN_SPI5_MOSI, PAD_SPI5_TX, PAD_SPI5_RX);
#endif
victorpv commented 6 years ago

We could do the same, but I think it's not done so far because a new SPI object takes flash and RAM, and that's limited in the maple mini and the bluepill, which are the 2 most common boards, even though they have 2 SPI ports. If I understand right, you plan to change this "uint8_t port" to a pointer to SPIClass, right? In that case, even if we only declare 1 SPI port in the SPI library, we could declare a second one at the start of the code before including the SdFs library, and that could could still be used, right?

greiman commented 6 years ago

We could do the same, but I think it's not done so far because a new SPI object takes flash and RAM

The linker should not load unused port objects. If you only use one Serial port on a Mega it take about 184 bytes of RAM but with two ports it uses 341 bytes.

void setup() {
  Serial.begin(9600);
}
void loop() {}
Sketch uses 1752 bytes (0%) of program storage space. Maximum is 253952 bytes.
Global variables use 184 bytes (2%) of dynamic memory, leaving 8008 bytes for local variables. Maximum is 8192 bytes.
void setup() {
  Serial.begin(9600);
  Serial1.begin(9600);
}
void loop() {}
Sketch uses 2008 bytes (0%) of program storage space. Maximum is 253952 bytes.
Global variables use 341 bytes (4%) of dynamic memory, leaving 7851 bytes for local variables. Maximum is 8192 bytes.

You just need to declare the SPI object and place a pointer to it in the SdSpiConfig.

Here is a bit of the current SdFs STM32 example with two card on separate SPI ports.

The cards can be formatted FAT16, FAT32, or exFAT.

The first card uses "standard" mode and the second uses extended multi-block mode.

I plan to replace the port numbers 1, and 2 with pointers.

// Chip select PA4, shared SPI, 18 MHz, port 1.
#define SD1_CONFIG SdSpiConfig(PA4, SHARED_SPI, SD_SCK_MHZ(18), 1) 
SdFs sd1;
FsFile file1;

// Chip select PB21, dedicated SPI, 18 MHz, port 2.
#define SD2_CONFIG SdSpiConfig(PB12, DEDICATED_SPI, SD_SCK_MHZ(18), 2) 
SdFs sd2;
FsFile file2;

  // in setup()

  // initialize the first card
  if (!sd1.begin(SD1_CONFIG)) {
    error("sd1.begin");
  }
  // initialize the second card
  if (!sd2.begin(SD2_CONFIG)) {
    error("sd2.begin");
  }
victorpv commented 6 years ago

I didn't expect unused objects to be pulled in, but noticed the SPI objects there taking RAM in the .map file. Then I went looking where those where and found them in the sdcard code. I was using SDIO, so I can't imagine why the compiler may have thought they were needed expect perhaps because the references go to an array, so it needs them to populate the references in the array. I will recheck, perhaps it was when I tested without any optimization option. Let me check and confirm it didn't have to do with the optimization settings, I'll test with Os, O1, O2 and O3.

victorpv commented 6 years ago

I always get these three objects, no matter what optimization option I choose:

 .bss._ZL6m_SPI1
                0x200020ac       0x70 ./libraries/SdFat/src/SpiDriver/SdSpiSTM32F1.cpp.o
 .bss._ZL6m_SPI2
                0x2000211c       0x70 ./libraries/SdFat/src/SpiDriver/SdSpiSTM32F1.cpp.o
 .bss._ZL6m_SPI3
                0x2000218c       0x70 ./libraries/SdFat/src/SpiDriver/SdSpiSTM32F1.cpp.o

But is not just because the references are used, I removed that and still shows up, so I think it may have to do with the code in the constructor. The compiler must think that the object is really needed when declared for some reason. I'll have a better look at the SPI library and see if I can find why.

greiman commented 6 years ago

I suspect the SdFat library forces the inclusion of the objects. That's why I want to use pointers and SdSpiConfig.