gremain sd fat - a debugging journey

[ag123]

this is somewhat off-topic or perhaps it belong to (steve's) libmaple and libraries
but as sd fat is used rather heavily for mass data storage, i'd like to post a pretty involved debugging session
related to gremain's sd fat and steve's libmaple core - actually the spi.h, spi.cpp Arduino-ish apis
https://github.com/greiman/SdFat
in a heavily patched up local copy of Sdinfo, i made it print the commands issued to the sd card over SPI
https://github.com/greiman/SdFat/tree/m ... les/SdInfo

A run looks like this

Code: Select all

SdFat version: 2.0.4

Assuming the SD is the only SPI device.
Edit DISABLE_CS_PIN to disable an SPI device.

Assuming the SD chip select pin is: 4
Edit SD_CS_PIN to change the SD chip select pin.

type any character to start
CMD:0
1
CMD:59
1
CMD:8
1
CMD:55
1
CMD:41
1
CMD:55
1
CMD:41
0
CMD:58
0
init time: 808 ms
CMD:2
4
CMD:3
4
CMD:10
0
000000000000000000000000000000004653
CMD:9
0
0000000000000000000000000000000032
CMD:58
0

Card type: SDHC

Manufacturer ID: 0X0
OEM ID:
Product:
Version: 0.0
Serial number: 0X0
Manufacturing date: 0/2000

cardSize: 0.00 MB (MB = 1,000,000 bytes)
flashEraseSize: 1 blocks
eraseSingleBlock: false

OCR: 0XC0FF8000
CMD:18
0
0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
... ...
000000000

SD Partition Table
part,boot,bgnCHS[3],type,endCHS[3],start,length
1,0X0,0X0,0X0,0X0,0X0,0X0,0X0,0X0,0,0
2,0X0,0X0,0X0,0X0,0X0,0X0,0X0,0X0,0,0
3,0X0,0X0,0X0,0X0,0X0,0X0,0X0,0X0,0,0
4,0X0,0X0,0X0,0X0,0X0,0X0,0X0,0X0,0,0
CMD:12
0
CMD:18
0
0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
...
000000000
CMD:12
0
CMD:18
0
0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
.. ...
000000000

volumeBegin failed. Is the card formatted?
SD errorCode: SD_CARD_ERROR_READ_REG = 0X1B
SD errorData = 0X0

type any character to start

to get to the gist of the problem, the manufacturer, card id and card size information is completely incorrect!
all the sectors partition tables and data read etc are zeros. the CMD sd card commands are inserted/patched into the codes using Serial.print() statements to trace execution.

it got me into a deep dive into the 'simplified' sd specs
https://www.sdcard.org/downloads/pls/
http://elm-chan.org/docs/mmc/mmc_e.html

the things as i learned are if you are using the spi interface (not the full sdio), the part and commands to read is only the spi portion of the specs.
the sdcard protocols are pretty complicated as it caters to sd 4 parallel channels, single channel and a fallback spi mode. the spi mode specs is one part to read while doing just spi. things used for the main 4 channel sd protocols are different from that of spi mode. the gist of this protocol specifcally for spi is something like this:
- CMD0 is like a soft reset command and this needs to be issued with CS held low. if that isn't done the card likely initialize to something other than spi mode. this means when you hit trouble with sd spi mode, the cs pin and setup is 1 important thing to look out for.
- CMD:8, CMD:55, ACMD:41, CMD 58 are initialization commands the details of which can be found in the 'simplified specs' (read the spi portion, the non-spi parts are different). CMD 58 that in part tells between types of sd cards e.g. SD (v1), SD2, SDHC etc
- CMD 10 reads card ID, CMD 9 reads CSD that contains the volume/size information - it is returning zeros in the above case.

that got me wondering what is wrong digging into the possible causes
- initially i started thinking it is the cs pin, then in doing many tests, i tried assigning different pins for cs. same problem zeros.
then one of the test i did is to simply disconnect cs pin. it stopped working. that kind of proves i'm still doing spi mode somehow.
- next i thought it is some series resistors (on the ili9341 lcd module with the sd card slot), i literally removed the resistors. same zeros
- next i thought maybe connectivity problems, so i tried printing the commands and more importantly responses. what i'm looking for is *non zero* responses, yup i got that. i actually patched my copy to issue cmd2 (get card ids) and cmd3 (card relative address) which are not intended for spi.
i got 4 which is an error code. looks good it at least confirms that i'm doing spi
- next reduced spi frequency / baud rate to 1mhz
- next i thought my sd cards do not compatibly support the spi mode - bummer (it is quite notorious some cards do not comply with the spi fallback).
then several different cards (sandisk, some less known brands etc) all give zeros. i'm about to setup a logic analyzer to dig further in
- there are some results with some data. it turns out if i memset() those buffers to zeros, that's what i got, zeros. i.e. nothing read from the sd card

[ag123]

the fix
https://github.com/greiman/SdFat/blob/m ... 32.cpp#L59

Code: Select all

uint8_t SdSpiArduinoDriver::receive(uint8_t* buf, size_t count) {
#if USE_STM32_DMA
  return m_spi->dmaTransfer(nullptr, buf, count);
#else  // USE_STM32_DMA
  m_spi->read(buf, count);
  return 0;
#endif  // USE_STM32_DMA
}

edit to read as:

Code: Select all

uint8_t SdSpiArduinoDriver::receive(uint8_t* buf, size_t count) {
#if USE_STM32_DMA
  //return m_spi->dmaTransfer(nullptr, buf, count);
  return m_spi->dmaTransfer((const uint16_t) 0xff, buf, count);

#else  // USE_STM32_DMA
  m_spi->read(buf, count);
  return 0;
#endif  // USE_STM32_DMA
}

then in roger's f4 and steve's f4
https://github.com/rogerclarkmelbourne/ ... SPI.h#L306

Code: Select all

    void dmaTransferInit(const void * transmitBuf, void * receiveBuf, uint16 length, uint16 flags = 0);
    void dmaTransferInit(const uint16_t tx_data, void * receiveBuf, uint16 length, uint16 flags = 0);

the calls to the dmatransfer() and dmasend() apis are expecting a return value, possibly modeled after the F1 apis.
in my local copy instead of returning void (i,e. no return value), i made it return an int8 or int16. for now i simply patched zero as a return value
they could possibly be helpful in situation where the dma transmit or receive errors can be detected so that we can possibly indicate the fail condition.

[ag123]

now this is what it looks like

Code: Select all

SdFat version: 2.0.4

Assuming the SD is the only SPI device.
Edit DISABLE_CS_PIN to disable an SPI device.

Assuming the SD chip select pin is: 4
Edit SD_CS_PIN to change the SD chip select pin.

type any character to start
CMD:0
1
CMD:59
1
CMD:8
1
CMD:55
1
CMD:41
1
CMD:55
1
CMD:41
0
CMD:58
0
init time: 808 ms
CMD:2
4
CMD:3
4
CMD:10
0
CMD:9
0
CMD:58
0

Card type: SDHC

Manufacturer ID: 0X84
OEM ID: TF
Product: SD
Version: 0.0
Serial number: 0XE03695E
Manufacturing date: 1/2011

cardSize: 7864.32 MB (MB = 1,000,000 bytes)
flashEraseSize: 128 blocks
eraseSingleBlock: true

OCR: 0XC0FF8000
CMD:18
0

SD Partition Table
part,boot,bgnCHS[3],type,endCHS[3],start,length
1,0X0,0X1,0X43,0X55,0XB,0X3,0XC6,0XFF,8192,15351808
2,0X0,0X0,0X0,0X0,0X0,0X0,0X0,0X0,0,0
3,0X0,0X0,0X0,0X0,0X0,0X0,0X0,0X0,0,0
4,0X0,0X0,0X0,0X0,0X0,0X0,0X0,0X0,0,0
CMD:12
0
CMD:18
0
CMD:12
0
CMD:18
0
CMD:12
0
CMD:18
0
CMD:12
0
CMD:18
0

Scanning FAT, please wait.
CMD:12
0
CMD:18
0

Volume is FAT32
sectorsPerCluster: 8
clusterCount:      1915230
freeClusterCount:  1915229
fatStartSector:    8224
dataStartSector:   38160
Data area is not aligned on flash erase boundary!
Download and use formatter from www.sdcard.org!

type any character to start

[stevestrong]

Ok, so basically I have to change the transfer functions to return the number of transferred bytes, right?

[ag123]

need to pull to bed for now, as sd fat seem to be expecting int8_t return value, i think it is expecting an error code. would review the code a while later.
i'd guess returning number of bytes transferred could be a good idea as well or -1 or -n on error. but i'd guess we'd take a look at existing f1 codes as well

[ag123]

i did some code review, in the F1 libmaple core, the dmatransfer() and dmasend() bulk transfers functions returns uint8_t
https://github.com/rogerclarkmelbourne/ ... SPI.h#L292

Code: Select all

	/**
     * @brief Sets up a DMA Transfer for "length" bytes.
	 * The transfer mode (8 or 16 bit mode) is evaluated from the SPI peripheral setting.
     *
     * This function transmits and receives to buffers.
     *
     * @param transmitBuf buffer Bytes to transmit. If passed as 0, it sends FF repeatedly for "length" bytes
     * @param receiveBuf buffer Bytes to save received data. 
     * @param length Number of bytes in buffer to transmit.
	 */
    uint8 dmaTransfer(const void * transmitBuf, void * receiveBuf, uint16 length);
    uint8 dmaTransfer(const uint16 value, void * receiveBuf, uint16 length);
    void dmaTransferSet(const void *transmitBuf, void *receiveBuf);
    uint8 dmaTransferRepeat(uint16 length);

	/**
     * @brief Sets up a DMA Transmit for SPI 8 or 16 bit transfer mode.
	 * The transfer mode (8 or 16 bit mode) is evaluated from the SPI peripheral setting.
     *
     * This function only transmits and does not care about the RX fifo.
     *
     * @param data buffer half words to transmit,
     * @param length Number of bytes in buffer to transmit.
	 * @param minc Set to use Memory Increment mode, clear to use Circular mode.
     */
    uint8 dmaSend(const void * transmitBuf, uint16 length, bool minc = 1);
    void dmaSendSet(const void * transmitBuf, bool minc);
    uint8 dmaSendRepeat(uint16 length);

    uint8 dmaSendAsync(const void * transmitBuf, uint16 length, bool minc = 1);

generally returns 2 on timeout
https://github.com/rogerclarkmelbourne/ ... I.cpp#L451
returns 0 on success

gremain sdfat defines it this way
https://github.com/greiman/SdFat/blob/m ... lass.h#L51

Code: Select all

  /** Receive multiple bytes.
  *
  * \param[out] buf Buffer to receive the data.
  * \param[in] count Number of bytes to receive.
  *
  * \return Zero for no error or nonzero error code.
  */
  virtual uint8_t receive(uint8_t* buf, size_t count) = 0;

[stevestrong]

It is not clear to me why

Code: Select all

  return m_spi->dmaTransfer(nullptr, buf, count);

had to be replaced with

Code: Select all

  return m_spi->dmaTransfer((const uint16_t) 0xff, buf, count);

because (nullptr, buf, count) should be assigned to

Code: Select all

void transfer(const uint8_t * tx_buf, uint8_t * rx_buf, uint32 len);

see https://github.com/rogerclarkmelbourne/ ... SPI.h#L285
A better workaround would be

Code: Select all

  m_spi->dmaTransfer(nullptr, buf, count);
  return 0;

similar to #else branch.

[ag123]

during my debug session, it seemed this is called in the F4 libmaple core
https://github.com/rogerclarkmelbourne/ ... I.cpp#L736

Code: Select all

//-----------------------------------------------------------------------------
//  Roger Clark and Victor Perez, 2015, stevestrong 2018
//  Performs a DMA SPI transfer with at least a receive buffer.
//  If a TX buffer is not provided, FF is sent over and over for the length of the transfer. 
//  On exit TX buffer is not modified, and RX buffer contains the received data.
//  Still in progress.
//-----------------------------------------------------------------------------
void SPIClass::dmaTransfer(const void *transmitBuf, void *receiveBuf, uint16 length, uint16 flags)
{
    PRINTF("<dTb-");
    dmaWaitCompletion();
    _currentSetting->dmaTxBuffer = transmitBuf;
    _currentSetting->dmaTrxLength = length;
    _currentSetting->dmaTrxAsync = (flags&DMA_ASYNC);
    dmaTransferSet(receiveBuf, (flags&(DMA_CIRC_MODE|DMA_TRNS_HALF)) | DMA_MINC_MODE);
    dmaTransferRepeat();
    PRINTF("-dTb>\n");
}

the thing is i'm not too sure if calling it with

Code: Select all

return m_spi->dmaTransfer(nullptr, buf, count);

actually transferred aribtary data across the spi MOSI starting from address location 0 while data is received.
that could have 'confused' the cards and i got zeros in the received data.

the several cards i've got works normally when it is changed to

Code: Select all

  return m_spi->dmaTransfer((const uint16_t) 0xff, buf, count);

[stevestrong]

Code: Select all

void SPIClass::dmaTransfer(const void *transmitBuf, void *receiveBuf, uint16 length, uint16 flags)
{
    PRINTF("<dTb-");
    dmaWaitCompletion();
    _currentSetting->dmaTxBuffer = transmitBuf;
    _currentSetting->dmaTrxLength = length;
    _currentSetting->dmaTrxAsync = (flags&DMA_ASYNC);
    dmaTransferSet(receiveBuf, (flags&(DMA_CIRC_MODE|DMA_TRNS_HALF)) | DMA_MINC_MODE);
    dmaTransferRepeat();
    PRINTF("-dTb>\n");
}

You're right, the case when transmitBuf (or receiveBuf) is NULL is not covered by the function, that leads to unexpected behavior.
Can you please make an issue in Roger's gihub regarding this? I will then fix it.

[ag123]

logged an issue
https://github.com/rogerclarkmelbourne/ ... issues/844
but literally transmitting from address location zero may be a valid intent. e.g. for 'debug' type apps wanting to look at data from locaiton 0
hence i'm not sure if we'd after all prohibit transfers from location zero.

maybe the condition check could be if (transmitBuf == nullptr && receiveBuf != nullptr) for the 'receive only' intent

the call

Code: Select all

dmaTransfer((const uint16_t) 0xff, buf, count);

don't have the same artifact as it simply transmits 0xff for each byte received while receiving the data in buf