Skip to content

Conversation

@B3W
Copy link

@B3W B3W commented Jul 25, 2025

Closes #26

Updated this port for the STM32F746G-DISCO to work with LVGL v9.3.0. This included updates to the following components:

  • STM32F7 BSP sources were updated to the latest F7 firmware package provided by ST (v1.17.3)
  • CMSIS sources were updated to the latest F7 firmware package provided by ST (v1.17.3)
  • Several other ST generated pieces (HAL layer, interrupt handling, etc.) also updated using the latest STM32CubeMX (v6.14.1)
  • IAR project updated to IAR v9.60.4
  • STM32CubeIDE project updated to STM32CubeIDE 1.18.1
  • CMake project updated to align with changes

B3W added 21 commits June 11, 2025 20:00
…river. Updated TFT driver to utilize ST LTDC driver. Updated some naming.
…ion to utilize relative path for executable.
…e endings. Can cause trouble when using Docker on Windows if you don't do this.
…nvironment uses GCC10 (gcc-arm-none-eabi-10-2020-q4-major).
Copy link

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

17 issues found across 330 files

Note: This PR contains a large number of files. cubic only reviews up to 150 files per PR, so some files may not have been reviewed.

Prompt for AI agents (all 17 issues)

Understand the root cause of the following 17 issues and fix them.


<file name="inc/usart.h">

<violation number="1" location="inc/usart.h:21">
Rename the include guard macro to a project-specific name that does not start with double underscores (e.g., USART_H or LVGL_USART_H) to avoid violating the C/C++ reserved identifier rules.</violation>
</file>

<file name="src/sdmmc.c">

<violation number="1" location="src/sdmmc.c:45">
Initialize the SD card in 1‑bit mode before widening the bus so that `HAL_SD_Init` follows the required SD card bring-up sequence.</violation>
</file>

<file name="HAL_Driver/Src/stm32f7xx_hal_sram.c">

<violation number="1" location="HAL_Driver/Src/stm32f7xx_hal_sram.c:682">
If HAL_DMA_Start_IT fails, hsram-&gt;State remains HAL_SRAM_STATE_BUSY and the driver can never be used again. Restore the previous state before returning the error so the caller can retry.</violation>

<violation number="2" location="HAL_Driver/Src/stm32f7xx_hal_sram.c:723">
HAL_SRAM_Write_DMA leaves the handle in HAL_SRAM_STATE_BUSY whenever HAL_DMA_Start_IT fails, so subsequent transfers are blocked forever. Revert the state to READY before returning on failure.</violation>
</file>

<file name="CMSIS/core/mpu_armv7.h">

<violation number="1" location="CMSIS/core/mpu_armv7.h:106">
`ARM_MPU_RASR_EX` drops the sub-region, size, and enable fields, so every MPU region configured with this macro remains size zero and disabled.</violation>
</file>

<file name="CMSIS/core/tz_context.h">

<violation number="1" location="CMSIS/core/tz_context.h:47">
Declare these TrustZone context APIs inside an `extern &quot;C&quot;` block so C++ sources include them with C linkage and can link against the secure C implementations.</violation>
</file>

<file name="src/usb_otg.c">

<violation number="1" location="src/usb_otg.c:47">
OTG FS transceiver power (VDDUSB) is never enabled, so USB host cannot function</violation>
</file>

<file name="HAL_Driver/Src/stm32f7xx_hal_spi_ex.c">

<violation number="1" location="HAL_Driver/Src/stm32f7xx_hal_spi_ex.c:88">
HAL_SPIEx_FlushRxFifo returns HAL_TIMEOUT even for a normal flush when the FIFO initially contains all 4 entries, because the guard triggers on the 4th legitimate read instead of only when excess iterations occur.</violation>
</file>

<file name="src/sai.c">

<violation number="1" location="src/sai.c:70">
Block A disables every SAI slot by setting SlotActive to 0, so the transmitter will never output audio data. Select at least one active slot (e.g., SAI_SLOTACTIVE_0) to match the configured SlotNumber.</violation>

<violation number="2" location="src/sai.c:96">
Block B also leaves SlotActive at 0, so no slot ever becomes active and the receiver cannot capture audio samples. Enable the slot (e.g., SAI_SLOTACTIVE_0) to match the configured slot count.</violation>
</file>

