SDMMC SD Card Drivers #45
Conversation
lekangwang
requested review from
Aidan-B,
antholuo,
StanleyTang17,
Chrisc110,
HardyYu,
billylin609,
SnackkOverflowError and
DerekTang04
There was a problem hiding this comment.
Changes mostly look good, especially if tested working.
I'm curious to see how it plays with freertos task scheduling and if that might cause problems with data stalling or weird interrupt issues. Might be worth trying to schedule two threads and having one thread write to the SD card while the other one does busy work?
| @@ -82,7 +82,7 @@ void MX_I2C2_Init(void) | |||
|
|
|||
| /* USER CODE END I2C2_Init 1 */ | |||
| hi2c2.Instance = I2C2; | |||
| hi2c2.Init.Timing = 0x40505681; | |||
| hi2c2.Init.Timing = 0x20505885; | |||
There was a problem hiding this comment.
Why did we need to change this timing? Or was it just a i2c configuration changed and this was part of the auto-generated files?
There was a problem hiding this comment.
I was using PLL clock for the SDMMC peripheral, and I changed the PLL freq to be slow enough for the SD card. I changed this so I'm not using PLL anymore and instead Clock48 with a clock divider factor of 8 for SDMMC peripheral.
| |GPIO_PIN_12; | ||
| GPIO_InitStruct.Mode = GPIO_MODE_AF_PP; | ||
| GPIO_InitStruct.Pull = GPIO_NOPULL; | ||
| GPIO_InitStruct.Speed = GPIO_SPEED_FREQ_VERY_HIGH; |
There was a problem hiding this comment.
Does our speed need to be very high? This tends to cause ringing and can cause bad data.
Would advise to bring it back down to FREQ_MEDIUM or FREQ_LOW.
There was a problem hiding this comment.
Changed to FREQ_MEDIUM
| HAL_GPIO_Init(GPIOD, &GPIO_InitStruct); | ||
|
|
||
| /* SDMMC1 interrupt Init */ | ||
| HAL_NVIC_SetPriority(SDMMC1_IRQn, 5, 0); |
There was a problem hiding this comment.
This isn't something that we need to consider yet, but we should probably start thinking about our task / interrupt priorities....
| @@ -46,7 +44,7 @@ void MX_LPUART1_UART_Init(void) | |||
|
|
|||
| /* USER CODE END LPUART1_Init 1 */ | |||
| hlpuart1.Instance = LPUART1; | |||
| hlpuart1.Init.BaudRate = 209700; | |||
| hlpuart1.Init.BaudRate = 230400; | |||
There was a problem hiding this comment.
This is a similar issue as the i2c, the baudrate is changed wiredly
There was a problem hiding this comment.
I did this so I could print stuff on my terminal on my laptop, will change this back later
…lib_reentrant for FREERTOS
lekangwang
force-pushed
the
feature/driver/sdmmc-sd-card
branch
from
390ec93
to
323f6f8
Compare
lekangwang
force-pushed
the
feature/driver/sdmmc-sd-card
branch
from
323f6f8
to
19ff31d
Compare
| */ | ||
| void HAL_Delay(uint32_t delay) | ||
| { | ||
| osDelay(delay); |
There was a problem hiding this comment.
HAL_Delay is not used anywhere in this driver. Overriding an existed hal function might be a little confusing. I think people should be using osDelay directly.
There was a problem hiding this comment.
its used in ZeroPilot-SW-3.5/Boardfiles/nucleol552zeq/Drivers/STM32L5xx_HAL_Driver/Src/stm32l5xx_hal_sd.c for function HAL_SD_InitCard which is called by HAL_SD_Init which is called by BSP_SD_Init which is called by SD_initialize
Description
What was completed, changed, or updated?
Why was this done (if applicable)?
Testing
What manual tests were used to validate the code?
Tested on 16 and 32 GB card
Can write the same message 100 times to a file
Reading the first line, data matches as expected
Multithreaded write to same file is handled correctly
What unit tests were used to validate the code?
Documentation
Milestone number and name: M2
Link to Asana task: https://app.asana.com/0/1203458353737758/1204645027312218/f
Link to Confluence documentation: https://uwarg-docs.atlassian.net/wiki/spaces/ZP/pages/2251620424/SDMMC+SD+Card
Reminders
Add reviewers to the PR
Mention the PR in the appropriate discord channel