Page 2 of 4
Re: MMC (eMMC) support
Posted: Wed Feb 18, 2015 8:40 pm
by barthess
Code: Select all
bool sdcErase(SDCDriver *sdcp, uint32_t startblk, uint32_t endblk) {
Is this function really needed? It calls nowhere.
Re: MMC (eMMC) support
Posted: Wed Feb 18, 2015 8:42 pm
by 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
Re: MMC (eMMC) support
Posted: Wed Feb 18, 2015 8:42 pm
by Giovanni
barthess wrote: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
Re: MMC (eMMC) support
Posted: Wed Feb 18, 2015 9:13 pm
by barthess
Giovanni wrote: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.
Giovanni wrote: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)
Giovanni wrote:In general I would limit changes because 3.0 is looooong overdue.
Re: MMC (eMMC) support
Posted: Wed Feb 18, 2015 9:18 pm
by Giovanni
Are you telling me you are no more young?
OK, lets go simply for the pointer.
Giovanni
Re: MMC (eMMC) support
Posted: Thu Feb 19, 2015 6:52 pm
by barthess
I suppose features implemented. Do you have any remarks/wishes about code?
Re: MMC (eMMC) support
Posted: Mon Feb 23, 2015 3:28 pm
by 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
Re: MMC (eMMC) support
Posted: Mon Feb 23, 2015 6:15 pm
by barthess
Giovanni wrote: 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?
Giovanni wrote:the bus size is STM32-specific.
I am surprised. Are there no other MCUs with selectable bus width?
Giovanni wrote: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?
Re: MMC (eMMC) support
Posted: Mon Feb 23, 2015 6:53 pm
by 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
Re: MMC (eMMC) support
Posted: Mon Feb 23, 2015 7:39 pm
by barthess
Giovanni wrote: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.
Giovanni wrote: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.
Giovanni wrote:About the code I don't understand much
I hate this hand waiving bitfucking but this is how SDC provides bus clock speed.