<file name="HAL_Driver/Src/stm32f7xx_hal_sdram.c">

<violation number="1" location="HAL_Driver/Src/stm32f7xx_hal_sdram.c:732">
If HAL_DMA_Start_IT fails in HAL_SDRAM_Read_DMA the handle stays stuck in HAL_SDRAM_STATE_BUSY, blocking all later SDRAM operations.</violation>

<violation number="2" location="HAL_Driver/Src/stm32f7xx_hal_sdram.c:777">
HAL_SDRAM_Write_DMA never restores the SDRAM state when HAL_DMA_Start_IT fails, leaving the handle permanently BUSY.</violation>
</file>

<file name="CMSIS/core/cmsis_armclang.h">

<violation number="1" location="CMSIS/core/cmsis_armclang.h:1855">
`__PKHTB` emulation uses a logical right shift, so negative values lose the required sign extension and the macro no longer matches the PKHTB instruction semantics.</violation>
</file>

<file name="HAL_Driver/Src/stm32f7xx_hal_uart_ex.c">

<violation number="1" location="HAL_Driver/Src/stm32f7xx_hal_uart_ex.c:425">
If HAL_UARTEx_StopModeWakeUpSourceConfig times out waiting for REACK it never resets huart-&gt;gState to READY, leaving the UART handle stuck in BUSY and preventing any further UART API usage until a full re-init.</violation>
</file>

<file name="HAL_Driver/Src/stm32f7xx_hal_spdifrx.c">

<violation number="1" location="HAL_Driver/Src/stm32f7xx_hal_spdifrx.c:181">
`SPDIFRX_TIMEOUT_VALUE` was reduced from 0xFFFF to 10U, making the internal wait loops abort almost immediately so interrupt/DMA receive start-up now always times out even under normal conditions.</violation>

<violation number="2" location="HAL_Driver/Src/stm32f7xx_hal_spdifrx.c:1287">
`HAL_SPDIFRX_IRQHandler` no longer clears the OVR/PERR status flags because it writes interrupt-enable bits into IFCR instead of the required `SPDIFRX_FLAG_*` masks.</violation>
</file>

<file name="CMSIS/core/cmsis_iccarm.h">

<violation number="1" location="CMSIS/core/cmsis_iccarm.h:588">
`__ROR` shifts by `sizeof(op1)*8` bits when the rotation count is 0 or ≥32, which is undefined behavior and breaks rotations that ARM treats as legal. Mask the rotation count before shifting.</violation>
</file>

Since this is your first cubic review, here's how it works:

  • cubic automatically reviews your code and comments on bugs and improvements
  • Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
  • Ask questions if you need clarification on any suggestion

Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR

*/
/* USER CODE END Header */
/* Define to prevent recursive inclusion -------------------------------------*/
#ifndef __USART_H__
Copy link

@cubic-dev-ai cubic-dev-ai bot Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rename the include guard macro to a project-specific name that does not start with double underscores (e.g., USART_H or LVGL_USART_H) to avoid violating the C/C++ reserved identifier rules.

Prompt for AI agents
Address the following comment on inc/usart.h at line 21:

<comment>Rename the include guard macro to a project-specific name that does not start with double underscores (e.g., USART_H or LVGL_USART_H) to avoid violating the C/C++ reserved identifier rules.</comment>

<file context>
@@ -0,0 +1,55 @@
+  */
+/* USER CODE END Header */
+/* Define to prevent recursive inclusion -------------------------------------*/
+#ifndef __USART_H__
+#define __USART_H__
+
</file context>
Fix with Cubic

hsd1.Init.ClockEdge = SDMMC_CLOCK_EDGE_RISING;
hsd1.Init.ClockBypass = SDMMC_CLOCK_BYPASS_DISABLE;
hsd1.Init.ClockPowerSave = SDMMC_CLOCK_POWER_SAVE_DISABLE;
hsd1.Init.BusWide = SDMMC_BUS_WIDE_4B;
Copy link

@cubic-dev-ai cubic-dev-ai bot Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initialize the SD card in 1‑bit mode before widening the bus so that HAL_SD_Init follows the required SD card bring-up sequence.

Prompt for AI agents
Address the following comment on src/sdmmc.c at line 45:

