From 84d1ddfeba5087174419f148ba17c25fdf811bc2 Mon Sep 17 00:00:00 2001 From: Etienne Carriere Date: Thu, 9 Oct 2025 11:03:41 +0200 Subject: [PATCH 1/3] drivers: disk: sdmmc_stm32: don't mix HAL return values and error Ensure STM32 SDMMC driver returns proper errno values and not HAL return codes. Signed-off-by: Etienne Carriere --- drivers/disk/sdmmc_stm32.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/drivers/disk/sdmmc_stm32.c b/drivers/disk/sdmmc_stm32.c index 0fc94d1e2e7d5..98532d01d9acd 100644 --- a/drivers/disk/sdmmc_stm32.c +++ b/drivers/disk/sdmmc_stm32.c @@ -449,9 +449,11 @@ static int stm32_sdmmc_access_init(struct disk_info *disk) static int stm32_sdmmc_access_deinit(struct stm32_sdmmc_priv *priv) { - int err = 0; + HAL_StatusTypeDef hal_ret; #if STM32_SDMMC_USE_DMA + int err; + err = stm32_sdmmc_dma_deinit(priv); if (err) { LOG_ERR("DMA deinit failed"); @@ -460,14 +462,14 @@ static int stm32_sdmmc_access_deinit(struct stm32_sdmmc_priv *priv) #endif #if defined(CONFIG_SDMMC_STM32_EMMC) - err = HAL_MMC_DeInit(&priv->hsd); + hal_ret = HAL_MMC_DeInit(&priv->hsd); #else - err = HAL_SD_DeInit(&priv->hsd); + hal_ret = HAL_SD_DeInit(&priv->hsd); stm32_sdmmc_clock_disable(priv); #endif - if (err != HAL_OK) { + if (hal_ret != HAL_OK) { LOG_ERR("failed to deinit stm32_sdmmc (ErrorCode 0x%X)", priv->hsd.ErrorCode); - return err; + return -EIO; } #if !defined(CONFIG_SDMMC_STM32_EMMC) From d5692691e89ec7940fb456507e8447b1c2f6571a Mon Sep 17 00:00:00 2001 From: Etienne Carriere Date: Fri, 10 Oct 2025 11:49:15 +0200 Subject: [PATCH 2/3] drivers: disk: sdmmc_stm32: don't assume HAL return value is an int Clarify HAL return value is of type HAL_StatusTypeDef and may not be a int. This change aims preventing one from mixing standard "errno" int return values and STM32 HAL return value finding misleading implementation in existing code. No functional change. Signed-off-by: Etienne Carriere --- drivers/disk/sdmmc_stm32.c | 73 +++++++++++++++++++++++--------------- 1 file changed, 44 insertions(+), 29 deletions(-) diff --git a/drivers/disk/sdmmc_stm32.c b/drivers/disk/sdmmc_stm32.c index 98532d01d9acd..a562cf909d2bb 100644 --- a/drivers/disk/sdmmc_stm32.c +++ b/drivers/disk/sdmmc_stm32.c @@ -361,6 +361,7 @@ static int stm32_sdmmc_access_init(struct disk_info *disk) { const struct device *dev = disk->dev; struct stm32_sdmmc_priv *priv = dev->data; + HAL_StatusTypeDef hal_ret; int err; err = stm32_sdmmc_pwr_on(priv); @@ -412,11 +413,11 @@ static int stm32_sdmmc_access_init(struct disk_info *disk) } #ifdef CONFIG_SDMMC_STM32_EMMC - err = HAL_MMC_Init(&priv->hsd); + hal_ret = HAL_MMC_Init(&priv->hsd); #else - err = HAL_SD_Init(&priv->hsd); + hal_ret = HAL_SD_Init(&priv->hsd); #endif - if (err != HAL_OK) { + if (hal_ret != HAL_OK) { LOG_ERR("failed to init stm32_sdmmc (ErrorCode 0x%X)", priv->hsd.ErrorCode); err = -EIO; goto error; @@ -424,8 +425,8 @@ static int stm32_sdmmc_access_init(struct disk_info *disk) if (SDMMC_BUS_WIDTH != SDMMC_BUS_WIDE_1B) { priv->hsd.Init.BusWide = SDMMC_BUS_WIDTH; - err = HAL_SD_ConfigWideBusOperation(&priv->hsd, priv->hsd.Init.BusWide); - if (err != HAL_OK) { + hal_ret = HAL_SD_ConfigWideBusOperation(&priv->hsd, priv->hsd.Init.BusWide); + if (hal_ret != HAL_OK) { LOG_ERR("failed to configure wide bus operation (ErrorCode 0x%X)", priv->hsd.ErrorCode); err = -EIO; @@ -501,23 +502,32 @@ static int stm32_sdmmc_is_card_in_transfer(HandleTypeDef *hsd) static int stm32_sdmmc_read_blocks(HandleTypeDef *hsd, uint8_t *data_buf, uint32_t start_sector, uint32_t num_sector) { + HAL_StatusTypeDef hal_ret; + #if STM32_SDMMC_USE_DMA || IS_ENABLED(DT_PROP(DT_DRV_INST(0), idma)) #ifdef CONFIG_SDMMC_STM32_EMMC - return HAL_MMC_ReadBlocks_DMA(hsd, data_buf, start_sector, num_sector); + hal_ret = HAL_MMC_ReadBlocks_DMA(hsd, data_buf, start_sector, num_sector); #else - return HAL_SD_ReadBlocks_DMA(hsd, data_buf, start_sector, num_sector); + hal_ret = HAL_SD_ReadBlocks_DMA(hsd, data_buf, start_sector, num_sector); #endif -#else +#else /* STM32_SDMMC_USE_DMA || IS_ENABLED(DT_PROP(DT_DRV_INST(0), idma)) */ #ifdef CONFIG_SDMMC_STM32_EMMC - return HAL_MMC_ReadBlocks_IT(hsd, data_buf, start_sector, num_sector); + hal_ret = HAL_MMC_ReadBlocks_IT(hsd, data_buf, start_sector, num_sector); #else - return HAL_SD_ReadBlocks_IT(hsd, data_buf, start_sector, num_sector); + hal_ret = HAL_SD_ReadBlocks_IT(hsd, data_buf, start_sector, num_sector); #endif -#endif +#endif /* STM32_SDMMC_USE_DMA || IS_ENABLED(DT_PROP(DT_DRV_INST(0), idma)) */ + + if (hal_ret != HAL_OK) { + LOG_ERR("sd read block failed %d", hal_ret); + return -EIO; + } + + return 0; } static int stm32_sdmmc_access_read(struct disk_info *disk, uint8_t *data_buf, @@ -539,9 +549,7 @@ static int stm32_sdmmc_access_read(struct disk_info *disk, uint8_t *data_buf, #endif err = stm32_sdmmc_read_blocks(&priv->hsd, data_buf, start_sector, num_sector); - if (err != HAL_OK) { - LOG_ERR("sd read block failed %d", err); - err = -EIO; + if (err != 0) { goto end; } @@ -569,23 +577,32 @@ static int stm32_sdmmc_write_blocks(HandleTypeDef *hsd, uint8_t *data_buf, uint32_t start_sector, uint32_t num_sector) { + HAL_StatusTypeDef hal_ret; + #if STM32_SDMMC_USE_DMA || IS_ENABLED(DT_PROP(DT_DRV_INST(0), idma)) #ifdef CONFIG_SDMMC_STM32_EMMC - return HAL_MMC_WriteBlocks_DMA(hsd, data_buf, start_sector, num_sector); + hal_ret = HAL_MMC_WriteBlocks_DMA(hsd, data_buf, start_sector, num_sector); #else - return HAL_SD_WriteBlocks_DMA(hsd, data_buf, start_sector, num_sector); + hal_ret = HAL_SD_WriteBlocks_DMA(hsd, data_buf, start_sector, num_sector); #endif -#else +#else /* STM32_SDMMC_USE_DMA || IS_ENABLED(DT_PROP(DT_DRV_INST(0), idma)) */ #ifdef CONFIG_SDMMC_STM32_EMMC - return HAL_MMC_WriteBlocks_IT(hsd, data_buf, start_sector, num_sector); + hal_ret = HAL_MMC_WriteBlocks_IT(hsd, data_buf, start_sector, num_sector); #else - return HAL_SD_WriteBlocks_IT(hsd, data_buf, start_sector, num_sector); + hal_ret = HAL_SD_WriteBlocks_IT(hsd, data_buf, start_sector, num_sector); #endif -#endif +#endif /* STM32_SDMMC_USE_DMA || IS_ENABLED(DT_PROP(DT_DRV_INST(0), idma)) */ + + if (hal_ret != HAL_OK) { + LOG_ERR("sd write block failed %d", hal_ret); + return -EIO; + } + + return 0; } static int stm32_sdmmc_access_write(struct disk_info *disk, @@ -608,9 +625,7 @@ static int stm32_sdmmc_access_write(struct disk_info *disk, #endif err = stm32_sdmmc_write_blocks(&priv->hsd, (uint8_t *)data_buf, start_sector, num_sector); - if (err != HAL_OK) { - LOG_ERR("sd write block failed %d", err); - err = -EIO; + if (err != 0) { goto end; } @@ -637,9 +652,9 @@ static int stm32_sdmmc_access_write(struct disk_info *disk, static int stm32_sdmmc_get_card_info(HandleTypeDef *hsd, CardInfoTypeDef *info) { #ifdef CONFIG_SDMMC_STM32_EMMC - return HAL_MMC_GetCardInfo(hsd, info); + return (HAL_MMC_GetCardInfo(hsd, info) == HAL_OK) ? 0 : -EIO; #else - return HAL_SD_GetCardInfo(hsd, info); + return (HAL_SD_GetCardInfo(hsd, info) == HAL_OK) ? 0 : -EIO; #endif } @@ -654,15 +669,15 @@ static int stm32_sdmmc_access_ioctl(struct disk_info *disk, uint8_t cmd, switch (cmd) { case DISK_IOCTL_GET_SECTOR_COUNT: err = stm32_sdmmc_get_card_info(&priv->hsd, &info); - if (err != HAL_OK) { - return -EIO; + if (err != 0) { + return err; } *(uint32_t *)buff = info.LogBlockNbr; break; case DISK_IOCTL_GET_SECTOR_SIZE: err = stm32_sdmmc_get_card_info(&priv->hsd, &info); - if (err != HAL_OK) { - return -EIO; + if (err != 0) { + return err; } *(uint32_t *)buff = info.LogBlockSize; break; From 63937bc4b151bee186773f7af05188f92c1e3cce Mon Sep 17 00:00:00 2001 From: Etienne Carriere Date: Thu, 9 Oct 2025 11:04:57 +0200 Subject: [PATCH 3/3] drivers: disk: sdmmc_stm32: test HAL init/deinit return values Add tests of the value returned by initialization and deinitialization HAL functions. Signed-off-by: Etienne Carriere --- drivers/disk/sdmmc_stm32.c | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/drivers/disk/sdmmc_stm32.c b/drivers/disk/sdmmc_stm32.c index a562cf909d2bb..38ae227de6117 100644 --- a/drivers/disk/sdmmc_stm32.c +++ b/drivers/disk/sdmmc_stm32.c @@ -300,7 +300,9 @@ static int stm32_sdmmc_dma_init(struct stm32_sdmmc_priv *priv) return err; } __HAL_LINKDMA(&priv->hsd, hdmatx, priv->dma_tx_handle); - HAL_DMA_Init(&priv->dma_tx_handle); + if (HAL_DMA_Init(&priv->dma_tx_handle) != HAL_OK) { + return -EIO; + } err = stm32_sdmmc_configure_dma(&priv->dma_rx_handle, &priv->dma_rx); if (err) { @@ -308,7 +310,9 @@ static int stm32_sdmmc_dma_init(struct stm32_sdmmc_priv *priv) return err; } __HAL_LINKDMA(&priv->hsd, hdmarx, priv->dma_rx_handle); - HAL_DMA_Init(&priv->dma_rx_handle); + if (HAL_DMA_Init(&priv->dma_rx_handle) != HAL_OK) { + return -EIO; + } #endif /* STM32_SDMMC_USE_DMA_SHARED */ return err; @@ -332,14 +336,17 @@ static int stm32_sdmmc_dma_deinit(struct stm32_sdmmc_priv *priv) #else struct sdmmc_dma_stream *dma_tx = &priv->dma_tx; struct sdmmc_dma_stream *dma_rx = &priv->dma_rx; + HAL_StatusTypeDef __maybe_unused hal_ret; ret = dma_stop(dma_tx->dev, dma_tx->channel); __ASSERT(ret == 0, "TX DMA channel index corrupted"); - HAL_DMA_DeInit(&priv->dma_tx_handle); + hal_ret = HAL_DMA_DeInit(&priv->dma_tx_handle); + __ASSERT_NO_MSG(hal_ret == HAL_OK); ret = dma_stop(dma_rx->dev, dma_rx->channel); __ASSERT(ret == 0, "RX DMA channel index corrupted"); - HAL_DMA_DeInit(&priv->dma_rx_handle); + hal_ret = HAL_DMA_DeInit(&priv->dma_rx_handle); + __ASSERT_NO_MSG(hal_ret == HAL_OK); #endif return 0; } @@ -556,7 +563,10 @@ static int stm32_sdmmc_access_read(struct disk_info *disk, uint8_t *data_buf, k_sem_take(&priv->sync, K_FOREVER); #if STM32_SDMMC_USE_DMA_SHARED - HAL_DMA_DeInit(&priv->dma_txrx_handle); + if (HAL_DMA_DeInit(&priv->dma_txrx_handle) != HAL_OK) { + err = -EIO; + goto end; + } #endif if (priv->status != DISK_STATUS_OK) { @@ -632,7 +642,11 @@ static int stm32_sdmmc_access_write(struct disk_info *disk, k_sem_take(&priv->sync, K_FOREVER); #if STM32_SDMMC_USE_DMA_SHARED - HAL_DMA_DeInit(&priv->dma_txrx_handle); + if (HAL_DMA_DeInit(&priv->dma_txrx_handle) != HAL_OK) { + LOG_ERR("DMA deinit error"); + err = -EIO; + goto end; + } #endif if (priv->status != DISK_STATUS_OK) {