MMC (eMMC) support

[barthess]

Code: Select all

bool sdcErase(SDCDriver *sdcp, uint32_t startblk, uint32_t endblk) {

Is this function really needed? It calls nowhere.

[Giovanni]

If the decode functions become standalone (with a parameter for the destination structure) then there is no more need for the debug switch. It is up to the user to use or not use the function.

About the pointer, I prefer limit dirty stuff to configuration structures and keep the API as simple as possible. Some driver implementations may also decide to have an internal buffer for that, if the RAM is not a problem. This is not yet the case for the STM32.

Edit: Let's make the HLD call the LLD in order to obtain the card size, this way the LLD can decide how to handle the buffer (configuration pointer, static, dynamic or whatever).

In general I would limit changes because 3.0 is looooong overdue.

Giovanni

[Giovanni]

Code: Select all

bool sdcErase(SDCDriver *sdcp, uint32_t startblk, uint32_t endblk) {

Is this function really needed? It calls nowhere.

Didn't you add that one or I remember wrong? it is not used anyway.

giovanni

[barthess]

Didn't you add that one or I remember wrong? it is not used anyway.

Hm... it was added when I was young and stupid. I will remove it.

Edit: Let's make the HLD call the LLD in order to obtain the card size, this way the LLD can decide how to handle the buffer (configuration pointer, static, dynamic or whatever).

I would prefer configuration pointer because:
1) Moving that stuff in LLD will cause code duplication and looks ugly in general. It is independent high level code.
2) This buffer (partly) used in SD clock detection (may be in some other procedures in future).
3)

In general I would limit changes because 3.0 is looooong overdue.

[Giovanni]

Are you telling me you are no more young?

OK, lets go simply for the pointer.

Giovanni

[barthess]

I suppose features implemented. Do you have any remarks/wishes about code?

[Giovanni]

Hi barthess,

Good work, I made some changes in sdcConnect() in order to make the buffer "less necessary", not it should be required only for MMC cards above 2GB (please verify this, I have no MMC cards).

I also made superclass functions not APIs and swapped the fields into the configuration structure because the scratchpad is mandatory, the bus size is STM32-specific.

I have a question about sdc_detect_bus_clk(), there the command is called twice, is this intentional?

Giovanni

[barthess]

I made some changes in sdcConnect() in order to make the buffer "less necessary", not it should be required only for MMC cards above 2GB (please verify this, I have no MMC cards).

Changes look correct. Tomorrow I will test it in real hardware.
We can totally avoid this buffer for SDC. It is used only for bus clock detection and only
64 bytes needed. Is it too bad to allocate uint8_t[64] on stack?

the bus size is STM32-specific.

I am surprised. Are there no other MCUs with selectable bus width?

question about sdc_detect_bus_clk(), there the command is called twice, is this intentional?

Can not find where is second call. Could you point me?

[Giovanni]

Hi,

I could live with a 64bytes buffer on the stack, usually the sdcConnect() is called from the main and the stack is available there.

About the bus width, couldn't some low level driver do that automatically?

About the code I don't understand much, this one:

Code: Select all

static bool sdc_detect_bus_clk(SDCDriver *sdcp, sdcbusclk_t *clk) {
  uint32_t cmdarg = 0;
  uint8_t *scratchpad = sdcp->config->scratchpad;

  /* Safe default.*/
  *clk = SDC_CLK_25MHz;

  /* Use safe default when there is no space for data.*/
  if (NULL == scratchpad)
    return HAL_SUCCESS;

  if (sdc_lld_read_special(sdcp, scratchpad, 64, MMCSD_CMD_SWITCH, cmdarg))
    return HAL_FAILED;

  if ((sdc_cmd6_extract_info(SD_SWITCH_FUNCTION_SPEED, scratchpad) & 2) == 2) {
    cmdarg = sdc_cmd6_construct(SD_SWITCH_SET, SD_SWITCH_FUNCTION_SPEED, 1);
    if (sdc_lld_read_special(sdcp, scratchpad, 64, MMCSD_CMD_SWITCH, cmdarg))
      return HAL_FAILED;
    if (HAL_SUCCESS == sdc_cmd6_check_status(SD_SWITCH_FUNCTION_SPEED, scratchpad))
      *clk = SDC_CLK_50MHz;
  }

  return HAL_SUCCESS;
}

The function sdc_lld_read_special() is called twich with the same parameters, is it necessary to read that buffer again?

Giovanni

[barthess]

I could live with a 64bytes buffer on the stack, usually the sdcConnect() is called from the main and the stack is available there.

Agreed. It usually called from main() or from fat32_storage() thread so it has a lot of stack space.

About the bus width, couldn't some low level driver do that automatically?

STM32 has some bus check abilities but I am not sure about its width detection suitability. Lets it be
platform specific.

About the code I don't understand much

I hate this hand waiving bitfucking but this is how SDC provides bus clock speed.