<comment>Initialize the SD card in 1‑bit mode before widening the bus so that `HAL_SD_Init` follows the required SD card bring-up sequence.</comment>

<file context>
@@ -0,0 +1,137 @@
+  hsd1.Init.ClockEdge = SDMMC_CLOCK_EDGE_RISING;
+  hsd1.Init.ClockBypass = SDMMC_CLOCK_BYPASS_DISABLE;
+  hsd1.Init.ClockPowerSave = SDMMC_CLOCK_POWER_SAVE_DISABLE;
+  hsd1.Init.BusWide = SDMMC_BUS_WIDE_4B;
+  hsd1.Init.HardwareFlowControl = SDMMC_HARDWARE_FLOW_CONTROL_DISABLE;
+  hsd1.Init.ClockDiv = 0;
</file context>
Fix with Cubic

hsram->hdma->XferErrorCallback = SRAM_DMAError;

/* Enable the DMA Stream */
status = HAL_DMA_Start_IT(hsram->hdma, (uint32_t)pSrcBuffer, (uint32_t)pAddress, (uint32_t)BufferSize);
Copy link

@cubic-dev-ai cubic-dev-ai bot Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HAL_SRAM_Write_DMA leaves the handle in HAL_SRAM_STATE_BUSY whenever HAL_DMA_Start_IT fails, so subsequent transfers are blocked forever. Revert the state to READY before returning on failure.

Prompt for AI agents
Address the following comment on HAL_Driver/Src/stm32f7xx_hal_sram.c at line 723:

<comment>HAL_SRAM_Write_DMA leaves the handle in HAL_SRAM_STATE_BUSY whenever HAL_DMA_Start_IT fails, so subsequent transfers are blocked forever. Revert the state to READY before returning on failure.</comment>

<file context>
@@ -1,688 +1,1115 @@
+    hsram-&gt;hdma-&gt;XferErrorCallback = SRAM_DMAError;
+
+    /* Enable the DMA Stream */
+    status = HAL_DMA_Start_IT(hsram-&gt;hdma, (uint32_t)pSrcBuffer, (uint32_t)pAddress, (uint32_t)BufferSize);
+
+    /* Process unlocked */
</file context>
Fix with Cubic

hsram->hdma->XferErrorCallback = SRAM_DMAError;

/* Enable the DMA Stream */
status = HAL_DMA_Start_IT(hsram->hdma, (uint32_t)pAddress, (uint32_t)pDstBuffer, (uint32_t)BufferSize);
Copy link

@cubic-dev-ai cubic-dev-ai bot Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If HAL_DMA_Start_IT fails, hsram->State remains HAL_SRAM_STATE_BUSY and the driver can never be used again. Restore the previous state before returning the error so the caller can retry.

Prompt for AI agents
Address the following comment on HAL_Driver/Src/stm32f7xx_hal_sram.c at line 682:

<comment>If HAL_DMA_Start_IT fails, hsram-&gt;State remains HAL_SRAM_STATE_BUSY and the driver can never be used again. Restore the previous state before returning the error so the caller can retry.</comment>

<file context>
@@ -1,688 +1,1115 @@
+    hsram-&gt;hdma-&gt;XferErrorCallback = SRAM_DMAError;
+
+    /* Enable the DMA Stream */
+    status = HAL_DMA_Start_IT(hsram-&gt;hdma, (uint32_t)pAddress, (uint32_t)pDstBuffer, (uint32_t)BufferSize);
+
+    /* Process unlocked */
</file context>
Fix with Cubic

#define ARM_MPU_RASR_EX(DisableExec, AccessPermission, AccessAttributes, SubRegionDisable, Size) \
((((DisableExec ) << MPU_RASR_XN_Pos) & MPU_RASR_XN_Msk) | \
(((AccessPermission) << MPU_RASR_AP_Pos) & MPU_RASR_AP_Msk) | \
(((AccessAttributes) ) & (MPU_RASR_TEX_Msk | MPU_RASR_S_Msk | MPU_RASR_C_Msk | MPU_RASR_B_Msk)))
Copy link

@cubic-dev-ai cubic-dev-ai bot Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ARM_MPU_RASR_EX drops the sub-region, size, and enable fields, so every MPU region configured with this macro remains size zero and disabled.

Prompt for AI agents
Address the following comment on CMSIS/core/mpu_armv7.h at line 106:

<comment>`ARM_MPU_RASR_EX` drops the sub-region, size, and enable fields, so every MPU region configured with this macro remains size zero and disabled.</comment>

<file context>
@@ -0,0 +1,270 @@
+#define ARM_MPU_RASR_EX(DisableExec, AccessPermission, AccessAttributes, SubRegionDisable, Size)      \
+  ((((DisableExec ) &lt;&lt; MPU_RASR_XN_Pos) &amp; MPU_RASR_XN_Msk)                                          | \
+   (((AccessPermission) &lt;&lt; MPU_RASR_AP_Pos) &amp; MPU_RASR_AP_Msk)                                      | \
+   (((AccessAttributes) ) &amp; (MPU_RASR_TEX_Msk | MPU_RASR_S_Msk | MPU_RASR_C_Msk | MPU_RASR_B_Msk)))
+  
+/**
</file context>
Fix with Cubic

((((uint32_t)(ARG2)) << (ARG3)) & 0xFFFF0000UL) )

#define __PKHTB(ARG1,ARG2,ARG3) ( ((((uint32_t)(ARG1)) ) & 0xFFFF0000UL) | \
((((uint32_t)(ARG2)) >> (ARG3)) & 0x0000FFFFUL) )
Copy link

@cubic-dev-ai cubic-dev-ai bot Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

__PKHTB emulation uses a logical right shift, so negative values lose the required sign extension and the macro no longer matches the PKHTB instruction semantics.

Prompt for AI agents
Address the following comment on CMSIS/core/cmsis_armclang.h at line 1855:

<comment>`__PKHTB` emulation uses a logical right shift, so negative values lose the required sign extension and the macro no longer matches the PKHTB instruction semantics.</comment>

<file context>
@@ -0,0 +1,1869 @@
+                                           ((((uint32_t)(ARG2)) &lt;&lt; (ARG3)) &amp; 0xFFFF0000UL)  )
+
+#define __PKHTB(ARG1,ARG2,ARG3)          ( ((((uint32_t)(ARG1))          ) &amp; 0xFFFF0000UL) |  \
+                                           ((((uint32_t)(ARG2)) &gt;&gt; (ARG3)) &amp; 0x0000FFFFUL)  )
+
+__STATIC_FORCEINLINE int32_t __SMMLA (int32_t op1, int32_t op2, int32_t op3)
</file context>
Suggested change
((((uint32_t)(ARG2)) >> (ARG3)) & 0x0000FFFFUL) )
((((uint32_t)((int32_t)(ARG2) >> (ARG3))) ) & 0x0000FFFFUL) )
Fix with Cubic

/* Wait until REACK flag is set */
if (UART_WaitOnFlagUntilTimeout(huart, USART_ISR_REACK, RESET, tickstart, HAL_UART_TIMEOUT_VALUE) != HAL_OK)
{
status = HAL_TIMEOUT;
Copy link

@cubic-dev-ai cubic-dev-ai bot Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If HAL_UARTEx_StopModeWakeUpSourceConfig times out waiting for REACK it never resets huart->gState to READY, leaving the UART handle stuck in BUSY and preventing any further UART API usage until a full re-init.

Prompt for AI agents
Address the following comment on HAL_Driver/Src/stm32f7xx_hal_uart_ex.c at line 425:

<comment>If HAL_UARTEx_StopModeWakeUpSourceConfig times out waiting for REACK it never resets huart-&gt;gState to READY, leaving the UART handle stuck in BUSY and preventing any further UART API usage until a full re-init.</comment>

<file context>
@@ -0,0 +1,792 @@
+  /* Wait until REACK flag is set */
+  if (UART_WaitOnFlagUntilTimeout(huart, USART_ISR_REACK, RESET, tickstart, HAL_UART_TIMEOUT_VALUE) != HAL_OK)
+  {
+    status = HAL_TIMEOUT;
+  }
+  else
</file context>
Fix with Cubic

/** @defgroup SPDIFRX_Private_Defines SPDIFRX Private Defines
* @{
*/
#define SPDIFRX_TIMEOUT_VALUE 10U
Copy link

@cubic-dev-ai cubic-dev-ai bot Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SPDIFRX_TIMEOUT_VALUE was reduced from 0xFFFF to 10U, making the internal wait loops abort almost immediately so interrupt/DMA receive start-up now always times out even under normal conditions.

Prompt for AI agents
Address the following comment on HAL_Driver/Src/stm32f7xx_hal_spdifrx.c at line 181:

<comment>`SPDIFRX_TIMEOUT_VALUE` was reduced from 0xFFFF to 10U, making the internal wait loops abort almost immediately so interrupt/DMA receive start-up now always times out even under normal conditions.</comment>

<file context>
@@ -1,1256 +1,1647 @@
+/** @defgroup SPDIFRX_Private_Defines SPDIFRX Private Defines
+  * @{
+  */
+#define SPDIFRX_TIMEOUT_VALUE  10U
+/**
+  * @}
</file context>
Suggested change
#define SPDIFRX_TIMEOUT_VALUE 10U
#define SPDIFRX_TIMEOUT_VALUE 0xFFFFU
Fix with Cubic

/* SPDIFRX Overrun error interrupt occurred */
if (((itFlag & SPDIFRX_FLAG_OVR) == SPDIFRX_FLAG_OVR) && ((itSource & SPDIFRX_IT_OVRIE) == SPDIFRX_IT_OVRIE))
{
__HAL_SPDIFRX_CLEAR_IT(hspdif, SPDIFRX_IT_OVRIE);
Copy link

@cubic-dev-ai cubic-dev-ai bot Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HAL_SPDIFRX_IRQHandler no longer clears the OVR/PERR status flags because it writes interrupt-enable bits into IFCR instead of the required SPDIFRX_FLAG_* masks.

Prompt for AI agents
Address the following comment on HAL_Driver/Src/stm32f7xx_hal_spdifrx.c at line 1287:

<comment>`HAL_SPDIFRX_IRQHandler` no longer clears the OVR/PERR status flags because it writes interrupt-enable bits into IFCR instead of the required `SPDIFRX_FLAG_*` masks.</comment>

<file context>
@@ -1,1256 +1,1647 @@
+  /* SPDIFRX Overrun error interrupt occurred */
+  if (((itFlag &amp; SPDIFRX_FLAG_OVR) == SPDIFRX_FLAG_OVR) &amp;&amp; ((itSource &amp;  SPDIFRX_IT_OVRIE) == SPDIFRX_IT_OVRIE))
+  {
+    __HAL_SPDIFRX_CLEAR_IT(hspdif, SPDIFRX_IT_OVRIE);
+
+    /* Change the SPDIFRX error code */
</file context>
Suggested change
__HAL_SPDIFRX_CLEAR_IT(hspdif, SPDIFRX_IT_OVRIE);
__HAL_SPDIFRX_CLEAR_IT(hspdif, SPDIFRX_FLAG_OVR);
Fix with Cubic


__IAR_FT uint32_t __ROR(uint32_t op1, uint32_t op2)
{
return (op1 >> op2) | (op1 << ((sizeof(op1)*8)-op2));
Copy link

@cubic-dev-ai cubic-dev-ai bot Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

__ROR shifts by sizeof(op1)*8 bits when the rotation count is 0 or ≥32, which is undefined behavior and breaks rotations that ARM treats as legal. Mask the rotation count before shifting.

Prompt for AI agents
Address the following comment on CMSIS/core/cmsis_iccarm.h at line 588:

<comment>`__ROR` shifts by `sizeof(op1)*8` bits when the rotation count is 0 or ≥32, which is undefined behavior and breaks rotations that ARM treats as legal. Mask the rotation count before shifting.</comment>

<file context>
@@ -0,0 +1,935 @@
+
+  __IAR_FT uint32_t __ROR(uint32_t op1, uint32_t op2)
+  {
+    return (op1 &gt;&gt; op2) | (op1 &lt;&lt; ((sizeof(op1)*8)-op2));
+  }
+
</file context>
Suggested change
return (op1 >> op2) | (op1 << ((sizeof(op1)*8)-op2));
return (op1 >> (op2 & 31U)) | (op1 << ((32U - (op2 & 31U)) & 31U));
Fix with Cubic

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update port to LVGL v9.3.0

1 participant